Harden light reveal transition logic

The previous code assumed a continuous order of transitions emitted by
the transition framework. Meaning if X -> LOCKSCREEN the next transition
has to be LOCKSCREEN -> Y. While this assumption is still correct, we
have seen bugs in transition code that would sometimes skip one of
these transitions. For these cases we could end up in a state where the
animation would have been skipped and all subsequent states are
revealed, therefore `willTransitionChangeEndState(it)` would block all
further calls from revealing the scrim.

The new code does not rely on this assumption and will attempt to repeat
the reveal animation if we are not in the desired reveal state and the
animation is not yet running.

Test: NONE
Bug: b/324442545 b/281655028
Flag: ACONFIG com.android.systemui.Flags.FLAG_LIGHT_REVEAL_MIGRATION STAGING
Change-Id: Iab622df064ce2e0800efac01235aa78c12621ff1

4 files changed, 41 insertions, 76 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt
index f6c056698967..fb46ed9d54ed 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt
@@ -175,6 +175,21 @@ class LightRevealScrimRepositoryTest : SysuiTestCase() {
             animatorTestRule.advanceTimeBy(500L)
             assertEquals(1.0f, value)
         }
+
+    @Test
+    @TestableLooper.RunWithLooper(setAsMainLooper = true)
+    fun revealAmount_startingRevealTwiceWontRerunAnimator() =
+        runTest(UnconfinedTestDispatcher()) {
+            val value by collectLastValue(underTest.revealAmount)
+            underTest.startRevealAmountAnimator(true)
+            assertEquals(0.0f, value)
+            animatorTestRule.advanceTimeBy(250L)
+            assertEquals(0.5f, value)
+            underTest.startRevealAmountAnimator(true)
+            animatorTestRule.advanceTimeBy(250L)
+            assertEquals(1.0f, value)
+        }
+
     @Test
     @TestableLooper.RunWithLooper(setAsMainLooper = true)
     fun revealAmount_emitsTo0AfterAnimationStartedReversed() =
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt
index 9b302ae60e21..2b6e6c7c1575 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt
@@ -22,7 +22,6 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.keyguard.data.fakeLightRevealScrimRepository
 import com.android.systemui.keyguard.data.repository.FakeLightRevealScrimRepository
 import com.android.systemui.keyguard.data.repository.fakeKeyguardTransitionRepository
-import com.android.systemui.keyguard.shared.model.KeyguardState
 import com.android.systemui.keyguard.shared.model.TransitionState
 import com.android.systemui.keyguard.shared.model.TransitionStep
 import com.android.systemui.kosmos.testScope
@@ -33,16 +32,11 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
-import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito
-import org.mockito.Mockito.anyBoolean
-import org.mockito.Mockito.never
-import org.mockito.Mockito.reset
-import org.mockito.Mockito.verify
 
 @SmallTest
 @OptIn(ExperimentalCoroutinesApi::class)
@@ -103,41 +97,4 @@ class LightRevealScrimInteractorTest : SysuiTestCase() {
 
             job.cancel()
         }
-
-    @Test
-    fun lightRevealEffect_startsAnimationOnlyForDifferentStateTargets() =
-        testScope.runTest {
-            runCurrent()
-            reset(fakeLightRevealScrimRepository)
-
-            fakeKeyguardTransitionRepository.sendTransitionStep(
-                TransitionStep(
-                    transitionState = TransitionState.STARTED,
-                    from = KeyguardState.OFF,
-                    to = KeyguardState.OFF
-                )
-            )
-            runCurrent()
-            verify(fakeLightRevealScrimRepository, never()).startRevealAmountAnimator(anyBoolean())
-
-            fakeKeyguardTransitionRepository.sendTransitionStep(
-                TransitionStep(
-                    transitionState = TransitionState.STARTED,
-                    from = KeyguardState.DOZING,
-                    to = KeyguardState.LOCKSCREEN
-                )
-            )
-            runCurrent()
-            verify(fakeLightRevealScrimRepository).startRevealAmountAnimator(true)
-
-            fakeKeyguardTransitionRepository.sendTransitionStep(
-                TransitionStep(
-                    transitionState = TransitionState.STARTED,
-                    from = KeyguardState.LOCKSCREEN,
-                    to = KeyguardState.DOZING
-                )
-            )
-            runCurrent()
-            verify(fakeLightRevealScrimRepository).startRevealAmountAnimator(false)
-        }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt
index 42f14f1eb317..9b3f13d16911 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt
@@ -151,9 +151,13 @@ constructor(
         awaitClose { revealAmountAnimator.removeUpdateListener(updateListener) }
     }
 
