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/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 */