From b7ab570094240b051e3dc4914b7bceffe2749b0e Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 20 Nov 2018 04:46:42 -0800 Subject: [PATCH] Use serialized listener notification for all listener notifications. This ensures that all callbacks use the same notification mechanism. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=222226913 --- .../android/exoplayer2/ExoPlayerImpl.java | 53 +++++++------------ .../android/exoplayer2/ExoPlayerTest.java | 7 ++- 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index 2c9f949d09..41c71b21d5 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -238,8 +238,8 @@ import java.util.concurrent.CopyOnWriteArraySet; } if (this.playWhenReady != playWhenReady) { this.playWhenReady = playWhenReady; - notifyListeners( - new PlayWhenReadyUpdate(listeners, playWhenReady, playbackInfo.playbackState)); + int playbackState = playbackInfo.playbackState; + notifyListeners(listener -> listener.onPlayerStateChanged(playWhenReady, playbackState)); } } @@ -253,9 +253,7 @@ import java.util.concurrent.CopyOnWriteArraySet; if (this.repeatMode != repeatMode) { this.repeatMode = repeatMode; internalPlayer.setRepeatMode(repeatMode); - for (Player.EventListener listener : listeners) { - listener.onRepeatModeChanged(repeatMode); - } + notifyListeners(listener -> listener.onRepeatModeChanged(repeatMode)); } } @@ -269,9 +267,7 @@ import java.util.concurrent.CopyOnWriteArraySet; if (this.shuffleModeEnabled != shuffleModeEnabled) { this.shuffleModeEnabled = shuffleModeEnabled; internalPlayer.setShuffleModeEnabled(shuffleModeEnabled); - for (Player.EventListener listener : listeners) { - listener.onShuffleModeEnabledChanged(shuffleModeEnabled); - } + notifyListeners(listener -> listener.onShuffleModeEnabledChanged(shuffleModeEnabled)); } } @@ -320,9 +316,7 @@ import java.util.concurrent.CopyOnWriteArraySet; maskingPeriodIndex = timeline.getIndexOfPeriod(periodUidAndPosition.first); } internalPlayer.seekTo(timeline, windowIndex, C.msToUs(positionMs)); - for (Player.EventListener listener : listeners) { - listener.onPositionDiscontinuity(DISCONTINUITY_REASON_SEEK); - } + notifyListeners(listener -> listener.onPositionDiscontinuity(DISCONTINUITY_REASON_SEEK)); } @Override @@ -594,17 +588,13 @@ import java.util.concurrent.CopyOnWriteArraySet; PlaybackParameters playbackParameters = (PlaybackParameters) msg.obj; if (!this.playbackParameters.equals(playbackParameters)) { this.playbackParameters = playbackParameters; - for (Player.EventListener listener : listeners) { - listener.onPlaybackParametersChanged(playbackParameters); - } + notifyListeners(listener -> listener.onPlaybackParametersChanged(playbackParameters)); } break; case ExoPlayerImplInternal.MSG_ERROR: ExoPlaybackException playbackError = (ExoPlaybackException) msg.obj; this.playbackError = playbackError; - for (Player.EventListener listener : listeners) { - listener.onPlayerError(playbackError); - } + notifyListeners(listener -> listener.onPlayerError(playbackError)); break; default: throw new IllegalStateException(); @@ -703,6 +693,15 @@ import java.util.concurrent.CopyOnWriteArraySet; playWhenReady)); } + private void notifyListeners(ListenerInvocation listenerInvocation) { + notifyListeners( + () -> { + for (Player.EventListener listener : listeners) { + listenerInvocation.invokeListener(listener); + } + }); + } + private void notifyListeners(Runnable listenerNotificationRunnable) { boolean isRunningRecursiveListenerNotification = !pendingListenerNotifications.isEmpty(); pendingListenerNotifications.addLast(listenerNotificationRunnable); @@ -806,24 +805,8 @@ import java.util.concurrent.CopyOnWriteArraySet; } } - private static final class PlayWhenReadyUpdate implements Runnable { + private interface ListenerInvocation { - private final Set listeners; - private final boolean playWhenReady; - private final int playbackState; - - public PlayWhenReadyUpdate( - Set listeners, boolean playWhenReady, int playbackState) { - this.listeners = listeners; - this.playWhenReady = playWhenReady; - this.playbackState = playbackState; - } - - @Override - public void run() { - for (Player.EventListener listener : listeners) { - listener.onPlayerStateChanged(playWhenReady, playbackState); - } - } + void invokeListener(Player.EventListener listener); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index d131ed0f51..1ae9e7a3ea 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -498,12 +498,12 @@ public final class ExoPlayerTest { .waitForPlaybackState(Player.STATE_READY) .seek(10) // Start playback and wait until playback reaches second window. - .play() - .waitForPositionDiscontinuity() + .playUntilStartOfWindow(/* windowIndex= */ 1) // Seek twice in concession, expecting the first seek to be replaced (and thus except // only on seek processed callback). .seek(5) .seek(60) + .play() .build(); final List playbackStatesWhenSeekProcessed = new ArrayList<>(); EventListener eventListener = @@ -2058,8 +2058,11 @@ public final class ExoPlayerTest { .pause() .waitForPlaybackState(Player.STATE_READY) .seek(/* windowIndex= */ 0, /* positionMs= */ 9999) + .waitForSeekProcessed() .seek(/* windowIndex= */ 0, /* positionMs= */ 1) + .waitForSeekProcessed() .seek(/* windowIndex= */ 0, /* positionMs= */ 9999) + .waitForSeekProcessed() .play() .build(); ExoPlayerTestRunner testRunner =