From bdea479a1faa0f1f7d7c9d9162212cce94bc9720 Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Sat, 16 Jan 2021 11:03:39 -0800
Subject: [PATCH] Fix Android crash: mark re-created nodes in Differ

Summary:
Android has some optimizations around view allocation and pre-allocation that, in the case of View Unflattening, can cause "Create" mutations to be skipped.

To make sure that doesn't happen, we add a flag to ShadowViewMutation (in the core) that any platform can consume, that indicates if the mutation is a "recreation" mutation.

It is still a bit unclear why this is needed, in the sense that I would expect props revision to increment if a view is unflattened. However, there is at least one documented reproduction where that is *not* the case. So for now, we'll have a hack pending further investigation.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D25935785

fbshipit-source-id: 6fb4f0a6dedba0fe46ba3cd558ac1daa70f671f5
---
 .../java/com/facebook/react/fabric/jni/Binding.cpp   |  2 +-
 .../react/renderer/mounting/Differentiator.cpp       |  4 ++--
 .../react/renderer/mounting/ShadowViewMutation.cpp   | 12 ++++++++----
 .../react/renderer/mounting/ShadowViewMutation.h     |  5 ++++-
 4 files changed, 15 insertions(+), 8 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 b6161d0db09..30bffa9720e 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
@@ -553,7 +553,7 @@ void Binding::schedulerDidFinishTransaction(
     switch (mutationType) {
       case ShadowViewMutation::Create: {
         if (disablePreallocateViews_ ||
-            newChildShadowView.props->revision > 1) {
+            newChildShadowView.props->revision > 1 || mutation.recreated) {
           cppCommonMountItems.push_back(
               CppMountItem::CreateMountItem(newChildShadowView));
         }
diff --git a/ReactCommon/react/renderer/mounting/Differentiator.cpp b/ReactCommon/react/renderer/mounting/Differentiator.cpp
index 5545674c3e2..b1ca59c4839 100644
--- a/ReactCommon/react/renderer/mounting/Differentiator.cpp
+++ b/ReactCommon/react/renderer/mounting/Differentiator.cpp
@@ -973,8 +973,8 @@ static void calculateShadowViewMutationsV2(
                   parentShadowView,
                   newChildPair.shadowView,
                   newChildPair.mountIndex));
-              createMutations.push_back(
-                  ShadowViewMutation::CreateMutation(newChildPair.shadowView));
+              createMutations.push_back(ShadowViewMutation::CreateMutation(
+                  newChildPair.shadowView, true));
             } else {
               removeMutations.push_back(ShadowViewMutation::RemoveMutation(
                   parentShadowView,
diff --git a/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp b/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp
index dbf4afd227c..fcc9b73d0d9 100644
--- a/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp
+++ b/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp
@@ -10,14 +10,16 @@
 namespace facebook {
 namespace react {
 
-ShadowViewMutation ShadowViewMutation::CreateMutation(ShadowView shadowView) {
+ShadowViewMutation ShadowViewMutation::CreateMutation(
+    ShadowView shadowView,
+    bool recreated) {
   return {
       /* .type = */ Create,
       /* .parentShadowView = */ {},
       /* .oldChildShadowView = */ {},
       /* .newChildShadowView = */ shadowView,
       /* .index = */ -1,
-  };
+      /* .recreated = */ recreated};
 }
 
 ShadowViewMutation ShadowViewMutation::DeleteMutation(ShadowView shadowView) {
@@ -27,6 +29,7 @@ ShadowViewMutation ShadowViewMutation::DeleteMutation(ShadowView shadowView) {
       /* .oldChildShadowView = */ shadowView,
       /* .newChildShadowView = */ {},
       /* .index = */ -1,
+      /* .recreated = */ false,
   };
 }
 
@@ -40,6 +43,7 @@ ShadowViewMutation ShadowViewMutation::InsertMutation(
       /* .oldChildShadowView = */ {},
       /* .newChildShadowView = */ childShadowView,
       /* .index = */ index,
+      /* .recreated = */ false,
   };
 }
 
@@ -53,7 +57,7 @@ ShadowViewMutation ShadowViewMutation::RemoveMutation(
       /* .oldChildShadowView = */ childShadowView,
       /* .newChildShadowView = */ {},
       /* .index = */ index,
-  };
+      /* .recreated = */ false};
 }
 
 ShadowViewMutation ShadowViewMutation::UpdateMutation(
@@ -65,7 +69,7 @@ ShadowViewMutation ShadowViewMutation::UpdateMutation(
       /* .oldChildShadowView = */ oldChildShadowView,
       /* .newChildShadowView = */ newChildShadowView,
       /* .index = */ -1,
-  };
+      /* .recreated = */ false};
 }
 
 #if RN_DEBUG_STRING_CONVERTIBLE
diff --git a/ReactCommon/react/renderer/mounting/ShadowViewMutation.h b/ReactCommon/react/renderer/mounting/ShadowViewMutation.h
index 3bd0067d5fd..1ea0adbe51f 100644
--- a/ReactCommon/react/renderer/mounting/ShadowViewMutation.h
+++ b/ReactCommon/react/renderer/mounting/ShadowViewMutation.h
@@ -28,7 +28,9 @@ struct ShadowViewMutation final {
   /*
    * Creates and returns an `Create` mutation.
    */
-  static ShadowViewMutation CreateMutation(ShadowView shadowView);
+  static ShadowViewMutation CreateMutation(
+      ShadowView shadowView,
+      bool recreated = false);
 
   /*
    * Creates and returns an `Delete` mutation.
@@ -69,6 +71,7 @@ struct ShadowViewMutation final {
   ShadowView oldChildShadowView = {};
   ShadowView newChildShadowView = {};
   int index = -1;
+  bool recreated; // for Create mutations, for platform-specific optimizations
 };
 
 using ShadowViewMutationList = std::vector<ShadowViewMutation>;