Fix cancellation of AnimatorSet when child animation has delay

Previously, AnimatorSet incorrectly checked whether child animations were
'running' to figure out what to cancel. If a child animation was started, but
sitting in a startDelay phase, it was not 'running', so the right cancel/end
events would not propagate.

The fix is to add a new isStarted() API to Animator, which returns true when
the animator has started (but not yet ended), regardless of whether the animator
has a startDelay or not. It's basically a superset of the existing isRunning()
method, which only returns true when an animator has actually started setting values.

Change-Id: I126814cb6637b58295b6d18d9b155235671f99be

3 files changed, 73 insertions, 20 deletions

diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java
index 57e0583..e01fa1a 100644
--- a/core/java/android/animation/Animator.java
+++ b/core/java/android/animation/Animator.java
@@ -111,12 +111,29 @@ public abstract class Animator implements Cloneable {
     public abstract void setInterpolator(TimeInterpolator value);
 
     /**
-     * Returns whether this Animator is currently running (having been started and not yet ended).
+     * Returns whether this Animator is currently running (having been started and gone past any
+     * initial startDelay period and not yet ended).
+     *
      * @return Whether the Animator is running.
      */
     public abstract boolean isRunning();
 
     /**
+     * Returns whether this Animator has been started and not yet ended. This state is a superset
+     * of the state of {@link #isRunning()}, because an Animator with a nonzero
+     * {@link #getStartDelay() startDelay} will return true for {@link #isStarted()} during the
+     * delay phase, whereas {@link #isRunning()} will return true only after the delay phase
+     * is complete.
+     *
+     * @return Whether the Animator has been started and not yet ended.
+     */
+    public boolean isStarted() {
+        // Default method returns value for isRunning(). Subclasses should override to return a
+        // real value.
+        return isRunning();
+    }
+
+    /**
      * Adds a listener to the set of listeners that are sent events through the life of an
      * animation, such as start, repeat, and end.
      *
diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java
index ce3dd13..0b68dd8 100644
--- a/core/java/android/animation/AnimatorSet.java
+++ b/core/java/android/animation/AnimatorSet.java
@@ -95,6 +95,12 @@ public final class AnimatorSet extends Animator {
      */
     boolean mTerminated = false;
 
+    /**
+     * Indicates whether an AnimatorSet has been start()'d, whether or
+     * not there is a nonzero startDelay.
+     */
+    private boolean mStarted = false;
+
     // The amount of time in ms to delay starting the animation after start() is called
     private long mStartDelay = 0;
 
@@ -267,14 +273,14 @@ public final class AnimatorSet extends Animator {
     /**
      * {@inheritDoc}
      *
-     * <p>Note that canceling a <code>AnimatorSet</code> also cancels all of the animations that it is
-     * responsible for.</p>
+     * <p>Note that canceling a <code>AnimatorSet</code> also cancels all of the animations that it
+     * is responsible for.</p>
      */
     @SuppressWarnings("unchecked")
     @Override
     public void cancel() {
         mTerminated = true;
-        if (isRunning()) {
+        if (isStarted()) {
             ArrayList<AnimatorListener> tmpListeners = null;
             if (mListeners != null) {
                 tmpListeners = (ArrayList<AnimatorListener>) mListeners.clone();
@@ -296,6 +302,7 @@ public final class AnimatorSet extends Animator {
                     listener.onAnimationEnd(this);
                 }
             }
+            mStarted = false;
         }
     }
 
@@ -308,7 +315,7 @@ public final class AnimatorSet extends Animator {
     @Override
     public void end() {
         mTerminated = true;
-        if (isRunning()) {
+        if (isStarted()) {
             if (mSortedNodes.size() != mNodes.size()) {
                 // hasn't been started yet - sort the nodes now, then end them
                 sortNodes();
@@ -334,12 +341,13 @@ public final class AnimatorSet extends Animator {
                     listener.onAnimationEnd(this);
                 }
             }
+            mStarted = false;
         }
     }
 
     /**
-     * Returns true if any of the child animations of this AnimatorSet have been started and have not
-     * yet ended.
+     * Returns true if any of the child animations of this AnimatorSet have been started and have
+     * not yet ended.
      * @return Whether this AnimatorSet has been started and has not yet ended.
      */
     @Override
@@ -349,8 +357,12 @@ public final class AnimatorSet extends Animator {
                 return true;
             }
         }
-        // Also return true if we're currently running the startDelay animator
-        return (mDelayAnim != null && mDelayAnim.isRunning());
+        return false;
+    }
+
+    @Override
+    public boolean isStarted() {
+        return mStarted;
     }
 
     /**
@@ -435,6 +447,7 @@ public final class AnimatorSet extends Animator {
     @Override
     public void start() {
         mTerminated = false;
+        mStarted = true;
 
         // First, sort the nodes (if necessary). This will ensure that sortedNodes
         // contains the animation nodes in the correct order.
@@ -514,9 +527,17 @@ public final class AnimatorSet extends Animator {
             int numListeners = tmpListeners.size();
             for (int i = 0; i < numListeners; ++i) {
                 tmpListeners.get(i).onAnimationStart(this);
-                if (mNodes.size() == 0) {
-                    // Handle unusual case where empty AnimatorSet is started - should send out
-                    // end event immediately since the event will not be sent out at all otherwise
+            }
+        }
+        if (mNodes.size() == 0 && mStartDelay == 0) {
+            // Handle unusual case where empty AnimatorSet is started - should send out
+            // end event immediately since the event will not be sent out at all otherwise
+            mStarted = false;
+            if (mListeners != null) {
+                ArrayList<AnimatorListener> tmpListeners =
+                        (ArrayList<AnimatorListener>) mListeners.clone();
+                int numListeners = tmpListeners.size();
+                for (int i = 0; i < numListeners; ++i) {
                     tmpListeners.get(i).onAnimationEnd(this);
                 }
             }
@@ -536,6 +557,7 @@ public final class AnimatorSet extends Animator {
          */
         anim.mNeedsSort = true;
         anim.mTerminated = false;
+        anim.mStarted = false;
         anim.mPlayingSet = new ArrayList<Animator>();
         anim.mNodeMap = new HashMap<Animator, Node>();
         anim.mNodes = new ArrayList<Node>();
@@ -732,6 +754,7 @@ public final class AnimatorSet extends Animator {
                             tmpListeners.get(i).onAnimationEnd(mAnimatorSet);
                         }
                     }
+                    mAnimatorSet.mStarted = false;
                 }
             }
         }
