Skip to content

Commit 361bc24

Browse files
pawicaometa-codesync[bot]
authored andcommitted
Fix differ creating or deleting a view that a nested (un)flattening moves (#58648)
Summary: Fixes #58647 When a view flattens in the same commit in which its parent unflattens, and one of its children has a negative `zIndex`, the differ creates that child again while it's still mounted, or deletes it even though it only moves. On iOS this crashes in `RCTComponentViewRegistry` (Debug) or with a SIGSEGV in `RCTMountingManager` (Release). The `zIndex` sorts the child before the view that contains it, and the nested recursion then matches it through a different `ShadowViewNodePair`, so `inOtherTree()` stayed false on the candidate. I made the final create/delete loop in `calculateShadowViewMutationsFlattener` also skip candidates whose tag the recursion recorded in the sub-visited map. I also fixed `unvisitedRecursiveChildPairs` storing a pointer to a loop-local copy. ## Changelog: [GENERAL] [FIXED] - Fix the differ creating a mounted view again, or deleting a moved view, when a child with a negative `zIndex` moves in a nested flatten/unflatten Pull Request resolved: #58648 Test Plan: The [reproducer](https://github.com/pawicao/rn-differ-zindex-flatten-repro) from #58647 no longer crashes on iOS with React Native built from source with this change; without it, it crashes on the first swap. Reviewed By: christophpurrer Differential Revision: D121619877 Pulled By: javache fbshipit-source-id: d0e692c8645e5cedff9fee0e88133e18809fd3c7
1 parent 53bf98b commit 361bc24

2 files changed

Lines changed: 76 additions & 3 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -708,9 +708,9 @@ static void calculateShadowViewMutationsFlattener(
708708
auto unvisitedOtherNodesIt =
709709
unvisitedOtherNodes.find(newChild.shadowView.tag);
710710
if (unvisitedOtherNodesIt != unvisitedOtherNodes.end()) {
711-
auto unvisitedItPair = *unvisitedOtherNodesIt->second;
711+
auto* unvisitedItPair = unvisitedOtherNodesIt->second;
712712
unvisitedRecursiveChildPairs.insert(
713-
{unvisitedItPair.shadowView.tag, &unvisitedItPair});
713+
{unvisitedItPair->shadowView.tag, unvisitedItPair});
714714
} else {
715715
unvisitedRecursiveChildPairs.insert(
716716
{newChild.shadowView.tag, &newChild});
@@ -820,6 +820,9 @@ static void calculateShadowViewMutationsFlattener(
820820
// Final step: go through creation/deletion candidates and delete/create
821821
// subtrees if they were never visited during the execution of the above
822822
// loop and recursions.
823+
const auto& subVisitedMap = reparentMode == ReparentMode::Flatten
824+
? *subVisitedOldMap
825+
: *subVisitedNewMap;
823826
for (auto& deletionCreationCandidatePair : deletionCreationCandidatePairs) {
824827
auto& treeChildPair = *deletionCreationCandidatePair.second;
825828

@@ -828,7 +831,10 @@ static void calculateShadowViewMutationsFlattener(
828831
// already created/deleted and we don't need to do that here.
829832
// It is always the responsibility of the matcher to update subtrees when
830833
// nodes are matched.
831-
if (treeChildPair.inOtherTree()) {
834+
// The recursion can match the node through a different pair instance
835+
// (e.g. when zIndex orders it before its parent), so check its tag too.
836+
if (treeChildPair.inOtherTree() ||
837+
subVisitedMap.contains(treeChildPair.shadowView.tag)) {
832838
continue;
833839
}
834840

‎packages/react-native/src/private/renderer/mounting/__tests__/Mounting-itest.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,73 @@ describe('ViewFlattening', () => {
446446
/>,
447447
);
448448
});
449+
450+
test('#58647: child with negative zIndex is kept when its parent flattens and its grandparent unflattens', () => {
451+
const root = Fantom.createRoot();
452+
453+
function render(opacityOnGrandparent: boolean) {
454+
Fantom.runTask(() => {
455+
root.render(
456+
<View nativeID="Q" style={{opacity: 0.5}}>
457+
<View style={opacityOnGrandparent ? {opacity: 0.5} : null}>
458+
<View style={opacityOnGrandparent ? null : {opacity: 0.5}}>
459+
<View nativeID="A" />
460+
<View nativeID="K" style={{zIndex: -1}} />
461+
<View nativeID="B" />
462+
</View>
463+
</View>
464+
</View>,
465+
);
466+
});
467+
}
468+
469+
const expectedOutput = (
470+
<rn-view nativeID="Q">
471+
<rn-view>
472+
<rn-view key="0" nativeID="K" />
473+
<rn-view key="1" nativeID="A" />
474+
<rn-view key="2" nativeID="B" />
475+
</rn-view>
476+
</rn-view>
477+
);
478+
479+
render(false);
480+
root.takeMountingManagerLogs();
481+
482+
render(true);
483+
expect(root.takeMountingManagerLogs()).toEqual([
484+
'Remove {type: "View", parentNativeID: "Q", index: 0, nativeID: (N/A)}',
485+
'Remove {type: "View", parentNativeID: (N/A), index: 2, nativeID: "B"}',
486+
'Remove {type: "View", parentNativeID: (N/A), index: 1, nativeID: "A"}',
487+
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "K"}',
488+
'Delete {type: "View", nativeID: (N/A)}',
489+
'Create {type: "View", nativeID: (N/A)}',
490+
'Insert {type: "View", parentNativeID: "Q", index: 0, nativeID: (N/A)}',
491+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "K"}',
492+
'Insert {type: "View", parentNativeID: (N/A), index: 1, nativeID: "A"}',
493+
'Insert {type: "View", parentNativeID: (N/A), index: 2, nativeID: "B"}',
494+
]);
495+
expect(root.getRenderedOutput({props: ['nativeID']}).toJSX()).toEqual(
496+
expectedOutput,
497+
);
498+
499+
render(false);
500+
expect(root.takeMountingManagerLogs()).toEqual([
501+
'Remove {type: "View", parentNativeID: (N/A), index: 2, nativeID: "B"}',
502+
'Remove {type: "View", parentNativeID: (N/A), index: 1, nativeID: "A"}',
503+
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "K"}',
504+
'Remove {type: "View", parentNativeID: "Q", index: 0, nativeID: (N/A)}',
505+
'Delete {type: "View", nativeID: (N/A)}',
506+
'Create {type: "View", nativeID: (N/A)}',
507+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "K"}',
508+
'Insert {type: "View", parentNativeID: (N/A), index: 1, nativeID: "A"}',
509+
'Insert {type: "View", parentNativeID: (N/A), index: 2, nativeID: "B"}',
510+
'Insert {type: "View", parentNativeID: "Q", index: 0, nativeID: (N/A)}',
511+
]);
512+
expect(root.getRenderedOutput({props: ['nativeID']}).toJSX()).toEqual(
513+
expectedOutput,
514+
);
515+
});
449516
});
450517

451518
describe('reconciliation of setNativeProps and React commit', () => {

0 commit comments

Comments
 (0)