Skip to content

Commit 7e726e0

Browse files
committed
Fix display: contents nodes having hasNewLayout set incorrectly (#57103)
Summary: Pull Request resolved: #57103 Changelog: [General][Fixed] Fixed `display: contents` nodes having `hasNewLayout` set incorrectly `cleanupContentsNodesRecursively` unconditionally sets `hasNewLayout=true` on `display: contents` children, including on code paths where their parent's layout was not actually performed in this pass. The stale flag can survive across layout passes and, in clone-on-write renderers (e.g. React Native Fabric), be observed by a subsequent pass whose parent was cloned but whose layout was served from cache, leaving the contents child's owner pointing at the previous parent revision. There are two paths through which the cleanup could stamp a contents child whose parent's `hasNewLayout` would end up false: 1. Measure-phase visit. Inside `calculateLayoutImpl`, the cleanup ran with no knowledge of `performLayout`. When the parent's `calculateLayoutImpl` was invoked only with `performLayout=false` (cache miss on measure, cache hit on layout), the cleanup stamped contents children even though the parent itself never had its `hasNewLayout` set. 2. Absolute-layout walk. `layoutAbsoluteDescendants` walks every static layout descendant of the containing block - including ones whose own `calculateLayoutImpl` was skipped via the layout-phase cache. The cleanup invoked along that walk unconditionally stamped contents children, but the parent's `hasNewLayout` was only updated when the recursion actually found new layout downstream. In both cases, the result is the same invariant violation: a contents node with `hasNewLayout=true` whose parent has `hasNewLayout=false`. A consumer iterating the tree via `hasNewLayout` skips the parent and never clears the stale flag. X-link: react/yoga#1970 Test Plan: Added `YGContentsNodeHasNewLayoutTest.cpp` with regression tests: - `contents_child_hasNewLayout_not_stamped_on_measure_only_visit` - pins the measure-phase fix - `absolute_descendant_through_contents_is_reachable_via_hasNewLayout` - pins the positive case for absolute-layout path - `absolute_phase_cleanup_does_not_stamp_when_parent_layout_skipped` - pins the negative case for absolute-layout path Reviewed By: javache Differential Revision: D107854528 Pulled By: j-piasecki fbshipit-source-id: cae5e889622296e8b6380a6428509b5ffea3e9ae
1 parent a632f9e commit 7e726e0

9 files changed

Lines changed: 20 additions & 15 deletions

File tree

packages/react-native/ReactCommon/yoga/yoga/algorithm/AbsoluteLayout.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,6 @@ bool layoutAbsoluteDescendants(
558558
// we need to mutate these descendents. Make sure the path of
559559
// nodes to them is mutable before positioning.
560560
child->cloneChildrenIfNeeded();
561-
cleanupContentsNodesRecursively(child);
562561
const Direction childDirection =
563562
child->resolveDirection(currentNodeDirection);
564563
// By now all descendants of the containing block that are not absolute
@@ -584,6 +583,8 @@ bool layoutAbsoluteDescendants(
584583
containingNodeAvailableInnerHeight) ||
585584
hasNewLayout;
586585

586+
cleanupContentsNodesRecursively(
587+
child, /* didPerformLayout */ hasNewLayout);
587588
if (hasNewLayout) {
588589
child->setHasNewLayout(hasNewLayout);
589590
}

packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -506,19 +506,23 @@ void zeroOutLayoutRecursively(yoga::Node* const node) {
506506
}
507507
}
508508

509-
void cleanupContentsNodesRecursively(yoga::Node* const node) {
509+
void cleanupContentsNodesRecursively(
510+
yoga::Node* const node,
511+
bool didPerformLayout) {
510512
if (node->hasContentsChildren()) [[unlikely]] {
511513
node->cloneContentsChildrenIfNeeded();
512514
for (auto child : node->getChildren()) {
513515
if (child->style().display() == Display::Contents) {
514516
child->getLayout() = {};
515517
child->setLayoutDimension(0, Dimension::Width);
516518
child->setLayoutDimension(0, Dimension::Height);
517-
child->setHasNewLayout(true);
519+
if (didPerformLayout) {
520+
child->setHasNewLayout(true);
521+
}
518522
child->setDirty(false);
519523
child->cloneChildrenIfNeeded();
520524

521-
cleanupContentsNodesRecursively(child);
525+
cleanupContentsNodesRecursively(child, didPerformLayout);
522526
}
523527
}
524528
}
@@ -1360,7 +1364,7 @@ static void calculateLayoutImpl(
13601364

13611365
// Clean and update all display: contents nodes with a direct path to the
13621366
// current node as they will not be traversed
1363-
cleanupContentsNodesRecursively(node);
1367+
cleanupContentsNodesRecursively(node, performLayout);
13641368
return;
13651369
}
13661370

@@ -1378,7 +1382,7 @@ static void calculateLayoutImpl(
13781382

13791383
// Clean and update all display: contents nodes with a direct path to the
13801384
// current node as they will not be traversed
1381-
cleanupContentsNodesRecursively(node);
1385+
cleanupContentsNodesRecursively(node, performLayout);
13821386
return;
13831387
}
13841388

@@ -1396,7 +1400,7 @@ static void calculateLayoutImpl(
13961400
ownerHeight)) {
13971401
// Clean and update all display: contents nodes with a direct path to the
13981402
// current node as they will not be traversed
1399-
cleanupContentsNodesRecursively(node);
1403+
cleanupContentsNodesRecursively(node, /* didPerformLayout */ false);
14001404
return;
14011405
}
14021406

@@ -1408,7 +1412,7 @@ static void calculateLayoutImpl(
14081412

14091413
// Clean and update all display: contents nodes with a direct path to the
14101414
// current node as they will not be traversed
1411-
cleanupContentsNodesRecursively(node);
1415+
cleanupContentsNodesRecursively(node, performLayout);
14121416

14131417
// STEP 1: CALCULATE VALUES FOR REMAINDER OF ALGORITHM
14141418
const FlexDirection mainAxis =

packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,6 @@ float calculateAvailableInnerDimension(
5555

5656
void zeroOutLayoutRecursively(yoga::Node* const node);
5757

58-
void cleanupContentsNodesRecursively(yoga::Node* const node);
58+
void cleanupContentsNodesRecursively(yoga::Node* node, bool didPerformLayout);
5959

6060
} // namespace facebook::yoga

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12822,7 +12822,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1282212822
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1282312823
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1282412824
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
12825-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
12825+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node* node, bool didPerformLayout);
1282612826
void facebook::yoga::constrainMaxSizeForMode(const facebook::yoga::Node* node, facebook::yoga::Direction direction, facebook::yoga::FlexDirection axis, float ownerAxisSize, float ownerWidth, facebook::yoga::SizingMode* mode, float* size);
1282712827
void facebook::yoga::fatalWithMessage(const char* message);
1282812828
void facebook::yoga::layoutAbsoluteChild(const facebook::yoga::Node* containingNode, const facebook::yoga::Node* node, facebook::yoga::Node* child, float containingBlockWidth, float containingBlockHeight, facebook::yoga::SizingMode widthMode, facebook::yoga::Direction direction, facebook::yoga::LayoutData& layoutMarkerData, uint32_t depth, uint32_t generationCount);

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12678,7 +12678,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1267812678
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1267912679
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1268012680
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
12681-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
12681+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node* node, bool didPerformLayout);
1268212682
void facebook::yoga::constrainMaxSizeForMode(const facebook::yoga::Node* node, facebook::yoga::Direction direction, facebook::yoga::FlexDirection axis, float ownerAxisSize, float ownerWidth, facebook::yoga::SizingMode* mode, float* size);
1268312683
void facebook::yoga::fatalWithMessage(const char* message);
1268412684
void facebook::yoga::layoutAbsoluteChild(const facebook::yoga::Node* containingNode, const facebook::yoga::Node* node, facebook::yoga::Node* child, float containingBlockWidth, float containingBlockHeight, facebook::yoga::SizingMode widthMode, facebook::yoga::Direction direction, facebook::yoga::LayoutData& layoutMarkerData, uint32_t depth, uint32_t generationCount);

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15021,7 +15021,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1502115021
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1502215022
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1502315023
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
15024-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
15024+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node* node, bool didPerformLayout);
1502515025
void facebook::yoga::constrainMaxSizeForMode(const facebook::yoga::Node* node, facebook::yoga::Direction direction, facebook::yoga::FlexDirection axis, float ownerAxisSize, float ownerWidth, facebook::yoga::SizingMode* mode, float* size);
1502615026
void facebook::yoga::fatalWithMessage(const char* message);
1502715027
void facebook::yoga::layoutAbsoluteChild(const facebook::yoga::Node* containingNode, const facebook::yoga::Node* node, facebook::yoga::Node* child, float containingBlockWidth, float containingBlockHeight, facebook::yoga::SizingMode widthMode, facebook::yoga::Direction direction, facebook::yoga::LayoutData& layoutMarkerData, uint32_t depth, uint32_t generationCount);

scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14887,7 +14887,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1488714887
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1488814888
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1488914889
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
14890-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
14890+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node* node, bool didPerformLayout);
1489114891
void facebook::yoga::constrainMaxSizeForMode(const facebook::yoga::Node* node, facebook::yoga::Direction direction, facebook::yoga::FlexDirection axis, float ownerAxisSize, float ownerWidth, facebook::yoga::SizingMode* mode, float* size);
1489214892
void facebook::yoga::fatalWithMessage(const char* message);
1489314893
void facebook::yoga::layoutAbsoluteChild(const facebook::yoga::Node* containingNode, const facebook::yoga::Node* node, facebook::yoga::Node* child, float containingBlockWidth, float containingBlockHeight, facebook::yoga::SizingMode widthMode, facebook::yoga::Direction direction, facebook::yoga::LayoutData& layoutMarkerData, uint32_t depth, uint32_t generationCount);

scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9838,7 +9838,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
98389838
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
98399839
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
98409840
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
9841-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
9841+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node* node, bool didPerformLayout);
98429842
void facebook::yoga::constrainMaxSizeForMode(const facebook::yoga::Node* node, facebook::yoga::Direction direction, facebook::yoga::FlexDirection axis, float ownerAxisSize, float ownerWidth, facebook::yoga::SizingMode* mode, float* size);
98439843
void facebook::yoga::fatalWithMessage(const char* message);
98449844
void facebook::yoga::layoutAbsoluteChild(const facebook::yoga::Node* containingNode, const facebook::yoga::Node* node, facebook::yoga::Node* child, float containingBlockWidth, float containingBlockHeight, facebook::yoga::SizingMode widthMode, facebook::yoga::Direction direction, facebook::yoga::LayoutData& layoutMarkerData, uint32_t depth, uint32_t generationCount);

scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9829,7 +9829,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
98299829
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
98309830
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
98319831
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
9832-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
9832+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node* node, bool didPerformLayout);
98339833
void facebook::yoga::constrainMaxSizeForMode(const facebook::yoga::Node* node, facebook::yoga::Direction direction, facebook::yoga::FlexDirection axis, float ownerAxisSize, float ownerWidth, facebook::yoga::SizingMode* mode, float* size);
98349834
void facebook::yoga::fatalWithMessage(const char* message);
98359835
void facebook::yoga::layoutAbsoluteChild(const facebook::yoga::Node* containingNode, const facebook::yoga::Node* node, facebook::yoga::Node* child, float containingBlockWidth, float containingBlockHeight, facebook::yoga::SizingMode widthMode, facebook::yoga::Direction direction, facebook::yoga::LayoutData& layoutMarkerData, uint32_t depth, uint32_t generationCount);

0 commit comments

Comments
 (0)