Merge "Update listener instead of creating a new one" into rvc-qpr-dev am: 85c1b3ebf2

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/12684877

Change-Id: I1ed067552e608fa0bf92e03089da6c9c29b5ef99

packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt

@@ -54,25 +54,22 @@ class MediaTimeoutListener @Inject constructor(
         if (mediaListeners.containsKey(key)) {
             return
         }
-        // Having an old key means that we're migrating from/to resumption. We should invalidate
+        // Having an old key means that we're migrating from/to resumption. We should update
-        // the old listener and create a new one.
+        // the old listener to make sure that events will be dispatched to the new location.
         val migrating = oldKey != null && key != oldKey
         var wasPlaying = false
         if (migrating) {
-            if (mediaListeners.containsKey(oldKey)) {
+            val reusedListener = mediaListeners.remove(oldKey)
-                val oldListener = mediaListeners.remove(oldKey)
+            if (reusedListener != null) {
-                wasPlaying = oldListener?.playing ?: false
+                wasPlaying = reusedListener.playing ?: false
-                oldListener?.destroy()
                 if (DEBUG) Log.d(TAG, "migrating key $oldKey to $key, for resumption")
-            } else {
+                reusedListener.mediaData = data
-                Log.w(TAG, "Old key $oldKey for player $key doesn't exist. Continuing...")
+                reusedListener.key = key
-            }
+                mediaListeners[key] = reusedListener
-        }
+                if (wasPlaying != reusedListener.playing) {
-        mediaListeners[key] = PlaybackStateListener(key, data)
+                    // If a player becomes active because of a migration, we'll need to broadcast
-
+                    // its state. Doing it now would lead to reentrant callbacks, so let's wait
-        // If a player becomes active because of a migration, we'll need to broadcast its state.
+                    // until we're done.
-        // Doing it now would lead to reentrant callbacks, so let's wait until we're done.
-        if (migrating && mediaListeners[key]?.playing != wasPlaying) {
                     mainExecutor.execute {
                         if (mediaListeners[key]?.playing == true) {
                             if (DEBUG) Log.d(TAG, "deliver delayed playback state for $key")
@@ -80,6 +77,12 @@ class MediaTimeoutListener @Inject constructor(
                         }
                     }
                 }
+                return
+            } else {
+                Log.w(TAG, "Old key $oldKey for player $key doesn't exist. Continuing...")
+            }
+        }
+        mediaListeners[key] = PlaybackStateListener(key, data)
     }
 
     override fun onMediaDataRemoved(key: String) {
@@ -91,28 +94,36 @@ class MediaTimeoutListener @Inject constructor(
     }
 
     private inner class PlaybackStateListener(
-        private val key: String,
+        var key: String,
         data: MediaData
     ) : MediaController.Callback() {
 
         var timedOut = false
         var playing: Boolean? = null
 
-        // Resume controls may have null token
+        var mediaData: MediaData = data
-        private val mediaController = if (data.token != null) {
+            set(value) {
-            mediaControllerFactory.create(data.token)
+                mediaController?.unregisterCallback(this)
+                field = value
+                mediaController = if (field.token != null) {
+                    mediaControllerFactory.create(field.token)
                 } else {
                     null
                 }
-        private var cancellation: Runnable? = null
-
-        init {
                 mediaController?.registerCallback(this)
                 // Let's register the cancellations, but not dispatch events now.
                 // Timeouts didn't happen yet and reentrant events are troublesome.
                 processState(mediaController?.playbackState, dispatchEvents = false)
             }
 
+        // Resume controls may have null token
+        private var mediaController: MediaController? = null
+        private var cancellation: Runnable? = null
+
+        init {
+            mediaData = data
+        }
+
         fun destroy() {
             mediaController?.unregisterCallback(this)
         }

packages/SystemUI/tests/src/com/android/systemui/media/MediaTimeoutListenerTest.kt

@@ -154,6 +154,22 @@ class MediaTimeoutListenerTest : SysuiTestCase() {
         verify(executor).execute(anyObject())
     }
 
+    @Test
+    fun testOnMediaDataLoaded_migratesKeys_noTimeoutExtension() {
+        // From not playing
+        mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData)
+        clearInvocations(mediaController)
+
+        // Migrate, still not playing
+        val playingState = mock(android.media.session.PlaybackState::class.java)
+        `when`(playingState.state).thenReturn(PlaybackState.STATE_PAUSED)
+        `when`(mediaController.playbackState).thenReturn(playingState)
+        mediaTimeoutListener.onMediaDataLoaded("NEWKEY", KEY, mediaData)
+
+        // Never cancels callback, or schedule another one
+        verify(cancellationRunnable, never()).run()
+    }
+
     @Test
     fun testOnPlaybackStateChanged_schedulesTimeout_whenPaused() {
         // Assuming we're registered