Merge "Fix SurfaceViewPositionListener race bugs"

3 files changed, 25 insertions, 33 deletions

diff --git a/core/jni/android_view_RenderNode.cpp b/core/jni/android_view_RenderNode.cpp
index 4fc546c98e97..b0028e1b6f5b 100644
--- a/core/jni/android_view_RenderNode.cpp
+++ b/core/jni/android_view_RenderNode.cpp
@@ -573,8 +573,9 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject,
                 bounds.roundOut();
             }
 
+            incStrong(0);
             auto functor = std::bind(
-                std::mem_fn(&SurfaceViewPositionUpdater::doUpdatePosition), this,
+                std::mem_fn(&SurfaceViewPositionUpdater::doUpdatePositionAsync), this,
                 (jlong) info.canvasContext.getFrameNumber(),
                 (jint) bounds.left, (jint) bounds.top,
                 (jint) bounds.right, (jint) bounds.bottom);
@@ -585,15 +586,18 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject,
         virtual void onPositionLost(RenderNode& node, const TreeInfo* info) override {
             if (CC_UNLIKELY(!mWeakRef || (info && !info->updateWindowPositions))) return;
 
-            if (info) {
-                auto functor = std::bind(
-                    std::mem_fn(&SurfaceViewPositionUpdater::doNotifyPositionLost), this,
-                    (jlong) info->canvasContext.getFrameNumber());
-
-                info->canvasContext.enqueueFrameWork(std::move(functor));
-            } else {
-                doNotifyPositionLost(0);
+            ATRACE_NAME("SurfaceView position lost");
+            JNIEnv* env = jnienv();
+            jobject localref = env->NewLocalRef(mWeakRef);
+            if (CC_UNLIKELY(!localref)) {
+                jnienv()->DeleteWeakGlobalRef(mWeakRef);
+                mWeakRef = nullptr;
+                return;
             }
+
+            env->CallVoidMethod(localref, gSurfaceViewPositionLostMethod,
+                    info ? info->canvasContext.getFrameNumber() : 0);
+            env->DeleteLocalRef(localref);
         }
 
     private:
@@ -605,36 +609,23 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject,
             return env;
         }
 
-        void doUpdatePosition(jlong frameNumber, jint left, jint top,
+        void doUpdatePositionAsync(jlong frameNumber, jint left, jint top,
                 jint right, jint bottom) {
             ATRACE_NAME("Update SurfaceView position");
 
             JNIEnv* env = jnienv();
             jobject localref = env->NewLocalRef(mWeakRef);
             if (CC_UNLIKELY(!localref)) {
-                jnienv()->DeleteWeakGlobalRef(mWeakRef);
-                mWeakRef = nullptr;
-                return;
-            }
-
-            env->CallVoidMethod(localref, gSurfaceViewPositionUpdateMethod,
-                    frameNumber, left, top, right, bottom);
-            env->DeleteLocalRef(localref);
-        }
-
-        void doNotifyPositionLost(jlong frameNumber) {
-            ATRACE_NAME("SurfaceView position lost");
-
-            JNIEnv* env = jnienv();
-            jobject localref = env->NewLocalRef(mWeakRef);
-            if (CC_UNLIKELY(!localref)) {
-                jnienv()->DeleteWeakGlobalRef(mWeakRef);
+                env->DeleteWeakGlobalRef(mWeakRef);
                 mWeakRef = nullptr;
-                return;
+            } else {
+                env->CallVoidMethod(localref, gSurfaceViewPositionUpdateMethod,
+                        frameNumber, left, top, right, bottom);
+                env->DeleteLocalRef(localref);
             }
 
-            env->CallVoidMethod(localref, gSurfaceViewPositionLostMethod, frameNumber);
-            env->DeleteLocalRef(localref);
+            // We need to release ourselves here
+            decStrong(0);
         }
 
         JavaVM* mVm;
diff --git a/libs/hwui/RenderNode.h b/libs/hwui/RenderNode.h
index 1eaf5d61d465..f9735a231d7a 100644
--- a/libs/hwui/RenderNode.h
+++ b/libs/hwui/RenderNode.h
@@ -232,7 +232,7 @@ public:
     // the frameNumber to appropriately batch/synchronize these transactions.
     // There is no other filtering/batching to ensure that only the "final"
     // state called once per frame.
-    class ANDROID_API PositionListener {
+    class ANDROID_API PositionListener : public VirtualLightRefBase {
     public:
         virtual ~PositionListener() {}
         // Called when the RenderNode's position changes
@@ -247,7 +247,7 @@ public:
     // before the RenderNode is used for drawing.
     // RenderNode takes ownership of the pointer
     ANDROID_API void setPositionListener(PositionListener* listener) {
-        mPositionListener.reset(listener);
+        mPositionListener = listener;
     }
 
     // This is only modified in MODE_FULL, so it can be safely accessed
@@ -366,7 +366,7 @@ private:
     // mDisplayList, not mStagingDisplayList.
     uint32_t mParentCount;
 
-    std::unique_ptr<PositionListener> mPositionListener;
+    sp<PositionListener> mPositionListener;
 }; // class RenderNode
 
 } /* namespace uirenderer */
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index c626c5452310..75e7fdf13091 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -783,6 +783,7 @@ void CanvasContext::enqueueFrameWork(std::function<void()>&& func) {
     }
     sp<FuncTask> task(new FuncTask());
     task->func = func;
+    mFrameFences.push_back(task);
     mFrameWorkProcessor->add(task);
 }