Unmount React applications when re-creating context [RFC]

Summary:
Currently in RN Android when re-creating the context we don't unmount the underlying React application. This violates ViewManager and React hooks contracts, who are no longer able to properly unmount views, and instead the view hierarchy is forcefully torn down by Android UI. This differs from iOS, where we do unmount the application on reloads.

This is a trade-off with performance, as we'll keep the JS thread alive slightly longer to complete shutdown, but it's the right call for correctness. It also only mainly affects development, as recreating the context is rare in production.

Repro steps:

```
  useEffect(() => {
    console.log('Playground useEffect invoked');
    return () => {
      console.log('Playground useEffect destructor invoked');
    };
  }, []);
```

Validate that when reloading the application, the second console.log is printed.

Changelog: [Android][Changed] React trees will be unmounted when the application is reloaded

Differential Revision: D45145520

fbshipit-source-id: 6800bba918b5d0a8cfac3a835eda0b397f47e480

Author: javache

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java

@@ -92,7 +92,6 @@
 import com.facebook.react.modules.core.DeviceEventManagerModule;
 import com.facebook.react.modules.core.ReactChoreographer;
 import com.facebook.react.modules.debug.interfaces.DeveloperSettings;
-import com.facebook.react.modules.fabric.ReactFabric;
 import com.facebook.react.packagerconnection.RequestHandler;
 import com.facebook.react.surface.ReactStage;
 import com.facebook.react.turbomodule.core.TurboModuleManager;
@@ -102,6 +101,7 @@
 import com.facebook.react.uimanager.ReactRoot;
 import com.facebook.react.uimanager.UIManagerHelper;
 import com.facebook.react.uimanager.ViewManager;
+import com.facebook.react.uimanager.common.UIManagerType;
 import com.facebook.react.views.imagehelper.ResourceDrawableIdHelper;
 import com.facebook.soloader.SoLoader;
 import com.facebook.systrace.Systrace;
