From 5783a9926d12346fe8d1eb5dcae096be6e53a2df Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Wed, 25 Sep 2024 16:23:31 -0700 Subject: [flexiglass] Fixes issue where user management settings didn't show In the issue, when the auth method is SWIPE and the device is not yet entered, opening the user management activity from the icon in the footer of the QS scene incorrectly stays on the gone scene and doesn't start the activity. The fix is to lean more heavily on the currentScene flow instead of the transitionState flow that KTF is set up to use. This way, the timing of the excuteDismissAction and resetDismissAction is correct (first, exectute, then reset). Fix: 366282915 Test: unit test added. It fails withoug the fix and succeeds with it Test: manually verified the mentioned user flow with SWIPE and with PATTERN to make sure it always works right Flag: com.android.systemui.scene_container Change-Id: I987d0dc88a35b4afeddc87c75e2dd71e853aee8c --- .../KeyguardDismissActionInteractorTest.kt | 84 ++++++++++++++++++++++ .../interactor/KeyguardDismissActionInteractor.kt | 37 ++++++++-- 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractorTest.kt index ba689179c33d..9504d9e17593 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractorTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractorTest.kt @@ -19,17 +19,22 @@ package com.android.systemui.keyguard.domain.interactor import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SmallTest +import com.android.compose.animation.scene.ObservableTransitionState import com.android.systemui.SysuiTestCase import com.android.systemui.authentication.data.repository.fakeAuthenticationRepository import com.android.systemui.authentication.shared.model.AuthenticationMethodModel import com.android.systemui.bouncer.data.repository.fakeKeyguardBouncerRepository import com.android.systemui.bouncer.domain.interactor.alternateBouncerInteractor import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.deviceentry.data.repository.fakeDeviceEntryRepository +import com.android.systemui.deviceentry.domain.interactor.deviceEntryInteractor import com.android.systemui.deviceentry.domain.interactor.deviceUnlockedInteractor import com.android.systemui.flags.EnableSceneContainer +import com.android.systemui.keyguard.data.repository.fakeDeviceEntryFingerprintAuthRepository import com.android.systemui.keyguard.data.repository.fakeKeyguardRepository import com.android.systemui.keyguard.shared.model.DismissAction import com.android.systemui.keyguard.shared.model.KeyguardDone +import com.android.systemui.keyguard.shared.model.SuccessFingerprintAuthenticationStatus import com.android.systemui.kosmos.testScope import com.android.systemui.power.data.repository.fakePowerRepository import com.android.systemui.power.domain.interactor.powerInteractor @@ -44,6 +49,7 @@ import com.android.systemui.shade.domain.interactor.shadeInteractor import com.android.systemui.testKosmos import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest @@ -180,7 +186,11 @@ class KeyguardDismissActionInteractorTest : SysuiTestCase() { ) assertThat(executeDismissAction).isNull() + kosmos.fakeDeviceEntryFingerprintAuthRepository.setAuthenticationStatus( + SuccessFingerprintAuthenticationStatus(0, true) + ) kosmos.setSceneTransition(Idle(Scenes.Gone)) + kosmos.sceneInteractor.changeScene(Scenes.Gone, "") assertThat(executeDismissAction).isNotNull() } @@ -303,4 +313,78 @@ class KeyguardDismissActionInteractorTest : SysuiTestCase() { underTest.setKeyguardDone(KeyguardDone.IMMEDIATE) assertThat(keyguardDoneTiming).isEqualTo(KeyguardDone.IMMEDIATE) } + + @Test + @EnableSceneContainer + fun dismissAction_executesBeforeItsReset_sceneContainerOn_swipeAuth_fromQsScene() = + testScope.runTest { + val canSwipeToEnter by collectLastValue(kosmos.deviceEntryInteractor.canSwipeToEnter) + val currentScene by collectLastValue(kosmos.sceneInteractor.currentScene) + val transitionState = + MutableStateFlow( + ObservableTransitionState.Idle(currentScene!!) + ) + kosmos.sceneInteractor.setTransitionState(transitionState) + val executeDismissAction by collectLastValue(underTest.executeDismissAction) + val resetDismissAction by collectLastValue(underTest.resetDismissAction) + assertThat(executeDismissAction).isNull() + assertThat(resetDismissAction).isNull() + kosmos.fakeAuthenticationRepository.setAuthenticationMethod( + AuthenticationMethodModel.None + ) + kosmos.fakeDeviceEntryRepository.setLockscreenEnabled(true) + assertThat(canSwipeToEnter).isTrue() + kosmos.sceneInteractor.changeScene(Scenes.QuickSettings, "") + transitionState.value = ObservableTransitionState.Idle(Scenes.QuickSettings) + assertThat(currentScene).isEqualTo(Scenes.QuickSettings) + + assertThat(executeDismissAction).isNull() + assertThat(resetDismissAction).isNull() + + val dismissAction = + DismissAction.RunImmediately( + onDismissAction = { KeyguardDone.LATER }, + onCancelAction = {}, + message = "message", + willAnimateOnLockscreen = true, + ) + underTest.setDismissAction(dismissAction) + // Should still be null because the transition to Gone has not yet happened. + assertThat(executeDismissAction).isNull() + assertThat(resetDismissAction).isNull() + + transitionState.value = + ObservableTransitionState.Transition.ChangeScene( + fromScene = Scenes.QuickSettings, + toScene = Scenes.Gone, + currentScene = flowOf(Scenes.QuickSettings), + currentOverlays = emptySet(), + progress = flowOf(0.5f), + isInitiatedByUserInput = true, + isUserInputOngoing = flowOf(false), + previewProgress = flowOf(0f), + isInPreviewStage = flowOf(false), + ) + runCurrent() + assertThat(executeDismissAction).isNull() + assertThat(resetDismissAction).isNull() + + transitionState.value = + ObservableTransitionState.Transition.ChangeScene( + fromScene = Scenes.QuickSettings, + toScene = Scenes.Gone, + currentScene = flowOf(Scenes.Gone), + currentOverlays = emptySet(), + progress = flowOf(1f), + isInitiatedByUserInput = true, + isUserInputOngoing = flowOf(false), + previewProgress = flowOf(0f), + isInPreviewStage = flowOf(false), + ) + kosmos.sceneInteractor.changeScene(Scenes.Gone, "") + assertThat(currentScene).isEqualTo(Scenes.Gone) + runCurrent() + assertThat(executeDismissAction).isNotNull() + assertThat(resetDismissAction).isNull() + } } diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractor.kt b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractor.kt index ea80911335fa..d323086eab58 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissActionInteractor.kt @@ -127,7 +127,20 @@ constructor( val executeDismissAction: Flow<() -> KeyguardDone> = merge( - finishedTransitionToGone, + if (SceneContainerFlag.isEnabled) { + // Using currentScene instead of finishedTransitionToGone because of a race + // condition that forms between finishedTransitionToGone and + // isOnShadeWhileUnlocked where the latter emits false before the former emits + // true, causing the merge to not emit until it's too late. + sceneInteractor + .get() + .currentScene + .map { it == Scenes.Gone } + .distinctUntilChanged() + .filter { it } + } else { + finishedTransitionToGone + }, isOnShadeWhileUnlocked.filter { it }.map {}, dismissInteractor.dismissKeyguardRequestWithImmediateDismissAction, ) @@ -137,10 +150,24 @@ constructor( val resetDismissAction: Flow = combine( - transitionInteractor.isFinishedIn( - scene = Scenes.Gone, - stateWithoutSceneContainer = GONE, - ), + if (SceneContainerFlag.isEnabled) { + // Using currentScene instead of isFinishedIn because of a race condition that + // forms between isFinishedIn(Gone) and isOnShadeWhileUnlocked where the latter + // emits false before the former emits true, causing the evaluation of the + // combine to come up with true, temporarily, before settling on false, which is + // a valid final state. That causes an incorrect reset of the dismiss action to + // occur before it gets executed. + sceneInteractor + .get() + .currentScene + .map { it == Scenes.Gone } + .distinctUntilChanged() + } else { + transitionInteractor.isFinishedIn( + scene = Scenes.Gone, + stateWithoutSceneContainer = GONE, + ) + }, transitionInteractor.isFinishedIn( scene = Scenes.Bouncer, stateWithoutSceneContainer = PRIMARY_BOUNCER, -- cgit v1.2.3-59-g8ed1b