Revert "Revert "Check RenderNode's owning view before attaching animators""

This reverts commit eb40178af3b7c8d925eaf6c1aa0bef739c8ea47e.

Change-Id: I6838ecb35b50847746ee66ac204f14eb5f579b91
diff --git a/libs/hwui/Animator.cpp b/libs/hwui/Animator.cpp
index 372bcb3..294edb6 100644
--- a/libs/hwui/Animator.cpp
+++ b/libs/hwui/Animator.cpp
@@ -33,6 +33,7 @@
 
 BaseRenderNodeAnimator::BaseRenderNodeAnimator(float finalValue)
         : mTarget(nullptr)
+        , mStagingTarget(nullptr)
         , mFinalValue(finalValue)
         , mDeltaValue(0)
         , mFromValue(0)
@@ -82,7 +83,7 @@
 }
 
 void BaseRenderNodeAnimator::attach(RenderNode* target) {
-    mTarget = target;
+    mStagingTarget = target;
     onAttached();
 }
 
@@ -145,6 +146,15 @@
 }
 
 void BaseRenderNodeAnimator::pushStaging(AnimationContext& context) {
+    if (mStagingTarget) {
+        RenderNode* oldTarget = mTarget;
+        mTarget = mStagingTarget;
+        mStagingTarget = nullptr;
+        if (oldTarget && oldTarget != mTarget) {
+            oldTarget->onAnimatorTargetChanged(this);
+        }
+    }
+
     if (!mHasStartValue) {
         doSetStartValue(getValue(mTarget));
     }
@@ -195,6 +205,7 @@
             }
         }
     }
+    onPushStaging();
 }
 
 void BaseRenderNodeAnimator::transitionToRunning(AnimationContext& context) {
@@ -309,18 +320,36 @@
 
 void RenderPropertyAnimator::onAttached() {
     if (!mHasStartValue
-            && mTarget->isPropertyFieldDirty(mPropertyAccess->dirtyMask)) {
-        setStartValue((mTarget->stagingProperties().*mPropertyAccess->getter)());
+            && mStagingTarget->isPropertyFieldDirty(mPropertyAccess->dirtyMask)) {
+        setStartValue((mStagingTarget->stagingProperties().*mPropertyAccess->getter)());
     }
 }
 
 void RenderPropertyAnimator::onStagingPlayStateChanged() {
     if (mStagingPlayState == PlayState::Running) {
-        (mTarget->mutateStagingProperties().*mPropertyAccess->setter)(finalValue());
+        if (mStagingTarget) {
+            (mStagingTarget->mutateStagingProperties().*mPropertyAccess->setter)(finalValue());
+        } else {
+            // In the case of start delay where stagingTarget has been sync'ed over and null'ed
+            // we delay the properties update to push staging.
+            mShouldUpdateStagingProperties = true;
+        }
     } else if (mStagingPlayState == PlayState::Finished) {
         // We're being canceled, so make sure that whatever values the UI thread
         // is observing for us is pushed over
+        mShouldSyncPropertyFields = true;
+    }
+}
+
+void RenderPropertyAnimator::onPushStaging() {
+    if (mShouldUpdateStagingProperties) {
+        (mTarget->mutateStagingProperties().*mPropertyAccess->setter)(finalValue());
+        mShouldUpdateStagingProperties = false;
+    }
+
+    if (mShouldSyncPropertyFields) {
         mTarget->setPropertyFieldsDirty(dirtyMask());
+        mShouldSyncPropertyFields = false;
     }
 }
 
diff --git a/libs/hwui/Animator.h b/libs/hwui/Animator.h
index fcbc11b..fdae0f3 100644
--- a/libs/hwui/Animator.h
+++ b/libs/hwui/Animator.h
@@ -85,6 +85,7 @@
 
     void forceEndNow(AnimationContext& context);
     RenderNode* target() { return mTarget; }
+    RenderNode* stagingTarget() { return mStagingTarget; }
 
 protected:
     // PlayState is used by mStagingPlayState and mPlayState to track the state initiated from UI
@@ -123,8 +124,10 @@
 
     virtual void onStagingPlayStateChanged() {}
     virtual void onPlayTimeChanged(nsecs_t playTime) {}
+    virtual void onPushStaging() {}
 
     RenderNode* mTarget;
+    RenderNode* mStagingTarget;
 
     float mFinalValue;
     float mDeltaValue;
@@ -188,6 +191,7 @@
     virtual void setValue(RenderNode* target, float value) override;
     virtual void onAttached() override;
     virtual void onStagingPlayStateChanged() override;
+    virtual void onPushStaging() override;
 
 private:
     typedef bool (RenderProperties::*SetFloatProperty)(float value);
@@ -197,6 +201,8 @@
     const PropertyAccessors* mPropertyAccess;
 
     static const PropertyAccessors PROPERTY_ACCESSOR_LUT[];
+    bool mShouldSyncPropertyFields = false;
+    bool mShouldUpdateStagingProperties = false;
 };
 
 class CanvasPropertyPrimitiveAnimator : public BaseRenderNodeAnimator {
diff --git a/libs/hwui/AnimatorManager.cpp b/libs/hwui/AnimatorManager.cpp
index 2b49b47..2198fcc 100644
--- a/libs/hwui/AnimatorManager.cpp
+++ b/libs/hwui/AnimatorManager.cpp
@@ -42,7 +42,23 @@
 }
 
 void AnimatorManager::addAnimator(const sp<BaseRenderNodeAnimator>& animator) {
+    RenderNode* stagingTarget = animator->stagingTarget();
+    if (stagingTarget == &mParent) {
+        return;
+    }
     mNewAnimators.emplace_back(animator.get());
+    // If the animator is already attached to other RenderNode, remove it from that RenderNode's
+    // new animator list. This ensures one animator only ends up in one newAnimatorList during one
+    // frame, even when it's added multiple times to multiple targets.
+    if (stagingTarget) {
+        stagingTarget->removeAnimator(animator);
+    }
+    animator->attach(&mParent);
+}
+
+void AnimatorManager::removeAnimator(const sp<BaseRenderNodeAnimator>& animator) {
+    mNewAnimators.erase(std::remove(mNewAnimators.begin(), mNewAnimators.end(), animator),
+            mNewAnimators.end());
 }
 
 void AnimatorManager::setAnimationHandle(AnimationHandle* handle) {
@@ -58,21 +74,12 @@
         LOG_ALWAYS_FATAL_IF(!mAnimationHandle,
                 "Trying to start new animators on %p (%s) without an animation handle!",
                 &mParent, mParent.getName());
-        // Only add animators that are not already in the on-going animator list.
-        for (auto& animator : mNewAnimators) {
-            RenderNode* targetRenderNode = animator->target();
-            if (targetRenderNode == &mParent) {
-                // Animator already in the animator list: skip adding again
-                continue;
-            }
 
-            if (targetRenderNode){
-                // If the animator is already in another RenderNode's animator list, remove animator from
-                // that list and add animator to current RenderNode's list.
-                targetRenderNode->animators().removeActiveAnimator(animator);
+        // Only add new animators that are not already in the mAnimators list
+        for (auto& anim : mNewAnimators) {
+            if (anim->target() != &mParent) {
+                mAnimators.push_back(std::move(anim));
             }
-            animator->attach(&mParent);
-            mAnimators.push_back(std::move(animator));
         }
         mNewAnimators.clear();
     }
@@ -81,6 +88,11 @@
     }
 }
 
