Fix memory leak with RenderNodeAnimator

Update View logic to cancel all RenderNodeAnimators
when it is detached from a window.
Updated HWUI Animation logic to enable a cancellation
flag to cancel all animators operating on a RenderNode
whenever the staging parameters are pushed to RenderThread

Fixes: 229136453
Test: Added core test to RenderNodeAnimatorTests
Change-Id: Id674e8474757bfc8dfe30394dde29da49d139bfc

6 files changed, 90 insertions, 5 deletions

diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index 8901d86e1f2a..3eaad51dac9e 100644
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -21149,6 +21149,11 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
         }
 
         AccessibilityNodeIdManager.getInstance().unregisterViewWithId(getAccessibilityViewId());
+
+        if (mBackgroundRenderNode != null) {
+            mBackgroundRenderNode.forceEndAnimators();
+        }
+        mRenderNode.forceEndAnimators();
     }
 
     private void cleanupDraw() {
diff --git a/core/tests/coretests/src/android/view/RenderNodeAnimatorTest.java b/core/tests/coretests/src/android/view/RenderNodeAnimatorTest.java
index 786c22bf04f6..9b6bcda8aed0 100644
--- a/core/tests/coretests/src/android/view/RenderNodeAnimatorTest.java
+++ b/core/tests/coretests/src/android/view/RenderNodeAnimatorTest.java
@@ -19,17 +19,24 @@ package android.view;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
+import android.animation.Animator;
+import android.animation.AnimatorListenerAdapter;
 import android.app.Activity;
 import android.content.Context;
+import android.widget.FrameLayout;
 
 import androidx.test.InstrumentationRegistry;
 import androidx.test.annotation.UiThreadTest;
 import androidx.test.filters.MediumTest;
 import androidx.test.rule.ActivityTestRule;
 
+import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
 @MediumTest
 public class RenderNodeAnimatorTest  {
     @Rule
@@ -57,4 +64,46 @@ public class RenderNodeAnimatorTest  {
         anim.start(); // should initialize mTransformationInfo
         assertNotNull(view.mTransformationInfo);
     }
+
+    @Test
+    public void testViewDetachCancelsRenderNodeAnimator() {
+        // Start a RenderNodeAnimator with a long duration time, then detach the target view
+        // before the animation completes. Detaching of a View from a window should force cancel all
+        // RenderNodeAnimators
+        CountDownLatch latch = new CountDownLatch(1);
+
+        FrameLayout container = new FrameLayout(getContext());
+        View view = new View(getContext());
+
+        getActivity().runOnUiThread(() -> {
+            container.addView(view, new FrameLayout.LayoutParams(100, 100));
+            getActivity().setContentView(container);
+        });
+        getActivity().runOnUiThread(() -> {
+            RenderNodeAnimator anim = new RenderNodeAnimator(0, 0, 10f, 30f);
+            anim.setDuration(10000);
+            anim.setTarget(view);
+            anim.addListener(new AnimatorListenerAdapter() {
+
+                @Override
+                public void onAnimationEnd(Animator animation) {
+                    super.onAnimationEnd(animation);
+                    latch.countDown();
+                }
+            });
+
+            anim.start();
+        });
+
+        getActivity().runOnUiThread(()-> {
+            container.removeView(view);
+        });
+
+        try {
+            Assert.assertTrue("onAnimationEnd not invoked",
+                    latch.await(3000, TimeUnit.MILLISECONDS));
+        } catch (InterruptedException excep) {
+            Assert.fail("Interrupted waiting for onAnimationEnd callback");
+        }
+    }
 }
diff --git a/graphics/java/android/graphics/RenderNode.java b/graphics/java/android/graphics/RenderNode.java
index 5fd53adc409a..dadbd8d2d1aa 100644
--- a/graphics/java/android/graphics/RenderNode.java
+++ b/graphics/java/android/graphics/RenderNode.java
@@ -1611,6 +1611,11 @@ public final class RenderNode {
         nEndAllAnimators(mNativeRenderNode);
     }
 
+    /** @hide */
+    public void forceEndAnimators() {
+        nForceEndAnimators(mNativeRenderNode);
+    }
+
     ///////////////////////////////////////////////////////////////////////////
     // Regular JNI methods
     ///////////////////////////////////////////////////////////////////////////
@@ -1633,6 +1638,8 @@ public final class RenderNode {
 
     private static native void nEndAllAnimators(long renderNode);
 
+    private static native void nForceEndAnimators(long renderNode);
+
     ///////////////////////////////////////////////////////////////////////////
     // @CriticalNative methods
     ///////////////////////////////////////////////////////////////////////////
diff --git a/libs/hwui/AnimatorManager.cpp b/libs/hwui/AnimatorManager.cpp
index 4826d5a0c8da..078041411a21 100644
--- a/libs/hwui/AnimatorManager.cpp
+++ b/libs/hwui/AnimatorManager.cpp
@@ -31,7 +31,8 @@ static void detach(sp<BaseRenderNodeAnimator>& animator) {
     animator->detach();
 }
 
-AnimatorManager::AnimatorManager(RenderNode& parent) : mParent(parent), mAnimationHandle(nullptr) {}
+AnimatorManager::AnimatorManager(RenderNode& parent)
+        : mParent(parent), mAnimationHandle(nullptr), mCancelAllAnimators(false) {}
 
 AnimatorManager::~AnimatorManager() {
     for_each(mNewAnimators.begin(), mNewAnimators.end(), detach);
@@ -82,8 +83,16 @@ void AnimatorManager::pushStaging() {
         }
         mNewAnimators.clear();
     }
-    for (auto& animator : mAnimators) {
-        animator->pushStaging(mAnimationHandle->context());
+
+    if (mCancelAllAnimators) {
+        for (auto& animator : mAnimators) {
+            animator->forceEndNow(mAnimationHandle->context());
+        }
+        mCancelAllAnimators = false;
+    } else {
+        for (auto& animator : mAnimators) {
+            animator->pushStaging(mAnimationHandle->context());
+        }
     }
 }
 
@@ -184,5 +193,9 @@ void AnimatorManager::endAllActiveAnimators() {
     mAnimationHandle->release();
 }
 
+void AnimatorManager::forceEndAnimators() {
+    mCancelAllAnimators = true;
+}
+
 } /* namespace uirenderer */
 } /* namespace android */
