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

Commit 2f6e4a97 authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Return running state before calling animation end callback 

This only affects if setPostNotifyEndListenerEnabled is set.
Previously, anim.isRunning() returns false if the end callback is
posted, then the app may miss to call end or cancel. And if the
app starts a new animation immediately, the callbacks between the
old and new animations will have overlap.

So when the end listener is pending, isRunning() and isStarted()
should return true. And if app invoke cancel() or end(), the
pending end listener should be called and consumed immediately.

Fix: 399819784
Bug: 300035126
Flag: com.android.window.flags.system_ui_post_animation_end
Test: atest ValueAnimatorTests#testEndOnPendingEndListener
            AnimatorSetCallsTest#testEndOnPendingEndListener

Change-Id: Ie3aa2ef14cb53d91a04fdfac012ab63494c69990
parent b16925e3

core/java/android/animation/AnimatorSet.java

+19 −10
Original line number Diff line number Diff line
@@ -423,8 +423,13 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim 
            notifyListeners(AnimatorCaller.ON_CANCEL, false);

            callOnPlayingSet(Animator::cancel);

            mPlayingSet.clear();

            endAnimationAndNotifyEndListenersImmediately();

        }

    }



    private void endAnimationAndNotifyEndListenersImmediately() {

        // If the end callback is pending, invoke the end callbacks of the animator nodes before

            // ending this set. Pass notifyListeners=false because this endAnimation will do that.

        // ending this set. Pass notifyListeners=false because endAnimation will do that.

        if (consumePendingEndListeners(false /* notifyListeners */)) {

            for (int i = mNodeMap.size() - 1; i >= 0; i--) {

                mNodeMap.keyAt(i).consumePendingEndListeners(true /* notifyListeners */);
@@ -432,7 +437,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim 
        }

        endAnimation();

    }

    }



    /**

     * Calls consumer on every Animator of mPlayingSet.
@@ -529,7 +533,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim 
                }

            }

        }

        endAnimation();

        endAnimationAndNotifyEndListenersImmediately();

    }



    /**
@@ -1455,8 +1459,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim 
    private void endAnimation(boolean fromLastFrame) {

        final boolean postNotifyEndListener = sPostNotifyEndListenerEnabled && mListeners != null

                && fromLastFrame && mTotalDuration > 0;

        mStarted = false;

        mLastFrameTime = -1;

        mFirstFrame = -1;

        mLastEventId = -1;

        mPaused = false;
@@ -1466,11 +1468,18 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim 


        // No longer receive callbacks

        removeAnimationCallback();

        // If postNotifyEndListener is false (most cases), then it is the same as calling

        // completeEndAnimation directly.

        notifyEndListenersFromEndAnimation(mReversing, postNotifyEndListener);

    }



    @Override

    void completeEndAnimation(boolean isReversing, String notifyListenerTraceName) {

        // The mStarted and mLastFrameTime are reset here because isStarted() and isRunning()

        // can be true before notifying the end listeners. When notifying the end listeners,

        // isStarted() and isRunning() should be false.

        mStarted = false;

        mLastFrameTime = -1;

        super.completeEndAnimation(isReversing, notifyListenerTraceName);

        removeAnimationEndListener();

        mSelfPulse = true;

core/java/android/animation/ValueAnimator.java

+11 −2
Original line number Diff line number Diff line
@@ -1212,6 +1212,10 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio 
            initAnimation();

        }

        animateValue(shouldPlayBackward(mRepeatCount, mReversing) ? 0f : 1f);

        if (mAnimationEndRequested) {

            consumePendingEndListeners(true /* notifyListeners */);

            return;

        }

        endAnimation();

    }