@@ -936,9 +959,9 @@ public final class AnimatorSet extends Animator {
      * The <code>Builder</code> object is a utility class to facilitate adding animations to a
      * <code>AnimatorSet</code> along with the relationships between the various animations. The
      * intention of the <code>Builder</code> methods, along with the {@link
-     * AnimatorSet#play(Animator) play()} method of <code>AnimatorSet</code> is to make it possible to
-     * express the dependency relationships of animations in a natural way. Developers can also use
-     * the {@link AnimatorSet#playTogether(Animator[]) playTogether()} and {@link
+     * AnimatorSet#play(Animator) play()} method of <code>AnimatorSet</code> is to make it possible
+     * to express the dependency relationships of animations in a natural way. Developers can also
+     * use the {@link AnimatorSet#playTogether(Animator[]) playTogether()} and {@link
      * AnimatorSet#playSequentially(Animator[]) playSequentially()} methods if these suit the need,
      * but it might be easier in some situations to express the AnimatorSet of animations in pairs.
      * <p/>
diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java
index c22306a..5df8bdc 100755
--- a/core/java/android/animation/ValueAnimator.java
+++ b/core/java/android/animation/ValueAnimator.java
@@ -193,6 +193,12 @@ public class ValueAnimator extends Animator {
      * Note that delayed animations are different: they are not started until their first
      * animation frame, which occurs after their delay elapses.
      */
+    private boolean mRunning = false;
+
+    /**
+     * Additional playing state to indicate whether an animator has been start()'d, whether or
+     * not there is a nonzero startDelay.
+     */
     private boolean mStarted = false;
 
     /**
@@ -628,7 +634,7 @@ public class ValueAnimator extends Animator {
                         for (int i = 0; i < numReadyAnims; ++i) {
                             ValueAnimator anim = readyAnims.get(i);
                             anim.startAnimation();
-                            anim.mStarted = true;
+                            anim.mRunning = true;
                             delayedAnims.remove(anim);
                         }
                         readyAnims.clear();
@@ -913,13 +919,14 @@ public class ValueAnimator extends Animator {
         mPlayingBackwards = playBackwards;
         mCurrentIteration = 0;
         mPlayingState = STOPPED;
+        mStarted = true;
         mStartedDelay = false;
         sPendingAnimations.get().add(this);
         if (mStartDelay == 0) {
             // This sets the initial value of the animation, prior to actually starting it running
             setCurrentPlayTime(getCurrentPlayTime());
             mPlayingState = STOPPED;
-            mStarted = true;
+            mRunning = true;
 
             if (mListeners != null) {
                 ArrayList<AnimatorListener> tmpListeners =
@@ -950,7 +957,7 @@ public class ValueAnimator extends Animator {
         if (mPlayingState != STOPPED || sPendingAnimations.get().contains(this) ||
                 sDelayedAnims.get().contains(this)) {
             // Only notify listeners if the animator has actually started
-            if (mStarted && mListeners != null) {
+            if (mRunning && mListeners != null) {
                 ArrayList<AnimatorListener> tmpListeners =
                         (ArrayList<AnimatorListener>) mListeners.clone();
                 for (AnimatorListener listener : tmpListeners) {
@@ -982,7 +989,12 @@ public class ValueAnimator extends Animator {
 
     @Override
     public boolean isRunning() {
-        return (mPlayingState == RUNNING || mStarted);
+        return (mPlayingState == RUNNING || mRunning);
+    }
+
+    @Override
+    public boolean isStarted() {
+        return mStarted;
     }
 
     /**
@@ -1013,7 +1025,7 @@ public class ValueAnimator extends Animator {
         sPendingAnimations.get().remove(this);
         sDelayedAnims.get().remove(this);
         mPlayingState = STOPPED;
-        if (mStarted && mListeners != null) {
+        if (mRunning && mListeners != null) {
             ArrayList<AnimatorListener> tmpListeners =
                     (ArrayList<AnimatorListener>) mListeners.clone();
             int numListeners = tmpListeners.size();
@@ -1021,6 +1033,7 @@ public class ValueAnimator extends Animator {
                 tmpListeners.get(i).onAnimationEnd(this);
             }
         }
+        mRunning = false;
         mStarted = false;
     }