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

Commit 886b887a authored by Josh Tsuji's avatar Josh Tsuji
Browse files

Directly startTransition in KeyguardDismissTransitionInteractor. 

Logs from some flaky presubmit runs showed that in some race conditions, the keyguard dismiss interactor would ask a specific From*TransitionInteractor to start a transition, but by the time it ran the coroutine that calls startTransitionTo, a transition to another state has started. The dismiss transition is then ignored, because the from state is invalid.

With the new approach, a coroutine is launched which then starts a transition from whatever state we're now currently in to GONE.

We also accept a callback to call if, by the time the coroutine runs, we're already GONE. This allows us to end the animation instead of waiting for it to time out. This should be safe, since worst case, if this callback is called erroneously due to another race condition, the remote animation will just short-circuit and end.

Bug: 278086361
Flag: com.android.systemui.keyguard_wm_state_refactor
Test: atest KeyguardTests
Test: abtd de-flake, manually
Change-Id: I286496295c9364c2a5c42e0f0efc386c0eee3383
parent 7beb61d1

packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/ui/binder/WindowManagerLockscreenVisibilityManagerTest.kt

+8 −4
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import android.platform.test.annotations.RequiresFlagsEnabled 
import android.platform.test.flag.junit.CheckFlagsRule

import android.platform.test.flag.junit.DeviceFlagsValueProvider

import android.view.IRemoteAnimationFinishedCallback

import android.view.RemoteAnimationTarget

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase
@@ -39,11 +40,11 @@ import org.junit.runner.RunWith 
import org.mockito.ArgumentMatchers.eq

import org.mockito.Mock

import org.mockito.Mockito.anyInt

import org.mockito.Mockito.mock

import org.mockito.Mockito.verify

import org.mockito.Mockito.verifyNoMoreInteractions

import org.mockito.MockitoAnnotations

import org.mockito.kotlin.any

import org.mockito.kotlin.doAnswer

import org.mockito.kotlin.mock