+    private var willBeOrIsRevealed: Boolean? = null
+
     override fun startRevealAmountAnimator(reveal: Boolean) {
+        if (reveal == willBeOrIsRevealed) return
+        willBeOrIsRevealed = reveal
         if (reveal) revealAmountAnimator.start() else revealAmountAnimator.reverse()
-        scrimLogger.d(TAG, "startRevealAmountAnimator, reveal", reveal)
+        scrimLogger.d(TAG, "startRevealAmountAnimator, reveal: ", reveal)
     }
 
     override val revealEffect =
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt
index c7f262a2ac80..4d731eccd9bb 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt
@@ -21,7 +21,6 @@ import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.dagger.qualifiers.Application
 import com.android.systemui.keyguard.data.repository.LightRevealScrimRepository
 import com.android.systemui.keyguard.shared.model.KeyguardState
-import com.android.systemui.keyguard.shared.model.TransitionStep
 import com.android.systemui.power.domain.interactor.PowerInteractor
 import com.android.systemui.power.shared.model.ScreenPowerState
 import com.android.systemui.statusbar.LightRevealEffect
@@ -53,11 +52,9 @@ constructor(
         scope.launch {
             transitionInteractor.startedKeyguardTransitionStep.collect {
                 scrimLogger.d(TAG, "listenForStartedKeyguardTransitionStep", it)
-                if (willTransitionChangeEndState(it)) {
-                    lightRevealScrimRepository.startRevealAmountAnimator(
-                        willBeRevealedInState(it.to)
-                    )
-                }
+                lightRevealScrimRepository.startRevealAmountAnimator(
+                    willBeRevealedInState(it.to),
+                )
             }
         }
     }
@@ -92,33 +89,25 @@ constructor(
 
     companion object {
 
-        /**
-         * Whether the transition requires a change in the reveal amount of the light reveal scrim.
-         * If not, we don't care about the transition and don't need to listen to it.
-         */
-        fun willTransitionChangeEndState(transition: TransitionStep): Boolean {
-            return willBeRevealedInState(transition.from) != willBeRevealedInState(transition.to)
-        }
-
-        /**
-         * Whether the light reveal scrim will be fully revealed (revealAmount = 1.0f) in the given
-         * state after the transition is complete. If false, scrim will be fully hidden.
-         */
-        fun willBeRevealedInState(state: KeyguardState): Boolean {
-            return when (state) {
-                KeyguardState.OFF -> false
-                KeyguardState.DOZING -> false
-                KeyguardState.AOD -> false
-                KeyguardState.DREAMING -> true
-                KeyguardState.DREAMING_LOCKSCREEN_HOSTED -> true
-                KeyguardState.GLANCEABLE_HUB -> true
-                KeyguardState.ALTERNATE_BOUNCER -> true
-                KeyguardState.PRIMARY_BOUNCER -> true
-                KeyguardState.LOCKSCREEN -> true
-                KeyguardState.GONE -> true
-                KeyguardState.OCCLUDED -> true
-            }
+    /**
+     * Whether the light reveal scrim will be fully revealed (revealAmount = 1.0f) in the given
+     * state after the transition is complete. If false, scrim will be fully hidden.
+     */
+    private fun willBeRevealedInState(state: KeyguardState): Boolean {
+        return when (state) {
+            KeyguardState.OFF -> false
+            KeyguardState.DOZING -> false
+            KeyguardState.AOD -> false
+            KeyguardState.DREAMING -> true
+            KeyguardState.DREAMING_LOCKSCREEN_HOSTED -> true
+            KeyguardState.GLANCEABLE_HUB -> true
+            KeyguardState.ALTERNATE_BOUNCER -> true
+            KeyguardState.PRIMARY_BOUNCER -> true
+            KeyguardState.LOCKSCREEN -> true
+            KeyguardState.GONE -> true
+            KeyguardState.OCCLUDED -> true
         }
+    }
 
         val TAG = LightRevealScrimInteractor::class.simpleName!!
     }