From 07a2e31819fa066ba2d1e7d28d76baebaf9eab29 Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Tue, 8 Aug 2023 12:50:43 -0700 Subject: [PATCH] [flexiglass] Dismiss keyguard when any scene goes to Gone. We have a bug where moving to the Gone scene from scenes that are not Bouncer (for example from lockscreen in Swipe) cause a black screen to appear. This happens because the code in KeyguardSecurityContainerController which is responsible for dismissing the keyguard only did so for the bouncer -> gone scene change. The CL fixes the bug by allowing any change to the gone scene to trigger the dismissal of the keyguard. Fix: 295038434 Fix: 295223686 Test: unit tests still pass Test: verified that I can see the launcher when swiping away the lockscreen Change-Id: Ied9fb452755a2791ee8bb28f0faa29f9d4ddfd0d Merged-In: Ied9fb452755a2791ee8bb28f0faa29f9d4ddfd0d --- .../KeyguardSecurityContainerController.java | 27 +++-- .../interactor/AuthenticationInteractor.kt | 8 +- .../domain/interactor/SceneInteractor.kt | 39 +++--- ...KeyguardSecurityContainerControllerTest.kt | 58 +++++++-- .../AuthenticationInteractorTest.kt | 66 ++++++++++- .../domain/interactor/SceneInteractorTest.kt | 112 ++++++------------ .../android/systemui/scene/SceneTestUtils.kt | 1 + 7 files changed, 191 insertions(+), 120 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java index 4e1cbc737d5fc..92c0d6453d1ec 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java @@ -69,6 +69,7 @@ import com.android.keyguard.dagger.KeyguardBouncerScope; import com.android.settingslib.utils.ThreadUtils; import com.android.systemui.Gefingerpoken; import com.android.systemui.R; +import com.android.systemui.authentication.domain.interactor.AuthenticationInteractor; import com.android.systemui.biometrics.FaceAuthAccessibilityDelegate; import com.android.systemui.biometrics.SideFpsController; import com.android.systemui.biometrics.SideFpsUiRequestSource; @@ -81,8 +82,6 @@ import com.android.systemui.keyguard.domain.interactor.KeyguardFaceAuthInteracto import com.android.systemui.log.SessionTracker; import com.android.systemui.plugins.ActivityStarter; import com.android.systemui.plugins.FalsingManager; -import com.android.systemui.scene.domain.interactor.SceneInteractor; -import com.android.systemui.scene.shared.model.SceneKey; import com.android.systemui.shared.system.SysUiStatsLog; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.KeyguardStateController; @@ -388,7 +387,7 @@ public class KeyguardSecurityContainerController extends ViewController mSceneInteractor; + private final Provider mAuthenticationInteractor; private final Provider mJavaAdapter; @Nullable private Job mSceneTransitionCollectionJob; @@ -419,7 +418,7 @@ public class KeyguardSecurityContainerController extends ViewController javaAdapter, UserInteractor userInteractor, FaceAuthAccessibilityDelegate faceAuthAccessibilityDelegate, - Provider sceneInteractor + Provider authenticationInteractor ) { super(view); view.setAccessibilityDelegate(faceAuthAccessibilityDelegate); @@ -448,7 +447,7 @@ public class KeyguardSecurityContainerController extends ViewController { - final int selectedUserId = mUserInteractor.getSelectedUserId(); - showNextSecurityScreenOrFinish( + mAuthenticationInteractor.get().isLockscreenDismissed(), + isLockscreenDismissed -> { + if (isLockscreenDismissed) { + final int selectedUserId = mUserInteractor.getSelectedUserId(); + showNextSecurityScreenOrFinish( /* authenticated= */ true, selectedUserId, /* bypassSecondaryLockScreen= */ true, mSecurityModel.getSecurityMode(selectedUserId)); - }); + } + } + ); } } diff --git a/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt b/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt index 4ab884494a067..ecd7baea77799 100644 --- a/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractor.kt @@ -114,14 +114,18 @@ constructor( * - `true` doesn't mean the lockscreen is invisible (since this state changes before the * transition occurs). */ - private val isLockscreenDismissed = + val isLockscreenDismissed: StateFlow = sceneInteractor.desiredScene .map { it.key } .filter { currentScene -> currentScene == SceneKey.Gone || currentScene == SceneKey.Lockscreen } .map { it == SceneKey.Gone } - .distinctUntilChanged() + .stateIn( + scope = applicationScope, + started = SharingStarted.WhileSubscribed(), + initialValue = false, + ) /** * Whether it's currently possible to swipe up to dismiss the lockscreen without requiring diff --git a/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt b/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt index cf7abdd34b707..76d9b039e1129 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt @@ -17,21 +17,22 @@ package com.android.systemui.scene.domain.interactor import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.scene.data.repository.SceneContainerRepository import com.android.systemui.scene.shared.logger.SceneLogger import com.android.systemui.scene.shared.model.ObservableTransitionState import com.android.systemui.scene.shared.model.RemoteUserInput import com.android.systemui.scene.shared.model.SceneKey import com.android.systemui.scene.shared.model.SceneModel -import com.android.systemui.util.kotlin.pairwise import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.flow.stateIn /** * Generic business logic and app state accessors for the scene framework. @@ -44,6 +45,7 @@ import kotlinx.coroutines.flow.mapNotNull class SceneInteractor @Inject constructor( + @Application applicationScope: CoroutineScope, private val repository: SceneContainerRepository, private val logger: SceneLogger, ) { @@ -88,6 +90,22 @@ constructor( */ val transitionState: StateFlow = repository.transitionState + /** + * The key of the scene that the UI is currently transitioning to or `null` if there is no + * active transition at the moment. + * + * This is a convenience wrapper around [transitionState], meant for flow-challenged consumers + * like Java code. + */ + val transitioningTo: StateFlow = + transitionState + .map { state -> (state as? ObservableTransitionState.Transition)?.toScene } + .stateIn( + scope = applicationScope, + started = SharingStarted.WhileSubscribed(), + initialValue = null, + ) + /** Whether the scene container is visible. */ val isVisible: StateFlow = repository.isVisible @@ -142,21 +160,6 @@ constructor( repository.setTransitionState(transitionState) } - /** - * Returns a stream of events that emits one [Unit] every time the framework transitions from - * [from] to [to]. - */ - fun finishedSceneTransitions(from: SceneKey, to: SceneKey): Flow { - return transitionState - .mapNotNull { it as? ObservableTransitionState.Idle } - .map { idleState -> idleState.scene } - .distinctUntilChanged() - .pairwise() - .mapNotNull { (previousSceneKey, currentSceneKey) -> - Unit.takeIf { previousSceneKey == from && currentSceneKey == to } - } - } - /** Handles a remote user input. */ fun onRemoteUserInput(input: RemoteUserInput) { _remoteUserInput.value = input diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.kt b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.kt index 9ba21da593611..8f27b59487f48 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.kt @@ -38,6 +38,7 @@ import com.android.keyguard.KeyguardSecurityContainer.UserSwitcherViewMode.UserS import com.android.keyguard.KeyguardSecurityModel.SecurityMode import com.android.systemui.R import com.android.systemui.SysuiTestCase +import com.android.systemui.authentication.domain.interactor.AuthenticationInteractor import com.android.systemui.biometrics.FaceAuthAccessibilityDelegate import com.android.systemui.biometrics.SideFpsController import com.android.systemui.biometrics.SideFpsUiRequestSource @@ -144,6 +145,7 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { private lateinit var testableResources: TestableResources private lateinit var sceneTestUtils: SceneTestUtils private lateinit var sceneInteractor: SceneInteractor + private lateinit var authenticationInteractor: AuthenticationInteractor private lateinit var sceneTransitionStateFlow: MutableStateFlow private lateinit var underTest: KeyguardSecurityContainerController @@ -207,6 +209,11 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { sceneTransitionStateFlow = MutableStateFlow(ObservableTransitionState.Idle(SceneKey.Lockscreen)) sceneInteractor.setTransitionState(sceneTransitionStateFlow) + authenticationInteractor = + sceneTestUtils.authenticationInteractor( + repository = sceneTestUtils.authenticationRepository(), + sceneInteractor = sceneInteractor + ) underTest = KeyguardSecurityContainerController( @@ -237,7 +244,7 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { userInteractor, faceAuthAccessibilityDelegate, ) { - sceneInteractor + authenticationInteractor } } @@ -753,7 +760,7 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { } @Test - fun dismissesKeyguard_whenSceneChangesFromBouncerToGone() = + fun dismissesKeyguard_whenSceneChangesToGone() = sceneTestUtils.testScope.runTest { featureFlags.set(Flags.SCENE_CONTAINER, true) @@ -790,12 +797,32 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { runCurrent() verify(viewMediatorCallback).keyguardDone(anyBoolean(), anyInt()) + // While listening, moving back to the lockscreen scene does not dismiss the keyguard + // again. + clearInvocations(viewMediatorCallback) + sceneInteractor.changeScene(SceneModel(SceneKey.Lockscreen, null), "reason") + sceneTransitionStateFlow.value = + ObservableTransitionState.Transition( + SceneKey.Gone, + SceneKey.Lockscreen, + flowOf(.5f) + ) + runCurrent() + sceneInteractor.onSceneChanged(SceneModel(SceneKey.Lockscreen, null), "reason") + sceneTransitionStateFlow.value = ObservableTransitionState.Idle(SceneKey.Lockscreen) + runCurrent() + verify(viewMediatorCallback, never()).keyguardDone(anyBoolean(), anyInt()) + // While listening, moving back to the bouncer scene does not dismiss the keyguard // again. clearInvocations(viewMediatorCallback) sceneInteractor.changeScene(SceneModel(SceneKey.Bouncer, null), "reason") sceneTransitionStateFlow.value = - ObservableTransitionState.Transition(SceneKey.Gone, SceneKey.Bouncer, flowOf(.5f)) + ObservableTransitionState.Transition( + SceneKey.Lockscreen, + SceneKey.Bouncer, + flowOf(.5f) + ) runCurrent() sceneInteractor.onSceneChanged(SceneModel(SceneKey.Bouncer, null), "reason") sceneTransitionStateFlow.value = ObservableTransitionState.Idle(SceneKey.Bouncer) @@ -815,7 +842,21 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { runCurrent() verify(viewMediatorCallback, never()).keyguardDone(anyBoolean(), anyInt()) - // While not listening, moving back to the bouncer does not dismiss the keyguard. + // While not listening, moving to the lockscreen does not dismiss the keyguard. + sceneInteractor.changeScene(SceneModel(SceneKey.Lockscreen, null), "reason") + sceneTransitionStateFlow.value = + ObservableTransitionState.Transition( + SceneKey.Gone, + SceneKey.Lockscreen, + flowOf(.5f) + ) + runCurrent() + sceneInteractor.onSceneChanged(SceneModel(SceneKey.Lockscreen, null), "reason") + sceneTransitionStateFlow.value = ObservableTransitionState.Idle(SceneKey.Lockscreen) + runCurrent() + verify(viewMediatorCallback, never()).keyguardDone(anyBoolean(), anyInt()) + + // While not listening, moving to the bouncer does not dismiss the keyguard. sceneInteractor.changeScene(SceneModel(SceneKey.Bouncer, null), "reason") sceneTransitionStateFlow.value = ObservableTransitionState.Transition(SceneKey.Gone, SceneKey.Bouncer, flowOf(.5f)) @@ -826,12 +867,15 @@ class KeyguardSecurityContainerControllerTest : SysuiTestCase() { verify(viewMediatorCallback, never()).keyguardDone(anyBoolean(), anyInt()) // Reattaching the view starts listening again so moving from the bouncer scene to the - // gone - // scene now does dismiss the keyguard again. + // gone scene now does dismiss the keyguard again, this time from lockscreen. underTest.onViewAttached() sceneInteractor.changeScene(SceneModel(SceneKey.Gone, null), "reason") sceneTransitionStateFlow.value = - ObservableTransitionState.Transition(SceneKey.Bouncer, SceneKey.Gone, flowOf(.5f)) + ObservableTransitionState.Transition( + SceneKey.Lockscreen, + SceneKey.Gone, + flowOf(.5f) + ) runCurrent() sceneInteractor.onSceneChanged(SceneModel(SceneKey.Gone, null), "reason") sceneTransitionStateFlow.value = ObservableTransitionState.Idle(SceneKey.Gone) diff --git a/packages/SystemUI/tests/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractorTest.kt index fc7d20aeb3565..707b1b37afd6c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/authentication/domain/interactor/AuthenticationInteractorTest.kt @@ -33,6 +33,7 @@ import com.google.common.truth.Truth.assertThat import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.seconds import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest @@ -605,7 +606,68 @@ class AuthenticationInteractorTest : SysuiTestCase() { assertThat(hintedPinLength).isNull() } - private fun switchToScene(sceneKey: SceneKey) { - sceneInteractor.changeScene(SceneModel(sceneKey), "reason") + @Test + fun isLockscreenDismissed() = + testScope.runTest { + val isLockscreenDismissed by collectLastValue(underTest.isLockscreenDismissed) + // Start on lockscreen. + switchToScene(SceneKey.Lockscreen) + assertThat(isLockscreenDismissed).isFalse() + + // The user swipes down to reveal shade. + switchToScene(SceneKey.Shade) + assertThat(isLockscreenDismissed).isFalse() + + // The user swipes down to reveal quick settings. + switchToScene(SceneKey.QuickSettings) + assertThat(isLockscreenDismissed).isFalse() + + // The user swipes up to go back to shade. + switchToScene(SceneKey.Shade) + assertThat(isLockscreenDismissed).isFalse() + + // The user swipes up to reveal bouncer. + switchToScene(SceneKey.Bouncer) + assertThat(isLockscreenDismissed).isFalse() + + // The user hits back to return to lockscreen. + switchToScene(SceneKey.Lockscreen) + assertThat(isLockscreenDismissed).isFalse() + + // The user swipes up to reveal bouncer. + switchToScene(SceneKey.Bouncer) + assertThat(isLockscreenDismissed).isFalse() + + // The user enters correct credentials and goes to gone. + switchToScene(SceneKey.Gone) + assertThat(isLockscreenDismissed).isTrue() + + // The user swipes down to reveal shade. + switchToScene(SceneKey.Shade) + assertThat(isLockscreenDismissed).isTrue() + + // The user swipes down to reveal quick settings. + switchToScene(SceneKey.QuickSettings) + assertThat(isLockscreenDismissed).isTrue() + + // The user swipes up to go back to shade. + switchToScene(SceneKey.Shade) + assertThat(isLockscreenDismissed).isTrue() + + // The user swipes up to go back to gone. + switchToScene(SceneKey.Gone) + assertThat(isLockscreenDismissed).isTrue() + + // The device goes to sleep, returning to the lockscreen. + switchToScene(SceneKey.Lockscreen) + assertThat(isLockscreenDismissed).isFalse() + } + + private fun TestScope.switchToScene(sceneKey: SceneKey) { + val model = SceneModel(sceneKey) + val loggingReason = "reason" + sceneInteractor.changeScene(model, loggingReason) + sceneInteractor.onSceneChanged(model, loggingReason) + runCurrent() } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt index 0a93a7ca465fb..16cc924b5754a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt @@ -28,9 +28,6 @@ import com.android.systemui.scene.shared.model.SceneModel import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.launch -import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Test import org.junit.runner.RunWith @@ -104,6 +101,40 @@ class SceneInteractorTest : SysuiTestCase() { ) } + @Test + fun transitioningTo() = + testScope.runTest { + val transitionState = + MutableStateFlow( + ObservableTransitionState.Idle(underTest.desiredScene.value.key) + ) + underTest.setTransitionState(transitionState) + + val transitionTo by collectLastValue(underTest.transitioningTo) + assertThat(transitionTo).isNull() + + underTest.changeScene(SceneModel(SceneKey.Shade), "reason") + assertThat(transitionTo).isNull() + + val progress = MutableStateFlow(0f) + transitionState.value = + ObservableTransitionState.Transition( + fromScene = underTest.desiredScene.value.key, + toScene = SceneKey.Shade, + progress = progress, + ) + assertThat(transitionTo).isEqualTo(SceneKey.Shade) + + progress.value = 0.5f + assertThat(transitionTo).isEqualTo(SceneKey.Shade) + + progress.value = 1f + assertThat(transitionTo).isEqualTo(SceneKey.Shade) + + transitionState.value = ObservableTransitionState.Idle(SceneKey.Shade) + assertThat(transitionTo).isNull() + } + @Test fun isVisible() = testScope.runTest { @@ -117,81 +148,6 @@ class SceneInteractorTest : SysuiTestCase() { assertThat(isVisible).isTrue() } - @Test - fun finishedSceneTransitions() = - testScope.runTest { - val transitionState = - MutableStateFlow( - ObservableTransitionState.Idle(SceneKey.Lockscreen) - ) - underTest.setTransitionState(transitionState) - var transitionCount = 0 - val job = launch { - underTest - .finishedSceneTransitions( - from = SceneKey.Shade, - to = SceneKey.QuickSettings, - ) - .collect { transitionCount++ } - } - - assertThat(transitionCount).isEqualTo(0) - - underTest.changeScene(SceneModel(SceneKey.Shade), "reason") - transitionState.value = - ObservableTransitionState.Transition( - fromScene = SceneKey.Lockscreen, - toScene = SceneKey.Shade, - progress = flowOf(0.5f), - ) - runCurrent() - underTest.onSceneChanged(SceneModel(SceneKey.Shade), "reason") - transitionState.value = ObservableTransitionState.Idle(SceneKey.Shade) - runCurrent() - assertThat(transitionCount).isEqualTo(0) - - underTest.changeScene(SceneModel(SceneKey.QuickSettings), "reason") - transitionState.value = - ObservableTransitionState.Transition( - fromScene = SceneKey.Shade, - toScene = SceneKey.QuickSettings, - progress = flowOf(0.5f), - ) - runCurrent() - underTest.onSceneChanged(SceneModel(SceneKey.QuickSettings), "reason") - transitionState.value = ObservableTransitionState.Idle(SceneKey.QuickSettings) - runCurrent() - assertThat(transitionCount).isEqualTo(1) - - underTest.changeScene(SceneModel(SceneKey.Shade), "reason") - transitionState.value = - ObservableTransitionState.Transition( - fromScene = SceneKey.QuickSettings, - toScene = SceneKey.Shade, - progress = flowOf(0.5f), - ) - runCurrent() - underTest.onSceneChanged(SceneModel(SceneKey.Shade), "reason") - transitionState.value = ObservableTransitionState.Idle(SceneKey.Shade) - runCurrent() - assertThat(transitionCount).isEqualTo(1) - - underTest.changeScene(SceneModel(SceneKey.QuickSettings), "reason") - transitionState.value = - ObservableTransitionState.Transition( - fromScene = SceneKey.Shade, - toScene = SceneKey.QuickSettings, - progress = flowOf(0.5f), - ) - runCurrent() - underTest.onSceneChanged(SceneModel(SceneKey.QuickSettings), "reason") - transitionState.value = ObservableTransitionState.Idle(SceneKey.QuickSettings) - runCurrent() - assertThat(transitionCount).isEqualTo(2) - - job.cancel() - } - @Test fun remoteUserInput() = testScope.runTest { diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/scene/SceneTestUtils.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/scene/SceneTestUtils.kt index 893bbf38ff265..dd45331df4b34 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/scene/SceneTestUtils.kt +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/scene/SceneTestUtils.kt @@ -125,6 +125,7 @@ class SceneTestUtils( repository: SceneContainerRepository = fakeSceneContainerRepository() ): SceneInteractor { return SceneInteractor( + applicationScope = applicationScope(), repository = repository, logger = mock(), )