+void AnimatorManager::onAnimatorTargetChanged(BaseRenderNodeAnimator* animator) {
+    LOG_ALWAYS_FATAL_IF(animator->target() == &mParent, "Target has not been changed");
+    mAnimators.erase(std::remove(mAnimators.begin(), mAnimators.end(), animator), mAnimators.end());
+}
+
 class AnimateFunctor {
 public:
     AnimateFunctor(TreeInfo& info, AnimationContext& context)
@@ -154,10 +166,6 @@
     mNewAnimators.clear();
 }
 
-void AnimatorManager::removeActiveAnimator(const sp<BaseRenderNodeAnimator>& animator) {
-    std::remove(mAnimators.begin(), mAnimators.end(), animator);
-}
-
 class EndActiveAnimatorsFunctor {
 public:
     EndActiveAnimatorsFunctor(AnimationContext& context) : mContext(context) {}
diff --git a/libs/hwui/AnimatorManager.h b/libs/hwui/AnimatorManager.h
index c24ef47..61f6179 100644
--- a/libs/hwui/AnimatorManager.h
+++ b/libs/hwui/AnimatorManager.h
@@ -39,11 +39,13 @@
     ~AnimatorManager();
 
     void addAnimator(const sp<BaseRenderNodeAnimator>& animator);
+    void removeAnimator(const sp<BaseRenderNodeAnimator>& animator);
 
     void setAnimationHandle(AnimationHandle* handle);
     bool hasAnimationHandle() { return mAnimationHandle; }
 
     void pushStaging();
+    void onAnimatorTargetChanged(BaseRenderNodeAnimator* animator);
 
     // Returns the combined dirty mask of all animators run
     uint32_t animate(TreeInfo& info);
@@ -62,15 +64,10 @@
 private:
     uint32_t animateCommon(TreeInfo& info);
 
-    // This would remove the animator from mAnimators list. It should only be called during
-    // push staging.
-    void removeActiveAnimator(const sp<BaseRenderNodeAnimator>& animator);
-
     RenderNode& mParent;
     AnimationHandle* mAnimationHandle;
 
     // To improve the efficiency of resizing & removing from the vector
-    // use manual ref counting instead of sp<>.
     std::vector< sp<BaseRenderNodeAnimator> > mNewAnimators;
     std::vector< sp<BaseRenderNodeAnimator> > mAnimators;
 };
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index bade216..9ac76a4 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -218,6 +218,10 @@
     mAnimatorManager.addAnimator(animator);
 }
 
+void RenderNode::removeAnimator(const sp<BaseRenderNodeAnimator>& animator) {
+    mAnimatorManager.removeAnimator(animator);
+}
+
 void RenderNode::damageSelf(TreeInfo& info) {
     if (isRenderable()) {
         if (properties().getClipDamageToBounds()) {
diff --git a/libs/hwui/RenderNode.h b/libs/hwui/RenderNode.h
index f248de54..e037645 100644
--- a/libs/hwui/RenderNode.h
+++ b/libs/hwui/RenderNode.h
@@ -187,6 +187,12 @@
 
     // UI thread only!
     ANDROID_API void addAnimator(const sp<BaseRenderNodeAnimator>& animator);
+    void removeAnimator(const sp<BaseRenderNodeAnimator>& animator);
+
+    // This can only happen during pushStaging()
+    void onAnimatorTargetChanged(BaseRenderNodeAnimator* animator) {
+        mAnimatorManager.onAnimatorTargetChanged(animator);
+    }
 
     AnimatorManager& animators() { return mAnimatorManager; }