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

9 files changed, 69 insertions, 41 deletions

diff --git a/core/jni/android_view_RenderNode.cpp b/core/jni/android_view_RenderNode.cpp
index 129683178a6f..949f4ff2066f 100644
--- a/core/jni/android_view_RenderNode.cpp
+++ b/core/jni/android_view_RenderNode.cpp
@@ -458,7 +458,7 @@ static void android_view_RenderNode_addAnimator(JNIEnv* env, jobject clazz,
 static void android_view_RenderNode_endAllAnimators(JNIEnv* env, jobject clazz,
         jlong renderNodePtr) {
     RenderNode* renderNode = reinterpret_cast<RenderNode*>(renderNodePtr);
-    renderNode->animators().endAllAnimators();
+    renderNode->animators().endAllStagingAnimators();
 }
 
 #endif // USE_OPENGL_RENDERER
diff --git a/core/jni/android_view_ThreadedRenderer.cpp b/core/jni/android_view_ThreadedRenderer.cpp
index 6ec6b006fb5a..7e6d3356d565 100644
--- a/core/jni/android_view_ThreadedRenderer.cpp
+++ b/core/jni/android_view_ThreadedRenderer.cpp
@@ -166,12 +166,7 @@ public:
     // Runs any animations still left in mCurrentFrameAnimations
     virtual void runRemainingAnimations(TreeInfo& info) {
         AnimationContext::runRemainingAnimations(info);
-        // post all the finished stuff
-        if (mOnFinishedEvents.size()) {
-            sp<InvokeAnimationListeners> message
-                    = new InvokeAnimationListeners(mOnFinishedEvents);
-            mRootNode->sendMessage(message);
-        }
+        postOnFinishedEvents();
     }
 
     virtual void callOnFinished(BaseRenderNodeAnimator* animator, AnimationListener* listener) {
@@ -179,9 +174,22 @@ public:
         mOnFinishedEvents.push_back(event);
     }
 
+    virtual void destroy() {
+        AnimationContext::destroy();
+        postOnFinishedEvents();
+    }
+
 private:
     sp<RootRenderNode> mRootNode;
     std::vector<OnFinishedEvent> mOnFinishedEvents;
+
+    void postOnFinishedEvents() {
+        if (mOnFinishedEvents.size()) {
+            sp<InvokeAnimationListeners> message
+                    = new InvokeAnimationListeners(mOnFinishedEvents);
+            mRootNode->sendMessage(message);
+        }
+    }
 };
 
 class ContextFactoryImpl : public IContextFactory {
diff --git a/libs/hwui/AnimationContext.cpp b/libs/hwui/AnimationContext.cpp
index d7d974389441..716dcf57de2d 100644
--- a/libs/hwui/AnimationContext.cpp
+++ b/libs/hwui/AnimationContext.cpp
@@ -31,11 +31,14 @@ AnimationContext::AnimationContext(renderthread::TimeLord& clock)
 }
 
 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 900d953ed6ac..9b3d5f486464 100644
--- a/libs/hwui/AnimationContext.h
+++ b/libs/hwui/AnimationContext.h
@@ -98,6 +98,8 @@ public:
 
     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 da65f381b514..4f3a57377b26 100644
--- a/libs/hwui/Animator.cpp
+++ b/libs/hwui/Animator.cpp
@@ -161,6 +161,13 @@ bool BaseRenderNodeAnimator::animate(AnimationContext& context) {
     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 c52a93fddaa8..1ab6235bb2d7 100644
--- a/libs/hwui/Animator.h
+++ b/libs/hwui/Animator.h
@@ -68,6 +68,8 @@ public:
 
     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 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 */
diff --git a/libs/hwui/AnimatorManager.h b/libs/hwui/AnimatorManager.h
index d5f56ed283bf..d03d4272e34d 100644
--- a/libs/hwui/AnimatorManager.h
+++ b/libs/hwui/AnimatorManager.h
@@ -50,8 +50,12 @@ public:
 
     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 967cb6ffcde5..428e42605107 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -59,6 +59,7 @@ void CanvasContext::destroy() {
     stopDrawing();
     freePrefetechedLayers();
     destroyHardwareResources();
+    mAnimationContext->destroy();
     if (mCanvas) {
         delete mCanvas;
         mCanvas = 0;