From 53862a1b5a58d3617bc2c0d5248f80e9d4397208 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Fri, 13 Nov 2020 08:48:49 -0800 Subject: [PATCH] Back out "Fabric: Strengthening StubView mutating validation" Summary: Changelog: [internal] Original commit changeset: 8c8a3d8e205c Reviewed By: ShikaSD Differential Revision: D24951341 fbshipit-source-id: b6109a45f1537a9edc702eafac1736e801fbedc9 --- .../react/renderer/mounting/StubView.cpp | 12 ------ .../react/renderer/mounting/StubView.h | 2 - .../react/renderer/mounting/StubViewTree.cpp | 38 ++++++++++++------- .../react/renderer/mounting/StubViewTree.h | 4 +- ReactCommon/react/renderer/mounting/stubs.cpp | 2 +- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/ReactCommon/react/renderer/mounting/StubView.cpp b/ReactCommon/react/renderer/mounting/StubView.cpp index 4d6b140da20a04..f476de31184899 100644 --- a/ReactCommon/react/renderer/mounting/StubView.cpp +++ b/ReactCommon/react/renderer/mounting/StubView.cpp @@ -10,18 +10,6 @@ namespace facebook { namespace react { -StubView::operator ShadowView() const { - auto shadowView = ShadowView{}; - shadowView.componentName = componentName; - shadowView.componentHandle = componentHandle; - shadowView.tag = tag; - shadowView.props = props; - shadowView.eventEmitter = eventEmitter; - shadowView.layoutMetrics = layoutMetrics; - shadowView.state = state; - return shadowView; -} - void StubView::update(ShadowView const &shadowView) { componentName = shadowView.componentName; componentHandle = shadowView.componentHandle; diff --git a/ReactCommon/react/renderer/mounting/StubView.h b/ReactCommon/react/renderer/mounting/StubView.h index 3c95f68f5be24f..8546baa157aee5 100644 --- a/ReactCommon/react/renderer/mounting/StubView.h +++ b/ReactCommon/react/renderer/mounting/StubView.h @@ -25,8 +25,6 @@ class StubView final { StubView() = default; StubView(StubView const &stubView) = default; - operator ShadowView() const; - void update(ShadowView const &shadowView); ComponentName componentName; diff --git a/ReactCommon/react/renderer/mounting/StubViewTree.cpp b/ReactCommon/react/renderer/mounting/StubViewTree.cpp index c2e3e3d62c5f76..0c9dedee025294 100644 --- a/ReactCommon/react/renderer/mounting/StubViewTree.cpp +++ b/ReactCommon/react/renderer/mounting/StubViewTree.cpp @@ -38,19 +38,30 @@ StubView const &StubViewTree::getRootStubView() const { return *registry.at(rootTag); } -void StubViewTree::mutate(ShadowViewMutationList const &mutations) { +/** + * ignoreDuplicateCreates: when stubs generates "fake" mutation instructions, in + * some cases it can produce too many "create" instructions. We ignore + * duplicates and treat them as noops. In the case of verifying actual diffing, + * that assert is left on. + * + * @param mutations + * @param ignoreDuplicateCreates + */ +void StubViewTree::mutate( + ShadowViewMutationList const &mutations, + bool ignoreDuplicateCreates) { STUB_VIEW_LOG({ LOG(ERROR) << "StubView: Mutating Begin"; }); for (auto const &mutation : mutations) { switch (mutation.type) { case ShadowViewMutation::Create: { STUB_VIEW_ASSERT(mutation.parentShadowView == ShadowView{}); STUB_VIEW_ASSERT(mutation.oldChildShadowView == ShadowView{}); - STUB_VIEW_ASSERT(mutation.newChildShadowView.props); auto stubView = std::make_shared(); - stubView->update(mutation.newChildShadowView); auto tag = mutation.newChildShadowView.tag; STUB_VIEW_LOG({ LOG(ERROR) << "StubView: Create: " << tag; }); - STUB_VIEW_ASSERT(registry.find(tag) == registry.end()); + if (!ignoreDuplicateCreates) { + STUB_VIEW_ASSERT(registry.find(tag) == registry.end()); + } registry[tag] = stubView; break; } @@ -62,9 +73,6 @@ void StubViewTree::mutate(ShadowViewMutationList const &mutations) { STUB_VIEW_ASSERT(mutation.newChildShadowView == ShadowView{}); auto tag = mutation.oldChildShadowView.tag; STUB_VIEW_ASSERT(registry.find(tag) != registry.end()); - auto stubView = registry[tag]; - STUB_VIEW_ASSERT( - (ShadowView)(*stubView) == mutation.oldChildShadowView); registry.erase(tag); break; } @@ -129,15 +137,19 @@ void StubViewTree::mutate(ShadowViewMutationList const &mutations) { STUB_VIEW_LOG({ LOG(ERROR) << "StubView: Update: " << mutation.newChildShadowView.tag; }); - STUB_VIEW_ASSERT(mutation.newChildShadowView.props); + + // We don't have a strict requirement that oldChildShadowView has any + // data. In particular, LayoutAnimations can produce UPDATEs with only a + // new node. STUB_VIEW_ASSERT( - mutation.newChildShadowView.tag == mutation.oldChildShadowView.tag); + mutation.newChildShadowView.tag == + mutation.oldChildShadowView.tag || + mutation.oldChildShadowView.tag == 0); + STUB_VIEW_ASSERT( registry.find(mutation.newChildShadowView.tag) != registry.end()); - auto oldStubView = registry[mutation.newChildShadowView.tag]; - STUB_VIEW_ASSERT( - (ShadowView)(*oldStubView) == mutation.oldChildShadowView); - oldStubView->update(mutation.newChildShadowView); + auto stubView = registry[mutation.newChildShadowView.tag]; + stubView->update(mutation.newChildShadowView); break; } } diff --git a/ReactCommon/react/renderer/mounting/StubViewTree.h b/ReactCommon/react/renderer/mounting/StubViewTree.h index 6ffb00e63ce96a..3dceaba3dc7e97 100644 --- a/ReactCommon/react/renderer/mounting/StubViewTree.h +++ b/ReactCommon/react/renderer/mounting/StubViewTree.h @@ -21,7 +21,9 @@ class StubViewTree { StubViewTree() = default; StubViewTree(ShadowView const &shadowView); - void mutate(ShadowViewMutationList const &mutations); + void mutate( + ShadowViewMutationList const &mutations, + bool ignoreDuplicateCreates = false); StubView const &getRootStubView() const; diff --git a/ReactCommon/react/renderer/mounting/stubs.cpp b/ReactCommon/react/renderer/mounting/stubs.cpp index 370989a21a8d7a..47ce20e9e2c400 100644 --- a/ReactCommon/react/renderer/mounting/stubs.cpp +++ b/ReactCommon/react/renderer/mounting/stubs.cpp @@ -54,7 +54,7 @@ StubViewTree stubViewTreeFromShadowNode(ShadowNode const &rootShadowNode) { ShadowNode::emptySharedShadowNodeSharedList()}); auto stubViewTree = StubViewTree(ShadowView(*emptyRootShadowNode)); - stubViewTree.mutate(mutations); + stubViewTree.mutate(mutations, true); return stubViewTree; }