Skip to content

Commit 33bea39

Browse files
toniheimicrokatz
tonihei
authored and
microkatz
committed
Ensure listener invocations use final state variable.
The MediaControllerImplBase listener invocations currently use the class member state that can change if one of the listener method implementations changes the state recursively. Updating the listener invocations to use a final local variable ensures all listeners get consistent updates. PiperOrigin-RevId: 487503373
1 parent c6a0ba2 commit 33bea39

File tree

3 files changed

+109
-70
lines changed

3 files changed

+109
-70
lines changed

‎libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java

Lines changed: 70 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2320,10 +2320,7 @@ void onCustomCommand(int seq, SessionCommand command, Bundle args) {
23202320
}
23212321

23222322
@SuppressWarnings("deprecation") // Implementing and calling deprecated listener method.
2323-
void onPlayerInfoChanged(
2324-
PlayerInfo newPlayerInfo,
2325-
@Player.TimelineChangeReason int timelineChangedReason,
2326-
boolean isTimelineExcluded) {
2323+
void onPlayerInfoChanged(PlayerInfo newPlayerInfo, boolean isTimelineExcluded) {
23272324
if (!isConnected()) {
23282325
return;
23292326
}
@@ -2335,169 +2332,178 @@ void onPlayerInfoChanged(
23352332
return;
23362333
}
23372334
PlayerInfo oldPlayerInfo = playerInfo;
2338-
playerInfo = newPlayerInfo;
23392335
if (isTimelineExcluded) {
2340-
playerInfo =
2341-
playerInfo.copyWithTimeline(
2336+
newPlayerInfo =
2337+
newPlayerInfo.copyWithTimeline(
23422338
pendingPlayerInfoUpdateTimeline != null
23432339
? pendingPlayerInfoUpdateTimeline
23442340
: oldPlayerInfo.timeline);
23452341
}
2342+
// Assigning class variable now so that all getters called from listeners see the updated value.
2343+
// But we need to use a local final variable to ensure listeners get consistent parameters.
2344+
playerInfo = newPlayerInfo;
2345+
PlayerInfo finalPlayerInfo = newPlayerInfo;
23462346
pendingPlayerInfoUpdateTimeline = null;
23472347
PlaybackException oldPlayerError = oldPlayerInfo.playerError;
2348-
PlaybackException playerError = playerInfo.playerError;
2348+
PlaybackException playerError = finalPlayerInfo.playerError;
23492349
boolean errorsMatch =
23502350
oldPlayerError == playerError
23512351
|| (oldPlayerError != null && oldPlayerError.errorInfoEquals(playerError));
23522352
if (!errorsMatch) {
23532353
listeners.queueEvent(
23542354
/* eventFlag= */ Player.EVENT_PLAYER_ERROR,
2355-
listener -> listener.onPlayerErrorChanged(playerInfo.playerError));
2356-
if (playerInfo.playerError != null) {
2355+
listener -> listener.onPlayerErrorChanged(finalPlayerInfo.playerError));
2356+
if (finalPlayerInfo.playerError != null) {
23572357
listeners.queueEvent(
23582358
/* eventFlag= */ Player.EVENT_PLAYER_ERROR,
2359-
listener -> listener.onPlayerError(playerInfo.playerError));
2359+
listener -> listener.onPlayerError(finalPlayerInfo.playerError));
23602360
}
23612361
}
23622362
MediaItem oldCurrentMediaItem = oldPlayerInfo.getCurrentMediaItem();
2363-
MediaItem currentMediaItem = playerInfo.getCurrentMediaItem();
2363+
MediaItem currentMediaItem = finalPlayerInfo.getCurrentMediaItem();
23642364
if (!Util.areEqual(oldCurrentMediaItem, currentMediaItem)) {
23652365
listeners.queueEvent(
23662366
/* eventFlag= */ Player.EVENT_MEDIA_ITEM_TRANSITION,
23672367
listener ->
23682368
listener.onMediaItemTransition(
2369-
currentMediaItem, playerInfo.mediaItemTransitionReason));
2369+
currentMediaItem, finalPlayerInfo.mediaItemTransitionReason));
23702370
}
2371-
if (!Util.areEqual(oldPlayerInfo.currentTracks, playerInfo.currentTracks)) {
2371+
if (!Util.areEqual(oldPlayerInfo.currentTracks, finalPlayerInfo.currentTracks)) {
23722372
listeners.queueEvent(
23732373
/* eventFlag= */ Player.EVENT_TRACKS_CHANGED,
2374-
listener -> listener.onTracksChanged(playerInfo.currentTracks));
2374+
listener -> listener.onTracksChanged(finalPlayerInfo.currentTracks));
23752375
}
2376-
if (!Util.areEqual(oldPlayerInfo.playbackParameters, playerInfo.playbackParameters)) {
2376+
if (!Util.areEqual(oldPlayerInfo.playbackParameters, finalPlayerInfo.playbackParameters)) {
23772377
listeners.queueEvent(
23782378
/* eventFlag= */ Player.EVENT_PLAYBACK_PARAMETERS_CHANGED,
2379-
listener -> listener.onPlaybackParametersChanged(playerInfo.playbackParameters));
2379+
listener -> listener.onPlaybackParametersChanged(finalPlayerInfo.playbackParameters));
23802380
}
2381-
if (oldPlayerInfo.repeatMode != playerInfo.repeatMode) {
2381+
if (oldPlayerInfo.repeatMode != finalPlayerInfo.repeatMode) {
23822382
listeners.queueEvent(
23832383
/* eventFlag= */ Player.EVENT_REPEAT_MODE_CHANGED,
2384-
listener -> listener.onRepeatModeChanged(playerInfo.repeatMode));
2384+
listener -> listener.onRepeatModeChanged(finalPlayerInfo.repeatMode));
23852385
}
2386-
if (oldPlayerInfo.shuffleModeEnabled != playerInfo.shuffleModeEnabled) {
2386+
if (oldPlayerInfo.shuffleModeEnabled != finalPlayerInfo.shuffleModeEnabled) {
23872387
listeners.queueEvent(
23882388
/* eventFlag= */ Player.EVENT_SHUFFLE_MODE_ENABLED_CHANGED,
2389-
listener -> listener.onShuffleModeEnabledChanged(playerInfo.shuffleModeEnabled));
2389+
listener -> listener.onShuffleModeEnabledChanged(finalPlayerInfo.shuffleModeEnabled));
23902390
}
2391-
if (!isTimelineExcluded && !Util.areEqual(oldPlayerInfo.timeline, playerInfo.timeline)) {
2391+
if (!isTimelineExcluded && !Util.areEqual(oldPlayerInfo.timeline, finalPlayerInfo.timeline)) {
23922392
listeners.queueEvent(
23932393
/* eventFlag= */ Player.EVENT_TIMELINE_CHANGED,
2394-
listener -> listener.onTimelineChanged(playerInfo.timeline, timelineChangedReason));
2394+
listener ->
2395+
listener.onTimelineChanged(
2396+
finalPlayerInfo.timeline, Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE));
23952397
}
2396-
if (!Util.areEqual(oldPlayerInfo.playlistMetadata, playerInfo.playlistMetadata)) {
2398+
if (!Util.areEqual(oldPlayerInfo.playlistMetadata, finalPlayerInfo.playlistMetadata)) {
23972399
listeners.queueEvent(
23982400
/* eventFlag= */ Player.EVENT_PLAYLIST_METADATA_CHANGED,
2399-
listener -> listener.onPlaylistMetadataChanged(playerInfo.playlistMetadata));
2401+
listener -> listener.onPlaylistMetadataChanged(finalPlayerInfo.playlistMetadata));
24002402
}
2401-
if (oldPlayerInfo.volume != playerInfo.volume) {
2403+
if (oldPlayerInfo.volume != finalPlayerInfo.volume) {
24022404
listeners.queueEvent(
24032405
/* eventFlag= */ Player.EVENT_VOLUME_CHANGED,
2404-
listener -> listener.onVolumeChanged(playerInfo.volume));
2406+
listener -> listener.onVolumeChanged(finalPlayerInfo.volume));
24052407
}
2406-
if (!Util.areEqual(oldPlayerInfo.audioAttributes, playerInfo.audioAttributes)) {
2408+
if (!Util.areEqual(oldPlayerInfo.audioAttributes, finalPlayerInfo.audioAttributes)) {
24072409
listeners.queueEvent(
24082410
/* eventFlag= */ Player.EVENT_AUDIO_ATTRIBUTES_CHANGED,
2409-
listener -> listener.onAudioAttributesChanged(playerInfo.audioAttributes));
2411+
listener -> listener.onAudioAttributesChanged(finalPlayerInfo.audioAttributes));
24102412
}
2411-
if (!oldPlayerInfo.cueGroup.cues.equals(playerInfo.cueGroup.cues)) {
2413+
if (!oldPlayerInfo.cueGroup.cues.equals(finalPlayerInfo.cueGroup.cues)) {
24122414
listeners.queueEvent(
24132415
/* eventFlag= */ Player.EVENT_CUES,
2414-
listener -> listener.onCues(playerInfo.cueGroup.cues));
2416+
listener -> listener.onCues(finalPlayerInfo.cueGroup.cues));
24152417
listeners.queueEvent(
2416-
/* eventFlag= */ Player.EVENT_CUES, listener -> listener.onCues(playerInfo.cueGroup));
2418+
/* eventFlag= */ Player.EVENT_CUES,
2419+
listener -> listener.onCues(finalPlayerInfo.cueGroup));
24172420
}
2418-
if (!Util.areEqual(oldPlayerInfo.deviceInfo, playerInfo.deviceInfo)) {
2421+
if (!Util.areEqual(oldPlayerInfo.deviceInfo, finalPlayerInfo.deviceInfo)) {
24192422
listeners.queueEvent(
24202423
/* eventFlag= */ Player.EVENT_DEVICE_INFO_CHANGED,
2421-
listener -> listener.onDeviceInfoChanged(playerInfo.deviceInfo));
2424+
listener -> listener.onDeviceInfoChanged(finalPlayerInfo.deviceInfo));
24222425
}
2423-
if (oldPlayerInfo.deviceVolume != playerInfo.deviceVolume
2424-
|| oldPlayerInfo.deviceMuted != playerInfo.deviceMuted) {
2426+
if (oldPlayerInfo.deviceVolume != finalPlayerInfo.deviceVolume
2427+
|| oldPlayerInfo.deviceMuted != finalPlayerInfo.deviceMuted) {
24252428
listeners.queueEvent(
24262429
/* eventFlag= */ Player.EVENT_DEVICE_VOLUME_CHANGED,
24272430
listener ->
2428-
listener.onDeviceVolumeChanged(playerInfo.deviceVolume, playerInfo.deviceMuted));
2431+
listener.onDeviceVolumeChanged(
2432+
finalPlayerInfo.deviceVolume, finalPlayerInfo.deviceMuted));
24292433
}
2430-
if (oldPlayerInfo.playWhenReady != playerInfo.playWhenReady) {
2434+
if (oldPlayerInfo.playWhenReady != finalPlayerInfo.playWhenReady) {
24312435
listeners.queueEvent(
24322436
/* eventFlag= */ Player.EVENT_PLAY_WHEN_READY_CHANGED,
24332437
listener ->
24342438
listener.onPlayWhenReadyChanged(
2435-
playerInfo.playWhenReady, playerInfo.playWhenReadyChangedReason));
2439+
finalPlayerInfo.playWhenReady, finalPlayerInfo.playWhenReadyChangedReason));
24362440
}
2437-
if (oldPlayerInfo.playbackSuppressionReason != playerInfo.playbackSuppressionReason) {
2441+
if (oldPlayerInfo.playbackSuppressionReason != finalPlayerInfo.playbackSuppressionReason) {
24382442
listeners.queueEvent(
24392443
/* eventFlag= */ Player.EVENT_PLAYBACK_SUPPRESSION_REASON_CHANGED,
24402444
listener ->
2441-
listener.onPlaybackSuppressionReasonChanged(playerInfo.playbackSuppressionReason));
2445+
listener.onPlaybackSuppressionReasonChanged(
2446+
finalPlayerInfo.playbackSuppressionReason));
24422447
}
2443-
if (oldPlayerInfo.playbackState != playerInfo.playbackState) {
2448+
if (oldPlayerInfo.playbackState != finalPlayerInfo.playbackState) {
24442449
listeners.queueEvent(
24452450
/* eventFlag= */ Player.EVENT_PLAYBACK_STATE_CHANGED,
2446-
listener -> listener.onPlaybackStateChanged(playerInfo.playbackState));
2451+
listener -> listener.onPlaybackStateChanged(finalPlayerInfo.playbackState));
24472452
}
2448-
if (oldPlayerInfo.isPlaying != playerInfo.isPlaying) {
2453+
if (oldPlayerInfo.isPlaying != finalPlayerInfo.isPlaying) {
24492454
listeners.queueEvent(
24502455
/* eventFlag= */ Player.EVENT_IS_PLAYING_CHANGED,
2451-
listener -> listener.onIsPlayingChanged(playerInfo.isPlaying));
2456+
listener -> listener.onIsPlayingChanged(finalPlayerInfo.isPlaying));
24522457
}
2453-
if (oldPlayerInfo.isLoading != playerInfo.isLoading) {
2458+
if (oldPlayerInfo.isLoading != finalPlayerInfo.isLoading) {
24542459
listeners.queueEvent(
24552460
/* eventFlag= */ Player.EVENT_IS_LOADING_CHANGED,
2456-
listener -> listener.onIsLoadingChanged(playerInfo.isLoading));
2461+
listener -> listener.onIsLoadingChanged(finalPlayerInfo.isLoading));
24572462
}
2458-
if (!Util.areEqual(oldPlayerInfo.videoSize, playerInfo.videoSize)) {
2463+
if (!Util.areEqual(oldPlayerInfo.videoSize, finalPlayerInfo.videoSize)) {
24592464
listeners.queueEvent(
24602465
/* eventFlag= */ Player.EVENT_VIDEO_SIZE_CHANGED,
2461-
listener -> listener.onVideoSizeChanged(playerInfo.videoSize));
2466+
listener -> listener.onVideoSizeChanged(finalPlayerInfo.videoSize));
24622467
}
2463-
if (!Util.areEqual(oldPlayerInfo.oldPositionInfo, playerInfo.oldPositionInfo)
2464-
|| !Util.areEqual(oldPlayerInfo.newPositionInfo, playerInfo.newPositionInfo)) {
2468+
if (!Util.areEqual(oldPlayerInfo.oldPositionInfo, finalPlayerInfo.oldPositionInfo)
2469+
|| !Util.areEqual(oldPlayerInfo.newPositionInfo, finalPlayerInfo.newPositionInfo)) {
24652470
listeners.queueEvent(
24662471
/* eventFlag= */ Player.EVENT_POSITION_DISCONTINUITY,
24672472
listener ->
24682473
listener.onPositionDiscontinuity(
2469-
playerInfo.oldPositionInfo,
2470-
playerInfo.newPositionInfo,
2471-
playerInfo.discontinuityReason));
2474+
finalPlayerInfo.oldPositionInfo,
2475+
finalPlayerInfo.newPositionInfo,
2476+
finalPlayerInfo.discontinuityReason));
24722477
}
2473-
if (!Util.areEqual(oldPlayerInfo.mediaMetadata, playerInfo.mediaMetadata)) {
2478+
if (!Util.areEqual(oldPlayerInfo.mediaMetadata, finalPlayerInfo.mediaMetadata)) {
24742479
listeners.queueEvent(
24752480
/* eventFlag= */ Player.EVENT_MEDIA_METADATA_CHANGED,
2476-
listener -> listener.onMediaMetadataChanged(playerInfo.mediaMetadata));
2481+
listener -> listener.onMediaMetadataChanged(finalPlayerInfo.mediaMetadata));
24772482
}
2478-
if (oldPlayerInfo.seekBackIncrementMs != playerInfo.seekBackIncrementMs) {
2483+
if (oldPlayerInfo.seekBackIncrementMs != finalPlayerInfo.seekBackIncrementMs) {
24792484
listeners.queueEvent(
24802485
/* eventFlag= */ Player.EVENT_SEEK_BACK_INCREMENT_CHANGED,
2481-
listener -> listener.onSeekBackIncrementChanged(playerInfo.seekBackIncrementMs));
2486+
listener -> listener.onSeekBackIncrementChanged(finalPlayerInfo.seekBackIncrementMs));
24822487
}
2483-
if (oldPlayerInfo.seekForwardIncrementMs != playerInfo.seekForwardIncrementMs) {
2488+
if (oldPlayerInfo.seekForwardIncrementMs != finalPlayerInfo.seekForwardIncrementMs) {
24842489
listeners.queueEvent(
24852490
/* eventFlag= */ Player.EVENT_SEEK_FORWARD_INCREMENT_CHANGED,
2486-
listener -> listener.onSeekForwardIncrementChanged(playerInfo.seekForwardIncrementMs));
2491+
listener ->
2492+
listener.onSeekForwardIncrementChanged(finalPlayerInfo.seekForwardIncrementMs));
24872493
}
2488-
if (oldPlayerInfo.maxSeekToPreviousPositionMs != newPlayerInfo.maxSeekToPreviousPositionMs) {
2494+
if (oldPlayerInfo.maxSeekToPreviousPositionMs != finalPlayerInfo.maxSeekToPreviousPositionMs) {
24892495
listeners.queueEvent(
24902496
/* eventFlag= */ Player.EVENT_MAX_SEEK_TO_PREVIOUS_POSITION_CHANGED,
24912497
listener ->
24922498
listener.onMaxSeekToPreviousPositionChanged(
2493-
newPlayerInfo.maxSeekToPreviousPositionMs));
2499+
finalPlayerInfo.maxSeekToPreviousPositionMs));
24942500
}
24952501
if (!Util.areEqual(
2496-
oldPlayerInfo.trackSelectionParameters, newPlayerInfo.trackSelectionParameters)) {
2502+
oldPlayerInfo.trackSelectionParameters, finalPlayerInfo.trackSelectionParameters)) {
24972503
listeners.queueEvent(
24982504
/* eventFlag= */ Player.EVENT_TRACK_SELECTION_PARAMETERS_CHANGED,
24992505
listener ->
2500-
listener.onTrackSelectionParametersChanged(newPlayerInfo.trackSelectionParameters));
2506+
listener.onTrackSelectionParametersChanged(finalPlayerInfo.trackSelectionParameters));
25012507
}
25022508
listeners.flushEvents();
25032509
}

