From 04eadf6a6c75ea71b71fc2cc6b702408b4541d9c Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 14 Feb 2024 04:19:35 -0800 Subject: [PATCH] RELAND [RN][CDP] [3/n] RuntimeTarget refactor - RuntimeAgent --> RuntimeAgentDelegate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: This is a resubmission of D53266707 with a fix in the OSS version of `HermesExecutorFactory` (it was incorrectly referencing `HermesRuntimeAgent.h` which doesn't exist anymore). The original diff summary follows. --- Changelog: [Internal] I'm refactoring the way the Runtime concept works in the modern CDP backend to bring it in line with the Page/Instance concepts. Overall, this will let us: * Integrate with engines that require us to instantiate a shared Target-like object (e.g. Hermes AsyncDebuggingAPI) in addition to an per-session Agent-like object. * Access JSI in a CDP context (both at target setup/teardown time and during a CDP session) to implement our own engine-agnostic functionality (`console` interception, `Runtime.addBinding`, etc). * Manage CDP execution contexts natively in RN, and (down the line) enable first-class debugging support for multiple Runtimes in an Instance. The core diffs in this stack will: * ~~Introduce a `RuntimeTarget` class similar to `{Page,Instance}Target`.~~ (D53233914) * ~~Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`).~~ (D53233914) * Rename the existing `RuntimeAgent` interface to `RuntimeAgentDelegate`. *← This diff* * Create a new concrete `RuntimeAgent` class similar to `{Page,Instance}Agent`. *← Also in this diff* * Provide `RuntimeTarget` and `RuntimeAgent` with primitives for safe JSI access, namely a `RuntimeExecutor` for scheduling work on the JS thread. * We'll likely develop a similar mechanism for scheduling work on the "main" thread from the JS thread, for when we need to do more than just send a CDP message (which we can already do with the thread-safe `FrontendChannel`) in response to a JS event. ## Architecture diagrams Before this stack: https://pxl.cl/4h7m0 After this stack: https://pxl.cl/4h7m7 Reviewed By: EdmondChuiHW Differential Revision: D53748590 fbshipit-source-id: bd0cf9f74b95abc52b4903f8a7afddcefa303d8a --- .../ReactCommon/cxxreact/JSExecutor.cpp | 5 +- .../ReactCommon/cxxreact/JSExecutor.h | 7 +- .../hermes/executor/HermesExecutorFactory.cpp | 9 +-- .../hermes/executor/HermesExecutorFactory.h | 3 +- ...ent.cpp => HermesRuntimeAgentDelegate.cpp} | 32 +++++---- ...meAgent.h => HermesRuntimeAgentDelegate.h} | 8 +-- .../jsinspector-modern/CONCEPTS.md | 4 ++ ...t.cpp => FallbackRuntimeAgentDelegate.cpp} | 11 +-- ...Agent.h => FallbackRuntimeAgentDelegate.h} | 8 +-- .../jsinspector-modern/InstanceAgent.h | 2 +- .../jsinspector-modern/InstanceTarget.h | 2 +- .../jsinspector-modern/PageAgent.h | 2 +- .../jsinspector-modern/PageTarget.h | 4 +- .../ReactCommon/jsinspector-modern/ReactCdp.h | 2 +- .../jsinspector-modern/RuntimeAgent.cpp | 22 +++++- .../jsinspector-modern/RuntimeAgent.h | 50 +++++++++++--- .../jsinspector-modern/RuntimeAgentDelegate.h | 35 ++++++++++ .../jsinspector-modern/RuntimeTarget.cpp | 6 +- .../jsinspector-modern/RuntimeTarget.h | 7 +- .../jsinspector-modern/tests/InspectorMocks.h | 10 +-- .../tests/PageTargetTest.cpp | 68 +++++++++---------- .../react/runtime/JSRuntimeFactory.cpp | 5 +- .../react/runtime/JSRuntimeFactory.h | 2 +- .../react/runtime/hermes/HermesInstance.cpp | 8 +-- 24 files changed, 207 insertions(+), 105 deletions(-) rename packages/react-native/ReactCommon/hermes/inspector-modern/chrome/{HermesRuntimeAgent.cpp => HermesRuntimeAgentDelegate.cpp} (85%) rename packages/react-native/ReactCommon/hermes/inspector-modern/chrome/{HermesRuntimeAgent.h => HermesRuntimeAgentDelegate.h} (89%) rename packages/react-native/ReactCommon/jsinspector-modern/{FallbackRuntimeAgent.cpp => FallbackRuntimeAgentDelegate.cpp} (84%) rename packages/react-native/ReactCommon/jsinspector-modern/{FallbackRuntimeAgent.h => FallbackRuntimeAgentDelegate.h} (89%) create mode 100644 packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgentDelegate.h diff --git a/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp b/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp index b760e7356f5..ba34e3f8fa2 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp +++ b/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp @@ -35,10 +35,11 @@ double JSExecutor::performanceNow() { return duration / NANOSECONDS_IN_MILLISECOND; } -std::unique_ptr JSExecutor::createAgent( +std::unique_ptr +JSExecutor::createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { - return std::make_unique( + return std::make_unique( std::move(frontendChannel), sessionState, getDescription()); } diff --git a/packages/react-native/ReactCommon/cxxreact/JSExecutor.h b/packages/react-native/ReactCommon/cxxreact/JSExecutor.h index cd66d0d43aa..0b7041e1375 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSExecutor.h +++ b/packages/react-native/ReactCommon/cxxreact/JSExecutor.h @@ -114,7 +114,7 @@ class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate { /** * Returns whether or not the underlying executor supports debugging via the * Chrome remote debugging protocol. If true, the executor should also - * override the \c createAgent method. + * override the \c createAgentDelegate method. */ virtual bool isInspectable() { return false; @@ -141,9 +141,10 @@ class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate { static double performanceNow(); /** - * Create a RuntimeAgent that can be used to debug the JS VM instance. + * Create a RuntimeAgentDelegate that can be used to debug the JS VM instance. */ - virtual std::unique_ptr createAgent( + virtual std::unique_ptr + createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override; }; diff --git a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp index 67fa9138ff9..52c2ece8a5d 100644 --- a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp +++ b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp @@ -15,7 +15,7 @@ #include #include -#include +#include #include #include @@ -257,12 +257,13 @@ HermesExecutor::HermesExecutor( runtime_(runtime), hermesRuntime_(hermesRuntime) {} -std::unique_ptr HermesExecutor::createAgent( +std::unique_ptr +HermesExecutor::createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { std::shared_ptr hermesRuntimeShared(runtime_, &hermesRuntime_); - return std::unique_ptr( - new jsinspector_modern::HermesRuntimeAgent( + return std::unique_ptr( + new jsinspector_modern::HermesRuntimeAgentDelegate( frontendChannel, sessionState, hermesRuntimeShared, diff --git a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h index 9cc96759792..4ae521539d4 100644 --- a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h +++ b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h @@ -54,7 +54,8 @@ class HermesExecutor : public JSIExecutor { RuntimeInstaller runtimeInstaller, hermes::HermesRuntime& hermesRuntime); - virtual std::unique_ptr createAgent( + virtual std::unique_ptr + createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override; diff --git a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.cpp b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp similarity index 85% rename from packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.cpp rename to packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp index 0430e9dcc86..2a6f089e048 100644 --- a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.cpp +++ b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -#include "HermesRuntimeAgent.h" +#include "HermesRuntimeAgentDelegate.h" // If HERMES_ENABLE_DEBUGGER isn't defined, we can't access any Hermes // CDPHandler headers or types. @@ -14,7 +14,7 @@ #include #include #else // HERMES_ENABLE_DEBUGGER -#include +#include #endif // HERMES_ENABLE_DEBUGGER #include @@ -30,12 +30,12 @@ namespace { /** * An implementation of the Hermes RuntimeAdapter interface (part of - * Hermes's CDPHandler API) for use within a React Native RuntimeAgent. + * Hermes's CDPHandler API) for use within a React Native RuntimeAgentDelegate. */ -class HermesRuntimeAgentAdapter +class HermesRuntimeAgentDelegateAdapter : public hermes::inspector_modern::RuntimeAdapter { public: - HermesRuntimeAgentAdapter( + HermesRuntimeAgentDelegateAdapter( std::shared_ptr runtime, RuntimeExecutor runtimeExecutor) : runtime_(runtime), runtimeExecutor_(runtimeExecutor) {} @@ -60,10 +60,10 @@ class HermesRuntimeAgentAdapter } // namespace /** - * A RuntimeAgent that handles requests from the Chrome DevTools Protocol for - * an instance of Hermes. + * A RuntimeAgentDelegate that handles requests from the Chrome DevTools + * Protocol for an instance of Hermes. */ -class HermesRuntimeAgent::Impl final : public RuntimeAgent { +class HermesRuntimeAgentDelegate::Impl final : public RuntimeAgentDelegate { using HermesCDPHandler = hermes::inspector_modern::chrome::CDPHandler; public: @@ -84,7 +84,7 @@ class HermesRuntimeAgent::Impl final : public RuntimeAgent { std::shared_ptr runtime, RuntimeExecutor runtimeExecutor) : hermes_(HermesCDPHandler::create( - std::make_unique( + std::make_unique( runtime, runtimeExecutor), /* waitForDebugger */ false, @@ -133,17 +133,18 @@ class HermesRuntimeAgent::Impl final : public RuntimeAgent { #else // !HERMES_ENABLE_DEBUGGER /** - * A stub for HermesRuntimeAgent when Hermes is compiled without debugging - * support. + * A stub for HermesRuntimeAgentDelegate when Hermes is compiled without + * debugging support. */ -class HermesRuntimeAgent::Impl final : public FallbackRuntimeAgent { +class HermesRuntimeAgentDelegate::Impl final + : public FallbackRuntimeAgentDelegate { public: Impl( FrontendChannel frontendChannel, SessionState& sessionState, std::shared_ptr runtime, RuntimeExecutor) - : FallbackRuntimeAgent( + : FallbackRuntimeAgentDelegate( std::move(frontendChannel), sessionState, runtime->description()) {} @@ -151,7 +152,7 @@ class HermesRuntimeAgent::Impl final : public FallbackRuntimeAgent { #endif // HERMES_ENABLE_DEBUGGER -HermesRuntimeAgent::HermesRuntimeAgent( +HermesRuntimeAgentDelegate::HermesRuntimeAgentDelegate( FrontendChannel frontendChannel, SessionState& sessionState, std::shared_ptr runtime, @@ -162,7 +163,8 @@ HermesRuntimeAgent::HermesRuntimeAgent( std::move(runtime), std::move(runtimeExecutor))) {} -bool HermesRuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) { +bool HermesRuntimeAgentDelegate::handleRequest( + const cdp::PreparsedRequest& req) { return impl_->handleRequest(req); } diff --git a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.h b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.h similarity index 89% rename from packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.h rename to packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.h index 20323f4329d..5700ab02e6a 100644 --- a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.h +++ b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.h @@ -15,10 +15,10 @@ namespace facebook::react::jsinspector_modern { /** - * A RuntimeAgent that handles requests from the Chrome DevTools Protocol for - * an instance of Hermes. + * A RuntimeAgentDelegate that handles requests from the Chrome DevTools + * Protocol for an instance of Hermes. */ -class HermesRuntimeAgent : public RuntimeAgent { +class HermesRuntimeAgentDelegate : public RuntimeAgentDelegate { public: /** * \param frontendChannel A channel used to send responses and events to the @@ -31,7 +31,7 @@ class HermesRuntimeAgent : public RuntimeAgent { * \c runtimeExecutor may drop scheduled work if the runtime is destroyed * first. */ - HermesRuntimeAgent( + HermesRuntimeAgentDelegate( FrontendChannel frontendChannel, SessionState& sessionState, std::shared_ptr runtime, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/CONCEPTS.md b/packages/react-native/ReactCommon/jsinspector-modern/CONCEPTS.md index b7353957e37..e07d2eeda97 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/CONCEPTS.md +++ b/packages/react-native/ReactCommon/jsinspector-modern/CONCEPTS.md @@ -21,3 +21,7 @@ A single connection between a debugger frontend and a target. There can be multi ### Agent A handler for a subset of CDP messages for a specific target as part of a specific session. + +### Agent Delegate + +An interface between an Agent class and some integration-specific, per-session logic/state it relies on (that does not fit in a Target Delegate). For example, a RuntimeAgentDelegate is used by RuntimeAgent to host Hermes's native CDP handler and delegate messages to it. The interface may look exactly like an Agent (purely CDP messages in/out) or there may be a more involved API to expose state/functionality needed by the Agent. diff --git a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.cpp similarity index 84% rename from packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.cpp rename to packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.cpp index cbb139746f0..75f74e5f285 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.cpp @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include "FallbackRuntimeAgentDelegate.h" #include #include @@ -22,7 +22,7 @@ namespace facebook::react::jsinspector_modern { #define ANSI_STYLE_RESET "\x1B[23m" #define ANSI_COLOR_BG_YELLOW "\x1B[48;2;253;247;231m" -FallbackRuntimeAgent::FallbackRuntimeAgent( +FallbackRuntimeAgentDelegate::FallbackRuntimeAgentDelegate( FrontendChannel frontendChannel, const SessionState& sessionState, std::string engineDescription) @@ -32,7 +32,8 @@ FallbackRuntimeAgent::FallbackRuntimeAgent( } } -bool FallbackRuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) { +bool FallbackRuntimeAgentDelegate::handleRequest( + const cdp::PreparsedRequest& req) { if (req.method == "Log.enable") { sendFallbackRuntimeWarning(); @@ -44,7 +45,7 @@ bool FallbackRuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) { return false; } -void FallbackRuntimeAgent::sendFallbackRuntimeWarning() { +void FallbackRuntimeAgentDelegate::sendFallbackRuntimeWarning() { sendWarningLogEntry( "The current JavaScript engine, " ANSI_STYLE_ITALIC + engineDescription_ + ANSI_STYLE_RESET @@ -52,7 +53,7 @@ void FallbackRuntimeAgent::sendFallbackRuntimeWarning() { "See https://reactnative.dev/docs/debugging for more information."); } -void FallbackRuntimeAgent::sendWarningLogEntry(std::string_view text) { +void FallbackRuntimeAgentDelegate::sendWarningLogEntry(std::string_view text) { frontendChannel_( folly::toJson(folly::dynamic::object("method", "Log.entryAdded")( "params", diff --git a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h similarity index 89% rename from packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.h rename to packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h index 8fc324e2b28..56e3e895959 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h @@ -15,10 +15,10 @@ namespace facebook::react::jsinspector_modern { /** - * A RuntimeAgent that handles requests from the Chrome DevTools Protocol for - * a JavaScript runtime that does not support debugging. + * A RuntimeAgentDelegate that handles requests from the Chrome DevTools + * Protocol for a JavaScript runtime that does not support debugging. */ -class FallbackRuntimeAgent : public RuntimeAgent { +class FallbackRuntimeAgentDelegate : public RuntimeAgentDelegate { public: /** * \param frontendChannel A channel used to send responses and events to the @@ -27,7 +27,7 @@ class FallbackRuntimeAgent : public RuntimeAgent { * \param engineDescription A description of the JavaScript engine being * debugged. This string will be used in messages sent to the frontend. */ - FallbackRuntimeAgent( + FallbackRuntimeAgentDelegate( FrontendChannel frontendChannel, const SessionState& sessionState, std::string engineDescription); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h index a67ff426b60..abb6cb21fce 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h @@ -24,7 +24,7 @@ class InstanceTarget; * An Agent that handles requests from the Chrome DevTools Protocol for the * given InstanceTarget. */ -class InstanceAgent { +class InstanceAgent final { public: /** * \param frontendChannel A channel used to send responses and events to the diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h index 53a215415f3..5be5e97b7c5 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h @@ -40,7 +40,7 @@ class InstanceTargetDelegate { /** * A Target that represents a single instance of React Native. */ -class InstanceTarget { +class InstanceTarget final { public: /** * \param delegate The object that will receive events from this target. diff --git a/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h index a9d3b2f3d0b..30f540abfcf 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h @@ -30,7 +30,7 @@ class InstanceTarget; * same thread, which is also the thread where the associated PageTarget is * constructed and managed. */ -class PageAgent { +class PageAgent final { public: /** * \param frontendChannel A channel used to send responses and events to the diff --git a/packages/react-native/ReactCommon/jsinspector-modern/PageTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/PageTarget.h index 0d9c1ff5a7c..a3e592650eb 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/PageTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/PageTarget.h @@ -78,7 +78,7 @@ class PageTargetDelegate { * The limited interface that PageTarget exposes to its associated * sessions/agents. */ -class PageTargetController { +class PageTargetController final { public: explicit PageTargetController(PageTarget& target); @@ -95,7 +95,7 @@ class PageTargetController { * "Host" in React Native's architecture - the entity that manages the * lifecycle of a React Instance. */ -class JSINSPECTOR_EXPORT PageTarget { +class JSINSPECTOR_EXPORT PageTarget final { public: struct SessionMetadata { std::optional integrationName; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h b/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h index a5775819ed2..78524402152 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h @@ -7,7 +7,7 @@ #pragma once -#include +#include #include #include #include diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp index 084d8891a38..a1761a2d0a5 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp @@ -5,10 +5,28 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include "RuntimeAgent.h" namespace facebook::react::jsinspector_modern { -RuntimeAgent::~RuntimeAgent() {} +RuntimeAgent::RuntimeAgent( + FrontendChannel frontendChannel, + RuntimeTarget& target, + SessionState& sessionState, + std::unique_ptr delegate) + : frontendChannel_(std::move(frontendChannel)), + target_(target), + sessionState_(sessionState), + delegate_(std::move(delegate)) { + (void)target_; + (void)sessionState_; +} + +bool RuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) { + if (delegate_) { + return delegate_->handleRequest(req); + } + return false; +} } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.h index 7beba2f653f..5cc60a7436b 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.h @@ -7,29 +7,59 @@ #pragma once +#include "InspectorInterfaces.h" +#include "RuntimeAgentDelegate.h" +#include "RuntimeTarget.h" +#include "SessionState.h" + #include namespace facebook::react::jsinspector_modern { +class RuntimeTarget; + /** - * An Agent interface that handles requests from the Chrome DevTools Protocol - * for a particular JS runtime instance. The exact mechanism of sending - * responses/events to the frontend is left up to the implementation, but - * implementations SHOULD use FrontendChannel or a similar abstraction. + * An Agent that handles requests from the Chrome DevTools Protocol + * for a particular JS runtime instance. RuntimeAgent implements + * engine-agnostic functionality based on interfaces available in JSI / + * RuntimeTarget, and delegates engine-specific functionality to a + * RuntimeAgentDelegate. */ -class RuntimeAgent { +class RuntimeAgent final { public: - virtual ~RuntimeAgent(); + /** + * \param frontendChannel A channel used to send responses and events to the + * frontend. + * \param target The RuntimeTarget that this agent is attached to. The + * caller is responsible for ensuring that the RuntimeTarget outlives this + * object. + * \param sessionState The state of the session that created this agent. + * \param delegate The RuntimeAgentDelegate providing engine-specific + * CDP functionality. + */ + RuntimeAgent( + FrontendChannel frontendChannel, + RuntimeTarget& target, + SessionState& sessionState, + std::unique_ptr delegate); /** - * Handle a CDP request. This implementation must perform any synchronization - * required between the thread on which this method is called and the thread - * where the JS runtime is executing. + * Handle a CDP request. The response will be sent over the provided + * \c FrontendChannel synchronously or asynchronously. Performs any + * synchronization required between the thread on which this method is + * called and the thread where the JS runtime is executing. + * \param req The parsed request. * \returns true if this agent has responded, or will respond asynchronously, * to the request (with either a success or error message). False if the * agent expects another agent to respond to the request instead. */ - virtual bool handleRequest(const cdp::PreparsedRequest& req) = 0; + bool handleRequest(const cdp::PreparsedRequest& req); + + private: + FrontendChannel frontendChannel_; + RuntimeTarget& target_; + SessionState& sessionState_; + const std::unique_ptr delegate_; }; } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgentDelegate.h b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgentDelegate.h new file mode 100644 index 00000000000..1c705001e3f --- /dev/null +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgentDelegate.h @@ -0,0 +1,35 @@ +/* + * 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 + +namespace facebook::react::jsinspector_modern { + +/** + * An Agent interface that handles requests from the Chrome DevTools Protocol + * for a particular JS runtime instance. The exact mechanism of sending + * responses/events to the frontend is left up to the implementation, but + * implementations SHOULD use FrontendChannel or a similar abstraction. + */ +class RuntimeAgentDelegate { + public: + virtual ~RuntimeAgentDelegate() = default; + + /** + * Handle a CDP request. This implementation must perform any synchronization + * required between the thread on which this method is called and the thread + * where the JS runtime is executing. + * \returns true if this agent has responded, or will respond asynchronously, + * to the request (with either a success or error message). False if the + * agent expects another agent to respond to the request instead. + */ + virtual bool handleRequest(const cdp::PreparsedRequest& req) = 0; +}; + +} // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp index 98364f52519..d7086b0305a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp @@ -14,7 +14,11 @@ RuntimeTarget::RuntimeTarget(RuntimeTargetDelegate& delegate) std::unique_ptr RuntimeTarget::createAgent( FrontendChannel channel, SessionState& sessionState) { - return delegate_.createAgent(channel, sessionState); + return std::make_unique( + channel, + *this, + sessionState, + delegate_.createAgentDelegate(channel, sessionState)); } } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h index ec0853b7cb9..774214bc82f 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h @@ -27,6 +27,9 @@ namespace facebook::react::jsinspector_modern { +class RuntimeAgent; +class RuntimeAgentDelegate; + /** * Receives events from a RuntimeTarget. This is a shared interface that * each React Native platform needs to implement in order to integrate with @@ -35,7 +38,7 @@ namespace facebook::react::jsinspector_modern { class RuntimeTargetDelegate { public: virtual ~RuntimeTargetDelegate() = default; - virtual std::unique_ptr createAgent( + virtual std::unique_ptr createAgentDelegate( FrontendChannel channel, SessionState& sessionState) = 0; }; @@ -43,7 +46,7 @@ class RuntimeTargetDelegate { /** * A Target corresponding to a JavaScript runtime. */ -class JSINSPECTOR_EXPORT RuntimeTarget { +class JSINSPECTOR_EXPORT RuntimeTarget final { public: /** * \param delegate The object that will receive events from this target. diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h index c0e4596bf34..4a7d31adfbb 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h @@ -127,21 +127,21 @@ class MockRuntimeTargetDelegate : public RuntimeTargetDelegate { public: // RuntimeTargetDelegate methods MOCK_METHOD( - std::unique_ptr, - createAgent, + std::unique_ptr, + createAgentDelegate, (FrontendChannel channel, SessionState& sessionState), (override)); }; -class MockRuntimeAgent : public RuntimeAgent { +class MockRuntimeAgentDelegate : public RuntimeAgentDelegate { public: - inline MockRuntimeAgent( + inline MockRuntimeAgentDelegate( FrontendChannel frontendChannel, SessionState& sessionState) : frontendChannel(std::move(frontendChannel)), sessionState(sessionState) {} - // RuntimeAgent methods + // RuntimeAgentDelegate methods MOCK_METHOD( bool, handleRequest, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp index 8a03588181e..9f68661833e 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp @@ -28,9 +28,9 @@ namespace { class PageTargetTest : public Test { protected: PageTargetTest() { - EXPECT_CALL(runtimeTargetDelegate_, createAgent(_, _)) + EXPECT_CALL(runtimeTargetDelegate_, createAgentDelegate(_, _)) .WillRepeatedly( - runtimeAgents_ + runtimeAgentDelegates_ .lazily_make_unique()); } @@ -57,7 +57,7 @@ class PageTargetTest : public Test { MockInstanceTargetDelegate instanceTargetDelegate_; MockRuntimeTargetDelegate runtimeTargetDelegate_; - UniquePtrFactory> runtimeAgents_; + UniquePtrFactory> runtimeAgentDelegates_; private: UniquePtrFactory> remoteConnections_; @@ -258,8 +258,8 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) { InSequence s; - ASSERT_TRUE(runtimeAgents_[0]); - EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgentDelegates_[0]); + EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) .WillOnce(Return(true)) .RetiresOnSaturation(); toPage_->sendMessage(R"({ @@ -278,48 +278,48 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) { })"; EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) .RetiresOnSaturation(); - runtimeAgents_[0]->frontendChannel(kFooResponse); + runtimeAgentDelegates_[0]->frontendChannel(kFooResponse); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, RuntimeAgentLifecycle) { +TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateLifecycle) { { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - EXPECT_TRUE(runtimeAgents_[0]); + EXPECT_TRUE(runtimeAgentDelegates_[0]); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } - EXPECT_FALSE(runtimeAgents_[0]); + EXPECT_FALSE(runtimeAgentDelegates_[0]); { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - EXPECT_TRUE(runtimeAgents_[1]); + EXPECT_TRUE(runtimeAgentDelegates_[1]); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } - EXPECT_FALSE(runtimeAgents_[1]); + EXPECT_FALSE(runtimeAgentDelegates_[1]); } -TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgent) { +TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgentDelegate) { InSequence s; auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgents_[0]); - EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgentDelegates_[0]); + EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) .WillOnce(Return(false)) .RetiresOnSaturation(); EXPECT_CALL( @@ -337,14 +337,14 @@ TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgent) { page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgent) { +TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgentDelegate) { InSequence s; auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgents_[0]); - EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgentDelegates_[0]); + EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) .WillOnce(Return(true)) .RetiresOnSaturation(); toPage_->sendMessage(R"({ @@ -363,13 +363,13 @@ TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgent) { })"; EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) .RetiresOnSaturation(); - runtimeAgents_[0]->frontendChannel(kFooResponse); + runtimeAgentDelegates_[0]->frontendChannel(kFooResponse); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { +TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgentDelegate) { InSequence s; EXPECT_CALL( @@ -386,8 +386,8 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgents_[0]); - EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgentDelegates_[0]); + EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) .WillOnce(Return(true)) .RetiresOnSaturation(); toPage_->sendMessage(R"({ @@ -406,12 +406,12 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { })"; EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) .RetiresOnSaturation(); - runtimeAgents_[0]->frontendChannel(kFooResponse); + runtimeAgentDelegates_[0]->frontendChannel(kFooResponse); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); - EXPECT_FALSE(runtimeAgents_[0]); + EXPECT_FALSE(runtimeAgentDelegates_[0]); EXPECT_CALL( fromPage(), onMessage(JsonParsed(AtJsonPtr("/error/code", Eq(-32601))))) @@ -425,16 +425,16 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { })"); } -TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgent) { +TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgentDelegate) { InSequence s; - EXPECT_CALL(runtimeTargetDelegate_, createAgent(_, _)) + EXPECT_CALL(runtimeTargetDelegate_, createAgentDelegate(_, _)) .WillRepeatedly(ReturnNull()); auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - EXPECT_FALSE(runtimeAgents_[0]); + EXPECT_FALSE(runtimeAgentDelegates_[0]); EXPECT_CALL( fromPage(), onMessage(JsonParsed(AtJsonPtr("/error/code", Eq(-32601))))) @@ -451,11 +451,11 @@ TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgent) { page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, RuntimeAgentHasAccessToSessionState) { +TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateHasAccessToSessionState) { InSequence s; // Send Runtime.enable before registering the Instance (which in turns creates - // the RuntimeAgent). + // the RuntimeAgentDelegate). EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({ "id": 1, "result": {} @@ -467,13 +467,13 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentHasAccessToSessionState) { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgents_[0]); + ASSERT_TRUE(runtimeAgentDelegates_[0]); - EXPECT_TRUE(runtimeAgents_[0]->sessionState.isRuntimeDomainEnabled); + EXPECT_TRUE(runtimeAgentDelegates_[0]->sessionState.isRuntimeDomainEnabled); - // Send Runtime.disable while the RuntimeAgent exists - it receives the - // message and can also observe the updated state. - EXPECT_CALL(*runtimeAgents_[0], handleRequest(Eq(cdp::preparse(R"({ + // Send Runtime.disable while the RuntimeAgentDelegate exists - it receives + // the message and can also observe the updated state. + EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(Eq(cdp::preparse(R"({ "id": 2, "method": "Runtime.disable" })")))); @@ -486,7 +486,7 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentHasAccessToSessionState) { "method": "Runtime.disable" })"); - EXPECT_FALSE(runtimeAgents_[0]->sessionState.isRuntimeDomainEnabled); + EXPECT_FALSE(runtimeAgentDelegates_[0]->sessionState.isRuntimeDomainEnabled); } } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp index bd90687c4c3..30d689a68c2 100644 --- a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp +++ b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp @@ -18,10 +18,11 @@ JSIRuntimeHolder::JSIRuntimeHolder(std::unique_ptr runtime) assert(runtime_ != nullptr); } -std::unique_ptr JSIRuntimeHolder::createAgent( +std::unique_ptr +JSIRuntimeHolder::createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { - return std::make_unique( + return std::make_unique( std::move(frontendChannel), sessionState, runtime_->description()); } diff --git a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h index ce8b50afb3f..a61830a7697 100644 --- a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h +++ b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h @@ -41,7 +41,7 @@ class JSRuntimeFactory { class JSIRuntimeHolder : public JSRuntime { public: jsi::Runtime& getRuntime() noexcept override; - std::unique_ptr createAgent( + std::unique_ptr createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override; diff --git a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp index 289cef1e170..15365f3e564 100644 --- a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp @@ -7,7 +7,7 @@ #include "HermesInstance.h" -#include +#include #include #include #include @@ -104,11 +104,11 @@ class HermesJSRuntime : public JSRuntime { return *runtime_; } - std::unique_ptr createAgent( + std::unique_ptr createAgentDelegate( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override { - return std::unique_ptr( - new jsinspector_modern::HermesRuntimeAgent( + return std::unique_ptr( + new jsinspector_modern::HermesRuntimeAgentDelegate( frontendChannel, sessionState, runtime_,