Fix persistent dot not removed due to race condition in scheduler

Bug: 300741186
Test: atest SystemStatusAnimationSchedulerImplTest
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:d027036adf637b5db0f119afef5218b6a8ce37af)
Merged-In: I7380cb956b48c38bdd540b297287786cf35b94f2
Change-Id: I7380cb956b48c38bdd540b297287786cf35b94f2

2 files changed, 52 insertions, 24 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt
index 68321f433c81..28b00b738a66 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt
@@ -182,19 +182,19 @@ constructor(
         // Set hasPersistentDot to false. If the animationState is anything before ANIMATING_OUT,
         // the disappear animation will not animate into a dot but remove the chip entirely
         hasPersistentDot = false
-        // if we are currently showing a persistent dot, hide it
-        if (animationState.value == SHOWING_PERSISTENT_DOT) notifyHidePersistentDot()
-        // if we are currently animating into a dot, wait for the animation to finish and then hide
-        // the dot
-        if (animationState.value == ANIMATING_OUT) {
-            coroutineScope.launch {
-                withTimeout(DISAPPEAR_ANIMATION_DURATION) {
-                    animationState.first {
-                        it == SHOWING_PERSISTENT_DOT || it == IDLE || it == ANIMATION_QUEUED
-                    }
-                    notifyHidePersistentDot()
-                }
+
+        if (animationState.value == SHOWING_PERSISTENT_DOT) {
+            // if we are currently showing a persistent dot, hide it and update the animationState
+            notifyHidePersistentDot()
+            if (scheduledEvent.value != null) {
+                animationState.value = ANIMATION_QUEUED
+            } else {
+                animationState.value = IDLE
             }
+        } else if (animationState.value == ANIMATING_OUT) {
+            // if we are currently animating out, hide the dot. The animationState will be updated
+            // once the animation has ended in the onAnimationEnd callback
+            notifyHidePersistentDot()
         }
     }
 
@@ -376,14 +376,6 @@ constructor(
         Assert.isMainThread()
         val anims: List<Animator> = listeners.mapNotNull { it.onHidePersistentDot() }
 
-        if (animationState.value == SHOWING_PERSISTENT_DOT) {
-            if (scheduledEvent.value != null) {
-                animationState.value = ANIMATION_QUEUED
-            } else {
-                animationState.value = IDLE
-            }
-        }
-
         if (anims.isNotEmpty()) {
             val aSet = AnimatorSet()
             aSet.playTogether(anims)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt
index 8a6dfe518cbf..1079a7df8842 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt
@@ -410,15 +410,16 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() {
 
         // remove persistent dot
         systemStatusAnimationScheduler.removePersistentDot()
-        testScheduler.runCurrent()
+
+        // verify that the onHidePersistentDot callback is invoked
+        verify(listener, times(1)).onHidePersistentDot()
 
         // skip disappear animation
         animatorTestRule.advanceTimeBy(DISAPPEAR_ANIMATION_DURATION)
         testScheduler.runCurrent()
 
-        // verify that animationState changes to IDLE and onHidePersistentDot callback is invoked
+        // verify that animationState changes to IDLE
         assertEquals(IDLE, systemStatusAnimationScheduler.getAnimationState())
-        verify(listener, times(1)).onHidePersistentDot()
     }
 
     @Test
@@ -483,7 +484,6 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() {
 
         // request removal of persistent dot
         systemStatusAnimationScheduler.removePersistentDot()
-        testScheduler.runCurrent()
 
         // schedule another high priority event while the event is animating out
         createAndScheduleFakePrivacyEvent()
@@ -499,6 +499,42 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() {
         verify(listener, times(1)).onHidePersistentDot()
     }
 
+    @Test
+    fun testDotIsRemoved_evenIfAnimatorCallbackIsDelayed() = runTest {
+        // Instantiate class under test with TestScope from runTest
+        initializeSystemStatusAnimationScheduler(testScope = this)
+
+        // create and schedule high priority event
+        createAndScheduleFakePrivacyEvent()
+
+        // skip chip animation lifecycle and fast forward to ANIMATING_OUT state
+        fastForwardAnimationToState(ANIMATING_OUT)
+        assertEquals(ANIMATING_OUT, systemStatusAnimationScheduler.getAnimationState())
+        verify(listener, times(1)).onSystemStatusAnimationTransitionToPersistentDot(any())
+
+        // request removal of persistent dot
+        systemStatusAnimationScheduler.removePersistentDot()
+
+        // verify that the state is still ANIMATING_OUT
+        assertEquals(ANIMATING_OUT, systemStatusAnimationScheduler.getAnimationState())
+
+        // skip disappear animation duration
+        testScheduler.advanceTimeBy(DISAPPEAR_ANIMATION_DURATION + 1)
+        // In an old implementation this would trigger a coroutine timeout causing the
+        // onHidePersistentDot callback to be missed.
+        testScheduler.runCurrent()
+
+        // advance animator time to invoke onAnimationEnd callback
+        animatorTestRule.advanceTimeBy(DISAPPEAR_ANIMATION_DURATION)
+        testScheduler.runCurrent()
+
+        // verify that onHidePersistentDot is invoked despite the animator callback being delayed
+        // (it's invoked more than DISAPPEAR_ANIMATION_DURATION after the dot removal was requested)
+        verify(listener, times(1)).onHidePersistentDot()
+        // verify that animationState is IDLE
+        assertEquals(IDLE, systemStatusAnimationScheduler.getAnimationState())
+    }
+
     private fun TestScope.fastForwardAnimationToState(@SystemAnimationState animationState: Int) {
         // this function should only be called directly after posting a status event
         assertEquals(ANIMATION_QUEUED, systemStatusAnimationScheduler.getAnimationState())