Fix bugs in MutationObserver

Summary:
Changelog: [internal]

(this is internal because this API isn't enabled in OSS yet)

This fixes 2 bugs in `MutationObserver`:
1. Incorrectly reporting updates for the same node multiple times, if changes are observable by different targets in the same observer.
2. Incorrectly ignoring observations of subsequent targets in a given observer.

These were caught when migrating the unit tests for `MutationObserver` that were mocking Fabric to a Fantom integration test that uses the whole C++ infra.

Differential Revision: D66232571

packages/react-native/ReactCommon/react/renderer/observers/mutation/MutationObserver.cpp

@@ -133,31 +133,30 @@ void MutationObserver::recordMutationsInTarget(
   }
 
   recordMutationsInSubtrees(
-      std::move(targetShadowNode),
-      *oldTargetShadowNode,
-      *newTargetShadowNode,
+      oldTargetShadowNode,
+      newTargetShadowNode,
       observeSubtree,
       recordedMutations,
       processedNodes);
 }
 
 void MutationObserver::recordMutationsInSubtrees(
-    ShadowNode::Shared targetShadowNode,
-    const ShadowNode& oldNode,
-    const ShadowNode& newNode,
+    const ShadowNode::Shared& oldNode,
+    const ShadowNode::Shared& newNode,
     bool observeSubtree,
     std::vector<MutationRecord>& recordedMutations,
-    SetOfShadowNodePointers processedNodes) const {
-  bool isSameNode = &oldNode == &newNode;
+    SetOfShadowNodePointers& processedNodes) const {
+  bool isSameNode = oldNode.get() == newNode.get();
   // If the nodes are referentially equal, their children are also the same.
-  if (isSameNode || processedNodes.find(&newNode) != processedNodes.end()) {
+  if (isSameNode ||
+      processedNodes.find(oldNode.get()) != processedNodes.end()) {
     return;
   }
 
-  processedNodes.insert(&newNode);
+  processedNodes.insert(oldNode.get());
 
-  auto oldChildren = oldNode.getChildren();
-  auto newChildren = newNode.getChildren();
+  auto oldChildren = oldNode->getChildren();
+  auto newChildren = newNode->getChildren();
 
   std::vector<ShadowNode::Shared> addedNodes;
   std::vector<ShadowNode::Shared> removedNodes;
@@ -171,9 +170,8 @@ void MutationObserver::recordMutationsInSubtrees(
       // Nodes are present in both tress. If `subtree` is set to true,
       // we continue checking their children.
       recordMutationsInSubtrees(
-          targetShadowNode,
-          *oldChild,
-          *newChild,
+          oldChild,
+          newChild,
           observeSubtree,
           recordedMutations,
           processedNodes);
@@ -191,7 +189,7 @@ void MutationObserver::recordMutationsInSubtrees(
   if (!addedNodes.empty() || !removedNodes.empty()) {
     recordedMutations.emplace_back(MutationRecord{
         mutationObserverId_,
-        targetShadowNode,
+        oldNode,
         std::move(addedNodes),
         std::move(removedNodes)});
   }

packages/react-native/ReactCommon/react/renderer/observers/mutation/MutationObserver.h

@@ -27,6 +27,15 @@ class MutationObserver {
  public:
   MutationObserver(MutationObserverId intersectionObserverId);
 
+  // delete copy constructor
+  MutationObserver(const MutationObserver&) = delete;
+
+  // delete copy assignment
+  MutationObserver& operator=(const MutationObserver&) = delete;
+
+  // allow move constructor
+  MutationObserver(MutationObserver&&) = default;
+
   void observe(ShadowNode::Shared targetShadowNode, bool observeSubtree);
   void unobserve(const ShadowNode& targetShadowNode);
 
@@ -53,12 +62,11 @@ class MutationObserver {
       SetOfShadowNodePointers& processedNodes) const;
 
   void recordMutationsInSubtrees(
-      ShadowNode::Shared targetShadowNode,
-      const ShadowNode& oldNode,
-      const ShadowNode& newNode,
+      const ShadowNode::Shared& oldNode,
+      const ShadowNode::Shared& newNode,
       bool observeSubtree,
       std::vector<MutationRecord>& recordedMutations,
-      SetOfShadowNodePointers processedNodes) const;
+      SetOfShadowNodePointers& processedNodes) const;
 };
 
 } // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/observers/mutation/MutationObserverManager.cpp

@@ -29,9 +29,9 @@ void MutationObserverManager::observe(
   if (observerIt == observers.end()) {
     auto observer = MutationObserver{mutationObserverId};
     observer.observe(shadowNode, observeSubtree);
-    observers.insert({mutationObserverId, std::move(observer)});
+    observers.emplace(mutationObserverId, std::move(observer));
   } else {
-    auto observer = observerIt->second;
+    auto& observer = observerIt->second;
     observer.observe(shadowNode, observeSubtree);
   }
 }