Fabric: Using `shallowSourceNode()` inside `ViewShadowNode::cloneAndReplaceChild()`

Summary:
We have to call shallowSourceNode() in all cases of cloning which were not caused by UIManager instructions,
otherwise the diffing alogorith might produce incorrect mutation instructions.

Reviewed By: mdvacca

Differential Revision: D7503383

fbshipit-source-id: b33e5c39b7ba8cbd0f925fd29b3af379441a40a4
This commit is contained in:
Valentin Shergin 2018-04-10 12:46:12 -07:00 committed by Facebook Github Bot
parent 53837c4a4f
commit 47c0ab91a5
4 changed files with 52 additions and 2 deletions

View File

@ -109,7 +109,11 @@ void ShadowNode::clearSourceNode() {
}
void ShadowNode::shallowSourceNode() {
sourceNode_ = sourceNode_.lock()->getSourceNode();
ensureUnsealed();
auto sourceNode = sourceNode_.lock();
assert(sourceNode);
sourceNode_ = sourceNode->getSourceNode();
}
#pragma mark - Equality

View File

@ -58,7 +58,15 @@ public:
Tag getTag() const;
Tag getRootTag() const;
InstanceHandle getInstanceHandle() const;
/*
* Returns the node which was used as a prototype in clone constructor.
* The node is held as a weak reference so that the method may return
* `nullptr` in cases where the node was constructed using the explicit
* constructor or the node was already deallocated.
*/
SharedShadowNode getSourceNode() const;
void sealRecursive() const;
#pragma mark - Mutating Methods

View File

@ -94,3 +94,27 @@ TEST(ShadowNodeTest, handleShadowNodeMutation) {
node5->clearSourceNode();
ASSERT_EQ(node5->getSourceNode(), nullptr);
}
TEST(ShadowNodeTest, handleSourceNode) {
auto nodeFirstGeneration = std::make_shared<TestShadowNode>(9, 1, (void *)NULL);
auto nodeSecondGeneration = std::make_shared<TestShadowNode>(nodeFirstGeneration);
auto nodeThirdGeneration = std::make_shared<TestShadowNode>(nodeSecondGeneration);
auto nodeForthGeneration = std::make_shared<TestShadowNode>(nodeThirdGeneration);
// Ensure established shource nodes structure.
ASSERT_EQ(nodeForthGeneration->getSourceNode(), nodeThirdGeneration);
ASSERT_EQ(nodeThirdGeneration->getSourceNode(), nodeSecondGeneration);
ASSERT_EQ(nodeSecondGeneration->getSourceNode(), nodeFirstGeneration);
// Shallow source node for the forth generation node.
nodeForthGeneration->shallowSourceNode();
ASSERT_EQ(nodeForthGeneration->getSourceNode(), nodeSecondGeneration);
// Shallow it one more time.
nodeForthGeneration->shallowSourceNode();
ASSERT_EQ(nodeForthGeneration->getSourceNode(), nodeFirstGeneration);
// Ensure that 3th and 2nd were not affected.
ASSERT_EQ(nodeThirdGeneration->getSourceNode(), nodeSecondGeneration);
ASSERT_EQ(nodeSecondGeneration->getSourceNode(), nodeFirstGeneration);
}

View File

@ -94,7 +94,21 @@ SharedLayoutableShadowNode ViewShadowNode::cloneAndReplaceChild(const SharedLayo
auto viewShadowNodeChild = std::dynamic_pointer_cast<const ViewShadowNode>(child);
assert(viewShadowNodeChild);
auto viewShadowNodeChildClone = std::make_shared<const ViewShadowNode>(viewShadowNodeChild);
auto viewShadowNodeChildClone = std::make_shared<ViewShadowNode>(viewShadowNodeChild);
// 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").
viewShadowNodeChildClone->shallowSourceNode();
ShadowNode::replaceChild(viewShadowNodeChild, viewShadowNodeChildClone);
return std::static_pointer_cast<const LayoutableShadowNode>(viewShadowNodeChildClone);
}