From d9f2491a713d872f2f3c8447dbf789fb17b94524 Mon Sep 17 00:00:00 2001
From: Xin Chen <xch@meta.com>
Date: Mon, 6 Feb 2023 19:12:46 -0800
Subject: [PATCH] Fix edge case when layout animation caused delete and create
 mutations in the same batch

Summary:
This is a two step (2/2) fix to a race that could caused a DELETE...CREATE mutations being sent over to the fabric mounting layer. Such combination was assumed not possible from the differ (https://fburl.com/code/kg8z9t4w), yet it happened at least in the existence of layout animation and when certain commits happen while animation is active.

This diff fixes all potential races in the Fabric mounting layer directly. It captures all the `DELETE...CREATE` combinations and stop those from passing down to the native platforms. This should fix all such races should them not captured by the fix in the layout animation.

To help understand other races better, I also logged here to indicate such race so that future crashes will have more context.

Changelog:
[General][Fixed] - Fix edge case when layout animation caused delete and create mutations in the same batch

Reviewed By: javache

Differential Revision: D41900201

fbshipit-source-id: 280502ca32ce87a9e483cd859b11bcd3e5c4a435
---
 .../react/config/ReactFeatureFlags.java       |  6 ++++
 .../react/fabric/FabricMountingManager.cpp    | 32 +++++++++++++++++--
 .../jni/react/fabric/FabricMountingManager.h  |  1 +
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
index 843bc6acc2dd07..44dbc60da5b8c6 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
@@ -115,4 +115,10 @@ public class ReactFeatureFlags {
    * state in Fabric SurfaceMountingManager.
    */
   public static boolean reduceDeleteCreateMutationLayoutAnimation = true;
+
+  /**
+   * Allow fix to drop delete...create mutations which could cause missing view state in Fabric
+   * SurfaceMountingManager.
+   */
+  public static boolean reduceDeleteCreateMutation = false;
 }
diff --git a/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp b/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
index 707ea46e50debb..b1097967515bfd 100644
--- a/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
+++ b/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
@@ -40,7 +40,9 @@ FabricMountingManager::FabricMountingManager(
     std::shared_ptr<const ReactNativeConfig> &config,
     global_ref<jobject> &javaUIManager)
     : javaUIManager_(javaUIManager),
-      useOverflowInset_(getFeatureFlagValue("useOverflowInset")) {
+      useOverflowInset_(getFeatureFlagValue("useOverflowInset")),
+      reduceDeleteCreateMutation_(
+          getFeatureFlagValue("reduceDeleteCreateMutation")) {
   CoreFeatures::enableMapBuffer = getFeatureFlagValue("useMapBufferProps");
 }
 
@@ -314,9 +316,33 @@ void FabricMountingManager::executeMount(
       bool isVirtual = mutation.mutatedViewIsVirtual();
       switch (mutationType) {
         case ShadowViewMutation::Create: {
-          bool allocationCheck =
+          bool shouldCreateView =
               !allocatedViewTags.contains(newChildShadowView.tag);
-          bool shouldCreateView = allocationCheck;
+          if (reduceDeleteCreateMutation_) {
+            // Detect DELETE...CREATE situation on the same node and do NOT push
+            // back to the mount items. This is an edge case that may happen
+            // when for example animation runs while commit happened, and we
+            // want to filter them out here to capture all possible sources of
+            // such mutations. The re-ordering logic here assumes no
+            // DELETE...CREATE in the mutations, as we will re-order mutations
+            // and batch all DELETE instructions in the end.
+            auto it = std::remove_if(
+                cppDeleteMountItems.begin(),
+                cppDeleteMountItems.end(),
+                [&](auto &deletedMountItem) -> bool {
+                  return deletedMountItem.oldChildShadowView.tag ==
+                      newChildShadowView.tag;
+                });
+            bool hasDeletedViewsWithSameTag = it != cppDeleteMountItems.end();
+            cppDeleteMountItems.erase(it, cppDeleteMountItems.end());
+
+            if (hasDeletedViewsWithSameTag) {
+              shouldCreateView = false;
+              LOG(ERROR)
+                  << "XIN: Detect DELETE...CREATE on the same tag from mutations in the same batch. The DELETE and CREATE mutations are removed before sending to the native platforms";
+            }
+          }
+
           if (shouldCreateView) {
             cppCommonMountItems.push_back(
                 CppMountItem::CreateMountItem(newChildShadowView));
diff --git a/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h b/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h
index 57e9eff06a6244..37440bf50198b0 100644
--- a/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h
+++ b/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h
@@ -70,6 +70,7 @@ class FabricMountingManager final {
   std::recursive_mutex allocatedViewsMutex_;
 
   bool const useOverflowInset_{false};
+  bool const reduceDeleteCreateMutation_{false};
 
   jni::local_ref<jobject> getProps(
       ShadowView const &oldShadowView,