Skip to content

Commit 3d100d9

Browse files
chethaaseAndroid (Google) Code Review
authored andcommitted
Merge "Fix logic of animator start/cancel/end callbacks"
2 parents b9a6d4d + 17cf42c commit 3d100d9

File tree

3 files changed

+41
-22
lines changed

3 files changed

+41
-22
lines changed

core/java/android/animation/LayoutTransition.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,6 +1005,8 @@ public void endChangingAnimations() {
10051005
anim.start();
10061006
anim.end();
10071007
}
1008+
// listeners should clean up the currentChangingAnimations list, but just in case...
1009+
currentChangingAnimations.clear();
10081010
}
10091011

10101012
/**

core/java/android/animation/ValueAnimator.java

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,13 @@ public class ValueAnimator extends Animator {
149149
*/
150150
private boolean mStarted = false;
151151

152+
/**
153+
* Tracks whether we've notified listeners of the onAnimationSTart() event. This can be
154+
* complex to keep track of since we notify listeners at different times depending on
155+
* startDelay and whether start() was called before end().
156+
*/
157+
private boolean mStartListenersCalled = false;
158+
152159
/**
153160
* Flag that denotes whether the animation is set up and ready to go. Used to
154161
* set up animation that has not yet been started.
@@ -885,6 +892,18 @@ public void setEvaluator(TypeEvaluator value) {
885892
}
886893
}
887894

895+
private void notifyStartListeners() {
896+
if (mListeners != null && !mStartListenersCalled) {
897+
ArrayList<AnimatorListener> tmpListeners =
898+
(ArrayList<AnimatorListener>) mListeners.clone();
899+
int numListeners = tmpListeners.size();
900+
for (int i = 0; i < numListeners; ++i) {
901+
tmpListeners.get(i).onAnimationStart(this);
902+
}
903+
}
904+
mStartListenersCalled = true;
905+
}
906+
888907
/**
889908
* Start the animation playing. This version of start() takes a boolean flag that indicates
890909
* whether the animation should play in reverse. The flag is usually false, but may be set
@@ -914,15 +933,7 @@ private void start(boolean playBackwards) {
914933
setCurrentPlayTime(getCurrentPlayTime());
915934
mPlayingState = STOPPED;
916935
mRunning = true;
917-
918-
if (mListeners != null) {
919-
ArrayList<AnimatorListener> tmpListeners =
920-
(ArrayList<AnimatorListener>) mListeners.clone();
921-
int numListeners = tmpListeners.size();
922-
for (int i = 0; i < numListeners; ++i) {
923-
tmpListeners.get(i).onAnimationStart(this);
924-
}
925-
}
936+
notifyStartListeners();
926937
}
927938
animationHandler.sendEmptyMessage(ANIMATION_START);
928939
}
@@ -941,7 +952,11 @@ public void cancel() {
941952
|| handler.mPendingAnimations.contains(this)
942953
|| handler.mDelayedAnims.contains(this)) {
943954
// Only notify listeners if the animator has actually started
944-
if (mRunning && mListeners != null) {
955+
if ((mStarted || mRunning) && mListeners != null) {
956+
if (!mRunning) {
957+
// If it's not yet running, then start listeners weren't called. Call them now.
958+
notifyStartListeners();
959+
}
945960
ArrayList<AnimatorListener> tmpListeners =
946961
(ArrayList<AnimatorListener>) mListeners.clone();
947962
for (AnimatorListener listener : tmpListeners) {
@@ -959,6 +974,7 @@ public void end() {
959974
// Special case if the animation has not yet started; get it ready for ending
960975
mStartedDelay = false;
961976
startAnimation(handler);
977+
mStarted = true;
962978
} else if (!mInitialized) {
963979
initAnimation();
964980
}
@@ -1010,7 +1026,11 @@ private void endAnimation(AnimationHandler handler) {
10101026
handler.mPendingAnimations.remove(this);
10111027
handler.mDelayedAnims.remove(this);
10121028
mPlayingState = STOPPED;
1013-
if (mRunning && mListeners != null) {
1029+
if ((mStarted || mRunning) && mListeners != null) {
1030+
if (!mRunning) {
1031+
// If it's not yet running, then start listeners weren't called. Call them now.
1032+
notifyStartListeners();
1033+
}
10141034
ArrayList<AnimatorListener> tmpListeners =
10151035
(ArrayList<AnimatorListener>) mListeners.clone();
10161036
int numListeners = tmpListeners.size();
@@ -1020,6 +1040,7 @@ private void endAnimation(AnimationHandler handler) {
10201040
}
10211041
mRunning = false;
10221042
mStarted = false;
1043+
mStartListenersCalled = false;
10231044
}
10241045

10251046
/**
@@ -1032,12 +1053,7 @@ private void startAnimation(AnimationHandler handler) {
10321053
if (mStartDelay > 0 && mListeners != null) {
10331054
// Listeners were already notified in start() if startDelay is 0; this is
10341055
// just for delayed animations
1035-
ArrayList<AnimatorListener> tmpListeners =
1036-
(ArrayList<AnimatorListener>) mListeners.clone();
1037-
int numListeners = tmpListeners.size();
1038-
for (int i = 0; i < numListeners; ++i) {
1039-
tmpListeners.get(i).onAnimationStart(this);
1040-
}
1056+
notifyStartListeners();
10411057
}
10421058
}
10431059

core/tests/coretests/src/android/animation/EventsTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,15 @@ public void onAnimationCancel(Animator animation) {
173173
// This should only be called on an animation that has been started and not
174174
// yet canceled or ended
175175
assertFalse(mCanceled);
176-
assertTrue(mRunning);
177-
assertTrue(mStarted);
176+
assertTrue(mRunning || mStarted);
178177
mCanceled = true;
179178
}
180179

181180
@Override
182181
public void onAnimationEnd(Animator animation) {
183182
// This should only be called on an animation that has been started and not
184183
// yet ended
185-
assertTrue(mRunning);
186-
assertTrue(mStarted);
184+
assertTrue(mRunning || mStarted);
187185
mRunning = false;
188186
mStarted = false;
189187
super.onAnimationEnd(animation);
@@ -210,11 +208,12 @@ public void testCancel() throws Exception {
210208
}
211209

212210
/**
213-
* Verify that calling end on an unstarted animator does nothing.
211+
* Verify that calling end on an unstarted animator starts/ends an animator.
214212
*/
215213
@UiThreadTest
216214
@SmallTest
217215
public void testEnd() throws Exception {
216+
mRunning = true; // end() implicitly starts an unstarted animator
218217
mAnimator.end();
219218
}
220219

@@ -496,6 +495,7 @@ public void run() {
496495
mRunning = true;
497496
mAnimator.start();
498497
mAnimator.end();
498+
mRunning = true; // end() implicitly starts an unstarted animator
499499
mAnimator.end();
500500
mFuture.release();
501501
} catch (junit.framework.AssertionFailedError e) {
@@ -544,6 +544,7 @@ public void run() {
544544
mRunning = true;
545545
mAnimator.start();
546546
mAnimator.end();
547+
mRunning = true; // end() implicitly starts an unstarted animator
547548
mAnimator.end();
548549
mFuture.release();
549550
} catch (junit.framework.AssertionFailedError e) {

0 commit comments

Comments
 (0)