From f5e463fa86ac5ce4789ecf92f0ddb868cd32c866 Mon Sep 17 00:00:00 2001 From: Robert Snoeberger Date: Wed, 23 Sep 2020 12:57:41 -0400 Subject: [PATCH] Cancel pending timeouts when media data is removed Bug: 160944177 Test: manual - verified repro steps in b/160944177#comment32 Change-Id: I2925814132c712b2482681f699f647bf5707e664 --- .../systemui/media/MediaTimeoutListener.kt | 3 +- .../media/MediaTimeoutListenerTest.kt | 42 ++++++++++++------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt index 6bd5274fa3315..51dbfa7335414 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt @@ -126,6 +126,7 @@ class MediaTimeoutListener @Inject constructor( fun destroy() { mediaController?.unregisterCallback(this) + cancellation?.run() } override fun onPlaybackStateChanged(state: PlaybackState?) { @@ -182,4 +183,4 @@ class MediaTimeoutListener @Inject constructor( cancellation = null } } -} \ No newline at end of file +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaTimeoutListenerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaTimeoutListenerTest.kt index f38524369b46b..f3979592c894b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaTimeoutListenerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaTimeoutListenerTest.kt @@ -23,8 +23,9 @@ import android.media.session.PlaybackState import android.testing.AndroidTestingRunner import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase -import com.android.systemui.util.concurrency.DelayableExecutor +import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.capture +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import org.junit.Before import org.junit.Rule @@ -33,7 +34,6 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyBoolean -import org.mockito.ArgumentMatchers.anyLong import org.mockito.ArgumentMatchers.anyString import org.mockito.Captor import org.mockito.Mock @@ -63,10 +63,8 @@ class MediaTimeoutListenerTest : SysuiTestCase() { @Mock private lateinit var mediaControllerFactory: MediaControllerFactory @Mock private lateinit var mediaController: MediaController - @Mock private lateinit var executor: DelayableExecutor + private lateinit var executor: FakeExecutor @Mock private lateinit var timeoutCallback: (String, Boolean) -> Unit - @Mock private lateinit var cancellationRunnable: Runnable - @Captor private lateinit var timeoutCaptor: ArgumentCaptor @Captor private lateinit var mediaCallbackCaptor: ArgumentCaptor @JvmField @Rule val mockito = MockitoJUnit.rule() private lateinit var metadataBuilder: MediaMetadata.Builder @@ -78,7 +76,7 @@ class MediaTimeoutListenerTest : SysuiTestCase() { @Before fun setup() { `when`(mediaControllerFactory.create(any())).thenReturn(mediaController) - `when`(executor.executeDelayed(any(), anyLong())).thenReturn(cancellationRunnable) + executor = FakeExecutor(FakeSystemClock()) mediaTimeoutListener = MediaTimeoutListener(mediaControllerFactory, executor) mediaTimeoutListener.timeoutCallback = timeoutCallback @@ -120,7 +118,7 @@ class MediaTimeoutListenerTest : SysuiTestCase() { fun testOnMediaDataLoaded_registersTimeout_whenPaused() { mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData) verify(mediaController).registerCallback(capture(mediaCallbackCaptor)) - verify(executor).executeDelayed(capture(timeoutCaptor), anyLong()) + assertThat(executor.numPending()).isEqualTo(1) verify(timeoutCallback, never()).invoke(anyString(), anyBoolean()) } @@ -136,6 +134,17 @@ class MediaTimeoutListenerTest : SysuiTestCase() { verify(mediaController, never()).unregisterCallback(anyObject()) } + @Test + fun testOnMediaDataRemoved_clearsTimeout() { + // GIVEN media that is paused + mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData) + assertThat(executor.numPending()).isEqualTo(1) + // WHEN the media is removed + mediaTimeoutListener.onMediaDataRemoved(KEY) + // THEN the timeout runnable is cancelled + assertThat(executor.numPending()).isEqualTo(0) + } + @Test fun testOnMediaDataLoaded_migratesKeys() { // From not playing @@ -151,7 +160,7 @@ class MediaTimeoutListenerTest : SysuiTestCase() { verify(mediaController).registerCallback(anyObject()) // Enqueues callback - verify(executor).execute(anyObject()) + assertThat(executor.numPending()).isEqualTo(1) } @Test @@ -166,8 +175,9 @@ class MediaTimeoutListenerTest : SysuiTestCase() { `when`(mediaController.playbackState).thenReturn(playingState) mediaTimeoutListener.onMediaDataLoaded("NEWKEY", KEY, mediaData) - // Never cancels callback, or schedule another one - verify(cancellationRunnable, never()).run() + // The number of queued timeout tasks remains the same. The timeout task isn't cancelled nor + // is another scheduled + assertThat(executor.numPending()).isEqualTo(1) } @Test @@ -177,7 +187,7 @@ class MediaTimeoutListenerTest : SysuiTestCase() { mediaCallbackCaptor.value.onPlaybackStateChanged(PlaybackState.Builder() .setState(PlaybackState.STATE_PAUSED, 0L, 0f).build()) - verify(executor).executeDelayed(capture(timeoutCaptor), anyLong()) + assertThat(executor.numPending()).isEqualTo(1) } @Test @@ -187,7 +197,7 @@ class MediaTimeoutListenerTest : SysuiTestCase() { mediaCallbackCaptor.value.onPlaybackStateChanged(PlaybackState.Builder() .setState(PlaybackState.STATE_PLAYING, 0L, 0f).build()) - verify(cancellationRunnable).run() + assertThat(executor.numPending()).isEqualTo(0) } @Test @@ -195,10 +205,9 @@ class MediaTimeoutListenerTest : SysuiTestCase() { // Assuming we have a pending timeout testOnPlaybackStateChanged_schedulesTimeout_whenPaused() - clearInvocations(cancellationRunnable) mediaCallbackCaptor.value.onPlaybackStateChanged(PlaybackState.Builder() .setState(PlaybackState.STATE_STOPPED, 0L, 0f).build()) - verify(cancellationRunnable, never()).run() + assertThat(executor.numPending()).isEqualTo(1) } @Test @@ -206,7 +215,10 @@ class MediaTimeoutListenerTest : SysuiTestCase() { // Assuming we're have a pending timeout testOnPlaybackStateChanged_schedulesTimeout_whenPaused() - timeoutCaptor.value.run() + with(executor) { + advanceClockToNext() + runAllReady() + } verify(timeoutCallback).invoke(eq(KEY), eq(true)) }