Only set `mEnded` to true if `pulseAnimationFrame` returns true

Previously this can set the value to false if `pulseAnimationFrame` returns false but the animation has already called its end listener and ended the animation, this would result in the value of `mEnded` being overwritten from true back to false and the `AnimatorSet` running indefinitely.

This can happen with an `ObjectAnimator` that is referencing a target that has been garbage collected, in this case `ObjectAnimator` will immediately cancel itself when animating values, which happens as part of `pulseAnimationFrame`, however the return value from `pulseAnimationFrame` is based only on the duration of the animation so even though the `ObjectAnimator` has ended by cancelling itself `pulseAnimationFrame` will return false as the current time has not exceeded the play time of the animator.

Change-Id: I08c7790346b092ad3cd8938bbe02973148ac108e

2 files changed, 18 insertions, 2 deletions

diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java
index f33d2991329a..4823f447f22b 100644
--- a/core/java/android/animation/AnimatorSet.java
+++ b/core/java/android/animation/AnimatorSet.java
@@ -1311,8 +1311,9 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim
         if (!node.mEnded) {
             float durationScale = ValueAnimator.getDurationScale();
             durationScale = durationScale == 0  ? 1 : durationScale;
-            node.mEnded = node.mAnimation.pulseAnimationFrame(
-                    (long) (animPlayTime * durationScale));
+            if (node.mAnimation.pulseAnimationFrame((long) (animPlayTime * durationScale))) {
+                node.mEnded = true;
+            }
         }
     }
 
diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java
index 43266a51502b..cb3f99c37a4f 100644
--- a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java
+++ b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java
@@ -17,6 +17,7 @@
 package android.animation;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import android.util.PollingCheck;
@@ -343,6 +344,20 @@ public class AnimatorSetCallsTest {
     }
 
     @Test
+    public void childAnimatorCancelsDuringUpdate_animatorSetIsEnded() throws Throwable {
+        mAnimator.addUpdateListener(new ValueAnimator.AnimatorUpdateListener() {
+            @Override
+            public void onAnimationUpdate(ValueAnimator animation) {
+                animation.cancel();
+            }
+        });
+        mActivity.runOnUiThread(() -> {
+            mSet1.start();
+            assertFalse(mSet1.isRunning());
+        });
+    }
+
+    @Test
     public void reentrantStart() throws Throwable {
         CountDownLatch latch = new CountDownLatch(3);
         AnimatorListenerAdapter listener = new AnimatorListenerAdapter() {