From caaea38ad9e5954397e81f625b06abe0efa02d1c Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Fri, 18 May 2018 20:28:57 -0700
Subject: [PATCH] Fabric: Using unique_ptr for storing YGNode inside
 YogaLayoutableShadowNode

Summary:
I recently realized (Thanks David!) that we should not use `shared_ptr` for storing YGNode*
because ShadowNode does not share ownership of the Yoga node with anybody.
So the lifecycle of shadow node and yoga node must be synchronized (this is already the case but changing to unique_ptr makes this explicit and a bit more performant).

Reviewed By: fkgozali

Differential Revision: D8030417

fbshipit-source-id: c7f85ea309598d2a5ebfed55b1d182d3fe1336ae
---
 .../view/yoga/YogaLayoutableShadowNode.cpp    | 54 +++++++++----------
 .../view/yoga/YogaLayoutableShadowNode.h      |  7 +--
 2 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp
index 9ad1be234..43586dea1 100644
--- a/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp
+++ b/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp
@@ -37,13 +37,12 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode(
   assert(props);
   assert(children);
 
-  auto yogaNode = std::make_shared<YGNode>();
-  yogaNode->setConfig(suitableYogaConfig().get());
-  yogaNode->setStyle(props->yogaStyle);
-  yogaNode->setContext(this);
-  yogaNode->setDirty(true);
-  YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(*yogaNode, children);
-  yogaNode_ = yogaNode;
+  yogaNode_ = std::make_unique<YGNode>();
+  yogaNode_->setConfig(suitableYogaConfig().get());
+  yogaNode_->setStyle(props->yogaStyle);
+  yogaNode_->setContext(this);
+  yogaNode_->setDirty(true);
+  YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(yogaNode_.get(), children);
 }
 
 YogaLayoutableShadowNode::YogaLayoutableShadowNode(
@@ -51,21 +50,19 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode(
   const SharedYogaStylableProps &props,
   const SharedShadowNodeSharedList &children
 ) {
-  auto yogaNode = std::make_shared<YGNode>(*shadowNode->yogaNode_);
-  yogaNode->setConfig(suitableYogaConfig().get());
-  yogaNode->setContext(this);
-  yogaNode->setOwner(nullptr);
-  yogaNode->setDirty(true);
+  yogaNode_ = std::make_unique<YGNode>(*shadowNode->yogaNode_);
+  yogaNode_->setConfig(suitableYogaConfig().get());
+  yogaNode_->setContext(this);
+  yogaNode_->setOwner(nullptr);
+  yogaNode_->setDirty(true);
 
   if (props) {
-    yogaNode->setStyle(props->yogaStyle);
+    yogaNode_->setStyle(props->yogaStyle);
   }
 
   if (children) {
-    YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(*yogaNode, children);
+    YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(yogaNode_.get(), children);
   }
-
-  yogaNode_ = yogaNode;
 }
 
 void YogaLayoutableShadowNode::cleanLayout() {
@@ -99,21 +96,20 @@ void YogaLayoutableShadowNode::enableMeasurement() {
 void YogaLayoutableShadowNode::appendChild(SharedYogaLayoutableShadowNode child) {
   ensureUnsealed();
 
-  auto nonConstYogaNode = std::const_pointer_cast<YGNode>(yogaNode_);
-  auto nonConstChildYogaNode = std::const_pointer_cast<YGNode>(child->yogaNode_);
-  nonConstYogaNode->insertChild(nonConstChildYogaNode.get(), nonConstYogaNode->getChildrenCount());
+  auto yogaNodeRawPtr = yogaNode_.get();
+  auto childYogaNodeRawPtr = child->yogaNode_.get();
+  yogaNodeRawPtr->insertChild(childYogaNodeRawPtr, yogaNodeRawPtr->getChildrenCount());
 
-  if (nonConstChildYogaNode->getOwner() == nullptr) {
+  if (childYogaNodeRawPtr->getOwner() == nullptr) {
     child->ensureUnsealed();
-    nonConstChildYogaNode->setOwner(nonConstYogaNode.get());
+    childYogaNodeRawPtr->setOwner(yogaNodeRawPtr);
   }
 }
 
 void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {
   if (!getIsLayoutClean()) {
     ensureUnsealed();
-    YGNode *yogaNode = const_cast<YGNode *>(yogaNode_.get());
-    YGNodeCalculateLayout(yogaNode, YGUndefined, YGUndefined, YGDirectionInherit);
+    YGNodeCalculateLayout(yogaNode_.get(), YGUndefined, YGUndefined, YGDirectionInherit);
   }
 
   LayoutableShadowNode::layout(layoutContext);
@@ -206,7 +202,7 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector(YGNode *yogaNo
   };
 }
 
-void YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode &yogaNode, const SharedShadowNodeSharedList &children) {
+void YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode *yogaNodeRawPtr, const SharedShadowNodeSharedList &children) {
   auto yogaNodeChildren = YGVector();
 
   for (const SharedShadowNode &shadowNode : *children) {
@@ -216,17 +212,17 @@ void YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(YGNo
       continue;
     }
 
-    YGNode *yogaNodeChild = (YGNode *)yogaLayoutableShadowNode->yogaNode_.get();
+    auto &&childYogaNodeRawPtr = (YGNode *)yogaLayoutableShadowNode->yogaNode_.get();
 
-    yogaNodeChildren.push_back(yogaNodeChild);
+    yogaNodeChildren.push_back(childYogaNodeRawPtr);
 
-    if (yogaNodeChild->getOwner() == nullptr) {
+    if (childYogaNodeRawPtr->getOwner() == nullptr) {
       yogaLayoutableShadowNode->ensureUnsealed();
-      yogaNodeChild->setOwner(&yogaNode);
+      childYogaNodeRawPtr->setOwner(yogaNodeRawPtr);
     }
   }
 
-  yogaNode.setChildren(yogaNodeChildren);
+  yogaNodeRawPtr->setChildren(yogaNodeChildren);
 }
 
 } // namespace react
diff --git a/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.h
index 7c3a1b2f3..4ed364b75 100644
--- a/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.h
+++ b/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.h
@@ -23,9 +23,6 @@ namespace react {
 
 class YogaLayoutableShadowNode;
 
-// We accept that Yoga node is highly mutable thing and we don't try to enforce immutability,
-// so it does not have `const` qualifier (and we mark it as `mutable` in the class).
-using SharedYogaNode = std::shared_ptr<YGNode>;
 using SharedYogaConfig = std::shared_ptr<YGConfig>;
 
 using SharedYogaLayoutableShadowNode = std::shared_ptr<const YogaLayoutableShadowNode>;
@@ -83,11 +80,11 @@ public:
   void layoutChildren(LayoutContext layoutContext) override;
 
 protected:
-  mutable SharedYogaNode yogaNode_;
+  std::unique_ptr<YGNode> yogaNode_;
 
 private:
   static SharedYogaConfig suitableYogaConfig();
-  static void setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode &yogaNode, const SharedShadowNodeSharedList &children);
+  static void setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode *yogaNodeRawPtr, const SharedShadowNodeSharedList &children);
   static YGNode *yogaNodeCloneCallbackConnector(YGNode *oldYogaNode, YGNode *parentYogaNode, int childIndex);
   static YGSize yogaNodeMeasureCallbackConnector(YGNode *yogaNode, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode);
 };