From 225ff72d2de55705c2631aa4c274065695554a5e Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Thu, 17 Jun 2021 17:27:11 -0700
Subject: [PATCH] Temporary mitigation for T93398735 crash: swallow PreAllocate
 crashes

Summary:
Because of the "disable preallocation of virtual views" experiment, for some reason, some views are being preallocated multiple times instead of not being preallocated at all.

This isn't really a problem for CreateView, and Preallocate is actually more strict here than it needs to be. I'm going to downgrade this to a soft error and will continue to analyze more. This is more of a perf issue than a correctness issue, so this should be fine.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D29206416

fbshipit-source-id: 9490a1a705c2b39def3a3e56086634439543515e
---
 .../mounting/SurfaceMountingManager.java      | 49 +++++++++++++++++--
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
index 4047935cea4..ba39ab4a613 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
@@ -19,6 +19,7 @@ import androidx.annotation.UiThread;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.infer.annotation.ThreadConfined;
+import com.facebook.react.bridge.ReactNoCrashSoftException;
 import com.facebook.react.bridge.ReactSoftException;
 import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;
@@ -535,10 +536,44 @@ public class SurfaceMountingManager {
     if (isStopped()) {
       return;
     }
+    // We treat this as a perf problem and not a logical error. View Preallocation or unexpected
+    // changes to Differ or C++ Binding could cause some redundant Create instructions.
+    // This is a NoCrash soft exception because we know there are cases where preallocation happens
+    // and a node is recreated: if a node is preallocated and then committed with revision 2+,
+    // an extra CREATE instruction will be generated.
+    // This represents a perf issue only, not a correctness issue. In the future we need to
+    // refactor View preallocation to correct the currently incorrect assumptions.
     if (getNullableViewState(reactTag) != null) {
+      ReactSoftException.logSoftException(
+          TAG,
+          new ReactNoCrashSoftException(
+              "Cannot CREATE view with tag [" + reactTag + "], already exists."));
       return;
     }
 
+    createViewUnsafe(
+        componentName, reactTag, props, stateWrapper, eventEmitterWrapper, isLayoutable);
+  }
+
+  /**
+   * Perform view creation without any safety checks. You must ensure safety before calling this
+   * method (see existing callsites).
+   *
+   * @param componentName
+   * @param reactTag
+   * @param props
+   * @param stateWrapper
+   * @param eventEmitterWrapper
+   * @param isLayoutable
+   */
+  @UiThread
+  public void createViewUnsafe(
+      @NonNull String componentName,
+      int reactTag,
+      @Nullable ReadableMap props,
+      @Nullable StateWrapper stateWrapper,
+      @Nullable EventEmitterWrapper eventEmitterWrapper,
+      boolean isLayoutable) {
     View view = null;
     ViewManager viewManager = null;
 
@@ -859,16 +894,22 @@ public class SurfaceMountingManager {
       @Nullable EventEmitterWrapper eventEmitterWrapper,
       boolean isLayoutable) {
     UiThreadUtil.assertOnUiThread();
+
     if (isStopped()) {
       return;
     }
-
+    // We treat this as a perf problem and not a logical error. View Preallocation or unexpected
+    // changes to Differ or C++ Binding could cause some redundant Create instructions.
     if (getNullableViewState(reactTag) != null) {
-      throw new IllegalStateException(
-          "View for component " + componentName + " with tag " + reactTag + " already exists.");
+      ReactSoftException.logSoftException(
+          TAG,
+          new IllegalStateException(
+              "Cannot Preallocate view with tag [" + reactTag + "], already exists."));
+      return;
     }
 
-    createView(componentName, reactTag, props, stateWrapper, eventEmitterWrapper, isLayoutable);
+    createViewUnsafe(
+        componentName, reactTag, props, stateWrapper, eventEmitterWrapper, isLayoutable);
   }
 
   @AnyThread