‎libraries/session/src/main/java/androidx/media3/session/MediaControllerStub.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import android.os.Handler;
2323
import android.text.TextUtils;
2424
import androidx.annotation.Nullable;
25-
import androidx.media3.common.Player;
2625
import androidx.media3.common.Player.Commands;
2726
import androidx.media3.common.util.BundleableUtil;
2827
import androidx.media3.common.util.Log;
@@ -180,11 +179,7 @@ public void onPlayerInfoChanged(int seq, Bundle playerInfoBundle, boolean isTime
180179
return;
181180
}
182181
dispatchControllerTaskOnHandler(
183-
controller ->
184-
controller.onPlayerInfoChanged(
185-
playerInfo,
186-
/* timelineChangedReason= */ Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE,
187-
isTimelineExcluded));
182+
controller -> controller.onPlayerInfoChanged(playerInfo, isTimelineExcluded));
188183
}
189184

190185
@Override

‎libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3300,6 +3300,44 @@ public void onEvents(Player player, Player.Events events) {
33003300
assertThat(getEventsAsList(eventsRef.get())).containsExactly(Player.EVENT_RENDERED_FIRST_FRAME);
33013301
}
33023302

3303+
@Test
3304+
public void recursiveChangesFromListeners_reportConsistentValuesForAllListeners()
3305+
throws Exception {
3306+
// We add two listeners to the controller. The first stops the player as soon as it's ready and
3307+
// both record the state change events they receive.
3308+
MediaController controller = controllerTestRule.createController(remoteSession.getToken());
3309+
List<Integer> listener1States = new ArrayList<>();
3310+
List<Integer> listener2States = new ArrayList<>();
3311+
CountDownLatch latch = new CountDownLatch(4);
3312+
Player.Listener listener1 =
3313+
new Player.Listener() {
3314+
@Override
3315+
public void onPlaybackStateChanged(@Player.State int playbackState) {
3316+
listener1States.add(playbackState);
3317+
if (playbackState == Player.STATE_READY) {
3318+
controller.stop();
3319+
}
3320+
latch.countDown();
3321+
}
3322+
};
3323+
Player.Listener listener2 =
3324+
new Player.Listener() {
3325+
@Override
3326+
public void onPlaybackStateChanged(@Player.State int playbackState) {
3327+
listener2States.add(playbackState);
3328+
latch.countDown();
3329+
}
3330+
};
3331+
controller.addListener(listener1);
3332+
controller.addListener(listener2);
3333+
3334+
remoteSession.getMockPlayer().notifyPlaybackStateChanged(Player.STATE_READY);
3335+
3336+
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
3337+
assertThat(listener1States).containsExactly(Player.STATE_READY, Player.STATE_IDLE).inOrder();
3338+
assertThat(listener2States).containsExactly(Player.STATE_READY, Player.STATE_IDLE).inOrder();
3339+
}
3340+
33033341
private void testControllerAfterSessionIsClosed(String id) throws Exception {
33043342
MediaController controller = controllerTestRule.createController(remoteSession.getToken());
33053343
// This causes the session service to die.

0 commit comments

Comments
 (0)