From c5c4c8772871cb3e0b2159d584a4dd00a4ba4dc9 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 19 Jun 2020 18:36:20 +0100 Subject: [PATCH] Restrict some Handler to current Looper only. They currently fall back to the main Looper if the current thread doesn't have a Looper. All the changed Handlers are guaranteed to be created on a thread with a Looper (mostly the ExoPlayer playback Looper) and thus can make this stricter assumption. This makes it easier to reason about the code as there are no ambiguities as to which thread the Handler is running on. PiperOrigin-RevId: 317334503 --- .../google/android/exoplayer2/util/Util.java | 40 ++++++++++++++++--- .../source/CompositeMediaSource.java | 2 +- .../source/ProgressiveMediaPeriod.java | 2 +- .../exoplayer2/source/ads/AdsMediaSource.java | 2 +- .../video/MediaCodecVideoRenderer.java | 2 +- .../source/ClippingMediaSourceTest.java | 2 +- .../source/ConcatenatingMediaSourceTest.java | 36 ++++++++--------- .../source/dash/DashMediaSource.java | 2 +- .../source/dash/PlayerEmsgHandler.java | 2 +- .../source/hls/HlsSampleStreamWrapper.java | 2 +- .../playlist/DefaultHlsPlaylistTracker.java | 2 +- .../source/smoothstreaming/SsMediaSource.java | 2 +- .../exoplayer2/testutil/FakeMediaPeriod.java | 2 +- .../exoplayer2/testutil/FakeMediaSource.java | 2 +- 14 files changed, 62 insertions(+), 38 deletions(-) diff --git a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java index 96c2d3622a..838de61db4 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java @@ -393,6 +393,32 @@ public final class Util { return concatenation; } + /** + * Creates a {@link Handler} on the current {@link Looper} thread. + * + * @throws IllegalStateException If the current thread doesn't have a {@link Looper}. + */ + public static Handler createHandlerForCurrentLooper() { + return createHandlerForCurrentLooper(/* callback= */ null); + } + + /** + * Creates a {@link Handler} with the specified {@link Handler.Callback} on the current {@link + * Looper} thread. + * + *

The method accepts partially initialized objects as callback under the assumption that the + * Handler won't be used to send messages until the callback is fully initialized. + * + * @param callback A {@link Handler.Callback}. May be a partially initialized class, or null if no + * callback is required. + * @return A {@link Handler} with the specified callback on the current {@link Looper} thread. + * @throws IllegalStateException If the current thread doesn't have a {@link Looper}. + */ + public static Handler createHandlerForCurrentLooper( + @Nullable Handler.@UnknownInitialization Callback callback) { + return createHandler(Assertions.checkStateNotNull(Looper.myLooper()), callback); + } + /** * Creates a {@link Handler} on the current {@link Looper} thread. * @@ -405,9 +431,10 @@ public final class Util { /** * Creates a {@link Handler} with the specified {@link Handler.Callback} on the current {@link - * Looper} thread. The method accepts partially initialized objects as callback under the - * assumption that the Handler won't be used to send messages until the callback is fully - * initialized. + * Looper} thread. + * + *

The method accepts partially initialized objects as callback under the assumption that the + * Handler won't be used to send messages until the callback is fully initialized. * *

If the current thread doesn't have a {@link Looper}, the application's main thread {@link * Looper} is used. @@ -423,9 +450,10 @@ public final class Util { /** * Creates a {@link Handler} with the specified {@link Handler.Callback} on the specified {@link - * Looper} thread. The method accepts partially initialized objects as callback under the - * assumption that the Handler won't be used to send messages until the callback is fully - * initialized. + * Looper} thread. + * + *

The method accepts partially initialized objects as callback under the assumption that the + * Handler won't be used to send messages until the callback is fully initialized. * * @param looper A {@link Looper} to run the callback on. * @param callback A {@link Handler.Callback}. May be a partially initialized class, or null if no diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/CompositeMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/CompositeMediaSource.java index 6693e53abe..3d31eeba92 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/CompositeMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/CompositeMediaSource.java @@ -48,7 +48,7 @@ public abstract class CompositeMediaSource extends BaseMediaSource { @CallSuper protected void prepareSourceInternal(@Nullable TransferListener mediaTransferListener) { this.mediaTransferListener = mediaTransferListener; - eventHandler = Util.createHandlerForCurrentOrMainLooper(); + eventHandler = Util.createHandlerForCurrentLooper(); } @Override diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java index 25283a0ecf..3bfb8356a5 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java @@ -192,7 +192,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; .onContinueLoadingRequested(ProgressiveMediaPeriod.this); } }; - handler = Util.createHandlerForCurrentOrMainLooper(); + handler = Util.createHandlerForCurrentLooper(); sampleQueueTrackIds = new TrackId[0]; sampleQueues = new SampleQueue[0]; pendingResetPositionUs = C.TIME_UNSET; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java index 3688c63ec1..ce45959325 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java @@ -336,7 +336,7 @@ public final class AdsMediaSource extends CompositeMediaSource { * events on the external event listener thread. */ public ComponentListener() { - playerHandler = Util.createHandlerForCurrentOrMainLooper(); + playerHandler = Util.createHandlerForCurrentLooper(); } /** Releases the component listener. */ diff --git a/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java b/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java index 814937717e..afc029b7cd 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java @@ -1763,7 +1763,7 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer { private final Handler handler; public OnFrameRenderedListenerV23(MediaCodec codec) { - handler = Util.createHandlerForCurrentOrMainLooper(/* callback= */ this); + handler = Util.createHandlerForCurrentLooper(/* callback= */ this); codec.setOnFrameRenderedListener(/* listener= */ this, handler); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java index 4f9331be62..69b38bd2e1 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java @@ -599,7 +599,7 @@ public final class ClippingMediaSourceTest { testRunner.runOnPlaybackThread( () -> clippingMediaSource.addEventListener( - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), new MediaSourceEventListener() { @Override public void onDownstreamFormatChanged( diff --git a/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java index 90e1eed47f..b3d0d1cc14 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java @@ -413,7 +413,7 @@ public final class ConcatenatingMediaSourceTest { () -> mediaSource.addMediaSource( createFakeMediaSource(), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), runnableInvoked::countDown)); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); dummyMainThread.release(); @@ -430,7 +430,7 @@ public final class ConcatenatingMediaSourceTest { () -> mediaSource.addMediaSources( Arrays.asList(new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), runnableInvoked::countDown)); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); dummyMainThread.release(); @@ -448,7 +448,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.addMediaSource( /* index */ 0, createFakeMediaSource(), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), runnableInvoked::countDown)); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); dummyMainThread.release(); @@ -466,7 +466,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.addMediaSources( /* index */ 0, Arrays.asList(new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), runnableInvoked::countDown)); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); dummyMainThread.release(); @@ -483,9 +483,7 @@ public final class ConcatenatingMediaSourceTest { () -> { mediaSource.addMediaSource(createFakeMediaSource()); mediaSource.removeMediaSource( - /* index */ 0, - Util.createHandlerForCurrentOrMainLooper(), - runnableInvoked::countDown); + /* index */ 0, Util.createHandlerForCurrentLooper(), runnableInvoked::countDown); }); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); dummyMainThread.release(); @@ -505,7 +503,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.moveMediaSource( /* fromIndex */ 1, /* toIndex */ 0, - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), runnableInvoked::countDown); }); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); @@ -523,9 +521,7 @@ public final class ConcatenatingMediaSourceTest { dummyMainThread.runOnMainThread( () -> mediaSource.addMediaSource( - createFakeMediaSource(), - Util.createHandlerForCurrentOrMainLooper(), - timelineGrabber)); + createFakeMediaSource(), Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(1); } finally { @@ -544,7 +540,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.addMediaSources( Arrays.asList( new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -564,7 +560,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.addMediaSource( /* index */ 0, createFakeMediaSource(), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(1); @@ -585,7 +581,7 @@ public final class ConcatenatingMediaSourceTest { /* index */ 0, Arrays.asList( new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -606,7 +602,7 @@ public final class ConcatenatingMediaSourceTest { dummyMainThread.runOnMainThread( () -> mediaSource.removeMediaSource( - /* index */ 0, Util.createHandlerForCurrentOrMainLooper(), timelineGrabber)); + /* index */ 0, Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(0); } finally { @@ -632,7 +628,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.moveMediaSource( /* fromIndex */ 1, /* toIndex */ 0, - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -654,7 +650,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.moveMediaSource( /* currentIndex= */ 0, /* newIndex= */ 1, - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), callbackCalledCondition::countDown); mediaSource.releaseSource(caller); }); @@ -907,7 +903,7 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( - () -> mediaSource.clear(Util.createHandlerForCurrentOrMainLooper(), timelineGrabber)); + () -> mediaSource.clear(Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.isEmpty()).isTrue(); @@ -1059,7 +1055,7 @@ public final class ConcatenatingMediaSourceTest { () -> mediaSource.setShuffleOrder( new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 0), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), runnableInvoked::countDown)); runnableInvoked.await(MediaSourceTestRunner.TIMEOUT_MS, TimeUnit.MILLISECONDS); dummyMainThread.release(); @@ -1079,7 +1075,7 @@ public final class ConcatenatingMediaSourceTest { () -> mediaSource.setShuffleOrder( new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 3), - Util.createHandlerForCurrentOrMainLooper(), + Util.createHandlerForCurrentLooper(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getFirstWindowIndex(/* shuffleModeEnabled= */ true)).isEqualTo(0); diff --git a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java index 103b689dc2..fe2b18814b 100644 --- a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java +++ b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java @@ -680,7 +680,7 @@ public final class DashMediaSource extends BaseMediaSource { } else { dataSource = manifestDataSourceFactory.createDataSource(); loader = new Loader("Loader:DashMediaSource"); - handler = Util.createHandlerForCurrentOrMainLooper(); + handler = Util.createHandlerForCurrentLooper(); startLoadingManifest(); } } diff --git a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/PlayerEmsgHandler.java b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/PlayerEmsgHandler.java index fed5ab74f5..58783ad745 100644 --- a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/PlayerEmsgHandler.java +++ b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/PlayerEmsgHandler.java @@ -105,7 +105,7 @@ public final class PlayerEmsgHandler implements Handler.Callback { this.allocator = allocator; manifestPublishTimeToExpiryTimeUs = new TreeMap<>(); - handler = Util.createHandlerForCurrentOrMainLooper(/* callback= */ this); + handler = Util.createHandlerForCurrentLooper(/* callback= */ this); decoder = new EventMessageDecoder(); lastLoadedChunkEndTimeUs = C.TIME_UNSET; lastLoadedChunkEndTimeBeforeRefreshUs = C.TIME_UNSET; diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java index 579af21bc4..78c9e2796d 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java @@ -227,7 +227,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull; @SuppressWarnings("nullness:methodref.receiver.bound.invalid") Runnable onTracksEndedRunnable = this::onTracksEnded; this.onTracksEndedRunnable = onTracksEndedRunnable; - handler = Util.createHandlerForCurrentOrMainLooper(); + handler = Util.createHandlerForCurrentLooper(); lastSeekPositionUs = positionUs; pendingResetPositionUs = positionUs; } diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/DefaultHlsPlaylistTracker.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/DefaultHlsPlaylistTracker.java index 2806a0bdd4..c9e92de9e2 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/DefaultHlsPlaylistTracker.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/DefaultHlsPlaylistTracker.java @@ -121,7 +121,7 @@ public final class DefaultHlsPlaylistTracker Uri initialPlaylistUri, EventDispatcher eventDispatcher, PrimaryPlaylistListener primaryPlaylistListener) { - this.playlistRefreshHandler = Util.createHandlerForCurrentOrMainLooper(); + this.playlistRefreshHandler = Util.createHandlerForCurrentLooper(); this.eventDispatcher = eventDispatcher; this.primaryPlaylistListener = primaryPlaylistListener; ParsingLoadable masterPlaylistLoadable = diff --git a/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java b/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java index 9f63a54650..019421b35f 100644 --- a/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java +++ b/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java @@ -620,7 +620,7 @@ public final class SsMediaSource extends BaseMediaSource manifestDataSource = manifestDataSourceFactory.createDataSource(); manifestLoader = new Loader("Loader:Manifest"); manifestLoaderErrorThrower = manifestLoader; - manifestRefreshHandler = Util.createHandlerForCurrentOrMainLooper(); + manifestRefreshHandler = Util.createHandlerForCurrentLooper(); startLoadingManifest(); } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java index 4e3d15b43f..e83d924293 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java @@ -162,7 +162,7 @@ public class FakeMediaPeriod implements MediaPeriod { /* mediaEndTimeUs = */ C.TIME_UNSET); prepareCallback = callback; if (deferOnPrepared) { - playerHandler = Util.createHandlerForCurrentOrMainLooper(); + playerHandler = Util.createHandlerForCurrentLooper(); } else { finishPreparation(); } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java index ded1da49b9..d4d4e76054 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java @@ -176,7 +176,7 @@ public class FakeMediaSource extends BaseMediaSource { drmSessionManager.prepare(); preparedSource = true; releasedSource = false; - sourceInfoRefreshHandler = Util.createHandlerForCurrentOrMainLooper(); + sourceInfoRefreshHandler = Util.createHandlerForCurrentLooper(); if (timeline != null) { finishSourcePreparation(/* sendManifestLoadEvents= */ true); }