add a test for view culling where during reparenting a view is culled (facebook#50885)

Summary:
Pull Request resolved: facebook#50885

changelog: [internal]

a special case inside of Differentiator handling parent-child switching from unflattened-flattened to flattened-unflattened. If a child has view that is culled, this needs to be handled.

This diff also simplifies the implementation inside of calculateShadowViewMutationsFlattener by passing only one cullingContext.

Reviewed By: mdvacca

Differential Revision: D73523523

fbshipit-source-id: d6f314da6b9ff40bcf3362243b03de1b39e7aabb

Author: sammy-SC

packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js

@@ -1468,3 +1468,95 @@ test('flattening and deleting a subtree that is partially culled', () => {
     'Delete {type: "View", nativeID: "child"}',
   ]);
 });
+
+test('parent-child switching from unflattened-flattened to flattened-unflattened and grandchild is culled', () => {
+  const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100});
+
+  // First render unflattened view container with flattened child that has a culled grandchild.
+  Fantom.runTask(() => {
+    root.render(
+      <ScrollView
+        style={{height: 100, width: 100}}
+        contentOffset={{x: 0, y: 60}}>
+        <View
+          style={{
+            marginTop: 100,
+            opacity: 0,
+          }}>
+          <View
+            style={{
+              marginTop: 50,
+            }}>
+            <View
+              nativeID={'grandchild'}
+              style={{height: 10, width: 10, marginTop: 11}}
+            />
+          </View>
+        </View>
+      </ScrollView>,
+    );
+  });
+
+  // Note that `grandchild` is not mounted.
+  expect(root.takeMountingManagerLogs()).toEqual([
+    'Update {type: "RootView", nativeID: (root)}',
+    'Create {type: "ScrollView", nativeID: (N/A)}',
+    'Create {type: "View", nativeID: (N/A)}',
+    'Create {type: "View", nativeID: (N/A)}',
+    'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}',
+    'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}',
+    'Insert {type: "ScrollView", parentNativeID: (root), index: 0, nativeID: (N/A)}',
+  ]);
+
+  let maybeNode = null;
+
+  // Now change unflattened view container to flattened and change its child to be unflattened.
+  Fantom.runTask(() => {
+    root.render(
+      <ScrollView
+        style={{height: 100, width: 100}}
+        ref={node => {
+          maybeNode = node;
+        }}
+        contentOffset={{x: 0, y: 60}}>
+        <View
+          style={{
+            marginTop: 100,
+          }}>
+          <View
+            style={{
+              marginTop: 50,
+              opacity: 0,
+            }}>
+            <View
+              nativeID={'grandchild'}
+              style={{height: 10, width: 10, marginTop: 11}}
+            />
+          </View>
+        </View>
+      </ScrollView>,
+    );
+  });
+
+  // Note that `grandchild` is not mounted.
+  expect(root.takeMountingManagerLogs()).toEqual([
+    'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}',
+    'Delete {type: "View", nativeID: (N/A)}',
+    'Create {type: "View", nativeID: (N/A)}',
+    'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}',
+  ]);
+
+  const element = ensureInstance(maybeNode, ReactNativeElement);
+
+  // Scroll to reveal grandchild.
+  Fantom.scrollTo(element, {
+    x: 0,
+    y: 70,
+  });
+
+  expect(root.takeMountingManagerLogs()).toEqual([
+    'Update {type: "ScrollView", nativeID: (N/A)}',
+    'Create {type: "View", nativeID: "grandchild"}',
+    'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "grandchild"}',
+  ]);
+});

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

