Skip to content

Commit 442d05a

Browse files
javachefacebook-github-bot
authored andcommitted
Add regression tests for YogaLayoutableShadowNode clone paths
Summary: Lock in the behaviour of the two branches in `YogaLayoutableShadowNode`'s clone constructor: cloning with `!fragment.children` (children inherited from the source) and cloning with `fragment.children` set (children list replaced). The new fixture also covers successive prop-only clones (the animation commit pattern) and verifies that mutating a clone does not bleed back into the source's layout state. Changelog: [Internal] Differential Revision: D107076027
1 parent c789880 commit 442d05a

2 files changed

Lines changed: 355 additions & 0 deletions

File tree

packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
namespace facebook::react {
2323

2424
class YogaLayoutableShadowNode : public LayoutableShadowNode {
25+
// Allow YogaCloneTest to read yogaNode_ for ownership assertions in unit
26+
// tests. The class is only defined in the tests target; production code
27+
// sees the friend as a forward declaration with no effect.
28+
friend class YogaCloneTest;
29+
2530
public:
2631
using Shared = std::shared_ptr<const YogaLayoutableShadowNode>;
2732
using ListOfShared = std::vector<Shared>;

packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp

Lines changed: 350 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
#include <react/renderer/core/PropsParserContext.h>
1818
#include <react/renderer/element/ComponentBuilder.h>
1919

20+
#include <react/renderer/components/view/YogaLayoutableShadowNode.h>
2021
#include <react/renderer/element/Element.h>
2122
#include <react/renderer/element/testUtils.h>
23+
#include <yoga/Yoga.h>
2224
#include <yoga/numeric/FloatOptional.h>
2325

2426
namespace facebook::react {
@@ -263,4 +265,352 @@ TEST_F(YogaDirtyFlagTest, clonedPropsPreserveAspectRatio) {
263265
});
264266
}
265267

268+
// Exercises the two branches of YogaLayoutableShadowNode's clone constructor:
269+
// `!fragment.children` (children inherited from source) and
270+
// `fragment.children` set (children list replaced). The cloned subtree must
271+
// lay out identically to a freshly built equivalent tree.
272+
class YogaCloneTest : public ::testing::Test {
273+
protected:
274+
ComponentBuilder builder_;
275+
std::shared_ptr<RootShadowNode> rootShadowNode_;
276+
std::shared_ptr<ViewShadowNode> parentShadowNode_;
277+
std::shared_ptr<ViewShadowNode> childAShadowNode_;
278+
std::shared_ptr<ViewShadowNode> childBShadowNode_;
279+
std::shared_ptr<ViewShadowNode> childCShadowNode_;
280+
281+
YogaCloneTest() : builder_(simpleComponentBuilder()) {
282+
// clang-format off
283+
auto element =
284+
Element<RootShadowNode>()
285+
.reference(rootShadowNode_)
286+
.tag(1)
287+
.props([] {
288+
auto sharedProps = std::make_shared<RootProps>();
289+
auto &props = *sharedProps;
290+
props.layoutConstraints = LayoutConstraints{
291+
.minimumSize = {.width = 0, .height = 0},
292+
.maximumSize = {.width = 300, .height = 300}};
293+
auto &yogaStyle = props.yogaStyle;
294+
yogaStyle.setDimension(
295+
yoga::Dimension::Width, yoga::StyleSizeLength::points(300));
296+
yogaStyle.setDimension(
297+
yoga::Dimension::Height, yoga::StyleSizeLength::points(300));
298+
yogaStyle.setFlexDirection(yoga::FlexDirection::Row);
299+
return sharedProps;
300+
})
301+
.children({
302+
Element<ViewShadowNode>()
303+
.reference(parentShadowNode_)
304+
.tag(2)
305+
.props([] {
306+
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
307+
auto &props = *sharedProps;
308+
auto &yogaStyle = props.yogaStyle;
309+
yogaStyle.setFlexDirection(yoga::FlexDirection::Row);
310+
yogaStyle.setDimension(
311+
yoga::Dimension::Width,
312+
yoga::StyleSizeLength::points(300));
313+
yogaStyle.setDimension(
314+
yoga::Dimension::Height,
315+
yoga::StyleSizeLength::points(100));
316+
return sharedProps;
317+
})
318+
.children({
319+
Element<ViewShadowNode>()
320+
.reference(childAShadowNode_)
321+
.tag(3)
322+
.props([] {
323+
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
324+
sharedProps->yogaStyle.setDimension(
325+
yoga::Dimension::Width,
326+
yoga::StyleSizeLength::points(100));
327+
sharedProps->yogaStyle.setDimension(
328+
yoga::Dimension::Height,
329+
yoga::StyleSizeLength::points(100));
330+
return sharedProps;
331+
}),
332+
Element<ViewShadowNode>()
333+
.reference(childBShadowNode_)
334+
.tag(4)
335+
.props([] {
336+
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
337+
sharedProps->yogaStyle.setDimension(
338+
yoga::Dimension::Width,
339+
yoga::StyleSizeLength::points(100));
340+
sharedProps->yogaStyle.setDimension(
341+
yoga::Dimension::Height,
342+
yoga::StyleSizeLength::points(100));
343+
return sharedProps;
344+
}),
345+
Element<ViewShadowNode>()
346+
.reference(childCShadowNode_)
347+
.tag(5)
348+
.props([] {
349+
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
350+
sharedProps->yogaStyle.setDimension(
351+
yoga::Dimension::Width,
352+
yoga::StyleSizeLength::points(100));
353+
sharedProps->yogaStyle.setDimension(
354+
yoga::Dimension::Height,
355+
yoga::StyleSizeLength::points(100));
356+
return sharedProps;
357+
})
358+
})
359+
});
360+
// clang-format on
361+
362+
builder_.build(element);
363+
rootShadowNode_->layoutIfNeeded();
364+
}
365+
366+
// Bridges friend access to `YogaLayoutableShadowNode::yogaNode_` for the
367+
// `TEST_F`-generated subclasses (which inherit from this fixture and would
368+
// otherwise have to be friended individually).
369+
static yoga::Node& yogaNodeOf(const ShadowNode& shadowNode) {
370+
// yogaNode_ is `mutable` in the header, so handing out a non-const
371+
// reference matches Yoga's C API (e.g. YGNodeGetOwner takes YGNodeRef).
372+
return static_cast<const YogaLayoutableShadowNode&>(shadowNode).yogaNode_;
373+
}
374+
};
375+
376+
// Cloning a non-leaf node WITHOUT touching the children list must produce a
377+
// subtree whose layout is byte-identical to the original (covers the
378+
// `!fragment.children` fast path that copies `yogaLayoutableChildren_` from
379+
// the source via `static_cast`).
380+
TEST_F(YogaCloneTest, unchangedChildrenClonePreservesLayout) {
381+
auto newRootShadowNode = rootShadowNode_->cloneTree(
382+
parentShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) {
383+
// Empty fragment — neither props nor children set — exercises the
384+
// `!fragment.children` fast path on the parent's clone.
385+
return oldShadowNode.clone(ShadowNodeFragment{});
386+
});
387+
388+
static_cast<RootShadowNode&>(*newRootShadowNode).layoutIfNeeded();
389+
390+
// The cloned parent must still have three layoutable children at the
391+
// expected positions — proving the copied `yogaLayoutableChildren_` is
392+
// consistent with the source.
393+
auto clonedParent = newRootShadowNode->getChildren()[0]->getChildren();
394+
ASSERT_EQ(clonedParent.size(), 3u);
395+
EXPECT_EQ(
396+
std::static_pointer_cast<const ViewShadowNode>(clonedParent[0])
397+
->getLayoutMetrics()
398+
.frame.origin.x,
399+
0);
400+
EXPECT_EQ(
401+
std::static_pointer_cast<const ViewShadowNode>(clonedParent[1])
402+
->getLayoutMetrics()
403+
.frame.origin.x,
404+
100);
405+
EXPECT_EQ(
406+
std::static_pointer_cast<const ViewShadowNode>(clonedParent[2])
407+
->getLayoutMetrics()
408+
.frame.origin.x,
409+
200);
410+
}
411+
412+
// Cloning a non-leaf node WITH a new children list must produce a subtree
413+
// whose layout reflects the new list (covers the `fragment.children` branch
414+
// where `updateYogaChildren()` rebuilds `yogaLayoutableChildren_`).
415+
TEST_F(YogaCloneTest, newChildrenCloneReflectsNewList) {
416+
// Drop the middle child.
417+
auto newChildren =
418+
std::make_shared<const std::vector<std::shared_ptr<const ShadowNode>>>(
419+
std::vector<std::shared_ptr<const ShadowNode>>{
420+
parentShadowNode_->getChildren()[0],
421+
parentShadowNode_->getChildren()[2]});
422+
423+
auto newRootShadowNode = rootShadowNode_->cloneTree(
424+
parentShadowNode_->getFamily(), [&](const ShadowNode& oldShadowNode) {
425+
return oldShadowNode.clone(
426+
{.props = ShadowNodeFragment::propsPlaceholder(),
427+
.children = newChildren});
428+
});
429+
430+
static_cast<RootShadowNode&>(*newRootShadowNode).layoutIfNeeded();
431+
432+
auto clonedParent = newRootShadowNode->getChildren()[0]->getChildren();
433+
ASSERT_EQ(clonedParent.size(), 2u);
434+
// After removing the middle child, the third original child should now sit
435+
// where the second used to be.
436+
EXPECT_EQ(
437+
std::static_pointer_cast<const ViewShadowNode>(clonedParent[0])
438+
->getLayoutMetrics()
439+
.frame.origin.x,
440+
0);
441+
EXPECT_EQ(
442+
std::static_pointer_cast<const ViewShadowNode>(clonedParent[1])
443+
->getLayoutMetrics()
444+
.frame.origin.x,
445+
100);
446+
}
447+
448+
// Successive prop-only clones (the animation commit pattern) must each
449+
// produce a self-consistent layout. Validates that the fast path's vector
450+
// copy yields a vector the next clone can again copy from.
451+
TEST_F(YogaCloneTest, repeatedUnchangedChildrenClonesYieldStableLayout) {
452+
std::shared_ptr<const RootShadowNode> current = rootShadowNode_;
453+
for (int i = 0; i < 5; i++) {
454+
auto next = current->cloneTree(
455+
parentShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) {
456+
return oldShadowNode.clone(ShadowNodeFragment{});
457+
});
458+
auto& mutableNext = static_cast<RootShadowNode&>(*next);
459+
mutableNext.layoutIfNeeded();
460+
current = std::static_pointer_cast<const RootShadowNode>(next);
461+
462+
auto parentChildren = current->getChildren()[0]->getChildren();
463+
ASSERT_EQ(parentChildren.size(), 3u);
464+
EXPECT_EQ(
465+
std::static_pointer_cast<const ViewShadowNode>(parentChildren[2])
466+
->getLayoutMetrics()
467+
.frame.origin.x,
468+
200);
469+
}
470+
}
471+
472+
// The fast path copies the source's `yogaLayoutableChildren_` vector; that
473+
// copy must be independent — mutating the clone via `appendChild` must not
474+
// disturb the source's vector. Indirect signal: source still lays out with
475+
// three children, clone with four.
476+
TEST_F(YogaCloneTest, appendChildOnCloneDoesNotAffectSource) {
477+
auto clonedParent = std::static_pointer_cast<ViewShadowNode>(
478+
parentShadowNode_->clone(ShadowNodeFragment{}));
479+
480+
auto extraChild = std::static_pointer_cast<ViewShadowNode>(
481+
childCShadowNode_->clone(ShadowNodeFragment{}));
482+
clonedParent->appendChild(extraChild);
483+
484+
EXPECT_EQ(parentShadowNode_->getChildren().size(), 3u);
485+
EXPECT_EQ(clonedParent->getChildren().size(), 4u);
486+
487+
// Source's layout (already computed in the fixture) is still valid because
488+
// its yoga subtree was untouched.
489+
EXPECT_EQ(childCShadowNode_->getLayoutMetrics().frame.origin.x, 200);
490+
}
491+
492+
// After cloning a non-leaf node with `!fragment.children`, the cloned
493+
// parent's `yogaNode_` is copy-constructed from the source's and therefore
494+
// shares the same child yoga-node pointers (no synchronous re-parenting,
495+
// no yoga-side reallocation). Yoga's clone callback handles ownership
496+
// transfer lazily on the cloned subtree's first layout, so the source must
497+
// remain undisturbed — both its child count AND its ownership of each
498+
// child yoga node are preserved.
499+
TEST_F(YogaCloneTest, cloneInheritsSourceYogaChildrenWithoutDisturbance) {
500+
const size_t sourceChildCount =
501+
YGNodeGetChildCount(&yogaNodeOf(*parentShadowNode_));
502+
// Snapshot each child's owner before the clone.
503+
std::vector<YGNodeRef> ownersBefore;
504+
for (const auto& child : parentShadowNode_->getChildren()) {
505+
ownersBefore.push_back(YGNodeGetOwner(&yogaNodeOf(*child)));
506+
EXPECT_EQ(ownersBefore.back(), &yogaNodeOf(*parentShadowNode_))
507+
<< "Sanity: source owns its children before clone";
508+
}
509+
510+
auto clonedParent = std::static_pointer_cast<ViewShadowNode>(
511+
parentShadowNode_->clone(ShadowNodeFragment{}));
512+
513+
// Cloned parent inherits source's child yoga-node references — same count,
514+
// same pointers — without running `updateYogaChildren()`.
515+
ASSERT_EQ(YGNodeGetChildCount(&yogaNodeOf(*clonedParent)), sourceChildCount);
516+
for (size_t i = 0; i < sourceChildCount; i++) {
517+
EXPECT_EQ(
518+
YGNodeGetChild(&yogaNodeOf(*clonedParent), i),
519+
YGNodeGetChild(&yogaNodeOf(*parentShadowNode_), i))
520+
<< "Cloned parent's yoga child at index " << i
521+
<< " must alias the source's";
522+
}
523+
524+
// Source's ownership of every child yoga node is unchanged by the clone.
525+
for (size_t i = 0; i < parentShadowNode_->getChildren().size(); i++) {
526+
const auto& child = *parentShadowNode_->getChildren()[i];
527+
EXPECT_EQ(YGNodeGetOwner(&yogaNodeOf(child)), ownersBefore[i])
528+
<< "Source's yoga ownership must be preserved across clone";
529+
}
530+
}
531+
532+
// `cloneTree` clones every node on the path from root to the target, AND
533+
// every sibling on that path — because Fabric forces ownership transfer via
534+
// `adoptYogaChild`, which clones any child whose yoga node is still owned by
535+
// the previous parent. Grandchildren below an unchanged sibling are NOT
536+
// touched (structural sharing kicks in there).
537+
//
538+
// This is the actual "number of clones" invariant for Fabric's cloneTree.
539+
TEST_F(YogaCloneTest, cloneTreeClonesPathPlusSiblings) {
540+
// Build a four-level tree so we have a grandchild we can pointer-compare
541+
// against under an unchanged sibling.
542+
std::shared_ptr<ViewShadowNode> grandchildOfA;
543+
std::shared_ptr<RootShadowNode> deepRoot;
544+
std::shared_ptr<ViewShadowNode> deepParent;
545+
std::shared_ptr<ViewShadowNode> deepChildA;
546+
std::shared_ptr<ViewShadowNode> deepChildB;
547+
// clang-format off
548+
auto element =
549+
Element<RootShadowNode>()
550+
.reference(deepRoot)
551+
.tag(101)
552+
.props([] {
553+
auto sharedProps = std::make_shared<RootProps>();
554+
sharedProps->layoutConstraints = LayoutConstraints{
555+
.minimumSize = {.width = 0, .height = 0},
556+
.maximumSize = {.width = 300, .height = 300}};
557+
return sharedProps;
558+
})
559+
.children({
560+
Element<ViewShadowNode>()
561+
.reference(deepParent)
562+
.tag(102)
563+
.children({
564+
Element<ViewShadowNode>()
565+
.reference(deepChildA)
566+
.tag(103)
567+
.children({
568+
Element<ViewShadowNode>()
569+
.reference(grandchildOfA)
570+
.tag(104)
571+
}),
572+
Element<ViewShadowNode>()
573+
.reference(deepChildB)
574+
.tag(105)
575+
})
576+
});
577+
// clang-format on
578+
builder_.build(element);
579+
deepRoot->layoutIfNeeded();
580+
581+
const ShadowNode* origRoot = deepRoot.get();
582+
const ShadowNode* origParent = deepParent.get();
583+
const ShadowNode* origChildA = deepChildA.get();
584+
const ShadowNode* origChildB = deepChildB.get();
585+
const ShadowNode* origGrandchildA = grandchildOfA.get();
586+
587+
// Target childB. Path: root → parent → childB.
588+
auto newRoot = deepRoot->cloneTree(
589+
deepChildB->getFamily(), [](const ShadowNode& oldShadowNode) {
590+
return oldShadowNode.clone(ShadowNodeFragment{});
591+
});
592+
593+
const ShadowNode* newRootPtr = newRoot.get();
594+
const ShadowNode* newParent = newRoot->getChildren()[0].get();
595+
const ShadowNode* newChildA = newParent->getChildren()[0].get();
596+
const ShadowNode* newChildB = newParent->getChildren()[1].get();
597+
const ShadowNode* newGrandchildA = newChildA->getChildren()[0].get();
598+
599+
// Path from root to target: every node is a fresh allocation.
600+
EXPECT_NE(newRootPtr, origRoot) << "Root cloned (on path)";
601+
EXPECT_NE(newParent, origParent) << "Parent cloned (on path)";
602+
EXPECT_NE(newChildB, origChildB) << "Target cloned";
603+
604+
// Siblings of the target ARE cloned in Fabric, because parent's
605+
// updateYogaChildren() → adoptYogaChild() clones any child whose yoga
606+
// node is still owned by the previous parent.
607+
EXPECT_NE(newChildA, origChildA)
608+
<< "Sibling cloned to take new yoga ownership";
609+
610+
// But the sibling's children are NOT cloned — structural sharing kicks in
611+
// one level below the disturbance.
612+
EXPECT_EQ(newGrandchildA, origGrandchildA)
613+
<< "Grandchild under unchanged sibling must be shared";
614+
}
615+
266616
} // namespace facebook::react

0 commit comments

Comments
 (0)