From 71571911a2539afa3827ecefd3fa0e03729a99ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 27 Mar 2024 15:06:11 -0700 Subject: [PATCH] Dispatch notifications from MutationObserver as microtasks if available (#43664) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43664 Changelog: [internal] Now that we have the event loop, we can modify the implementation of `MutationObserver` (which is still not enabled by default) to dispatch the notifications as microtasks, making the API more spec-compliant. Reviewed By: javache Differential Revision: D55380178 --- .../NativeMutationObserver.cpp | 60 ++++++++++--------- .../mutationobserver/NativeMutationObserver.h | 20 ++++--- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.cpp b/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.cpp index c725c36252fbd7..c860176c387f46 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.cpp @@ -6,6 +6,7 @@ */ #include "NativeMutationObserver.h" +#include #include #include #include @@ -53,8 +54,8 @@ void NativeMutationObserver::unobserve( void NativeMutationObserver::connect( jsi::Runtime& runtime, - AsyncCallback<> notifyMutationObservers, - jsi::Function getPublicInstanceFromInstanceHandle) { + jsi::Function notifyMutationObservers, + SyncCallback getPublicInstanceFromInstanceHandle) { auto& uiManager = getUIManagerFromRuntime(runtime); // MutationObserver is not compatible with background executor. @@ -68,25 +69,10 @@ void NativeMutationObserver::connect( "MutationObserver: could not start observation because MutationObserver is incompatible with UIManager using background executor."); } - getPublicInstanceFromInstanceHandle_ = - jsi::Value(runtime, getPublicInstanceFromInstanceHandle); - - // This should always be called from the JS thread, as it's unsafe to call - // into JS otherwise (via `getPublicInstanceFromInstanceHandle`). - getPublicInstanceFromShadowNode_ = [&](const ShadowNode& shadowNode) { - auto instanceHandle = shadowNode.getInstanceHandle(runtime); - if (!instanceHandle.isObject() || - !getPublicInstanceFromInstanceHandle_.isObject() || - !getPublicInstanceFromInstanceHandle_.asObject(runtime).isFunction( - runtime)) { - return jsi::Value::null(); - } - return getPublicInstanceFromInstanceHandle_.asObject(runtime) - .asFunction(runtime) - .call(runtime, instanceHandle); - }; - - notifyMutationObservers_ = std::move(notifyMutationObservers); + runtime_ = &runtime; + notifyMutationObservers_.emplace(std::move(notifyMutationObservers)); + getPublicInstanceFromInstanceHandle_.emplace( + std::move(getPublicInstanceFromInstanceHandle)); auto onMutationsCallback = [&](std::vector& records) { return onMutations(records); @@ -98,9 +84,9 @@ void NativeMutationObserver::connect( void NativeMutationObserver::disconnect(jsi::Runtime& runtime) { auto& uiManager = getUIManagerFromRuntime(runtime); mutationObserverManager_.disconnect(uiManager); - getPublicInstanceFromInstanceHandle_ = jsi::Value::undefined(); - getPublicInstanceFromShadowNode_ = nullptr; - notifyMutationObservers_ = nullptr; + runtime_ = nullptr; + notifyMutationObservers_.reset(); + getPublicInstanceFromInstanceHandle_.reset(); } std::vector NativeMutationObserver::takeRecords( @@ -112,6 +98,16 @@ std::vector NativeMutationObserver::takeRecords( return records; } +jsi::Value NativeMutationObserver::getPublicInstanceFromShadowNode( + const ShadowNode& shadowNode) const { + auto instanceHandle = shadowNode.getInstanceHandle(*runtime_); + if (!instanceHandle.isObject()) { + return jsi::Value::null(); + } + return getPublicInstanceFromInstanceHandle_.value().call( + std::move(instanceHandle)); +} + std::vector NativeMutationObserver::getPublicInstancesFromShadowNodes( const std::vector& shadowNodes) const { @@ -119,7 +115,7 @@ NativeMutationObserver::getPublicInstancesFromShadowNodes( publicInstances.reserve(shadowNodes.size()); for (const auto& shadowNode : shadowNodes) { - publicInstances.push_back(getPublicInstanceFromShadowNode_(*shadowNode)); + publicInstances.push_back(getPublicInstanceFromShadowNode(*shadowNode)); } return publicInstances; @@ -133,7 +129,7 @@ void NativeMutationObserver::onMutations(std::vector& records) { record.mutationObserverId, // FIXME(T157129303) Instead of assuming we can call into JS from here, // we should use an API that explicitly indicates it. - getPublicInstanceFromShadowNode_(*record.targetShadowNode), + getPublicInstanceFromShadowNode(*record.targetShadowNode), getPublicInstancesFromShadowNodes(record.addedShadowNodes), getPublicInstancesFromShadowNodes(record.removedShadowNodes)}); } @@ -157,7 +153,17 @@ void NativeMutationObserver::notifyMutationObserversIfNecessary() { if (dispatchNotification) { SystraceSection s("NativeMutationObserver::notifyObservers"); - notifyMutationObservers_(); + if (ReactNativeFeatureFlags::enableMicrotasks()) { + runtime_->queueMicrotask(notifyMutationObservers_.value()); + } else { + jsInvoker_->invokeAsync([&](jsi::Runtime& runtime) { + // It's possible that the last observer was disconnected before we could + // dispatch this notification. + if (notifyMutationObservers_) { + notifyMutationObservers_.value().call(runtime); + } + }); + } } } diff --git a/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.h b/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.h index 1c067677b3a574..92114889ebb82e 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.h +++ b/packages/react-native/ReactCommon/react/nativemodule/mutationobserver/NativeMutationObserver.h @@ -10,7 +10,6 @@ #include #include #include -#include #include namespace facebook::react { @@ -60,8 +59,8 @@ class NativeMutationObserver void connect( jsi::Runtime& runtime, - AsyncCallback<> notifyMutationObservers, - jsi::Function getPublicInstanceFromInstanceHandle); + jsi::Function notifyMutationObservers, + SyncCallback getPublicInstanceFromInstanceHandle); void disconnect(jsi::Runtime& runtime); @@ -72,17 +71,22 @@ class NativeMutationObserver std::vector pendingRecords_; - // This is passed to `connect` so we can retain references to public instances - // when mutation occur, before React cleans up unmounted instances. - jsi::Value getPublicInstanceFromInstanceHandle_ = jsi::Value::undefined(); - std::function getPublicInstanceFromShadowNode_; + // We need to keep a reference to the JS runtime so we can schedule the + // notifications as microtasks when mutations occur. This is safe because + // mutations will only happen when executing JavaScript and because this + // native module will never survive the runtime. + jsi::Runtime* runtime_{}; bool notifiedMutationObservers_{}; - std::function notifyMutationObservers_; + std::optional notifyMutationObservers_; + std::optional> + getPublicInstanceFromInstanceHandle_; void onMutations(std::vector& records); void notifyMutationObserversIfNecessary(); + jsi::Value getPublicInstanceFromShadowNode( + const ShadowNode& shadowNode) const; std::vector getPublicInstancesFromShadowNodes( const std::vector& shadowNodes) const; };