diff --git a/libs/hwui/AnimatorManager.h b/libs/hwui/AnimatorManager.h
index a0df01d5962c..6002661dc82a 100644
--- a/libs/hwui/AnimatorManager.h
+++ b/libs/hwui/AnimatorManager.h
@@ -16,11 +16,11 @@
 #ifndef ANIMATORMANAGER_H
 #define ANIMATORMANAGER_H
 
-#include <vector>
-
 #include <cutils/compiler.h>
 #include <utils/StrongPointer.h>
 
+#include <vector>
+
 #include "utils/Macros.h"
 
 namespace android {
@@ -56,6 +56,8 @@ public:
     // Hard-ends all animators. May only be called on the UI thread.
     void endAllStagingAnimators();
 
+    void forceEndAnimators();
+
     // Hard-ends all animators that have been pushed. Used for cleanup if
     // the ActivityContext is being destroyed
     void endAllActiveAnimators();
@@ -71,6 +73,8 @@ private:
     // To improve the efficiency of resizing & removing from the vector
     std::vector<sp<BaseRenderNodeAnimator> > mNewAnimators;
     std::vector<sp<BaseRenderNodeAnimator> > mAnimators;
+
+    bool mCancelAllAnimators;
 };
 
 } /* namespace uirenderer */
diff --git a/libs/hwui/jni/android_graphics_RenderNode.cpp b/libs/hwui/jni/android_graphics_RenderNode.cpp
index 944393c63ad6..db7639029187 100644
--- a/libs/hwui/jni/android_graphics_RenderNode.cpp
+++ b/libs/hwui/jni/android_graphics_RenderNode.cpp
@@ -543,6 +543,12 @@ static void android_view_RenderNode_endAllAnimators(JNIEnv* env, jobject clazz,
     renderNode->animators().endAllStagingAnimators();
 }
 
+static void android_view_RenderNode_forceEndAnimators(JNIEnv* env, jobject clazz,
+                                                      jlong renderNodePtr) {
+    RenderNode* renderNode = reinterpret_cast<RenderNode*>(renderNodePtr);
+    renderNode->animators().forceEndAnimators();
+}
+
 // ----------------------------------------------------------------------------
 // SurfaceView position callback
 // ----------------------------------------------------------------------------
@@ -745,6 +751,7 @@ static const JNINativeMethod gMethods[] = {
         {"nGetAllocatedSize", "(J)I", (void*)android_view_RenderNode_getAllocatedSize},
         {"nAddAnimator", "(JJ)V", (void*)android_view_RenderNode_addAnimator},
         {"nEndAllAnimators", "(J)V", (void*)android_view_RenderNode_endAllAnimators},
+        {"nForceEndAnimators", "(J)V", (void*)android_view_RenderNode_forceEndAnimators},
         {"nRequestPositionUpdates", "(JLjava/lang/ref/WeakReference;)V",
          (void*)android_view_RenderNode_requestPositionUpdates},