Merge "Track order of visible media players" into tm-qpr-dev am: 3009a9e14e

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

Change-Id: Id70a112b61091a0256e53ecdd27c6dd18db853a2
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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

@@ -133,7 +133,7 @@ class MediaCarouselController @Inject constructor(
     private val visualStabilityCallback: OnReorderingAllowedListener
     private var needsReordering: Boolean = false
     private var keysNeedRemoval = mutableSetOf<String>()
-    var shouldScrollToActivePlayer: Boolean = false
+    var shouldScrollToKey: Boolean = false
     private var isRtl: Boolean = false
         set(value) {
             if (value != field) {
@@ -436,7 +436,10 @@ class MediaCarouselController @Inject constructor(
         return mediaCarousel
     }
 
-    private fun reorderAllPlayers(previousVisiblePlayerKey: MediaPlayerData.MediaSortKey?) {
+    private fun reorderAllPlayers(
+            previousVisiblePlayerKey: MediaPlayerData.MediaSortKey?,
+            key: String? = null
+    ) {
         mediaContent.removeAllViews()
         for (mediaPlayer in MediaPlayerData.players()) {
             mediaPlayer.mediaViewHolder?.let {
@@ -446,18 +449,18 @@ class MediaCarouselController @Inject constructor(
             }
         }
         mediaCarouselScrollHandler.onPlayersChanged()
-
+        MediaPlayerData.updateVisibleMediaPlayers()
         // Automatically scroll to the active player if needed
-        if (shouldScrollToActivePlayer) {
+        if (shouldScrollToKey) {
-            shouldScrollToActivePlayer = false
+            shouldScrollToKey = false
-            val activeMediaIndex = MediaPlayerData.firstActiveMediaIndex()
+            val mediaIndex = key?.let { MediaPlayerData.getMediaPlayerIndex(it) } ?: -1
-            if (activeMediaIndex != -1) {
+            if (mediaIndex != -1) {
                 previousVisiblePlayerKey?.let {
                     val previousVisibleIndex = MediaPlayerData.playerKeys()
                             .indexOfFirst { key -> it == key }
                     mediaCarouselScrollHandler
-                            .scrollToPlayer(previousVisibleIndex, activeMediaIndex)
+                            .scrollToPlayer(previousVisibleIndex, mediaIndex)
-                } ?: mediaCarouselScrollHandler.scrollToPlayer(destIndex = activeMediaIndex)
+                } ?: mediaCarouselScrollHandler.scrollToPlayer(destIndex = mediaIndex)
             }
         }
     }
@@ -471,9 +474,8 @@ class MediaCarouselController @Inject constructor(
     ): Boolean = traceSection("MediaCarouselController#addOrUpdatePlayer") {
         MediaPlayerData.moveIfExists(oldKey, key)
         val existingPlayer = MediaPlayerData.getMediaPlayer(key)
-        val curVisibleMediaKey = MediaPlayerData.playerKeys()
+        val curVisibleMediaKey = MediaPlayerData.visiblePlayerKeys()
                 .elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex)
-        val isCurVisibleMediaPlaying = curVisibleMediaKey?.data?.isPlaying
         if (existingPlayer == null) {
             val newPlayer = mediaControlPanelFactory.get()
             newPlayer.attachPlayer(MediaViewHolder.create(
@@ -488,8 +490,10 @@ class MediaCarouselController @Inject constructor(
                 key, data, newPlayer, systemClock, isSsReactivated, debugLogger
             )
             updatePlayerToState(newPlayer, noAnimation = true)
-            if (data.active) {
+            // Media data added from a recommendation card should starts playing.
-                reorderAllPlayers(curVisibleMediaKey)
+            if ((shouldScrollToKey && data.isPlaying == true) ||
+                    (!shouldScrollToKey && data.active)) {
+                reorderAllPlayers(curVisibleMediaKey, key)
             } else {
                 needsReordering = true
             }
@@ -498,14 +502,16 @@ class MediaCarouselController @Inject constructor(
             MediaPlayerData.addMediaPlayer(
                 key, data, existingPlayer, systemClock, isSsReactivated, debugLogger
             )
-            // Check the playing status of both current visible and new media players
+            val packageName = MediaPlayerData.smartspaceMediaData?.packageName ?: String()
-            // To make sure we scroll to the active playing media card.
+            // In case of recommendations hits.
+            // Check the playing status of media player and the package name.
+            // To make sure we scroll to the right app's media player.
             if (isReorderingAllowed ||
-                    shouldScrollToActivePlayer &&
+                    shouldScrollToKey &&
                     data.isPlaying == true &&
-                    isCurVisibleMediaPlaying == false
+                    packageName == data.packageName
             ) {
-                reorderAllPlayers(curVisibleMediaKey)
+                reorderAllPlayers(curVisibleMediaKey, key)
             } else {
                 needsReordering = true
             }
@@ -534,7 +540,7 @@ class MediaCarouselController @Inject constructor(
 
         val existingSmartspaceMediaKey = MediaPlayerData.smartspaceMediaKey()
         existingSmartspaceMediaKey?.let {
-            val removedPlayer = MediaPlayerData.removeMediaPlayer(existingSmartspaceMediaKey)
+            val removedPlayer = MediaPlayerData.removeMediaPlayer(existingSmartspaceMediaKey, true)
             removedPlayer?.run { debugLogger.logPotentialMemoryLeak(existingSmartspaceMediaKey) }
         }
 
@@ -546,7 +552,7 @@ class MediaCarouselController @Inject constructor(
                 ViewGroup.LayoutParams.WRAP_CONTENT)
         newRecs.recommendationViewHolder?.recommendations?.setLayoutParams(lp)
         newRecs.bindRecommendation(data)
-        val curVisibleMediaKey = MediaPlayerData.playerKeys()
+        val curVisibleMediaKey = MediaPlayerData.visiblePlayerKeys()
                 .elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex)
         MediaPlayerData.addMediaRecommendation(
             key, data, newRecs, shouldPrioritize, systemClock, debugLogger
@@ -572,7 +578,10 @@ class MediaCarouselController @Inject constructor(
                 logger.logRecommendationRemoved(it.packageName, it.instanceId)
             }
         }
-        val removed = MediaPlayerData.removeMediaPlayer(key)
+        val removed = MediaPlayerData.removeMediaPlayer(
+                key,
+                dismissMediaData || dismissRecommendation
+        )
         removed?.apply {
             mediaCarouselScrollHandler.onPrePlayerRemoved(removed)
             mediaContent.removeView(removed.mediaViewHolder?.player)
@@ -835,18 +844,20 @@ class MediaCarouselController @Inject constructor(
     fun logSmartspaceImpression(qsExpanded: Boolean) {
         val visibleMediaIndex = mediaCarouselScrollHandler.visibleMediaIndex
         if (MediaPlayerData.players().size > visibleMediaIndex) {
-            val mediaControlPanel = MediaPlayerData.players().elementAt(visibleMediaIndex)
+            val mediaControlPanel = MediaPlayerData.getMediaControlPanel(visibleMediaIndex)
             val hasActiveMediaOrRecommendationCard =
                     MediaPlayerData.hasActiveMediaOrRecommendationCard()
             if (!hasActiveMediaOrRecommendationCard && !qsExpanded) {
                 // Skip logging if on LS or QQS, and there is no active media card
                 return
             }
+            mediaControlPanel?.let {
                 logSmartspaceCardReported(800, // SMARTSPACE_CARD_SEEN
-                    mediaControlPanel.mSmartspaceId,
+                        it.mSmartspaceId,
-                    mediaControlPanel.mUid,
+                        it.mUid,
-                    intArrayOf(mediaControlPanel.surfaceForSmartspaceLogging))
+                        intArrayOf(it.surfaceForSmartspaceLogging))
-            mediaControlPanel.mIsImpressed = true
+                it.mIsImpressed = true
+            }
         }
     }
 
@@ -885,7 +896,7 @@ class MediaCarouselController @Inject constructor(
             return
         }
 
-        val mediaControlKey = MediaPlayerData.playerKeys().elementAt(rank)
+        val mediaControlKey = MediaPlayerData.visiblePlayerKeys().elementAt(rank)
         // Only log media resume card when Smartspace data is available
         if (!mediaControlKey.isSsMediaRec &&
                 !mediaManager.smartspaceMediaData.isActive &&
@@ -960,7 +971,8 @@ class MediaCarouselController @Inject constructor(
         pw.apply {
             println("keysNeedRemoval: $keysNeedRemoval")
             println("dataKeys: ${MediaPlayerData.dataKeys()}")
-            println("playerSortKeys: ${MediaPlayerData.playerKeys()}")
+            println("orderedPlayerSortKeys: ${MediaPlayerData.playerKeys()}")
+            println("visiblePlayerSortKeys: ${MediaPlayerData.visiblePlayerKeys()}")
             println("smartspaceMediaData: ${MediaPlayerData.smartspaceMediaData}")
             println("shouldPrioritizeSs: ${MediaPlayerData.shouldPrioritizeSs}")
             println("current size: $currentCarouselWidth x $currentCarouselHeight")
@@ -1000,6 +1012,7 @@ internal object MediaPlayerData {
     data class MediaSortKey(
         val isSsMediaRec: Boolean, // Whether the item represents a Smartspace media recommendation.
         val data: MediaData,
+        val key: String,
         val updateTime: Long = 0,
         val isSsReactivated: Boolean = false
     )
@@ -1018,6 +1031,8 @@ internal object MediaPlayerData {
 
     private val mediaPlayers = TreeMap<MediaSortKey, MediaControlPanel>(comparator)
     private val mediaData: MutableMap<String, MediaSortKey> = mutableMapOf()
+    // A map that tracks order of visible media players before they get reordered.
+    private val visibleMediaPlayers = LinkedHashMap<String, MediaSortKey>()
 
     fun addMediaPlayer(
         key: String,
@@ -1032,9 +1047,10 @@ internal object MediaPlayerData {
             debugLogger?.logPotentialMemoryLeak(key)
         }
         val sortKey = MediaSortKey(isSsMediaRec = false,
-                data, clock.currentTimeMillis(), isSsReactivated = isSsReactivated)
+                data, key, clock.currentTimeMillis(), isSsReactivated = isSsReactivated)
         mediaData.put(key, sortKey)
         mediaPlayers.put(sortKey, player)
+        visibleMediaPlayers.put(key, sortKey)
     }
 
     fun addMediaRecommendation(
@@ -1050,10 +1066,16 @@ internal object MediaPlayerData {
         if (removedPlayer != null && removedPlayer != player) {
             debugLogger?.logPotentialMemoryLeak(key)
         }
-        val sortKey = MediaSortKey(isSsMediaRec = true,
+        val sortKey = MediaSortKey(
-            EMPTY.copy(isPlaying = false), clock.currentTimeMillis(), isSsReactivated = true)
+            isSsMediaRec = true,
+            EMPTY.copy(isPlaying = false),
+            key,
+            clock.currentTimeMillis(),
+            isSsReactivated = true
+        )
         mediaData.put(key, sortKey)
         mediaPlayers.put(sortKey, player)
+        visibleMediaPlayers.put(key, sortKey)
         smartspaceMediaData = data
     }
 
@@ -1067,12 +1089,18 @@ internal object MediaPlayerData {
         }
 
         mediaData.remove(oldKey)?.let {
+            // MediaPlayer should not be visible
+            // no need to set isDismissed flag.
             val removedPlayer = removeMediaPlayer(newKey)
             removedPlayer?.run { debugLogger?.logPotentialMemoryLeak(newKey) }
             mediaData.put(newKey, it)
         }
     }
 
+    fun getMediaControlPanel(visibleIndex: Int): MediaControlPanel? {
+        return mediaPlayers.get(visiblePlayerKeys().elementAt(visibleIndex))
+    }
+
     fun getMediaPlayer(key: String): MediaControlPanel? {
         return mediaData.get(key)?.let { mediaPlayers.get(it) }
     }
@@ -1087,10 +1115,17 @@ internal object MediaPlayerData {
         return -1
     }
 
-    fun removeMediaPlayer(key: String) = mediaData.remove(key)?.let {
+    /**
+     * Removes media player given the key.
+     * @param isDismissed determines whether the media player is removed from the carousel.
+     */
+    fun removeMediaPlayer(key: String, isDismissed: Boolean = false) = mediaData.remove(key)?.let {
         if (it.isSsMediaRec) {
             smartspaceMediaData = null
         }
+        if (isDismissed) {
+            visibleMediaPlayers.remove(key)
+        }
         mediaPlayers.remove(it)
     }
 
@@ -1102,6 +1137,8 @@ internal object MediaPlayerData {
 
     fun playerKeys() = mediaPlayers.keys
 
+    fun visiblePlayerKeys() = visibleMediaPlayers.values
+
     /** Returns the index of the first non-timeout media. */
     fun firstActiveMediaIndex(): Int {
         mediaPlayers.entries.forEachIndexed { index, e ->
@@ -1126,6 +1163,7 @@ internal object MediaPlayerData {
     fun clear() {
         mediaData.clear()
         mediaPlayers.clear()
+        visibleMediaPlayers.clear()
     }
 
     /* Returns true if there is active media player card or recommendation card */
@@ -1140,4 +1178,16 @@ internal object MediaPlayerData {
     }
 
     fun isSsReactivated(key: String): Boolean = mediaData.get(key)?.isSsReactivated ?: false
+
+    /**
+     * This method is called when media players are reordered.
+     * To make sure we have the new version of the order of
+     * media players visible to user.
+     */
+    fun updateVisibleMediaPlayers() {
+        visibleMediaPlayers.clear()
+        playerKeys().forEach {
+            visibleMediaPlayers.put(it.key, it)
+        }
+    }
 }

packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java

@@ -1441,7 +1441,7 @@ public class MediaControlPanel {
             }
 
             // Automatically scroll to the active player once the media is loaded.
-            mMediaCarouselController.setShouldScrollToActivePlayer(true);
+            mMediaCarouselController.setShouldScrollToKey(true);
         });
     }
 

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

@@ -76,7 +76,6 @@ class MediaCarouselControllerTest : SysuiTestCase() {
     @Mock lateinit var dumpManager: DumpManager
     @Mock lateinit var logger: MediaUiEventLogger
     @Mock lateinit var debugLogger: MediaCarouselControllerLogger
-    @Mock lateinit var mediaPlayer: MediaControlPanel
     @Mock lateinit var mediaViewController: MediaViewController
     @Mock lateinit var smartspaceMediaData: SmartspaceMediaData
     @Captor lateinit var listener: ArgumentCaptor<MediaDataManager.Listener>
@@ -107,8 +106,8 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         verify(mediaDataManager).addListener(capture(listener))
         verify(visualStabilityProvider)
             .addPersistentReorderingAllowedListener(capture(visualStabilityCallback))
-        whenever(mediaControlPanelFactory.get()).thenReturn(mediaPlayer)
+        whenever(mediaControlPanelFactory.get()).thenReturn(panel)
-        whenever(mediaPlayer.mediaViewController).thenReturn(mediaViewController)
+        whenever(panel.mediaViewController).thenReturn(mediaViewController)
         whenever(mediaDataManager.smartspaceMediaData).thenReturn(smartspaceMediaData)
         MediaPlayerData.clear()
     }
@@ -189,6 +188,10 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         for ((index, key) in MediaPlayerData.playerKeys().withIndex()) {
             assertEquals(expected.get(index).first, key.data.notificationKey)
         }
+
+        for ((index, key) in MediaPlayerData.visiblePlayerKeys().withIndex()) {
+            assertEquals(expected.get(index).first, key.data.notificationKey)
+        }
     }
 
     @Test
@@ -203,6 +206,22 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         assertTrue(MediaPlayerData.playerKeys().elementAt(2).isSsMediaRec)
     }
 
+    @Test
+    fun testOrderWithSmartspace_prioritized_updatingVisibleMediaPlayers() {
+        testPlayerOrdering()
+
+        // If smartspace is prioritized
+        listener.value.onSmartspaceMediaDataLoaded(
+                SMARTSPACE_KEY,
+                EMPTY_SMARTSPACE_MEDIA_DATA.copy(isActive = true),
+                true
+        )
+
+        // Then it should be shown immediately after any actively playing controls
+        assertTrue(MediaPlayerData.playerKeys().elementAt(2).isSsMediaRec)
+        assertTrue(MediaPlayerData.visiblePlayerKeys().elementAt(2).isSsMediaRec)
+    }
+
     @Test
     fun testOrderWithSmartspace_notPrioritized() {
         testPlayerOrdering()
@@ -216,6 +235,31 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         assertTrue(MediaPlayerData.playerKeys().elementAt(idx).isSsMediaRec)
     }
 
+    @Test
+    fun testPlayingExistingMediaPlayerFromCarousel_visibleMediaPlayersNotUpdated() {
+        testPlayerOrdering()
+        // playing paused player
+        listener.value.onMediaDataLoaded("paused local",
+                "paused local",
+                DATA.copy(active = true, isPlaying = true,
+                        playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false))
+        listener.value.onMediaDataLoaded("playing local",
+                "playing local",
+                DATA.copy(active = true, isPlaying = false,
+                        playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = true)
+        )
+
+        assertEquals(
+                MediaPlayerData.getMediaPlayerIndex("paused local"),
+                mediaCarouselController.mediaCarouselScrollHandler.visibleMediaIndex
+        )
+        // paused player order should stays the same in visibleMediaPLayer map.
+        // paused player order should be first in mediaPlayer map.
+        assertEquals(
+                MediaPlayerData.visiblePlayerKeys().elementAt(3),
+                MediaPlayerData.playerKeys().elementAt(0)
+        )
+    }
     @Test
     fun testSwipeDismiss_logged() {
         mediaCarouselController.mediaCarouselScrollHandler.dismissCallback.invoke()
@@ -295,7 +339,7 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         // adding a media recommendation card.
         listener.value.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, EMPTY_SMARTSPACE_MEDIA_DATA,
                 false)
-        mediaCarouselController.shouldScrollToActivePlayer = true
+        mediaCarouselController.shouldScrollToKey = true
         // switching between media players.
         listener.value.onMediaDataLoaded("playing local",
         "playing local",
@@ -315,8 +359,11 @@ class MediaCarouselControllerTest : SysuiTestCase() {
 
     @Test
     fun testMediaLoadedFromRecommendationCard_ScrollToActivePlayer() {
-        MediaPlayerData.addMediaRecommendation(SMARTSPACE_KEY, EMPTY_SMARTSPACE_MEDIA_DATA, panel,
+        listener.value.onSmartspaceMediaDataLoaded(
-                false, clock)
+                SMARTSPACE_KEY,
+                EMPTY_SMARTSPACE_MEDIA_DATA.copy(packageName = "PACKAGE_NAME", isActive = true),
+                false
+        )
         listener.value.onMediaDataLoaded("playing local",
                 null,
                 DATA.copy(active = true, isPlaying = true,
@@ -332,10 +379,12 @@ class MediaCarouselControllerTest : SysuiTestCase() {
 
         // Replaying the same media player one more time.
         // And check that the card stays in its position.
+        mediaCarouselController.shouldScrollToKey = true
         listener.value.onMediaDataLoaded("playing local",
                 null,
                 DATA.copy(active = true, isPlaying = true,
-                        playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false)
+                        playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false,
+                        packageName = "PACKAGE_NAME")
         )
         playerIndex = MediaPlayerData.getMediaPlayerIndex("playing local")
         assertEquals(playerIndex, 0)