Fix issue with RNA destruction mid-animation

Bug: 18521508

Fix an issue where an RNA's native object was destroyed
before the java-side object was started

Change-Id: I487fb476e0ecdf7000515f4f7320e8cfbc50a52b

diff --git a/core/java/android/view/RenderNodeAnimator.java b/core/java/android/view/RenderNodeAnimator.java
index 7feca30..7b35a3b 100644 (file)
--- a/core/java/android/view/RenderNodeAnimator.java
+++ b/core/java/android/view/RenderNodeAnimator.java
@@ -148,6 +148,10 @@ public class RenderNodeAnimator extends Animator {
         if (mState != STATE_PREPARE) {
             throw new IllegalStateException("Animator has already started, cannot change it now!");
         }
+        if (mNativePtr == null) {
+            throw new IllegalStateException("Animator's target has been destroyed "
+                    + "(trying to modify an animation after activity destroy?)");
+        }
     }
 
     static boolean isNativeInterpolator(TimeInterpolator interpolator) {
@@ -180,7 +184,10 @@ public class RenderNodeAnimator extends Animator {
         mState = STATE_DELAYED;
         applyInterpolator();
 
-        if (mStartDelay <= 0 || !mUiThreadHandlesDelay) {
+        if (mNativePtr == null) {
+            // It's dead, immediately cancel
+            cancel();
+        } else if (mStartDelay <= 0 || !mUiThreadHandlesDelay) {
             nSetStartDelay(mNativePtr.get(), mStartDelay);
             doStart();
         } else {
@@ -208,7 +215,9 @@ public class RenderNodeAnimator extends Animator {
 
     private void moveToRunningState() {
         mState = STATE_RUNNING;
-        nStart(mNativePtr.get(), this);
+        if (mNativePtr != null) {
+            nStart(mNativePtr.get());
+        }
         notifyStartListeners();
     }
 
@@ -227,7 +236,6 @@ public class RenderNodeAnimator extends Animator {
                 getHelper().removeDelayedAnimation(this);
                 moveToRunningState();
             }
-            nEnd(mNativePtr.get());
 
             final ArrayList<AnimatorListener> listeners = cloneListeners();
             final int numListeners = listeners == null ? 0 : listeners.size();
@@ -235,10 +243,7 @@ public class RenderNodeAnimator extends Animator {
                 listeners.get(i).onAnimationCancel(this);
             }
 
-            if (mViewTarget != null) {
-                // Kick off a frame to flush the state change
-                mViewTarget.invalidateViewProperty(true, false);
-            }
+            end();
         }
     }
 
@@ -249,10 +254,15 @@ public class RenderNodeAnimator extends Animator {
                 getHelper().removeDelayedAnimation(this);
                 doStart();
             }
-            nEnd(mNativePtr.get());
-            if (mViewTarget != null) {
-                // Kick off a frame to flush the state change
-                mViewTarget.invalidateViewProperty(true, false);
+            if (mNativePtr != null) {
+                nEnd(mNativePtr.get());
+                if (mViewTarget != null) {
+                    // Kick off a frame to flush the state change
+                    mViewTarget.invalidateViewProperty(true, false);
+                }
+            } else {
+                // It's already dead, jump to onFinish
+                onFinished();
             }
         }
     }
@@ -281,9 +291,11 @@ public class RenderNodeAnimator extends Animator {
     }
 
     private void setTarget(RenderNode node) {
+        checkMutable();
         if (mTarget != null) {
             throw new IllegalStateException("Target already set!");
         }
+        nSetListener(mNativePtr.get(), this);
         mTarget = node;
         mTarget.addAnimator(this);
     }
@@ -346,6 +358,12 @@ public class RenderNodeAnimator extends Animator {
     }
 
     protected void onFinished() {
+        if (mState == STATE_PREPARE) {
+            // Unlikely but possible, the native side has been destroyed
+            // before we have started.
+            releaseNativePtr();
+            return;
+        }
         if (mState == STATE_DELAYED) {
             getHelper().removeDelayedAnimation(this);
             notifyStartListeners();
@@ -361,8 +379,14 @@ public class RenderNodeAnimator extends Animator {
         // Release the native object, as it has a global reference to us. This
         // breaks the cyclic reference chain, and allows this object to be
         // GC'd
-        mNativePtr.release();
-        mNativePtr = null;
+        releaseNativePtr();
+    }
+
+    private void releaseNativePtr() {
+        if (mNativePtr != null) {
+            mNativePtr.release();
+            mNativePtr = null;
+        }
     }
 
     @SuppressWarnings("unchecked")
@@ -484,7 +508,8 @@ public class RenderNodeAnimator extends Animator {
     private static native void nSetStartDelay(long nativePtr, long startDelay);
     private static native void nSetInterpolator(long animPtr, long interpolatorPtr);
     private static native void nSetAllowRunningAsync(long animPtr, boolean mayRunAsync);
+    private static native void nSetListener(long animPtr, RenderNodeAnimator listener);
 
-    private static native void nStart(long animPtr, RenderNodeAnimator finishListener);
+    private static native void nStart(long animPtr);
     private static native void nEnd(long animPtr);
 }

diff --git a/core/jni/android_view_RenderNodeAnimator.cpp b/core/jni/android_view_RenderNodeAnimator.cpp
index 311882d..eb56639 100644 (file)
--- a/core/jni/android_view_RenderNodeAnimator.cpp
+++ b/core/jni/android_view_RenderNodeAnimator.cpp
@@ -177,9 +177,13 @@ static void setAllowRunningAsync(JNIEnv* env, jobject clazz, jlong animatorPtr,
     animator->setAllowRunningAsync(mayRunAsync);
 }
 
-static void start(JNIEnv* env, jobject clazz, jlong animatorPtr, jobject finishListener) {
+static void setListener(JNIEnv* env, jobject clazz, jlong animatorPtr, jobject finishListener) {
     BaseRenderNodeAnimator* animator = reinterpret_cast<BaseRenderNodeAnimator*>(animatorPtr);
     animator->setListener(new AnimationListenerBridge(env, finishListener));
+}
+
+static void start(JNIEnv* env, jobject clazz, jlong animatorPtr) {
+    BaseRenderNodeAnimator* animator = reinterpret_cast<BaseRenderNodeAnimator*>(animatorPtr);
     animator->start();
 }
 
@@ -208,7 +212,8 @@ static JNINativeMethod gMethods[] = {
     { "nSetStartDelay", "(JJ)V", (void*) setStartDelay },
     { "nSetInterpolator", "(JJ)V", (void*) setInterpolator },
     { "nSetAllowRunningAsync", "(JZ)V", (void*) setAllowRunningAsync },
-    { "nStart", "(JLandroid/view/RenderNodeAnimator;)V", (void*) start },
+    { "nSetListener", "(JLandroid/view/RenderNodeAnimator;)V", (void*) setListener},
+    { "nStart", "(J)V", (void*) start},
     { "nEnd", "(J)V", (void*) end },
 #endif
 };

diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/RevealActivity.java b/tests/HwAccelerationTest/src/com/android/test/hwui/RevealActivity.java
index 256a1d4..1216fc4 100644 (file)
--- a/tests/HwAccelerationTest/src/com/android/test/hwui/RevealActivity.java
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/RevealActivity.java
@@ -17,12 +17,14 @@
 package com.android.test.hwui;
 
 import android.animation.Animator;
+import android.animation.Animator.AnimatorListener;
 import android.animation.AnimatorSet;
 import android.app.Activity;
 import android.content.Context;
 import android.graphics.Canvas;
 import android.graphics.Color;
 import android.os.Bundle;
+import android.util.Log;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.ViewAnimationUtils;
@@ -37,6 +39,29 @@ public class RevealActivity extends Activity implements OnClickListener {
     private boolean mShouldBlock;
     private int mIteration = 0;
 
+    private AnimatorListener mListener = new AnimatorListener() {
+
+        @Override
+        public void onAnimationStart(Animator animation) {
+            Log.d("Reveal", "onAnimatorStart " + animation);
+        }
+
+        @Override
+        public void onAnimationRepeat(Animator animation) {
+            Log.d("Reveal", "onAnimationRepeat " + animation);
+        }
+
+        @Override
+        public void onAnimationEnd(Animator animation) {
+            Log.d("Reveal", "onAnimationEnd " + animation);
+        }
+
+        @Override
+        public void onAnimationCancel(Animator animation) {
+            Log.d("Reveal", "onAnimationCancel " + animation);
+        }
+    };
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -59,6 +84,8 @@ public class RevealActivity extends Activity implements OnClickListener {
         Animator animator = ViewAnimationUtils.createCircularReveal(view,
                 view.getWidth() / 2, view.getHeight() / 2,
                 0, Math.max(view.getWidth(), view.getHeight()));
+        Log.d("Reveal", "Calling start...");
+        animator.addListener(mListener);
         if (mIteration < 2) {
             animator.setDuration(DURATION);
             animator.start();
@@ -66,6 +93,7 @@ public class RevealActivity extends Activity implements OnClickListener {
             AnimatorSet set = new AnimatorSet();
             set.playTogether(animator);
             set.setDuration(DURATION);
+            set.addListener(mListener);
             set.start();
         }