From f5682d64ebef4a6a9cb30e0bead6190c2b0b41c4 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 5 Mar 2025 11:18:46 -0800 Subject: [PATCH] fix a crash in Fabric View Culling when wrapping view is unflattened (#49845) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/49845 changelog: [internal] Fixes a crash that occurs when culled view's ancestor is flattened/unflattened. Reviewed By: lenaic Differential Revision: D70620103 fbshipit-source-id: 20b7f9acd59c5e74e768d31ff1f96bdd06747389 --- .../__tests__/ScrollView-viewCulling-itest.js | 109 ++++++++++++++++++ .../renderer/mounting/Differentiator.cpp | 15 ++- 2 files changed, 119 insertions(+), 5 deletions(-) diff --git a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js index 0eb07f196e5..b0daeb7ec4d 100644 --- a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js +++ b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js @@ -840,3 +840,112 @@ test('culling when ScrollView parent has transform', () => { 'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}', ]); }); + +test('view flattening with culling', () => { + const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100}); + let maybeNode; + + Fantom.runTask(() => { + root.render( + { + maybeNode = node; + }}> + + + + , + ); + }); + + expect(root.takeMountingManagerLogs()).toEqual([ + 'Update {type: "RootView", nativeID: (root)}', + 'Create {type: "ScrollView", nativeID: (N/A)}', + 'Create {type: "View", nativeID: (N/A)}', + 'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}', + 'Insert {type: "ScrollView", parentNativeID: (root), index: 0, nativeID: (N/A)}', + ]); + + const element = ensureInstance(maybeNode, ReactNativeElement); + + Fantom.runOnUIThread(() => { + Fantom.scrollTo(element, { + x: 0, + y: 60, + }); + }); + + Fantom.runWorkLoop(); + + expect(root.takeMountingManagerLogs()).toEqual([ + 'Update {type: "ScrollView", nativeID: (N/A)}', + 'Create {type: "View", nativeID: "child"}', + 'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}', + ]); + + // force view to be unflattened. + Fantom.runTask(() => { + root.render( + { + maybeNode = node; + }}> + + + + , + ); + }); + + expect(root.takeMountingManagerLogs()).toEqual([ + 'Update {type: "View", nativeID: "child"}', + 'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}', + '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: "child"}', + ]); + + // force view to be flattened. + Fantom.runTask(() => { + root.render( + { + maybeNode = node; + }}> + + + + , + ); + }); + + expect(root.takeMountingManagerLogs()).toEqual([ + 'Update {type: "View", nativeID: "child"}', + 'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}', + 'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}', + 'Delete {type: "View", nativeID: (N/A)}', + 'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}', + ]); +}); diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp index eef05353a07..c47af72936a 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp @@ -201,6 +201,11 @@ static void updateMatchedPairSubtrees( << newPair << " with parent [" << parentTag << "]"; }); + auto oldCullingContextCopy = + oldCullingContext.adjustCullingContextIfNeeded(oldPair); + auto newCullingContextCopy = + newCullingContext.adjustCullingContextIfNeeded(newPair); + // Flattening if (!oldPair.flattened) { // Flatten old tree into new list @@ -217,8 +222,8 @@ static void updateMatchedPairSubtrees( oldPair.shadowView.tag, nullptr, nullptr, - oldCullingContext, - newCullingContext); + oldCullingContextCopy, + newCullingContextCopy); } // Unflattening else { @@ -230,7 +235,7 @@ static void updateMatchedPairSubtrees( // + zIndex: the children could be listed before the parent, // interwoven with children from other nodes, etc. auto oldFlattenedNodes = sliceChildShadowNodeViewPairsFromViewNodePair( - oldPair, scope, true, oldCullingContext); + oldPair, scope, true, oldCullingContextCopy); for (size_t i = 0, j = 0; i < oldChildPairs.size() && j < oldFlattenedNodes.size(); i++) { @@ -252,8 +257,8 @@ static void updateMatchedPairSubtrees( parentTag, nullptr, nullptr, - oldCullingContext, - newCullingContext); + oldCullingContextCopy, + newCullingContextCopy); // If old nodes were not visited, we know that we can delete // them now. They will be removed from the hierarchy by the