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

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

@@ -42,7 +42,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
 ) {
@@ -109,6 +109,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)) {
+                addOrUpdatePlayer(key, oldKey, data)
-                    // This view is inactive, let's remove this! This happens e.g when dismissing /
+                val canRemove = data.isPlaying?.let { !it } ?: data.isClearable
-                    // timing out a view. We still have the data around because resumption could
+                if (canRemove && !Utils.useMediaResumption(context)) {
-                    // be on, but we should save the resources and release this.
+                    // This view isn't playing, let's remove this! This happens e.g when
-                    onMediaDataRemoved(key)
+                    // 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 updateTime: Long = 0
-        val isPlaying: Boolean = false
     )
 
     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)
     }

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. */

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()) {
+    fun hasAnyMedia() = userEntries.isNotEmpty()
-        userEntries.isNotEmpty()
-    } else {
-        hasActiveMedia()
-    }
 
     /**
      * Add a listener for filtered [MediaData] changes

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

@@ -47,6 +47,7 @@ import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.ActivityStarter
+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
@@ -350,6 +351,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)
     }
 
@@ -500,6 +511,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
@@ -509,7 +521,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()))
         }
     }
 

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

@@ -125,8 +125,6 @@ class MediaResumeListener @Inject constructor(
         }, Settings.Secure.MEDIA_CONTROLS_RESUME_BLOCKED)
     }
 
-    fun isResumptionEnabled() = useMediaResumption
-
     private fun loadSavedComponents() {
         // Make sure list is empty (if we switched users)
         resumeComponents.clear()

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);
     }
 

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

@@ -25,7 +25,6 @@ import org.junit.Ignore
 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)
@@ -34,6 +33,8 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     companion object {
         val LOCAL = true
         val RESUMPTION = true
+        val PLAYING = true
+        val UNDETERMINED = null
     }
 
     @Before
@@ -44,15 +45,13 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     @Test
     fun addPlayingThenRemote() {
         val playerIsPlaying = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying.isPlaying).thenReturn(true)
+        val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying = createMediaData("app1", LOCAL, !RESUMPTION)
 
         val playerIsRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsRemote.isPlaying).thenReturn(false)
+        val dataIsRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)
-        val dataIsRemote = createMediaData("app2", !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)
@@ -63,18 +62,16 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     @Ignore("Flaky")
     fun switchPlayersPlaying() {
         val playerIsPlaying1 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying1.isPlaying).thenReturn(true)
+        var dataIsPlaying1 = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying1 = createMediaData("app1", LOCAL, !RESUMPTION)
 
         val playerIsPlaying2 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying2.isPlaying).thenReturn(false)
+        var dataIsPlaying2 = createMediaData("app2", !PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying2 = createMediaData("app2", LOCAL, !RESUMPTION)
 
         MediaPlayerData.addMediaPlayer("1", dataIsPlaying1, playerIsPlaying1)
         MediaPlayerData.addMediaPlayer("2", dataIsPlaying2, playerIsPlaying2)
 
-        whenever(playerIsPlaying1.isPlaying).thenReturn(false)
+        dataIsPlaying1 = createMediaData("app1", !PLAYING, LOCAL, !RESUMPTION)
-        whenever(playerIsPlaying2.isPlaying).thenReturn(true)
+        dataIsPlaying2 = createMediaData("app2", PLAYING, LOCAL, !RESUMPTION)
 
         MediaPlayerData.addMediaPlayer("1", dataIsPlaying1, playerIsPlaying1)
         MediaPlayerData.addMediaPlayer("2", dataIsPlaying2, playerIsPlaying2)
@@ -87,38 +84,43 @@ public class MediaPlayerDataTest : SysuiTestCase() {
     @Test
     fun fullOrderTest() {
         val playerIsPlaying = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying.isPlaying).thenReturn(true)
+        val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying = createMediaData("app1", LOCAL, !RESUMPTION)
 
         val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlayingAndRemote.isPlaying).thenReturn(true)
+        val dataIsPlayingAndRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)
-        val dataIsPlayingAndRemote = createMediaData("app2", !LOCAL, !RESUMPTION)
 
         val playerIsStoppedAndLocal = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
+        val dataIsStoppedAndLocal = createMediaData("app3", !PLAYING, LOCAL, !RESUMPTION)
-        val dataIsStoppedAndLocal = createMediaData("app3", LOCAL, !RESUMPTION)
 
         val playerIsStoppedAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
+        val dataIsStoppedAndRemote = createMediaData("app4", !PLAYING, !LOCAL, !RESUMPTION)
-        val dataIsStoppedAndRemote = createMediaData("app4", !LOCAL, !RESUMPTION)
 
         val playerCanResume = mock(MediaControlPanel::class.java)
-        whenever(playerCanResume.isPlaying).thenReturn(false)
+        val dataCanResume = createMediaData("app5", !PLAYING, LOCAL, RESUMPTION)
-        val dataCanResume = createMediaData("app5", 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(app: String, isLocalSession: Boolean, resumption: Boolean) =
+    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)
+            null, null, null, true, null, isLocalSession, resumption, null, false, isPlaying)
 }