From 3998a01e8fcc78fd16999e98832ccdb0349a6526 Mon Sep 17 00:00:00 2001 From: John Reck Date: Wed, 28 Jul 2021 11:04:30 -0400 Subject: [PATCH] Move AnimatedImageDrawable off of NewWeakGlobalRef Bug: 194893628 Test: make && atest android.graphics.drawable.cts.AnimatedImageDrawableTest Change-Id: I5c9845bc1408a3b1221e1ba45409ee194189387f --- .../drawable/AnimatedImageDrawable.java | 12 +++- libs/hwui/jni/AnimatedImageDrawable.cpp | 55 ++++++++++--------- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java b/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java index dd64327ebfd8d..82b3f6875d67e 100644 --- a/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java +++ b/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java @@ -48,6 +48,7 @@ import org.xmlpull.v1.XmlPullParserException; import java.io.IOException; import java.io.InputStream; +import java.lang.ref.WeakReference; import java.util.ArrayList; /** @@ -494,7 +495,7 @@ public class AnimatedImageDrawable extends Drawable implements Animatable2 { if (mAnimationCallbacks == null) { mAnimationCallbacks = new ArrayList(); - nSetOnAnimationEndListener(mState.mNativePtr, this); + nSetOnAnimationEndListener(mState.mNativePtr, new WeakReference<>(this)); } if (!mAnimationCallbacks.contains(callback)) { @@ -562,6 +563,13 @@ public class AnimatedImageDrawable extends Drawable implements Animatable2 { * callback, so no need to post. */ @SuppressWarnings("unused") + private static void callOnAnimationEnd(WeakReference weakDrawable) { + AnimatedImageDrawable drawable = weakDrawable.get(); + if (drawable != null) { + drawable.onAnimationEnd(); + } + } + private void onAnimationEnd() { if (mAnimationCallbacks != null) { for (Animatable2.AnimationCallback callback : mAnimationCallbacks) { @@ -603,7 +611,7 @@ public class AnimatedImageDrawable extends Drawable implements Animatable2 { private static native void nSetRepeatCount(long nativePtr, int repeatCount); // Pass the drawable down to native so it can call onAnimationEnd. private static native void nSetOnAnimationEndListener(long nativePtr, - @Nullable AnimatedImageDrawable drawable); + @Nullable WeakReference drawable); @FastNative private static native long nNativeByteSize(long nativePtr); @FastNative diff --git a/libs/hwui/jni/AnimatedImageDrawable.cpp b/libs/hwui/jni/AnimatedImageDrawable.cpp index c9433ec8a9dae..c40b858268bef 100644 --- a/libs/hwui/jni/AnimatedImageDrawable.cpp +++ b/libs/hwui/jni/AnimatedImageDrawable.cpp @@ -30,7 +30,8 @@ using namespace android; -static jmethodID gAnimatedImageDrawable_onAnimationEndMethodID; +static jclass gAnimatedImageDrawableClass; +static jmethodID gAnimatedImageDrawable_callOnAnimationEndMethodID; // Note: jpostProcess holds a handle to the ImageDecoder. static jlong AnimatedImageDrawable_nCreate(JNIEnv* env, jobject /*clazz*/, @@ -178,26 +179,23 @@ class InvokeListener : public MessageHandler { public: InvokeListener(JNIEnv* env, jobject javaObject) { LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&mJvm) != JNI_OK); - // Hold a weak reference to break a cycle that would prevent GC. - mWeakRef = env->NewWeakGlobalRef(javaObject); + mCallbackRef = env->NewGlobalRef(javaObject); } ~InvokeListener() override { auto* env = requireEnv(mJvm); - env->DeleteWeakGlobalRef(mWeakRef); + env->DeleteGlobalRef(mCallbackRef); } virtual void handleMessage(const Message&) override { auto* env = get_env_or_die(mJvm); - jobject localRef = env->NewLocalRef(mWeakRef); - if (localRef) { - env->CallVoidMethod(localRef, gAnimatedImageDrawable_onAnimationEndMethodID); - } + env->CallStaticVoidMethod(gAnimatedImageDrawableClass, + gAnimatedImageDrawable_callOnAnimationEndMethodID, mCallbackRef); } private: JavaVM* mJvm; - jweak mWeakRef; + jobject mCallbackRef; }; class JniAnimationEndListener : public OnAnimationEndListener { @@ -253,26 +251,31 @@ static void AnimatedImageDrawable_nSetBounds(JNIEnv* env, jobject /*clazz*/, jlo } static const JNINativeMethod gAnimatedImageDrawableMethods[] = { - { "nCreate", "(JLandroid/graphics/ImageDecoder;IIJZLandroid/graphics/Rect;)J",(void*) AnimatedImageDrawable_nCreate }, - { "nGetNativeFinalizer", "()J", (void*) AnimatedImageDrawable_nGetNativeFinalizer }, - { "nDraw", "(JJ)J", (void*) AnimatedImageDrawable_nDraw }, - { "nSetAlpha", "(JI)V", (void*) AnimatedImageDrawable_nSetAlpha }, - { "nGetAlpha", "(J)I", (void*) AnimatedImageDrawable_nGetAlpha }, - { "nSetColorFilter", "(JJ)V", (void*) AnimatedImageDrawable_nSetColorFilter }, - { "nIsRunning", "(J)Z", (void*) AnimatedImageDrawable_nIsRunning }, - { "nStart", "(J)Z", (void*) AnimatedImageDrawable_nStart }, - { "nStop", "(J)Z", (void*) AnimatedImageDrawable_nStop }, - { "nGetRepeatCount", "(J)I", (void*) AnimatedImageDrawable_nGetRepeatCount }, - { "nSetRepeatCount", "(JI)V", (void*) AnimatedImageDrawable_nSetRepeatCount }, - { "nSetOnAnimationEndListener", "(JLandroid/graphics/drawable/AnimatedImageDrawable;)V", (void*) AnimatedImageDrawable_nSetOnAnimationEndListener }, - { "nNativeByteSize", "(J)J", (void*) AnimatedImageDrawable_nNativeByteSize }, - { "nSetMirrored", "(JZ)V", (void*) AnimatedImageDrawable_nSetMirrored }, - { "nSetBounds", "(JLandroid/graphics/Rect;)V", (void*) AnimatedImageDrawable_nSetBounds }, + {"nCreate", "(JLandroid/graphics/ImageDecoder;IIJZLandroid/graphics/Rect;)J", + (void*)AnimatedImageDrawable_nCreate}, + {"nGetNativeFinalizer", "()J", (void*)AnimatedImageDrawable_nGetNativeFinalizer}, + {"nDraw", "(JJ)J", (void*)AnimatedImageDrawable_nDraw}, + {"nSetAlpha", "(JI)V", (void*)AnimatedImageDrawable_nSetAlpha}, + {"nGetAlpha", "(J)I", (void*)AnimatedImageDrawable_nGetAlpha}, + {"nSetColorFilter", "(JJ)V", (void*)AnimatedImageDrawable_nSetColorFilter}, + {"nIsRunning", "(J)Z", (void*)AnimatedImageDrawable_nIsRunning}, + {"nStart", "(J)Z", (void*)AnimatedImageDrawable_nStart}, + {"nStop", "(J)Z", (void*)AnimatedImageDrawable_nStop}, + {"nGetRepeatCount", "(J)I", (void*)AnimatedImageDrawable_nGetRepeatCount}, + {"nSetRepeatCount", "(JI)V", (void*)AnimatedImageDrawable_nSetRepeatCount}, + {"nSetOnAnimationEndListener", "(JLjava/lang/ref/WeakReference;)V", + (void*)AnimatedImageDrawable_nSetOnAnimationEndListener}, + {"nNativeByteSize", "(J)J", (void*)AnimatedImageDrawable_nNativeByteSize}, + {"nSetMirrored", "(JZ)V", (void*)AnimatedImageDrawable_nSetMirrored}, + {"nSetBounds", "(JLandroid/graphics/Rect;)V", (void*)AnimatedImageDrawable_nSetBounds}, }; int register_android_graphics_drawable_AnimatedImageDrawable(JNIEnv* env) { - jclass animatedImageDrawable_class = FindClassOrDie(env, "android/graphics/drawable/AnimatedImageDrawable"); - gAnimatedImageDrawable_onAnimationEndMethodID = GetMethodIDOrDie(env, animatedImageDrawable_class, "onAnimationEnd", "()V"); + gAnimatedImageDrawableClass = reinterpret_cast(env->NewGlobalRef( + FindClassOrDie(env, "android/graphics/drawable/AnimatedImageDrawable"))); + gAnimatedImageDrawable_callOnAnimationEndMethodID = + GetStaticMethodIDOrDie(env, gAnimatedImageDrawableClass, "callOnAnimationEnd", + "(Ljava/lang/ref/WeakReference;)V"); return android::RegisterMethodsOrDie(env, "android/graphics/drawable/AnimatedImageDrawable", gAnimatedImageDrawableMethods, NELEM(gAnimatedImageDrawableMethods));