Media - Leave playing media in QS

Previously, with resumption off, when dismissing a playing media
player from QQS would also remove the player from QS, leaving the user
with no notification to control the media. Only remove players from QS
when the media is paused.

Fixes: 160425592
Test: atest MediaDataFilterTest
Test: manual, switch resumption setting off, play media, and dismiss
from QQS

Change-Id: Ia7c9a8d9b100506f2c12ef2d14e4a952e151bd05
Merged-in: Ia7c9a8d9b100506f2c12ef2d14e4a952e151bd05
(cherry picked from commit 11a332b7c4f899b2bdab19f5710a876dda23acb9)

7 files changed, 81 insertions, 51 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
index d4ae0b53d15d..1808035df027 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
@@ -43,7 +43,7 @@ class MediaCarouselController @Inject constructor(
     private val mediaHostStatesManager: MediaHostStatesManager,
     private val activityStarter: ActivityStarter,
     @Main executor: DelayableExecutor,
-    mediaManager: MediaDataManager,
+    private val mediaManager: MediaDataManager,
     configurationController: ConfigurationController,
     falsingManager: FalsingManager
 ) {
@@ -110,6 +110,7 @@ class MediaCarouselController @Inject constructor(
     private val pageIndicator: PageIndicator
     private val visualStabilityCallback: VisualStabilityManager.Callback
     private var needsReordering: Boolean = false
+    private var keysNeedRemoval = mutableSetOf<String>()
     private var isRtl: Boolean = false
         set(value) {
             if (value != field) {
@@ -161,6 +162,10 @@ class MediaCarouselController @Inject constructor(
                 needsReordering = false
                 reorderAllPlayers()
             }
+
+            keysNeedRemoval.forEach { removePlayer(it) }
+            keysNeedRemoval.clear()
+
             // Let's reset our scroll position
             mediaCarouselScrollHandler.scrollToStart()
         }
@@ -168,13 +173,19 @@ class MediaCarouselController @Inject constructor(
                 true /* persistent */)
         mediaManager.addListener(object : MediaDataManager.Listener {
             override fun onMediaDataLoaded(key: String, oldKey: String?, data: MediaData) {
-                if (!data.active && !Utils.useMediaResumption(context)) {
-                    // This view is inactive, let's remove this! This happens e.g when dismissing /
-                    // timing out a view. We still have the data around because resumption could
-                    // be on, but we should save the resources and release this.
-                    onMediaDataRemoved(key)
+                addOrUpdatePlayer(key, oldKey, data)
+                val canRemove = data.isPlaying?.let { !it } ?: data.isClearable
+                if (canRemove && !Utils.useMediaResumption(context)) {
+                    // This view isn't playing, let's remove this! This happens e.g when
+                    // dismissing/timing out a view. We still have the data around because
+                    // resumption could be on, but we should save the resources and release this.
+                    if (visualStabilityManager.isReorderingAllowed) {
+                        onMediaDataRemoved(key)
+                    } else {
+                        keysNeedRemoval.add(key)
+                    }
                 } else {
-                    addOrUpdatePlayer(key, oldKey, data)
+                    keysNeedRemoval.remove(key)
                 }
             }
 
@@ -236,12 +247,12 @@ class MediaCarouselController @Inject constructor(
             var newPlayer = mediaControlPanelFactory.get()
             newPlayer.attach(PlayerViewHolder.create(LayoutInflater.from(context), mediaContent))
             newPlayer.mediaViewController.sizeChangedListener = this::updateCarouselDimensions
-            MediaPlayerData.addMediaPlayer(key, data, newPlayer)
             val lp = LinearLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
                     ViewGroup.LayoutParams.WRAP_CONTENT)
             newPlayer.view?.player?.setLayoutParams(lp)
             newPlayer.bind(data)
             newPlayer.setListening(currentlyExpanded)
+            MediaPlayerData.addMediaPlayer(key, data, newPlayer)
             updatePlayerToState(newPlayer, noAnimation = true)
             reorderAllPlayers()
         } else {
@@ -271,6 +282,9 @@ class MediaCarouselController @Inject constructor(
             removed.onDestroy()
             mediaCarouselScrollHandler.onPlayersChanged()
             updatePageIndicator()
+
+            // Inform the media manager of a potentially late dismissal
+            mediaManager.dismissMediaData(key, 0L)
         }
     }
 
@@ -478,12 +492,11 @@ class MediaCarouselController @Inject constructor(
 internal object MediaPlayerData {
     private data class MediaSortKey(
         val data: MediaData,
-        val updateTime: Long = 0,
-        val isPlaying: Boolean = false
+        val updateTime: Long = 0
     )
 
     private val comparator =
-        compareByDescending<MediaSortKey> { it.isPlaying }
+        compareByDescending<MediaSortKey> { it.data.isPlaying }
         .thenByDescending { it.data.isLocalSession }
         .thenByDescending { !it.data.resumption }
         .thenByDescending { it.updateTime }
@@ -493,7 +506,7 @@ internal object MediaPlayerData {
 
     fun addMediaPlayer(key: String, data: MediaData, player: MediaControlPanel) {
         removeMediaPlayer(key)
-        val sortKey = MediaSortKey(data, System.currentTimeMillis(), player.isPlaying())
+        val sortKey = MediaSortKey(data, System.currentTimeMillis())
         mediaData.put(key, sortKey)
         mediaPlayers.put(sortKey, player)
     }
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt
index d6a02687c905..40a879abde34 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt
@@ -94,7 +94,17 @@ data class MediaData(
      * Notification key for cancelling a media player after a timeout (when not using resumption.)
      */
     val notificationKey: String? = null,
-    var hasCheckedForResume: Boolean = false
+    var hasCheckedForResume: Boolean = false,
+
+    /**
+     * If apps do not report PlaybackState, set as null to imply 'undetermined'
+     */
+    val isPlaying: Boolean? = null,
+
+    /**
+     * Set from the notification and used as fallback when PlaybackState cannot be determined
+     */
+    val isClearable: Boolean = true
 )
 
 /** State of a media action. */
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt
index 0664a41f841d..1f580a953d09 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt
@@ -136,14 +136,8 @@ class MediaDataFilter @Inject constructor(
 
     /**
      * Are there any media entries we should display?
-     * If resumption is enabled, this will include inactive players
-     * If resumption is disabled, we only want to show active players
      */
-    fun hasAnyMedia() = if (mediaResumeListener.isResumptionEnabled()) {
-        userEntries.isNotEmpty()
-    } else {
-        hasActiveMedia()
-    }
+    fun hasAnyMedia() = userEntries.isNotEmpty()
 
     /**
      * Add a listener for filtered [MediaData] changes
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt
index a2a738788e3e..bb11efeebf08 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt
@@ -45,6 +45,7 @@ import com.android.systemui.broadcast.BroadcastDispatcher
 import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.dump.DumpManager
+import com.android.systemui.statusbar.NotificationMediaManager.isPlayingState
 import com.android.systemui.statusbar.notification.MediaNotificationProcessor
 import com.android.systemui.statusbar.notification.row.HybridGroupManager
 import com.android.systemui.util.Assert
@@ -336,6 +337,16 @@ class MediaDataManager(
     }
 
     fun dismissMediaData(key: String, delay: Long) {
+        backgroundExecutor.execute {
+            mediaEntries[key]?.let { mediaData ->
+                if (mediaData.isLocalSession) {
+                    mediaData.token?.let {
+                        val mediaController = mediaControllerFactory.create(it)
+                        mediaController.transportControls.stop()
+                    }
+                }
+            }
+        }
         foregroundExecutor.executeDelayed({ removeEntry(key) }, delay)
     }
 
@@ -483,6 +494,7 @@ class MediaDataManager(
 
         val isLocalSession = mediaController.playbackInfo?.playbackType ==
             MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL ?: true
+        val isPlaying = mediaController.playbackState?.let { isPlayingState(it.state) } ?: null
 
         foregroundExecutor.execute {
             val resumeAction: Runnable? = mediaEntries[key]?.resumeAction
@@ -492,7 +504,8 @@ class MediaDataManager(
                     smallIconDrawable, artist, song, artWorkIcon, actionIcons,
                     actionsToShowCollapsed, sbn.packageName, token, notif.contentIntent, null,
                     active, resumeAction = resumeAction, isLocalSession = isLocalSession,
-                    notificationKey = key, hasCheckedForResume = hasCheckedForResume))
+                    notificationKey = key, hasCheckedForResume = hasCheckedForResume,
+                    isPlaying = isPlaying, isClearable = sbn.isClearable()))
         }
     }
 
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
index 4ec746fcb153..1ac3034ea7c1 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
@@ -116,8 +116,6 @@ class MediaResumeListener @Inject constructor(
         }, Settings.Secure.MEDIA_CONTROLS_RESUME)
     }
 
-    fun isResumptionEnabled() = useMediaResumption
-
     private fun loadSavedComponents() {
         // Make sure list is empty (if we switched users)
         resumeComponents.clear()
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java
index 89538ac8bc9f..609b8474d134 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java
@@ -74,7 +74,7 @@ public class MediaDataCombineLatestTest extends SysuiTestCase {
 
         mMediaData = new MediaData(USER_ID, true, BG_COLOR, APP, null, ARTIST, TITLE, null,
                 new ArrayList<>(), new ArrayList<>(), PACKAGE, null, null, null, true, null, true,
-                false, KEY, false);
+                false, KEY, false, false, false);
         mDeviceData = new MediaDeviceData(true, null, DEVICE_NAME);
     }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt
index 118cffc2d5b8..00b003d290ee 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt
@@ -24,7 +24,6 @@ import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.mock
-import org.mockito.Mockito.`when` as whenever
 
 @SmallTest
 @RunWith(AndroidTestingRunner::class)
@@ -33,6 +32,8 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     companion object {
         val LOCAL = true
         val RESUMPTION = true
+        val PLAYING = true
+        val UNDETERMINED = null
     }
 
     @Before
@@ -43,15 +44,13 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     @Test
     fun addPlayingThenRemote() {
         val playerIsPlaying = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying.isPlaying).thenReturn(true)
-        val dataIsPlaying = createMediaData(LOCAL, !RESUMPTION)
+        val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
 
         val playerIsRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsRemote.isPlaying).thenReturn(false)
-        val dataIsRemote = createMediaData(!LOCAL, !RESUMPTION)
+        val dataIsRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)
 
-        MediaPlayerData.addMediaPlayer("1", dataIsPlaying, playerIsPlaying)
         MediaPlayerData.addMediaPlayer("2", dataIsRemote, playerIsRemote)
+        MediaPlayerData.addMediaPlayer("1", dataIsPlaying, playerIsPlaying)
 
         val players = MediaPlayerData.players()
         assertThat(players).hasSize(2)
@@ -61,18 +60,16 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     @Test
     fun switchPlayersPlaying() {
         val playerIsPlaying1 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying1.isPlaying).thenReturn(true)
-        val dataIsPlaying1 = createMediaData(LOCAL, !RESUMPTION)
+        var dataIsPlaying1 = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
 
         val playerIsPlaying2 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying2.isPlaying).thenReturn(false)
-        val dataIsPlaying2 = createMediaData(LOCAL, !RESUMPTION)
+        var dataIsPlaying2 = createMediaData("app2", !PLAYING, LOCAL, !RESUMPTION)
 
         MediaPlayerData.addMediaPlayer("1", dataIsPlaying1, playerIsPlaying1)
         MediaPlayerData.addMediaPlayer("2", dataIsPlaying2, playerIsPlaying2)
 
-        whenever(playerIsPlaying1.isPlaying).thenReturn(false)
-        whenever(playerIsPlaying2.isPlaying).thenReturn(true)
+        dataIsPlaying1 = createMediaData("app1", !PLAYING, LOCAL, !RESUMPTION)
+        dataIsPlaying2 = createMediaData("app2", PLAYING, LOCAL, !RESUMPTION)
 
         MediaPlayerData.addMediaPlayer("1", dataIsPlaying1, playerIsPlaying1)
         MediaPlayerData.addMediaPlayer("2", dataIsPlaying2, playerIsPlaying2)
@@ -85,38 +82,43 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     @Test
     fun fullOrderTest() {
         val playerIsPlaying = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying.isPlaying).thenReturn(true)
-        val dataIsPlaying = createMediaData(LOCAL, !RESUMPTION)
+        val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
 
         val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlayingAndRemote.isPlaying).thenReturn(true)
-        val dataIsPlayingAndRemote = createMediaData(!LOCAL, !RESUMPTION)
+        val dataIsPlayingAndRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)
 
         val playerIsStoppedAndLocal = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
-        val dataIsStoppedAndLocal = createMediaData(LOCAL, !RESUMPTION)
+        val dataIsStoppedAndLocal = createMediaData("app3", !PLAYING, LOCAL, !RESUMPTION)
 
         val playerIsStoppedAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
-        val dataIsStoppedAndRemote = createMediaData(!LOCAL, !RESUMPTION)
+        val dataIsStoppedAndRemote = createMediaData("app4", !PLAYING, !LOCAL, !RESUMPTION)
 
         val playerCanResume = mock(MediaControlPanel::class.java)
-        whenever(playerCanResume.isPlaying).thenReturn(false)
-        val dataCanResume = createMediaData(LOCAL, RESUMPTION)
+        val dataCanResume = createMediaData("app5", !PLAYING, LOCAL, RESUMPTION)
+
+        val playerUndetermined = mock(MediaControlPanel::class.java)
+        val dataUndetermined = createMediaData("app6", UNDETERMINED, LOCAL, RESUMPTION)
 
         MediaPlayerData.addMediaPlayer("3", dataIsStoppedAndLocal, playerIsStoppedAndLocal)
         MediaPlayerData.addMediaPlayer("5", dataIsStoppedAndRemote, playerIsStoppedAndRemote)
         MediaPlayerData.addMediaPlayer("4", dataCanResume, playerCanResume)
         MediaPlayerData.addMediaPlayer("1", dataIsPlaying, playerIsPlaying)
         MediaPlayerData.addMediaPlayer("2", dataIsPlayingAndRemote, playerIsPlayingAndRemote)
+        MediaPlayerData.addMediaPlayer("6", dataUndetermined, playerUndetermined)
 
         val players = MediaPlayerData.players()
-        assertThat(players).hasSize(5)
+        assertThat(players).hasSize(6)
         assertThat(players).containsExactly(playerIsPlaying, playerIsPlayingAndRemote,
-            playerIsStoppedAndLocal, playerCanResume, playerIsStoppedAndRemote).inOrder()
+            playerIsStoppedAndLocal, playerCanResume, playerIsStoppedAndRemote,
+            playerUndetermined).inOrder()
     }
 
-    private fun createMediaData(isLocalSession: Boolean, resumption: Boolean) =
-        MediaData(0, false, 0, null, null, null, null, null, emptyList(), emptyList<Int>(), "",
-            null, null, null, true, null, isLocalSession, resumption, null, false)
+    private fun createMediaData(
+        app: String,
+        isPlaying: Boolean?,
+        isLocalSession: Boolean,
+        resumption: Boolean
+    ) =
+        MediaData(0, false, 0, app, null, null, null, null, emptyList(), emptyList<Int>(), "",
+            null, null, null, true, null, isLocalSession, resumption, null, false, isPlaying)
 }