Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 584ee44d authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[SB][Screen Chips] Don't let the timers reset on theme change. 

b/347726238 was happening because: When the theme changes,
CollapsedStatusBarFragment reattaches its views, which means that
CollapsedStatusBarViewBinder stopped listening to flows and then started
listening to flows again. Because all of the chip-related flows were
`WhileSubscribed`, the flows stopped and then started again. When they
re-started, the ongoing event was re-sent from the repositories and we
re-calculated what the start time should be (because we just use the
current time as the start time).

This CL updates the main chips flow to use `SharingStarted.Lazily`,
which will start the flow once a subscriber appears but then never
_stops_ the flow. That means all the upstream flows also won't stop, so
they won't re-calculate the start time.

This CL also updates each individual chip flow to use `Lazily`. That
isn't strictly necessary (the downstream `chip` flow using `Lazily`
guarantees that the upstream ones are), but it seemed good to protect
the individual flows, and also put the bug reference closer to where the
timer start times are actually created.

Fixes: 347726238
Bug: 332662551
Flag: com.android.systemui.status_bar_screen_sharing_chips

Test: Start a screen recording -> change theme -> verify time in screen
recording chip doesn't reset
Test: atest OngoingActivityChipsViewModelTest

Change-Id: I8ae07863c0f52f3e4c0175e9eb6fbdede4d08d74
parent 247f35ac

packages/SystemUI/src/com/android/systemui/statusbar/chips/casttootherdevice/ui/viewmodel/CastToOtherDeviceChipViewModel.kt

+2 −1
Original line number Diff line number Diff line
@@ -70,7 +70,8 @@ constructor( 
                    }

                }

            }

            .stateIn(scope, SharingStarted.WhileSubscribed(), OngoingActivityChipModel.Hidden)

            // See b/347726238.

            .stateIn(scope, SharingStarted.Lazily, OngoingActivityChipModel.Hidden)



    /** Stops the currently active projection. */

    private fun stopProjecting() {

packages/SystemUI/src/com/android/systemui/statusbar/chips/screenrecord/ui/viewmodel/ScreenRecordChipViewModel.kt

+2 −1
Original line number Diff line number Diff line
@@ -76,7 +76,8 @@ constructor( 
                    }

                }

            }

            .stateIn(scope, SharingStarted.WhileSubscribed(), OngoingActivityChipModel.Hidden)

            // See b/347726238.

            .stateIn(scope, SharingStarted.Lazily, OngoingActivityChipModel.Hidden)



    private fun createDelegate(

        recordedTask: ActivityManager.RunningTaskInfo?

packages/SystemUI/src/com/android/systemui/statusbar/chips/sharetoapp/ui/viewmodel/ShareToAppChipViewModel.kt

+2 −1
Original line number Diff line number Diff line
@@ -66,7 +66,8 @@ constructor( 
                    }

                }

            }

            .stateIn(scope, SharingStarted.WhileSubscribed(), OngoingActivityChipModel.Hidden)

            // See b/347726238.

            .stateIn(scope, SharingStarted.Lazily, OngoingActivityChipModel.Hidden)



    /** Stops the currently active projection. */

    private fun stopProjecting() {

packages/SystemUI/src/com/android/systemui/statusbar/chips/ui/viewmodel/OngoingActivityChipsViewModel.kt

+4 −1
Original line number Diff line number Diff line
@@ -72,5 +72,8 @@ constructor( 
                    else -> call

                }

            }

            .stateIn(scope, SharingStarted.WhileSubscribed(), OngoingActivityChipModel.Hidden)

            // Some of the chips could have timers in them and we don't want the start time

            // for those timers to get reset for any reason. So, as soon as any subscriber has

            // requested the chip information, we need to maintain it forever. See b/347726238.

            .stateIn(scope, SharingStarted.Lazily, OngoingActivityChipModel.Hidden)

}

packages/SystemUI/tests/src/com/android/systemui/statusbar/chips/ui/viewmodel/OngoingActivityChipsViewModelTest.kt

+38 −0
Original line number Diff line number Diff line
@@ -34,7 +34,11 @@ import com.android.systemui.statusbar.chips.mediaprojection.domain.interactor.Me 
import com.android.systemui.statusbar.chips.ui.model.OngoingActivityChipModel

import com.android.systemui.statusbar.phone.ongoingcall.data.repository.ongoingCallRepository

import com.android.systemui.statusbar.phone.ongoingcall.shared.model.OngoingCallModel

import com.android.systemui.util.time.fakeSystemClock

import com.google.common.truth.Truth.assertThat

import kotlinx.coroutines.flow.launchIn

import kotlinx.coroutines.flow.onEach

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runTest

import org.junit.Before

import org.junit.Test
@@ -43,6 +47,7 @@ import org.junit.Test 
class OngoingActivityChipsViewModelTest : SysuiTestCase() {

    private val kosmos = Kosmos().also { it.testCase = this }

    private val testScope = kosmos.testScope

    private val systemClock = kosmos.fakeSystemClock



    private val screenRecordState = kosmos.screenRecordRepository.screenRecordState

    private val mediaProjectionState = kosmos.fakeMediaProjectionRepository.mediaProjectionState
@@ -188,6 +193,39 @@ class OngoingActivityChipsViewModelTest : SysuiTestCase() { 
            assertIsCallChip(latest)

        }



    /** Regression test for b/347726238. */

    @Test

    fun chip_timerDoesNotResetAfterSubscribersRestart() =

        testScope.runTest {

            var latest: OngoingActivityChipModel? = null



            val job1 = underTest.chip.onEach { latest = it }.launchIn(this)



            // Start a chip with a timer

            systemClock.setElapsedRealtime(1234)

            screenRecordState.value = ScreenRecordModel.Recording



            runCurrent()



            assertThat((latest as OngoingActivityChipModel.Shown.Timer).startTimeMs).isEqualTo(1234)



            // Stop subscribing to the chip flow

            job1.cancel()



            // Let time pass

            systemClock.setElapsedRealtime(5678)



            // WHEN we re-subscribe to the chip flow

            val job2 = underTest.chip.onEach { latest = it }.launchIn(this)



            runCurrent()



            // THEN the old start time is still used

            assertThat((latest as OngoingActivityChipModel.Shown.Timer).startTimeMs).isEqualTo(1234)



            job2.cancel()

        }



    companion object {

        fun assertIsScreenRecordChip(latest: OngoingActivityChipModel?) {

            assertThat(latest).isInstanceOf(OngoingActivityChipModel.Shown::class.java)