From a172b3f15bf9a459271762109b90b19d686bba96 Mon Sep 17 00:00:00 2001 From: Paul Sowden Date: Tue, 12 Dec 2023 07:39:20 +0000 Subject: 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 --- core/java/android/animation/AnimatorSet.java | 5 +++-- .../src/android/animation/AnimatorSetCallsTest.java | 15 +++++++++++++++ 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; @@ -342,6 +343,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); -- cgit v1.2.3-59-g8ed1b