Fabric: ConcreteShadowNode::getProps() was renamed to `getConcreteP…

…rops()` and got new return type. Summary: Having the overridden function that returns a different type is apparently not a good idea (and might cause bugs and unexpected behavior), so it was renamed. The function also got a new return type (`const &` instead of `std::shared_ptr`) for simplicity, better performance, and smaller code size. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D19837694 fbshipit-source-id: b7a96424bd040409371724907b3fb3931cd8a2e8
zerkella · Feb 14, 2020 · 967c9dc · 967c9dc
1 parent 6e3389c
commit 967c9dc
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 32 deletions.
diff --git a/ReactCommon/fabric/components/image/ImageShadowNode.cpp b/ReactCommon/fabric/components/image/ImageShadowNode.cpp
@@ -38,7 +38,7 @@ void ImageShadowNode::updateStateIfNeeded() {
 }
 
 ImageSource ImageShadowNode::getImageSource() const {
-  auto sources = getProps()->sources;
+  auto sources = getConcreteProps().sources;
 
   if (sources.size() == 0) {
     return {

diff --git a/ReactCommon/fabric/components/root/RootShadowNode.cpp b/ReactCommon/fabric/components/root/RootShadowNode.cpp
@@ -25,7 +25,7 @@ bool RootShadowNode::layoutIfNeeded(
 
   ensureUnsealed();
 
-  auto layoutContext = getProps()->layoutContext;
+  auto layoutContext = getConcreteProps().layoutContext;
   layoutContext.affectedNodes = affectedNodes;
 
   layout(layoutContext);
@@ -44,7 +44,7 @@ RootShadowNode::Unshared RootShadowNode::clone(
     LayoutConstraints const &layoutConstraints,
     LayoutContext const &layoutContext) const {
   auto props = std::make_shared<RootProps const>(
-      *getProps(), layoutConstraints, layoutContext);
+      getConcreteProps(), layoutConstraints, layoutContext);
   auto newRootShadowNode = std::make_shared<RootShadowNode>(
       *this,
       ShadowNodeFragment{

diff --git a/ReactCommon/fabric/components/slider/SliderShadowNode.cpp b/ReactCommon/fabric/components/slider/SliderShadowNode.cpp
@@ -66,19 +66,19 @@ void SliderShadowNode::updateStateIfNeeded() {
 }
 
 ImageSource SliderShadowNode::getTrackImageSource() const {
-  return getProps()->trackImage;
+  return getConcreteProps().trackImage;
 }
 
 ImageSource SliderShadowNode::getMinimumTrackImageSource() const {
-  return getProps()->minimumTrackImage;
+  return getConcreteProps().minimumTrackImage;
 }
 
 ImageSource SliderShadowNode::getMaximumTrackImageSource() const {
-  return getProps()->maximumTrackImage;
+  return getConcreteProps().maximumTrackImage;
 }
 
 ImageSource SliderShadowNode::getThumbImageSource() const {
-  return getProps()->thumbImage;
+  return getConcreteProps().thumbImage;
 }
 
 #pragma mark - LayoutableShadowNode

diff --git a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp
@@ -28,7 +28,7 @@ AttributedString BaseTextShadowNode::getAttributedString(
         std::dynamic_pointer_cast<const RawTextShadowNode>(childNode);
     if (rawTextShadowNode) {
       auto fragment = AttributedString::Fragment{};
-      fragment.string = rawTextShadowNode->getProps()->text;
+      fragment.string = rawTextShadowNode->getConcreteProps().text;
       fragment.textAttributes = textAttributes;
 
       // Storing a retaining pointer to `ParagraphShadowNode` inside
@@ -45,7 +45,8 @@ AttributedString BaseTextShadowNode::getAttributedString(
         std::dynamic_pointer_cast<const TextShadowNode>(childNode);
     if (textShadowNode) {
       auto localTextAttributes = textAttributes;
-      localTextAttributes.apply(textShadowNode->getProps()->textAttributes);
+      localTextAttributes.apply(
+          textShadowNode->getConcreteProps().textAttributes);
       attributedString.appendAttributedString(
           textShadowNode->getAttributedString(
               localTextAttributes, *textShadowNode));

diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp
@@ -18,7 +18,7 @@ char const ParagraphComponentName[] = "Paragraph";
 AttributedString ParagraphShadowNode::getAttributedString() const {
   if (!cachedAttributedString_.has_value()) {
     auto textAttributes = TextAttributes::defaultTextAttributes();
-    textAttributes.apply(getProps()->textAttributes);
+    textAttributes.apply(getConcreteProps().textAttributes);
 
     cachedAttributedString_ =
         BaseTextShadowNode::getAttributedString(textAttributes, *this);
@@ -49,8 +49,9 @@ void ParagraphShadowNode::updateStateIfNeeded() {
     return;
   }
 
-  setStateData(ParagraphState{
-      attributedString, getProps()->paragraphAttributes, textLayoutManager_});
+  setStateData(ParagraphState{attributedString,
+                              getConcreteProps().paragraphAttributes,
+                              textLayoutManager_});
 }
 
 #pragma mark - LayoutableShadowNode
@@ -64,7 +65,7 @@ Size ParagraphShadowNode::measure(LayoutConstraints layoutConstraints) const {
 
   return textLayoutManager_->measure(
       AttributedStringBox{attributedString},
-      getProps()->paragraphAttributes,
+      getConcreteProps().paragraphAttributes,
       layoutConstraints);
 }
 

diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp
@@ -32,17 +32,17 @@ void AndroidTextInputShadowNode::setContextContainer(
 AttributedString AndroidTextInputShadowNode::getAttributedString() const {
   // Use BaseTextShadowNode to get attributed string from children
   auto childTextAttributes = TextAttributes::defaultTextAttributes();
-  childTextAttributes.apply(getProps()->textAttributes);
+  childTextAttributes.apply(getConcreteProps().textAttributes);
   auto attributedString =
       BaseTextShadowNode::getAttributedString(childTextAttributes, *this);
 
   // BaseTextShadowNode only gets children. We must detect and prepend text
   // value attributes manually.
-  if (!getProps()->text.empty()) {
+  if (!getConcreteProps().text.empty()) {
     auto textAttributes = TextAttributes::defaultTextAttributes();
-    textAttributes.apply(getProps()->textAttributes);
+    textAttributes.apply(getConcreteProps().textAttributes);
     auto fragment = AttributedString::Fragment{};
-    fragment.string = getProps()->text;
+    fragment.string = getConcreteProps().text;
     fragment.textAttributes = textAttributes;
     // If the TextInput opacity is 0 < n < 1, the opacity of the TextInput and
     // text value's background will stack. This is a hack/workaround to prevent
@@ -64,14 +64,14 @@ AttributedString AndroidTextInputShadowNode::getPlaceholderAttributedString()
   // Return placeholder text, since text and children are empty.
   auto textAttributedString = AttributedString{};
   auto fragment = AttributedString::Fragment{};
-  fragment.string = getProps()->placeholder;
+  fragment.string = getConcreteProps().placeholder;
 
   if (fragment.string.empty()) {
     fragment.string = " ";
   }
 
   auto textAttributes = TextAttributes::defaultTextAttributes();
-  textAttributes.apply(getProps()->textAttributes);
+  textAttributes.apply(getConcreteProps().textAttributes);
 
   // If there's no text, it's possible that this Fragment isn't actually
   // appended to the AttributedString (see implementation of appendFragment)
@@ -125,12 +125,12 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() {
   // in the AttributedString, and when State is updated, it needs some way to
   // reconstruct a Fragment with default TextAttributes.
   auto defaultTextAttributes = TextAttributes::defaultTextAttributes();
-  defaultTextAttributes.apply(getProps()->textAttributes);
+  defaultTextAttributes.apply(getConcreteProps().textAttributes);
 
   auto newEventCount =
       (state.reactTreeAttributedString == reactTreeAttributedString
            ? 0
-           : getProps()->mostRecentEventCount);
+           : getConcreteProps().mostRecentEventCount);
   auto newAttributedString = getMostRecentAttributedString();
 
   // Even if we're here and updating state, it may be only to update the layout
@@ -141,7 +141,7 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() {
   setStateData(AndroidTextInputState{newEventCount,
                                      newAttributedString,
                                      reactTreeAttributedString,
-                                     getProps()->paragraphAttributes,
+                                     getConcreteProps().paragraphAttributes,
                                      defaultTextAttributes,
                                      ShadowView(*this),
                                      textLayoutManager_});
@@ -168,7 +168,7 @@ Size AndroidTextInputShadowNode::measure(
 
   return textLayoutManager_->measure(
       AttributedStringBox{attributedString},
-      getProps()->paragraphAttributes,
+      getConcreteProps().paragraphAttributes,
       layoutConstraints);
 }
 

diff --git a/ReactCommon/fabric/components/textinput/iostextinput/TextInputShadowNode.cpp b/ReactCommon/fabric/components/textinput/iostextinput/TextInputShadowNode.cpp
@@ -34,23 +34,23 @@ AttributedStringBox TextInputShadowNode::attributedStringBoxToMeasure() const {
       hasMeaningfulState ? AttributedString{} : getAttributedString();
 
   if (attributedString.isEmpty()) {
-    auto placeholder = getProps()->placeholder;
+    auto placeholder = getConcreteProps().placeholder;
     // Note: `zero-width space` is insufficient in some cases (e.g. when we need
     // to measure the "hight" of the font).
     auto string = !placeholder.empty() ? placeholder : "I";
-    auto textAttributes = getProps()->getEffectiveTextAttributes();
+    auto textAttributes = getConcreteProps().getEffectiveTextAttributes();
     attributedString.appendFragment({string, textAttributes, {}});
   }
 
   return AttributedStringBox{attributedString};
 }
 
 AttributedString TextInputShadowNode::getAttributedString() const {
-  auto textAttributes = getProps()->getEffectiveTextAttributes();
+  auto textAttributes = getConcreteProps().getEffectiveTextAttributes();
   auto attributedString = AttributedString{};
 
   attributedString.appendFragment(
-      AttributedString::Fragment{getProps()->text, textAttributes});
+      AttributedString::Fragment{getConcreteProps().text, textAttributes});
 
   attributedString.appendAttributedString(
       BaseTextShadowNode::getAttributedString(textAttributes, *this));
@@ -69,7 +69,7 @@ void TextInputShadowNode::updateStateIfNeeded() {
   if (!getState() || getState()->getRevision() == 0) {
     auto state = TextInputState{};
     state.attributedStringBox = AttributedStringBox{getAttributedString()};
-    state.paragraphAttributes = getProps()->paragraphAttributes;
+    state.paragraphAttributes = getConcreteProps().paragraphAttributes;
     state.layoutManager = textLayoutManager_;
     setStateData(std::move(state));
   }
@@ -80,7 +80,7 @@ void TextInputShadowNode::updateStateIfNeeded() {
 Size TextInputShadowNode::measure(LayoutConstraints layoutConstraints) const {
   return textLayoutManager_->measure(
       attributedStringBoxToMeasure(),
-      getProps()->getEffectiveParagraphAttributes(),
+      getConcreteProps().getEffectiveParagraphAttributes(),
       layoutConstraints);
 }
 

diff --git a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h
@@ -111,7 +111,7 @@ class ConcreteViewShadowNode : public ConcreteShadowNode<
   }
 
   Transform getTransform() const override {
-    return BaseShadowNode::getProps()->transform;
+    return BaseShadowNode::getConcreteProps().transform;
   }
 
 #pragma mark - DebugStringConvertible

diff --git a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h
@@ -80,12 +80,16 @@ class ConcreteShadowNode : public ShadowNode {
     return {};
   }
 
-  const SharedConcreteProps getProps() const {
+  /*
+   * Returns a concrete props object associated with the node.
+   * Thread-safe after the node is sealed.
+   */
+  ConcreteProps const &getConcreteProps() const {
     assert(props_ && "Props must not be `nullptr`.");
     assert(
         std::dynamic_pointer_cast<ConcreteProps const>(props_) &&
         "Props must be an instance of ConcreteProps class.");
-    return std::static_pointer_cast<ConcreteProps const>(props_);
+    return *static_cast<ConcreteProps const *>(props_.get());
   }
 
   /*