import org.mockito.kotlin.whenever
@@ -230,12 +231,15 @@ class WindowManagerLockscreenVisibilityManagerTest : SysuiTestCase() { 
    @Test

    fun remoteAnimationInstantlyFinished_ifDismissTransitionNotStarted() {

        val mockedCallback = mock<IRemoteAnimationFinishedCallback>()

        whenever(keyguardDismissTransitionInteractor.startDismissKeyguardTransition(any()))

            .thenReturn(false)



        // Call the onAlreadyGone callback immediately.

        doAnswer { invocation -> (invocation.getArgument(1) as (() -> Unit)).invoke() }

            .whenever(keyguardDismissTransitionInteractor)

            .startDismissKeyguardTransition(any(), any())



        underTest.onKeyguardGoingAwayRemoteAnimationStart(

            transit = 0,

            apps = emptyArray(),

            apps = arrayOf(mock<RemoteAnimationTarget>()),

            wallpapers = emptyArray(),

            nonApps = emptyArray(),

            finishedCallback = mockedCallback,

packages/SystemUI/src/com/android/systemui/keyguard/WindowManagerLockscreenVisibilityManager.kt

+18 −19
Original line number Diff line number Diff line
@@ -174,16 +174,14 @@ constructor( 
        if (!isKeyguardGoingAway) {

            // Since WM triggered this, we're likely not transitioning to GONE yet. See if we can

            // start that transition.

            val startedDismiss =

            keyguardDismissTransitionInteractor.startDismissKeyguardTransition(

                    reason = "Going away remote animation started"

                )



            if (!startedDismiss) {

                // If the transition wasn't started, we're already GONE. This can happen with timing

                // issues, where the remote animation took a long time to start, and something else

                // caused us to unlock in the meantime. Since we're already GONE, simply end the

                // remote animatiom immediately.

                reason = "Going away remote animation started",

                onAlreadyGone = {

                    // Called if we're already GONE by the time the dismiss transition would have

                    // started. This can happen due to timing issues, where the remote animation

                    // took a long time to start, and something else caused us to unlock in the

                    // meantime. Since we're already GONE, simply end the remote animation

                    // immediately.

                    Log.d(

                        TAG,

                        "onKeyguardGoingAwayRemoteAnimationStart: " +
@@ -191,8 +189,9 @@ constructor( 
                            "Ending remote animation.",

                    )

                    finishedCallback.onAnimationFinished()

                return

            }

                    isKeyguardGoingAway = false

                },

            )



            isKeyguardGoingAway = true

        }

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

+53 −27
Original line number Diff line number Diff line
@@ -16,23 +16,31 @@ 


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



import android.animation.ValueAnimator

import android.util.Log

import com.android.systemui.Flags.transitionRaceCondition

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.keyguard.data.repository.KeyguardTransitionRepository

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

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

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

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

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

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

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

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

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

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

import javax.inject.Inject

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.launch



@SysUISingleton

class KeyguardDismissTransitionInteractor

@Inject

constructor(

    @Background private val scope: CoroutineScope,

    private val repository: KeyguardTransitionRepository,

    private val fromLockscreenTransitionInteractor: FromLockscreenTransitionInteractor,

    private val fromPrimaryBouncerTransitionInteractor: FromPrimaryBouncerTransitionInteractor,
@@ -43,47 +51,65 @@ constructor( 
) {



    /**

     * Called to start a transition that will ultimately dismiss the keyguard from the current

     * state.

     * Launches a coroutine to start a transition that will ultimately dismiss the keyguard from the

     * current state.

     *

     * This is called exclusively by sources that can authoritatively say we should be unlocked,

     * including KeyguardSecurityContainerController and WindowManager.

     *

     * Returns [false] if the transition was not started, because we're already GONE or we don't

     * know how to dismiss keyguard from the current state.

     * This is one of the few transitions that is started outside of the From*TransitionInteractor

     * classes. This is because this is an external call that must be respected, so it doesn't

     * matter what state we're in/coming from - we must transition from that state to GONE.

     *

     * Invokes [onAlreadyGone] if the transition was not started because we're already GONE by the

     * time the coroutine runs.

     */

    fun startDismissKeyguardTransition(reason: String = ""): Boolean {

        if (SceneContainerFlag.isEnabled) return false

    @JvmOverloads

    fun startDismissKeyguardTransition(reason: String = "", onAlreadyGone: (() -> Unit)? = null) {

        if (SceneContainerFlag.isEnabled) return

        Log.d(TAG, "#startDismissKeyguardTransition(reason=$reason)")



        scope.launch {

            val startedState =

                if (transitionRaceCondition()) {

                    repository.currentTransitionInfo.to

                } else {

                    repository.currentTransitionInfoInternal.value.to

                }



            val animator: ValueAnimator? =

                when (startedState) {

            LOCKSCREEN -> fromLockscreenTransitionInteractor.dismissKeyguard()

            PRIMARY_BOUNCER -> fromPrimaryBouncerTransitionInteractor.dismissPrimaryBouncer()

            ALTERNATE_BOUNCER -> fromAlternateBouncerTransitionInteractor.dismissAlternateBouncer()

            AOD -> fromAodTransitionInteractor.dismissAod()

            DOZING -> fromDozingTransitionInteractor.dismissFromDozing()

            KeyguardState.OCCLUDED -> fromOccludedTransitionInteractor.dismissFromOccluded()

            KeyguardState.GONE -> {

                    LOCKSCREEN -> fromLockscreenTransitionInteractor

                    PRIMARY_BOUNCER -> fromPrimaryBouncerTransitionInteractor

                    ALTERNATE_BOUNCER -> fromAlternateBouncerTransitionInteractor

                    AOD -> fromAodTransitionInteractor

                    DOZING -> fromDozingTransitionInteractor

                    OCCLUDED -> fromOccludedTransitionInteractor

                    else -> null

                }?.getDefaultAnimatorForTransitionsToState(KeyguardState.GONE)



            if (startedState != KeyguardState.GONE && animator != null) {

                repository.startTransition(

                    TransitionInfo(

                        "KeyguardDismissTransitionInteractor" +

                            if (reason.isNotBlank()) "($reason)" else "",

                        startedState,

                        KeyguardState.GONE,

                        animator,

                        TransitionModeOnCanceled.LAST_VALUE,

                    )

                )

            } else {

                Log.i(

                    TAG,

                    "Already transitioning to GONE; ignoring startDismissKeyguardTransition.",

                    "Can't transition to GONE from $startedState; " +

                        "ignoring startDismissKeyguardTransition.",

                )

                return false

                onAlreadyGone?.invoke()

            }

            else -> {

                Log.e(TAG, "We don't know how to dismiss keyguard from state $startedState.")

                return false

        }

    }



        return true

    }



    companion object {

        private val TAG = KeyguardDismissTransitionInteractor::class.simpleName

    }

packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/DismissKeyguardInteractor.kt→packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt

+2 −0
Original line number Diff line number Diff line
@@ -18,10 +18,12 @@ package com.android.systemui.keyguard.domain.interactor 


import com.android.systemui.keyguard.data.repository.keyguardTransitionRepository

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.testScope



val Kosmos.keyguardDismissTransitionInteractor: KeyguardDismissTransitionInteractor by

    Kosmos.Fixture {

        KeyguardDismissTransitionInteractor(

            scope = testScope,

            repository = keyguardTransitionRepository,

            fromLockscreenTransitionInteractor = fromLockscreenTransitionInteractor,

            fromPrimaryBouncerTransitionInteractor = fromPrimaryBouncerTransitionInteractor,