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

4 files changed, 81 insertions, 50 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/ui/binder/WindowManagerLockscreenVisibilityManagerTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/ui/binder/WindowManagerLockscreenVisibilityManagerTest.kt
index 605a5d261424..bafabe07d370 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/ui/binder/WindowManagerLockscreenVisibilityManagerTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/ui/binder/WindowManagerLockscreenVisibilityManagerTest.kt
@@ -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,
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/WindowManagerLockscreenVisibilityManager.kt b/packages/SystemUI/src/com/android/systemui/keyguard/WindowManagerLockscreenVisibilityManager.kt
index a74384f61469..df41535ff783 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/WindowManagerLockscreenVisibilityManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/WindowManagerLockscreenVisibilityManager.kt
@@ -174,25 +174,24 @@ 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.
-                Log.d(
-                    TAG,
-                    "onKeyguardGoingAwayRemoteAnimationStart: " +
-                        "Dismiss transition was not started; we're already GONE. " +
-                        "Ending remote animation.",
-                )
-                finishedCallback.onAnimationFinished()
-                return
-            }
+            keyguardDismissTransitionInteractor.startDismissKeyguardTransition(
+                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: " +
+                            "Dismiss transition was not started; we're already GONE. " +
+                            "Ending remote animation.",
+                    )
+                    finishedCallback.onAnimationFinished()
+                    isKeyguardGoingAway = false
+                },
+            )
 
             isKeyguardGoingAway = true
         }
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt
index 089e5dc42df3..c0a486c005ab 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt
@@ -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,45 +51,63 @@ 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)")
-        val startedState =
-            if (transitionRaceCondition()) {
-                repository.currentTransitionInfo.to
+
+        scope.launch {
+            val startedState =
+                if (transitionRaceCondition()) {
+                    repository.currentTransitionInfo.to
+                } else {
+                    repository.currentTransitionInfoInternal.value.to
+                }
+
+            val animator: ValueAnimator? =
+                when (startedState) {
+                    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 {
-                repository.currentTransitionInfoInternal.value.to
-            }
-        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 -> {
                 Log.i(
                     TAG,
-                    "Already transitioning to GONE; ignoring startDismissKeyguardTransition.",
+                    "Can't transition to GONE from $startedState; " +
+                        "ignoring startDismissKeyguardTransition.",
                 )
-                return false
-            }
-            else -> {
-                Log.e(TAG, "We don't know how to dismiss keyguard from state $startedState.")
-                return false
+                onAlreadyGone?.invoke()
             }
         }
-
-        return true
     }
 
     companion object {
diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/DismissKeyguardInteractor.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt
index 82a5311269f0..56a7b4db5455 100644
--- a/packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/DismissKeyguardInteractor.kt
+++ b/packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/domain/interactor/KeyguardDismissTransitionInteractor.kt
@@ -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,