Cancel pending timeouts when media data is removed
Bug: 160944177
Test: manual - verified repro steps in b/160944177#comment32
Change-Id: I2925814132c712b2482681f699f647bf5707e664
(cherry picked from commit f5e463fa86ac5ce4789ecf92f0ddb868cd32c866)
Merged-In: I2925814132c712b2482681f699f647bf5707e664
(cherry picked from commit 283a2e1d6e850ac80c7fb3b88e67ad23eb9e816e)
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt
index 6eef131..dcb7767 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 @@
fun destroy() {
mediaController?.unregisterCallback(this)
+ cancellation?.run()
}
override fun onPlaybackStateChanged(state: PlaybackState?) {
@@ -182,4 +183,4 @@
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 f385243..f397959 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.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.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 @@
@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<Runnable>
@Captor private lateinit var mediaCallbackCaptor: ArgumentCaptor<MediaController.Callback>
@JvmField @Rule val mockito = MockitoJUnit.rule()
private lateinit var metadataBuilder: MediaMetadata.Builder
@@ -78,7 +76,7 @@
@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 @@
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())
}
@@ -137,6 +135,17 @@
}
@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
mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData)
@@ -151,7 +160,7 @@
verify(mediaController).registerCallback(anyObject())
// Enqueues callback
- verify(executor).execute(anyObject())
+ assertThat(executor.numPending()).isEqualTo(1)
}
@Test
@@ -166,8 +175,9 @@
`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 @@
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 @@
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 @@
// 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 @@
// Assuming we're have a pending timeout
testOnPlaybackStateChanged_schedulesTimeout_whenPaused()
- timeoutCaptor.value.run()
+ with(executor) {
+ advanceClockToNext()
+ runAllReady()
+ }
verify(timeoutCallback).invoke(eq(KEY), eq(true))
}
