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

Commit 84ac3904 authored by Alejandro Nijamkin's avatar Alejandro Nijamkin
Browse files

[flexiglass] Fix for "return to lockscreen" bug. 

This is a highly complex bug that started occurring following the
integration of the KeguardTransition framework (KTF) with Flexiglass
(AKA "STL").

There are multiple root causes that have conspired together to create
this highly disruptive bug, this CL addresses them all:

1. lockscreenVisibility in WindowManagerLockscreenVisibilityInteractor
   was improperly defined to only depend on transitions to and from the
   Gone scene. This CL changes that to depend on whether the keyguard
   "views" should be visible, which is the intent of the original work
   (despite a bug in it, see b/341400177) - this uses isDeviceEntered
   from DeviceEntryInteractor which is the blessed single source of
   truth for knowing whether the lockscreen has been (by)passed.
2. show in StatusBarKeyguardViewManager was directly causing a scene
   change to the Lockscreen scene. This was added in ag/26445610 to
   make sure that, when a foldable is folded closed, we change to the
   Lockscreen scene. Since the show method can be called from more than
   one place and doesn't always necessarily mean that the actual
   Lockscreen UI should be shown, this CL moves that changeScene to
   happen upstream, in showDismissibleKeyguard of KeyguardService -
   which is a callback invoked by system server when the device is
   folded to a closed position after being open.
3. There's a bug where the system server signal for showing the
   dismissible lockscreen isn't being respected when the
   keyguard_wm_refactor flag is on, this is explained thoroughly in
   b/341400177. While this isn't really a root cause for the bug that
   this CL is fixing, it complicated the debugging effort so it's worth
   mentioning here.

Fix: 340536968
Test: existing unit tests pass
Test: manually verified that scene transition work as expected. Started
on LS, swiped down to reveal shade, swiped down to reveal QS, swiped up
twice to go back to LS, swipe up again to open the bouncer, enter the
correct credentials to unlock, ended up on the gone scene, swipe down
once and again to the shade and QS scenes, swipe up twice to return to
gone.
Test: manually verified that folding the unfolded device completely
closed correctly moves to the lockscreen scene. Discovered and logged b/341445974
Flag: com.android.systemui.scene_container

Change-Id: I011b5cd43979e75b3e805bc363ca1a86d64ea5de
parent 45855bdd

packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java

+15 −3
Original line number Diff line number Diff line
@@ -84,20 +84,25 @@ import com.android.systemui.keyguard.ui.viewmodel.KeyguardSurfaceBehindViewModel 
import com.android.systemui.keyguard.ui.viewmodel.WindowManagerLockscreenVisibilityViewModel;

import com.android.systemui.power.domain.interactor.PowerInteractor;

import com.android.systemui.power.shared.model.ScreenPowerState;

import com.android.systemui.scene.domain.interactor.SceneInteractor;

import com.android.systemui.scene.shared.flag.SceneContainerFlag;

import com.android.systemui.scene.shared.model.Scenes;

import com.android.systemui.settings.DisplayTracker;

import com.android.wm.shell.shared.CounterRotator;

import com.android.wm.shell.shared.ShellTransitions;

import com.android.wm.shell.shared.TransitionUtil;

import com.android.wm.shell.transition.Transitions;



import dagger.Lazy;



import kotlinx.coroutines.CoroutineScope;



import java.util.ArrayList;

import java.util.Map;

import java.util.WeakHashMap;



import javax.inject.Inject;



import kotlinx.coroutines.CoroutineScope;



public class KeyguardService extends Service {

    static final String TAG = "KeyguardService";

