From 01805636b9c19262ff01c6db06ee3acb34c0a4ca Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 28 Jan 2020 09:27:56 -0800 Subject: [PATCH] Move ShadowNode::getAncestors to ShadowNodeFamily Summary: Changelog: [internal] 1. Moves `ShadowNode::getAncestors` to `ShadowNodeFamily`. 2. Exposes shadowNode's family through `ShadowNode::getFamily()`. # Why? This is a first step in order to merge `StateCoordinator` into `ShadowNodeFamily` and use it as target for state updates. Reviewed By: shergin Differential Revision: D19465188 fbshipit-source-id: b5a3625aa21c040301259de02beedbf97e11f20e --- .../fabric/components/root/RootShadowNode.cpp | 5 +- .../fabric/components/root/RootShadowNode.h | 2 +- .../ConcreteComponentDescriptor.h | 1 - .../core/layout/LayoutableShadowNode.cpp | 2 +- .../fabric/core/shadownode/ShadowNode.cpp | 42 +------ .../fabric/core/shadownode/ShadowNode.h | 15 +-- .../core/shadownode/ShadowNodeFamily.cpp | 43 ++++++++ .../fabric/core/shadownode/ShadowNodeFamily.h | 18 +++ ReactCommon/fabric/core/state/State.cpp | 1 - .../core/tests/ShadowNodeFamilyTest.cpp | 104 ++++++++++++++++++ .../fabric/core/tests/ShadowNodeTest.cpp | 12 -- ReactCommon/fabric/uimanager/UIManager.cpp | 4 +- 12 files changed, 176 insertions(+), 73 deletions(-) create mode 100644 ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp diff --git a/ReactCommon/fabric/components/root/RootShadowNode.cpp b/ReactCommon/fabric/components/root/RootShadowNode.cpp index 425eaeea3dd..7226afe6015 100644 --- a/ReactCommon/fabric/components/root/RootShadowNode.cpp +++ b/ReactCommon/fabric/components/root/RootShadowNode.cpp @@ -47,10 +47,10 @@ RootShadowNode::Unshared RootShadowNode::clone( } RootShadowNode::Unshared RootShadowNode::clone( - ShadowNode const &shadowNode, + ShadowNodeFamily const &shadowNodeFamily, std::function callback) const { - auto ancestors = shadowNode.getAncestors(*this); + auto ancestors = shadowNodeFamily.getAncestors(*this); if (ancestors.size() == 0) { return RootShadowNode::Unshared{nullptr}; @@ -59,7 +59,6 @@ RootShadowNode::Unshared RootShadowNode::clone( auto &parent = ancestors.back(); auto &oldShadowNode = parent.first.get().getChildren().at(parent.second); - assert(ShadowNode::sameFamily(shadowNode, *oldShadowNode)); auto newShadowNode = callback(*oldShadowNode); auto childNode = newShadowNode; diff --git a/ReactCommon/fabric/components/root/RootShadowNode.h b/ReactCommon/fabric/components/root/RootShadowNode.h index 9899a6e170e..1e2761cc2a6 100644 --- a/ReactCommon/fabric/components/root/RootShadowNode.h +++ b/ReactCommon/fabric/components/root/RootShadowNode.h @@ -55,7 +55,7 @@ class RootShadowNode final * Returns `nullptr` if the operation cannot be performed successfully. */ RootShadowNode::Unshared clone( - ShadowNode const &shadowNode, + ShadowNodeFamily const &shadowNodeFamily, std::function callback) const; diff --git a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h index 3b62f34d3d7..24ba7656a9a 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h @@ -17,7 +17,6 @@ #include #include #include -#include namespace facebook { namespace react { diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp index 206aa2a2bd4..cad2cb7e231 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp @@ -99,7 +99,7 @@ LayoutMetrics LayoutableShadowNode::getRelativeLayoutMetrics( return layoutMetrics; } - auto ancestors = shadowNode.getAncestors(ancestorShadowNode); + auto ancestors = shadowNode.getFamily().getAncestors(ancestorShadowNode); if (ancestors.size() == 0) { return EmptyLayoutMetrics; diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.cpp b/ReactCommon/fabric/core/shadownode/ShadowNode.cpp index f3567d33a7f..5ac8064798b 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.cpp +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.cpp @@ -17,8 +17,6 @@ namespace facebook { namespace react { -using AncestorList = ShadowNode::AncestorList; - SharedShadowNodeSharedList ShadowNode::emptySharedShadowNodeSharedList() { static const auto emptySharedShadowNodeSharedList = std::make_shared(); @@ -215,44 +213,8 @@ void ShadowNode::setMounted(bool mounted) const { family_->eventEmitter_->setEnabled(mounted); } -AncestorList ShadowNode::getAncestors( - ShadowNode const &ancestorShadowNode) const { - auto families = better::small_vector{}; - auto ancestorFamily = ancestorShadowNode.family_.get(); - - auto family = family_.get(); - while (family && family != ancestorFamily) { - families.push_back(family); - family = family->parent_.lock().get(); - } - - if (family != ancestorFamily) { - return {}; - } - - auto ancestors = AncestorList{}; - auto parentNode = &ancestorShadowNode; - for (auto it = families.rbegin(); it != families.rend(); it++) { - auto childFamily = *it; - auto found = false; - auto childIndex = 0; - for (const auto &childNode : *parentNode->children_) { - if (childNode->family_.get() == childFamily) { - ancestors.push_back({*parentNode, childIndex}); - parentNode = childNode.get(); - found = true; - break; - } - childIndex++; - } - - if (!found) { - ancestors.clear(); - return ancestors; - } - } - - return ancestors; +ShadowNodeFamily const &ShadowNode::getFamily() const { + return *family_; } #pragma mark - DebugStringConvertible diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.h b/ReactCommon/fabric/core/shadownode/ShadowNode.h index cc3471f8073..bd9399d6689 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.h @@ -28,7 +28,6 @@ static constexpr const int kShadowNodeChildrenSmallVectorSize = 8; class ComponentDescriptor; struct ShadowNodeFragment; - class ShadowNode; using SharedShadowNode = std::shared_ptr; @@ -127,6 +126,8 @@ class ShadowNode : public virtual Sealable, void sealRecursive() const; + ShadowNodeFamily const &getFamily() const; + #pragma mark - Mutating Methods void appendChild(ShadowNode::Shared const &child); @@ -142,17 +143,6 @@ class ShadowNode : public virtual Sealable, */ void setMounted(bool mounted) const; - /* - * Returns a list of all ancestors of the node relative to the given ancestor. - * The list starts from the given ancestor node and ends with the parent node - * of `this` node. The elements of the list have a reference to some parent - * node and an index of the child of the parent node. - * Returns an empty array if there is no ancestor-descendant relationship. - * Can be called from any thread. - * The theoretical complexity of the algorithm is `O(ln(n))`. Use it wisely. - */ - AncestorList getAncestors(ShadowNode const &ancestorShadowNode) const; - #pragma mark - DebugStringConvertible #if RN_DEBUG_STRING_CONVERTIBLE @@ -175,6 +165,7 @@ class ShadowNode : public virtual Sealable, State::Shared state_; private: + friend ShadowNodeFamily; /* * Clones the list of children (and creates a new `shared_ptr` to it) if * `childrenAreShared_` flag is `true`. diff --git a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp index 30a74b54a00..d96bae1a6eb 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp +++ b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp @@ -6,12 +6,15 @@ */ #include "ShadowNodeFamily.h" +#include "ShadowNode.h" #include namespace facebook { namespace react { +using AncestorList = ShadowNode::AncestorList; + ShadowNodeFamily::ShadowNodeFamily( ShadowNodeFamilyFragment const &fragment, ComponentDescriptor const &componentDescriptor) @@ -40,5 +43,45 @@ ComponentName ShadowNodeFamily::getComponentName() const { return componentName_; } +AncestorList ShadowNodeFamily::getAncestors( + ShadowNode const &ancestorShadowNode) const { + auto families = better::small_vector{}; + auto ancestorFamily = ancestorShadowNode.family_.get(); + + auto family = this; + while (family && family != ancestorFamily) { + families.push_back(family); + family = family->parent_.lock().get(); + } + + if (family != ancestorFamily) { + return {}; + } + + auto ancestors = AncestorList{}; + auto parentNode = &ancestorShadowNode; + for (auto it = families.rbegin(); it != families.rend(); it++) { + auto childFamily = *it; + auto found = false; + auto childIndex = 0; + for (const auto &childNode : *parentNode->children_) { + if (childNode->family_.get() == childFamily) { + ancestors.push_back({*parentNode, childIndex}); + parentNode = childNode.get(); + found = true; + break; + } + childIndex++; + } + + if (!found) { + ancestors.clear(); + return ancestors; + } + } + + return ancestors; +} + } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h index 43500a88767..f4ff7169edb 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h @@ -17,6 +17,7 @@ namespace facebook { namespace react { class ComponentDescriptor; +class ShadowNode; /* * Represents all things that shadow nodes from the same family have in common. @@ -27,6 +28,12 @@ class ShadowNodeFamily { using Shared = std::shared_ptr; using Weak = std::weak_ptr; + using AncestorList = better::small_vector< + std::pair< + std::reference_wrapper /* parentNode */, + int /* childIndex */>, + 64>; + ShadowNodeFamily( ShadowNodeFamilyFragment const &fragment, ComponentDescriptor const &componentDescriptor); @@ -44,6 +51,17 @@ class ShadowNodeFamily { ComponentHandle getComponentHandle() const; ComponentName getComponentName() const; + /* + * Returns a list of all ancestors of the node relative to the given ancestor. + * The list starts from the given ancestor node and ends with the parent node + * of `this` node. The elements of the list have a reference to some parent + * node and an index of the child of the parent node. + * Returns an empty array if there is no ancestor-descendant relationship. + * Can be called from any thread. + * The theoretical complexity of the algorithm is `O(ln(n))`. Use it wisely. + */ + AncestorList getAncestors(ShadowNode const &ancestorShadowNode) const; + private: friend ShadowNode; diff --git a/ReactCommon/fabric/core/state/State.cpp b/ReactCommon/fabric/core/state/State.cpp index a880bcf046c..1b16e370c2a 100644 --- a/ReactCommon/fabric/core/state/State.cpp +++ b/ReactCommon/fabric/core/state/State.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #ifdef ANDROID #include diff --git a/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp new file mode 100644 index 00000000000..bd06d26d001 --- /dev/null +++ b/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp @@ -0,0 +1,104 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include +#include "TestComponent.h" + +using namespace facebook::react; + +TEST(ShadowNodeFamilyTest, sealObjectCorrectly) { + /* + * The structure: + * + * + * + * + * + */ + SurfaceId surfaceId = 1; + auto eventDispatcher = std::shared_ptr(); + auto componentDescriptor = TestComponentDescriptor({eventDispatcher}); + auto props = std::make_shared(); + + auto familyAAA = std::make_shared( + ShadowNodeFamilyFragment{ + /* .tag = */ 12, + /* .surfaceId = */ surfaceId, + /* .eventEmitter = */ nullptr, + }, + componentDescriptor); + + auto nodeAAA = std::make_shared( + ShadowNodeFragment{ + /* .props = */ props, + /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), + }, + familyAAA, + ShadowNodeTraits{}); + + auto nodeAAChildren = + std::make_shared(SharedShadowNodeList{nodeAAA}); + auto familyAA = std::make_shared( + ShadowNodeFamilyFragment{ + /* .tag = */ 11, + /* .surfaceId = */ surfaceId, + /* .eventEmitter = */ nullptr, + }, + componentDescriptor); + auto nodeAA = std::make_shared( + ShadowNodeFragment{ + /* .props = */ props, + /* .children = */ nodeAAChildren, + }, + familyAA, + ShadowNodeTraits{}); + + auto nodeAChildren = + std::make_shared(SharedShadowNodeList{nodeAA}); + + auto familyA = std::make_shared( + ShadowNodeFamilyFragment{ + /* .tag = */ 17, + /* .surfaceId = */ surfaceId, + /* .eventEmitter = */ nullptr, + }, + componentDescriptor); + auto nodeA = std::make_shared( + ShadowNodeFragment{ + /* .props = */ props, + /* .children = */ nodeAChildren, + }, + familyA, + ShadowNodeTraits{}); + + auto familyZ = std::make_shared( + ShadowNodeFamilyFragment{ + /* .tag = */ 18, + /* .surfaceId = */ surfaceId, + /* .eventEmitter = */ nullptr, + }, + componentDescriptor); + auto nodeZ = std::make_shared( + ShadowNodeFragment{ + /* .props = */ props, + /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), + }, + familyZ, + ShadowNodeTraits{}); + + // Negative case: + auto ancestors1 = nodeZ->getFamily().getAncestors(*nodeA); + EXPECT_EQ(ancestors1.size(), 0); + + // Positive case: + auto ancestors2 = nodeAAA->getFamily().getAncestors(*nodeA); + EXPECT_EQ(ancestors2.size(), 2); + EXPECT_EQ(&ancestors2[0].first.get(), nodeA.get()); + EXPECT_EQ(&ancestors2[1].first.get(), nodeAA.get()); +} diff --git a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp index e1d9c2ae809..ef367fd9229 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp @@ -222,18 +222,6 @@ TEST_F(ShadowNodeTest, handleCloneFunction) { EXPECT_EQ(nodeAB_->getProps(), nodeABClone->getProps()); } -TEST_F(ShadowNodeTest, handleBacktracking) { - // Negative case: - auto ancestors1 = nodeZ_->getAncestors(*nodeA_); - EXPECT_EQ(ancestors1.size(), 0); - - // Positive case: - auto ancestors2 = nodeABB_->getAncestors(*nodeA_); - EXPECT_EQ(ancestors2.size(), 2); - EXPECT_EQ(&ancestors2[0].first.get(), nodeA_.get()); - EXPECT_EQ(&ancestors2[1].first.get(), nodeAB_.get()); -} - TEST_F(ShadowNodeTest, handleState) { auto family = std::make_shared( ShadowNodeFamilyFragment{ diff --git a/ReactCommon/fabric/uimanager/UIManager.cpp b/ReactCommon/fabric/uimanager/UIManager.cpp index 88f57b32785..3dfc80e6e1a 100644 --- a/ReactCommon/fabric/uimanager/UIManager.cpp +++ b/ReactCommon/fabric/uimanager/UIManager.cpp @@ -153,7 +153,7 @@ void UIManager::setNativeProps( shadowTree.tryCommit( [&](RootShadowNode::Shared const &oldRootShadowNode) { return oldRootShadowNode->clone( - shadowNode, [&](ShadowNode const &oldShadowNode) { + shadowNode.getFamily(), [&](ShadowNode const &oldShadowNode) { return oldShadowNode.clone({ /* .props = */ props, }); @@ -200,7 +200,7 @@ void UIManager::updateState( shadowTree.tryCommit([&](RootShadowNode::Shared const &oldRootShadowNode) { return oldRootShadowNode->clone( - shadowNode, [&](ShadowNode const &oldShadowNode) { + shadowNode.getFamily(), [&](ShadowNode const &oldShadowNode) { auto &componentDescriptor = oldShadowNode.getComponentDescriptor(); auto state = componentDescriptor.createState(