From 139b27a456662b3a624428da6fff9eda006cec12 Mon Sep 17 00:00:00 2001 From: George Mount Date: Wed, 15 Feb 2023 16:28:34 -0800 Subject: [PATCH 1/4] Make Animators pass internal tests Fixes: 269524599 AnimatorSet wasn't properly managing cloning and ViewPropertyAnimatorTest didn't work when the animator took too long to start. This CL fixes both problems. Test: ran animation tests (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:9cbebf9fb2066cf8765ba794a9181c6e6fca2ca4) Merged-In: Icdf2e9d75d93fa6a4e99833c24a40b48b051f568 Change-Id: Icdf2e9d75d93fa6a4e99833c24a40b48b051f568 --- core/java/android/animation/AnimatorSet.java | 1 + .../src/android/animation/ViewPropertyAnimatorTest.java | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index 257adfe390d66..a700efb7234b1 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -1503,6 +1503,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim anim.mNodeMap = new ArrayMap(); anim.mNodes = new ArrayList(nodeCount); anim.mEvents = new ArrayList(); + anim.mStartListenersCalled = false; anim.mAnimationEndListener = new AnimatorListenerAdapter() { @Override public void onAnimationEnd(Animator animation) { diff --git a/core/tests/coretests/src/android/animation/ViewPropertyAnimatorTest.java b/core/tests/coretests/src/android/animation/ViewPropertyAnimatorTest.java index 81cd4da4f425e..8cc88ea230a15 100644 --- a/core/tests/coretests/src/android/animation/ViewPropertyAnimatorTest.java +++ b/core/tests/coretests/src/android/animation/ViewPropertyAnimatorTest.java @@ -135,11 +135,15 @@ public class ViewPropertyAnimatorTest { * @throws Exception */ @Before - public void setUp() throws Exception { + public void setUp() throws Throwable { final BasicAnimatorActivity activity = mActivityRule.getActivity(); Button button = activity.findViewById(R.id.animatingButton); mAnimator = button.animate().x(100).y(100); + mActivityRule.runOnUiThread(() -> { + mAnimator.start(); + mAnimator.cancel(); + }); // mListener is the main testing mechanism of this file. The asserts of each test // are embedded in the listener callbacks that it implements. From fd41f0f638bcca0a47de57562fe48c5f4601fec8 Mon Sep 17 00:00:00 2001 From: George Mount Date: Wed, 15 Feb 2023 15:38:48 -0800 Subject: [PATCH 2/4] Call pause() and resume() on child animators. Fixes: 269529338 When AnimatorSet was paused or resumed, the child animators were not being paused or resumed. This CL makes fixeds that. This CL also updates the iteration over children so that temporary allocations are reduced. A cached array of items are saved and reused for the next iteration when possible. Test: new tests (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:218395a67afc6a16a53b5bef8d7c6fa764bed9b4) Merged-In: I40fa2115099116cfe9320b0587948f08a8650f91 Change-Id: I40fa2115099116cfe9320b0587948f08a8650f91 --- core/java/android/animation/Animator.java | 115 ++++++-- core/java/android/animation/AnimatorSet.java | 49 ++-- .../java/android/animation/ValueAnimator.java | 45 +--- .../animation/AnimatorSetCallsTest.java | 251 ++++++++++++++++++ 4 files changed, 378 insertions(+), 82 deletions(-) create mode 100644 core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index d0ce701334146..21ba403d332ac 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -71,6 +71,13 @@ public abstract class Animator implements Cloneable { */ private static long sBackgroundPauseDelay = 1000; + /** + * A cache of the values in a list. Used so that when calling the list, we have a copy + * of it in case the list is modified while iterating. The array can be reused to avoid + * allocation on every notification. + */ + private Object[] mCachedList; + /** * Sets the duration for delaying pausing animators when apps go into the background. * Used by AnimationHandler when requested to pause animators. @@ -160,14 +167,7 @@ public abstract class Animator implements Cloneable { public void pause() { if (isStarted() && !mPaused) { mPaused = true; - if (mPauseListeners != null) { - ArrayList tmpListeners = - (ArrayList) mPauseListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationPause(this); - } - } + notifyPauseListeners(AnimatorCaller.ON_PAUSE); } } @@ -184,14 +184,7 @@ public abstract class Animator implements Cloneable { public void resume() { if (mPaused) { mPaused = false; - if (mPauseListeners != null) { - ArrayList tmpListeners = - (ArrayList) mPauseListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationResume(this); - } - } + notifyPauseListeners(AnimatorCaller.ON_RESUME); } } @@ -450,6 +443,7 @@ public abstract class Animator implements Cloneable { if (mPauseListeners != null) { anim.mPauseListeners = new ArrayList(mPauseListeners); } + anim.mCachedList = null; return anim; } catch (CloneNotSupportedException e) { throw new AssertionError(); @@ -590,6 +584,70 @@ public abstract class Animator implements Cloneable { } } + /** + * Calls notification for each AnimatorListener. + * + * @param notification The notification method to call on each listener. + * @param isReverse When this is used with start/end, this is the isReverse parameter. For + * other calls, this is ignored. + */ + void notifyListeners( + AnimatorCaller notification, + boolean isReverse + ) { + callOnList(mListeners, notification, this, isReverse); + } + + /** + * Call pause/resume on each AnimatorPauseListener. + * + * @param notification Either ON_PAUSE or ON_RESUME to call onPause or onResume on each + * listener. + */ + void notifyPauseListeners(AnimatorCaller notification) { + callOnList(mPauseListeners, notification, this, false); + } + + /** + * Calls call for every item in list with animator and + * isReverse as parameters. + * + * @param list The list of items to make calls on. + * @param call The method to call for each item in list. + * @param animator The animator parameter of call. + * @param isReverse The isReverse parameter of call. + * @param The item type of list + * @param The Animator type of animator. + */ + void callOnList( + ArrayList list, + AnimatorCaller call, + A animator, + boolean isReverse + ) { + int size = list == null ? 0 : list.size(); + if (size > 0) { + // Try to reuse mCacheList to store the items of list. + Object[] array; + if (mCachedList == null || mCachedList.length < size) { + array = new Object[size]; + } else { + array = mCachedList; + // Clear it in case there is some reentrancy + mCachedList = null; + } + list.toArray(array); + for (int i = 0; i < size; i++) { + //noinspection unchecked + T item = (T) array[i]; + call.call(item, animator, isReverse); + array[i] = null; + } + // Store it for the next call so we can reuse this array, if needed. + mCachedList = array; + } + } + /** *

An animation listener receives notifications from an animation. * Notifications indicate animation related events, such as the end or the @@ -748,4 +806,29 @@ public abstract class Animator implements Cloneable { return clone; } } + + /** + * Internally used by {@link #callOnList(ArrayList, AnimatorCaller, Object, boolean)} to + * make a call on all children of a list. This can be for start, stop, pause, cancel, update, + * etc notifications. + * + * @param The type of listener to make the call on + * @param The type of animator that is passed as a parameter + */ + interface AnimatorCaller { + void call(T listener, A animator, boolean isReverse); + + AnimatorCaller ON_START = AnimatorListener::onAnimationStart; + AnimatorCaller ON_END = AnimatorListener::onAnimationEnd; + AnimatorCaller ON_CANCEL = + (listener, animator, isReverse) -> listener.onAnimationCancel(animator); + AnimatorCaller ON_REPEAT = + (listener, animator, isReverse) -> listener.onAnimationRepeat(animator); + AnimatorCaller ON_PAUSE = + (listener, animator, isReverse) -> listener.onAnimationPause(animator); + AnimatorCaller ON_RESUME = + (listener, animator, isReverse) -> listener.onAnimationResume(animator); + AnimatorCaller ON_UPDATE = + (listener, animator, isReverse) -> listener.onAnimationUpdate(animator); + } } diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index a700efb7234b1..09eec9d25a4bd 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -32,6 +32,7 @@ import java.util.Collection; import java.util.Comparator; import java.util.HashMap; import java.util.List; +import java.util.function.Consumer; /** * This class plays a set of {@link Animator} objects in the specified order. Animations @@ -424,24 +425,28 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } if (isStarted()) { - ArrayList tmpListeners = null; - if (mListeners != null) { - tmpListeners = (ArrayList) mListeners.clone(); - int size = tmpListeners.size(); - for (int i = 0; i < size; i++) { - tmpListeners.get(i).onAnimationCancel(this); - } - } - ArrayList playingSet = new ArrayList<>(mPlayingSet); - int setSize = playingSet.size(); - for (int i = 0; i < setSize; i++) { - playingSet.get(i).mAnimation.cancel(); - } + notifyListeners(AnimatorCaller.ON_CANCEL, false); + callOnPlayingSet(Animator::cancel); mPlayingSet.clear(); endAnimation(); } } + /** + * Calls consumer on every Animator of mPlayingSet. + * + * @param consumer The method to call on every Animator of mPlayingSet. + */ + private void callOnPlayingSet(Consumer consumer) { + final ArrayList list = mPlayingSet; + final int size = list.size(); + //noinspection ForLoopReplaceableByForEach + for (int i = 0; i < size; i++) { + final Animator animator = list.get(i).mAnimation; + consumer.accept(animator); + } + } + // Force all the animations to end when the duration scale is 0. private void forceToEnd() { if (mEndCanBeCalled) { @@ -662,6 +667,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim super.pause(); if (!previouslyPaused && mPaused) { mPauseTime = -1; + callOnPlayingSet(Animator::pause); } } @@ -676,6 +682,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim if (mPauseTime >= 0) { addAnimationCallback(0); } + callOnPlayingSet(Animator::resume); } } @@ -751,26 +758,14 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim private void notifyStartListeners(boolean inReverse) { if (mListeners != null && !mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - AnimatorListener listener = tmpListeners.get(i); - listener.onAnimationStart(this, inReverse); - } + notifyListeners(AnimatorCaller.ON_START, inReverse); } mStartListenersCalled = true; } private void notifyEndListeners(boolean inReverse) { if (mListeners != null && mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - AnimatorListener listener = tmpListeners.get(i); - listener.onAnimationEnd(this, inReverse); - } + notifyListeners(AnimatorCaller.ON_END, inReverse); } mStartListenersCalled = false; } diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index 7009725aa32bc..46d9847c2a94e 100644 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -1110,24 +1110,14 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio private void notifyStartListeners(boolean isReversing) { if (mListeners != null && !mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationStart(this, isReversing); - } + notifyListeners(AnimatorCaller.ON_START, isReversing); } mStartListenersCalled = true; } private void notifyEndListeners(boolean isReversing) { if (mListeners != null && mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationEnd(this, isReversing); - } + notifyListeners(AnimatorCaller.ON_END, isReversing); } mStartListenersCalled = false; } @@ -1224,15 +1214,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio // If it's not yet running, then start listeners weren't called. Call them now. notifyStartListeners(mReversing); } - int listenersSize = mListeners.size(); - if (listenersSize > 0) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - for (int i = 0; i < listenersSize; i++) { - AnimatorListener listener = tmpListeners.get(i); - listener.onAnimationCancel(this); - } - } + notifyListeners(AnimatorCaller.ON_CANCEL, false); } endAnimation(); @@ -1435,12 +1417,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio done = true; } else if (newIteration && !lastIterationFinished) { // Time to repeat - if (mListeners != null) { - int numListeners = mListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mListeners.get(i).onAnimationRepeat(this); - } - } + notifyListeners(AnimatorCaller.ON_REPEAT, false); } else if (lastIterationFinished) { done = true; } @@ -1494,12 +1471,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio lastIteration = Math.min(lastIteration, mRepeatCount); if (iteration != lastIteration) { - if (mListeners != null) { - int numListeners = mListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mListeners.get(i).onAnimationRepeat(this); - } - } + notifyListeners(AnimatorCaller.ON_REPEAT, false); } } @@ -1697,12 +1669,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio for (int i = 0; i < numValues; ++i) { mValues[i].calculateValue(fraction); } - if (mUpdateListeners != null) { - int numListeners = mUpdateListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mUpdateListeners.get(i).onAnimationUpdate(this); - } - } + callOnList(mUpdateListeners, AnimatorCaller.ON_UPDATE, this, false); } @Override diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java new file mode 100644 index 0000000000000..fda08ff1e6773 --- /dev/null +++ b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java @@ -0,0 +1,251 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.animation; + +import static org.junit.Assert.assertEquals; + +import android.util.PollingCheck; +import android.view.View; + +import androidx.test.ext.junit.rules.ActivityScenarioRule; +import androidx.test.filters.MediumTest; + +import com.android.frameworks.coretests.R; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +@MediumTest +public class AnimatorSetCallsTest { + @Rule + public final ActivityScenarioRule mRule = + new ActivityScenarioRule<>(AnimatorSetActivity.class); + + private AnimatorSetActivity mActivity; + private AnimatorSet mSet1; + private CountListener mListener1; + private CountListener mListener2; + private CountListener mListener3; + + @Before + public void setUp() throws Exception { + mRule.getScenario().onActivity((activity) -> { + mActivity = activity; + View square = mActivity.findViewById(R.id.square1); + + mSet1 = new AnimatorSet(); + mListener1 = new CountListener(); + mSet1.addListener(mListener1); + mSet1.addPauseListener(mListener1); + + AnimatorSet set2 = new AnimatorSet(); + mListener2 = new CountListener(); + set2.addListener(mListener2); + set2.addPauseListener(mListener2); + + ObjectAnimator anim = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); + mListener3 = new CountListener(); + anim.addListener(mListener3); + anim.addPauseListener(mListener3); + anim.setDuration(1); + + set2.play(anim); + mSet1.play(set2); + }); + } + + @Test + public void startEndCalledOnChildren() { + mRule.getScenario().onActivity((a) -> mSet1.start()); + waitForOnUiThread(() -> mListener1.endForward > 0); + + // only startForward and endForward should have been called once + mListener1.assertValues( + 1, 0, 1, 0, 0, 0, 0, 0 + ); + mListener2.assertValues( + 1, 0, 1, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 1, 0, 1, 0, 0, 0, 0, 0 + ); + } + + @Test + public void cancelCalledOnChildren() { + mRule.getScenario().onActivity((a) -> { + mSet1.start(); + mSet1.cancel(); + }); + waitForOnUiThread(() -> mListener1.endForward > 0); + + // only startForward and endForward should have been called once + mListener1.assertValues( + 1, 0, 1, 0, 1, 0, 0, 0 + ); + mListener2.assertValues( + 1, 0, 1, 0, 1, 0, 0, 0 + ); + mListener3.assertValues( + 1, 0, 1, 0, 1, 0, 0, 0 + ); + } + + @Test + public void startEndReversedCalledOnChildren() { + mRule.getScenario().onActivity((a) -> mSet1.reverse()); + waitForOnUiThread(() -> mListener1.endReverse > 0); + + // only startForward and endForward should have been called once + mListener1.assertValues( + 0, 1, 0, 1, 0, 0, 0, 0 + ); + mListener2.assertValues( + 0, 1, 0, 1, 0, 0, 0, 0 + ); + mListener3.assertValues( + 0, 1, 0, 1, 0, 0, 0, 0 + ); + } + + @Test + public void pauseResumeCalledOnChildren() { + mRule.getScenario().onActivity((a) -> { + mSet1.start(); + mSet1.pause(); + }); + waitForOnUiThread(() -> mListener1.pause > 0); + + // only startForward and pause should have been called once + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + + mRule.getScenario().onActivity((a) -> mSet1.resume()); + waitForOnUiThread(() -> mListener1.endForward > 0); + + // resume and endForward should have been called once + mListener1.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + mListener2.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + mListener3.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + } + + private void waitForOnUiThread(PollingCheck.PollingCheckCondition condition) { + final boolean[] value = new boolean[1]; + PollingCheck.waitFor(() -> { + mActivity.runOnUiThread(() -> value[0] = condition.canProceed()); + return value[0]; + }); + } + + private static class CountListener implements Animator.AnimatorListener, + Animator.AnimatorPauseListener { + public int startNoParam; + public int endNoParam; + public int startReverse; + public int startForward; + public int endForward; + public int endReverse; + public int cancel; + public int repeat; + public int pause; + public int resume; + + public void assertValues( + int startForward, + int startReverse, + int endForward, + int endReverse, + int cancel, + int repeat, + int pause, + int resume + ) { + assertEquals(0, startNoParam); + assertEquals(0, endNoParam); + assertEquals(startForward, this.startForward); + assertEquals(startReverse, this.startReverse); + assertEquals(endForward, this.endForward); + assertEquals(endReverse, this.endReverse); + assertEquals(cancel, this.cancel); + assertEquals(repeat, this.repeat); + assertEquals(pause, this.pause); + assertEquals(resume, this.resume); + } + + @Override + public void onAnimationStart(Animator animation, boolean isReverse) { + if (isReverse) { + startReverse++; + } else { + startForward++; + } + } + + @Override + public void onAnimationEnd(Animator animation, boolean isReverse) { + if (isReverse) { + endReverse++; + } else { + endForward++; + } + } + + @Override + public void onAnimationStart(Animator animation) { + startNoParam++; + } + + @Override + public void onAnimationEnd(Animator animation) { + endNoParam++; + } + + @Override + public void onAnimationCancel(Animator animation) { + cancel++; + } + + @Override + public void onAnimationRepeat(Animator animation) { + repeat++; + } + + @Override + public void onAnimationPause(Animator animation) { + pause++; + } + + @Override + public void onAnimationResume(Animator animation) { + resume++; + } + } +} From c720d0af3063595c18e51af3eb11dbc7ae14a8fe Mon Sep 17 00:00:00 2001 From: George Mount Date: Fri, 17 Feb 2023 11:39:19 -0800 Subject: [PATCH 3/4] Don't notify updates while initializing AnimatorSet Fixes: 269785397 When an AnimatorSet runs, it should not call onAnimationUpdate() during initialization. Test: new test (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:90eabef83b63d42f73233a9c025850999f0d8175) Merged-In: Ic53d346b17c06a8d9ae9af8c28681dd84cc630fc Change-Id: Ic53d346b17c06a8d9ae9af8c28681dd84cc630fc --- .../java/android/animation/ValueAnimator.java | 6 +- .../animation/AnimatorSetCallsTest.java | 60 +++++++++++++++++-- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index 46d9847c2a94e..719f596949bb2 100644 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -1470,7 +1470,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio iteration = Math.min(iteration, mRepeatCount); lastIteration = Math.min(lastIteration, mRepeatCount); - if (iteration != lastIteration) { + if (notify && iteration != lastIteration) { notifyListeners(AnimatorCaller.ON_REPEAT, false); } } @@ -1669,7 +1669,9 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio for (int i = 0; i < numValues; ++i) { mValues[i].calculateValue(fraction); } - callOnList(mUpdateListeners, AnimatorCaller.ON_UPDATE, this, false); + if (mSeekFraction >= 0 || mStartListenersCalled) { + callOnList(mUpdateListeners, AnimatorCaller.ON_UPDATE, this, false); + } } @Override diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java index fda08ff1e6773..22da0aa4597e6 100644 --- a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java +++ b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java @@ -30,6 +30,8 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import java.util.ArrayList; + @MediumTest public class AnimatorSetCallsTest { @Rule @@ -38,6 +40,7 @@ public class AnimatorSetCallsTest { private AnimatorSetActivity mActivity; private AnimatorSet mSet1; + private ObjectAnimator mAnimator; private CountListener mListener1; private CountListener mListener2; private CountListener mListener3; @@ -58,13 +61,13 @@ public class AnimatorSetCallsTest { set2.addListener(mListener2); set2.addPauseListener(mListener2); - ObjectAnimator anim = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); + mAnimator = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); mListener3 = new CountListener(); - anim.addListener(mListener3); - anim.addPauseListener(mListener3); - anim.setDuration(1); + mAnimator.addListener(mListener3); + mAnimator.addPauseListener(mListener3); + mAnimator.setDuration(1); - set2.play(anim); + set2.play(mAnimator); mSet1.play(set2); }); } @@ -157,6 +160,53 @@ public class AnimatorSetCallsTest { ); } + @Test + public void updateOnlyWhileChangingValues() { + ArrayList updateValues = new ArrayList<>(); + mAnimator.addUpdateListener(new ValueAnimator.AnimatorUpdateListener() { + @Override + public void onAnimationUpdate(ValueAnimator animation) { + updateValues.add((Float) animation.getAnimatedValue()); + } + }); + + mSet1.setCurrentPlayTime(0); + + assertEquals(1, updateValues.size()); + assertEquals(0f, updateValues.get(0), 0f); + } + @Test + public void updateOnlyWhileRunning() { + ArrayList updateValues = new ArrayList<>(); + mAnimator.addUpdateListener(new ValueAnimator.AnimatorUpdateListener() { + @Override + public void onAnimationUpdate(ValueAnimator animation) { + updateValues.add((Float) animation.getAnimatedValue()); + } + }); + + mRule.getScenario().onActivity((a) -> { + mSet1.start(); + }); + + waitForOnUiThread(() -> mListener1.endForward > 0); + + // the duration is only 1ms, so there should only be two values, 0 and 100. + assertEquals(0f, updateValues.get(0), 0f); + assertEquals(100f, updateValues.get(updateValues.size() - 1), 0f); + + // now check all the values in the middle, which can never go from 100->0. + boolean isAtEnd = false; + for (int i = 1; i < updateValues.size() - 1; i++) { + float actual = updateValues.get(i); + if (actual == 100f) { + isAtEnd = true; + } + float expected = isAtEnd ? 100f : 0f; + assertEquals(expected, actual, 0f); + } + } + private void waitForOnUiThread(PollingCheck.PollingCheckCondition condition) { final boolean[] value = new boolean[1]; PollingCheck.waitFor(() -> { From 3e832d273bbccce80776a4b11c2ed323b3602243 Mon Sep 17 00:00:00 2001 From: George Mount Date: Tue, 14 Mar 2023 20:37:18 +0000 Subject: [PATCH 4/4] Revert^2 "AnimatorSet sends pause/resume for seeked animators" Fixed reentrancy issue. Test: New tests for reentrancy. Ran old tests. b4915629d783902ba915b58b1e3b4f2cba204d1b (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:1817208a10f56539f7fbc54ed20d5ff95d46c9af) Merged-In: Idb83b101eee8f7ec38e5b0f7d685fd41695796f2 Change-Id: Idb83b101eee8f7ec38e5b0f7d685fd41695796f2 Fixes: 270709315 --- core/java/android/animation/Animator.java | 28 +- core/java/android/animation/AnimatorSet.java | 52 ++-- .../java/android/animation/ValueAnimator.java | 33 +-- .../animation/AnimatorSetActivityTest.java | 8 +- .../animation/AnimatorSetCallsTest.java | 255 ++++++++++++++++-- .../android/animation/ValueAnimatorTests.java | 60 +++++ 6 files changed, 362 insertions(+), 74 deletions(-) diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index 21ba403d332ac..12026aa3f72a6 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -78,6 +78,13 @@ public abstract class Animator implements Cloneable { */ private Object[] mCachedList; + /** + * 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(). + */ + boolean mStartListenersCalled = false; + /** * Sets the duration for delaying pausing animators when apps go into the background. * Used by AnimationHandler when requested to pause animators. @@ -165,7 +172,9 @@ public abstract class Animator implements Cloneable { * @see AnimatorPauseListener */ public void pause() { - if (isStarted() && !mPaused) { + // We only want to pause started Animators or animators that setCurrentPlayTime() + // have been called on. mStartListenerCalled will be true if seek has happened. + if ((isStarted() || mStartListenersCalled) && !mPaused) { mPaused = true; notifyPauseListeners(AnimatorCaller.ON_PAUSE); } @@ -444,6 +453,7 @@ public abstract class Animator implements Cloneable { anim.mPauseListeners = new ArrayList(mPauseListeners); } anim.mCachedList = null; + anim.mStartListenersCalled = false; return anim; } catch (CloneNotSupportedException e) { throw new AssertionError(); @@ -608,6 +618,22 @@ public abstract class Animator implements Cloneable { callOnList(mPauseListeners, notification, this, false); } + void notifyStartListeners(boolean isReversing) { + boolean startListenersCalled = mStartListenersCalled; + mStartListenersCalled = true; + if (mListeners != null && !startListenersCalled) { + notifyListeners(AnimatorCaller.ON_START, isReversing); + } + } + + void notifyEndListeners(boolean isReversing) { + boolean startListenersCalled = mStartListenersCalled; + mStartListenersCalled = false; + if (mListeners != null && startListenersCalled) { + notifyListeners(AnimatorCaller.ON_END, isReversing); + } + } + /** * Calls call for every item in list with animator and * isReverse as parameters. diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index 09eec9d25a4bd..60659dc12342c 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -189,11 +189,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim */ private long[] mChildStartAndStopTimes; - /** - * Tracks whether we've notified listeners of the onAnimationStart() event. - */ - private boolean mStartListenersCalled; - // This is to work around a bug in b/34736819. This needs to be removed once app team // fixes their side. private AnimatorListenerAdapter mAnimationEndListener = new AnimatorListenerAdapter() { @@ -424,7 +419,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim if (Looper.myLooper() == null) { throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } - if (isStarted()) { + if (isStarted() || mStartListenersCalled) { notifyListeners(AnimatorCaller.ON_CANCEL, false); callOnPlayingSet(Animator::cancel); mPlayingSet.clear(); @@ -486,13 +481,13 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim return; } if (isStarted()) { + mStarted = false; // don't allow reentrancy // Iterate the animations that haven't finished or haven't started, and end them. if (mReversing) { // Between start() and first frame, mLastEventId would be unset (i.e. -1) mLastEventId = mLastEventId == -1 ? mEvents.size() : mLastEventId; - while (mLastEventId > 0) { - mLastEventId = mLastEventId - 1; - AnimationEvent event = mEvents.get(mLastEventId); + for (int eventId = mLastEventId - 1; eventId >= 0; eventId--) { + AnimationEvent event = mEvents.get(eventId); Animator anim = event.mNode.mAnimation; if (mNodeMap.get(anim).mEnded) { continue; @@ -508,11 +503,10 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim } } } else { - while (mLastEventId < mEvents.size() - 1) { + for (int eventId = mLastEventId + 1; eventId < mEvents.size(); eventId++) { // Avoid potential reentrant loop caused by child animators manipulating // AnimatorSet's lifecycle (i.e. not a recommended approach). - mLastEventId = mLastEventId + 1; - AnimationEvent event = mEvents.get(mLastEventId); + AnimationEvent event = mEvents.get(eventId); Animator anim = event.mNode.mAnimation; if (mNodeMap.get(anim).mEnded) { continue; @@ -527,7 +521,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim } } } - mPlayingSet.clear(); } endAnimation(); } @@ -723,6 +716,10 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim if (Looper.myLooper() == null) { throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } + if (inReverse == mReversing && selfPulse == mSelfPulse && mStarted) { + // It is already started + return; + } mStarted = true; mSelfPulse = selfPulse; mPaused = false; @@ -756,20 +753,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim } } - private void notifyStartListeners(boolean inReverse) { - if (mListeners != null && !mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_START, inReverse); - } - mStartListenersCalled = true; - } - - private void notifyEndListeners(boolean inReverse) { - if (mListeners != null && mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_END, inReverse); - } - mStartListenersCalled = false; - } - // Returns true if set is empty or contains nothing but animator sets with no start delay. private static boolean isEmptySet(AnimatorSet set) { if (set.getStartDelay() > 0) { @@ -936,12 +919,18 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim lastPlayTime - node.mStartTime, notify ); + if (notify) { + mPlayingSet.remove(node); + } } else if (start <= currentPlayTime && currentPlayTime <= end) { animator.animateSkipToEnds( currentPlayTime - node.mStartTime, lastPlayTime - node.mStartTime, notify ); + if (notify && !mPlayingSet.contains(node)) { + mPlayingSet.add(node); + } } } } @@ -969,12 +958,18 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim lastPlayTime - node.mStartTime, notify ); + if (notify) { + mPlayingSet.remove(node); + } } else if (start <= currentPlayTime && currentPlayTime <= end) { animator.animateSkipToEnds( currentPlayTime - node.mStartTime, lastPlayTime - node.mStartTime, notify ); + if (notify && !mPlayingSet.contains(node)) { + mPlayingSet.add(node); + } } } } @@ -1115,8 +1110,8 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim mSeekState.setPlayTime(0, mReversing); } } - animateBasedOnPlayTime(playTime, lastPlayTime, mReversing, true); mSeekState.setPlayTime(playTime, mReversing); + animateBasedOnPlayTime(playTime, lastPlayTime, mReversing, true); } /** @@ -1498,7 +1493,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim anim.mNodeMap = new ArrayMap(); anim.mNodes = new ArrayList(nodeCount); anim.mEvents = new ArrayList(); - anim.mStartListenersCalled = false; anim.mAnimationEndListener = new AnimatorListenerAdapter() { @Override public void onAnimationEnd(Animator animation) { diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index 719f596949bb2..5d69f8b80799a 100644 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -198,13 +198,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio */ 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. @@ -1108,20 +1101,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio } } - private void notifyStartListeners(boolean isReversing) { - if (mListeners != null && !mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_START, isReversing); - } - mStartListenersCalled = true; - } - - private void notifyEndListeners(boolean isReversing) { - if (mListeners != null && mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_END, isReversing); - } - mStartListenersCalled = false; - } - /** * 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 @@ -1139,6 +1118,10 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio if (Looper.myLooper() == null) { throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } + if (playBackwards == mResumed && mSelfPulse == !mSuppressSelfPulseRequested && mStarted) { + // already started + return; + } mReversing = playBackwards; mSelfPulse = !mSuppressSelfPulseRequested; // Special case: reversing from seek-to-0 should act as if not seeked at all. @@ -1209,7 +1192,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio // Only cancel if the animation is actually running or has been started and is about // to run // Only notify listeners if the animator has actually started - if ((mStarted || mRunning) && mListeners != null) { + if ((mStarted || mRunning || mStartListenersCalled) && mListeners != null) { if (!mRunning) { // If it's not yet running, then start listeners weren't called. Call them now. notifyStartListeners(mReversing); @@ -1217,7 +1200,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio notifyListeners(AnimatorCaller.ON_CANCEL, false); } endAnimation(); - } @Override @@ -1320,11 +1302,11 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio // If it's not yet running, then start listeners weren't called. Call them now. notifyStartListeners(mReversing); } - mRunning = false; - mStarted = false; mLastFrameTime = -1; mFirstFrameTime = -1; mStartTime = -1; + mRunning = false; + mStarted = false; notifyEndListeners(mReversing); // mReversing needs to be reset *after* notifying the listeners for the end callbacks. mReversing = false; @@ -1687,7 +1669,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio anim.mRunning = false; anim.mPaused = false; anim.mResumed = false; - anim.mStartListenersCalled = false; anim.mStartTime = -1; anim.mStartTimeCommitted = false; anim.mAnimationEndRequested = false; diff --git a/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java b/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java index 7a1de0c5d4fe4..a7538701807a3 100644 --- a/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java +++ b/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java @@ -435,9 +435,11 @@ public class AnimatorSetActivityTest { mActivityRule.runOnUiThread(s::start); while (!listener.endIsCalled) { - boolean passedStartDelay = a1.isStarted() || a2.isStarted() || a3.isStarted() || - a4.isStarted() || a5.isStarted(); - assertEquals(passedStartDelay, s.isRunning()); + mActivityRule.runOnUiThread(() -> { + boolean passedStartDelay = a1.isStarted() || a2.isStarted() || a3.isStarted() + || a4.isStarted() || a5.isStarted(); + assertEquals(passedStartDelay, s.isRunning()); + }); Thread.sleep(50); } assertFalse(s.isRunning()); diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java index 22da0aa4597e6..43266a51502b3 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.assertTrue; import android.util.PollingCheck; import android.view.View; @@ -31,6 +32,8 @@ import org.junit.Rule; import org.junit.Test; import java.util.ArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; @MediumTest public class AnimatorSetCallsTest { @@ -40,6 +43,7 @@ public class AnimatorSetCallsTest { private AnimatorSetActivity mActivity; private AnimatorSet mSet1; + private AnimatorSet mSet2; private ObjectAnimator mAnimator; private CountListener mListener1; private CountListener mListener2; @@ -56,10 +60,10 @@ public class AnimatorSetCallsTest { mSet1.addListener(mListener1); mSet1.addPauseListener(mListener1); - AnimatorSet set2 = new AnimatorSet(); + mSet2 = new AnimatorSet(); mListener2 = new CountListener(); - set2.addListener(mListener2); - set2.addPauseListener(mListener2); + mSet2.addListener(mListener2); + mSet2.addPauseListener(mListener2); mAnimator = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); mListener3 = new CountListener(); @@ -67,8 +71,8 @@ public class AnimatorSetCallsTest { mAnimator.addPauseListener(mListener3); mAnimator.setDuration(1); - set2.play(mAnimator); - mSet1.play(set2); + mSet2.play(mAnimator); + mSet1.play(mSet2); }); } @@ -175,6 +179,7 @@ public class AnimatorSetCallsTest { assertEquals(1, updateValues.size()); assertEquals(0f, updateValues.get(0), 0f); } + @Test public void updateOnlyWhileRunning() { ArrayList updateValues = new ArrayList<>(); @@ -207,6 +212,226 @@ public class AnimatorSetCallsTest { } } + @Test + public void pauseResumeSeekingAnimators() { + ValueAnimator animator2 = ValueAnimator.ofFloat(0f, 1f); + mSet2.play(animator2).after(mAnimator); + mSet2.setStartDelay(100); + mSet1.setStartDelay(100); + mAnimator.setDuration(100); + + mActivity.runOnUiThread(() -> { + mSet1.setCurrentPlayTime(0); + mSet1.pause(); + + // only startForward and pause should have been called once + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener2.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + mListener2.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + + mSet1.setCurrentPlayTime(200); + + // resume and endForward should have been called once + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 0, 0 + ); + + mSet1.pause(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 2, 1 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 2, 2 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + + // now go to animator2 + mSet1.setCurrentPlayTime(400); + mSet1.pause(); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 3, 3 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 2, 2 + ); + mListener3.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + + // now go back to mAnimator + mSet1.setCurrentPlayTime(250); + mSet1.pause(); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 4, 4 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 3, 3 + ); + mListener3.assertValues( + 1, 1, 1, 0, 0, 0, 2, 2 + ); + + // now go back to before mSet2 was being run + mSet1.setCurrentPlayTime(1); + mSet1.pause(); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 5, 5 + ); + mListener2.assertValues( + 1, 0, 0, 1, 0, 0, 3, 3 + ); + mListener3.assertValues( + 1, 1, 1, 1, 0, 0, 2, 2 + ); + }); + } + + @Test + public void endInCancel() throws Throwable { + AnimatorListenerAdapter listener = new AnimatorListenerAdapter() { + @Override + public void onAnimationCancel(Animator animation) { + mSet1.end(); + } + }; + mSet1.addListener(listener); + mActivity.runOnUiThread(() -> { + mSet1.start(); + mSet1.cancel(); + // Should go to the end value + View square = mActivity.findViewById(R.id.square1); + assertEquals(100f, square.getTranslationX(), 0.001f); + }); + } + + @Test + public void reentrantStart() throws Throwable { + CountDownLatch latch = new CountDownLatch(3); + AnimatorListenerAdapter listener = new AnimatorListenerAdapter() { + @Override + public void onAnimationStart(Animator animation, boolean isReverse) { + mSet1.start(); + latch.countDown(); + } + }; + mSet1.addListener(listener); + mSet2.addListener(listener); + mAnimator.addListener(listener); + mActivity.runOnUiThread(() -> mSet1.start()); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread hasn't been destroyed by a stack overflow... + mActivity.runOnUiThread(() -> {}); + } + + @Test + public void reentrantEnd() throws Throwable { + CountDownLatch latch = new CountDownLatch(3); + AnimatorListenerAdapter listener = new AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(Animator animation, boolean isReverse) { + mSet1.end(); + latch.countDown(); + } + }; + mSet1.addListener(listener); + mSet2.addListener(listener); + mAnimator.addListener(listener); + mActivity.runOnUiThread(() -> { + mSet1.start(); + mSet1.end(); + }); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread hasn't been destroyed by a stack overflow... + mActivity.runOnUiThread(() -> {}); + } + + @Test + public void reentrantPause() throws Throwable { + CountDownLatch latch = new CountDownLatch(3); + AnimatorListenerAdapter listener = new AnimatorListenerAdapter() { + @Override + public void onAnimationPause(Animator animation) { + mSet1.pause(); + latch.countDown(); + } + }; + mSet1.addPauseListener(listener); + mSet2.addPauseListener(listener); + mAnimator.addPauseListener(listener); + mActivity.runOnUiThread(() -> { + mSet1.start(); + mSet1.pause(); + }); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread hasn't been destroyed by a stack overflow... + mActivity.runOnUiThread(() -> {}); + } + + @Test + public void reentrantResume() throws Throwable { + CountDownLatch latch = new CountDownLatch(3); + AnimatorListenerAdapter listener = new AnimatorListenerAdapter() { + @Override + public void onAnimationResume(Animator animation) { + mSet1.resume(); + latch.countDown(); + } + }; + mSet1.addPauseListener(listener); + mSet2.addPauseListener(listener); + mAnimator.addPauseListener(listener); + mActivity.runOnUiThread(() -> { + mSet1.start(); + mSet1.pause(); + mSet1.resume(); + }); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread hasn't been destroyed by a stack overflow... + mActivity.runOnUiThread(() -> {}); + } + private void waitForOnUiThread(PollingCheck.PollingCheckCondition condition) { final boolean[] value = new boolean[1]; PollingCheck.waitFor(() -> { @@ -238,16 +463,16 @@ public class AnimatorSetCallsTest { int pause, int resume ) { - assertEquals(0, startNoParam); - assertEquals(0, endNoParam); - assertEquals(startForward, this.startForward); - assertEquals(startReverse, this.startReverse); - assertEquals(endForward, this.endForward); - assertEquals(endReverse, this.endReverse); - assertEquals(cancel, this.cancel); - assertEquals(repeat, this.repeat); - assertEquals(pause, this.pause); - assertEquals(resume, this.resume); + assertEquals("onAnimationStart() without direction", 0, startNoParam); + assertEquals("onAnimationEnd() without direction", 0, endNoParam); + assertEquals("onAnimationStart(forward)", startForward, this.startForward); + assertEquals("onAnimationStart(reverse)", startReverse, this.startReverse); + assertEquals("onAnimationEnd(forward)", endForward, this.endForward); + assertEquals("onAnimationEnd(reverse)", endReverse, this.endReverse); + assertEquals("onAnimationCancel()", cancel, this.cancel); + assertEquals("onAnimationRepeat()", repeat, this.repeat); + assertEquals("onAnimationPause()", pause, this.pause); + assertEquals("onAnimationResume()", resume, this.resume); } @Override diff --git a/core/tests/coretests/src/android/animation/ValueAnimatorTests.java b/core/tests/coretests/src/android/animation/ValueAnimatorTests.java index dee0a3ecdbe00..a53d57f0383c2 100644 --- a/core/tests/coretests/src/android/animation/ValueAnimatorTests.java +++ b/core/tests/coretests/src/android/animation/ValueAnimatorTests.java @@ -40,6 +40,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import java.util.ArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; @RunWith(AndroidJUnit4.class) @MediumTest @@ -1067,6 +1069,64 @@ public class ValueAnimatorTests { }); } + @Test + public void reentrantStart() throws Throwable { + CountDownLatch latch = new CountDownLatch(1); + a1.addListener(new AnimatorListenerAdapter() { + @Override + public void onAnimationStart(Animator animation, boolean isReverse) { + a1.start(); + latch.countDown(); + } + }); + mActivityRule.runOnUiThread(() -> a1.start()); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread isn't blocked by an infinite loop: + mActivityRule.runOnUiThread(() -> {}); + } + + @Test + public void reentrantPause() throws Throwable { + CountDownLatch latch = new CountDownLatch(1); + a1.addPauseListener(new AnimatorListenerAdapter() { + @Override + public void onAnimationPause(Animator animation) { + a1.pause(); + latch.countDown(); + } + }); + mActivityRule.runOnUiThread(() -> { + a1.start(); + a1.pause(); + }); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread isn't blocked by an infinite loop: + mActivityRule.runOnUiThread(() -> {}); + } + + @Test + public void reentrantResume() throws Throwable { + CountDownLatch latch = new CountDownLatch(1); + a1.addPauseListener(new AnimatorListenerAdapter() { + @Override + public void onAnimationResume(Animator animation) { + a1.resume(); + latch.countDown(); + } + }); + mActivityRule.runOnUiThread(() -> { + a1.start(); + a1.pause(); + a1.resume(); + }); + assertTrue(latch.await(1, TimeUnit.SECONDS)); + + // Make sure that the UI thread isn't blocked by an infinite loop: + mActivityRule.runOnUiThread(() -> {}); + } + class MyUpdateListener implements ValueAnimator.AnimatorUpdateListener { boolean wasRunning = false; long firstRunningFrameTime = -1;