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

Commit cc77d82a authored by Selim Cinek's avatar Selim Cinek
Browse files

Fixed an issue where a view could be stuck with an offset 

When cancelling an animation with a delay before it started
running, it's endlisteners weren't called on cancellation.
However, the listeners were already notified of the start
and the initial offset was set.
We now make sure that we also notify the listeners of
the end, as well as reset relevant state.

Flag: com.android.systemui.physical_notification_movement
Test: atest SystemUITests
Fixes: 417008364
Change-Id: Iafbcd98184bb562c63211fde836c52b77f076458
parent c59da455

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/PhysicsPropertyAnimatorTest.kt

+21 −0
Original line number Diff line number Diff line
@@ -221,6 +221,27 @@ class PhysicsPropertyAnimatorTest : SysuiTestCase() { 
        Assert.assertTrue(propertyData.offset == 0f)

    }



    @Test

    fun testEndedBeforeStartingCleanupHandler() {

        effectiveProperty.setValue(view, 100f)

        animationProperties.setDelay(200)

        PhysicsPropertyAnimator.setProperty(

            view,

            property,

            200f,

            animationProperties,

            true,

            finishListener,

        )

        val propertyData = ViewState.getChildTag(view, property.tag) as PropertyData

        Assert.assertTrue(propertyData.endedBeforeStartingCleanupHandler != null)

        propertyData.endedBeforeStartingCleanupHandler?.invoke(true)

        Assert.assertTrue(propertyData.endedBeforeStartingCleanupHandler == null)

        Assert.assertTrue(propertyData.offset == 0f)

        Assert.assertTrue(propertyData.animator == null)

        Assert.assertTrue(propertyData.doubleOvershootAvoidingListener == null)

    }



    @Test

    fun testUsingListenerProperties() {

        val finishListener2 = Mockito.mock(DynamicAnimation.OnAnimationEndListener::class.java)

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/stack/ViewStateTest.kt

+65 −0
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import com.android.systemui.log.assertLogsWtf 
import com.android.systemui.statusbar.notification.PhysicsPropertyAnimator

import com.android.systemui.statusbar.notification.PhysicsPropertyAnimator.Companion.TAG_ANIMATOR_TRANSLATION_Y

import com.android.systemui.statusbar.notification.PhysicsPropertyAnimator.Companion.Y_TRANSLATION

import com.android.systemui.statusbar.notification.PropertyData

import org.junit.Assert

import org.junit.Rule

import org.junit.Test
@@ -92,6 +93,70 @@ class ViewStateTest : SysuiTestCase() { 
        Assert.assertTrue(PhysicsPropertyAnimator.isAnimating(animatedView, Y_TRANSLATION))

    }



    @Test

    fun testCancellationCallsHandlers() {

        val animatedView = View(context)

        viewState.setUsePhysicsForMovement(true)

        viewState.applyToView(animatedView)

        viewState.yTranslation = 100f

        val animationFilter = AnimationFilter().animateY()

        val animationProperties = object : AnimationProperties() {

            override fun getAnimationFilter(): AnimationFilter {

                return animationFilter

            }

        }

        animationProperties.delay = 100

        viewState.animateTo(animatedView, animationProperties)

        Assert.assertFalse(PhysicsPropertyAnimator.isAnimating(animatedView, Y_TRANSLATION))

        val propertyData =

            ViewState.getChildTag(animatedView, TAG_ANIMATOR_TRANSLATION_Y) as PropertyData

        Assert.assertTrue(

            "no handler set to cancel delayed animation",

            propertyData.endedBeforeStartingCleanupHandler != null

        )

        viewState.cancelAnimations(animatedView)

        Assert.assertTrue(

            "Handler is still set after canceling early",

            propertyData.endedBeforeStartingCleanupHandler == null

        )

        Assert.assertTrue(

            "offset not reset after cancelling",

            propertyData.offset == 0f

        )

    }



    @Test

    fun testSkipToEndCallsHandlers() {

        val animatedView = View(context)

        viewState.setUsePhysicsForMovement(true)

        viewState.applyToView(animatedView)

        viewState.yTranslation = 100f

        val animationFilter = AnimationFilter().animateY()

        val animationProperties = object : AnimationProperties() {

            override fun getAnimationFilter(): AnimationFilter {

                return animationFilter

            }

        }

        animationProperties.delay = 100

        viewState.animateTo(animatedView, animationProperties)

        Assert.assertFalse(PhysicsPropertyAnimator.isAnimating(animatedView, Y_TRANSLATION))

        val propertyData =

            ViewState.getChildTag(animatedView, TAG_ANIMATOR_TRANSLATION_Y) as PropertyData

        Assert.assertTrue(

            "no handler set to cancel delayed animation",

            propertyData.endedBeforeStartingCleanupHandler != null

        )

        viewState.finishAnimations(animatedView)

        Assert.assertTrue(

            "Handler is still set after canceling early",

            propertyData.endedBeforeStartingCleanupHandler == null

        )

        Assert.assertTrue(

            "offset not reset after cancelling",

            propertyData.offset == 0f

        )

    }



    @Test

    fun testNotUsingPhysics() {

        val animatedView = View(context)

packages/SystemUI/src/com/android/systemui/statusbar/notification/PhysicsPropertyAnimator.kt

+22 −0
Original line number Diff line number Diff line
@@ -66,6 +66,12 @@ data class PropertyData( 
    var offset: Float = 0f,

    var animator: SpringAnimation? = null,

    var delayRunnable: Runnable? = null,



    /**

     * A runnable that should be executed if the animation is skipped to end / cancelled before

     * the animation actually starts running.

     */

    var endedBeforeStartingCleanupHandler: ((Boolean) -> Unit)? = null,

    var startOffset: Float = 0f,

    var doubleOvershootAvoidingListener: DynamicAnimation.OnAnimationUpdateListener? = null

)
@@ -208,6 +214,22 @@ private fun startAnimation( 
        // conditions and will never actually end them only calling start explicitly does that,

        // so let's start them again!

        animator.start()

        propertyData.endedBeforeStartingCleanupHandler = null;

    }

    propertyData.endedBeforeStartingCleanupHandler = { cancelled ->

        val listener = properties?.getAnimationEndListener(animatableProperty.property)

        listener?.onAnimationEnd(propertyData.animator,

            cancelled,

            0f /* value */,

            0f /* velocity */

        )

        propertyData.animator = null

        propertyData.doubleOvershootAvoidingListener = null

        propertyData.offset = 0f

        // We always reset the offset as we never want to get stuck with old values. This is

        // consistent with the end listener above.

        property.set(view, propertyData.finalValue)

        propertyData.endedBeforeStartingCleanupHandler = null;

    }

    if (properties != null && properties.delay > 0 && !animator.isRunning) {

        propertyData.delayRunnable = startRunnable

packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ViewState.java

+25 −0
Original line number Diff line number Diff line
@@ -44,6 +44,9 @@ import com.android.systemui.statusbar.notification.PropertyData; 
import com.android.systemui.statusbar.notification.headsup.HeadsUpUtil;

import com.android.systemui.statusbar.notification.row.ExpandableView;



import kotlin.Unit;

import kotlin.jvm.functions.Function1;



import java.io.PrintWriter;

import java.lang.reflect.Field;

import java.lang.reflect.Modifier;
@@ -801,7 +804,18 @@ public class ViewState implements Dumpable { 
                child.removeCallbacks(delayRunnable);

                SpringAnimation animator = propertyData.getAnimator();

                if (animator != null) {

                    boolean wasRunning = animator.isRunning();

                    animator.cancel();

                    if (!wasRunning) {

                        // The animation was never started, so the cancel above doesn't do much.

                        // We need to notify the endListeners manually that the animation has ended

                        // since they need to reset some state.

                        Function1<Boolean, Unit> listener =

                                propertyData.getEndedBeforeStartingCleanupHandler();

                        if (listener != null) {

                            listener.invoke(true /* cancelled */);

                        }

                    }

                }

            }

        }
@@ -818,7 +832,18 @@ public class ViewState implements Dumpable { 
                child.removeCallbacks(delayRunnable);

                SpringAnimation animator = propertyData.getAnimator();

                if (animator != null) {

                    boolean wasRunning = animator.isRunning();

                    animator.skipToEnd();

                    if (!wasRunning) {

                        // The animation was ever started, so the skipToEnd above doesn't do much.

                        // We need to notify the endListeners manually that the animation has ended

                        // since they need to reset some state.

                        Function1<Boolean, Unit> listener =

                                propertyData.getEndedBeforeStartingCleanupHandler();

                        if (listener != null) {

                            listener.invoke(false /* cancelled */);

                        }

                    }

                }

            }

        }