From b4915629d783902ba915b58b1e3b4f2cba204d1b Mon Sep 17 00:00:00 2001 From: Ravindra Nallabilli Date: Mon, 13 Mar 2023 03:57:15 +0000 Subject: [PATCH] Revert "AnimatorSet sends pause/resume for seeked animators" Revert submission 21648238-animation_pause_resume Reason for revert: b/272997760, b/273129708 Reverted changes: /q/submissionid:21648238-animation_pause_resume Change-Id: Ice1f69ff5114953a91572365c8b4e34ad4c3de57 --- core/java/android/animation/Animator.java | 26 +--- core/java/android/animation/AnimatorSet.java | 36 +++-- .../java/android/animation/ValueAnimator.java | 24 ++- .../animation/AnimatorSetActivityTest.java | 8 +- .../animation/AnimatorSetCallsTest.java | 144 ++---------------- 5 files changed, 64 insertions(+), 174 deletions(-) diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index de7b80f2d7615..322d23b55d5d1 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -78,13 +78,6 @@ public abstract class Animator implements Cloneable { */ private Object[] mCachedList; - /** - * Tracks whether we've notified listeners of the onAnimationStart() event. This can be - * complex to keep track of since we notify listeners at different times depending on - * startDelay and whether start() was called before end(). - */ - boolean mStartListenersCalled = false; - /** * Sets the duration for delaying pausing animators when apps go into the background. * Used by AnimationHandler when requested to pause animators. @@ -172,9 +165,7 @@ public abstract class Animator implements Cloneable { * @see AnimatorPauseListener */ public void pause() { - // We only want to pause started Animators or animators that setCurrentPlayTime() - // have been called on. mStartListenerCalled will be true if seek has happened. - if ((isStarted() || mStartListenersCalled) && !mPaused) { + if (isStarted() && !mPaused) { mPaused = true; notifyPauseListeners(AnimatorCaller.ON_PAUSE); } @@ -453,7 +444,6 @@ public abstract class Animator implements Cloneable { anim.mPauseListeners = new ArrayList(mPauseListeners); } anim.mCachedList = null; - anim.mStartListenersCalled = false; return anim; } catch (CloneNotSupportedException e) { throw new AssertionError(); @@ -618,20 +608,6 @@ public abstract class Animator implements Cloneable { callOnList(mPauseListeners, notification, this, false); } - void notifyStartListeners(boolean isReversing) { - if (mListeners != null && !mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_START, isReversing); - } - mStartListenersCalled = true; - } - - void notifyEndListeners(boolean isReversing) { - if (mListeners != null && mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_END, isReversing); - } - mStartListenersCalled = false; - } - /** * Calls call for every item in list with animator and * isReverse as parameters. diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index ffdb9a21b39c8..09eec9d25a4bd 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -189,6 +189,11 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim */ private long[] mChildStartAndStopTimes; + /** + * Tracks whether we've notified listeners of the onAnimationStart() event. + */ + private boolean mStartListenersCalled; + // This is to work around a bug in b/34736819. This needs to be removed once app team // fixes their side. private AnimatorListenerAdapter mAnimationEndListener = new AnimatorListenerAdapter() { @@ -419,7 +424,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim if (Looper.myLooper() == null) { throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } - if (isStarted() || mStartListenersCalled) { + if (isStarted()) { notifyListeners(AnimatorCaller.ON_CANCEL, false); callOnPlayingSet(Animator::cancel); mPlayingSet.clear(); @@ -751,6 +756,20 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim } } + private void notifyStartListeners(boolean inReverse) { + if (mListeners != null && !mStartListenersCalled) { + notifyListeners(AnimatorCaller.ON_START, inReverse); + } + mStartListenersCalled = true; + } + + private void notifyEndListeners(boolean inReverse) { + if (mListeners != null && mStartListenersCalled) { + notifyListeners(AnimatorCaller.ON_END, inReverse); + } + mStartListenersCalled = false; + } + // Returns true if set is empty or contains nothing but animator sets with no start delay. private static boolean isEmptySet(AnimatorSet set) { if (set.getStartDelay() > 0) { @@ -917,18 +936,12 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim lastPlayTime - node.mStartTime, notify ); - if (notify) { - mPlayingSet.remove(node); - } } else if (start <= currentPlayTime && currentPlayTime <= end) { animator.animateSkipToEnds( currentPlayTime - node.mStartTime, lastPlayTime - node.mStartTime, notify ); - if (notify && !mPlayingSet.contains(node)) { - mPlayingSet.add(node); - } } } } @@ -956,18 +969,12 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim lastPlayTime - node.mStartTime, notify ); - if (notify) { - mPlayingSet.remove(node); - } } else if (start <= currentPlayTime && currentPlayTime <= end) { animator.animateSkipToEnds( currentPlayTime - node.mStartTime, lastPlayTime - node.mStartTime, notify ); - if (notify && !mPlayingSet.contains(node)) { - mPlayingSet.add(node); - } } } } @@ -1108,8 +1115,8 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim mSeekState.setPlayTime(0, mReversing); } } - mSeekState.setPlayTime(playTime, mReversing); animateBasedOnPlayTime(playTime, lastPlayTime, mReversing, true); + mSeekState.setPlayTime(playTime, mReversing); } /** @@ -1491,6 +1498,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim anim.mNodeMap = new ArrayMap(); anim.mNodes = new ArrayList(nodeCount); anim.mEvents = new ArrayList(); + anim.mStartListenersCalled = false; anim.mAnimationEndListener = new AnimatorListenerAdapter() { @Override public void onAnimationEnd(Animator animation) { diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index d77e3222ad569..719f596949bb2 100644 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -198,6 +198,13 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio */ private boolean mStarted = false; + /** + * Tracks whether we've notified listeners of the onAnimationStart() event. This can be + * complex to keep track of since we notify listeners at different times depending on + * startDelay and whether start() was called before end(). + */ + private boolean mStartListenersCalled = false; + /** * Flag that denotes whether the animation is set up and ready to go. Used to * set up animation that has not yet been started. @@ -1101,6 +1108,20 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio } } + private void notifyStartListeners(boolean isReversing) { + if (mListeners != null && !mStartListenersCalled) { + notifyListeners(AnimatorCaller.ON_START, isReversing); + } + mStartListenersCalled = true; + } + + private void notifyEndListeners(boolean isReversing) { + if (mListeners != null && mStartListenersCalled) { + notifyListeners(AnimatorCaller.ON_END, isReversing); + } + mStartListenersCalled = false; + } + /** * Start the animation playing. This version of start() takes a boolean flag that indicates * whether the animation should play in reverse. The flag is usually false, but may be set @@ -1188,7 +1209,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio // Only cancel if the animation is actually running or has been started and is about // to run // Only notify listeners if the animator has actually started - if ((mStarted || mRunning || mStartListenersCalled) && mListeners != null) { + if ((mStarted || mRunning) && mListeners != null) { if (!mRunning) { // If it's not yet running, then start listeners weren't called. Call them now. notifyStartListeners(mReversing); @@ -1666,6 +1687,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio anim.mRunning = false; anim.mPaused = false; anim.mResumed = false; + anim.mStartListenersCalled = false; anim.mStartTime = -1; anim.mStartTimeCommitted = false; anim.mAnimationEndRequested = false; diff --git a/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java b/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java index a7538701807a3..7a1de0c5d4fe4 100644 --- a/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java +++ b/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java @@ -435,11 +435,9 @@ public class AnimatorSetActivityTest { mActivityRule.runOnUiThread(s::start); while (!listener.endIsCalled) { - mActivityRule.runOnUiThread(() -> { - boolean passedStartDelay = a1.isStarted() || a2.isStarted() || a3.isStarted() - || a4.isStarted() || a5.isStarted(); - assertEquals(passedStartDelay, s.isRunning()); - }); + boolean passedStartDelay = a1.isStarted() || a2.isStarted() || a3.isStarted() || + a4.isStarted() || a5.isStarted(); + assertEquals(passedStartDelay, s.isRunning()); Thread.sleep(50); } assertFalse(s.isRunning()); diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java index 41a66771986fa..22da0aa4597e6 100644 --- a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java +++ b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java @@ -40,7 +40,6 @@ public class AnimatorSetCallsTest { private AnimatorSetActivity mActivity; private AnimatorSet mSet1; - private AnimatorSet mSet2; private ObjectAnimator mAnimator; private CountListener mListener1; private CountListener mListener2; @@ -57,10 +56,10 @@ public class AnimatorSetCallsTest { mSet1.addListener(mListener1); mSet1.addPauseListener(mListener1); - mSet2 = new AnimatorSet(); + AnimatorSet set2 = new AnimatorSet(); mListener2 = new CountListener(); - mSet2.addListener(mListener2); - mSet2.addPauseListener(mListener2); + set2.addListener(mListener2); + set2.addPauseListener(mListener2); mAnimator = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); mListener3 = new CountListener(); @@ -68,8 +67,8 @@ public class AnimatorSetCallsTest { mAnimator.addPauseListener(mListener3); mAnimator.setDuration(1); - mSet2.play(mAnimator); - mSet1.play(mSet2); + set2.play(mAnimator); + mSet1.play(set2); }); } @@ -176,7 +175,6 @@ public class AnimatorSetCallsTest { assertEquals(1, updateValues.size()); assertEquals(0f, updateValues.get(0), 0f); } - @Test public void updateOnlyWhileRunning() { ArrayList updateValues = new ArrayList<>(); @@ -209,118 +207,6 @@ public class AnimatorSetCallsTest { } } - @Test - public void pauseResumeSeekingAnimators() { - ValueAnimator animator2 = ValueAnimator.ofFloat(0f, 1f); - mSet2.play(animator2).after(mAnimator); - mSet2.setStartDelay(100); - mSet1.setStartDelay(100); - mAnimator.setDuration(100); - - mActivity.runOnUiThread(() -> { - mSet1.setCurrentPlayTime(0); - mSet1.pause(); - - // only startForward and pause should have been called once - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 1, 0 - ); - mListener2.assertValues( - 0, 0, 0, 0, 0, 0, 0, 0 - ); - mListener3.assertValues( - 0, 0, 0, 0, 0, 0, 0, 0 - ); - - mSet1.resume(); - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 1, 1 - ); - mListener2.assertValues( - 0, 0, 0, 0, 0, 0, 0, 0 - ); - mListener3.assertValues( - 0, 0, 0, 0, 0, 0, 0, 0 - ); - - mSet1.setCurrentPlayTime(200); - - // resume and endForward should have been called once - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 1, 1 - ); - mListener2.assertValues( - 1, 0, 0, 0, 0, 0, 0, 0 - ); - mListener3.assertValues( - 1, 0, 0, 0, 0, 0, 0, 0 - ); - - mSet1.pause(); - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 2, 1 - ); - mListener2.assertValues( - 1, 0, 0, 0, 0, 0, 1, 0 - ); - mListener3.assertValues( - 1, 0, 0, 0, 0, 0, 1, 0 - ); - mSet1.resume(); - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 2, 2 - ); - mListener2.assertValues( - 1, 0, 0, 0, 0, 0, 1, 1 - ); - mListener3.assertValues( - 1, 0, 0, 0, 0, 0, 1, 1 - ); - - // now go to animator2 - mSet1.setCurrentPlayTime(400); - mSet1.pause(); - mSet1.resume(); - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 3, 3 - ); - mListener2.assertValues( - 1, 0, 0, 0, 0, 0, 2, 2 - ); - mListener3.assertValues( - 1, 0, 1, 0, 0, 0, 1, 1 - ); - - // now go back to mAnimator - mSet1.setCurrentPlayTime(250); - mSet1.pause(); - mSet1.resume(); - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 4, 4 - ); - mListener2.assertValues( - 1, 0, 0, 0, 0, 0, 3, 3 - ); - mListener3.assertValues( - 1, 1, 1, 0, 0, 0, 2, 2 - ); - - // now go back to before mSet2 was being run - mSet1.setCurrentPlayTime(1); - mSet1.pause(); - mSet1.resume(); - mListener1.assertValues( - 1, 0, 0, 0, 0, 0, 5, 5 - ); - mListener2.assertValues( - 1, 0, 0, 1, 0, 0, 3, 3 - ); - mListener3.assertValues( - 1, 1, 1, 1, 0, 0, 2, 2 - ); - }); - } - private void waitForOnUiThread(PollingCheck.PollingCheckCondition condition) { final boolean[] value = new boolean[1]; PollingCheck.waitFor(() -> { @@ -352,16 +238,16 @@ public class AnimatorSetCallsTest { int pause, int resume ) { - assertEquals("onAnimationStart() without direction", 0, startNoParam); - assertEquals("onAnimationEnd() without direction", 0, endNoParam); - assertEquals("onAnimationStart(forward)", startForward, this.startForward); - assertEquals("onAnimationStart(reverse)", startReverse, this.startReverse); - assertEquals("onAnimationEnd(forward)", endForward, this.endForward); - assertEquals("onAnimationEnd(reverse)", endReverse, this.endReverse); - assertEquals("onAnimationCancel()", cancel, this.cancel); - assertEquals("onAnimationRepeat()", repeat, this.repeat); - assertEquals("onAnimationPause()", pause, this.pause); - assertEquals("onAnimationResume()", resume, this.resume); + assertEquals(0, startNoParam); + assertEquals(0, endNoParam); + assertEquals(startForward, this.startForward); + assertEquals(startReverse, this.startReverse); + assertEquals(endForward, this.endForward); + assertEquals(endReverse, this.endReverse); + assertEquals(cancel, this.cancel); + assertEquals(repeat, this.repeat); + assertEquals(pause, this.pause); + assertEquals(resume, this.resume); } @Override