Track order of visible media players

Create a new map that track the order of visible media players before
they are reordered as the media carousel is visible to user. Media
player map that was being used before is not consistent with UI during
the time the carousel is visible to user. So during this time we need to
keep track of the order.
This also makes sure the media carousel scroll to the right app media player when user selects one of the recommendations.

Bug: 248340474
Test: atest MediaCarouselControllerTest
Test: Checked order of UMO cards in carousel.
Change-Id: I549535125b251b03740a387cc4a1cfae91f4f4f5
This commit is contained in:
Michael Mikhail
2022-09-16 13:45:50 +00:00
parent cf49d6f46f
commit 49f68af1be
3 changed files with 140 additions and 41 deletions

View File

@@ -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) {
shouldScrollToActivePlayer = false
val activeMediaIndex = MediaPlayerData.firstActiveMediaIndex()
if (activeMediaIndex != -1) {
if (shouldScrollToKey) {
shouldScrollToKey = false
val mediaIndex = key?.let { MediaPlayerData.getMediaPlayerIndex(it) } ?: -1
if (mediaIndex != -1) {
previousVisiblePlayerKey?.let {
val previousVisibleIndex = MediaPlayerData.playerKeys()
.indexOfFirst { key -> it == key }
mediaCarouselScrollHandler
.scrollToPlayer(previousVisibleIndex, activeMediaIndex)
} ?: mediaCarouselScrollHandler.scrollToPlayer(destIndex = activeMediaIndex)
.scrollToPlayer(previousVisibleIndex, mediaIndex)
} ?: 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) {
reorderAllPlayers(curVisibleMediaKey)
// Media data added from a recommendation card should starts playing.
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
// To make sure we scroll to the active playing media card.
val packageName = MediaPlayerData.smartspaceMediaData?.packageName ?: String()
// 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
}
logSmartspaceCardReported(800, // SMARTSPACE_CARD_SEEN
mediaControlPanel.mSmartspaceId,
mediaControlPanel.mUid,
intArrayOf(mediaControlPanel.surfaceForSmartspaceLogging))
mediaControlPanel.mIsImpressed = true
mediaControlPanel?.let {
logSmartspaceCardReported(800, // SMARTSPACE_CARD_SEEN
it.mSmartspaceId,
it.mUid,
intArrayOf(it.surfaceForSmartspaceLogging))
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,
EMPTY.copy(isPlaying = false), clock.currentTimeMillis(), isSsReactivated = true)
val sortKey = MediaSortKey(
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)
}
}
}

View File

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

View File

@@ -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(mediaPlayer.mediaViewController).thenReturn(mediaViewController)
whenever(mediaControlPanelFactory.get()).thenReturn(panel)
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,
false, clock)
listener.value.onSmartspaceMediaDataLoaded(
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)