Merge "Media - Leave playing media in QS" into rvc-qpr-dev am: 0766cdc08c

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

Change-Id: I26afbb1c8c6c34603d5191a56e3dad6a4fffe4b7

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)) {
+                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

@@ -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()))
         }
     }
 

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()

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

@@ -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("app1", PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying = createMediaData(LOCAL, !RESUMPTION)
 
         val playerIsRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsRemote.isPlaying).thenReturn(false)
+        val dataIsRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)
-        val dataIsRemote = createMediaData(!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)
+        var dataIsPlaying1 = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying1 = createMediaData(LOCAL, !RESUMPTION)
 
         val playerIsPlaying2 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying2.isPlaying).thenReturn(false)
+        var dataIsPlaying2 = createMediaData("app2", !PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying2 = createMediaData(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)
@@ -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("app1", PLAYING, LOCAL, !RESUMPTION)
-        val dataIsPlaying = createMediaData(LOCAL, !RESUMPTION)
 
         val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlayingAndRemote.isPlaying).thenReturn(true)
+        val dataIsPlayingAndRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)
-        val dataIsPlayingAndRemote = createMediaData(!LOCAL, !RESUMPTION)
 
         val playerIsStoppedAndLocal = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
+        val dataIsStoppedAndLocal = createMediaData("app3", !PLAYING, LOCAL, !RESUMPTION)
-        val dataIsStoppedAndLocal = createMediaData(LOCAL, !RESUMPTION)
 
         val playerIsStoppedAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
+        val dataIsStoppedAndRemote = createMediaData("app4", !PLAYING, !LOCAL, !RESUMPTION)
-        val dataIsStoppedAndRemote = createMediaData(!LOCAL, !RESUMPTION)
 
         val playerCanResume = mock(MediaControlPanel::class.java)
-        whenever(playerCanResume.isPlaying).thenReturn(false)
+        val dataCanResume = createMediaData("app5", !PLAYING, LOCAL, RESUMPTION)
-        val dataCanResume = createMediaData(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) =
+    private fun createMediaData(
-        MediaData(0, false, 0, null, null, null, null, null, emptyList(), emptyList<Int>(), "",
+        app: String,
-            null, null, null, true, null, isLocalSession, resumption, null, false)
+        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)
 }