Make animators more robust against ending mid-stream

The logic in the frame processing code of ValueAnimator did not handle
the situation of animators being ended while the current animation list
was being processed. In particular, if a call to an animation update
(which could result in a call out to user code) caused that animation, or
other current animators, to end, then there was the risk of running off the
end of the list of current animators.

The fix is to work from a copy of the current animator list, processing frames
on each one only if they also exist in the real animations list.

Issue #6992223 Frequent System UI crash

Change-Id: I742964558f8354f04c311b7b51c7686f26a4dacf

1 files changed, 11 insertions, 18 deletions

diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java
index 107e980..f874d56 100755
--- a/core/java/android/animation/ValueAnimator.java
+++ b/core/java/android/animation/ValueAnimator.java
@@ -536,6 +536,9 @@ public class ValueAnimator extends Animator {
         // The per-thread list of all active animations
         private final ArrayList<ValueAnimator> mAnimations = new ArrayList<ValueAnimator>();
 
+        // Used in doAnimationFrame() to avoid concurrent modifications of mAnimations
+        private final ArrayList<ValueAnimator> mTmpAnimations = new ArrayList<ValueAnimator>();
+
         // The per-thread set of animations to be started on the next animation frame
         private final ArrayList<ValueAnimator> mPendingAnimations = new ArrayList<ValueAnimator>();
 
@@ -605,28 +608,18 @@ public class ValueAnimator extends Animator {
             // Now process all active animations. The return value from animationFrame()
             // tells the handler whether it should now be ended
             int numAnims = mAnimations.size();
-            int i = 0;
-            while (i < numAnims) {
-                ValueAnimator anim = mAnimations.get(i);
-                if (anim.doAnimationFrame(frameTime)) {
+            for (int i = 0; i < numAnims; ++i) {
+                mTmpAnimations.add(mAnimations.get(i));
+            }
+            for (int i = 0; i < numAnims; ++i) {
+                ValueAnimator anim = mTmpAnimations.get(i);
+                if (mAnimations.contains(anim) && anim.doAnimationFrame(frameTime)) {
                     mEndingAnims.add(anim);
                 }
-                if (mAnimations.size() == numAnims) {
-                    ++i;
-                } else {
-                    // An animation might be canceled or ended by client code
-                    // during the animation frame. Check to see if this happened by
-                    // seeing whether the current index is the same as it was before
-                    // calling animationFrame(). Another approach would be to copy
-                    // animations to a temporary list and process that list instead,
-                    // but that entails garbage and processing overhead that would
-                    // be nice to avoid.
-                    --numAnims;
-                    mEndingAnims.remove(anim);
-                }
             }
+            mTmpAnimations.clear();
             if (mEndingAnims.size() > 0) {
-                for (i = 0; i < mEndingAnims.size(); ++i) {
+                for (int i = 0; i < mEndingAnims.size(); ++i) {
                     mEndingAnims.get(i).endAnimation(this);
                 }
                 mEndingAnims.clear();