Remove controls from carousel when not in manager

If the user is in a bad state where controls exist in the carousel but
their keys have been removed from the manager, allow the dismiss button
to remove the player from the carousel directly.

The device isn't expected to get into this state normally but this will
allow users to remove the bad controls without needing to reboot.
Also added dump to MediaCarouselController to help debug future reports.

Bug: 190799184
Test: atest MediaDataManagerTest MediaControlPanelTest
Test: take bugreport, verify MediaCarouselController data is in dumpsys
Test: manual - force bad controls state, verify can still remove
Change-Id: I7af130c90c7b661cc7cc8bbeb3ca1fd5d81c8913
This commit is contained in:
Beth Thibodeau
2021-06-30 15:44:17 -04:00
parent ece342ba8c
commit b12fdd8073
5 changed files with 56 additions and 7 deletions

View File

@@ -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

View File

@@ -455,8 +455,12 @@ public class MediaControlPanel {
if (mKey != null) {
closeGuts();
mMediaDataManagerLazy.get().dismissMediaData(mKey,
MediaViewController.GUTS_ANIMATION_DURATION + 100);
if (!mMediaDataManagerLazy.get().dismissMediaData(mKey,
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());

View File

@@ -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
}
/**

View File

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

View File

@@ -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