Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert^2 "AnimatorSet sends pause/resume for seeked animators"

Browse Source 
Fixed reentrancy issue.

Test: New tests for reentrancy. Ran old tests.

b4915629d7
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:1817208a10f56539f7fbc54ed20d5ff95d46c9af)
Merged-In: Idb83b101eee8f7ec38e5b0f7d685fd41695796f2
Change-Id: Idb83b101eee8f7ec38e5b0f7d685fd41695796f2
Fixes: 270709315
This commit is contained in:
George Mount
2023-03-14 20:37:18 +00:00
parent c720d0af30
commit 3e832d273b
6 changed files with 362 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
core/java/android/animation/Animator.java
View File

@@ -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<AnimatorPauseListener>(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 <code>call</code> for every item in <code>list</code> with <code>animator</code> and
* <code>isReverse</code> as parameters.

52
core/java/android/animation/AnimatorSet.java
View File

@@ -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<Animator, Node>();
anim.mNodes = new ArrayList<Node>(nodeCount);
anim.mEvents = new ArrayList<AnimationEvent>();
anim.mStartListenersCalled = false;
anim.mAnimationEndListener = new AnimatorListenerAdapter() {
@Override
public void onAnimationEnd(Animator animation) {

33
core/java/android/animation/ValueAnimator.java
View File

@@ -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;

8
core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java
View File

@@ -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());

255
core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java
View File

@@ -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<Float> 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

60
core/tests/coretests/src/android/animation/ValueAnimatorTests.java
View File

@@ -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;