From 9cbca1a6adaa58907e229bfdff0d7c28c82a0ca2 Mon Sep 17 00:00:00 2001 From: cecilia Date: Wed, 12 May 2021 22:02:02 -0400 Subject: [PATCH] Always show media recommendation on headphones connection. - If all the resumable media age > MAX_AGE, show the media recommendations as the first card. All the existing resumable media remains as is; - Otherwise, reactivate all the resumable media with age <= MAX_AGE to be shown, followed by the media recommendations as the last card. Also, shorten the maximum media age from 3 hours to 30 minutes. Fixes: 187999902 Fixes: 187943295 Test: Local builds Test: MediaDataFilterTest Change-Id: I40bdbb2a172de1c110edfd9a8e3a1f6521438ba3 --- .../systemui/media/MediaCarouselController.kt | 27 +++++--- .../systemui/media/MediaDataCombineLatest.kt | 6 +- .../android/systemui/media/MediaDataFilter.kt | 68 +++++++++++++------ .../systemui/media/MediaDataManager.kt | 13 +++- .../com/android/systemui/media/MediaHost.kt | 6 +- .../systemui/media/MediaSessionBasedFilter.kt | 6 +- .../statusbar/NotificationMediaManager.java | 6 +- .../systemui/media/MediaDataFilterTest.kt | 53 +++++++++++---- .../systemui/media/MediaDataManagerTest.kt | 7 +- 9 files changed, 140 insertions(+), 52 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt index 25e6e94e8e7d4..a1c06fcec1cd8 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt @@ -208,9 +208,13 @@ class MediaCarouselController @Inject constructor( } } - override fun onSmartspaceMediaDataLoaded(key: String, data: SmartspaceTarget) { + override fun onSmartspaceMediaDataLoaded( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean + ) { Log.d(TAG, "My Smartspace media update is here") - addSmartspaceMediaRecommendations(key, data) + addSmartspaceMediaRecommendations(key, data, shouldPrioritize) MediaPlayerData.getMediaPlayer(key, null)?.let { logSmartspaceCardReported(759, // SMARTSPACE_CARD_RECEIVED it.mInstanceId, @@ -327,7 +331,11 @@ class MediaCarouselController @Inject constructor( return existingPlayer == null } - private fun addSmartspaceMediaRecommendations(key: String, data: SmartspaceTarget) { + private fun addSmartspaceMediaRecommendations( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean + ) { Log.d(TAG, "Updating smartspace target in carousel") if (MediaPlayerData.getMediaPlayer(key, null) != null) { Log.w(TAG, "Skip adding smartspace target in carousel") @@ -342,7 +350,7 @@ class MediaCarouselController @Inject constructor( ViewGroup.LayoutParams.WRAP_CONTENT) newRecs.recommendationViewHolder?.recommendations?.setLayoutParams(lp) newRecs.bindRecommendation(data, bgColor) - MediaPlayerData.addMediaRecommendation(key, newRecs) + MediaPlayerData.addMediaRecommendation(key, newRecs, shouldPrioritize) updatePlayerToState(newRecs, noAnimation = true) reorderAllPlayers() updatePageIndicator() @@ -671,17 +679,19 @@ class MediaCarouselController @Inject constructor( internal object MediaPlayerData { private val EMPTY = MediaData(-1, false, 0, null, null, null, null, null, emptyList(), emptyList(), "INVALID", null, null, null, true, null) + // Whether should prioritize Smartspace card. + private var shouldPrioritizeSs: Boolean = false data class MediaSortKey( - // Is Smartspace media recommendation. When the Smartspace media is present, it should - // always be the first card in carousel. + // Whether the item represents a Smartspace media recommendation. val isSsMediaRec: Boolean, val data: MediaData, val updateTime: Long = 0 ) private val comparator = - compareByDescending { it.isSsMediaRec } + compareByDescending + { if (shouldPrioritizeSs) it.isSsMediaRec else !it.isSsMediaRec } .thenByDescending { it.data.isPlaying } .thenByDescending { it.data.isLocalSession } .thenByDescending { !it.data.resumption } @@ -697,7 +707,8 @@ internal object MediaPlayerData { mediaPlayers.put(sortKey, player) } - fun addMediaRecommendation(key: String, player: MediaControlPanel) { + fun addMediaRecommendation(key: String, player: MediaControlPanel, shouldPrioritize: Boolean) { + shouldPrioritizeSs = shouldPrioritize removeMediaPlayer(key) val sortKey = MediaSortKey(isSsMediaRec = true, EMPTY, System.currentTimeMillis()) mediaData.put(key, sortKey) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataCombineLatest.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataCombineLatest.kt index 2c094b885032b..87af9e08bd718 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataCombineLatest.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataCombineLatest.kt @@ -38,7 +38,11 @@ class MediaDataCombineLatest @Inject constructor() : MediaDataManager.Listener, } } - override fun onSmartspaceMediaDataLoaded(key: String, data: SmartspaceTarget) { + override fun onSmartspaceMediaDataLoaded( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean + ) { listeners.toSet().forEach { it.onSmartspaceMediaDataLoaded(key, data) } } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt index e5c15107be371..3deb5d13a27f8 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataFilter.kt @@ -26,9 +26,11 @@ import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.settings.CurrentUserTracker import com.android.systemui.statusbar.NotificationLockscreenUserManager import com.android.systemui.util.time.SystemClock +import java.util.SortedMap import java.util.concurrent.Executor import java.util.concurrent.TimeUnit import javax.inject.Inject +import kotlin.collections.LinkedHashMap private const val TAG = "MediaDataFilter" private const val DEBUG = true @@ -39,7 +41,7 @@ private const val DEBUG = true */ @VisibleForTesting internal val SMARTSPACE_MAX_AGE = SystemProperties - .getLong("debug.sysui.smartspace_max_age", TimeUnit.HOURS.toMillis(3)) + .getLong("debug.sysui.smartspace_max_age", TimeUnit.MINUTES.toMillis(30)) /** * Filters data updates from [MediaDataCombineLatest] based on the current user ID, and handles user @@ -65,7 +67,8 @@ class MediaDataFilter @Inject constructor( private val allEntries: LinkedHashMap = LinkedHashMap() // The filtered userEntries, which will be a subset of all userEntries in MediaDataManager private val userEntries: LinkedHashMap = LinkedHashMap() - private var hasSmartspace: Boolean = false + var hasSmartspace: Boolean = false + private set private var reactivatedKey: String? = null init { @@ -99,39 +102,39 @@ class MediaDataFilter @Inject constructor( } } - override fun onSmartspaceMediaDataLoaded(key: String, data: SmartspaceTarget) { + override fun onSmartspaceMediaDataLoaded( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean + ) { + var shouldPrioritizeMutable = shouldPrioritize hasSmartspace = true // Before forwarding the smartspace target, first check if we have recently inactive media - val now = systemClock.elapsedRealtime() val sorted = userEntries.toSortedMap(compareBy { userEntries.get(it)?.lastActive ?: -1 }) - if (sorted.size > 0) { + val timeSinceActive = timeSinceActiveForMostRecentMedia(sorted) + if (timeSinceActive < SMARTSPACE_MAX_AGE) { val lastActiveKey = sorted.lastKey() // most recently active - val timeSinceActive = sorted.get(lastActiveKey)?.let { - now - it.lastActive - } ?: Long.MAX_VALUE - if (timeSinceActive < SMARTSPACE_MAX_AGE) { - // Notify listeners to consider this media active - Log.d(TAG, "reactivating $lastActiveKey instead of smartspace") - reactivatedKey = lastActiveKey - val mediaData = sorted.get(lastActiveKey)!!.copy(active = true) - listeners.forEach { - it.onMediaDataLoaded(lastActiveKey, lastActiveKey, mediaData) - } - return + // Notify listeners to consider this media active + Log.d(TAG, "reactivating $lastActiveKey instead of smartspace") + reactivatedKey = lastActiveKey + val mediaData = sorted.get(lastActiveKey)!!.copy(active = true) + listeners.forEach { + it.onMediaDataLoaded(lastActiveKey, lastActiveKey, mediaData) } + } else { + // Mark to prioritize Smartspace card if no recent media. + shouldPrioritizeMutable = true } - // If no recent media, continue with smartspace update + // Only proceed with the Smartspace update if the recommendation is not empty. if (isMediaRecommendationEmpty(data)) { Log.d(TAG, "Empty media recommendations. Skip showing the card") return } - - // Proceed only if the Smartspace recommendation is not empty. - listeners.forEach { it.onSmartspaceMediaDataLoaded(key, data) } + listeners.forEach { it.onSmartspaceMediaDataLoaded(key, data, shouldPrioritizeMutable) } } override fun onMediaDataRemoved(key: String) { @@ -158,7 +161,6 @@ class MediaDataFilter @Inject constructor( it.onMediaDataLoaded(lastActiveKey, lastActiveKey, mediaData) } } - return } listeners.forEach { it.onSmartspaceMediaDataRemoved(key) } @@ -230,4 +232,26 @@ class MediaDataFilter @Inject constructor( val mediaRecommendationList: List = data.getIconGrid() return mediaRecommendationList == null || mediaRecommendationList.isEmpty() } + + /** + * Return the time since last active for the most-recent media. + * + * @param sortedEntries userEntries sorted from the earliest to the most-recent. + * + * @return The duration in milliseconds from the most-recent media's last active timestamp to + * the present. MAX_VALUE will be returned if there is no media. + */ + private fun timeSinceActiveForMostRecentMedia( + sortedEntries: SortedMap + ): Long { + if (sortedEntries.isEmpty()) { + return Long.MAX_VALUE + } + + val now = systemClock.elapsedRealtime() + val lastActiveKey = sortedEntries.lastKey() // most recently active + return sortedEntries.get(lastActiveKey)?.let { + now - it.lastActive + } ?: Long.MAX_VALUE + } } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt index a45bd33c346a3..ffcec29061677 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt @@ -817,8 +817,17 @@ class MediaDataManager( */ fun onMediaDataLoaded(key: String, oldKey: String?, data: MediaData) {} - /** Called whenever there's new Smartspace media data loaded. */ - fun onSmartspaceMediaDataLoaded(key: String, data: SmartspaceTarget) {} + /** + * Called whenever there's new Smartspace media data loaded. + * + * shouldPrioritize indicates the sorting priority of the Smartspace card. If true, it will + * be prioritized as the first card. Otherwise, it will show up as the last card as default. + */ + fun onSmartspaceMediaDataLoaded( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean = false + ) {} /** * Called whenever a previously existing Media notification was removed diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt index 28e46409e5c68..7402107672937 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt @@ -56,7 +56,11 @@ class MediaHost constructor( updateViewVisibility() } - override fun onSmartspaceMediaDataLoaded(key: String, data: SmartspaceTarget) { + override fun onSmartspaceMediaDataLoaded( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean + ) { updateViewVisibility() } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaSessionBasedFilter.kt b/packages/SystemUI/src/com/android/systemui/media/MediaSessionBasedFilter.kt index d973478625de1..3e5e82484df6d 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaSessionBasedFilter.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaSessionBasedFilter.kt @@ -135,7 +135,11 @@ class MediaSessionBasedFilter @Inject constructor( } } - override fun onSmartspaceMediaDataLoaded(key: String, data: SmartspaceTarget) { + override fun onSmartspaceMediaDataLoaded( + key: String, + data: SmartspaceTarget, + shouldPrioritize: Boolean + ) { backgroundExecutor.execute { dispatchSmartspaceMediaDataLoaded(key, data) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java index 27264880a4623..f99436f677284 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java @@ -250,7 +250,8 @@ public class NotificationMediaManager implements Dumpable { @Override public void onSmartspaceMediaDataLoaded(@NonNull String key, - @NonNull SmartspaceTarget data) { + @NonNull SmartspaceTarget data, boolean shouldPrioritize) { + } @Override @@ -323,7 +324,8 @@ public class NotificationMediaManager implements Dumpable { @Override public void onSmartspaceMediaDataLoaded(@NonNull String key, - @NonNull SmartspaceTarget data) { + @NonNull SmartspaceTarget data, boolean shouldPrioritize) { + } @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt index bfd60b969287a..fc0506abbdac7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt @@ -30,6 +30,7 @@ import com.google.common.truth.Truth.assertThat import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mock import org.mockito.Mockito @@ -228,37 +229,39 @@ class MediaDataFilterTest : SysuiTestCase() { } @Test - fun testOnSmartspaceMediaDataLoaded_noMedia_nonEmptyRecommendation_usesSmartspace() { + fun testOnSmartspaceMediaDataLoaded_noMedia_nonEmptyRec_prioritizesSmartspace() { mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) - verify(listener).onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData)) + verify(listener) + .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData), eq(true)) assertThat(mediaDataFilter.hasActiveMedia()).isTrue() } @Test - fun testOnSmartspaceMediaDataLoaded_noMedia_emptyRecommendation_showsNothing() { + fun testOnSmartspaceMediaDataLoaded_noMedia_emptyRec_showsNothing() { `when`(smartspaceData.iconGrid).thenReturn(listOf()) mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) verify(listener, never()) - .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData)) + .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData), anyBoolean()) assertThat(mediaDataFilter.hasActiveMedia()).isTrue() } @Test - fun testOnSmartspaceMediaDataLoaded_noRecentMedia_nonEmptyRecommendation_usesSmartspace() { + fun testOnSmartspaceMediaDataLoaded_noRecentMedia_nonEmptyRec_prioritizesSmartspace() { val dataOld = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataOld) clock.advanceTime(SMARTSPACE_MAX_AGE + 100) mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) - verify(listener).onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData)) + verify(listener) + .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData), eq(true)) assertThat(mediaDataFilter.hasActiveMedia()).isTrue() } @Test - fun testOnSmartspaceMediaDataLoaded_noRecentMedia_emptyRecommendation_showsNothing() { + fun testOnSmartspaceMediaDataLoaded_noRecentMedia_emptyRec_showsNothing() { `when`(smartspaceData.iconGrid).thenReturn(listOf()) val dataOld = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) @@ -267,12 +270,14 @@ class MediaDataFilterTest : SysuiTestCase() { mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) verify(listener, never()) - .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData)) + .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData), anyBoolean()) assertThat(mediaDataFilter.hasActiveMedia()).isTrue() } @Test - fun testOnSmartspaceMediaDataLoaded_hasRecentMedia_usesMedia() { + fun testOnSmartspaceMediaDataLoaded_hasRecentMedia_emptyRec_usesMedia() { + `when`(smartspaceData.iconGrid).thenReturn(listOf()) + // WHEN we have media that was recently played, but not currently active val dataCurrent = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataCurrent) @@ -285,19 +290,41 @@ class MediaDataFilterTest : SysuiTestCase() { val dataCurrentAndActive = dataCurrent.copy(active = true) verify(listener).onMediaDataLoaded(eq(KEY), eq(KEY), eq(dataCurrentAndActive)) assertThat(mediaDataFilter.hasActiveMedia()).isTrue() + // Smartspace update shouldn't be propagated for the empty rec list. + verify(listener, never()) + .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData), anyBoolean()) } @Test - fun testOnSmartspaceMediaDataRemoved_usedSmartspace_clearsMedia() { + fun testOnSmartspaceMediaDataLoaded_hasRecentMedia_nonEmptyRec_usesBoth() { + // WHEN we have media that was recently played, but not currently active + val dataCurrent = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) + mediaDataFilter.onMediaDataLoaded(KEY, null, dataCurrent) + verify(listener).onMediaDataLoaded(eq(KEY), eq(null), eq(dataCurrent)) + + // AND we get a smartspace signal + mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) + + // THEN we should tell listeners to treat the media as active instead + val dataCurrentAndActive = dataCurrent.copy(active = true) + verify(listener).onMediaDataLoaded(eq(KEY), eq(KEY), eq(dataCurrentAndActive)) + assertThat(mediaDataFilter.hasActiveMedia()).isTrue() + // Smartspace update should also be propagated but not prioritized. + verify(listener) + .onSmartspaceMediaDataLoaded(eq(SMARTSPACE_KEY), eq(smartspaceData), eq(false)) + } + + @Test + fun testOnSmartspaceMediaDataRemoved_usedSmartspace_clearsSmartspace() { mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) mediaDataFilter.onSmartspaceMediaDataRemoved(SMARTSPACE_KEY) verify(listener).onSmartspaceMediaDataRemoved(SMARTSPACE_KEY) - assertThat(mediaDataFilter.hasActiveMedia()).isFalse() + assertThat(mediaDataFilter.hasSmartspace).isFalse() } @Test - fun testOnSmartspaceMediaDataRemoved_usedMedia_clearsMedia() { + fun testOnSmartspaceMediaDataRemoved_usedMediaAndSmartspace_clearsBoth() { val dataCurrent = dataMain.copy(active = false, lastActive = clock.elapsedRealtime()) mediaDataFilter.onMediaDataLoaded(KEY, null, dataCurrent) mediaDataFilter.onSmartspaceMediaDataLoaded(SMARTSPACE_KEY, smartspaceData) @@ -305,6 +332,8 @@ class MediaDataFilterTest : SysuiTestCase() { mediaDataFilter.onSmartspaceMediaDataRemoved(SMARTSPACE_KEY) verify(listener).onMediaDataLoaded(eq(KEY), eq(KEY), eq(dataCurrent)) + verify(listener).onSmartspaceMediaDataRemoved(SMARTSPACE_KEY) assertThat(mediaDataFilter.hasActiveMedia()).isFalse() + assertThat(mediaDataFilter.hasSmartspace).isFalse() } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt index acfc513eb27a0..59527f6b97404 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt @@ -28,6 +28,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito @@ -41,7 +42,6 @@ import org.mockito.Mockito.`when` as whenever private const val KEY = "KEY" private const val KEY_2 = "KEY_2" private const val KEY_MEDIA_SMARTSPACE = "MEDIA_SMARTSPACE_ID" -private const val KEY_NONE_MEDIA_SMARTSPACE = "NONE_MEDIA_SMARTSPACE_ID" private const val PACKAGE_NAME = "com.android.systemui" private const val APP_NAME = "SystemUI" private const val SESSION_ARTIST = "artist" @@ -364,13 +364,14 @@ class MediaDataManagerTest : SysuiTestCase() { fun testOnSmartspaceMediaDataLoaded_hasNewMediaTarget_callsListener() { smartspaceMediaDataProvider.onTargetsAvailable(listOf(mediaSmartspaceTarget)) verify(listener).onSmartspaceMediaDataLoaded( - eq(KEY_MEDIA_SMARTSPACE), eq(mediaSmartspaceTarget)) + eq(KEY_MEDIA_SMARTSPACE), eq(mediaSmartspaceTarget), eq(false)) } @Test fun testOnSmartspaceMediaDataLoaded_hasNoneMediaTarget_notCallsListener() { smartspaceMediaDataProvider.onTargetsAvailable(listOf()) - verify(listener, never()).onSmartspaceMediaDataLoaded(anyObject(), anyObject()) + verify(listener, never()) + .onSmartspaceMediaDataLoaded(anyObject(), anyObject(), anyBoolean()) } @Test