From 433027a2adf14fdeaff38d31dd2c4603cbdf55bc Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 30 Jan 2026 09:44:01 -0800 Subject: [PATCH 1/4] Remove const qualifier from side-effectful methods of Agents and Targets (#55314) Summary: TSIA - minor refactor for correctness and clarity before making changes to some of these methods up the stack. Changelog: [Internal] Reviewed By: hoxyq Differential Revision: D91140491 --- .../ReactCommon/jsinspector-modern/HostAgent.cpp | 7 +++---- .../ReactCommon/jsinspector-modern/HostAgent.h | 4 ++-- .../ReactCommon/jsinspector-modern/HostTarget.cpp | 5 ++--- .../ReactCommon/jsinspector-modern/HostTarget.h | 2 +- .../ReactCommon/jsinspector-modern/TracingAgent.cpp | 4 ++-- .../ReactCommon/jsinspector-modern/TracingAgent.h | 4 ++-- 6 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp index 41d81767df35..888d27936e90 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp @@ -385,8 +385,7 @@ class HostAgent::Impl final { return fuseboxClientType_ == FuseboxClientType::Fusebox; } - void emitExternalTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) { assert( hasFuseboxClientConnected() && "Attempted to emit a trace recording to a non-Fusebox client"); @@ -543,11 +542,11 @@ bool HostAgent::hasFuseboxClientConnected() const { } void HostAgent::emitExternalTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { impl_->emitExternalTracingProfile(std::move(tracingProfile)); } -void HostAgent::emitSystemStateChanged(bool isSingleHost) const { +void HostAgent::emitSystemStateChanged(bool isSingleHost) { impl_->emitSystemStateChanged(isSingleHost); } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h index 8789cb5d32c1..3046a7788c96 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h @@ -77,13 +77,13 @@ class HostAgent final { * Emits the HostTracingProfile that was captured externally, not via the * CDP-initiated request. */ - void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) const; + void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile); /** * Emits a system state changed event when the number of ReactHost instances * changes. */ - void emitSystemStateChanged(bool isSingleHost) const; + void emitSystemStateChanged(bool isSingleHost); private: // We use the private implementation idiom to ensure this class has the same diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp index e34ca99bfad2..0e3cca677f86 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp @@ -111,8 +111,7 @@ class HostTargetSession { return hostAgent_.hasFuseboxClientConnected(); } - void emitHostTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile) { hostAgent_.emitExternalTracingProfile(std::move(tracingProfile)); } @@ -384,7 +383,7 @@ bool HostTarget::hasActiveSessionWithFuseboxClient() const { } void HostTarget::emitTracingProfileForFirstFuseboxClient( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { bool emitted = false; sessions_.forEach([&](HostTargetSession& session) { if (emitted) { diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index aed1396b14e6..b1640f31a473 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -363,7 +363,7 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis * * @see \c hasActiveFrontendSession */ - void emitTracingProfileForFirstFuseboxClient(tracing::HostTracingProfile tracingProfile) const; + void emitTracingProfileForFirstFuseboxClient(tracing::HostTracingProfile tracingProfile); /** * An endpoint for the Host to report frame timings that will be recorded if and only if there is currently an active diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp index 3fc0957c9010..4a876eb1b02d 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp @@ -120,14 +120,14 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) { } void TracingAgent::emitExternalHostTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { frontendChannel_( cdp::jsonNotification("ReactNativeApplication.traceRequested")); emitHostTracingProfile(std::move(tracingProfile)); } void TracingAgent::emitHostTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { auto dataCollectedCallback = [this](folly::dynamic&& eventsChunk) { frontendChannel_( cdp::jsonNotification( diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h index d0f474f1234b..3b2a63259898 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h @@ -44,7 +44,7 @@ class TracingAgent { /** * Emits the HostTracingProfile that was stashed externally by the HostTarget. */ - void emitExternalHostTracingProfile(tracing::HostTracingProfile tracingProfile) const; + void emitExternalHostTracingProfile(tracing::HostTracingProfile tracingProfile); private: /** @@ -60,7 +60,7 @@ class TracingAgent { * Emits captured HostTracingProfile in a series of * Tracing.dataCollected events, followed by a Tracing.tracingComplete event. */ - void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile) const; + void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile); }; } // namespace facebook::react::jsinspector_modern From f791cc0fd680943397cd51bea7a9b17e039e5a6d Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 30 Jan 2026 09:44:01 -0800 Subject: [PATCH 2/4] WeakList: add anyOf + const-propagating forEach (#55313) Summary: Changelog: [Internal] TSIA - minor refactor for convenience and correctness. NOTE: When we get C++23, we can deduplicate the identical const/non-const method bodies using the nifty [deducing `this`](https://en.cppreference.com/w/cpp/language/member_functions.html#Explicit_object_member_functions) feature. Reviewed By: hoxyq Differential Revision: D91140490 --- .../jsinspector-modern/HostTarget.cpp | 4 +- .../ReactCommon/jsinspector-modern/WeakList.h | 61 ++++++++++++++++++- .../jsinspector-modern/tests/WeakListTest.cpp | 32 +++++++++- 3 files changed, 93 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp index 0e3cca677f86..fae9d52605ff 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp @@ -376,7 +376,7 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) { bool HostTarget::hasActiveSessionWithFuseboxClient() const { bool hasActiveFuseboxSession = false; - sessions_.forEach([&](HostTargetSession& session) { + sessions_.forEach([&](auto& session) { hasActiveFuseboxSession |= session.hasFuseboxClient(); }); return hasActiveFuseboxSession; @@ -385,7 +385,7 @@ bool HostTarget::hasActiveSessionWithFuseboxClient() const { void HostTarget::emitTracingProfileForFirstFuseboxClient( tracing::HostTracingProfile tracingProfile) { bool emitted = false; - sessions_.forEach([&](HostTargetSession& session) { + sessions_.forEach([&](auto& session) { if (emitted) { /** * HostTracingProfile object is not copiable for performance reasons, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h b/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h index 00338c344ab5..8b299742449e 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h @@ -30,7 +30,7 @@ class WeakList { * to destroyed elements) will be removed during iteration. */ template - void forEach(Fn &&fn) const + void forEach(Fn &&fn) { for (auto it = ptrs_.begin(); it != ptrs_.end();) { if (auto ptr = it->lock()) { @@ -42,6 +42,65 @@ class WeakList { } } + /** + * Call the given function for every element in the list, ensuring the element + * is not destroyed for the duration of the call. Elements are visited in the + * order they were inserted. + * + * As a side effect, any null pointers in the underlying list (corresponding + * to destroyed elements) will be removed during iteration. + */ + template + void forEach(Fn &&fn) const + { + for (auto it = ptrs_.cbegin(); it != ptrs_.cend();) { + if (auto ptr = it->lock()) { + fn(*ptr); + ++it; + } else { + it = ptrs_.erase(it); + } + } + } + + /** + * Returns true if the given function returns true for any element in the + * list, ensuring the element is not destroyed for the duration of the call. + * + * As a side effect, any null pointers in the underlying list (corresponding + * to destroyed elements) will be removed during iteration. + */ + template + bool anyOf(Fn &&fn) + { + bool found = false; + forEach([&](auto &element) { + if (!found && fn(element)) { + found = true; + } + }); + return found; + } + + /** + * Returns true if the given function returns true for any element in the + * list, ensuring the element is not destroyed for the duration of the call. + * + * As a side effect, any null pointers in the underlying list (corresponding + * to destroyed elements) will be removed during iteration. + */ + template + bool anyOf(Fn &&fn) const + { + bool found = false; + forEach([&](const auto &element) { + if (!found && fn(element)) { + found = true; + } + }); + return found; + } + /** * Returns the number of (non-null) elements in the list. The count will only * remain accurate as long as the list is not modified and elements are diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp index ee86a8e68a94..d228c7cc0ff2 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp @@ -70,7 +70,7 @@ TEST(WeakListTest, ForEach) { EXPECT_THAT(visited, ElementsAre(1, 3)); } -TEST(WeakListTest, ElementsAreAliveDuringCallback) { +TEST(WeakListTest, ElementsAreAliveDuringForEachCallback) { WeakList list; auto p1 = std::make_shared(1); // A separate weak_ptr to observe the lifetime of `p1`. @@ -88,4 +88,34 @@ TEST(WeakListTest, ElementsAreAliveDuringCallback) { EXPECT_THAT(visited, ElementsAre(1)); } +TEST(WeakListTest, AnyOf) { + WeakList list; + auto p1 = std::make_shared(1); + auto p2 = std::make_shared(2); + auto p3 = std::make_shared(3); + list.insert(p1); + list.insert(p2); + list.insert(p3); + + EXPECT_TRUE(list.anyOf([](const int& value) { return value == 2; })); + EXPECT_FALSE(list.anyOf([](const int& value) { return value == 4; })); +} + +TEST(WeakListTest, ElementsAreAliveDuringAnyOfCallback) { + WeakList list; + auto p1 = std::make_shared(42); + // A separate weak_ptr to observe the lifetime of `p1`. + std::weak_ptr wp1 = p1; + list.insert(p1); + + bool result = list.anyOf([&](const int& value) { + p1.reset(); + EXPECT_FALSE(wp1.expired()); + return value == 42; + }); + + EXPECT_TRUE(result); + EXPECT_TRUE(wp1.expired()); +} + } // namespace facebook::react::jsinspector_modern From 25bb5e16e81ea26c20d3868d34a1be4e3985c2d9 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 30 Jan 2026 09:44:01 -0800 Subject: [PATCH 3/4] Stash background trace directly in HostTarget, simplify API (#55347) Summary: Changelog: [Internal] Moves state and logic related to stashing a background trace from `JReactHostInspectorTarget` (JNI wrapper boilerplate, Android-specific) to `HostTarget` (cross-platform C++). Managing the full lifecycle of a background trace inside `HostTarget` lets us remove a fair amount of API surface: * `HostTargetDelegate::unstable_getHostTracingProfileThatWillBeEmittedOnInitialization()` * `HostTarget::hasActiveSessionWithFuseboxClient()` * `TracingAgent::emitExternalHostTracingProfile()` We also refactor some of the surrounding code and add tests (previously missing) for the behaviour of stashed background traces when there is more than one session. Reviewed By: huntie, hoxyq Differential Revision: D91589884 --- .../react/runtime/ReactHostInspectorTarget.kt | 4 +- .../runtime/jni/JReactHostInspectorTarget.cpp | 77 +++------- .../runtime/jni/JReactHostInspectorTarget.h | 25 +--- .../jsinspector-modern/HostAgent.cpp | 12 +- .../jsinspector-modern/HostTarget.cpp | 47 +++--- .../jsinspector-modern/HostTarget.h | 47 +++--- .../jsinspector-modern/tests/TracingTest.cpp | 137 ++++++++++++++++++ 7 files changed, 214 insertions(+), 135 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostInspectorTarget.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostInspectorTarget.kt index f3b521232413..936bcac115ca 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostInspectorTarget.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostInspectorTarget.kt @@ -41,7 +41,7 @@ internal class ReactHostInspectorTarget(reactHostImpl: ReactHostImpl) : external fun stopAndMaybeEmitBackgroundTrace(): Boolean - external fun stopAndDiscardBackgroundTrace() + external fun stopTracing() external override fun getTracingState(): TracingState @@ -65,7 +65,7 @@ internal class ReactHostInspectorTarget(reactHostImpl: ReactHostImpl) : } override fun stopBackgroundTrace() { - stopAndDiscardBackgroundTrace() + stopTracing() } fun handleNativePerfIssueAdded( diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp index 507c0f54470a..4db322c8dab6 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp @@ -73,13 +73,16 @@ JReactHostInspectorTarget::initHybrid( } void JReactHostInspectorTarget::sendDebuggerResumeCommand() { + inspectorTarget().sendCommand(HostCommand::DebuggerResume); +} + +jsinspector_modern::HostTarget& JReactHostInspectorTarget::inspectorTarget() { if (inspectorTarget_) { - inspectorTarget_->sendCommand(HostCommand::DebuggerResume); - } else { - jni::throwNewJavaException( - "java/lang/IllegalStateException", - "Cannot send command while the Fusebox backend is not enabled"); + return *inspectorTarget_; } + jni::throwNewJavaException( + "java/lang/IllegalStateException", + "Inspector method called while the Fusebox backend is not enabled."); } jsinspector_modern::HostTargetMetadata @@ -147,58 +150,22 @@ HostTarget* JReactHostInspectorTarget::getInspectorTarget() { } bool JReactHostInspectorTarget::startBackgroundTrace() { - if (inspectorTarget_) { - return inspectorTarget_->startTracing( - tracing::Mode::Background, - { - tracing::Category::HiddenTimeline, - tracing::Category::RuntimeExecution, - tracing::Category::Timeline, - tracing::Category::UserTiming, - }); - } else { - jni::throwNewJavaException( - "java/lang/IllegalStateException", - "Cannot start Tracing session while the Fusebox backend is not enabled."); - } + return inspectorTarget().startTracing( + tracing::Mode::Background, + { + tracing::Category::HiddenTimeline, + tracing::Category::RuntimeExecution, + tracing::Category::Timeline, + tracing::Category::UserTiming, + }); } -tracing::HostTracingProfile JReactHostInspectorTarget::stopTracing() { - if (inspectorTarget_) { - return inspectorTarget_->stopTracing(); - } else { - jni::throwNewJavaException( - "java/lang/IllegalStateException", - "Cannot stop Tracing session while the Fusebox backend is not enabled."); - } +void JReactHostInspectorTarget::stopTracing() { + inspectorTarget().stopTracing(); } jboolean JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace() { - auto capturedTrace = inspectorTarget_->stopTracing(); - if (inspectorTarget_->hasActiveSessionWithFuseboxClient()) { - inspectorTarget_->emitTracingProfileForFirstFuseboxClient( - std::move(capturedTrace)); - return jboolean(true); - } - - stashTracingProfile(std::move(capturedTrace)); - return jboolean(false); -} - -void JReactHostInspectorTarget::stopAndDiscardBackgroundTrace() { - inspectorTarget_->stopTracing(); -} - -void JReactHostInspectorTarget::stashTracingProfile( - tracing::HostTracingProfile&& hostTracingProfile) { - stashedTracingProfile_ = std::move(hostTracingProfile); -} - -std::optional JReactHostInspectorTarget:: - unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() { - auto tracingProfile = std::move(stashedTracingProfile_); - stashedTracingProfile_.reset(); - return tracingProfile; + return jboolean(inspectorTarget().stopAndMaybeEmitBackgroundTrace()); } void JReactHostInspectorTarget::registerNatives() { @@ -213,9 +180,7 @@ void JReactHostInspectorTarget::registerNatives() { makeNativeMethod( "stopAndMaybeEmitBackgroundTrace", JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace), - makeNativeMethod( - "stopAndDiscardBackgroundTrace", - JReactHostInspectorTarget::stopAndDiscardBackgroundTrace), + makeNativeMethod("stopTracing", JReactHostInspectorTarget::stopTracing), makeNativeMethod( "getTracingState", JReactHostInspectorTarget::getTracingState), makeNativeMethod( @@ -256,7 +221,7 @@ HostTargetTracingDelegate* JReactHostInspectorTarget::getTracingDelegate() { void JReactHostInspectorTarget::recordFrameTimings( jni::alias_ref frameTimingSequence) { - inspectorTarget_->recordFrameTimings({ + inspectorTarget().recordFrameTimings({ frameTimingSequence->getId(), frameTimingSequence->getThreadId(), frameTimingSequence->getBeginDrawingTimestamp(), diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h index 9e7c6fc67aed..032c96f32ee3 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h @@ -240,7 +240,7 @@ class JReactHostInspectorTarget : public jni::HybridClass executor) override; - std::optional - unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() override; jsinspector_modern::HostTargetTracingDelegate *getTracingDelegate() override; private: @@ -286,6 +284,13 @@ class JReactHostInspectorTarget : public jni::HybridClass jobj, jni::alias_ref reactHostImpl, jni::alias_ref javaExecutor); + + /** + * Returns a reference to the HostTarget, throwing a Java IllegalStateException + * if the Fusebox backend is not enabled (i.e., inspectorTarget_ is null). + */ + jsinspector_modern::HostTarget &inspectorTarget(); + jni::global_ref jobj_; // This weak reference breaks the cycle between the C++ HostTarget and the // Java ReactHostImpl, preventing memory leaks in apps that create multiple @@ -296,20 +301,6 @@ class JReactHostInspectorTarget : public jni::HybridClass inspectorTarget_; std::optional inspectorPageId_; - /** - * Stops previously started trace recording and returns the captured HostTracingProfile. - */ - jsinspector_modern::tracing::HostTracingProfile stopTracing(); - /** - * Stashes previously recorded HostTracingProfile that will be emitted when - * CDP session is created. Once emitted, the value will be cleared from this - * instance. - */ - void stashTracingProfile(jsinspector_modern::tracing::HostTracingProfile &&hostTracingProfile); - /** - * Previously recorded HostTracingProfile that will be emitted when CDP session is created. - */ - std::optional stashedTracingProfile_; /** * Encapsulates the logic around tracing for this HostInspectorTarget. */ diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp index 888d27936e90..172ff290ff79 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp @@ -236,13 +236,11 @@ class HostAgent::Impl final { emitSystemStateChanged(isSingleHost); } - auto stashedTraceRecording = - targetController_.getDelegate() - .unstable_getHostTracingProfileThatWillBeEmittedOnInitialization(); - if (stashedTraceRecording.has_value()) { - tracingAgent_.emitExternalHostTracingProfile( - std::move(stashedTraceRecording.value())); - } + auto emitted = targetController_.maybeEmitStashedBackgroundTrace(); + assert( + emitted && + "Expected to find at least one session eligible to receive a background trace after ReactNativeApplication.enable"); + (void)emitted; return { .isFinishedHandlingRequest = true, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp index fae9d52605ff..7934de96289a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp @@ -102,17 +102,8 @@ class HostTargetSession { } } - /** - * Returns whether the ReactNativeApplication CDP domain is enabled. - * - * Chrome DevTools Frontend enables this domain as a client. - */ - bool hasFuseboxClient() const { - return hostAgent_.hasFuseboxClientConnected(); - } - - void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile) { - hostAgent_.emitExternalTracingProfile(std::move(tracingProfile)); + HostAgent& agent() { + return hostAgent_; } private: @@ -323,6 +314,10 @@ bool HostTargetController::decrementPauseOverlayCounter() { return --pauseOverlayCounter_ != 0; } +bool HostTargetController::maybeEmitStashedBackgroundTrace() { + return target_.maybeEmitStashedBackgroundTrace(); +} + namespace { struct StaticHostTargetMetadata { @@ -374,32 +369,26 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) { return result; } -bool HostTarget::hasActiveSessionWithFuseboxClient() const { - bool hasActiveFuseboxSession = false; - sessions_.forEach([&](auto& session) { - hasActiveFuseboxSession |= session.hasFuseboxClient(); - }); - return hasActiveFuseboxSession; -} - -void HostTarget::emitTracingProfileForFirstFuseboxClient( - tracing::HostTracingProfile tracingProfile) { +bool HostTarget::maybeEmitStashedBackgroundTrace() { bool emitted = false; sessions_.forEach([&](auto& session) { if (emitted) { - /** - * HostTracingProfile object is not copiable for performance reasons, - * because it could contain large Runtime sampling profile object. - * - * This approach would not work with multi-client debugger setup. - */ return; } - if (session.hasFuseboxClient()) { - session.emitHostTracingProfile(std::move(tracingProfile)); + if (session.agent().hasFuseboxClientConnected()) { + auto stashedTrace = std::exchange(stashedTracingProfile_, std::nullopt); + if (stashedTrace) { + session.agent().emitExternalTracingProfile(std::move(*stashedTrace)); + } emitted = true; } }); + return emitted; +} + +bool HostTarget::stopAndMaybeEmitBackgroundTrace() { + stashedTracingProfile_ = stopTracing(); + return maybeEmitStashedBackgroundTrace(); } } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index b1640f31a473..c4ee8bb1e6d5 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -181,19 +181,6 @@ class HostTargetDelegate : public LoadNetworkResourceDelegate { "LoadNetworkResourceDelegate.loadNetworkResource is not implemented by this host target delegate."); } - /** - * [Experimental] Will be called at the CDP session initialization to get the - * trace recording that may have been stashed by the Host from the previous - * background session. - * - * \return the HostTracingProfile if there is one that needs to be - * displayed, otherwise std::nullopt. - */ - virtual std::optional unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() - { - return std::nullopt; - } - /** * An optional delegate that will be used by HostTarget to notify about tracing-related events. */ @@ -256,6 +243,12 @@ class HostTargetController final { */ tracing::HostTracingProfile stopTracing(); + /** + * If there is a stashed background trace, emit it to the first eligible session. + * \return true if an eligible session is found (even if there was no stashed background trace). + */ + bool maybeEmitStashedBackgroundTrace(); + private: HostTarget &target_; size_t pauseOverlayCounter_{0}; @@ -352,18 +345,13 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis tracing::HostTracingProfile stopTracing(); /** - * Returns whether there is an active session with the Fusebox client, i.e. - * with Chrome DevTools Frontend fork for React Native. + * Stops previously started trace recording and: + * - If there is an active CDP session with ReactNativeApplication domain + * enabled, emits the trace and returns true. + * - Otherwise, stashes the captured trace, that will be emitted when a CDP + * session enables ReactNativeApplication. Returns false. */ - bool hasActiveSessionWithFuseboxClient() const; - - /** - * Emits the HostTracingProfile for the first active session with the Fusebox - * client. - * - * @see \c hasActiveFrontendSession - */ - void emitTracingProfileForFirstFuseboxClient(tracing::HostTracingProfile tracingProfile); + bool stopAndMaybeEmitBackgroundTrace(); /** * An endpoint for the Host to report frame timings that will be recorded if and only if there is currently an active @@ -403,6 +391,11 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis * Should only be allocated when there is an active tracing session. */ std::unique_ptr traceRecording_{nullptr}; + /** + * Previously recorded HostTracingProfile that will be emitted when CDP session is created + * and enables ReactNativeApplication. Once emitted, the value will be cleared. + */ + std::optional stashedTracingProfile_; /** * Protects the state inside traceRecording_. * @@ -429,6 +422,12 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis */ void installPerfIssuesBinding(); + /** + * If there is a stashed background trace, emit it to the first eligible session. + * \return true if an eligible session is found (even if there was no stashed background trace). + */ + bool maybeEmitStashedBackgroundTrace(); + // Necessary to allow HostAgent to access HostTarget's internals in a // controlled way (i.e. only HostTargetController gets friend access, while // HostAgent itself doesn't). diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp index b04290604c9a..f21d61f06583 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp @@ -185,4 +185,141 @@ TEST_F(TracingTest, BackgroundTracingIsRejectedWhileCDPTracingIsRunning) { page_->stopTracing(); } +TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) { + auto secondaryFusebox = this->connectSecondary(); + auto secondaryNonFusebox = this->connectSecondary(); + + // Enable ReactNativeApplication domain on primary and secondaryFusebox + // sessions (but NOT on secondaryNonFusebox) + { + InSequence s; + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.metadataUpdated")))); + EXPECT_CALL( + this->fromPage(), onMessage(JsonEq(R"({"id": 1, "result": {}})"))); + } + this->toPage_->sendMessage( + R"({"id": 1, "method": "ReactNativeApplication.enable"})"); + + { + InSequence s; + EXPECT_CALL( + secondaryFusebox.fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.metadataUpdated")))); + EXPECT_CALL( + secondaryFusebox.fromPage(), + onMessage(JsonEq(R"({"id": 1, "result": {}})"))); + } + secondaryFusebox.toPage().sendMessage( + R"({"id": 1, "method": "ReactNativeApplication.enable"})"); + + // Start background tracing + this->page_->startTracing( + tracing::Mode::Background, {tracing::Category::Timeline}); + + // Record some frame timings + auto now = HighResTimeStamp::now(); + this->page_->recordFrameTimings( + tracing::FrameTimingSequence( + 1, // id + 11, // threadId + now, + now + HighResDuration::fromNanoseconds(10), + now + HighResDuration::fromNanoseconds(50))); + + // Primary session should receive the trace (first eligible session). + // Events within a session are ordered. + Sequence primarySeq; + + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.traceRequested")))) + .InSequence(primarySeq); + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.dataCollected")))) + .Times(AtLeast(1)) + .InSequence(primarySeq); + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete")))) + .InSequence(primarySeq); + + // secondaryFusebox and secondaryNonFusebox should NOT receive anything + // (only first eligible session receives the trace) + EXPECT_CALL(secondaryFusebox.fromPage(), onMessage(_)).Times(0); + EXPECT_CALL(secondaryNonFusebox.fromPage(), onMessage(_)).Times(0); + + // Stop tracing and emit to first eligible session + EXPECT_TRUE(this->page_->stopAndMaybeEmitBackgroundTrace()); +} + +TEST_F(TracingTest, StashedTraceIsEmittedOnlyToFirstEligibleSession) { + // Start background tracing with no sessions having ReactNativeApplication + // enabled + this->page_->startTracing( + tracing::Mode::Background, {tracing::Category::Timeline}); + + // Record some frame timings + auto now = HighResTimeStamp::now(); + this->page_->recordFrameTimings( + tracing::FrameTimingSequence( + 1, // id + 11, // threadId + now, + now + HighResDuration::fromNanoseconds(10), + now + HighResDuration::fromNanoseconds(50))); + + // Stop tracing - no eligible sessions exist, so the trace is stashed + EXPECT_FALSE(this->page_->stopAndMaybeEmitBackgroundTrace()); + + // Now the primary session enables ReactNativeApplication - it should receive + // the stashed trace. Events within a session are ordered. + Sequence primarySeq; + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.metadataUpdated")))) + .InSequence(primarySeq); + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.traceRequested")))) + .InSequence(primarySeq); + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.dataCollected")))) + .Times(AtLeast(1)) + .InSequence(primarySeq); + EXPECT_CALL( + this->fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete")))) + .InSequence(primarySeq); + EXPECT_CALL(this->fromPage(), onMessage(JsonEq(R"({"id": 1, "result": {}})"))) + .InSequence(primarySeq); + this->toPage_->sendMessage( + R"({"id": 1, "method": "ReactNativeApplication.enable"})"); + + // Connect a secondary session and enable ReactNativeApplication - it should + // NOT receive the already-emitted stashed trace + auto secondary = this->connectSecondary(); + Sequence secondarySeq; + EXPECT_CALL( + secondary.fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.metadataUpdated")))) + .InSequence(secondarySeq); + // No traceRequested, dataCollected, or tracingComplete expected for + // secondary + EXPECT_CALL( + secondary.fromPage(), onMessage(JsonEq(R"({"id": 1, "result": {}})"))) + .InSequence(secondarySeq); + secondary.toPage().sendMessage( + R"({"id": 1, "method": "ReactNativeApplication.enable"})"); +} + } // namespace facebook::react::jsinspector_modern From 4382a95833e28ab0db085fd38a17754f84ae76b6 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 30 Jan 2026 09:44:01 -0800 Subject: [PATCH 4/4] Stream background traces to multiple RNDT sessions if present (#55351) Summary: Changelog: [Internal] ## Behaviour Fixes an edge case in the way background traces are emitted when multiple RNDT frontends (or other clients with the `ReactNativeApplication` domain enabled) are connected to a single Host. Previously, only one of the clients would receive the CDP events for the trace. With this diff, *all* of them will. NOTE: This leaves another, even smaller edge case / inconsistency: if there are *no* RNDT frontends at the time a background trace ends, we still only send it to the first frontend (if any) that connects. This is logically more defensible than picking one client out of multiple active connections ( = what's fixed in this diff). ## Implementation Tracing functionality is somewhat awkwardly split between `HostTarget` (background traces) and `TracingAgent` (CDP-initiated traces). This diff doesn't attempt to fully clean this up[1], but we do reduce duplication and boilerplate by creating a helper function used by both. This function encapsulates the gnarly/surprising details of dealing with `tracing::HostTracingProfile` objects in a memory-efficient way: 1. As per the existing implementation, we serialise them to JSON chunk-by-chunk, destroying the original object in the process. 2. Each chunk is sent to all relevant sessions *and destroyed* before we begin serialising the next one. [1] In future work, we should probably move the Host's tracing state and logic into its own `TracingTarget` class. The naturally close coupling of a Target with its corresponding Agent would be helpful here. Reviewed By: huntie Differential Revision: D90888852 --- .../jsinspector-modern/HostAgent.cpp | 27 ++---- .../jsinspector-modern/HostAgent.h | 13 +-- .../jsinspector-modern/HostTarget.cpp | 36 +++++--- .../jsinspector-modern/HostTarget.h | 2 +- .../jsinspector-modern/HostTargetTracing.h | 89 +++++++++++++++++++ .../jsinspector-modern/TracingAgent.cpp | 50 ++--------- .../jsinspector-modern/TracingAgent.h | 11 --- .../jsinspector-modern/tests/TracingTest.cpp | 30 +++++-- 8 files changed, 150 insertions(+), 108 deletions(-) create mode 100644 packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.h diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp index 172ff290ff79..8fd200024d3a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp @@ -379,15 +379,8 @@ class HostAgent::Impl final { } } - bool hasFuseboxClientConnected() const { - return fuseboxClientType_ == FuseboxClientType::Fusebox; - } - - void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) { - assert( - hasFuseboxClientConnected() && - "Attempted to emit a trace recording to a non-Fusebox client"); - tracingAgent_.emitExternalHostTracingProfile(std::move(tracingProfile)); + bool isEligibleForBackgroundTrace() const { + return sessionState_.isReactNativeApplicationDomainEnabled; } void emitSystemStateChanged(bool isSingleHost) { @@ -500,10 +493,9 @@ class HostAgent::Impl final { void handleRequest(const cdp::PreparsedRequest& req) {} void setCurrentInstanceAgent(std::shared_ptr agent) {} - bool hasFuseboxClientConnected() const { + bool isEligibleForBackgroundTrace() const { return false; } - void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) {} void emitSystemStateChanged(bool isSingleHost) {} }; @@ -535,17 +527,8 @@ void HostAgent::setCurrentInstanceAgent( impl_->setCurrentInstanceAgent(std::move(instanceAgent)); } -bool HostAgent::hasFuseboxClientConnected() const { - return impl_->hasFuseboxClientConnected(); -} - -void HostAgent::emitExternalTracingProfile( - tracing::HostTracingProfile tracingProfile) { - impl_->emitExternalTracingProfile(std::move(tracingProfile)); -} - -void HostAgent::emitSystemStateChanged(bool isSingleHost) { - impl_->emitSystemStateChanged(isSingleHost); +bool HostAgent::isEligibleForBackgroundTrace() const { + return impl_->isEligibleForBackgroundTrace(); } #pragma mark - Tracing diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h index 3046a7788c96..083632667f14 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h @@ -67,17 +67,10 @@ class HostAgent final { void setCurrentInstanceAgent(std::shared_ptr agent); /** - * Returns whether this HostAgent is part of the session that has an active - * Fusebox client connecte, i.e. with Chrome DevTools Frontend fork for React - * Native. + * Returns whether this HostAgent is eligible to receive notifications about + * background traces. */ - bool hasFuseboxClientConnected() const; - - /** - * Emits the HostTracingProfile that was captured externally, not via the - * CDP-initiated request. - */ - void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile); + bool isEligibleForBackgroundTrace() const; /** * Emits a system state changed event when the number of ReactHost instances diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp index 7934de96289a..3d61df29b8c2 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp @@ -8,6 +8,7 @@ #include "HostTarget.h" #include "HostAgent.h" #include "HostTargetTraceRecording.h" +#include "HostTargetTracing.h" #include "InspectorInterfaces.h" #include "InspectorUtilities.h" #include "InstanceTarget.h" @@ -106,6 +107,10 @@ class HostTargetSession { return hostAgent_; } + FrontendChannel dangerouslyGetFrontendChannel() { + return frontendChannel_; + } + private: // Owned by this instance, but shared (weakly) with the frontend channel std::shared_ptr remote_; @@ -370,20 +375,27 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) { } bool HostTarget::maybeEmitStashedBackgroundTrace() { - bool emitted = false; - sessions_.forEach([&](auto& session) { - if (emitted) { - return; - } - if (session.agent().hasFuseboxClientConnected()) { - auto stashedTrace = std::exchange(stashedTracingProfile_, std::nullopt); - if (stashedTrace) { - session.agent().emitExternalTracingProfile(std::move(*stashedTrace)); - } - emitted = true; + std::vector eligibleFrontendChannels; + eligibleFrontendChannels.reserve(sessions_.size()); + sessions_.forEach([&eligibleFrontendChannels](auto& session) { + if (session.agent().isEligibleForBackgroundTrace()) { + eligibleFrontendChannels.push_back( + session.dangerouslyGetFrontendChannel()); } }); - return emitted; + + if (eligibleFrontendChannels.empty()) { + return false; + } + + auto stashedTrace = std::exchange(stashedTracingProfile_, std::nullopt); + if (stashedTrace) { + emitNotificationsForTracingProfile( + std::move(*stashedTrace), + eligibleFrontendChannels, + /* isBackgroundTrace */ true); + } + return true; } bool HostTarget::stopAndMaybeEmitBackgroundTrace() { diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index c4ee8bb1e6d5..c7b8281bb89d 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -244,7 +244,7 @@ class HostTargetController final { tracing::HostTracingProfile stopTracing(); /** - * If there is a stashed background trace, emit it to the first eligible session. + * If there is a stashed background trace, emit it to all eligible sessions. * \return true if an eligible session is found (even if there was no stashed background trace). */ bool maybeEmitStashedBackgroundTrace(); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.h new file mode 100644 index 000000000000..e5a676d5195a --- /dev/null +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.h @@ -0,0 +1,89 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include "InspectorInterfaces.h" + +#include "cdp/CdpJson.h" +#include "tracing/HostTracingProfile.h" +#include "tracing/HostTracingProfileSerializer.h" + +#include +#include +#include +#include + +namespace facebook::react::jsinspector_modern { + +/** + * Emits a captured HostTracingProfile in a series of + * Tracing.dataCollected events, followed by a Tracing.tracingComplete event, to zero or more + * FrontendChannels. If \p isBackgroundTrace is true, a ReactNativeApplication.traceRequested + * notification is sent to each FrontendChannel before the trace events are emitted. + */ +template +void emitNotificationsForTracingProfile( + tracing::HostTracingProfile &&hostTracingProfile, + const ChannelsRange &channels, + bool isBackgroundTrace) + requires std::ranges::range && + std::convertible_to, FrontendChannel> +{ + /** + * Threshold for the size Trace Event chunk, that will be flushed out with + * Tracing.dataCollected event. + */ + static constexpr uint16_t TRACE_EVENT_CHUNK_SIZE = 1000; + + /** + * The maximum number of ProfileChunk trace events + * that will be sent in a single CDP Tracing.dataCollected message. + */ + static constexpr uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 10; + + if (std::ranges::empty(channels)) { + return; + } + + if (isBackgroundTrace) { + for (auto &frontendChannel : channels) { + frontendChannel(cdp::jsonNotification("ReactNativeApplication.traceRequested")); + } + } + + // Serialize each chunk once and send it to all eligible sessions. + tracing::HostTracingProfileSerializer::emitAsDataCollectedChunks( + std::move(hostTracingProfile), + [&](folly::dynamic &&serializedChunk) { + for (auto &frontendChannel : channels) { + frontendChannel( + cdp::jsonNotification("Tracing.dataCollected", folly::dynamic::object("value", serializedChunk))); + } + }, + TRACE_EVENT_CHUNK_SIZE, + PROFILE_TRACE_EVENT_CHUNK_SIZE); + + for (auto &frontendChannel : channels) { + frontendChannel( + cdp::jsonNotification("Tracing.tracingComplete", folly::dynamic::object("dataLossOccurred", false))); + } +} + +/** + * Convenience overload of emitNotificationsForTracingProfile() for a single FrontendChannel. + */ +inline void emitNotificationsForTracingProfile( + tracing::HostTracingProfile &&hostTracingProfile, + const FrontendChannel &channel, + bool isBackgroundTrace) +{ + std::array channels{channel}; + emitNotificationsForTracingProfile(std::move(hostTracingProfile), channels, isBackgroundTrace); +} + +} // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp index 4a876eb1b02d..d20203d0e884 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp @@ -6,8 +6,8 @@ */ #include "TracingAgent.h" +#include "HostTargetTracing.h" -#include #include #include #include @@ -15,22 +15,6 @@ namespace facebook::react::jsinspector_modern { -namespace { - -/** - * Threshold for the size Trace Event chunk, that will be flushed out with - * Tracing.dataCollected event. - */ -const uint16_t TRACE_EVENT_CHUNK_SIZE = 1000; - -/** - * The maximum number of ProfileChunk trace events - * that will be sent in a single CDP Tracing.dataCollected message. - */ -const uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 10; - -} // namespace - TracingAgent::TracingAgent( FrontendChannel frontendChannel, SessionState& sessionState, @@ -112,38 +96,14 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) { // Send response to Tracing.end request. frontendChannel_(cdp::jsonResult(req.id)); - emitHostTracingProfile(std::move(tracingProfile)); + emitNotificationsForTracingProfile( + std::move(tracingProfile), + frontendChannel_, + /* isBackgroundTrace */ false); return true; } return false; } -void TracingAgent::emitExternalHostTracingProfile( - tracing::HostTracingProfile tracingProfile) { - frontendChannel_( - cdp::jsonNotification("ReactNativeApplication.traceRequested")); - emitHostTracingProfile(std::move(tracingProfile)); -} - -void TracingAgent::emitHostTracingProfile( - tracing::HostTracingProfile tracingProfile) { - auto dataCollectedCallback = [this](folly::dynamic&& eventsChunk) { - frontendChannel_( - cdp::jsonNotification( - "Tracing.dataCollected", - folly::dynamic::object("value", std::move(eventsChunk)))); - }; - tracing::HostTracingProfileSerializer::emitAsDataCollectedChunks( - std::move(tracingProfile), - dataCollectedCallback, - TRACE_EVENT_CHUNK_SIZE, - PROFILE_TRACE_EVENT_CHUNK_SIZE); - - frontendChannel_( - cdp::jsonNotification( - "Tracing.tracingComplete", - folly::dynamic::object("dataLossOccurred", false))); -} - } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h index 3b2a63259898..2fe4c8ad6669 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h @@ -41,11 +41,6 @@ class TracingAgent { */ bool handleRequest(const cdp::PreparsedRequest &req); - /** - * Emits the HostTracingProfile that was stashed externally by the HostTarget. - */ - void emitExternalHostTracingProfile(tracing::HostTracingProfile tracingProfile); - private: /** * A channel used to send responses and events to the frontend. @@ -55,12 +50,6 @@ class TracingAgent { SessionState &sessionState_; HostTargetController &hostTargetController_; - - /** - * Emits captured HostTracingProfile in a series of - * Tracing.dataCollected events, followed by a Tracing.tracingComplete event. - */ - void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile); }; } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp index f21d61f06583..75805b6a3473 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp @@ -185,7 +185,7 @@ TEST_F(TracingTest, BackgroundTracingIsRejectedWhileCDPTracingIsRunning) { page_->stopTracing(); } -TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) { +TEST_F(TracingTest, EmitsToAllSessionsWithReactNativeApplicationDomainEnabled) { auto secondaryFusebox = this->connectSecondary(); auto secondaryNonFusebox = this->connectSecondary(); @@ -230,9 +230,11 @@ TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) { now + HighResDuration::fromNanoseconds(10), now + HighResDuration::fromNanoseconds(50))); - // Primary session should receive the trace (first eligible session). - // Events within a session are ordered. + // Primary and secondaryFusebox sessions should receive the trace. + // Events within each session are ordered, but order between sessions is + // arbitrary. Sequence primarySeq; + Sequence secondarySeq; EXPECT_CALL( this->fromPage(), @@ -249,12 +251,26 @@ TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) { onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete")))) .InSequence(primarySeq); - // secondaryFusebox and secondaryNonFusebox should NOT receive anything - // (only first eligible session receives the trace) - EXPECT_CALL(secondaryFusebox.fromPage(), onMessage(_)).Times(0); + EXPECT_CALL( + secondaryFusebox.fromPage(), + onMessage(JsonParsed( + AtJsonPtr("/method", "ReactNativeApplication.traceRequested")))) + .InSequence(secondarySeq); + EXPECT_CALL( + secondaryFusebox.fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.dataCollected")))) + .Times(AtLeast(1)) + .InSequence(secondarySeq); + EXPECT_CALL( + secondaryFusebox.fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete")))) + .InSequence(secondarySeq); + + // secondaryNonFusebox should NOT receive anything (it did not enable the + // domain) EXPECT_CALL(secondaryNonFusebox.fromPage(), onMessage(_)).Times(0); - // Stop tracing and emit to first eligible session + // Stop tracing and emit to all eligible sessions EXPECT_TRUE(this->page_->stopAndMaybeEmitBackgroundTrace()); }