From 7dfacdb1c820f955cb3cd6032ff5fbc2dd7d9df5 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Mon, 11 Jul 2011 17:01:56 -0700 Subject: [PATCH] Fix Animator cancel() behavior Previously, calling cancel() on an Animator would cause onAnimationCancel events to be sent to all listeners. This was confusing for listeners that were keying off this event for performing other actions, when the original animator wasn't truly canceled (because it wasn't even running, or had already been canceled earlier). This change hinges listener notification on the animator actually running; no events are sent otherwise. Also added the first set of Animator tests to verify that this behavior is correct. Change-Id: I81ab56559b5c0343c8dc7880e1c8235f3020975b --- core/java/android/animation/AnimatorSet.java | 148 +++++----- .../java/android/animation/ValueAnimator.java | 14 +- core/tests/coretests/Android.mk | 2 +- core/tests/coretests/AndroidManifest.xml | 7 + .../coretests/res/layout/animator_basic.xml | 11 + .../animation/AnimatorSetEventsTest.java | 39 +++ .../animation/BasicAnimatorActivity.java | 29 ++ .../src/android/animation/EventsTest.java | 265 ++++++++++++++++++ .../src/android/animation/FutureWaiter.java | 40 +++ .../animation/ObjectAnimatorEventsTest.java | 35 +++ .../animation/ValueAnimatorEventsTest.java | 29 ++ 11 files changed, 547 insertions(+), 72 deletions(-) create mode 100644 core/tests/coretests/res/layout/animator_basic.xml create mode 100644 core/tests/coretests/src/android/animation/AnimatorSetEventsTest.java create mode 100644 core/tests/coretests/src/android/animation/BasicAnimatorActivity.java create mode 100644 core/tests/coretests/src/android/animation/EventsTest.java create mode 100644 core/tests/coretests/src/android/animation/FutureWaiter.java create mode 100644 core/tests/coretests/src/android/animation/ObjectAnimatorEventsTest.java create mode 100644 core/tests/coretests/src/android/animation/ValueAnimatorEventsTest.java diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index d77dbdccef17e..e6c2a0fa281dd 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -87,11 +87,13 @@ public final class AnimatorSet extends Animator { private AnimatorSetListener mSetListener = null; /** - * Flag indicating that the AnimatorSet has been canceled (by calling cancel() or end()). + * Flag indicating that the AnimatorSet has been manually + * terminated (by calling cancel() or end()). * This flag is used to avoid starting other animations when currently-playing - * child animations of this AnimatorSet end. + * child animations of this AnimatorSet end. It also determines whether cancel/end + * notifications are sent out via the normal AnimatorSetListener mechanism. */ - boolean mCanceled = false; + boolean mTerminated = false; // The amount of time in ms to delay starting the animation after start() is called private long mStartDelay = 0; @@ -271,31 +273,29 @@ public final class AnimatorSet extends Animator { @SuppressWarnings("unchecked") @Override public void cancel() { - mCanceled = true; - if (mListeners != null) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - for (AnimatorListener listener : tmpListeners) { - listener.onAnimationCancel(this); - } - } - if (mDelayAnim != null && mDelayAnim.isRunning()) { - // If we're currently in the startDelay period, just cancel that animator and - // send out the end event to all listeners - mDelayAnim.cancel(); + mTerminated = true; + if (isRunning()) { + ArrayList tmpListeners = null; if (mListeners != null) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); + tmpListeners = (ArrayList) mListeners.clone(); + for (AnimatorListener listener : tmpListeners) { + listener.onAnimationCancel(this); + } + } + if (mDelayAnim != null && mDelayAnim.isRunning()) { + // If we're currently in the startDelay period, just cancel that animator and + // send out the end event to all listeners + mDelayAnim.cancel(); + } else if (mSortedNodes.size() > 0) { + for (Node node : mSortedNodes) { + node.animation.cancel(); + } + } + if (tmpListeners != null) { for (AnimatorListener listener : tmpListeners) { listener.onAnimationEnd(this); } } - return; - } - if (mSortedNodes.size() > 0) { - for (Node node : mSortedNodes) { - node.animation.cancel(); - } } } @@ -307,23 +307,32 @@ public final class AnimatorSet extends Animator { */ @Override public void end() { - mCanceled = true; - if (mSortedNodes.size() != mNodes.size()) { - // hasn't been started yet - sort the nodes now, then end them - sortNodes(); - for (Node node : mSortedNodes) { - if (mSetListener == null) { - mSetListener = new AnimatorSetListener(this); + mTerminated = true; + if (isRunning()) { + if (mSortedNodes.size() != mNodes.size()) { + // hasn't been started yet - sort the nodes now, then end them + sortNodes(); + for (Node node : mSortedNodes) { + if (mSetListener == null) { + mSetListener = new AnimatorSetListener(this); + } + node.animation.addListener(mSetListener); } - node.animation.addListener(mSetListener); } - } - if (mDelayAnim != null) { - mDelayAnim.cancel(); - } - if (mSortedNodes.size() > 0) { - for (Node node : mSortedNodes) { - node.animation.end(); + if (mDelayAnim != null) { + mDelayAnim.cancel(); + } + if (mSortedNodes.size() > 0) { + for (Node node : mSortedNodes) { + node.animation.end(); + } + } + if (mListeners != null) { + ArrayList tmpListeners = + (ArrayList) mListeners.clone(); + for (AnimatorListener listener : tmpListeners) { + listener.onAnimationEnd(this); + } } } } @@ -424,7 +433,7 @@ public final class AnimatorSet extends Animator { @SuppressWarnings("unchecked") @Override public void start() { - mCanceled = false; + mTerminated = false; // First, sort the nodes (if necessary). This will ensure that sortedNodes // contains the animation nodes in the correct order. @@ -437,7 +446,8 @@ public final class AnimatorSet extends Animator { ArrayList oldListeners = node.animation.getListeners(); if (oldListeners != null && oldListeners.size() > 0) { for (AnimatorListener listener : oldListeners) { - if (listener instanceof DependencyListener) { + if (listener instanceof DependencyListener || + listener instanceof AnimatorSetListener) { node.animation.removeListener(listener); } } @@ -522,7 +532,7 @@ public final class AnimatorSet extends Animator { * and will populate any appropriate lists, when it is started. */ anim.mNeedsSort = true; - anim.mCanceled = false; + anim.mTerminated = false; anim.mPlayingSet = new ArrayList(); anim.mNodeMap = new HashMap(); anim.mNodes = new ArrayList(); @@ -640,7 +650,7 @@ public final class AnimatorSet extends Animator { * @param dependencyAnimation the animation that sent the event. */ private void startIfReady(Animator dependencyAnimation) { - if (mAnimatorSet.mCanceled) { + if (mAnimatorSet.mTerminated) { // if the parent AnimatorSet was canceled, then don't start any dependent anims return; } @@ -676,11 +686,15 @@ public final class AnimatorSet extends Animator { } public void onAnimationCancel(Animator animation) { - if (mPlayingSet.size() == 0) { - if (mListeners != null) { - int numListeners = mListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mListeners.get(i).onAnimationCancel(mAnimatorSet); + if (!mTerminated) { + // Listeners are already notified of the AnimatorSet canceling in cancel(). + // The logic below only kicks in when animations end normally + if (mPlayingSet.size() == 0) { + if (mListeners != null) { + int numListeners = mListeners.size(); + for (int i = 0; i < numListeners; ++i) { + mListeners.get(i).onAnimationCancel(mAnimatorSet); + } } } } @@ -692,24 +706,28 @@ public final class AnimatorSet extends Animator { mPlayingSet.remove(animation); Node animNode = mAnimatorSet.mNodeMap.get(animation); animNode.done = true; - ArrayList sortedNodes = mAnimatorSet.mSortedNodes; - boolean allDone = true; - int numSortedNodes = sortedNodes.size(); - for (int i = 0; i < numSortedNodes; ++i) { - if (!sortedNodes.get(i).done) { - allDone = false; - break; + if (!mTerminated) { + // Listeners are already notified of the AnimatorSet ending in cancel() or + // end(); the logic below only kicks in when animations end normally + ArrayList sortedNodes = mAnimatorSet.mSortedNodes; + boolean allDone = true; + int numSortedNodes = sortedNodes.size(); + for (int i = 0; i < numSortedNodes; ++i) { + if (!sortedNodes.get(i).done) { + allDone = false; + break; + } } - } - if (allDone) { - // If this was the last child animation to end, then notify listeners that this - // AnimatorSet has ended - if (mListeners != null) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationEnd(mAnimatorSet); + if (allDone) { + // If this was the last child animation to end, then notify listeners that this + // AnimatorSet has ended + if (mListeners != null) { + ArrayList tmpListeners = + (ArrayList) mListeners.clone(); + int numListeners = tmpListeners.size(); + for (int i = 0; i < numListeners; ++i) { + tmpListeners.get(i).onAnimationEnd(mAnimatorSet); + } } } } @@ -791,6 +809,8 @@ public final class AnimatorSet extends Animator { } } } + // nodes are 'done' by default; they become un-done when started, and done + // again when ended node.done = false; } } diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index 1dcaa042d071d..90d676ea29041 100755 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -933,17 +933,17 @@ public class ValueAnimator extends Animator { @Override public void cancel() { - if (mListeners != null) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - for (AnimatorListener listener : tmpListeners) { - listener.onAnimationCancel(this); - } - } // Only cancel if the animation is actually running or has been started and is about // to run if (mPlayingState != STOPPED || sPendingAnimations.get().contains(this) || sDelayedAnims.get().contains(this)) { + if (mListeners != null) { + ArrayList tmpListeners = + (ArrayList) mListeners.clone(); + for (AnimatorListener listener : tmpListeners) { + listener.onAnimationCancel(this); + } + } endAnimation(); } } diff --git a/core/tests/coretests/Android.mk b/core/tests/coretests/Android.mk index 00f47facb5760..b2ebb08a8f6aa 100644 --- a/core/tests/coretests/Android.mk +++ b/core/tests/coretests/Android.mk @@ -12,7 +12,7 @@ LOCAL_SRC_FILES := \ $(call all-java-files-under, EnabledTestApp/src) LOCAL_DX_FLAGS := --core-library -LOCAL_STATIC_JAVA_LIBRARIES := core-tests android-common frameworks-core-util-lib mockwebserver +LOCAL_STATIC_JAVA_LIBRARIES := core-tests android-common frameworks-core-util-lib mockwebserver guava LOCAL_JAVA_LIBRARIES := android.test.runner LOCAL_PACKAGE_NAME := FrameworksCoreTests diff --git a/core/tests/coretests/AndroidManifest.xml b/core/tests/coretests/AndroidManifest.xml index 8e2d925ae908f..059503cc8883a 100644 --- a/core/tests/coretests/AndroidManifest.xml +++ b/core/tests/coretests/AndroidManifest.xml @@ -1235,6 +1235,13 @@ + + + + + + + + + +