Update media hosts after recommendations expire

Previously updateViewVisibility was only being called if the
recommendation card was removed from the carousel immediately, and never
if it timed out or was removed due to carousel visibility change.

This adds calls to that function when the card is actually removed in
those cases, so that MediaHosts have the correct visibility

Also adds more debug logging

Fixes: 245566782
Test: manual - verify shade looks normal after recommendations expire (steps in bug);
      verify recommendations do not disappear while visible
Test: atest
Change-Id: Ied7d504ca9fba85c66e5d72e16cac1347420b3e5

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

@@ -202,6 +202,7 @@ class MediaCarouselController @Inject constructor(
      * It will be called when the container is out of view.
      */
     lateinit var updateUserVisibility: () -> Unit
+    lateinit var updateHostVisibility: () -> Unit
 
     private val isReorderingAllowed: Boolean
         get() = visualStabilityProvider.isReorderingAllowed
@@ -225,7 +226,13 @@ class MediaCarouselController @Inject constructor(
                 reorderAllPlayers(previousVisiblePlayerKey = null)
             }
 
-            keysNeedRemoval.forEach { removePlayer(it) }
+            keysNeedRemoval.forEach {
+                removePlayer(it)
+            }
+            if (keysNeedRemoval.size > 0) {
+                // Carousel visibility may need to be updated after late removals
+                updateHostVisibility()
+            }
             keysNeedRemoval.clear()
 
             // Update user visibility so that no extra impression will be logged when
@@ -247,6 +254,7 @@ class MediaCarouselController @Inject constructor(
                 receivedSmartspaceCardLatency: Int,
                 isSsReactivated: Boolean
             ) {
+                debugLogger.logMediaLoaded(key)
                 if (addOrUpdatePlayer(key, oldKey, data, isSsReactivated)) {
                     // Log card received if a new resumable media card is added
                     MediaPlayerData.getMediaPlayer(key)?.let {
@@ -315,7 +323,7 @@ class MediaCarouselController @Inject constructor(
                 data: SmartspaceMediaData,
                 shouldPrioritize: Boolean
             ) {
-                if (DEBUG) Log.d(TAG, "Loading Smartspace media update")
+                debugLogger.logRecommendationLoaded(key)
                 // Log the case where the hidden media carousel with the existed inactive resume
                 // media is shown by the Smartspace signal.
                 if (data.isActive) {
@@ -370,13 +378,21 @@ class MediaCarouselController @Inject constructor(
             }
 
             override fun onMediaDataRemoved(key: String) {
+                debugLogger.logMediaRemoved(key)
                 removePlayer(key)
             }
 
             override fun onSmartspaceMediaDataRemoved(key: String, immediately: Boolean) {
-                if (DEBUG) Log.d(TAG, "My Smartspace media removal request is received")
+                debugLogger.logRecommendationRemoved(key, immediately)
                 if (immediately || isReorderingAllowed) {
-                    onMediaDataRemoved(key)
+                    removePlayer(key)
+                    if (!immediately) {
+                        // Although it wasn't requested, we were able to process the removal
+                        // immediately since reordering is allowed. So, notify hosts to update
+                        if (this@MediaCarouselController::updateHostVisibility.isInitialized) {
+                            updateHostVisibility()
+                        }
+                    }
                 } else {
                     keysNeedRemoval.add(key)
                 }

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

@@ -40,6 +40,37 @@ class MediaCarouselControllerLogger @Inject constructor(
                     "Removing control panel for $str1 from map without calling #onDestroy"
         }
     )
+
+    fun logMediaLoaded(key: String) = buffer.log(
+        TAG,
+        LogLevel.DEBUG,
+        { str1 = key },
+        { "add player $str1" }
+    )
+
+    fun logMediaRemoved(key: String) = buffer.log(
+        TAG,
+        LogLevel.DEBUG,
+        { str1 = key },
+        { "removing player $str1" }
+    )
+
+    fun logRecommendationLoaded(key: String) = buffer.log(
+        TAG,
+        LogLevel.DEBUG,
+        { str1 = key },
+        { "add recommendation $str1" }
+    )
+
+    fun logRecommendationRemoved(key: String, immediately: Boolean) = buffer.log(
+        TAG,
+        LogLevel.DEBUG,
+        {
+            str1 = key
+            bool1 = immediately
+        },
+        { "removing recommendation $str1, immediate=$bool1" }
+    )
 }
 
 private const val TAG = "MediaCarouselCtlrLog"

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

@@ -1322,6 +1322,7 @@ class MediaDataManager(
             println("externalListeners: ${mediaDataFilter.listeners}")
             println("mediaEntries: $mediaEntries")
             println("useMediaResumption: $useMediaResumption")
+            println("allowMediaRecommendations: $allowMediaRecommendations")
         }
     }
 }

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

@@ -546,6 +546,11 @@ class MediaHierarchyManager @Inject constructor(
         mediaCarouselController.updateUserVisibility = {
             mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = isVisibleToUser()
         }
+        mediaCarouselController.updateHostVisibility = {
+            mediaHosts.forEach {
+                it?.updateViewVisibility()
+            }
+        }
 
         panelEventsEvents.registerListener(object : NotifPanelEvents.Listener {
             override fun onExpandImmediateChanged(isExpandImmediateEnabled: Boolean) {

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

@@ -167,7 +167,11 @@ class MediaHost constructor(
         }
     }
 
-    private fun updateViewVisibility() {
+    /**
+     * Updates this host's state based on the current media data's status, and invokes listeners if
+     * the visibility has changed
+     */
+    fun updateViewVisibility() {
         state.visible = if (showsOnlyActiveMedia) {
             mediaDataManager.hasActiveMediaOrRecommendation()
         } else {

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

@@ -353,7 +353,7 @@ class MediaViewController @Inject constructor(
         type: TYPE
     ) = traceSection("MediaViewController#attach") {
         updateMediaViewControllerType(type)
-        logger.logMediaLocation("attach", currentStartLocation, currentEndLocation)
+        logger.logMediaLocation("attach $type", currentStartLocation, currentEndLocation)
         this.transitionLayout = transitionLayout
         layoutController.attach(transitionLayout)
         if (currentEndLocation == -1) {

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

@@ -26,6 +26,7 @@ import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.ActivityStarter
 import com.android.systemui.plugins.FalsingManager
+import com.android.systemui.statusbar.notification.collection.provider.OnReorderingAllowedListener
 import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider
 import com.android.systemui.statusbar.policy.ConfigurationController
 import com.android.systemui.util.animation.TransitionLayout
@@ -78,6 +79,7 @@ class MediaCarouselControllerTest : SysuiTestCase() {
     @Mock lateinit var mediaViewController: MediaViewController
     @Mock lateinit var smartspaceMediaData: SmartspaceMediaData
     @Captor lateinit var listener: ArgumentCaptor<MediaDataManager.Listener>
+    @Captor lateinit var visualStabilityCallback: ArgumentCaptor<OnReorderingAllowedListener>
 
     private val clock = FakeSystemClock()
     private lateinit var mediaCarouselController: MediaCarouselController
@@ -102,6 +104,8 @@ class MediaCarouselControllerTest : SysuiTestCase() {
             debugLogger
         )
         verify(mediaDataManager).addListener(capture(listener))
+        verify(visualStabilityProvider)
+            .addPersistentReorderingAllowedListener(capture(visualStabilityCallback))
         whenever(mediaControlPanelFactory.get()).thenReturn(mediaPlayer)
         whenever(mediaPlayer.mediaViewController).thenReturn(mediaViewController)
         whenever(mediaDataManager.smartspaceMediaData).thenReturn(smartspaceMediaData)
@@ -374,4 +378,28 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         playerIndex = MediaPlayerData.getMediaPlayerIndex("playing local")
         assertEquals(playerIndex, 0)
     }
+
+    @Test
+    fun testRecommendationRemovedWhileNotVisible_updateHostVisibility() {
+        var result = false
+        mediaCarouselController.updateHostVisibility = { result = true }
+
+        whenever(visualStabilityProvider.isReorderingAllowed).thenReturn(true)
+        listener.value.onSmartspaceMediaDataRemoved(SMARTSPACE_KEY, false)
+
+        assertEquals(true, result)
+    }
+
+    @Test
+    fun testRecommendationRemovedWhileVisible_thenReorders_updateHostVisibility() {
+        var result = false
+        mediaCarouselController.updateHostVisibility = { result = true }
+
+        whenever(visualStabilityProvider.isReorderingAllowed).thenReturn(false)
+        listener.value.onSmartspaceMediaDataRemoved(SMARTSPACE_KEY, false)
+        assertEquals(false, result)
+
+        visualStabilityCallback.value.onReorderingAllowed()
+        assertEquals(true, result)
+    }
 }