From 7186a65b13de2ea0d319fad75f9362b7b8bb3cdc Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 21 Oct 2019 09:42:43 -0700 Subject: [PATCH] Fabric: Changes in `BaseTextShadowNode` to remove usage of `shared_from_this()` Summary: This is another step on the journey of removing `enable_shared_from_this`. It's unclear why we used that before but it's clear now that using shared_ptr here is not necessary because all computation around happens inside the single callstack, so by definition we don't have object life-time concerns here. Changelog: [Internal] Small Fabric-specific optimization. Reviewed By: sammy-SC Differential Revision: D17973957 fbshipit-source-id: 09a65c78e22083ed21b041240307f4858379cc60 --- .../components/text/basetext/BaseTextShadowNode.cpp | 10 +++++----- .../components/text/basetext/BaseTextShadowNode.h | 4 ++-- .../components/text/paragraph/ParagraphShadowNode.cpp | 4 ++-- .../androidtextinput/AndroidTextInputShadowNode.cpp | 3 +-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp index 37f7cb9360..927445be33 100644 --- a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp +++ b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp @@ -18,11 +18,11 @@ namespace facebook { namespace react { AttributedString BaseTextShadowNode::getAttributedString( - const TextAttributes &textAttributes, - const SharedShadowNode &parentNode) { + TextAttributes const &textAttributes, + ShadowNode const &parentNode) { auto attributedString = AttributedString{}; - for (const auto &childNode : parentNode->getChildren()) { + for (auto const &childNode : parentNode.getChildren()) { // RawShadowNode auto rawTextShadowNode = std::dynamic_pointer_cast(childNode); @@ -35,7 +35,7 @@ AttributedString BaseTextShadowNode::getAttributedString( // `attributedString` causes a retain cycle (besides that fact that we // don't need it at all). Storing a `ShadowView` instance instead of // `ShadowNode` should properly fix this problem. - fragment.parentShadowView = ShadowView(*parentNode); + fragment.parentShadowView = ShadowView(parentNode); attributedString.appendFragment(fragment); continue; } @@ -48,7 +48,7 @@ AttributedString BaseTextShadowNode::getAttributedString( localTextAttributes.apply(textShadowNode->getProps()->textAttributes); attributedString.appendAttributedString( textShadowNode->getAttributedString( - localTextAttributes, textShadowNode)); + localTextAttributes, *textShadowNode)); continue; } diff --git a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.h b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.h index 894642d701..c29ddb6650 100644 --- a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.h +++ b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.h @@ -27,8 +27,8 @@ class BaseTextShadowNode { * function, or if TextInput should inherit from BaseTextShadowNode. */ static AttributedString getAttributedString( - const TextAttributes &baseTextAttributes, - const SharedShadowNode &parentNode); + TextAttributes const &baseTextAttributes, + ShadowNode const &parentNode); }; } // namespace react diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp index c4cfbe85a5..ee02779f8b 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp @@ -19,8 +19,8 @@ AttributedString ParagraphShadowNode::getAttributedString() const { auto textAttributes = TextAttributes::defaultTextAttributes(); textAttributes.apply(getProps()->textAttributes); - cachedAttributedString_ = BaseTextShadowNode::getAttributedString( - textAttributes, shared_from_this()); + cachedAttributedString_ = + BaseTextShadowNode::getAttributedString(textAttributes, *this); } return cachedAttributedString_.value(); diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp index fd49a62588..22716875c8 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp @@ -35,8 +35,7 @@ AttributedString AndroidTextInputShadowNode::getAttributedString() const { textAttributes.apply(getProps()->textAttributes); // Use BaseTextShadowNode to get attributed string from children - return BaseTextShadowNode::getAttributedString( - textAttributes, shared_from_this()); + return BaseTextShadowNode::getAttributedString(textAttributes, *this); } #pragma mark - LayoutableShadowNode