From 17cf42cb85c22b50ecfa8d21efc992f99d20fc45 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Tue, 17 Apr 2012 13:18:14 -0700 Subject: [PATCH] Fix logic of animator start/cancel/end callbacks The callbacks for animators in some corner cases were not being called correctly. For example, startDelayed animators that were started and then ended didn't send out the proper events. This CL fixes that logic. Specifically: - An animator that is end()'d will implicitly start() itself and then assign an end value. This was already the case, but listeners were not getting notified. Now this situation causes callbacks to listeners for both the start and end events. - startDelayed animators that are end()'d or cancel()'d prior to finishing the startDelay phase will send out events (start and cancel/end, as appropriate) to listeners. Change-Id: I40a0f2fdb19d9ec7c3726a91363686c6ecb7d915 --- .../android/animation/LayoutTransition.java | 2 + .../java/android/animation/ValueAnimator.java | 50 ++++++++++++------- .../src/android/animation/EventsTest.java | 11 ++-- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/core/java/android/animation/LayoutTransition.java b/core/java/android/animation/LayoutTransition.java index 871bb2c292033..c643137d0c6ca 100644 --- a/core/java/android/animation/LayoutTransition.java +++ b/core/java/android/animation/LayoutTransition.java @@ -1005,6 +1005,8 @@ public class LayoutTransition { anim.start(); anim.end(); } + // listeners should clean up the currentChangingAnimations list, but just in case... + currentChangingAnimations.clear(); } /** diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index fade20c77ccc5..2154b1412c70c 100755 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -149,6 +149,13 @@ public class ValueAnimator extends Animator { */ 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. @@ -885,6 +892,18 @@ public class ValueAnimator extends Animator { } } + private void notifyStartListeners() { + if (mListeners != null && !mStartListenersCalled) { + ArrayList tmpListeners = + (ArrayList) mListeners.clone(); + int numListeners = tmpListeners.size(); + for (int i = 0; i < numListeners; ++i) { + tmpListeners.get(i).onAnimationStart(this); + } + } + mStartListenersCalled = true; + } + /** * 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 @@ -914,15 +933,7 @@ public class ValueAnimator extends Animator { setCurrentPlayTime(getCurrentPlayTime()); mPlayingState = STOPPED; mRunning = true; - - if (mListeners != null) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationStart(this); - } - } + notifyStartListeners(); } animationHandler.sendEmptyMessage(ANIMATION_START); } @@ -941,7 +952,11 @@ public class ValueAnimator extends Animator { || handler.mPendingAnimations.contains(this) || handler.mDelayedAnims.contains(this)) { // Only notify listeners if the animator has actually started - if (mRunning && 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(); + } ArrayList tmpListeners = (ArrayList) mListeners.clone(); for (AnimatorListener listener : tmpListeners) { @@ -959,6 +974,7 @@ public class ValueAnimator extends Animator { // Special case if the animation has not yet started; get it ready for ending mStartedDelay = false; startAnimation(handler); + mStarted = true; } else if (!mInitialized) { initAnimation(); } @@ -1010,7 +1026,11 @@ public class ValueAnimator extends Animator { handler.mPendingAnimations.remove(this); handler.mDelayedAnims.remove(this); mPlayingState = STOPPED; - if (mRunning && 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(); + } ArrayList tmpListeners = (ArrayList) mListeners.clone(); int numListeners = tmpListeners.size(); @@ -1020,6 +1040,7 @@ public class ValueAnimator extends Animator { } mRunning = false; mStarted = false; + mStartListenersCalled = false; } /** @@ -1032,12 +1053,7 @@ public class ValueAnimator extends Animator { if (mStartDelay > 0 && mListeners != null) { // Listeners were already notified in start() if startDelay is 0; this is // just for delayed animations - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationStart(this); - } + notifyStartListeners(); } } diff --git a/core/tests/coretests/src/android/animation/EventsTest.java b/core/tests/coretests/src/android/animation/EventsTest.java index 701a3f064f687..8df711b913564 100644 --- a/core/tests/coretests/src/android/animation/EventsTest.java +++ b/core/tests/coretests/src/android/animation/EventsTest.java @@ -173,8 +173,7 @@ public abstract class EventsTest // This should only be called on an animation that has been started and not // yet canceled or ended assertFalse(mCanceled); - assertTrue(mRunning); - assertTrue(mStarted); + assertTrue(mRunning || mStarted); mCanceled = true; } @@ -182,8 +181,7 @@ public abstract class EventsTest public void onAnimationEnd(Animator animation) { // This should only be called on an animation that has been started and not // yet ended - assertTrue(mRunning); - assertTrue(mStarted); + assertTrue(mRunning || mStarted); mRunning = false; mStarted = false; super.onAnimationEnd(animation); @@ -210,11 +208,12 @@ public abstract class EventsTest } /** - * Verify that calling end on an unstarted animator does nothing. + * Verify that calling end on an unstarted animator starts/ends an animator. */ @UiThreadTest @SmallTest public void testEnd() throws Exception { + mRunning = true; // end() implicitly starts an unstarted animator mAnimator.end(); } @@ -496,6 +495,7 @@ public abstract class EventsTest mRunning = true; mAnimator.start(); mAnimator.end(); + mRunning = true; // end() implicitly starts an unstarted animator mAnimator.end(); mFuture.release(); } catch (junit.framework.AssertionFailedError e) { @@ -544,6 +544,7 @@ public abstract class EventsTest mRunning = true; mAnimator.start(); mAnimator.end(); + mRunning = true; // end() implicitly starts an unstarted animator mAnimator.end(); mFuture.release(); } catch (junit.framework.AssertionFailedError e) {