@@ -1308,8 +1312,8 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio 
        mLastFrameTime = -1;

        mFirstFrameTime = -1;

        mStartTime = -1;

        mRunning = false;

        mStarted = false;

        // If postNotifyEndListener is false (most cases), then it is the same as calling

        // completeEndAnimation directly.

        notifyEndListenersFromEndAnimation(mReversing, postNotifyEndListener);

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.asyncTraceEnd(Trace.TRACE_TAG_VIEW, getNameForTrace(),
@@ -1319,6 +1323,11 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio 


    @Override

    void completeEndAnimation(boolean isReversing, String notifyListenerTraceName) {

        // The mRunning and mStarted are reset here because isStarted() and isRunning()

        // can be true before notifying the end listeners. When notifying the end listeners,

        // isStarted() and isRunning() should be false.

        mRunning = false;

        mStarted = false;

        super.completeEndAnimation(isReversing, notifyListenerTraceName);

        // mReversing needs to be reset *after* notifying the listeners for the end callbacks.

        mReversing = false;

core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java

+22 −6
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import java.util.ArrayList; 
import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.atomic.AtomicReference;

import java.util.function.Consumer;





@MediumTest
@@ -490,9 +491,24 @@ public class AnimatorSetCallsTest { 


    @Test

    public void testCancelOnPendingEndListener() throws Throwable {

        testPendingEndListener(AnimatorSet::cancel);

    }



    @Test

    public void testEndOnPendingEndListener() throws Throwable {

        testPendingEndListener(animatorSet -> {

            // This verifies that isRunning() and isStarted() are true at last frame.

            // Then the end() should invoke the end callback immediately.

            if (animatorSet.isRunning() && animatorSet.isStarted()) {

                animatorSet.end();

            }

        });

    }



    private void testPendingEndListener(Consumer<AnimatorSet> finishOnLastFrame) throws Throwable {

        final CountDownLatch endLatch = new CountDownLatch(1);

        final Handler handler = new Handler(Looper.getMainLooper());

        final boolean[] endCalledRightAfterCancel = new boolean[2];

        final boolean[] endCalledImmediately = new boolean[2];

        final AnimatorSet set = new AnimatorSet();

        final ValueAnimatorTests.MyListener asListener = new ValueAnimatorTests.MyListener();

        final ValueAnimatorTests.MyListener vaListener = new ValueAnimatorTests.MyListener();
@@ -502,9 +518,9 @@ public class AnimatorSetCallsTest { 
        va.addUpdateListener(animation -> {

            if (animation.getAnimatedFraction() == 1f) {

                handler.post(() -> {

                    set.cancel();

                    endCalledRightAfterCancel[0] = vaListener.endCalled;

                    endCalledRightAfterCancel[1] = asListener.endCalled;

                    finishOnLastFrame.accept(set);

                    endCalledImmediately[0] = vaListener.endCalled;

                    endCalledImmediately[1] = asListener.endCalled;

                    endLatch.countDown();

                });

            }
@@ -517,8 +533,8 @@ public class AnimatorSetCallsTest { 
        try {

            handler.post(set::start);

            assertTrue(endLatch.await(1, TimeUnit.SECONDS));

            assertTrue(endCalledRightAfterCancel[0]);

            assertTrue(endCalledRightAfterCancel[1]);

            assertTrue(endCalledImmediately[0]);

            assertTrue(endCalledImmediately[1]);

        } finally {

            ValueAnimator.setPostNotifyEndListenerEnabled(false);

        }

core/tests/coretests/src/android/animation/ValueAnimatorTests.java

+21 −4
Original line number Diff line number Diff line
@@ -44,6 +44,7 @@ import org.junit.runner.RunWith; 
import java.util.ArrayList;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeUnit;

import java.util.function.Consumer;



@RunWith(AndroidJUnit4.class)

@MediumTest
@@ -923,9 +924,25 @@ public class ValueAnimatorTests { 


    @Test

    public void testCancelOnPendingEndListener() throws Throwable {

        testPendingEndListener(ValueAnimator::cancel);

    }



    @Test

    public void testEndOnPendingEndListener() throws Throwable {

        testPendingEndListener(animator -> {

            // This verifies that isRunning() and isStarted() are true at last frame.

            // Then the end() should invoke the end callback immediately.

            if (animator.isRunning() && animator.isStarted()) {

                animator.end();

            }

        });

    }



    private void testPendingEndListener(Consumer<ValueAnimator> finishOnLastFrame)

            throws Throwable {

        final boolean[] endCalledImmediately = new boolean[1];

        final CountDownLatch endLatch = new CountDownLatch(1);

        final Handler handler = new Handler(Looper.getMainLooper());

        final boolean[] endCalledRightAfterCancel = new boolean[1];

        final MyListener listener = new MyListener();

        final ValueAnimator va = new ValueAnimator();

        va.setFloatValues(0f, 1f);
@@ -933,8 +950,8 @@ public class ValueAnimatorTests { 
        va.addUpdateListener(animation -> {

            if (animation.getAnimatedFraction() == 1f) {

                handler.post(() -> {

                    va.cancel();

                    endCalledRightAfterCancel[0] = listener.endCalled;

                    finishOnLastFrame.accept(va);

                    endCalledImmediately[0] = listener.endCalled;

                    endLatch.countDown();

                });

            }
@@ -945,7 +962,7 @@ public class ValueAnimatorTests { 
        try {

            handler.post(va::start);

            assertThat(endLatch.await(1, TimeUnit.SECONDS)).isTrue();

            assertThat(endCalledRightAfterCancel[0]).isTrue();

            assertThat(endCalledImmediately[0]).isTrue();

        } finally {

            ValueAnimator.setPostNotifyEndListenerEnabled(false);

        }