From b64d6c6040528012cee5ef09ece5cdcbb78564f9 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Mon, 5 Aug 2024 20:43:30 +0000 Subject: [SB][Screen chips] Don't change recording start time once it's started. If the user is screen recording a single app, `ScreenRecordChipInteractor` emits two models very close together (~7ms apart): 1) Recording(taskPackage=null) 2) Recording(taskPackage="some.package") This is because the recording state comes from ScreenRecordRepository, but the taskPackage comes from MediaProjectionRepository, so the taskPackage comes in slightly later. Previously, ScreenRecordChipViewModel would change the start time when the second `Recording` event came in. Changing the start time on the Chronometer makes it skip a second, due to some of the internal workings of the Chronometer class. This CL updates the ScreenRecordChipViewModel to not change the start time once the recording has started. Fixes: 349620526 Bug: 332662551 Flag: com.android.systemui.status_bar_screen_sharing_chips Test: Screen record a single app -> verify chip timer doesn't skip from 00: 00 to 00:02 Test: atest ScreenRecordChipViewModelTest Change-Id: Ia6fb78ee050bad835403b781862b53028eacbb51 --- .../ui/viewmodel/ScreenRecordChipViewModel.kt | 27 +++++++++++++++++++-- .../ui/viewmodel/ScreenRecordChipViewModelTest.kt | 28 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModel.kt index 6ba4fefd6f3c..9e6cacb8b9ff 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModel.kt @@ -41,6 +41,7 @@ import com.android.systemui.statusbar.chips.ui.model.OngoingActivityChipModel import com.android.systemui.statusbar.chips.ui.viewmodel.ChipTransitionHelper import com.android.systemui.statusbar.chips.ui.viewmodel.OngoingActivityChipViewModel import com.android.systemui.statusbar.chips.ui.viewmodel.OngoingActivityChipViewModel.Companion.createDialogLaunchOnClickListener +import com.android.systemui.util.kotlin.pairwise import com.android.systemui.util.time.SystemClock import javax.inject.Inject import kotlinx.coroutines.CoroutineScope @@ -64,7 +65,8 @@ constructor( @StatusBarChipsLog private val logger: LogBuffer, ) : OngoingActivityChipViewModel { - private val internalChip = + /** A direct mapping from [ScreenRecordChipModel] to [OngoingActivityChipModel]. */ + private val simpleChip = interactor.screenRecordState .map { state -> when (state) { @@ -105,10 +107,31 @@ constructor( // See b/347726238 for [SharingStarted.Lazily] reasoning. .stateIn(scope, SharingStarted.Lazily, OngoingActivityChipModel.Hidden()) + /** + * The screen record chip to show that also ensures that the start time doesn't change once we + * enter the recording state. If we change the start time while we're recording, the chronometer + * could skip a second. See b/349620526. + */ + private val chipWithConsistentTimer: StateFlow = + simpleChip + .pairwise(initialValue = OngoingActivityChipModel.Hidden()) + .map { (old, new) -> + if ( + old is OngoingActivityChipModel.Shown.Timer && + new is OngoingActivityChipModel.Shown.Timer + ) { + new.copy(startTimeMs = old.startTimeMs) + } else { + new + } + } + // See b/347726238 for [SharingStarted.Lazily] reasoning. + .stateIn(scope, SharingStarted.Lazily, OngoingActivityChipModel.Hidden()) + private val chipTransitionHelper = ChipTransitionHelper(scope) override val chip: StateFlow = - chipTransitionHelper.createChipFlow(internalChip) + chipTransitionHelper.createChipFlow(chipWithConsistentTimer) private fun createDelegate( recordedTask: ActivityManager.RunningTaskInfo? diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModelTest.kt index e68fa0bc6eb3..804eb5cf597c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModelTest.kt @@ -231,6 +231,34 @@ class ScreenRecordChipViewModelTest : SysuiTestCase() { assertThat((latest as OngoingActivityChipModel.Shown.Timer).startTimeMs).isEqualTo(5678) } + /** Regression test for b/349620526. */ + @Test + fun chip_recordingState_thenGetsTaskInfo_startTimeDoesNotChange() = + testScope.runTest { + val latest by collectLastValue(underTest.chip) + + // Start recording, but without any task info + systemClock.setElapsedRealtime(1234) + screenRecordRepo.screenRecordState.value = ScreenRecordModel.Recording + mediaProjectionRepo.mediaProjectionState.value = MediaProjectionState.NotProjecting + + assertThat(latest).isInstanceOf(OngoingActivityChipModel.Shown::class.java) + assertThat((latest as OngoingActivityChipModel.Shown.Timer).startTimeMs).isEqualTo(1234) + + // WHEN we receive the recording task info a few milliseconds later + systemClock.setElapsedRealtime(1240) + mediaProjectionRepo.mediaProjectionState.value = + MediaProjectionState.Projecting.SingleTask( + "host.package", + hostDeviceName = null, + FakeActivityTaskManager.createTask(taskId = 1) + ) + + // THEN the start time is still the old start time + assertThat(latest).isInstanceOf(OngoingActivityChipModel.Shown::class.java) + assertThat((latest as OngoingActivityChipModel.Shown.Timer).startTimeMs).isEqualTo(1234) + } + @Test fun chip_notProjecting_clickListenerShowsDialog() = testScope.runTest { -- cgit v1.2.3-59-g8ed1b