Skip to content

Fix display: contents nodes having hasNewLayout set incorrectly (#57103)#57103

Closed
j-piasecki wants to merge 1 commit into
react:mainfrom
j-piasecki:export-D107854528
Closed

Fix display: contents nodes having hasNewLayout set incorrectly (#57103)#57103
j-piasecki wants to merge 1 commit into
react:mainfrom
j-piasecki:export-D107854528

Conversation

@j-piasecki

@j-piasecki j-piasecki commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

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

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 8, 2026
@meta-codesync

meta-codesync Bot commented Jun 8, 2026

Copy link
Copy Markdown

@j-piasecki has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107854528.

@facebook-github-tools facebook-github-tools Bot added p: Software Mansion Partner: Software Mansion Partner labels Jun 8, 2026
@facebook-github-tools facebook-github-tools Bot added the p: Facebook Partner: Facebook label Jun 8, 2026
@meta-codesync meta-codesync Bot changed the title Fix display: contents nodes having hasNewLayout set incorrectly Fix display: contents nodes having hasNewLayout set incorrectly (#57103) Jun 8, 2026
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 8, 2026
…eact#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

Differential Revision: D107854528

Pulled By: j-piasecki
@j-piasecki j-piasecki force-pushed the export-D107854528 branch from 2b56860 to 49ab556 Compare June 8, 2026 09:21
j-piasecki added a commit to j-piasecki/yoga that referenced this pull request Jun 8, 2026
…eact#1970)

Summary:
X-link: react/react-native#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.


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

Differential Revision: D107854528

Pulled By: j-piasecki
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 8, 2026
…eact#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
@j-piasecki j-piasecki force-pushed the export-D107854528 branch from 49ab556 to e55656d Compare June 8, 2026 09:50
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 8, 2026
…eact#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
@j-piasecki j-piasecki force-pushed the export-D107854528 branch from e55656d to 45bebbb Compare June 8, 2026 10:06
j-piasecki added a commit to j-piasecki/yoga that referenced this pull request Jun 8, 2026
…eact#1970)

Summary:
X-link: react/react-native#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.


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
j-piasecki added a commit to j-piasecki/yoga that referenced this pull request Jun 8, 2026
…eact#1970)

Summary:
X-link: react/react-native#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.


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
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 8, 2026
…eact#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
@j-piasecki j-piasecki force-pushed the export-D107854528 branch from 45bebbb to a3312c9 Compare June 8, 2026 10:14
…eact#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
@j-piasecki j-piasecki force-pushed the export-D107854528 branch from a3312c9 to b2915ce Compare June 8, 2026 13:33
meta-codesync Bot pushed a commit to react/yoga that referenced this pull request Jun 8, 2026
…1970)

Summary:
X-link: react/react-native#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.

Pull Request resolved: #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
@meta-codesync meta-codesync Bot closed this in 2546ce4 Jun 8, 2026
@facebook-github-tools facebook-github-tools Bot added the Merged This PR has been merged. label Jun 8, 2026
@meta-codesync

meta-codesync Bot commented Jun 8, 2026

Copy link
Copy Markdown

@j-piasecki merged this pull request in 2546ce4.

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @j-piasecki in 2546ce4

When will my fix make it into a release? | How to file a pick request?

j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
Backport prerequisite: make cleanupContentsNodesRecursively externally
linkable and declare it in CalculateLayout.h so AbsoluteLayout.cpp can
call it. This mirrors the relevant slice of react#55876 (26cef64), which
is not present on this branch, and is required before applying react#56422
and react#57103.
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
…eact#57103)

Summary:
Pull Request resolved: react#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
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
Backport prerequisite: make cleanupContentsNodesRecursively externally
linkable and declare it in CalculateLayout.h so AbsoluteLayout.cpp can
call it. This mirrors the relevant slice of react#55876 (26cef64), which
is not present on this branch, and is required before applying react#56422
and react#57103.
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
…eact#57103)

Summary:
Pull Request resolved: react#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
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
Backport prerequisite: make cleanupContentsNodesRecursively externally
linkable and declare it in CalculateLayout.h so AbsoluteLayout.cpp can
call it. This mirrors the relevant slice of react#55876 (26cef64), which
is not present on this branch, and is required before applying react#56422
and react#57103.
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
…eact#57103)

Summary:
Pull Request resolved: react#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
j-piasecki added a commit to j-piasecki/react-native that referenced this pull request Jun 9, 2026
…eact#57103)

Summary:
Pull Request resolved: react#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. meta-exported p: Facebook Partner: Facebook p: Software Mansion Partner: Software Mansion Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants