commit 72d6e4facb1abd81809fdaddbe42f41ad885189c
author John Reck <jreck@google.com> Fri Nov 21 14:10:10 2014 -0800
committer John Reck <jreck@google.com> Fri Nov 21 14:27:21 2014 -0800
tree 50b40f0469131c33c8e3d77cd7045860c4298d6c
parent 08229e817ecb67b0c7ebbd6b5b9ce4aef1b38cc2

It's super critical to call nStart

Bug: 18204974

Even if we are canceling or ending an animation
nStart() *must* be called, otherwise the native-side listener
is not attached (lazy-attached for JNI cyclic reference reasons),
and then Animator::callOnFinishedListener() no-ops as there's
no listener set

Add a lifecycle verifier to ensure that nStart is always
called on animators that are attached and get finished

Change-Id: Ibc345b5be97b6d3f95a11c361ebe020d030fd3b6

core/java/android/view/RenderNodeAnimator.java

diff --git a/core/java/android/view/RenderNodeAnimator.java b/core/java/android/view/RenderNodeAnimator.java
index b95f9a4..7feca30 100644
--- a/core/java/android/view/RenderNodeAnimator.java
+++ b/core/java/android/view/RenderNodeAnimator.java
@@ -189,9 +189,6 @@
     }
 
     private void doStart() {
-        mState = STATE_RUNNING;
-        nStart(mNativePtr.get(), this);
-
         // Alpha is a special snowflake that has the canonical value stored
         // in mTransformationInfo instead of in RenderNode, so we need to update
         // it with the final value here.
@@ -201,7 +198,7 @@
             mViewTarget.mTransformationInfo.mAlpha = mFinalValue;
         }
 
-        notifyStartListeners();
+        moveToRunningState();
 
         if (mViewTarget != null) {
             // Kick off a frame to start the process
@@ -209,6 +206,12 @@
         }
     }
 
+    private void moveToRunningState() {
+        mState = STATE_RUNNING;
+        nStart(mNativePtr.get(), this);
+        notifyStartListeners();
+    }
+
     private void notifyStartListeners() {
         final ArrayList<AnimatorListener> listeners = cloneListeners();
         final int numListeners = listeners == null ? 0 : listeners.size();
@@ -222,7 +225,7 @@
         if (mState != STATE_PREPARE && mState != STATE_FINISHED) {
             if (mState == STATE_DELAYED) {
                 getHelper().removeDelayedAnimation(this);
-                notifyStartListeners();
+                moveToRunningState();
             }
             nEnd(mNativePtr.get());
 
@@ -242,7 +245,15 @@
     @Override
     public void end() {
         if (mState != STATE_FINISHED) {
+            if (mState < STATE_RUNNING) {
+                getHelper().removeDelayedAnimation(this);
+                doStart();
+            }
             nEnd(mNativePtr.get());
+            if (mViewTarget != null) {
+                // Kick off a frame to flush the state change
+                mViewTarget.invalidateViewProperty(true, false);
+            }
         }
     }
 

core/jni/android_view_RenderNodeAnimator.cpp

diff --git a/core/jni/android_view_RenderNodeAnimator.cpp b/core/jni/android_view_RenderNodeAnimator.cpp
index 84b7913..311882d 100644
--- a/core/jni/android_view_RenderNodeAnimator.cpp
+++ b/core/jni/android_view_RenderNodeAnimator.cpp
@@ -45,6 +45,15 @@
     return env;
 }
 
+class AnimationListenerLifecycleChecker : public AnimationListener {
+public:
+    virtual void onAnimationFinished(BaseRenderNodeAnimator* animator) {
+        LOG_ALWAYS_FATAL("Lifecycle failure, nStart(%p) wasn't called", animator);
+    }
+};
+
+static AnimationListenerLifecycleChecker sLifecycleChecker;
+
 class AnimationListenerBridge : public AnimationListener {
 public:
     // This holds a strong reference to a Java WeakReference<T> object. This avoids
@@ -100,6 +109,7 @@
         jint propertyRaw, jfloat finalValue) {
     RenderPropertyAnimator::RenderProperty property = toRenderProperty(propertyRaw);
     BaseRenderNodeAnimator* animator = new RenderPropertyAnimator(property, finalValue);
+    animator->setListener(&sLifecycleChecker);
     return reinterpret_cast<jlong>( animator );
 }
 
@@ -107,6 +117,7 @@
         jlong canvasPropertyPtr, jfloat finalValue) {
     CanvasPropertyPrimitive* canvasProperty = reinterpret_cast<CanvasPropertyPrimitive*>(canvasPropertyPtr);
     BaseRenderNodeAnimator* animator = new CanvasPropertyPrimitiveAnimator(canvasProperty, finalValue);
+    animator->setListener(&sLifecycleChecker);
     return reinterpret_cast<jlong>( animator );
 }
 
@@ -117,12 +128,14 @@
     CanvasPropertyPaintAnimator::PaintField paintField = toPaintField(paintFieldRaw);
     BaseRenderNodeAnimator* animator = new CanvasPropertyPaintAnimator(
             canvasProperty, paintField, finalValue);
+    animator->setListener(&sLifecycleChecker);
     return reinterpret_cast<jlong>( animator );
 }
 
 static jlong createRevealAnimator(JNIEnv* env, jobject clazz,
         jint centerX, jint centerY, jfloat startRadius, jfloat endRadius) {
     BaseRenderNodeAnimator* animator = new RevealAnimator(centerX, centerY, startRadius, endRadius);
+    animator->setListener(&sLifecycleChecker);
     return reinterpret_cast<jlong>( animator );
 }
 
@@ -166,9 +179,7 @@
 
 static void start(JNIEnv* env, jobject clazz, jlong animatorPtr, jobject finishListener) {
     BaseRenderNodeAnimator* animator = reinterpret_cast<BaseRenderNodeAnimator*>(animatorPtr);
-    if (finishListener) {
-        animator->setListener(new AnimationListenerBridge(env, finishListener));
-    }
+    animator->setListener(new AnimationListenerBridge(env, finishListener));
     animator->start();
 }
 
@@ -211,6 +222,9 @@
         LOG_FATAL_IF(! var, "Unable to find method " methodName);
 
 int register_android_view_RenderNodeAnimator(JNIEnv* env) {
+#ifdef USE_OPENGL_RENDERER
+    sLifecycleChecker.incStrong(0);
+#endif
     FIND_CLASS(gRenderNodeAnimatorClassInfo.clazz, kClassPathName);
     gRenderNodeAnimatorClassInfo.clazz = jclass(env->NewGlobalRef(gRenderNodeAnimatorClassInfo.clazz));