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

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();
     }
 
     /**

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<AnimatorListener> tmpListeners =
+                    (ArrayList<AnimatorListener>) 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<AnimatorListener> tmpListeners =
-                        (ArrayList<AnimatorListener>) 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<AnimatorListener> tmpListeners =
                         (ArrayList<AnimatorListener>) 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<AnimatorListener> tmpListeners =
                     (ArrayList<AnimatorListener>) 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<AnimatorListener> tmpListeners =
-                    (ArrayList<AnimatorListener>) mListeners.clone();
-            int numListeners = tmpListeners.size();
-            for (int i = 0; i < numListeners; ++i) {
-                tmpListeners.get(i).onAnimationStart(this);
-            }
+            notifyStartListeners();
         }
     }
 

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) {