diff options
2 files changed, 226 insertions, 11 deletions
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt index 2308f4cc96d1..c1816ed3e5bf 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt @@ -29,6 +29,7 @@ import com.android.systemui.coroutines.collectLastValue import com.android.systemui.kosmos.testScope import com.android.systemui.plugins.BcSmartspaceDataPlugin.SmartspaceTargetListener import com.android.systemui.testKosmos +import com.android.systemui.util.time.fakeSystemClock import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runCurrent @@ -54,12 +55,18 @@ class CommunalSmartspaceRepositoryImplTest : SysuiTestCase() { private val smartspaceController = mock<CommunalSmartspaceController>() private val fakeExecutor = kosmos.fakeExecutor + private val systemClock = kosmos.fakeSystemClock private lateinit var underTest: CommunalSmartspaceRepositoryImpl @Before fun setUp() { - underTest = CommunalSmartspaceRepositoryImpl(smartspaceController, fakeExecutor) + underTest = + CommunalSmartspaceRepositoryImpl( + smartspaceController, + fakeExecutor, + systemClock, + ) } @DisableFlags(FLAG_REMOTE_VIEWS) @@ -127,10 +134,184 @@ class CommunalSmartspaceRepositoryImplTest : SysuiTestCase() { runCurrent() // Verify that only the valid target is listed - assertThat(communalTimers?.size).isEqualTo(1) + assertThat(communalTimers).hasSize(1) assertThat(communalTimers?.first()?.smartspaceTargetId).isEqualTo("timer-1-started") } + @EnableFlags(FLAG_REMOTE_VIEWS) + @Test + fun communalTimers_cacheCreationTime() = + testScope.runTest { + underTest.startListening() + + val communalTimers by collectLastValue(underTest.timers) + runCurrent() + fakeExecutor.runAllReady() + + val listener = captureSmartspaceTargetListener() + listener.onSmartspaceTargetsUpdated( + listOf( + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-0-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(1000) + }, + ) + ) + runCurrent() + + // Verify that the creation time is the current time, not the creation time passed in + // the target, because this value can be inaccurate (due to b/318535930). + val currentTime = systemClock.currentTimeMillis() + assertThat(communalTimers?.get(0)?.createdTimestampMillis).isEqualTo(currentTime) + assertThat(communalTimers?.get(0)?.createdTimestampMillis).isNotEqualTo(1000) + + // A second timer is added. + listener.onSmartspaceTargetsUpdated( + listOf( + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-0-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(2000) + }, + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-1-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(3000) + }, + ) + ) + runCurrent() + + // Verify that the created timestamp for the first time is consistent + assertThat(communalTimers?.get(0)?.createdTimestampMillis).isEqualTo(currentTime) + + // Verify that the second timer has a new creation time + assertThat(communalTimers?.get(1)?.createdTimestampMillis) + .isEqualTo(systemClock.currentTimeMillis()) + } + + @EnableFlags(FLAG_REMOTE_VIEWS) + @Test + fun communalTimers_creationTimeRemovedFromCacheWhenTimerRemoved() = + testScope.runTest { + underTest.startListening() + + val communalTimers by collectLastValue(underTest.timers) + runCurrent() + fakeExecutor.runAllReady() + + // Start timer 0 + val listener = captureSmartspaceTargetListener() + listener.onSmartspaceTargetsUpdated( + listOf( + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-0-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(1000) + }, + ) + ) + runCurrent() + + // Verify timer 0 creation time + val expectedCreationTimeForTimer0 = systemClock.currentTimeMillis() + assertThat(communalTimers?.first()?.createdTimestampMillis) + .isEqualTo(expectedCreationTimeForTimer0) + + // Advance some time + systemClock.advanceTime(1000) + + // Start timer 1 + listener.onSmartspaceTargetsUpdated( + listOf( + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-0-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(1000) + }, + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-1-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(2000) + }, + ) + ) + runCurrent() + + // Verify timer 1 creation time is new + val expectedCreationTimeForTimer1 = expectedCreationTimeForTimer0 + 1000 + assertThat(communalTimers?.get(1)?.createdTimestampMillis) + .isEqualTo(expectedCreationTimeForTimer1) + + // Removed timer 0 + listener.onSmartspaceTargetsUpdated( + listOf( + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-1-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(2000) + }, + ) + ) + runCurrent() + + // Verify timer 0 removed, and timer 1 creation time is correct + assertThat(communalTimers).hasSize(1) + assertThat(communalTimers?.first()?.createdTimestampMillis) + .isEqualTo(expectedCreationTimeForTimer1) + + // Advance some time + systemClock.advanceTime(1000) + + // Start timer 0 again. Technically this is a new timer, but timers can reused stable + // ids. + listener.onSmartspaceTargetsUpdated( + listOf( + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-1-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(2000) + }, + mock<SmartspaceTarget> { + on { smartspaceTargetId }.doReturn("timer-0-started") + on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER) + on { remoteViews }.doReturn(mock()) + on { creationTimeMillis }.doReturn(3000) + }, + ) + ) + runCurrent() + + // Verify new timer added, and timer 1 creation time is still correct + assertThat(communalTimers).hasSize(2) + assertThat(communalTimers?.get(0)?.createdTimestampMillis) + .isEqualTo(expectedCreationTimeForTimer1) + + // Verify creation time for the new timer is new, meaning that cache for timer 0 was + // removed when it was removed + assertThat(communalTimers?.get(1)?.createdTimestampMillis) + .isEqualTo(expectedCreationTimeForTimer1 + 1000) + } + + @Test + fun stableId() { + assertThat(CommunalSmartspaceRepositoryImpl.stableId("timer-0-12345-started")) + .isEqualTo("timer-0") + assertThat(CommunalSmartspaceRepositoryImpl.stableId("timer-1-67890-paused")) + .isEqualTo("timer-1") + assertThat(CommunalSmartspaceRepositoryImpl.stableId("i_am_an_unexpected_id")) + .isEqualTo("i_am_an_unexpected_id") + } + private fun captureSmartspaceTargetListener(): SmartspaceTargetListener { verify(smartspaceController).addListener(listenerCaptor.capture()) return listenerCaptor.firstValue diff --git a/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt b/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt index 5d8af375fe99..e1d9bef67490 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt @@ -18,11 +18,13 @@ package com.android.systemui.communal.data.repository import android.app.smartspace.SmartspaceTarget import android.os.Parcelable +import androidx.annotation.VisibleForTesting import com.android.systemui.communal.data.model.CommunalSmartspaceTimer import com.android.systemui.communal.smartspace.CommunalSmartspaceController import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.plugins.BcSmartspaceDataPlugin +import com.android.systemui.util.time.SystemClock import java.util.concurrent.Executor import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -45,28 +47,44 @@ class CommunalSmartspaceRepositoryImpl constructor( private val communalSmartspaceController: CommunalSmartspaceController, @Main private val uiExecutor: Executor, + private val systemClock: SystemClock, ) : CommunalSmartspaceRepository, BcSmartspaceDataPlugin.SmartspaceTargetListener { private val _timers: MutableStateFlow<List<CommunalSmartspaceTimer>> = MutableStateFlow(emptyList()) override val timers: Flow<List<CommunalSmartspaceTimer>> = _timers + private var targetCreationTimes = emptyMap<String, Long>() + override fun onSmartspaceTargetsUpdated(targetsNullable: MutableList<out Parcelable>?) { val targets = targetsNullable?.filterIsInstance<SmartspaceTarget>() ?: emptyList() - - _timers.value = + val timerTargets = targets .filter { target -> target.featureType == SmartspaceTarget.FEATURE_TIMER && target.remoteViews != null } - .map { target -> - CommunalSmartspaceTimer( - smartspaceTargetId = target.smartspaceTargetId, - createdTimestampMillis = target.creationTimeMillis, - remoteViews = target.remoteViews!!, - ) - } + .associateBy { stableId(it.smartspaceTargetId) } + + // The creation times from smartspace targets are unreliable (b/318535930). Therefore, + // SystemUI uses the timestamp of which a timer first appears, and caches these values to + // prevent timers from swapping positions in the hub. + targetCreationTimes = + timerTargets.mapValues { (stableId, _) -> + targetCreationTimes[stableId] ?: systemClock.currentTimeMillis() + } + + _timers.value = + timerTargets.map { (stableId, target) -> + CommunalSmartspaceTimer( + // The view layer should have the instance based smartspaceTargetId instead of + // stable id, so that when a new instance of the timer is created, for example, + // when it is paused, the view should re-render its remote views. + smartspaceTargetId = target.smartspaceTargetId, + createdTimestampMillis = targetCreationTimes[stableId]!!, + remoteViews = target.remoteViews!!, + ) + } } override fun startListening() { @@ -86,4 +104,20 @@ constructor( ) } } + + companion object { + /** + * The smartspace target id is instance-based, meaning a single timer (from the user's + * perspective) can have multiple instances. For example, when a timer is paused, a new + * instance is created. To address this, SystemUI manually removes the instance id to + * maintain a consistent id across sessions. + * + * It is assumed that timer target ids follow this format: timer-${stableId}-${instanceId}. + * This function returns timer-${stableId}, stripping out the instance id. + */ + @VisibleForTesting + fun stableId(targetId: String): String { + return targetId.split("-").take(2).joinToString("-") + } + } } |