Skip to content

Commit 45bebbb

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Fix display: contents nodes having hasNewLayout set incorrectly (#57103)
Summary: 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
1 parent 69b58f3 commit 45bebbb

12 files changed

Lines changed: 23 additions & 18 deletions

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
@@ -516,19 +516,23 @@ void zeroOutLayoutRecursively(yoga::Node* const node) {
516516
}
517517
}
518518

519-
void cleanupContentsNodesRecursively(yoga::Node* const node) {
519+
void cleanupContentsNodesRecursively(
520+
yoga::Node* const node,
521+
bool didPerformLayout) {
520522
if (node->hasContentsChildren()) [[unlikely]] {
521523
node->cloneContentsChildrenIfNeeded();
522524
for (auto child : node->getChildren()) {
523525
if (child->style().display() == Display::Contents) {
524526
child->getLayout() = {};
525527
child->setLayoutDimension(0, Dimension::Width);
526528
child->setLayoutDimension(0, Dimension::Height);
527-
child->setHasNewLayout(true);
529+
if (didPerformLayout) {
530+
child->setHasNewLayout(true);
531+
}
528532
child->setDirty(false);
529533
child->cloneChildrenIfNeeded();
530534

531-
cleanupContentsNodesRecursively(child);
535+
cleanupContentsNodesRecursively(child, didPerformLayout);
532536
}
533537
}
534538
}
@@ -1617,7 +1621,7 @@ static void calculateLayoutImpl(
16171621

16181622
// Clean and update all display: contents nodes with a direct path to the
16191623
// current node as they will not be traversed
1620-
cleanupContentsNodesRecursively(node);
1624+
cleanupContentsNodesRecursively(node, performLayout);
16211625
return;
16221626
}
16231627

@@ -1635,7 +1639,7 @@ static void calculateLayoutImpl(
16351639

16361640
// Clean and update all display: contents nodes with a direct path to the
16371641
// current node as they will not be traversed
1638-
cleanupContentsNodesRecursively(node);
1642+
cleanupContentsNodesRecursively(node, performLayout);
16391643
return;
16401644
}
16411645

@@ -1653,7 +1657,7 @@ static void calculateLayoutImpl(
16531657
ownerHeight)) {
16541658
// Clean and update all display: contents nodes with a direct path to the
16551659
// current node as they will not be traversed
1656-
cleanupContentsNodesRecursively(node);
1660+
cleanupContentsNodesRecursively(node, /* didPerformLayout */ false);
16571661
return;
16581662
}
16591663

@@ -1665,7 +1669,7 @@ static void calculateLayoutImpl(
16651669

16661670
// Clean and update all display: contents nodes with a direct path to the
16671671
// current node as they will not be traversed
1668-
cleanupContentsNodesRecursively(node);
1672+
cleanupContentsNodesRecursively(node, performLayout);
16691673

16701674
// STEP 1: CALCULATE VALUES FOR REMAINDER OF ALGORITHM
16711675
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
@@ -12934,7 +12934,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1293412934
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1293512935
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1293612936
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
12937-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
12937+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
1293812938
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);
1293912939
void facebook::yoga::fatalWithMessage(const char* message);
1294012940
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/ReactAndroidNewarchCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12572,7 +12572,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1257212572
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1257312573
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1257412574
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
12575-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
12575+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
1257612576
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);
1257712577
void facebook::yoga::fatalWithMessage(const char* message);
1257812578
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
@@ -12787,7 +12787,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1278712787
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1278812788
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1278912789
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
12790-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
12790+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
1279112791
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);
1279212792
void facebook::yoga::fatalWithMessage(const char* message);
1279312793
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
@@ -14748,7 +14748,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1474814748
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1474914749
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1475014750
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
14751-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
14751+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
1475214752
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);
1475314753
void facebook::yoga::fatalWithMessage(const char* message);
1475414754
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/ReactAppleNewarchCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14448,7 +14448,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1444814448
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1444914449
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1445014450
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
14451-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
14451+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
1445214452
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);
1445314453
void facebook::yoga::fatalWithMessage(const char* message);
1445414454
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
@@ -14611,7 +14611,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
1461114611
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
1461214612
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
1461314613
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
14614-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
14614+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
1461514615
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);
1461614616
void facebook::yoga::fatalWithMessage(const char* message);
1461714617
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
@@ -9899,7 +9899,7 @@ void facebook::yoga::assertFatal(bool condition, const char* message);
98999899
void facebook::yoga::assertFatalWithConfig(const facebook::yoga::Config* config, bool condition, const char* message);
99009900
void facebook::yoga::assertFatalWithNode(const facebook::yoga::Node* node, bool condition, const char* message);
99019901
void facebook::yoga::calculateLayout(facebook::yoga::Node* node, float ownerWidth, float ownerHeight, facebook::yoga::Direction ownerDirection);
9902-
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node);
9902+
void facebook::yoga::cleanupContentsNodesRecursively(facebook::yoga::Node *const node, bool performLayout);
99039903
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);
99049904
void facebook::yoga::fatalWithMessage(const char* message);
99059905
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)