Fix some wrong-thread issues around animator management
Bug: 17372309
Fixes a case where UI thread and RT thread both used the same method
which wasn't safe for either of them.
Adds additional assertions & logging in unusual circumstances to
try and track down where the issue is occuring from.
Change-Id: I93d31a6fd0c5927259b67bdf96a475944226eee6
diff --git a/libs/hwui/AnimationContext.cpp b/libs/hwui/AnimationContext.cpp
index d7d9743..716dcf5 100644
--- a/libs/hwui/AnimationContext.cpp
+++ b/libs/hwui/AnimationContext.cpp
@@ -31,11 +31,14 @@
}
AnimationContext::~AnimationContext() {
+}
+
+void AnimationContext::destroy() {
startFrame();
while (mCurrentFrameAnimations.mNextHandle) {
AnimationHandle* current = mCurrentFrameAnimations.mNextHandle;
AnimatorManager& animators = current->mRenderNode->animators();
- animators.endAllAnimators();
+ animators.endAllActiveAnimators();
LOG_ALWAYS_FATAL_IF(mCurrentFrameAnimations.mNextHandle == current,
"endAllAnimators failed to remove from current frame list!");
}
diff --git a/libs/hwui/AnimationContext.h b/libs/hwui/AnimationContext.h
index 900d953..9b3d5f4 100644
--- a/libs/hwui/AnimationContext.h
+++ b/libs/hwui/AnimationContext.h
@@ -98,6 +98,8 @@
ANDROID_API virtual void callOnFinished(BaseRenderNodeAnimator* animator, AnimationListener* listener);
+ ANDROID_API virtual void destroy();
+
private:
friend class AnimationHandle;
void addAnimationHandle(AnimationHandle* handle);
diff --git a/libs/hwui/Animator.cpp b/libs/hwui/Animator.cpp
index da65f38..4f3a573 100644
--- a/libs/hwui/Animator.cpp
+++ b/libs/hwui/Animator.cpp
@@ -161,6 +161,13 @@
return false;
}
+void BaseRenderNodeAnimator::forceEndNow(AnimationContext& context) {
+ if (mPlayState < FINISHED) {
+ mPlayState = FINISHED;
+ callOnFinishedListener(context);
+ }
+}
+
void BaseRenderNodeAnimator::callOnFinishedListener(AnimationContext& context) {
if (mListener.get()) {
context.callOnFinished(this, mListener.get());
diff --git a/libs/hwui/Animator.h b/libs/hwui/Animator.h
index c52a93f..1ab6235b 100644
--- a/libs/hwui/Animator.h
+++ b/libs/hwui/Animator.h
@@ -68,6 +68,8 @@
ANDROID_API virtual uint32_t dirtyMask() = 0;
+ void forceEndNow(AnimationContext& context);
+
protected:
BaseRenderNodeAnimator(float finalValue);
virtual ~BaseRenderNodeAnimator();
diff --git a/libs/hwui/AnimatorManager.cpp b/libs/hwui/AnimatorManager.cpp
index 678b1ee..e06d800 100644
--- a/libs/hwui/AnimatorManager.cpp
+++ b/libs/hwui/AnimatorManager.cpp
@@ -49,6 +49,9 @@
void AnimatorManager::setAnimationHandle(AnimationHandle* handle) {
LOG_ALWAYS_FATAL_IF(mAnimationHandle && handle, "Already have an AnimationHandle!");
mAnimationHandle = handle;
+ LOG_ALWAYS_FATAL_IF(!mAnimationHandle && mAnimators.size(),
+ "Lost animation handle on %p (%s) with outstanding animators!",
+ &mParent, mParent.getName());
}
template<typename T>
@@ -62,6 +65,9 @@
void AnimatorManager::pushStaging() {
if (mNewAnimators.size()) {
+ LOG_ALWAYS_FATAL_IF(!mAnimationHandle,
+ "Trying to start new animators on %p (%s) without an animation handle!",
+ &mParent, mParent.getName());
// Since this is a straight move, we don't need to inc/dec the ref count
move_all(mNewAnimators, mAnimators);
}
@@ -128,22 +134,7 @@
return functor.dirtyMask;
}
-class EndAnimatorsFunctor {
-public:
- EndAnimatorsFunctor(AnimationContext& context) : mContext(context) {}
-
- void operator() (BaseRenderNodeAnimator* animator) {
- animator->end();
- animator->pushStaging(mContext);
- animator->animate(mContext);
- animator->decStrong(0);
- }
-
-private:
- AnimationContext& mContext;
-};
-
-static void endAnimatorsHard(BaseRenderNodeAnimator* animator) {
+static void endStagingAnimator(BaseRenderNodeAnimator* animator) {
animator->end();
if (animator->listener()) {
animator->listener()->onAnimationFinished(animator);
@@ -151,24 +142,34 @@
animator->decStrong(0);
}
-void AnimatorManager::endAllAnimators() {
- if (mNewAnimators.size()) {
- // Since this is a straight move, we don't need to inc/dec the ref count
- move_all(mNewAnimators, mAnimators);
+void AnimatorManager::endAllStagingAnimators() {
+ ALOGD("endAllStagingAnimators on %p (%s)", &mParent, mParent.getName());
+ // This works because this state can only happen on the UI thread,
+ // which means we're already on the right thread to invoke listeners
+ for_each(mNewAnimators.begin(), mNewAnimators.end(), endStagingAnimator);
+ mNewAnimators.clear();
+}
+
+class EndActiveAnimatorsFunctor {
+public:
+ EndActiveAnimatorsFunctor(AnimationContext& context) : mContext(context) {}
+
+ void operator() (BaseRenderNodeAnimator* animator) {
+ animator->forceEndNow(mContext);
+ animator->decStrong(0);
}
- // First try gracefully ending them
- if (mAnimationHandle) {
- EndAnimatorsFunctor functor(mAnimationHandle->context());
- for_each(mAnimators.begin(), mAnimators.end(), functor);
- mAnimators.clear();
- mAnimationHandle->release();
- } else {
- // We have no context, so bust out the sledgehammer
- // This works because this state can only happen on the UI thread,
- // which means we're already on the right thread to invoke listeners
- for_each(mAnimators.begin(), mAnimators.end(), endAnimatorsHard);
- mAnimators.clear();
- }
+
+private:
+ AnimationContext& mContext;
+};
+
+void AnimatorManager::endAllActiveAnimators() {
+ ALOGD("endAllStagingAnimators on %p (%s) with handle %p",
+ &mParent, mParent.getName(), mAnimationHandle);
+ EndActiveAnimatorsFunctor functor(mAnimationHandle->context());
+ for_each(mAnimators.begin(), mAnimators.end(), functor);
+ mAnimators.clear();
+ mAnimationHandle->release();
}
} /* namespace uirenderer */
diff --git a/libs/hwui/AnimatorManager.h b/libs/hwui/AnimatorManager.h
index d5f56ed..d03d427 100644
--- a/libs/hwui/AnimatorManager.h
+++ b/libs/hwui/AnimatorManager.h
@@ -50,8 +50,12 @@
void animateNoDamage(TreeInfo& info);
- // Hard-ends all animators. Used for cleanup if the root is being destroyed.
- ANDROID_API void endAllAnimators();
+ // Hard-ends all animators. May only be called on the UI thread.
+ ANDROID_API void endAllStagingAnimators();
+
+ // Hard-ends all animators that have been pushed. Used for cleanup if
+ // the ActivityContext is being destroyed
+ void endAllActiveAnimators();
bool hasAnimators() { return mAnimators.size(); }
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 967cb6f..428e426 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -59,6 +59,7 @@
stopDrawing();
freePrefetechedLayers();
destroyHardwareResources();
+ mAnimationContext->destroy();
if (mCanvas) {
delete mCanvas;
mCanvas = 0;