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
2020-08-31 09:07:48 -04:00
parent 2c313a31d2
commit 11a332b7c4
7 changed files with 80 additions and 50 deletions
--- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
+++ b/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)) {
-                    // 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)
    }
--- 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. */
--- 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
--- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt
+++ b/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()))
        }
    }

--- a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt
+++ b/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()
--- 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);
    }

--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt
+++ b/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", LOCAL, !RESUMPTION)
+        val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)

        val playerIsRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsRemote.isPlaying).thenReturn(false)
-        val dataIsRemote = createMediaData("app2", !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)
@@ -63,18 +62,16 @@ public class MediaPlayerDataTest : SysuiTestCase() {
    @Ignore("Flaky")
    fun switchPlayersPlaying() {
        val playerIsPlaying1 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying1.isPlaying).thenReturn(true)
-        val dataIsPlaying1 = createMediaData("app1", LOCAL, !RESUMPTION)
+        var dataIsPlaying1 = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)

        val playerIsPlaying2 = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlaying2.isPlaying).thenReturn(false)
-        val dataIsPlaying2 = createMediaData("app2", 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)
@@ -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", LOCAL, !RESUMPTION)
+        val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)

        val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsPlayingAndRemote.isPlaying).thenReturn(true)
-        val dataIsPlayingAndRemote = createMediaData("app2", !LOCAL, !RESUMPTION)
+        val dataIsPlayingAndRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION)

        val playerIsStoppedAndLocal = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
-        val dataIsStoppedAndLocal = createMediaData("app3", LOCAL, !RESUMPTION)
+        val dataIsStoppedAndLocal = createMediaData("app3", !PLAYING, LOCAL, !RESUMPTION)

        val playerIsStoppedAndRemote = mock(MediaControlPanel::class.java)
-        whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false)
-        val dataIsStoppedAndRemote = createMediaData("app4", !LOCAL, !RESUMPTION)
+        val dataIsStoppedAndRemote = createMediaData("app4", !PLAYING, !LOCAL, !RESUMPTION)

        val playerCanResume = mock(MediaControlPanel::class.java)
-        whenever(playerCanResume.isPlaying).thenReturn(false)
-        val dataCanResume = createMediaData("app5", 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(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)
 }