From 3a833d3f2f06d290286ee36ba6ef0745ad1f9fa0 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Thu, 24 Jul 2025 13:24:12 -0700 Subject: [PATCH] Move all Tracing method handling to TracingAgent (#52814) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/52814 Changelog: [Internal] The handling of `Tracing.start` was needlessly split between `HostAgent` and `TracingAgent` because `TracingAgent` did not have a reference to the session state (which, being an Agent, it's allowed to have). This diff cleans that up. Reviewed By: huntie Differential Revision: D78799899 fbshipit-source-id: b05e6dae2e9b287b8708debe756b19f81d5dae06 --- .../jsinspector-modern/HostAgent.cpp | 22 +------------------ .../jsinspector-modern/TracingAgent.cpp | 14 ++++++++++++ .../jsinspector-modern/TracingAgent.h | 7 ++++-- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp index a81d7b17667..b16d89395ae 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp @@ -47,7 +47,7 @@ class HostAgent::Impl final { hostMetadata_(std::move(hostMetadata)), sessionState_(sessionState), networkIOAgent_(NetworkIOAgent(frontendChannel, std::move(executor))), - tracingAgent_(TracingAgent(frontendChannel)) {} + tracingAgent_(TracingAgent(frontendChannel, sessionState)) {} ~Impl() { if (isPausedInDebuggerOverlayVisible_) { @@ -207,26 +207,6 @@ class HostAgent::Impl final { .shouldSendOKResponse = true, }; } - if (req.method == "Tracing.start") { - if (sessionState_.isDebuggerDomainEnabled) { - frontendChannel_(cdp::jsonError( - req.id, - cdp::ErrorCode::InternalError, - "Debugger domain is expected to be disabled before starting Tracing")); - - return { - .isFinishedHandlingRequest = true, - .shouldSendOKResponse = false, - }; - } - - // We delegate handling of this request to TracingAgent. If not handled, - // then something unexpected happened - don't send an OK response. - return { - .isFinishedHandlingRequest = false, - .shouldSendOKResponse = false, - }; - } return { .isFinishedHandlingRequest = false, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp index 0a3e365fd3d..d76e7632c77 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp @@ -31,9 +31,23 @@ const uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 1; } // namespace +TracingAgent::TracingAgent( + FrontendChannel frontendChannel, + const SessionState& sessionState) + : frontendChannel_(std::move(frontendChannel)), + sessionState_(sessionState) {} + bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) { if (req.method == "Tracing.start") { // @cdp Tracing.start support is experimental. + if (sessionState_.isDebuggerDomainEnabled) { + frontendChannel_(cdp::jsonError( + req.id, + cdp::ErrorCode::InternalError, + "Debugger domain is expected to be disabled before starting Tracing")); + + return true; + } if (!instanceAgent_) { frontendChannel_(cdp::jsonError( req.id, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h index 95ed57efc45..f58e01f2683 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h @@ -25,8 +25,9 @@ class TracingAgent { * \param frontendChannel A channel used to send responses to the * frontend. */ - explicit TracingAgent(FrontendChannel frontendChannel) - : frontendChannel_(std::move(frontendChannel)) {} + TracingAgent( + FrontendChannel frontendChannel, + const SessionState& sessionState); /** * Handle a CDP request. The response will be sent over the provided @@ -60,6 +61,8 @@ class TracingAgent { * in this trace. */ HighResTimeStamp instanceTracingStartTimestamp_; + + const SessionState& sessionState_; }; } // namespace facebook::react::jsinspector_modern