Fix bugs in MutationObserver (#47760)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47760

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.

Reviewed By: sammy-SC

Differential Revision: D66232571

fbshipit-source-id: b6e967ca4deaa1a69d35f14d4f921103fec2bbaf
This commit is contained in:
Rubén Norte
2024-11-20 10:00:48 -08:00
committed by Facebook GitHub Bot
parent 43554b5a0f
commit 105beed569
3 changed files with 31 additions and 22 deletions
@@ -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)});
}
@@ -27,6 +27,18 @@ 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;
// allow move assignment
MutationObserver& operator=(MutationObserver&&) = default;
void observe(ShadowNode::Shared targetShadowNode, bool observeSubtree);
void unobserve(const ShadowNode& targetShadowNode);
@@ -53,12 +65,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
@@ -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);
}
}