Clean up YogaLayoutableShadowNode (re-enable assert, name sentinel, document ABA)

Summary:
Three small cleanups uncovered during review of the clone path:

- Re-enable the dirty-flag inheritance assert in the clone
  constructor. It was disabled while the background executor was
  potentially racing the JS thread; background executor is no
  longer in use, so the assert is meaningful again.
- Lift the `0xBADC0FFEE0DDF00D` magic owner sentinel out into a
  named `yoga::Node* const` (`reinterpret_cast` is not constexpr,
  so it can't be `constexpr`). The bit pattern stays the same so
  it remains recognisable in debuggers.
- Move the explanatory comment for `updateYogaChildrenOwnersIfNeeded`
  next to its implementation and rewrite it to clearly describe the
  ABA scenario it guards against (and the common no-op case).

Changelog:
[Internal]

Reviewed By: christophpurrer

Differential Revision: D107079944

Author: javache

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

@@ -64,6 +64,16 @@ static int FabricDefaultYogaLog(
 
 thread_local LayoutContext threadLocalLayoutContext;
 
+// Owner sentinel used to detach our previously-adopted Yoga children from
+// `yogaNode_` without leaving the pointer at nullptr (which would lose the
+// "this node was owned" signal) and without leaving it at our address
+// (which would invite ABA when our `yoga::Node` slot is reused). The bit
+// pattern is intentionally recognisable in debuggers.
+// `reinterpret_cast` is not constexpr, so this is a runtime-initialised
+// `const` rather than `constexpr`.
+yoga::Node* const kDetachedYogaNodeOwnerSentinel =
+    reinterpret_cast<yoga::Node*>(0xBADC0FFEE0DDF00DULL);
+
 YogaLayoutableShadowNode::YogaLayoutableShadowNode(
     const ShadowNodeFragment& fragment,
     const ShadowNodeFamily::Shared& family,
@@ -100,19 +110,14 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode(
       yogaNode_(
           static_cast<const YogaLayoutableShadowNode&>(sourceShadowNode)
               .yogaNode_) {
-// Note, cloned `yoga::Node` instance (copied using copy-constructor) inherits
-// dirty flag, measure function, and other properties being set originally in
-// the `YogaLayoutableShadowNode` constructor above.
-
-// There is a known race condition when background executor is enabled, where
-// a tree may be laid out on the Fabric background thread concurrently with
-// the ShadowTree being created on the JS thread. This assert can be
-// re-enabled after disabling background executor everywhere.
-#if 0
-  react_native_assert(YGNodeIsDirty(&static_cast<const YogaLayoutableShadowNode&>(sourceShadowNode)
-              .yogaNode_) == YGNodeIsDirty(&yogaNode_) &&
+  // Note, cloned `yoga::Node` instance (copied using copy-constructor)
+  // inherits dirty flag, measure function, and other properties being set
+  // originally in the `YogaLayoutableShadowNode` constructor above.
+  react_native_assert(
+      YGNodeIsDirty(
+          &static_cast<const YogaLayoutableShadowNode&>(sourceShadowNode)
+               .yogaNode_) == YGNodeIsDirty(&yogaNode_) &&
       "Yoga node must inherit dirty flag.");
-#endif
   if (!getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode) &&
       !fragment.children) {
     // Children unchanged: copy the filtered list directly from the source,
@@ -326,11 +331,21 @@ bool YogaLayoutableShadowNode::shouldNewRevisionDirtyMeasurement(
   return true;
 }
 
+// Detects the ABA scenario where a freshly-allocated `yogaNode_` happens
+// to land at the same address a previous parent occupied. After such a
+// realloc, any Yoga child whose owner pointer still equals `&yogaNode_`
+// is a stale match — yoga would mistake it for "owned by us" and skip the
+// clone-on-write check. We rewrite those spurious owner pointers to
+// `kDetachedYogaNodeOwnerSentinel` so the next
+// `YGNodeGetOwner(child) == this` check correctly returns false and yoga
+// clones the child as it would for any foreign tree.
+//
+// No-op in the common case: right after a clone, children's owner
+// pointers still reference the source node, not us.
 void YogaLayoutableShadowNode::updateYogaChildrenOwnersIfNeeded() {
   for (auto& childYogaNode : yogaNode_.getChildren()) {
     if (YGNodeGetOwner(childYogaNode) == &yogaNode_) {
-      childYogaNode->setOwner(
-          reinterpret_cast<yoga::Node*>(0xBADC0FFEE0DDF00D));
+      childYogaNode->setOwner(kDetachedYogaNodeOwnerSentinel);
     }
   }
 }

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

@@ -108,14 +108,6 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode {
   mutable yoga::Node yogaNode_;
 
  private:
-  /*
-   * Goes over `yogaNode_.getChildren()` and in case child's owner is
-   * equal to address of `yogaNode_`, it sets child's owner address
-   * to `0xBADC0FFEE0DDF00D`. This is magic constant, the intention
-   * is to make debugging easier when the address pops up in debugger.
-   * This prevents ABA problem where child yoga node goes from owned -> unowned
-   * -> back to owned because its parent is allocated at the same address.
-   */
   void updateYogaChildrenOwnersIfNeeded();
 
   /*