Commit c4772a4
authored
[0.83] Fix display: contents nodes having hasNewLayout set incorrectly (#57241)
* Expose cleanupContentsNodesRecursively for backport
Backport prerequisite: make cleanupContentsNodesRecursively externally
linkable and declare it in CalculateLayout.h so AbsoluteLayout.cpp can
call it. This mirrors the relevant slice of #55876 (26cef64), which
is not present on this branch, and is required before applying #56422
and #57103.
* Fix node ownership when `display: contents` is used (#56422)
Summary:
Pull Request resolved: #56422
Changelog: [GENERAL][FIXED] Fixed Yoga node ownership when `display: contents` is used in absolutely positioned subtrees
Fixes an edge case where nodes with `display: contents` weren't cloned properly inside absolutely positioned subtrees.
Adds a test case covering this scenario.
X-link: react/yoga#1924
Reviewed By: NickGerleman
Differential Revision: D100581579
Pulled By: j-piasecki
fbshipit-source-id: e05e9a4076bd11a71be438ef910a262044659a9b
* 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: cae5e889622296e8b6380a6428509b5ffea3e9ae1 parent 34faea9 commit c4772a4
4 files changed
Lines changed: 21 additions & 8 deletions
File tree
- packages/react-native/ReactCommon/yoga/yoga
- algorithm
- node
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
553 | 553 | | |
554 | 554 | | |
555 | 555 | | |
| 556 | + | |
| 557 | + | |
556 | 558 | | |
557 | 559 | | |
558 | 560 | | |
| |||
Lines changed: 11 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
480 | 480 | | |
481 | 481 | | |
482 | 482 | | |
483 | | - | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
484 | 486 | | |
485 | 487 | | |
486 | 488 | | |
487 | 489 | | |
488 | 490 | | |
489 | 491 | | |
490 | 492 | | |
491 | | - | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
492 | 496 | | |
493 | 497 | | |
494 | 498 | | |
495 | | - | |
| 499 | + | |
496 | 500 | | |
497 | 501 | | |
498 | 502 | | |
| |||
1317 | 1321 | | |
1318 | 1322 | | |
1319 | 1323 | | |
1320 | | - | |
| 1324 | + | |
1321 | 1325 | | |
1322 | 1326 | | |
1323 | 1327 | | |
| |||
1335 | 1339 | | |
1336 | 1340 | | |
1337 | 1341 | | |
1338 | | - | |
| 1342 | + | |
1339 | 1343 | | |
1340 | 1344 | | |
1341 | 1345 | | |
| |||
1353 | 1357 | | |
1354 | 1358 | | |
1355 | 1359 | | |
1356 | | - | |
| 1360 | + | |
1357 | 1361 | | |
1358 | 1362 | | |
1359 | 1363 | | |
| |||
1365 | 1369 | | |
1366 | 1370 | | |
1367 | 1371 | | |
1368 | | - | |
| 1372 | + | |
1369 | 1373 | | |
1370 | 1374 | | |
1371 | 1375 | | |
| |||
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
38 | 40 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
392 | 392 | | |
393 | 393 | | |
394 | 394 | | |
395 | | - | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
396 | 401 | | |
397 | 402 | | |
398 | 403 | | |
| |||
0 commit comments