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

Commit 94587440 authored by Matt Pietal's avatar Matt Pietal
Browse files

Use shared flow for keyguard going away

It is possible for a slow consumer to receive conflated values from
a state flow. Events like keyguardGoingAway should never be conflated,
or visual glitches could occur.

Fixes: 407825005
Test: atest KeyguardRepositoryImplTest
Flag: EXEMPT bugfix
Change-Id: I23d72503c2b44e78dcfd5533f2f1a34d8c7bdf6e
parent bd599322
Loading
Loading
Loading
Loading
+8 −40
Original line number Diff line number Diff line
@@ -45,9 +45,8 @@ 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.StandardTestDispatcher
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Test
@@ -73,9 +72,8 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
    @Mock private lateinit var userTracker: UserTracker
    @Mock private lateinit var lockPatternUtils: LockPatternUtils
    @Captor private lateinit var updateCallbackCaptor: ArgumentCaptor<KeyguardUpdateMonitorCallback>
    private val mainDispatcher = StandardTestDispatcher()
    private val testDispatcher = StandardTestDispatcher()
    private val testScope = TestScope(testDispatcher)
    private val dispatcher = UnconfinedTestDispatcher()
    private val testScope = TestScope(dispatcher)
    private lateinit var systemClock: FakeSystemClock
    private lateinit var facePropertyRepository: FakeFacePropertyRepository

@@ -94,7 +92,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
                dozeTransitionListener,
                authController,
                dreamOverlayCallbackController,
                mainDispatcher,
                dispatcher,
                testScope.backgroundScope,
                systemClock,
                facePropertyRepository,
@@ -172,7 +170,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            var latest: Boolean? = null
            val job = underTest.isKeyguardShowing.onEach { latest = it }.launchIn(this)

            runCurrent()
            assertThat(latest).isFalse()
            assertThat(underTest.isKeyguardShowing()).isFalse()

