From c5321e85147b007dc810a1065b05b48dbe96769f Mon Sep 17 00:00:00 2001
From: David Vacca <dvacca@fb.com>
Date: Fri, 25 Oct 2019 15:07:18 -0700
Subject: [PATCH] Refactor the cancellation of DispatchUIFrameCallback

Summary:
This diff refactors the stopping of DispatchUIFrameCallback on FabricUIManager to make it thread safe

Changelog: Refactor the cancellation of dispatching of Mounting operations for Fabric

Reviewed By: JoshuaGross

Differential Revision: D18010922

fbshipit-source-id: 305bc65576698cb785a2a2308cbd03db4a9a97e4
---
 .../react/fabric/FabricUIManager.java         | 24 +++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
index 39d9c75fa0e..fec5627bd89 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@@ -19,6 +19,7 @@ import static com.facebook.react.uimanager.common.UIManagerType.FABRIC;
 import android.annotation.SuppressLint;
 import android.os.SystemClock;
 import android.view.View;
+import androidx.annotation.AnyThread;
 import androidx.annotation.GuardedBy;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -121,9 +122,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
   @NonNull
   private final DispatchUIFrameCallback mDispatchUIFrameCallback;
 
-  @ThreadConfined(UI)
-  private volatile boolean mIsMountingEnabled = true;
-
   /**
    * This is used to keep track of whether or not the FabricUIManager has been destroyed. Once the
    * Catalyst instance is being destroyed, we should cease all operation here.
@@ -238,17 +236,22 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
     // This is not technically thread-safe, since it's read on the UI thread and written
     // here on the JS thread. We've marked it as volatile so that this writes to UI-thread
     // memory immediately.
-    mIsMountingEnabled = false;
+    mDispatchUIFrameCallback.stop();
 
     mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager);
     mEventDispatcher.unregisterEventEmitter(FABRIC);
 
     // Remove lifecycle listeners (onHostResume, onHostPause) since the FabricUIManager is going
-    // away. This and setting `mIsMountingEnabled` to false will cause the choreographer
+    // away. Then stop the mDispatchUIFrameCallback false will cause the choreographer
     // callbacks to stop firing.
     mReactApplicationContext.removeLifecycleEventListener(this);
     onHostPause();
 
+    // This is not technically thread-safe, since it's read on the UI thread and written
+    // here on the JS thread. We've marked it as volatile so that this writes to UI-thread
+    // memory immediately.
+    mDispatchUIFrameCallback.stop();
+
     mBinding.unregister();
     mBinding = null;
 
@@ -709,10 +712,17 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
 
   private class DispatchUIFrameCallback extends GuardedFrameCallback {
 
-    private DispatchUIFrameCallback(ReactContext reactContext) {
+    private volatile boolean mIsMountingEnabled = true;
+
+    private DispatchUIFrameCallback(@NonNull ReactContext reactContext) {
       super(reactContext);
     }
 
+    @AnyThread
+    void stop() {
+      mIsMountingEnabled = false;
+    }
+
     @Override
     public void doFrameGuarded(long frameTimeNanos) {
       if (!mIsMountingEnabled || mDestroyed) {
@@ -730,7 +740,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
 
       } catch (Exception ex) {
         FLog.i(ReactConstants.TAG, "Exception thrown when executing UIFrameGuarded", ex);
-        mIsMountingEnabled = false;
+        stop();
         throw ex;
       } finally {
         ReactChoreographer.getInstance()