@@ -165,8 +165,7 @@ static void calculateShadowViewMutationsFlattener(
     Tag parentTagForUpdate,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherNewNodes,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherOldNodes,
-    const CullingContext& oldCullingContext,
-    const CullingContext& newCullingContext);
+    const CullingContext& cullingContext);
 
 /**
  * Updates the subtrees of any matched ShadowViewNodePair. This handles
@@ -222,8 +221,7 @@ static void updateMatchedPairSubtrees(
           oldPair.shadowView.tag,
           nullptr,
           nullptr,
-          oldCullingContextCopy,
-          newCullingContextCopy);
+          oldCullingContextCopy);
     }
     // Unflattening
     else {
@@ -257,7 +255,6 @@ static void updateMatchedPairSubtrees(
           parentTag,
           nullptr,
           nullptr,
-          oldCullingContextCopy,
           newCullingContextCopy);
 
       // If old nodes were not visited, we know that we can delete
@@ -423,12 +420,11 @@ static void calculateShadowViewMutationsFlattener(
     Tag parentTagForUpdate,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherNewNodes,
     TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherOldNodes,
-    const CullingContext& oldCullingContext,
-    const CullingContext& newCullingContext) {
+    const CullingContext& cullingContext) {
   // Step 1: iterate through entire tree
   std::vector<ShadowViewNodePair*> treeChildren =
       sliceChildShadowNodeViewPairsFromViewNodePair(
-          node, scope, false, newCullingContext);
+          node, scope, false, cullingContext);
 
   DEBUG_LOGS({
     LOG(ERROR) << "Differ Flattener: "
@@ -620,16 +616,13 @@ static void calculateShadowViewMutationsFlattener(
       }
 
       auto adjustedOldCullingContext =
-          oldCullingContext.adjustCullingContextIfNeeded(oldTreeNodePair);
+          cullingContext.adjustCullingContextIfNeeded(oldTreeNodePair);
       auto adjustedNewCullingContext =
-          newCullingContext.adjustCullingContextIfNeeded(newTreeNodePair);
+          cullingContext.adjustCullingContextIfNeeded(newTreeNodePair);
 
       // Update children if appropriate.
       if (!oldTreeNodePair.flattened && !newTreeNodePair.flattened) {
-        if (oldTreeNodePair.shadowNode != newTreeNodePair.shadowNode ||
-            oldCullingContext != newCullingContext) {
-          // TODO(T217775046): Find a test case for oldCullingContext !=
-          // newCullingContext condition above.
+        if (oldTreeNodePair.shadowNode != newTreeNodePair.shadowNode) {
           ViewNodePairScope innerScope{};
           auto oldGrandChildPairs =
               sliceChildShadowNodeViewPairsFromViewNodePair(
@@ -677,12 +670,9 @@ static void calculateShadowViewMutationsFlattener(
                    : parentTag),
               subVisitedNewMap,
               subVisitedOldMap,
-              oldCullingContext,
-              newCullingContext);
+              cullingContext.adjustCullingContextIfNeeded(treeChildPair));
         } else {
           // Get flattened nodes from either new or old tree
-          // TODO(T217775046): Find a test case for this branch of view
-          // flattening + culling.
           auto flattenedNodes = sliceChildShadowNodeViewPairsFromViewNodePair(
               (childReparentMode == ReparentMode::Flatten ? newTreeNodePair
                                                           : oldTreeNodePair),
@@ -735,8 +725,7 @@ static void calculateShadowViewMutationsFlattener(
                      : parentTagForUpdateWhenUnflattened),
                 subVisitedNewMap,
                 subVisitedOldMap,
-                adjustedOldCullingContext,
-                adjustedNewCullingContext);
+                cullingContext.adjustCullingContextIfNeeded(oldTreeNodePair));
           }
           // Flatten parent, unflatten child
           else {
@@ -762,8 +751,9 @@ static void calculateShadowViewMutationsFlattener(
                      : parentTag),
                 /* parentSubVisitedOtherNewNodes */ subVisitedNewMap,
                 /* parentSubVisitedOtherOldNodes */ subVisitedOldMap,
-                oldCullingContext,
-                newCullingContext);
+                reparentMode == ReparentMode::Flatten
+                    ? adjustedOldCullingContext
+                    : adjustedNewCullingContext);
 
             // If old nodes were not visited, we know that we can delete them
             // now. They will be removed from the hierarchy by the outermost
@@ -848,10 +838,8 @@ static void calculateShadowViewMutationsFlattener(
       continue;
     }
 
-    auto adjustedOldCullingContext =
-        oldCullingContext.adjustCullingContextIfNeeded(treeChildPair);
-    auto adjustedNewCullingContext =
-        newCullingContext.adjustCullingContextIfNeeded(treeChildPair);
+    auto adjustedCullingContext =
+        cullingContext.adjustCullingContextIfNeeded(treeChildPair);
 
     if (reparentMode == ReparentMode::Flatten) {
       mutationContainer.deleteMutations.push_back(
@@ -864,10 +852,10 @@ static void calculateShadowViewMutationsFlattener(
             mutationContainer.destructiveDownwardMutations,
             treeChildPair.shadowView.tag,
             sliceChildShadowNodeViewPairsFromViewNodePair(
-                treeChildPair, innerScope, false, adjustedOldCullingContext),
+                treeChildPair, innerScope, false, adjustedCullingContext),
             {},
-            adjustedOldCullingContext,
-            newCullingContext);
+            adjustedCullingContext,
+            {});
       }
     } else {
       mutationContainer.createMutations.push_back(
@@ -881,9 +869,9 @@ static void calculateShadowViewMutationsFlattener(
             treeChildPair.shadowView.tag,
             {},
             sliceChildShadowNodeViewPairsFromViewNodePair(
-                treeChildPair, innerScope, false, adjustedNewCullingContext),
-            oldCullingContext,
-            adjustedNewCullingContext);
+                treeChildPair, innerScope, false, adjustedCullingContext),
+            {},
+            adjustedCullingContext);
       }
     }
   }