Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 528b0d28 authored by Nader Jawad's avatar Nader Jawad
Browse files

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
parent 998d86ca
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -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() {
+49 −0
Original line number Diff line number Diff line
@@ -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");
        }
    }
}
+7 −0
Original line number Diff line number Diff line
@@ -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
    ///////////////////////////////////////////////////////////////////////////
+16 −3
Original line number Diff line number Diff line
@@ -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,10 +83,18 @@ void AnimatorManager::pushStaging() {
        }
        mNewAnimators.clear();
    }

    if (mCancelAllAnimators) {
        for (auto& animator : mAnimators) {
            animator->forceEndNow(mAnimationHandle->context());
        }
        mCancelAllAnimators = false;
    } else {
        for (auto& animator : mAnimators) {
            animator->pushStaging(mAnimationHandle->context());
        }
    }
}

void AnimatorManager::onAnimatorTargetChanged(BaseRenderNodeAnimator* animator) {
    LOG_ALWAYS_FATAL_IF(animator->target() == &mParent, "Target has not been changed");
@@ -184,5 +193,9 @@ void AnimatorManager::endAllActiveAnimators() {
    mAnimationHandle->release();
}

void AnimatorManager::forceEndAnimators() {
    mCancelAllAnimators = true;
}

} /* namespace uirenderer */
} /* namespace android */
+6 −2
Original line number Diff line number Diff line
@@ -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 */
Loading