Merge "Remove controls from carousel when not in manager" into sc-dev am: 1c3d54b979

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

Change-Id: I616cb6a16f12d75327b1c9a7e86a6ce796f68b99

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

@@ -12,10 +12,12 @@ import android.view.View
 import android.view.ViewGroup
 import android.widget.LinearLayout
 import androidx.annotation.VisibleForTesting
+import com.android.systemui.Dumpable
 import com.android.systemui.R
 import com.android.systemui.classifier.FalsingCollector
 import com.android.systemui.dagger.SysUISingleton
 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.qs.PageIndicator
@@ -27,6 +29,8 @@ import com.android.systemui.util.animation.UniqueObjectHostView
 import com.android.systemui.util.animation.requiresRemeasuring
 import com.android.systemui.util.concurrency.DelayableExecutor
 import com.android.systemui.util.time.SystemClock
+import java.io.FileDescriptor
+import java.io.PrintWriter
 import java.util.TreeMap
 import javax.inject.Inject
 import javax.inject.Provider
@@ -51,8 +55,9 @@ class MediaCarouselController @Inject constructor(
     private val mediaManager: MediaDataManager,
     configurationController: ConfigurationController,
     falsingCollector: FalsingCollector,
-    falsingManager: FalsingManager
+    falsingManager: FalsingManager,
-) {
+    dumpManager: DumpManager
+) : Dumpable {
     /**
      * The current width of the carousel
      */
@@ -166,6 +171,7 @@ class MediaCarouselController @Inject constructor(
     lateinit var updateUserVisibility: () -> Unit
 
     init {
+        dumpManager.registerDumpable(TAG, this)
         mediaFrame = inflateMediaCarousel()
         mediaCarousel = mediaFrame.requireViewById(R.id.media_carousel_scroller)
         pageIndicator = mediaFrame.requireViewById(R.id.media_page_indicator)
@@ -421,7 +427,7 @@ class MediaCarouselController @Inject constructor(
         }
     }
 
-    private fun removePlayer(
+    fun removePlayer(
         key: String,
         dismissMediaData: Boolean = true,
         dismissRecommendation: Boolean = true
@@ -748,6 +754,15 @@ class MediaCarouselController @Inject constructor(
         }
         mediaManager.onSwipeToDismiss()
     }
+
+    override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array<out String>) {
+        pw.apply {
+            println("keysNeedRemoval: $keysNeedRemoval")
+            println("playerKeys: ${MediaPlayerData.playerKeys()}")
+            println("smartspaceMediaData: ${MediaPlayerData.smartspaceMediaData}")
+            println("shouldPrioritizeSs: ${MediaPlayerData.shouldPrioritizeSs}")
+        }
+    }
 }
 
 @VisibleForTesting

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

@@ -455,8 +455,12 @@ public class MediaControlPanel {
 
             if (mKey != null) {
                 closeGuts();
-                mMediaDataManagerLazy.get().dismissMediaData(mKey,
+                if (!mMediaDataManagerLazy.get().dismissMediaData(mKey,
-                        MediaViewController.GUTS_ANIMATION_DURATION + 100);
+                        MediaViewController.GUTS_ANIMATION_DURATION + 100)) {
+                    Log.w(TAG, "Manager failed to dismiss media " + mKey);
+                    // Remove directly from carousel to let user recover - TODO(b/190799184)
+                    mMediaCarouselController.removePlayer(key, false, false);
+                }
             } else {
                 Log.w(TAG, "Dismiss media with null notification. Token uid="
                         + data.getToken().getUid());

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

@@ -430,7 +430,11 @@ class MediaDataManager(
         notifyMediaDataRemoved(key)
     }
 
-    fun dismissMediaData(key: String, delay: Long) {
+    /**
+     * Dismiss a media entry. Returns false if the key was not found.
+     */
+    fun dismissMediaData(key: String, delay: Long): Boolean {
+        val existed = mediaEntries[key] != null
         backgroundExecutor.execute {
             mediaEntries[key]?.let { mediaData ->
                 if (mediaData.isLocalSession) {
@@ -442,6 +446,7 @@ class MediaDataManager(
             }
         }
         foregroundExecutor.executeDelayed({ removeEntry(key) }, delay)
+        return existed
     }
 
     /**

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

@@ -345,4 +345,22 @@ public class MediaControlPanelTest : SysuiTestCase() {
 
         assertThat(dismiss.isEnabled).isEqualTo(false)
     }
+
+    @Test
+    fun dismissButtonClick_notInManager() {
+        val mediaKey = "key for dismissal"
+        whenever(mediaDataManager.dismissMediaData(eq(mediaKey), anyLong())).thenReturn(false)
+
+        player.attachPlayer(holder)
+        val state = MediaData(USER_ID, true, BG_COLOR, APP, null, ARTIST, TITLE, null, emptyList(),
+                emptyList(), PACKAGE, session.getSessionToken(), null, null, true, null,
+                notificationKey = KEY)
+        player.bindPlayer(state, mediaKey)
+
+        assertThat(dismiss.isEnabled).isEqualTo(true)
+        dismiss.callOnClick()
+
+        verify(mediaDataManager).dismissMediaData(eq(mediaKey), anyLong())
+        verify(mediaCarouselController).removePlayer(eq(mediaKey), eq(false), eq(false))
+    }
 }

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

@@ -317,7 +317,8 @@ class MediaDataManagerTest : SysuiTestCase() {
     fun testDismissMedia_listenerCalled() {
         mediaDataManager.onNotificationAdded(KEY, mediaNotification)
         mediaDataManager.onMediaDataLoaded(KEY, oldKey = null, data = mock(MediaData::class.java))
-        mediaDataManager.dismissMediaData(KEY, 0L)
+        val removed = mediaDataManager.dismissMediaData(KEY, 0L)
+        assertThat(removed).isTrue()
 
         foregroundExecutor.advanceClockToLast()
         foregroundExecutor.runAllReady()
@@ -325,6 +326,12 @@ class MediaDataManagerTest : SysuiTestCase() {
         verify(listener).onMediaDataRemoved(eq(KEY))
     }
 
+    @Test
+    fun testDismissMedia_keyDoesNotExist_returnsFalse() {
+        val removed = mediaDataManager.dismissMediaData(KEY, 0L)
+        assertThat(removed).isFalse()
+    }
+
     @Test
     fun testBadArtwork_doesNotUse() {
         // WHEN notification has a too-small artwork