From c9a9e5dc82df764c6591f697de470e9f2622dd69 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 13 Feb 2024 21:51:46 -0800 Subject: [PATCH] Back out "RuntimeTarget refactor - RuntimeAgent --> RuntimeAgentDelegate" Summary: This one snuck in with new OSS buid failures after when we had another change cause a failure. Back it out, to get CI passing. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D53745683 fbshipit-source-id: f889bf7541e6f664053d5c0e4851cb448cdbb615 --- .../ReactCommon/cxxreact/JSExecutor.cpp | 5 +- .../ReactCommon/cxxreact/JSExecutor.h | 7 +- .../hermes/executor/HermesExecutorFactory.cpp | 7 +- .../hermes/executor/HermesExecutorFactory.h | 3 +- ...entDelegate.cpp => HermesRuntimeAgent.cpp} | 32 ++++----- ...meAgentDelegate.h => HermesRuntimeAgent.h} | 8 +-- .../jsinspector-modern/CONCEPTS.md | 4 -- ...tDelegate.cpp => FallbackRuntimeAgent.cpp} | 11 ++- ...AgentDelegate.h => FallbackRuntimeAgent.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, 104 insertions(+), 206 deletions(-) rename packages/react-native/ReactCommon/hermes/inspector-modern/chrome/{HermesRuntimeAgentDelegate.cpp => HermesRuntimeAgent.cpp} (85%) rename packages/react-native/ReactCommon/hermes/inspector-modern/chrome/{HermesRuntimeAgentDelegate.h => HermesRuntimeAgent.h} (89%) rename packages/react-native/ReactCommon/jsinspector-modern/{FallbackRuntimeAgentDelegate.cpp => FallbackRuntimeAgent.cpp} (84%) rename packages/react-native/ReactCommon/jsinspector-modern/{FallbackRuntimeAgentDelegate.h => FallbackRuntimeAgent.h} (89%) delete 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 ba34e3f8fa2..b760e7356f5 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp +++ b/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp @@ -35,11 +35,10 @@ double JSExecutor::performanceNow() { return duration / NANOSECONDS_IN_MILLISECOND; } -std::unique_ptr -JSExecutor::createAgentDelegate( +std::unique_ptr JSExecutor::createAgent( 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 0b7041e1375..cd66d0d43aa 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 createAgentDelegate method. + * override the \c createAgent method. */ virtual bool isInspectable() { return false; @@ -141,10 +141,9 @@ class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate { static double performanceNow(); /** - * Create a RuntimeAgentDelegate that can be used to debug the JS VM instance. + * Create a RuntimeAgent that can be used to debug the JS VM instance. */ - virtual std::unique_ptr - createAgentDelegate( + virtual std::unique_ptr createAgent( 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 5d3a3e457b7..67fa9138ff9 100644 --- a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp +++ b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp @@ -257,13 +257,12 @@ HermesExecutor::HermesExecutor( runtime_(runtime), hermesRuntime_(hermesRuntime) {} -std::unique_ptr -HermesExecutor::createAgentDelegate( +std::unique_ptr HermesExecutor::createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { std::shared_ptr hermesRuntimeShared(runtime_, &hermesRuntime_); - return std::unique_ptr( - new jsinspector_modern::HermesRuntimeAgentDelegate( + return std::unique_ptr( + new jsinspector_modern::HermesRuntimeAgent( frontendChannel, sessionState, hermesRuntimeShared, diff --git a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h index 4ae521539d4..9cc96759792 100644 --- a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h +++ b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h @@ -54,8 +54,7 @@ class HermesExecutor : public JSIExecutor { RuntimeInstaller runtimeInstaller, hermes::HermesRuntime& hermesRuntime); - virtual std::unique_ptr - createAgentDelegate( + virtual std::unique_ptr createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override; diff --git a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.cpp similarity index 85% rename from packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp rename to packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.cpp index 2a6f089e048..0430e9dcc86 100644 --- a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp +++ b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.cpp @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -#include "HermesRuntimeAgentDelegate.h" +#include "HermesRuntimeAgent.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 RuntimeAgentDelegate. + * Hermes's CDPHandler API) for use within a React Native RuntimeAgent. */ -class HermesRuntimeAgentDelegateAdapter +class HermesRuntimeAgentAdapter : public hermes::inspector_modern::RuntimeAdapter { public: - HermesRuntimeAgentDelegateAdapter( + HermesRuntimeAgentAdapter( std::shared_ptr runtime, RuntimeExecutor runtimeExecutor) : runtime_(runtime), runtimeExecutor_(runtimeExecutor) {} @@ -60,10 +60,10 @@ class HermesRuntimeAgentDelegateAdapter } // namespace /** - * A RuntimeAgentDelegate that handles requests from the Chrome DevTools - * Protocol for an instance of Hermes. + * A RuntimeAgent that handles requests from the Chrome DevTools Protocol for + * an instance of Hermes. */ -class HermesRuntimeAgentDelegate::Impl final : public RuntimeAgentDelegate { +class HermesRuntimeAgent::Impl final : public RuntimeAgent { using HermesCDPHandler = hermes::inspector_modern::chrome::CDPHandler; public: @@ -84,7 +84,7 @@ class HermesRuntimeAgentDelegate::Impl final : public RuntimeAgentDelegate { std::shared_ptr runtime, RuntimeExecutor runtimeExecutor) : hermes_(HermesCDPHandler::create( - std::make_unique( + std::make_unique( runtime, runtimeExecutor), /* waitForDebugger */ false, @@ -133,18 +133,17 @@ class HermesRuntimeAgentDelegate::Impl final : public RuntimeAgentDelegate { #else // !HERMES_ENABLE_DEBUGGER /** - * A stub for HermesRuntimeAgentDelegate when Hermes is compiled without - * debugging support. + * A stub for HermesRuntimeAgent when Hermes is compiled without debugging + * support. */ -class HermesRuntimeAgentDelegate::Impl final - : public FallbackRuntimeAgentDelegate { +class HermesRuntimeAgent::Impl final : public FallbackRuntimeAgent { public: Impl( FrontendChannel frontendChannel, SessionState& sessionState, std::shared_ptr runtime, RuntimeExecutor) - : FallbackRuntimeAgentDelegate( + : FallbackRuntimeAgent( std::move(frontendChannel), sessionState, runtime->description()) {} @@ -152,7 +151,7 @@ class HermesRuntimeAgentDelegate::Impl final #endif // HERMES_ENABLE_DEBUGGER -HermesRuntimeAgentDelegate::HermesRuntimeAgentDelegate( +HermesRuntimeAgent::HermesRuntimeAgent( FrontendChannel frontendChannel, SessionState& sessionState, std::shared_ptr runtime, @@ -163,8 +162,7 @@ HermesRuntimeAgentDelegate::HermesRuntimeAgentDelegate( std::move(runtime), std::move(runtimeExecutor))) {} -bool HermesRuntimeAgentDelegate::handleRequest( - const cdp::PreparsedRequest& req) { +bool HermesRuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) { return impl_->handleRequest(req); } diff --git a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.h b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.h similarity index 89% rename from packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.h rename to packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.h index 5700ab02e6a..20323f4329d 100644 --- a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.h +++ b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgent.h @@ -15,10 +15,10 @@ namespace facebook::react::jsinspector_modern { /** - * A RuntimeAgentDelegate that handles requests from the Chrome DevTools - * Protocol for an instance of Hermes. + * A RuntimeAgent that handles requests from the Chrome DevTools Protocol for + * an instance of Hermes. */ -class HermesRuntimeAgentDelegate : public RuntimeAgentDelegate { +class HermesRuntimeAgent : public RuntimeAgent { public: /** * \param frontendChannel A channel used to send responses and events to the @@ -31,7 +31,7 @@ class HermesRuntimeAgentDelegate : public RuntimeAgentDelegate { * \c runtimeExecutor may drop scheduled work if the runtime is destroyed * first. */ - HermesRuntimeAgentDelegate( + HermesRuntimeAgent( 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 e07d2eeda97..b7353957e37 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/CONCEPTS.md +++ b/packages/react-native/ReactCommon/jsinspector-modern/CONCEPTS.md @@ -21,7 +21,3 @@ 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/FallbackRuntimeAgentDelegate.cpp b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.cpp similarity index 84% rename from packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.cpp rename to packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.cpp index 75f74e5f285..cbb139746f0 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.cpp @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -#include "FallbackRuntimeAgentDelegate.h" +#include #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" -FallbackRuntimeAgentDelegate::FallbackRuntimeAgentDelegate( +FallbackRuntimeAgent::FallbackRuntimeAgent( FrontendChannel frontendChannel, const SessionState& sessionState, std::string engineDescription) @@ -32,8 +32,7 @@ FallbackRuntimeAgentDelegate::FallbackRuntimeAgentDelegate( } } -bool FallbackRuntimeAgentDelegate::handleRequest( - const cdp::PreparsedRequest& req) { +bool FallbackRuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) { if (req.method == "Log.enable") { sendFallbackRuntimeWarning(); @@ -45,7 +44,7 @@ bool FallbackRuntimeAgentDelegate::handleRequest( return false; } -void FallbackRuntimeAgentDelegate::sendFallbackRuntimeWarning() { +void FallbackRuntimeAgent::sendFallbackRuntimeWarning() { sendWarningLogEntry( "The current JavaScript engine, " ANSI_STYLE_ITALIC + engineDescription_ + ANSI_STYLE_RESET @@ -53,7 +52,7 @@ void FallbackRuntimeAgentDelegate::sendFallbackRuntimeWarning() { "See https://reactnative.dev/docs/debugging for more information."); } -void FallbackRuntimeAgentDelegate::sendWarningLogEntry(std::string_view text) { +void FallbackRuntimeAgent::sendWarningLogEntry(std::string_view text) { frontendChannel_( folly::toJson(folly::dynamic::object("method", "Log.entryAdded")( "params", diff --git a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.h similarity index 89% rename from packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h rename to packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.h index 56e3e895959..8fc324e2b28 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgent.h @@ -15,10 +15,10 @@ namespace facebook::react::jsinspector_modern { /** - * A RuntimeAgentDelegate that handles requests from the Chrome DevTools - * Protocol for a JavaScript runtime that does not support debugging. + * A RuntimeAgent that handles requests from the Chrome DevTools Protocol for + * a JavaScript runtime that does not support debugging. */ -class FallbackRuntimeAgentDelegate : public RuntimeAgentDelegate { +class FallbackRuntimeAgent : public RuntimeAgent { public: /** * \param frontendChannel A channel used to send responses and events to the @@ -27,7 +27,7 @@ class FallbackRuntimeAgentDelegate : public RuntimeAgentDelegate { * \param engineDescription A description of the JavaScript engine being * debugged. This string will be used in messages sent to the frontend. */ - FallbackRuntimeAgentDelegate( + FallbackRuntimeAgent( 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 abb6cb21fce..a67ff426b60 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 final { +class InstanceAgent { 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 5be5e97b7c5..53a215415f3 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 final { +class InstanceTarget { 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 30f540abfcf..a9d3b2f3d0b 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 final { +class PageAgent { 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 a3e592650eb..0d9c1ff5a7c 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 final { +class PageTargetController { public: explicit PageTargetController(PageTarget& target); @@ -95,7 +95,7 @@ class PageTargetController final { * "Host" in React Native's architecture - the entity that manages the * lifecycle of a React Instance. */ -class JSINSPECTOR_EXPORT PageTarget final { +class JSINSPECTOR_EXPORT PageTarget { 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 78524402152..a5775819ed2 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 a1761a2d0a5..084d8891a38 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp @@ -5,28 +5,10 @@ * LICENSE file in the root directory of this source tree. */ -#include "RuntimeAgent.h" +#include namespace facebook::react::jsinspector_modern { -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; -} +RuntimeAgent::~RuntimeAgent() {} } // 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 5cc60a7436b..7beba2f653f 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.h @@ -7,59 +7,29 @@ #pragma once -#include "InspectorInterfaces.h" -#include "RuntimeAgentDelegate.h" -#include "RuntimeTarget.h" -#include "SessionState.h" - #include namespace facebook::react::jsinspector_modern { -class RuntimeTarget; - /** - * 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. + * 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 RuntimeAgent final { +class RuntimeAgent { public: - /** - * \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); + virtual ~RuntimeAgent(); /** - * 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. + * 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. */ - bool handleRequest(const cdp::PreparsedRequest& req); - - private: - FrontendChannel frontendChannel_; - RuntimeTarget& target_; - SessionState& sessionState_; - const std::unique_ptr delegate_; + virtual bool handleRequest(const cdp::PreparsedRequest& req) = 0; }; } // 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 deleted file mode 100644 index 1c705001e3f..00000000000 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgentDelegate.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * 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 d7086b0305a..98364f52519 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp @@ -14,11 +14,7 @@ RuntimeTarget::RuntimeTarget(RuntimeTargetDelegate& delegate) std::unique_ptr RuntimeTarget::createAgent( FrontendChannel channel, SessionState& sessionState) { - return std::make_unique( - channel, - *this, - sessionState, - delegate_.createAgentDelegate(channel, sessionState)); + return delegate_.createAgent(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 774214bc82f..ec0853b7cb9 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h @@ -27,9 +27,6 @@ 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 @@ -38,7 +35,7 @@ class RuntimeAgentDelegate; class RuntimeTargetDelegate { public: virtual ~RuntimeTargetDelegate() = default; - virtual std::unique_ptr createAgentDelegate( + virtual std::unique_ptr createAgent( FrontendChannel channel, SessionState& sessionState) = 0; }; @@ -46,7 +43,7 @@ class RuntimeTargetDelegate { /** * A Target corresponding to a JavaScript runtime. */ -class JSINSPECTOR_EXPORT RuntimeTarget final { +class JSINSPECTOR_EXPORT RuntimeTarget { 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 4a7d31adfbb..c0e4596bf34 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, - createAgentDelegate, + std::unique_ptr, + createAgent, (FrontendChannel channel, SessionState& sessionState), (override)); }; -class MockRuntimeAgentDelegate : public RuntimeAgentDelegate { +class MockRuntimeAgent : public RuntimeAgent { public: - inline MockRuntimeAgentDelegate( + inline MockRuntimeAgent( FrontendChannel frontendChannel, SessionState& sessionState) : frontendChannel(std::move(frontendChannel)), sessionState(sessionState) {} - // RuntimeAgentDelegate methods + // RuntimeAgent 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 9f68661833e..8a03588181e 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_, createAgentDelegate(_, _)) + EXPECT_CALL(runtimeTargetDelegate_, createAgent(_, _)) .WillRepeatedly( - runtimeAgentDelegates_ + runtimeAgents_ .lazily_make_unique()); } @@ -57,7 +57,7 @@ class PageTargetTest : public Test { MockInstanceTargetDelegate instanceTargetDelegate_; MockRuntimeTargetDelegate runtimeTargetDelegate_; - UniquePtrFactory> runtimeAgentDelegates_; + UniquePtrFactory> runtimeAgents_; private: UniquePtrFactory> remoteConnections_; @@ -258,8 +258,8 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) { InSequence s; - ASSERT_TRUE(runtimeAgentDelegates_[0]); - EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgents_[0]); + EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) .WillOnce(Return(true)) .RetiresOnSaturation(); toPage_->sendMessage(R"({ @@ -278,48 +278,48 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) { })"; EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) .RetiresOnSaturation(); - runtimeAgentDelegates_[0]->frontendChannel(kFooResponse); + runtimeAgents_[0]->frontendChannel(kFooResponse); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateLifecycle) { +TEST_F(PageTargetProtocolTest, RuntimeAgentLifecycle) { { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - EXPECT_TRUE(runtimeAgentDelegates_[0]); + EXPECT_TRUE(runtimeAgents_[0]); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } - EXPECT_FALSE(runtimeAgentDelegates_[0]); + EXPECT_FALSE(runtimeAgents_[0]); { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - EXPECT_TRUE(runtimeAgentDelegates_[1]); + EXPECT_TRUE(runtimeAgents_[1]); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } - EXPECT_FALSE(runtimeAgentDelegates_[1]); + EXPECT_FALSE(runtimeAgents_[1]); } -TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgentDelegate) { +TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgent) { InSequence s; auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgentDelegates_[0]); - EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgents_[0]); + EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) .WillOnce(Return(false)) .RetiresOnSaturation(); EXPECT_CALL( @@ -337,14 +337,14 @@ TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgentDelegate) { page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgentDelegate) { +TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgent) { InSequence s; auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgentDelegates_[0]); - EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgents_[0]); + EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) .WillOnce(Return(true)) .RetiresOnSaturation(); toPage_->sendMessage(R"({ @@ -363,13 +363,13 @@ TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgentDelegate) { })"; EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) .RetiresOnSaturation(); - runtimeAgentDelegates_[0]->frontendChannel(kFooResponse); + runtimeAgents_[0]->frontendChannel(kFooResponse); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgentDelegate) { +TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { InSequence s; EXPECT_CALL( @@ -386,8 +386,8 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgentDelegate) { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgentDelegates_[0]); - EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_)) + ASSERT_TRUE(runtimeAgents_[0]); + EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) .WillOnce(Return(true)) .RetiresOnSaturation(); toPage_->sendMessage(R"({ @@ -406,12 +406,12 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgentDelegate) { })"; EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) .RetiresOnSaturation(); - runtimeAgentDelegates_[0]->frontendChannel(kFooResponse); + runtimeAgents_[0]->frontendChannel(kFooResponse); instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); - EXPECT_FALSE(runtimeAgentDelegates_[0]); + EXPECT_FALSE(runtimeAgents_[0]); EXPECT_CALL( fromPage(), onMessage(JsonParsed(AtJsonPtr("/error/code", Eq(-32601))))) @@ -425,16 +425,16 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgentDelegate) { })"); } -TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgentDelegate) { +TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgent) { InSequence s; - EXPECT_CALL(runtimeTargetDelegate_, createAgentDelegate(_, _)) + EXPECT_CALL(runtimeTargetDelegate_, createAgent(_, _)) .WillRepeatedly(ReturnNull()); auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - EXPECT_FALSE(runtimeAgentDelegates_[0]); + EXPECT_FALSE(runtimeAgents_[0]); EXPECT_CALL( fromPage(), onMessage(JsonParsed(AtJsonPtr("/error/code", Eq(-32601))))) @@ -451,11 +451,11 @@ TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgentDelegate) { page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateHasAccessToSessionState) { +TEST_F(PageTargetProtocolTest, RuntimeAgentHasAccessToSessionState) { InSequence s; // Send Runtime.enable before registering the Instance (which in turns creates - // the RuntimeAgentDelegate). + // the RuntimeAgent). EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({ "id": 1, "result": {} @@ -467,13 +467,13 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateHasAccessToSessionState) { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgentDelegates_[0]); + ASSERT_TRUE(runtimeAgents_[0]); - EXPECT_TRUE(runtimeAgentDelegates_[0]->sessionState.isRuntimeDomainEnabled); + EXPECT_TRUE(runtimeAgents_[0]->sessionState.isRuntimeDomainEnabled); - // 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"({ + // 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"({ "id": 2, "method": "Runtime.disable" })")))); @@ -486,7 +486,7 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateHasAccessToSessionState) { "method": "Runtime.disable" })"); - EXPECT_FALSE(runtimeAgentDelegates_[0]->sessionState.isRuntimeDomainEnabled); + EXPECT_FALSE(runtimeAgents_[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 30d689a68c2..bd90687c4c3 100644 --- a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp +++ b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp @@ -18,11 +18,10 @@ JSIRuntimeHolder::JSIRuntimeHolder(std::unique_ptr runtime) assert(runtime_ != nullptr); } -std::unique_ptr -JSIRuntimeHolder::createAgentDelegate( +std::unique_ptr JSIRuntimeHolder::createAgent( 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 a61830a7697..ce8b50afb3f 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 createAgentDelegate( + std::unique_ptr createAgent( 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 15365f3e564..289cef1e170 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 createAgentDelegate( + std::unique_ptr createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override { - return std::unique_ptr( - new jsinspector_modern::HermesRuntimeAgentDelegate( + return std::unique_ptr( + new jsinspector_modern::HermesRuntimeAgent( frontendChannel, sessionState, runtime_,