Make sure start() is called for custom animators

BUG: 34758281
Test: ag/1848837

Change-Id: Iae5d2fb41c4556ee201b9c4dfbc356996dc475a5

3 files changed, 67 insertions, 26 deletions

diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java
index 08ad976acb06..4ebcc446e5a2 100644
--- a/core/java/android/animation/Animator.java
+++ b/core/java/android/animation/Animator.java
@@ -26,7 +26,7 @@ import java.util.ArrayList;
  * This is the superclass for classes which provide basic support for animations which can be
  * started, ended, and have <code>AnimatorListeners</code> added to them.
  */
-public abstract class Animator implements Cloneable, AnimationHandler.AnimationFrameCallback {
+public abstract class Animator implements Cloneable {
 
     /**
      * The value used to indicate infinite duration (e.g. when Animators repeat infinitely).
@@ -464,24 +464,14 @@ public abstract class Animator implements Cloneable, AnimationHandler.AnimationF
         throw new IllegalStateException("Reverse is not supported");
     }
 
-    /**
-     * @hide
-     */
-    @Override
-    public boolean doAnimationFrame(long frameTime) {
+    // Pulse an animation frame into the animation.
+    boolean pulseAnimationFrame(long frameTime) {
         // TODO: Need to find a better signal than this. There's a bug in SystemUI that's preventing
         // returning !isStarted() from working.
         return false;
     }
 
     /**
-     * @hide
-     */
-    @Override
-    public void commitAnimationFrame(long frameTime) {}
-
-
-    /**
      * Internal use only.
      * This call starts the animation in regular or reverse direction without requiring them to
      * register frame callbacks. The caller will be responsible for all the subsequent animation
diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java
index 8aba4055a904..4e3b7d0739a9 100644
--- a/core/java/android/animation/AnimatorSet.java
+++ b/core/java/android/animation/AnimatorSet.java
@@ -948,7 +948,8 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim
         for (int i = 0; i < mPlayingSet.size(); i++) {
             Node node = mPlayingSet.get(i);
             if (!node.mEnded) {
-                node.mEnded = node.mAnimation.doAnimationFrame(getPlayTimeForNode(playTime, node));
+                node.mEnded = node.mAnimation.pulseAnimationFrame(
+                    getPlayTimeForNode(playTime, node));
             }
         }
 
@@ -978,6 +979,19 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim
     }
 
     /**
+     * @hide
+     */
+    @Override
+    public void commitAnimationFrame(long frameTime) {
+        // No op.
+    }
+
+    @Override
+    boolean pulseAnimationFrame(long frameTime) {
+        return doAnimationFrame(frameTime);
+    }
+
+    /**
      * When playing forward, we call start() at the animation's scheduled start time, and make sure
      * to pump a frame at the animation's scheduled end time.
      *
@@ -993,11 +1007,10 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim
                 if (event.mEvent == AnimationEvent.ANIMATION_END) {
                     mPlayingSet.add(event.mNode);
                     node.mAnimation.startWithoutPulsing(true);
-                    node.mAnimation.doAnimationFrame(0);
+                    pulseFrame(node, 0);
                 } else if (event.mEvent == AnimationEvent.ANIMATION_DELAY_ENDED && !node.mEnded) {
                     // end event:
-                    node.mEnded =
-                            node.mAnimation.doAnimationFrame(getPlayTimeForNode(playTime, node));
+                    pulseFrame(node, getPlayTimeForNode(playTime, node));
                 }
             }
         } else {
@@ -1007,16 +1020,21 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim
                 if (event.mEvent == AnimationEvent.ANIMATION_START) {
                     mPlayingSet.add(event.mNode);
                     node.mAnimation.startWithoutPulsing(false);
-                    node.mAnimation.doAnimationFrame(0);
+                    pulseFrame(node, 0);
                 } else if (event.mEvent == AnimationEvent.ANIMATION_END && !node.mEnded) {
                     // start event:
-                    node.mEnded =
-                            node.mAnimation.doAnimationFrame(getPlayTimeForNode(playTime, node));
+                    pulseFrame(node, getPlayTimeForNode(playTime, node));
                 }
             }
         }
     }
 
+    private void pulseFrame(Node node, long frameTime) {
+        if (!node.mEnded) {
+            node.mEnded = node.mAnimation.pulseAnimationFrame(frameTime);
+        }
+    }
+
     private long getPlayTimeForNode(long overallPlayTime, Node node) {
         return getPlayTimeForNode(overallPlayTime, node, mReversing);
     }
diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java
index 470523fc4252..55ac1f478e22 100644
--- a/core/java/android/animation/ValueAnimator.java
+++ b/core/java/android/animation/ValueAnimator.java
@@ -68,7 +68,7 @@ import java.util.HashMap;
  * </div>
  */
 @SuppressWarnings("unchecked")
-public class ValueAnimator extends Animator {
+public class ValueAnimator extends Animator implements AnimationHandler.AnimationFrameCallback {
     private static final String TAG = "ValueAnimator";
     private static final boolean DEBUG = false;
 
@@ -225,6 +225,12 @@ public class ValueAnimator extends Animator {
     private boolean mSelfPulse = true;
 
     /**
+     * Whether or not the animator has been requested to start without pulsing. This flag gets set
+     * in startWithoutPulsing(), and reset in start().
+     */
+    private boolean mSuppressSelfPulseRequested = false;
+
+    /**
      * The time interpolator to be used. The elapsed fraction of the animation will be passed
      * through this interpolator to calculate the interpolated fraction, which is then used to
      * calculate the animated values.
@@ -997,12 +1003,12 @@ public class ValueAnimator extends Animator {
      *
      * @param playBackwards Whether the ValueAnimator should start playing in reverse.
      */
-    private void start(boolean playBackwards, boolean selfPulse) {
+    private void start(boolean playBackwards) {
         if (Looper.myLooper() == null) {
             throw new AndroidRuntimeException("Animators may only be run on Looper threads");
         }
         mReversing = playBackwards;
-        mSelfPulse = selfPulse;
+        mSelfPulse = !mSuppressSelfPulseRequested;
         // Special case: reversing from seek-to-0 should act as if not seeked at all.
         if (playBackwards && mSeekFraction != -1 && mSeekFraction != 0) {
             if (mRepeatCount == INFINITE) {
@@ -1041,12 +1047,18 @@ public class ValueAnimator extends Animator {
     }
 
     void startWithoutPulsing(boolean inReverse) {
-        start(inReverse, false);
+        mSuppressSelfPulseRequested = true;
+        if (inReverse) {
+            reverse();
+        } else {
+            start();
+        }
+        mSuppressSelfPulseRequested = false;
     }
 
     @Override
     public void start() {
-        start(false, true);
+        start(false);
     }
 
     @Override
@@ -1150,7 +1162,7 @@ public class ValueAnimator extends Animator {
             mReversing = !mReversing;
             end();
         } else {
-            start(true, true);
+            start(true);
         }
     }
 
@@ -1364,6 +1376,11 @@ public class ValueAnimator extends Animator {
         animateValue(endFraction);
     }
 
+    @Override
+    boolean isInitialized() {
+        return mInitialized;
+    }
+
     /**
      * Processes a frame of the animation, adjusting the start time if needed.
      *
@@ -1430,6 +1447,20 @@ public class ValueAnimator extends Animator {
         return finished;
     }
 
+    @Override
+    boolean pulseAnimationFrame(long frameTime) {
+        if (mSelfPulse) {
+            // Pulse animation frame will *always* be after calling start(). If mSelfPulse isn't
+            // set to false at this point, that means child animators did not call super's start().
+            // This can happen when the Animator is just a non-animating wrapper around a real
+            // functional animation. In this case, we can't really pulse a frame into the animation,
+            // because the animation cannot necessarily be properly initialized (i.e. no start/end
+            // values set).
+            return false;
+        }
+        return doAnimationFrame(frameTime);
+    }
+
     private void addOneShotCommitCallback() {
         if (!mSelfPulse) {
             return;
@@ -1514,6 +1545,8 @@ public class ValueAnimator extends Animator {
         anim.mFirstFrameTime = -1;
         anim.mOverallFraction = 0;
         anim.mCurrentFraction = 0;
+        anim.mSelfPulse = true;
+        anim.mSuppressSelfPulseRequested = false;
 
         PropertyValuesHolder[] oldValues = mValues;
         if (oldValues != null) {