From 47cd7edf14b8a15f526085f206d8f73cb61c475f Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Mon, 25 Jan 2021 12:50:12 -0800
Subject: [PATCH] Fix race between initial updateRootLayoutSpecs and
 startSurface

Summary:
In some cases, onMeasure/onLayout are called on the RootView before `startSurface` in Fabric has been able to set surfaceId on the RootView.

With the new SurfaceMountingManager, this causes a crash because we need a valid surfaceId to perform an operation. Before the SurfaceMountingManager refactor, a surfaceId of 0 would be passed to `mBinding.setConstraints` in the FabricUIManager, and setConstraints in C++ noops if there's an invalid surfaceId.

For now, FabricUIManager will also fail silently if the surfaceId is invalid when updateRootLayoutSpecs is called, just to be conservative and to be consistent with previous behavior. This will be upgraded to a hard-crash in the future.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D26051266

fbshipit-source-id: ca2d80f899cdba9b3962af68546bd83b77be0680
---
 .../java/com/facebook/react/ReactRootView.java  | 17 +++++++++++++++--
 .../facebook/react/fabric/FabricUIManager.java  | 15 ++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java
index 6219ab08075..bbf8b5db68d 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java
@@ -85,7 +85,8 @@ public class ReactRootView extends FrameLayout implements RootView, ReactRoot {
   private @Nullable String mInitialUITemplate;
   private @Nullable CustomGlobalLayoutListener mCustomGlobalLayoutListener;
   private @Nullable ReactRootViewEventListener mRootViewEventListener;
-  private int mRootViewTag;
+  private int mRootViewTag =
+      0; /* This should be View.NO_ID, but for legacy reasons we haven't migrated yet */
   private boolean mIsAttachedToInstance;
   private boolean mShouldLogContentAppeared;
   private @Nullable JSTouchDispatcher mJSTouchDispatcher;
@@ -451,6 +452,14 @@ public class ReactRootView extends FrameLayout implements RootView, ReactRoot {
       FLog.w(TAG, "Unable to update root layout specs for uninitialized ReactInstanceManager");
       return;
     }
+    // In Fabric we cannot call `updateRootLayoutSpecs` until a SurfaceId has been set.
+    // Sometimes,
+    if (getUIManagerType() == FABRIC && !isRootViewTagSet()) {
+      ReactMarker.logMarker(ReactMarkerConstants.ROOT_VIEW_UPDATE_LAYOUT_SPECS_END);
+      FLog.e(TAG, "Unable to update root layout specs for ReactRootView: no rootViewTag set yet");
+      return;
+    }
+
     final ReactContext reactApplicationContext = mReactInstanceManager.getCurrentReactContext();
 
     if (reactApplicationContext != null) {
@@ -572,7 +581,7 @@ public class ReactRootView extends FrameLayout implements RootView, ReactRoot {
   public void setAppProperties(@Nullable Bundle appProperties) {
     UiThreadUtil.assertOnUiThread();
     mAppProperties = appProperties;
-    if (getRootViewTag() != 0) {
+    if (isRootViewTagSet()) {
       runApplication();
     }
   }
@@ -667,6 +676,10 @@ public class ReactRootView extends FrameLayout implements RootView, ReactRoot {
     return mRootViewTag;
   }
 
+  private boolean isRootViewTagSet() {
+    return mRootViewTag != 0 && mRootViewTag != NO_ID;
+  }
+
   public void setRootViewTag(int rootViewTag) {
     mRootViewTag = rootViewTag;
   }
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 c5aef40652d..51fa1b80c1e 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@@ -869,11 +869,20 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
       final int offsetY) {
 
     if (ENABLE_FABRIC_LOGS) {
-      FLog.d(TAG, "Updating Root Layout Specs");
+      FLog.d(TAG, "Updating Root Layout Specs for [%d]", surfaceId);
+    }
+
+    SurfaceMountingManager surfaceMountingManager = mMountingManager.getSurfaceManager(surfaceId);
+
+    // TODO T83615646: make this a hard-crash in the future.
+    if (surfaceMountingManager == null) {
+      ReactSoftException.logSoftException(
+          TAG,
+          new IllegalViewOperationException(
+              "Cannot updateRootLayoutSpecs on surfaceId that does not exist: " + surfaceId));
+      return;
     }
 
-    SurfaceMountingManager surfaceMountingManager =
-        mMountingManager.getSurfaceManagerEnforced(surfaceId, "updateRootLayoutSpecs");
     ThemedReactContext reactContext = surfaceMountingManager.getContext();
     boolean isRTL = false;
     boolean doLeftAndRightSwapInRTL = false;