@@ -181,13 +178,11 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {

            whenever(keyguardStateController.isShowing).thenReturn(true)
            captor.value.onKeyguardShowingChanged()
            runCurrent()
            assertThat(latest).isTrue()
            assertThat(underTest.isKeyguardShowing()).isTrue()

            whenever(keyguardStateController.isShowing).thenReturn(false)
            captor.value.onKeyguardShowingChanged()
            runCurrent()
            assertThat(latest).isFalse()
            assertThat(underTest.isKeyguardShowing()).isFalse()

@@ -201,7 +196,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            var latest: Boolean? = null
            val job = underTest.isKeyguardOccluded.onEach { latest = it }.launchIn(this)

            runCurrent()
            assertThat(latest).isFalse()

            val captor = argumentCaptor<KeyguardStateController.Callback>()
@@ -209,12 +203,10 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {

            whenever(keyguardStateController.isOccluded).thenReturn(true)
            captor.value.onKeyguardShowingChanged()
            runCurrent()
            assertThat(latest).isTrue()

            whenever(keyguardStateController.isOccluded).thenReturn(false)
            captor.value.onKeyguardShowingChanged()
            runCurrent()
            assertThat(latest).isFalse()

            job.cancel()
@@ -226,7 +218,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            whenever(keyguardStateController.isUnlocked).thenReturn(false)
            val isKeyguardUnlocked by collectLastValue(underTest.isKeyguardDismissible)

            runCurrent()
            assertThat(isKeyguardUnlocked).isFalse()

            val captor = argumentCaptor<KeyguardStateController.Callback>()
@@ -234,12 +225,10 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {

            whenever(keyguardStateController.isUnlocked).thenReturn(true)
            captor.value.onUnlockedChanged()
            runCurrent()
            assertThat(isKeyguardUnlocked).isTrue()

            whenever(keyguardStateController.isUnlocked).thenReturn(false)
            captor.value.onKeyguardShowingChanged()
            runCurrent()
            assertThat(isKeyguardUnlocked).isFalse()
        }

@@ -277,12 +266,11 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
    fun isKeyguardGoingAway() =
        testScope.runTest {
            val isGoingAway by collectLastValue(underTest.isKeyguardGoingAway)
            assertThat(isGoingAway).isFalse()

            underTest.isKeyguardGoingAway.value = true
            underTest.isKeyguardGoingAway.tryEmit(true)
            assertThat(isGoingAway).isTrue()

            underTest.isKeyguardGoingAway.value = false
            underTest.isKeyguardGoingAway.tryEmit(false)
            assertThat(isGoingAway).isFalse()
        }

@@ -306,7 +294,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            var latest: Boolean? = null
            val job = underTest.isDreamingWithOverlay.onEach { latest = it }.launchIn(this)

            runCurrent()
            assertThat(latest).isFalse()

            val listener =
@@ -315,11 +302,9 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
                }

            listener.onStartDream()
            runCurrent()
            assertThat(latest).isTrue()

            listener.onWakeUp()
            runCurrent()
            assertThat(latest).isFalse()

            job.cancel()
@@ -335,7 +320,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            val values = mutableListOf<DozeTransitionModel>()
            val job = underTest.dozeTransitionModel.onEach(values::add).launchIn(this)

            runCurrent()
            val listener =
                withArgCaptor<DozeTransitionCallback> {
                    verify(dozeTransitionListener).addCallback(capture())
@@ -344,26 +328,20 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            // These don't have to reflect real transitions from the DozeMachine. Only that the
            // transitions are properly emitted
            listener.onDozeTransition(DozeMachine.State.INITIALIZED, DozeMachine.State.DOZE)
            runCurrent()
            listener.onDozeTransition(DozeMachine.State.DOZE, DozeMachine.State.DOZE_AOD)
            runCurrent()
            listener.onDozeTransition(DozeMachine.State.DOZE_AOD_DOCKED, DozeMachine.State.FINISH)
            runCurrent()
            listener.onDozeTransition(
                DozeMachine.State.DOZE_REQUEST_PULSE,
                DozeMachine.State.DOZE_PULSING,
            )
            runCurrent()
            listener.onDozeTransition(
                DozeMachine.State.DOZE_SUSPEND_TRIGGERS,
                DozeMachine.State.DOZE_PULSE_DONE,
            )
            runCurrent()
            listener.onDozeTransition(
                DozeMachine.State.DOZE_AOD_PAUSING,
                DozeMachine.State.DOZE_AOD_PAUSED,
            )
            runCurrent()

            assertThat(values)
                .isEqualTo(
@@ -392,7 +370,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
                )

            job.cancel()
            runCurrent()
            verify(dozeTransitionListener).removeCallback(listener)
        }

@@ -402,7 +379,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            val values = mutableListOf<Point?>()
            val job = underTest.fingerprintSensorLocation.onEach(values::add).launchIn(this)

            runCurrent()
            val captor = argumentCaptor<AuthController.Callback>()
            verify(authController).addCallback(captor.capture())

@@ -416,7 +392,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
                .onEach {
                    whenever(authController.fingerprintSensorLocation).thenReturn(it)
                    captor.value.onFingerprintLocationChanged()
                    runCurrent()
                }
                .also { dispatchedSensorLocations ->
                    assertThat(values).isEqualTo(listOf(null) + dispatchedSensorLocations)
@@ -430,7 +405,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
        testScope.runTest {
            val values = mutableListOf<Point?>()
            val job = underTest.faceSensorLocation.onEach(values::add).launchIn(this)
            runCurrent()

            // An initial, null value should be initially emitted so that flows combined with this
            // one
@@ -439,10 +413,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            assertThat(values).isEqualTo(listOf(null))

            listOf(Point(500, 500), Point(0, 0), null, Point(250, 250))
                .onEach {
                    facePropertyRepository.setSensorLocation(it)
                    runCurrent()
                }
                .onEach { facePropertyRepository.setSensorLocation(it) }
                .also { dispatchedSensorLocations ->
                    assertThat(values).isEqualTo(listOf(null) + dispatchedSensorLocations)
                }
@@ -456,8 +427,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
            val values = mutableListOf<BiometricUnlockSource?>()
            val job = underTest.biometricUnlockState.onEach { values.add(it.source) }.launchIn(this)

            runCurrent()

            // An initial, null value should be initially emitted so that flows combined with this
            // one
            // emit values immediately. The biometric unlock source is expected to be nullable, so
@@ -476,7 +445,6 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {
                        BiometricUnlockMode.NONE,
                        BiometricUnlockSource.Companion.fromBiometricSourceType(biometricSourceType),
                    )
                    runCurrent()
                }

            assertThat(values)
@@ -496,7 +464,7 @@ class KeyguardRepositoryImplTest : SysuiTestCase() {

    @Test
    fun isEncryptedOrLockdown() =
        TestScope(mainDispatcher).runTest {
        testScope.runTest {
            whenever(userTracker.userId).thenReturn(0)
            whenever(keyguardUpdateMonitor.isEncryptedOrLockdown(0)).thenReturn(true)

+0 −13
Original line number Diff line number Diff line
@@ -134,19 +134,6 @@ class FromDozingTransitionInteractorTest(flags: FlagsParameterization?) : SysuiT
                .startedTransition(from = KeyguardState.DOZING, to = KeyguardState.LOCKSCREEN)
        }

    @Test
    @DisableFlags(FLAG_KEYGUARD_WM_STATE_REFACTOR)
    fun testTransitionToGone_onWakeup_whenGoingAway() =
        kosmos.runTest {
            keyguardInteractor.setIsKeyguardGoingAway(true)

            powerInteractor.setAwakeForTest()
            testScope.advanceTimeBy(60L)

            assertThat(transitionRepository)
                .startedTransition(from = KeyguardState.DOZING, to = KeyguardState.GONE)
        }

    @Test
    fun testTransitionToDreaming() =
        kosmos.runTest {
+9 −3
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

package com.android.systemui.keyguard.data.repository

import android.annotation.SuppressLint
import android.graphics.Point
import com.android.app.tracing.coroutines.launchTraced as launch
import com.android.internal.widget.LockPatternUtils
@@ -49,6 +50,7 @@ import com.android.systemui.utils.coroutines.flow.conflatedCallbackFlow
import javax.inject.Inject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
@@ -108,7 +110,7 @@ interface KeyguardRepository {
            "away' is isInTransitionToState(GONE), but consider using more specific flows " +
            "whenever possible."
    )
    val isKeyguardGoingAway: MutableStateFlow<Boolean>
    val isKeyguardGoingAway: MutableSharedFlow<Boolean>

    /**
     * Whether the keyguard is enabled, per [KeyguardService]. If the keyguard is not enabled, the
@@ -373,8 +375,12 @@ constructor(
    override val isKeyguardDismissible: MutableStateFlow<Boolean> =
        MutableStateFlow(keyguardStateController.isUnlocked)

    override val isKeyguardGoingAway: MutableStateFlow<Boolean> =
        MutableStateFlow(keyguardStateController.isKeyguardGoingAway)
    @SuppressLint("SharedFlowCreation")
    override val isKeyguardGoingAway: MutableSharedFlow<Boolean> =
        MutableSharedFlow<Boolean>(
            extraBufferCapacity = 3,
            onBufferOverflow = BufferOverflow.DROP_OLDEST,
        )

    private val _isKeyguardEnabled =
        MutableStateFlow(!lockPatternUtils.isLockScreenDisabled(userTracker.userId))
+2 −8
Original line number Diff line number Diff line
@@ -157,7 +157,6 @@ constructor(
                .collect { (detailedWakefulness, isCommunalAvailable, shouldShowCommunal) ->
                    val isKeyguardOccludedLegacy = keyguardInteractor.isKeyguardOccluded.value
                    val primaryBouncerShowing = keyguardInteractor.primaryBouncerShowing.value
                    val isKeyguardGoingAway = keyguardInteractor.isKeyguardGoingAway.value

                    if (!deviceEntryInteractor.isLockscreenEnabled()) {
                        if (!SceneContainerFlag.isEnabled) {
@@ -166,16 +165,11 @@ constructor(
                                ownerReason = "lockscreen not enabled",
                            )
                        }
                    } else if (canDismissLockscreen() || isKeyguardGoingAway) {
                    } else if (canDismissLockscreen()) {
                        if (!SceneContainerFlag.isEnabled) {
                            startTransitionTo(
                                KeyguardState.GONE,
                                ownerReason =
                                    if (canDismissLockscreen()) {
                                        "canDismissLockscreen()"
                                    } else {
                                        "isKeyguardGoingAway"
                                    },
                                ownerReason = "canDismissLockscreen()",
                            )
                        }
                    } else if (primaryBouncerShowing) {
+4 −2
Original line number Diff line number Diff line
@@ -59,8 +59,10 @@ import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.combine
@@ -238,7 +240,7 @@ constructor(

    /** Whether the keyguard is going away. */
    @Deprecated("Use KeyguardTransitionInteractor + KeyguardState.GONE")
    val isKeyguardGoingAway: StateFlow<Boolean> = repository.isKeyguardGoingAway.asStateFlow()
    val isKeyguardGoingAway: SharedFlow<Boolean> = repository.isKeyguardGoingAway.asSharedFlow()

    /** Keyguard can be clipped at the top as the shade is dragged */
    val topClippingBounds: Flow<Int?> by lazy {
@@ -527,7 +529,7 @@ constructor(
    }

    fun setIsKeyguardGoingAway(isGoingAway: Boolean) {
        repository.isKeyguardGoingAway.value = isGoingAway
        repository.isKeyguardGoingAway.tryEmit(isGoingAway)
    }

    fun setNotificationStackAbsoluteBottom(bottom: Float) {