Fabric: Strengthening StubView mutating validation

Summary:
This diff adds more enforcement for consistency of `ShadowNodeMutation`s, including:
* `Props` object for newly created or updated view must not be nullptr;
* `oldShadowView` must describe the previous state of the view for `Update` instruction;
* `ignoreDuplicateCreates` option was removed.

I suspect some of the crashes we see in Fabric are caused by a violation of one of these constraints. If one of these fails in debug builds, we will get an early signal.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D24880821

fbshipit-source-id: 8c8a3d8e205ce34f6e0335e8a2b0cf676930c284

ReactCommon/react/renderer/mounting/StubView.cpp

@@ -10,6 +10,18 @@
 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;

ReactCommon/react/renderer/mounting/StubView.h

@@ -25,6 +25,8 @@ class StubView final {
   StubView() = default;
   StubView(StubView const &stubView) = default;
 
+  operator ShadowView() const;
+
   void update(ShadowView const &shadowView);
 
   ComponentName componentName;

ReactCommon/react/renderer/mounting/StubViewTree.cpp

@@ -38,30 +38,19 @@ StubView const &StubViewTree::getRootStubView() const {
   return *registry.at(rootTag);
 }
 
-/**
- * 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) {
+void StubViewTree::mutate(ShadowViewMutationList const &mutations) {
   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>();
+        stubView->update(mutation.newChildShadowView);
         auto tag = mutation.newChildShadowView.tag;
         STUB_VIEW_LOG({ LOG(ERROR) << "StubView: Create: " << tag; });
-        if (!ignoreDuplicateCreates) {
-          STUB_VIEW_ASSERT(registry.find(tag) == registry.end());
-        }
+        STUB_VIEW_ASSERT(registry.find(tag) == registry.end());
         registry[tag] = stubView;
         break;
       }
@@ -73,6 +62,9 @@ void StubViewTree::mutate(
         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;
       }
@@ -137,19 +129,15 @@ void StubViewTree::mutate(
         STUB_VIEW_LOG({
           LOG(ERROR) << "StubView: Update: " << mutation.newChildShadowView.tag;
         });
-
-        // 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.props);
         STUB_VIEW_ASSERT(
-            mutation.newChildShadowView.tag ==
-                mutation.oldChildShadowView.tag ||
-            mutation.oldChildShadowView.tag == 0);
-
+            mutation.newChildShadowView.tag == mutation.oldChildShadowView.tag);
         STUB_VIEW_ASSERT(
             registry.find(mutation.newChildShadowView.tag) != registry.end());
-        auto stubView = registry[mutation.newChildShadowView.tag];
-        stubView->update(mutation.newChildShadowView);
+        auto oldStubView = registry[mutation.newChildShadowView.tag];
+        STUB_VIEW_ASSERT(
+            (ShadowView)(*oldStubView) == mutation.oldChildShadowView);
+        oldStubView->update(mutation.newChildShadowView);
         break;
       }
     }

ReactCommon/react/renderer/mounting/StubViewTree.h

@@ -21,9 +21,7 @@ class StubViewTree {
   StubViewTree() = default;
   StubViewTree(ShadowView const &shadowView);
 
-  void mutate(
-      ShadowViewMutationList const &mutations,
-      bool ignoreDuplicateCreates = false);
+  void mutate(ShadowViewMutationList const &mutations);
 
   StubView const &getRootStubView() const;
 

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, true);
+  stubViewTree.mutate(mutations);
   return stubViewTree;
 }