From a20939f49d5a7daf77c50e27bc7b9e149fe6b6cd Mon Sep 17 00:00:00 2001
From: David Vacca <dvacca@fb.com>
Date: Sat, 9 Nov 2019 10:36:30 -0800
Subject: [PATCH] Fix Collapsing of mutation instructions

Summary:
This diff fixes the algorithm that collapses mutation instructions in Fabric Android
The problem was that we were always relying on the fact that the oldShadowNode is present as part of the mutation instruction (which is not correct for mutation instructions of type CREATE)

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D18411360

fbshipit-source-id: 36f5e75f792db87003fcaef2ddcd9452415a0ad6
---
 .../com/facebook/react/fabric/jni/Binding.cpp | 32 +++++++++++++++----
 1 file changed, 26 insertions(+), 6 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 0f992e70a2f..e54dd19081c 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
@@ -574,13 +574,24 @@ void Binding::schedulerDidFinishTransaction(
   auto &mutations = mountingTransaction->getMutations();
 
   facebook::better::set<int> createAndDeleteTagsToProcess;
+  // When collapseDeleteCreateMountingInstructions_ is enabled, the
+  // createAndDeleteTagsToProcess set will contain all the tags belonging to
+  // CREATE and DELETE mutation instructions that needs to be processed. If a
+  // CREATE or DELETE mutation instruction does not belong in the set, it means
+  // that the we received a pair of mutation instructions: DELETE - CREATE and
+  // it is not necessary to create or delete on the screen.
   if (collapseDeleteCreateMountingInstructions_) {
     for (const auto &mutation : mutations) {
       if (mutation.type == ShadowViewMutation::Delete) {
+        // TAG on 'Delete' mutation instructions are part of the
+        // oldChildShadowView
         createAndDeleteTagsToProcess.insert(mutation.oldChildShadowView.tag);
       } else if (mutation.type == ShadowViewMutation::Create) {
-        int tag = mutation.oldChildShadowView.tag;
-        if (createAndDeleteTagsToProcess.find(tag) != createAndDeleteTagsToProcess.end()) {
+        // TAG on 'Create' mutation instructions are part of the
+        // newChildShadowView
+        int tag = mutation.newChildShadowView.tag;
+        if (createAndDeleteTagsToProcess.find(tag) ==
+            createAndDeleteTagsToProcess.end()) {
           createAndDeleteTagsToProcess.insert(tag);
         } else {
           createAndDeleteTagsToProcess.erase(tag);
@@ -610,10 +621,19 @@ void Binding::schedulerDidFinishTransaction(
     auto mutationType = mutation.type;
 
     if (collapseDeleteCreateMountingInstructions_ &&
-        (mutationType == ShadowViewMutation::Create || mutationType == ShadowViewMutation::Delete) &&
-        createAndDeleteTagsToProcess.size() > 0 &&
-        createAndDeleteTagsToProcess.find(mutation.newChildShadowView.tag) == createAndDeleteTagsToProcess.end())  {
-      continue;
+        (mutationType == ShadowViewMutation::Create ||
+         mutationType == ShadowViewMutation::Delete) &&
+        createAndDeleteTagsToProcess.size() > 0) {
+      // The TAG on 'Delete' mutation instructions are part of the
+      // oldChildShadowView. On the other side, the TAG on 'Create' mutation
+      // instructions are part of the newChildShadowView
+      int tag = mutationType == ShadowViewMutation::Create
+          ? mutation.newChildShadowView.tag
+          : mutation.oldChildShadowView.tag;
+      if (createAndDeleteTagsToProcess.find(tag) ==
+          createAndDeleteTagsToProcess.end()) {
+        continue;
+      }
     }
 
     bool isVirtual = newChildShadowView.layoutMetrics == EmptyLayoutMetrics &&