Skip to content

Commit 6fa3306

Browse files
5ZYSZ3Kmeta-codesync[bot]
authored andcommitted
fix: yoga crash with display:none (#57197)
Summary: X-link: react/yoga#1976 It fixes this issue: #52349 Basically, in some rare cases, having an element with `display:none` style caused the app to crash. It was caused by a stale `hasNewLayout` flag on the hidden view That flag was always set in `computeFlexBasisForChildren` for elements with `display:none` style, but could be never consumed/reset, because of a cache hit or something. Since such elements contribute nothing to the layout sizes, I made the change to actually touch them only during actual layout passes ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [GENERAL] [FIXED] - Crash: YogaLayoutableShadowNode.cpp: function layout: assertion failed (YGNodeGetOwner(childYogaNode) == &yogaNode_) #52349 Pull Request resolved: #57197 Test Plan: I created a reproduction repo: https://github.com/5ZYSZ3K/native-tabs-crash-repro, to see the issue And to see, that my change fixes it, you can switch to `patch-yoga` branch there, and install it again (don't forget to install pods with `RCT_USE_PREBUILT_RNCORE=0 RCT_USE_RN_DEP=0` variables) Reviewed By: christophpurrer Differential Revision: D108796888 Pulled By: javache fbshipit-source-id: 455e3ddbec760dbee875c02f0ed6266b9a417e9a
1 parent 2abba7c commit 6fa3306

2 files changed

Lines changed: 132 additions & 3 deletions

File tree

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
import '@react-native/fantom/src/setUpDefaultReactNativeEnvironment';
12+
13+
import * as Fantom from '@react-native/fantom';
14+
import * as React from 'react';
15+
import {useState} from 'react';
16+
import {TextInput, View} from 'react-native';
17+
18+
const VIEWPORT_WIDTH = 390;
19+
const VIEWPORT_HEIGHT = 844;
20+
21+
// State setters captured during render so the test can drive the two-step repro
22+
// from outside the component. In RNTesterPlayground these are wired to two
23+
// buttons ("Step 1: resize container" / "Step 2: restyle input"); here we call
24+
// them directly, each inside its own `runTask` so every step is a full commit
25+
// plus Yoga layout pass.
26+
let triggerResize: () => void = () => {};
27+
let triggerRestyle: () => void = () => {};
28+
29+
function Repro(): React.MixedElement {
30+
const [tall, setTall] = useState(false);
31+
const [highlighted, setHighlighted] = useState(false);
32+
33+
triggerResize = () => setTall(v => !v);
34+
triggerRestyle = () => setHighlighted(v => !v);
35+
36+
return (
37+
<View style={{flex: 1}}>
38+
{/* Step 1 changes the height of this wrapper. The row's own size is
39+
content-determined and does not change. */}
40+
<View style={{height: tall ? 400 : 300, justifyContent: 'center'}}>
41+
<View
42+
style={{
43+
flexDirection: 'row',
44+
alignItems: 'center',
45+
justifyContent: 'center',
46+
}}>
47+
{/* The centered wrapper + its `display: none` child are the nodes
48+
whose Yoga ownership leaks across generations. */}
49+
<View style={{justifyContent: 'center'}}>
50+
<View style={{display: 'none'}} />
51+
</View>
52+
<TextInput
53+
style={[
54+
{borderWidth: 1, minWidth: 120},
55+
highlighted && {borderColor: 'red'},
56+
]}
57+
/>
58+
</View>
59+
</View>
60+
</View>
61+
);
62+
}
63+
64+
// Repro for (New Architecture, debug build):
65+
//
66+
// Assertion failed: (YGNodeGetOwner(childYogaNode) == &yogaNode_),
67+
// function layout, file YogaLayoutableShadowNode.cpp, line 709
68+
//
69+
// Mechanism:
70+
//
71+
// 1. Step 1 resizes the container wrapping the row. The row's content-determined
72+
// size does not change, so Yoga re-MEASURES the row subtree with new
73+
// constraints (a measure-only pass: `zeroOutLayoutRecursively` wipes the
74+
// `display: none` child and sets `hasNewLayout` on it) but restores the row's
75+
// final layout from cache. RN's metrics traversal never visits the centered
76+
// wrapper, so the `hasNewLayout` flag on the hidden child is never consumed --
77+
// it leaks across the commit.
78+
//
79+
// 2. Step 2 changes only the TextInput's props. Fabric clones the row with a new
80+
// children list; `adoptYogaChild` clones the untouched centered wrapper via
81+
// `clone({})`, which SHARES the hidden child's yoga node (stale
82+
// `hasNewLayout`, owner = previous generation's wrapper). During layout the
83+
// wrapper is a clean cache hit, so `cloneChildrenIfNeeded()` never repairs
84+
// ownership -- yet `calculateLayoutInternal` still flags the wrapper with
85+
// `hasNewLayout`. RN's traversal then descends into the wrapper, finds the
86+
// shared hidden child flagged but owned by the old generation, and trips the
87+
// assert.
88+
//
89+
// The KeyboardAvoidingView + focus/typing path reproduces the same sequence
90+
// organically (keyboard resize = step 1, restyle-on-change = step 2). If Yoga
91+
// aborts, `runTask` re-throws the native error synchronously and this test fails.
92+
test('does not trip the Yoga node owner assertion after resize then restyle', () => {
93+
const root = Fantom.createRoot({
94+
viewportWidth: VIEWPORT_WIDTH,
95+
viewportHeight: VIEWPORT_HEIGHT,
96+
});
97+
98+
// Initial commit + layout.
99+
Fantom.runTask(() => {
100+
root.render(<Repro />);
101+
});
102+
103+
// Step 1: resize container -> measure-only pass leaks `hasNewLayout` on the
104+
// hidden child.
105+
Fantom.runTask(() => {
106+
triggerResize();
107+
});
108+
109+
// Step 2: restyle input -> clones the row, shares the hidden child's yoga node
110+
// with stale ownership, and (when the bug is present) trips the assert during
111+
// the layout pass.
112+
Fantom.runTask(() => {
113+
triggerRestyle();
114+
});
115+
116+
// Reaching this point means Yoga did not abort during the second layout pass.
117+
expect(root.getRenderedOutput().toJSX()).not.toBe(null);
118+
119+
root.destroy();
120+
});

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -615,9 +615,18 @@ static float computeFlexBasisForChildren(
615615
for (auto child : children) {
616616
child->processDimensions();
617617
if (child->style().display() == Display::None) {
618-
zeroOutLayoutRecursively(child);
619-
child->setHasNewLayout(true);
620-
child->setDirty(false);
618+
// Only mutate display: none children during layout passes. Zeroing them
619+
// out during measure-only passes contributes nothing to the measurement,
620+
// but sets `hasNewLayout` on nodes the parent's layout pass may never
621+
// visit (e.g. when its layout is restored from cache, skipping
622+
// `cloneChildrenIfNeeded()`). Such a leaked flag survives the commit and
623+
// is copied into lazily-shared clones, later tripping the ownership
624+
// assertion in `YogaLayoutableShadowNode::layout`.
625+
if (performLayout) {
626+
zeroOutLayoutRecursively(child);
627+
child->setHasNewLayout(true);
628+
child->setDirty(false);
629+
}
621630
continue;
622631
}
623632
if (performLayout) {

0 commit comments

Comments
 (0)