    static final String PERMISSION = android.Manifest.permission.CONTROL_KEYGUARD;
@@ -109,6 +114,7 @@ public class KeyguardService extends Service { 
    private final ShellTransitions mShellTransitions;

    private final DisplayTracker mDisplayTracker;

    private final PowerInteractor mPowerInteractor;

    private final Lazy<SceneInteractor> mSceneInteractorLazy;



    private static RemoteAnimationTarget[] wrap(TransitionInfo info, boolean wallpapers,

            SurfaceControl.Transaction t, ArrayMap<SurfaceControl, SurfaceControl> leashMap,
@@ -316,7 +322,8 @@ public class KeyguardService extends Service { 
            @Application CoroutineScope scope,

            FeatureFlags featureFlags,

            PowerInteractor powerInteractor,

            WindowManagerOcclusionManager windowManagerOcclusionManager) {

            WindowManagerOcclusionManager windowManagerOcclusionManager,

            Lazy<SceneInteractor> sceneInteractorLazy) {

        super();

        mKeyguardViewMediator = keyguardViewMediator;

        mKeyguardLifecyclesDispatcher = keyguardLifecyclesDispatcher;
@@ -325,6 +332,7 @@ public class KeyguardService extends Service { 
        mDisplayTracker = displayTracker;

        mFlags = featureFlags;

        mPowerInteractor = powerInteractor;

        mSceneInteractorLazy = sceneInteractorLazy;



        if (KeyguardWmStateRefactor.isEnabled()) {

            WindowManagerLockscreenVisibilityViewBinder.bind(
@@ -601,6 +609,10 @@ public class KeyguardService extends Service { 
            trace("showDismissibleKeyguard");

            checkPermission();

            mKeyguardViewMediator.showDismissibleKeyguard();

            if (SceneContainerFlag.isEnabled()) {

                mSceneInteractorLazy.get().changeScene(

                        Scenes.Lockscreen, "KeyguardService.showDismissibleKeyguard");

            }

        }



        @Override // Binder interface

packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/WindowManagerLockscreenVisibilityInteractor.kt

+8 −18
Original line number Diff line number Diff line
@@ -14,20 +14,21 @@ 
 * limitations under the License.

 */



@file:OptIn(ExperimentalCoroutinesApi::class)



package com.android.systemui.keyguard.domain.interactor



import com.android.compose.animation.scene.ObservableTransitionState

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.deviceentry.domain.interactor.DeviceEntryInteractor

import com.android.systemui.keyguard.shared.model.BiometricUnlockMode

import com.android.systemui.keyguard.shared.model.Edge

import com.android.systemui.keyguard.shared.model.KeyguardState

import com.android.systemui.keyguard.shared.model.TransitionState

import com.android.systemui.scene.domain.interactor.SceneInteractor

import com.android.systemui.scene.shared.flag.SceneContainerFlag

import com.android.systemui.scene.shared.model.Scenes

import com.android.systemui.statusbar.notification.domain.interactor.NotificationLaunchAnimationInteractor

import com.android.systemui.util.kotlin.pairwise

import com.android.systemui.util.kotlin.sample

import dagger.Lazy

import javax.inject.Inject

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.flow.Flow
@@ -48,7 +49,8 @@ constructor( 
    fromBouncerInteractor: FromPrimaryBouncerTransitionInteractor,

    fromAlternateBouncerInteractor: FromAlternateBouncerTransitionInteractor,

    notificationLaunchAnimationInteractor: NotificationLaunchAnimationInteractor,

    sceneInteractor: SceneInteractor,

    sceneInteractor: Lazy<SceneInteractor>,

    deviceEntryInteractor: Lazy<DeviceEntryInteractor>,

) {

    private val defaultSurfaceBehindVisibility =

        transitionInteractor.finishedKeyguardState.map(::isSurfaceVisible)
@@ -112,7 +114,7 @@ constructor( 
    val usingKeyguardGoingAwayAnimation: Flow<Boolean> =

        if (SceneContainerFlag.isEnabled) {

            combine(

                    sceneInteractor.transitionState,

                    sceneInteractor.get().transitionState,

                    surfaceBehindInteractor.isAnimatingSurface,

                    notificationLaunchAnimationInteractor.isLaunchAnimationRunning,

                ) { transition, isAnimatingSurface, isLaunchAnimationRunning ->
@@ -156,19 +158,7 @@ constructor( 
     */

    val lockscreenVisibility: Flow<Boolean> =

        if (SceneContainerFlag.isEnabled) {

            sceneInteractor.transitionState

                .pairwise(ObservableTransitionState.Idle(Scenes.Lockscreen))

                .map { (prevTransitionState, transitionState) ->

                    val isReturningToGoneAfterCancellation =

                        prevTransitionState.isTransitioning(from = Scenes.Gone) &&

                            transitionState.isTransitioning(to = Scenes.Gone)

                    val isNotOnGone =

                        !transitionState.isTransitioning(from = Scenes.Gone) &&

                            !transitionState.isIdle(Scenes.Gone)



                    isNotOnGone && !isReturningToGoneAfterCancellation

                }

                .distinctUntilChanged()

            deviceEntryInteractor.get().isDeviceEntered.map { !it }

        } else {

            transitionInteractor.currentKeyguardState

                .sample(transitionInteractor.startedStepWithPrecedingStep, ::Pair)

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarKeyguardViewManager.java

+0 −5
Original line number Diff line number Diff line
@@ -695,11 +695,6 @@ public class StatusBarKeyguardViewManager implements RemoteInputController.Callb 
    public void show(Bundle options) {

        Trace.beginSection("StatusBarKeyguardViewManager#show");

        mNotificationShadeWindowController.setKeyguardShowing(true);

        if (SceneContainerFlag.isEnabled()) {

            // TODO(b/336581871): add sceneState?

            mSceneInteractorLazy.get().changeScene(

                    Scenes.Lockscreen, "StatusBarKeyguardViewManager.show");

        }

        mKeyguardStateController.notifyKeyguardState(true, mKeyguardStateController.isOccluded());

        reset(true /* hideBouncerWhenShowing */);

        SysUiStatsLog.write(SysUiStatsLog.KEYGUARD_STATE_CHANGED,

packages/SystemUI/tests/src/com/android/systemui/keyguard/domain/interactor/WindowManagerLockscreenVisibilityInteractorTest.kt

+32 −21
Original line number Diff line number Diff line
@@ -20,8 +20,11 @@ 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.domain.interactor.authenticationInteractor

import com.android.systemui.coroutines.collectLastValue

import com.android.systemui.coroutines.collectValues

import com.android.systemui.deviceentry.domain.interactor.deviceUnlockedInteractor

import com.android.systemui.flags.DisableSceneContainer

import com.android.systemui.flags.EnableSceneContainer

import com.android.systemui.keyguard.data.repository.fakeKeyguardTransitionRepository
@@ -30,6 +33,7 @@ import com.android.systemui.keyguard.shared.model.TransitionState 
import com.android.systemui.keyguard.shared.model.TransitionStep

import com.android.systemui.kosmos.testScope

import com.android.systemui.scene.data.repository.sceneContainerRepository

import com.android.systemui.scene.domain.interactor.sceneInteractor

import com.android.systemui.scene.shared.model.Scenes

import com.android.systemui.testKosmos

import com.android.systemui.util.mockito.mock
@@ -38,6 +42,7 @@ import com.google.common.truth.Truth.assertThat 
import junit.framework.Assert.assertEquals

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.flowOf

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runTest

import org.junit.Before
@@ -727,42 +732,48 @@ class WindowManagerLockscreenVisibilityInteractorTest : SysuiTestCase() { 


    @Test

    @EnableSceneContainer

    fun sceneContainer_lockscreenVisibility_visibleWhenNotGone() =

    fun lockscreenVisibility() =

        testScope.runTest {

            val lockscreenVisibility by collectLastValue(underTest.value.lockscreenVisibility)

            val isDeviceUnlocked by

                collectLastValue(

                    kosmos.deviceUnlockedInteractor.deviceUnlockStatus.map { it.isUnlocked }

                )

            assertThat(isDeviceUnlocked).isFalse()



            sceneTransitions.value = lsToGone

            assertThat(lockscreenVisibility).isTrue()

            val currentScene by collectLastValue(kosmos.sceneInteractor.currentScene)

            assertThat(currentScene).isEqualTo(Scenes.Lockscreen)



            sceneTransitions.value = ObservableTransitionState.Idle(Scenes.Gone)

            assertThat(lockscreenVisibility).isFalse()



            sceneTransitions.value = goneToLs

            assertThat(lockscreenVisibility).isFalse()

            val lockscreenVisibility by collectLastValue(underTest.value.lockscreenVisibility)

            assertThat(lockscreenVisibility).isTrue()



            sceneTransitions.value = ObservableTransitionState.Idle(Scenes.Lockscreen)

            kosmos.sceneInteractor.changeScene(Scenes.Bouncer, "")

            assertThat(currentScene).isEqualTo(Scenes.Bouncer)

            assertThat(lockscreenVisibility).isTrue()

        }



    @Test

    @EnableSceneContainer

    fun sceneContainer_lockscreenVisibility_notVisibleWhenReturningToGone() =

        testScope.runTest {

            val lockscreenVisibility by collectLastValue(underTest.value.lockscreenVisibility)

            kosmos.authenticationInteractor.authenticate(FakeAuthenticationRepository.DEFAULT_PIN)

            assertThat(isDeviceUnlocked).isTrue()

            kosmos.sceneInteractor.changeScene(Scenes.Gone, "")

            assertThat(currentScene).isEqualTo(Scenes.Gone)

            assertThat(lockscreenVisibility).isFalse()



            sceneTransitions.value = goneToLs

            kosmos.sceneInteractor.changeScene(Scenes.Shade, "")

            assertThat(currentScene).isEqualTo(Scenes.Shade)

            assertThat(lockscreenVisibility).isFalse()



            sceneTransitions.value = lsToGone

            kosmos.sceneInteractor.changeScene(Scenes.QuickSettings, "")

            assertThat(currentScene).isEqualTo(Scenes.QuickSettings)

            assertThat(lockscreenVisibility).isFalse()



            sceneTransitions.value = ObservableTransitionState.Idle(Scenes.Gone)

            kosmos.sceneInteractor.changeScene(Scenes.Shade, "")

            assertThat(currentScene).isEqualTo(Scenes.Shade)

            assertThat(lockscreenVisibility).isFalse()



            sceneTransitions.value = goneToLs

            kosmos.sceneInteractor.changeScene(Scenes.Gone, "")

            assertThat(currentScene).isEqualTo(Scenes.Gone)

            assertThat(lockscreenVisibility).isFalse()



            sceneTransitions.value = ObservableTransitionState.Idle(Scenes.Lockscreen)

            kosmos.sceneInteractor.changeScene(Scenes.Lockscreen, "")

            assertThat(currentScene).isEqualTo(Scenes.Lockscreen)

            assertThat(lockscreenVisibility).isTrue()

        }

packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/WindowManagerLockscreenVisibilityInteractorKosmos.kt

+3 −1
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.keyguard.domain.interactor



import com.android.systemui.deviceentry.domain.interactor.deviceEntryInteractor

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.scene.domain.interactor.sceneInteractor

import com.android.systemui.statusbar.notification.domain.interactor.notificationLaunchAnimationInteractor
@@ -30,6 +31,7 @@ val Kosmos.windowManagerLockscreenVisibilityInteractor by 
            fromBouncerInteractor = fromPrimaryBouncerTransitionInteractor,

            fromAlternateBouncerInteractor = fromAlternateBouncerTransitionInteractor,

            notificationLaunchAnimationInteractor = notificationLaunchAnimationInteractor,

            sceneInteractor = sceneInteractor,

            sceneInteractor = { sceneInteractor },

            deviceEntryInteractor = { deviceEntryInteractor },

        )

    }