@@ -855,7 +855,10 @@ private void clearReactRoot(ReactRoot reactRoot) {
    * with the provided reactRoot reactRoot will be started asynchronously, i.e this method won't
    * block. This reactRoot will then be tracked by this manager and in case of catalyst instance
    * restart it will be re-attached.
+   *
+   * @deprecated This method should be internal to ReactRootView and ReactInstanceManager
    */
+  @Deprecated
   @ThreadConfined(UI)
   public void attachRootView(ReactRoot reactRoot) {
     UiThreadUtil.assertOnUiThread();
@@ -865,35 +868,37 @@ public void attachRootView(ReactRoot reactRoot) {
     // Ideally reactRoot should be initialized with id == NO_ID
     if (mAttachedReactRoots.add(reactRoot)) {
       clearReactRoot(reactRoot);
+    } else {
+      FLog.e(ReactConstants.TAG, "ReactRoot was attached multiple times");
     }
 
     // If react context is being created in the background, JS application will be started
     // automatically when creation completes, as reactRoot is part of the attached
     // reactRoot list.
     ReactContext currentContext = getCurrentReactContext();
     if (mCreateReactContextThread == null && currentContext != null) {
-      if (reactRoot.getState().compareAndSet(ReactRoot.STATE_STOPPED, ReactRoot.STATE_STARTED)) {
-        attachRootViewToInstance(reactRoot);
-      }
+      attachRootViewToInstance(reactRoot);
     }
   }
 
   /**
    * Detach given {@param reactRoot} from current catalyst instance. It's safe to call this method
    * multiple times on the same {@param reactRoot} - in that case view will be detached with the
    * first call.
+   *
+   * @deprecated This method should be internal to ReactRootView and ReactInstanceManager
    */
+  @Deprecated
   @ThreadConfined(UI)
   public void detachRootView(ReactRoot reactRoot) {
     UiThreadUtil.assertOnUiThread();
-    synchronized (mAttachedReactRoots) {
-      if (mAttachedReactRoots.contains(reactRoot)) {
-        ReactContext currentContext = getCurrentReactContext();
-        mAttachedReactRoots.remove(reactRoot);
-        if (currentContext != null && currentContext.hasActiveReactInstance()) {
-          detachViewFromInstance(reactRoot, currentContext.getCatalystInstance());
-        }
-      }
+    if (!mAttachedReactRoots.remove(reactRoot)) {
+      return;
+    }
+
+    ReactContext reactContext = mCurrentReactContext;
+    if (reactContext != null && reactContext.hasActiveReactInstance()) {
+      detachRootViewFromInstance(reactRoot, reactContext);
     }
   }
 
@@ -1150,9 +1155,7 @@ private void setupReactContext(final ReactApplicationContext reactContext) {
 
       ReactMarker.logMarker(ATTACH_MEASURED_ROOT_VIEWS_START);
       for (ReactRoot reactRoot : mAttachedReactRoots) {
-        if (reactRoot.getState().compareAndSet(ReactRoot.STATE_STOPPED, ReactRoot.STATE_STARTED)) {
-          attachRootViewToInstance(reactRoot);
-        }
+        attachRootViewToInstance(reactRoot);
       }
       ReactMarker.logMarker(ATTACH_MEASURED_ROOT_VIEWS_END);
     }
@@ -1194,6 +1197,11 @@ private void setupReactContext(final ReactApplicationContext reactContext) {
 
   private void attachRootViewToInstance(final ReactRoot reactRoot) {
     FLog.d(ReactConstants.TAG, "ReactInstanceManager.attachRootViewToInstance()");
+    if (!reactRoot.getState().compareAndSet(ReactRoot.STATE_STOPPED, ReactRoot.STATE_STARTED)) {
+      // Already started
+      return;
+    }
+
     Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "attachRootViewToInstance");
 
     @Nullable
@@ -1210,7 +1218,6 @@ private void attachRootViewToInstance(final ReactRoot reactRoot) {
     @Nullable Bundle initialProperties = reactRoot.getAppProperties();
 
     final int rootTag;
-
     if (reactRoot.getUIManagerType() == FABRIC) {
       rootTag =
           uiManager.startSurface(
@@ -1245,18 +1252,48 @@ private void attachRootViewToInstance(final ReactRoot reactRoot) {
     Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
   }
 
-  private void detachViewFromInstance(ReactRoot reactRoot, CatalystInstance catalystInstance) {
-    FLog.d(ReactConstants.TAG, "ReactInstanceManager.detachViewFromInstance()");
+  private void detachRootViewFromInstance(ReactRoot reactRoot, ReactContext reactContext) {
+    FLog.d(ReactConstants.TAG, "ReactInstanceManager.detachRootViewFromInstance()");
     UiThreadUtil.assertOnUiThread();
-    if (reactRoot.getUIManagerType() == FABRIC) {
-      catalystInstance
-          .getJSModule(ReactFabric.class)
-          .unmountComponentAtNode(reactRoot.getRootViewTag());
+
+    if (!reactRoot.getState().compareAndSet(ReactRoot.STATE_STARTED, ReactRoot.STATE_STOPPED)) {
+      // ReactRoot was already stopped
+      return;
+    }
+
+    @UIManagerType int uiManagerType = reactRoot.getUIManagerType();
+    if (uiManagerType == UIManagerType.FABRIC) {
+      // Stop surface in Fabric.
+      // Calling FabricUIManager.stopSurface causes the C++ Binding.stopSurface
+      // to be called synchronously over the JNI, which causes an empty tree
+      // to be committed via the Scheduler, which will cause mounting instructions
+      // to be queued up and synchronously executed to delete and remove
+      // all the views in the hierarchy.
+      final int surfaceId = reactRoot.getRootViewTag();
+      if (surfaceId != View.NO_ID) {
+        UIManager uiManager = UIManagerHelper.getUIManager(reactContext, uiManagerType);
+        if (uiManager != null) {
+          uiManager.stopSurface(surfaceId);
+        } else {
+          FLog.w(ReactConstants.TAG, "Failed to stop surface, UIManager has already gone away");
+          reactRoot.getRootViewGroup().removeAllViews();
+        }
+      } else {
+        ReactSoftExceptionLogger.logSoftException(
+            TAG,
+            new RuntimeException(
+                "detachRootViewFromInstance called with ReactRootView with invalid id"));
+        reactRoot.getRootViewGroup().removeAllViews();
+      }
     } else {
-      catalystInstance
+      reactContext
+          .getCatalystInstance()
           .getJSModule(AppRegistry.class)
           .unmountApplicationComponentAtRootTag(reactRoot.getRootViewTag());
     }
+
+    // The view is no longer attached, so mark it as such by resetting its ID.
+    reactRoot.getRootViewGroup().setId(View.NO_ID);
   }
 
   @ThreadConfined(UI)
@@ -1269,7 +1306,7 @@ private void tearDownReactContext(ReactContext reactContext) {
 
     synchronized (mAttachedReactRoots) {
       for (ReactRoot reactRoot : mAttachedReactRoots) {
-        clearReactRoot(reactRoot);
+        detachRootViewFromInstance(reactRoot, reactContext);
       }
     }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java

@@ -611,41 +611,6 @@ private void updateRootLayoutSpecs(
   @ThreadConfined(UI)
   public void unmountReactApplication() {
     UiThreadUtil.assertOnUiThread();
-    // Stop surface in Fabric.
-    // Calling FabricUIManager.stopSurface causes the C++ Binding.stopSurface
-    // to be called synchronously over the JNI, which causes an empty tree
-    // to be committed via the Scheduler, which will cause mounting instructions
-    // to be queued up and synchronously executed to delete and remove
-    // all the views in the hierarchy.
-    if (hasActiveReactInstance()) {
-      final ReactContext reactApplicationContext = getCurrentReactContext();
-      if (reactApplicationContext != null && isFabric()) {
-        @Nullable
-        UIManager uiManager =
-            UIManagerHelper.getUIManager(reactApplicationContext, getUIManagerType());
-        if (uiManager != null) {
-          final int surfaceId = this.getId();
-
-          // In case of "retry" or error dialogues being shown, this ReactRootView could be
-          // reused (with the same surfaceId, or a different one). Ensure the ReactRootView
-          // is marked as unused in case of that.
-          setId(NO_ID);
-
-          // Remove all children from ReactRootView
-          removeAllViews();
-
-          if (surfaceId == NO_ID) {
-            ReactSoftExceptionLogger.logSoftException(
-                TAG,
-                new RuntimeException(
-                    "unmountReactApplication called on ReactRootView with invalid id"));
-          } else {
-            uiManager.stopSurface(surfaceId);
-          }
-        }
-      }
-    }
-
     if (mReactInstanceManager != null && mIsAttachedToInstance) {
       mReactInstanceManager.detachRootView(this);
       mIsAttachedToInstance = false;