From dd994750321c80c7d70ac78187ac83fd2a612e99 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 29 Sep 2021 08:00:53 -0700 Subject: [PATCH] Fix view preallocation during cloning Summary: changelog: [internal] This code assured that view preallocation is only triggered if ShadowNode is cloned from revision 0 to revision 1 (first time ShadowNode is cloned). Node can go from virtual to view forming in subsequent clones, not just the first one. This is more of a case in Concurrent React where the node can be cloned many times before it is first mounted. Reviewed By: mdvacca Differential Revision: D31237719 fbshipit-source-id: 13fe6d10fdc815dbdae785d1d9d86d1c8fd36be4 --- .../com/facebook/react/fabric/jni/Binding.cpp | 15 ++++++++++----- .../java/com/facebook/react/fabric/jni/Binding.h | 1 + .../fabric/mounting/SurfaceMountingManager.java | 4 ---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index 1a4bf7b58bd1d0..e07977cb8b4c8a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -508,6 +508,9 @@ void Binding::installFabricUIManager( enableFabricLogs_ = config->getBool("react_fabric:enabled_android_fabric_logs"); + disableRevisionCheckForPreallocation_ = + config->getBool("react_fabric:disable_revision_check_for_preallocation"); + if (enableFabricLogs_) { LOG(WARNING) << "Binding::installFabricUIManager() was called (address: " << this << ")."; @@ -1242,11 +1245,13 @@ void Binding::schedulerDidCloneShadowNode( // 1. The revision is exactly 1 // 2. At revision 0 (the old node), View Preallocation would have been skipped - if (newShadowNode.getProps()->revision != 1) { - return; - } - if (oldShadowNode.getProps()->revision != 0) { - return; + if (!disableRevisionCheckForPreallocation_) { + if (newShadowNode.getProps()->revision != 1) { + return; + } + if (oldShadowNode.getProps()->revision != 0) { + return; + } } // If the new node is concrete and the old wasn't, we can preallocate diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h index 5ae4f0189e29ba..9750a51bf2376d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h @@ -201,6 +201,7 @@ class Binding : public jni::HybridClass, bool disablePreallocateViews_{false}; bool enableFabricLogs_{false}; bool enableEarlyEventEmitterUpdate_{false}; + bool disableRevisionCheckForPreallocation_{false}; }; } // namespace react 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 f93d1c20dcadf8..737a29559aab5f 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 @@ -910,10 +910,6 @@ public void preallocateView( // 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) { - ReactSoftExceptionLogger.logSoftException( - TAG, - new IllegalStateException( - "Cannot Preallocate view with tag [" + reactTag + "], already exists.")); return; }