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

1 files changed, 32 insertions, 31 deletions

diff --git a/libs/hwui/AnimatorManager.cpp b/libs/hwui/AnimatorManager.cpp
index 678b1eedbd96..e06d80044f4c 100644
--- a/libs/hwui/AnimatorManager.cpp
+++ b/libs/hwui/AnimatorManager.cpp
@@ -49,6 +49,9 @@ void AnimatorManager::addAnimator(const sp<BaseRenderNodeAnimator>& animator) {
 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 @@ static void move_all(T& source, T& dest) {
 
 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,14 +134,28 @@ uint32_t AnimatorManager::animateCommon(TreeInfo& info) {
     return functor.dirtyMask;
 }
 
-class EndAnimatorsFunctor {
+static void endStagingAnimator(BaseRenderNodeAnimator* animator) {
+    animator->end();
+    if (animator->listener()) {
+        animator->listener()->onAnimationFinished(animator);
+    }
+    animator->decStrong(0);
+}
+
+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:
-    EndAnimatorsFunctor(AnimationContext& context) : mContext(context) {}
+    EndActiveAnimatorsFunctor(AnimationContext& context) : mContext(context) {}
 
     void operator() (BaseRenderNodeAnimator* animator) {
-        animator->end();
-        animator->pushStaging(mContext);
-        animator->animate(mContext);
+        animator->forceEndNow(mContext);
         animator->decStrong(0);
     }
 
@@ -143,32 +163,13 @@ private:
     AnimationContext& mContext;
 };
 
-static void endAnimatorsHard(BaseRenderNodeAnimator* animator) {
-    animator->end();
-    if (animator->listener()) {
-        animator->listener()->onAnimationFinished(animator);
-    }
-    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);
-    }
-    // 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();
-    }
+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 */