Fix differentiator emitting updates with incorrect parentTag

Summary:
Address the test-case identified in D66557919, where Differentiator could emit updates for views referencing an incorrect parentTag.

The longer-term fix here is to avoid emitting any updates for nodes which are being reparented, but that requires bigger changes, including to the LayoutAnimation system. As a short-term patch, we're passing through an explicit `parentShadowViewForUpdate` which will be used as the current parent for update purposes.

 {F1971278019} 

Changelog: [Android][Fixed] Fix Fabric mutations sometimes triggering a `getViewState` crash when referencing an invalid parentTag.

Differential Revision: D66654293

Author: javache

packages/react-native/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp

@@ -392,6 +392,11 @@ LayoutAnimationKeyFrameManager::pullTransaction(
                 if (keyframe.type == AnimationConfigurationType::Update &&
                     mutation.newChildShadowView.tag > 0) {
                   keyframe.viewPrev = mutation.newChildShadowView;
+                  keyframe.parentView = mutation.parentShadowView;
+                  react_native_assert(
+                      keyframe.finalMutationsForKeyFrame.size() == 1);
+                  keyframe.finalMutationsForKeyFrame[0].parentShadowView =
+                      mutation.parentShadowView;
                 }
               }
             }

packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp

@@ -403,6 +403,7 @@ static void calculateShadowViewMutationsFlattener(
     const ShadowView& parentShadowView,
     TinyMap<Tag, ShadowViewNodePair*>& unvisitedOtherNodes,
     const ShadowViewNodePair& node,
+    const ShadowView& parentShadowViewForUpdate,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherNewNodes = nullptr,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherOldNodes = nullptr);
 
@@ -450,7 +451,8 @@ static void updateMatchedPairSubtrees(
           mutationContainer,
           parentShadowView,
           newRemainingPairs,
-          oldPair);
+          oldPair,
+          oldPair.shadowView);
     }
     // Unflattening
     else {
@@ -480,7 +482,8 @@ static void updateMatchedPairSubtrees(
           mutationContainer,
           parentShadowView,
           unvisitedOldChildPairs,
-          newPair);
+          newPair,
+          parentShadowView);
 
       // If old nodes were not visited, we know that we can delete
       // them now. They will be removed from the hierarchy by the
@@ -621,6 +624,10 @@ static void updateMatchedPair(
  *    performed in the subtree. If it *is* in the map, it means the node is not
  *    in the Tree, and should be Deleted/Created  **after this function is
  *    called**, by the caller.
+ *
+ * @param parentShadowView shadowView under which nodes should be mounted
+ * @param parentShadowViewForUpdate current parent in which node is mounted,
+ *    used for update mutations
  */
 static void calculateShadowViewMutationsFlattener(
     ViewNodePairScope& scope,
@@ -629,6 +636,7 @@ static void calculateShadowViewMutationsFlattener(
     const ShadowView& parentShadowView,
     TinyMap<Tag, ShadowViewNodePair*>& unvisitedOtherNodes,
     const ShadowViewNodePair& node,
+    const ShadowView& parentShadowViewForUpdate,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherNewNodes,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherOldNodes) {
   DEBUG_LOGS({
@@ -838,11 +846,15 @@ static void calculateShadowViewMutationsFlattener(
       // ShadowNode.
       if (newTreeNodePair.shadowView != oldTreeNodePair.shadowView &&
           newTreeNodePair.isConcreteView && oldTreeNodePair.isConcreteView) {
+        // We execute updates before creates, so pass the current parent in when
+        // unflattening.
+        // TODO: whenever we insert, we already update the relevant properties,
+        // so this update is redundant. We should remove this.
         mutationContainer.updateMutations.push_back(
             ShadowViewMutation::UpdateMutation(
                 oldTreeNodePair.shadowView,
                 newTreeNodePair.shadowView,
-                node.shadowView));
+                parentShadowViewForUpdate));
       }
 
       // Update children if appropriate.
@@ -877,6 +889,9 @@ static void calculateShadowViewMutationsFlattener(
                    : newTreeNodePair.shadowView),
               unvisitedOtherNodes,
               treeChildPair,
+              (reparentMode == ReparentMode::Flatten
+                   ? oldTreeNodePair.shadowView
+                   : parentShadowView),
               subVisitedNewMap,
               subVisitedOldMap);
         } else {
@@ -918,6 +933,9 @@ static void calculateShadowViewMutationsFlattener(
                      : newTreeNodePair.shadowView),
                 unvisitedRecursiveChildPairs,
                 oldTreeNodePair,
+                (reparentMode == ReparentMode::Flatten
+                     ? oldTreeNodePair.shadowView
+                     : parentShadowView),
                 subVisitedNewMap,
                 subVisitedOldMap);
           }
@@ -933,6 +951,9 @@ static void calculateShadowViewMutationsFlattener(
                      : newTreeNodePair.shadowView),
                 unvisitedRecursiveChildPairs,
                 newTreeNodePair,
+                (reparentMode == ReparentMode::Flatten
+                     ? oldTreeNodePair.shadowView
+                     : parentShadowView),
                 subVisitedNewMap,
                 subVisitedOldMap);
 

packages/react-native/ReactCommon/react/renderer/mounting/stubs/StubViewTree.cpp

@@ -221,6 +221,11 @@ void StubViewTree::mutate(const ShadowViewMutationList& mutations) {
         react_native_assert(hasTag(mutation.newChildShadowView.tag));
         auto oldStubView = registry_[mutation.newChildShadowView.tag];
         react_native_assert(oldStubView->tag != 0);
+        if (mutation.parentShadowView.tag != 0) {
+          react_native_assert(hasTag(mutation.parentShadowView.tag));
+          react_native_assert(
+              oldStubView->parentTag == mutation.parentShadowView.tag);
+        }
         if ((ShadowView)(*oldStubView) != mutation.oldChildShadowView) {
           LOG(ERROR)
               << "StubView: ASSERT FAILURE: UPDATE mutation assertion failure: oldChildShadowView does not match oldStubView: ["

packages/react-native/ReactCommon/react/renderer/mounting/tests/MountingTest.cpp

@@ -558,8 +558,7 @@ TEST(MountingTest, testViewReparentingInstructionGeneration) {
   EXPECT_EQ(mutations1[0].oldChildShadowView.tag, childG->getTag());
   EXPECT_EQ(mutations1[1].type, ShadowViewMutation::Update);
   EXPECT_EQ(mutations1[1].oldChildShadowView.tag, reparentedViewA->getTag());
-  // This is incorrect! ChildH does not exist yet at this point
-  EXPECT_EQ(mutations1[1].parentShadowView.tag, childH->getTag());
+  EXPECT_EQ(mutations1[1].parentShadowView.tag, childG->getTag());
   EXPECT_EQ(mutations1[2].type, ShadowViewMutation::Remove);
   EXPECT_EQ(mutations1[2].oldChildShadowView.tag, reparentedViewA->getTag());
   EXPECT_EQ(mutations1[3].type, ShadowViewMutation::Create);