Changing signature of RootShadowNode::clone() to remove a shared_from_this() call

Summary:
This is the first diff in the journey of removing `enable_shared_from_this` from `ShadowNode` class.

In general, using `enable_shared_from_this` is fine, it's a normal standard feature and there is nothing wrong with that... besides the fact that using this thing is usually an indication of overall poor design.
In Fabric, we don't really have a good reason for that, I think in all cases it can be avoided, so I think we should remove that.
Removing that is also code-size and perf win.

This particular diff changes the signature of `RootShadowNode::clone()` which allows removing the necessity  of usage `.shared_from_this()` from callsites.
The rest changes are purely cosmetical.

Changelog: [Internal] Small Fabric-specific optimization.

Reviewed By: sammy-SC

Differential Revision: D17973958

fbshipit-source-id: 5539ceff9d11b4281a6ebe8d80e90d6bd90e44d8

ReactCommon/fabric/components/root/RootShadowNode.cpp

@@ -33,10 +33,10 @@ void RootShadowNode::layout(
   }
 }
 
-UnsharedRootShadowNode RootShadowNode::clone(
-    const LayoutConstraints &layoutConstraints,
-    const LayoutContext &layoutContext) const {
-  auto props = std::make_shared<const RootProps>(
+RootShadowNode::Unshared RootShadowNode::clone(
+    LayoutConstraints const &layoutConstraints,
+    LayoutContext const &layoutContext) const {
+  auto props = std::make_shared<RootProps const>(
       *getProps(), layoutConstraints, layoutContext);
   auto newRootShadowNode = std::make_shared<RootShadowNode>(
       *this,
@@ -48,13 +48,13 @@ UnsharedRootShadowNode RootShadowNode::clone(
   return newRootShadowNode;
 }
 
-UnsharedRootShadowNode RootShadowNode::clone(
-    SharedShadowNode const &oldShadowNode,
-    SharedShadowNode const &newShadowNode) const {
-  auto ancestors = oldShadowNode->getAncestors(*this);
+RootShadowNode::Unshared RootShadowNode::clone(
+    ShadowNode const &oldShadowNode,
+    ShadowNode::Shared const &newShadowNode) const {
+  auto ancestors = oldShadowNode.getAncestors(*this);
 
   if (ancestors.size() == 0) {
-    return UnsharedRootShadowNode{nullptr};
+    return RootShadowNode::Unshared{nullptr};
   }
 
   auto childNode = newShadowNode;

ReactCommon/fabric/components/root/RootShadowNode.h

@@ -18,9 +18,6 @@ namespace react {
 
 class RootShadowNode;
 
-using SharedRootShadowNode = std::shared_ptr<const RootShadowNode>;
-using UnsharedRootShadowNode = std::shared_ptr<RootShadowNode>;
-
 extern const char RootComponentName[];
 
 /*
@@ -34,6 +31,9 @@ class RootShadowNode final
  public:
   using ConcreteViewShadowNode::ConcreteViewShadowNode;
 
+  using Shared = std::shared_ptr<RootShadowNode const>;
+  using Unshared = std::shared_ptr<RootShadowNode>;
+
   /*
    * Layouts the shadow tree.
    */
@@ -42,9 +42,9 @@ class RootShadowNode final
   /*
    * Clones the node with given `layoutConstraints` and `layoutContext`.
    */
-  UnsharedRootShadowNode clone(
-      const LayoutConstraints &layoutConstraints,
-      const LayoutContext &layoutContext) const;
+  RootShadowNode::Unshared clone(
+      LayoutConstraints const &layoutConstraints,
+      LayoutContext const &layoutContext) const;
 
   /*
    * Clones the node replacing a given old shadow node with a new one in the
@@ -52,9 +52,9 @@ class RootShadowNode final
    * the tree. Returns `nullptr` if the operation cannot be finished
    * successfully.
    */
-  UnsharedRootShadowNode clone(
-      const SharedShadowNode &oldShadowNode,
-      const SharedShadowNode &newShadowNode) const;
+  RootShadowNode::Unshared clone(
+      ShadowNode const &oldShadowNode,
+      ShadowNode::Shared const &newShadowNode) const;
 
  private:
   using YogaLayoutableShadowNode::layout;

ReactCommon/fabric/mounting/ShadowTree.cpp

@@ -135,7 +135,7 @@ bool ShadowTree::tryCommit(ShadowTreeCommitTransaction transaction) const {
   auto telemetry = MountingTelemetry{};
   telemetry.willCommit();
 
-  SharedRootShadowNode oldRootShadowNode;
+  RootShadowNode::Shared oldRootShadowNode;
 
   {
     // Reading `rootShadowNode_` in shared manner.
@@ -143,7 +143,7 @@ bool ShadowTree::tryCommit(ShadowTreeCommitTransaction transaction) const {
     oldRootShadowNode = rootShadowNode_;
   }
 
-  UnsharedRootShadowNode newRootShadowNode = transaction(oldRootShadowNode);
+  RootShadowNode::Unshared newRootShadowNode = transaction(oldRootShadowNode);
 
   if (!newRootShadowNode) {
     return false;
@@ -197,8 +197,8 @@ bool ShadowTree::tryCommit(ShadowTreeCommitTransaction transaction) const {
 
 void ShadowTree::commitEmptyTree() const {
   commit(
-      [](const SharedRootShadowNode &oldRootShadowNode)
-          -> UnsharedRootShadowNode {
+      [](RootShadowNode::Shared const &oldRootShadowNode)
+          -> RootShadowNode::Unshared {
         return std::make_shared<RootShadowNode>(
             *oldRootShadowNode,
             ShadowNodeFragment{

ReactCommon/fabric/mounting/ShadowTree.h

@@ -23,8 +23,8 @@
 namespace facebook {
 namespace react {
 
-using ShadowTreeCommitTransaction = std::function<UnsharedRootShadowNode(
-    const SharedRootShadowNode &oldRootShadowNode)>;
+using ShadowTreeCommitTransaction = std::function<RootShadowNode::Unshared(
+    RootShadowNode::Shared const &oldRootShadowNode)>;
 
 /*
  * Represents the shadow tree and its lifecycle.
@@ -36,9 +36,9 @@ class ShadowTree final {
    */
   ShadowTree(
       SurfaceId surfaceId,
-      const LayoutConstraints &layoutConstraints,
-      const LayoutContext &layoutContext,
-      const RootComponentDescriptor &rootComponentDescriptor);
+      LayoutConstraints const &layoutConstraints,
+      LayoutContext const &layoutContext,
+      RootComponentDescriptor const &rootComponentDescriptor);
 
   ~ShadowTree();
 
@@ -76,17 +76,18 @@ class ShadowTree final {
   ShadowTreeDelegate const *getDelegate() const;
 
  private:
-  UnsharedRootShadowNode cloneRootShadowNode(
-      const SharedRootShadowNode &oldRootShadowNode,
-      const LayoutConstraints &layoutConstraints,
-      const LayoutContext &layoutContext) const;
+  RootShadowNode::Unshared cloneRootShadowNode(
+      RootShadowNode::Shared const &oldRootShadowNode,
+      LayoutConstraints const &layoutConstraints,
+      LayoutContext const &layoutContext) const;
 
   void emitLayoutEvents(
       std::vector<LayoutableShadowNode const *> &affectedLayoutableNodes) const;
 
   SurfaceId const surfaceId_;
   mutable better::shared_mutex commitMutex_;
-  mutable SharedRootShadowNode rootShadowNode_; // Protected by `commitMutex_`.
+  mutable RootShadowNode::Shared
+      rootShadowNode_; // Protected by `commitMutex_`.
   mutable ShadowTreeRevision::Number revisionNumber_{
       0}; // Protected by `commitMutex_`.
   ShadowTreeDelegate const *delegate_;

ReactCommon/fabric/uimanager/Scheduler.cpp

@@ -32,7 +32,8 @@ Scheduler::Scheduler(
   auto uiManager = std::make_shared<UIManager>();
   auto eventOwnerBox = std::make_shared<EventBeat::OwnerBox>();
 
-  auto eventPipe = [=](jsi::Runtime &runtime,
+  auto eventPipe = [uiManager](
+                       jsi::Runtime &runtime,
                        const EventTarget *eventTarget,
                        const std::string &type,
                        const ValueFactory &payloadFactory) {
@@ -42,10 +43,10 @@ Scheduler::Scheduler(
     });
   };
 
-  auto statePipe = [=](const StateData::Shared &data,
+  auto statePipe = [uiManager](
+                       const StateData::Shared &data,
                        const StateTarget &stateTarget) {
-    uiManager->updateState(
-        stateTarget.getShadowNode().shared_from_this(), data);
+    uiManager->updateState(stateTarget.getShadowNode(), data);
   };
 
   eventDispatcher_ = std::make_shared<EventDispatcher>(
@@ -127,7 +128,7 @@ void Scheduler::renderTemplateToSurface(
 
     uiManager_->getShadowTreeRegistry().visit(
         surfaceId, [=](const ShadowTree &shadowTree) {
-          return shadowTree.tryCommit([&](const SharedRootShadowNode
+          return shadowTree.tryCommit([&](RootShadowNode::Shared const
                                               &oldRootShadowNode) {
             return std::make_shared<RootShadowNode>(
                 *oldRootShadowNode,
@@ -189,7 +190,7 @@ Size Scheduler::measureSurface(
   uiManager_->getShadowTreeRegistry().visit(
       surfaceId, [&](const ShadowTree &shadowTree) {
         shadowTree.tryCommit(
-            [&](const SharedRootShadowNode &oldRootShadowNode) {
+            [&](RootShadowNode::Shared const &oldRootShadowNode) {
               auto rootShadowNode =
                   oldRootShadowNode->clone(layoutConstraints, layoutContext);
               rootShadowNode->layout();
@@ -207,8 +208,8 @@ void Scheduler::constraintSurfaceLayout(
   SystraceSection s("Scheduler::constraintSurfaceLayout");
 
   uiManager_->getShadowTreeRegistry().visit(
-      surfaceId, [&](const ShadowTree &shadowTree) {
-        shadowTree.commit([&](const SharedRootShadowNode &oldRootShadowNode) {
+      surfaceId, [&](ShadowTree const &shadowTree) {
+        shadowTree.commit([&](RootShadowNode::Shared const &oldRootShadowNode) {
           return oldRootShadowNode->clone(layoutConstraints, layoutContext);
         });
       });
@@ -250,7 +251,7 @@ void Scheduler::uiManagerDidFinishTransaction(
 
   uiManager_->getShadowTreeRegistry().visit(
       surfaceId, [&](const ShadowTree &shadowTree) {
-        shadowTree.commit([&](const SharedRootShadowNode &oldRootShadowNode) {
+        shadowTree.commit([&](RootShadowNode::Shared const &oldRootShadowNode) {
           return std::make_shared<RootShadowNode>(
               *oldRootShadowNode,
               ShadowNodeFragment{

ReactCommon/fabric/uimanager/UIManager.cpp

@@ -125,22 +125,22 @@ void UIManager::clearJSResponder() const {
 }
 
 void UIManager::setNativeProps(
-    const SharedShadowNode &shadowNode,
-    const RawProps &rawProps) const {
+    ShadowNode const &shadowNode,
+    RawProps const &rawProps) const {
   SystraceSection s("UIManager::setNativeProps");
 
-  auto &componentDescriptor = shadowNode->getComponentDescriptor();
-  auto props = componentDescriptor.cloneProps(shadowNode->getProps(), rawProps);
-  auto newShadowNode = shadowNode->clone({
+  auto &componentDescriptor = shadowNode.getComponentDescriptor();
+  auto props = componentDescriptor.cloneProps(shadowNode.getProps(), rawProps);
+  auto newShadowNode = shadowNode.clone({
       /* .tag = */ ShadowNodeFragment::tagPlaceholder(),
       /* .surfaceId = */ ShadowNodeFragment::surfaceIdPlaceholder(),
       /* .props = */ props,
   });
 
   shadowTreeRegistry_.visit(
-      shadowNode->getSurfaceId(), [&](const ShadowTree &shadowTree) {
+      shadowNode.getSurfaceId(), [&](ShadowTree const &shadowTree) {
         shadowTree.tryCommit(
-            [&](const SharedRootShadowNode &oldRootShadowNode) {
+            [&](RootShadowNode::Shared const &oldRootShadowNode) {
               return oldRootShadowNode->clone(shadowNode, newShadowNode);
             });
       });
@@ -155,7 +155,7 @@ LayoutMetrics UIManager::getRelativeLayoutMetrics(
     shadowTreeRegistry_.visit(
         shadowNode.getSurfaceId(), [&](const ShadowTree &shadowTree) {
           shadowTree.tryCommit(
-              [&](const SharedRootShadowNode &oldRootShadowNode) {
+              [&](RootShadowNode::Shared const &oldRootShadowNode) {
                 ancestorShadowNode = oldRootShadowNode.get();
                 return nullptr;
               });
@@ -176,12 +176,12 @@ LayoutMetrics UIManager::getRelativeLayoutMetrics(
 }
 
 void UIManager::updateState(
-    const SharedShadowNode &shadowNode,
-    const StateData::Shared &rawStateData) const {
-  auto &componentDescriptor = shadowNode->getComponentDescriptor();
+    ShadowNode const &shadowNode,
+    StateData::Shared const &rawStateData) const {
+  auto &componentDescriptor = shadowNode.getComponentDescriptor();
   auto state =
-      componentDescriptor.createState(shadowNode->getState(), rawStateData);
-  auto newShadowNode = shadowNode->clone({
+      componentDescriptor.createState(shadowNode.getState(), rawStateData);
+  auto newShadowNode = shadowNode.clone({
       /* .tag = */ ShadowNodeFragment::tagPlaceholder(),
       /* .surfaceId = */ ShadowNodeFragment::surfaceIdPlaceholder(),
       /* .props = */ ShadowNodeFragment::propsPlaceholder(),
@@ -192,9 +192,9 @@ void UIManager::updateState(
   });
 
   shadowTreeRegistry_.visit(
-      shadowNode->getSurfaceId(), [&](const ShadowTree &shadowTree) {
+      shadowNode.getSurfaceId(), [&](const ShadowTree &shadowTree) {
         shadowTree.tryCommit(
-            [&](const SharedRootShadowNode &oldRootShadowNode) {
+            [&](RootShadowNode::Shared const &oldRootShadowNode) {
               return oldRootShadowNode->clone(shadowNode, newShadowNode);
             });
       });
@@ -236,7 +236,7 @@ ShadowNode::Shared UIManager::findShadowNodeByTag_DEPRECATED(Tag tag) const {
     // pointer to a root node because of the possible data race.
     // To work around this, we ask for a commit and immediately cancel it
     // returning `nullptr` instead of a new shadow tree.
-    shadowTree.tryCommit([&](SharedRootShadowNode const &oldRootShadowNode) {
+    shadowTree.tryCommit([&](RootShadowNode::Shared const &oldRootShadowNode) {
       rootShadowNode = oldRootShadowNode;
       return nullptr;
     });

ReactCommon/fabric/uimanager/UIManager.h

@@ -70,9 +70,8 @@ class UIManager {
       SurfaceId surfaceId,
       const SharedShadowNodeUnsharedList &rootChildren) const;
 
-  void setNativeProps(
-      const SharedShadowNode &shadowNode,
-      const RawProps &rawProps) const;
+  void setNativeProps(ShadowNode const &shadowNode, RawProps const &rawProps)
+      const;
 
   void setJSResponder(
       const SharedShadowNode &shadowNode,
@@ -94,8 +93,8 @@ class UIManager {
    * and performs a commit.
    */
   void updateState(
-      const SharedShadowNode &shadowNode,
-      const StateData::Shared &rawStateData) const;
+      ShadowNode const &shadowNode,
+      StateData::Shared const &rawStateData) const;
 
   void dispatchCommand(
       const SharedShadowNode &shadowNode,

ReactCommon/fabric/uimanager/UIManagerBinding.cpp

@@ -551,7 +551,7 @@ jsi::Value UIManagerBinding::get(
             const jsi::Value *arguments,
             size_t count) -> jsi::Value {
           uiManager->setNativeProps(
-              shadowNodeFromValue(runtime, arguments[0]),
+              *shadowNodeFromValue(runtime, arguments[0]),
               RawProps(runtime, arguments[1]));
 
           return jsi::Value::undefined();