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

Commit 72d6e4fa authored by John Reck's avatar John Reck
Browse files

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
parent 08229e81

core/java/android/view/RenderNodeAnimator.java

+16 −5
Original line number Diff line number Diff line
@@ -189,9 +189,6 @@ public class RenderNodeAnimator extends Animator { 
    }



    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 @@ public class RenderNodeAnimator extends Animator { 
            mViewTarget.mTransformationInfo.mAlpha = mFinalValue;

        }



        notifyStartListeners();

        moveToRunningState();



        if (mViewTarget != null) {

            // Kick off a frame to start the process
@@ -209,6 +206,12 @@ public class RenderNodeAnimator extends Animator { 
        }

    }



    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 @@ public class RenderNodeAnimator extends Animator { 
        if (mState != STATE_PREPARE && mState != STATE_FINISHED) {

            if (mState == STATE_DELAYED) {

                getHelper().removeDelayedAnimation(this);

                notifyStartListeners();

                moveToRunningState();

            }

            nEnd(mNativePtr.get());
@@ -242,7 +245,15 @@ public class RenderNodeAnimator extends Animator { 
    @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

+17 −3
Original line number Diff line number Diff line
@@ -45,6 +45,15 @@ static JNIEnv* getEnv(JavaVM* vm) { 
    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 @@ static jlong createAnimator(JNIEnv* env, jobject clazz, 
        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 @@ static jlong createCanvasPropertyFloatAnimator(JNIEnv* env, jobject clazz, 
        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 @@ static jlong createCanvasPropertyPaintAnimator(JNIEnv* env, jobject clazz, 
    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 setAllowRunningAsync(JNIEnv* env, jobject clazz, jlong animatorPtr, 


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->start();

}
@@ -211,6 +222,9 @@ static JNINativeMethod gMethods[] = { 
        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));