From e78bf723bff914dae5bb0da6280d5eb15c2ae293 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sun, 15 Jul 2018 16:46:17 -0700 Subject: [PATCH] Fabric: Removed last two plactical usages of `ShadowNode::sourceNode_` Summary: @public * In case of `ShadowTree` we just pass original old node as a `commit` method argument; * In case of `ConcreteViewShadowNode` we just don't need that because diffing algorithm does not use that information anymore. Reviewed By: mdvacca Differential Revision: D8753906 fbshipit-source-id: b8555083c7e72e9b3c0f9a8065745946d4cf44c7 --- ReactCommon/fabric/uimanager/ShadowTree.cpp | 6 +++--- ReactCommon/fabric/uimanager/ShadowTree.h | 2 +- ReactCommon/fabric/view/ConcreteViewShadowNode.h | 15 --------------- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/ReactCommon/fabric/uimanager/ShadowTree.cpp b/ReactCommon/fabric/uimanager/ShadowTree.cpp index 5dd54ecfa93..d179d9dcad1 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.cpp +++ b/ReactCommon/fabric/uimanager/ShadowTree.cpp @@ -78,7 +78,7 @@ void ShadowTree::complete(UnsharedRootShadowNode newRootShadowNode) { newRootShadowNode ); - if (commit(newRootShadowNode)) { + if (commit(oldRootShadowNode, newRootShadowNode)) { emitLayoutEvents(instructions); if (delegate_) { @@ -87,10 +87,10 @@ void ShadowTree::complete(UnsharedRootShadowNode newRootShadowNode) { } } -bool ShadowTree::commit(const SharedRootShadowNode &newRootShadowNode) { +bool ShadowTree::commit(const SharedRootShadowNode &oldRootShadowNode, const SharedRootShadowNode &newRootShadowNode) { std::lock_guard lock(commitMutex_); - if (newRootShadowNode->getSourceNode() != rootShadowNode_) { + if (oldRootShadowNode != rootShadowNode_) { return false; } diff --git a/ReactCommon/fabric/uimanager/ShadowTree.h b/ReactCommon/fabric/uimanager/ShadowTree.h index 3bb6af8b44d..26927fc8b63 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.h +++ b/ReactCommon/fabric/uimanager/ShadowTree.h @@ -77,7 +77,7 @@ private: UnsharedRootShadowNode cloneRootShadowNode(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const; void complete(UnsharedRootShadowNode newRootShadowNode); - bool commit(const SharedRootShadowNode &newRootShadowNode); + bool commit(const SharedRootShadowNode &oldRootShadowNode, const SharedRootShadowNode &newRootShadowNode); void emitLayoutEvents(const TreeMutationInstructionList &instructions); const Tag rootTag_; diff --git a/ReactCommon/fabric/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/view/ConcreteViewShadowNode.h index c65f9ea060d..38f0c966def 100644 --- a/ReactCommon/fabric/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/view/ConcreteViewShadowNode.h @@ -105,21 +105,6 @@ public: auto childShadowNode = std::dynamic_pointer_cast(child); assert(childShadowNode); auto childShadowNodeClone = childShadowNode->clone(); - - // This is overloading of `SharedLayoutableShadowNode::cloneAndReplaceChild`, - // the method is used to clone some node as a preparation for future mutation - // caused by relayout. - // Because those changes are not requested by UIManager, they add a layer - // of node generation (between the committed stage and new proposed stage). - // That additional layer confuses the Diffing algorithm which uses - // `sourceNode` for referencing the previous (aka committed) stage - // of the tree to produce mutation instructions. - // In other words, if we don't compensate this change here, - // the Diffing algorithm will compare wrong trees - // ("new-but-not-laid-out-yet vs. new" instead of "committed vs. new"). - auto nonConstChildShadowNodeClone = std::const_pointer_cast(childShadowNodeClone); - nonConstChildShadowNodeClone->shallowSourceNode(); - ShadowNode::replaceChild(childShadowNode, childShadowNodeClone); return std::dynamic_pointer_cast(childShadowNodeClone); }