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

Commit c86dbabd authored by Johannes Gallmann's avatar Johannes Gallmann
Browse files

Fix persistent dot not removed due to race condition in scheduler 

Bug: 290784635
Test: atest SystemStatusAnimationSchedulerImplTest
Change-Id: I7380cb956b48c38bdd540b297287786cf35b94f2
parent 985b432d

packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt

+12 −20
Original line number Original line Diff line number Diff line
@@ -170,19 +170,19 @@ constructor( 
        // Set hasPersistentDot to false. If the animationState is anything before ANIMATING_OUT,

        // 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

        // the disappear animation will not animate into a dot but remove the chip entirely

        hasPersistentDot = false

        hasPersistentDot = false

        // if we are currently showing a persistent dot, hide it



        if (animationState.value == SHOWING_PERSISTENT_DOT) notifyHidePersistentDot()

        if (animationState.value == SHOWING_PERSISTENT_DOT) {

        // if we are currently animating into a dot, wait for the animation to finish and then hide

            // if we are currently showing a persistent dot, hide it and update the animationState

        // 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()

            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()

        }

        }

    }

    }
@@ -366,14 +366,6 @@ constructor( 
        logger?.logHidePersistentDotCallbackInvoked()

        logger?.logHidePersistentDotCallbackInvoked()

        val anims: List<Animator> = listeners.mapNotNull { it.onHidePersistentDot() }

        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()) {

        if (anims.isNotEmpty()) {

            val aSet = AnimatorSet()

            val aSet = AnimatorSet()

            aSet.playTogether(anims)

            aSet.playTogether(anims)

packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt

+40 −4
Original line number Original line Diff line number Diff line
@@ -400,15 +400,16 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { 




        // remove persistent dot

        // remove persistent dot

        systemStatusAnimationScheduler.removePersistentDot()

        systemStatusAnimationScheduler.removePersistentDot()

        testScheduler.runCurrent()



        // verify that the onHidePersistentDot callback is invoked

        verify(listener, times(1)).onHidePersistentDot()





        // skip disappear animation

        // skip disappear animation

        animatorTestRule.advanceTimeBy(DISAPPEAR_ANIMATION_DURATION)

        animatorTestRule.advanceTimeBy(DISAPPEAR_ANIMATION_DURATION)

        testScheduler.runCurrent()

        testScheduler.runCurrent()





        // verify that animationState changes to IDLE and onHidePersistentDot callback is invoked

        // verify that animationState changes to IDLE

        assertEquals(IDLE, systemStatusAnimationScheduler.getAnimationState())

        assertEquals(IDLE, systemStatusAnimationScheduler.getAnimationState())

        verify(listener, times(1)).onHidePersistentDot()

    }

    }





    @Test

    @Test
@@ -473,7 +474,6 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { 




        // request removal of persistent dot

        // request removal of persistent dot

        systemStatusAnimationScheduler.removePersistentDot()

        systemStatusAnimationScheduler.removePersistentDot()

        testScheduler.runCurrent()





        // schedule another high priority event while the event is animating out

        // schedule another high priority event while the event is animating out

        createAndScheduleFakePrivacyEvent()

        createAndScheduleFakePrivacyEvent()
@@ -489,6 +489,42 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { 
        verify(listener, times(1)).onHidePersistentDot()

        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) {

    private fun TestScope.fastForwardAnimationToState(@SystemAnimationState animationState: Int) {

        // this function should only be called directly after posting a status event

        // this function should only be called directly after posting a status event

        assertEquals(ANIMATION_QUEUED, systemStatusAnimationScheduler.getAnimationState())

        assertEquals(ANIMATION_QUEUED, systemStatusAnimationScheduler.getAnimationState())