From 415bb718ff3041c41a1f093dec434af29f2a592b Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 13 Feb 2024 08:14:17 -0800 Subject: [PATCH] RuntimeTarget refactor - Create RuntimeTarget, RuntimeTargetDelegate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: 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`. *← This diff* * Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`). *← Also in this diff* * Rename the existing `RuntimeAgent` interface to `RuntimeAgentDelegate`. * Create a new concrete `RuntimeAgent` class similar to `{Page,Instance}Agent`. * 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: hoxyq Differential Revision: D53233914 fbshipit-source-id: 166ae3e25059bd9c9c051a0a3312a3ba78a3935a --- .../ReactCommon/cxxreact/Instance.cpp | 10 +- .../ReactCommon/cxxreact/Instance.h | 6 +- .../ReactCommon/cxxreact/JSExecutor.cpp | 3 +- .../ReactCommon/cxxreact/JSExecutor.h | 8 +- .../ReactCommon/cxxreact/NativeToJsBridge.cpp | 10 +- .../ReactCommon/cxxreact/NativeToJsBridge.h | 8 +- .../hermes/executor/HermesExecutorFactory.cpp | 3 +- .../hermes/executor/HermesExecutorFactory.h | 2 +- .../jsinspector-modern/InstanceAgent.cpp | 13 +- .../jsinspector-modern/InstanceAgent.h | 18 ++- .../jsinspector-modern/InstanceTarget.cpp | 29 +++- .../jsinspector-modern/InstanceTarget.h | 38 +++-- .../jsinspector-modern/PageAgent.cpp | 2 +- .../jsinspector-modern/PageAgent.h | 4 +- .../ReactCommon/jsinspector-modern/ReactCdp.h | 2 +- .../jsinspector-modern/RuntimeTarget.cpp | 20 +++ .../jsinspector-modern/RuntimeTarget.h | 77 +++++++++ .../jsinspector-modern/tests/InspectorMocks.h | 8 +- .../tests/PageTargetTest.cpp | 151 ++++++++++++------ .../react/runtime/JSRuntimeFactory.cpp | 3 +- .../react/runtime/JSRuntimeFactory.h | 14 +- .../react/runtime/ReactInstance.cpp | 11 +- .../ReactCommon/react/runtime/ReactInstance.h | 5 +- .../react/runtime/hermes/HermesInstance.cpp | 2 +- 24 files changed, 304 insertions(+), 143 deletions(-) create mode 100644 packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp create mode 100644 packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.cpp b/packages/react-native/ReactCommon/cxxreact/Instance.cpp index e282ac8cbe6..702ed3a88c8 100644 --- a/packages/react-native/ReactCommon/cxxreact/Instance.cpp +++ b/packages/react-native/ReactCommon/cxxreact/Instance.cpp @@ -40,6 +40,8 @@ Instance::~Instance() { void Instance::unregisterFromInspector() { if (inspectorTarget_) { + assert(runtimeInspectorTarget_); + inspectorTarget_->unregisterRuntime(*runtimeInspectorTarget_); assert(parentInspectorTarget_); parentInspectorTarget_->unregisterInstance(*inspectorTarget_); parentInspectorTarget_ = nullptr; @@ -65,6 +67,8 @@ void Instance::initializeBridge( if (parentInspectorTarget) { inspectorTarget_ = &parentInspectorTarget->registerInstance(*this); + runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime( + nativeToJsBridge_->getInspectorTargetDelegate()); } /** @@ -316,10 +320,4 @@ void Instance::JSCallInvoker::scheduleAsync( } } -std::unique_ptr Instance::createRuntimeAgent( - jsinspector_modern::FrontendChannel frontendChannel, - jsinspector_modern::SessionState& sessionState) { - return nativeToJsBridge_->createRuntimeAgent(frontendChannel, sessionState); -} - } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.h b/packages/react-native/ReactCommon/cxxreact/Instance.h index df63690f0c2..bc4e58ebb1d 100644 --- a/packages/react-native/ReactCommon/cxxreact/Instance.h +++ b/packages/react-native/ReactCommon/cxxreact/Instance.h @@ -151,11 +151,6 @@ class RN_EXPORT Instance : private jsinspector_modern::InstanceTargetDelegate { std::unique_ptr startupScript, std::string startupScriptSourceURL); - // From InstanceTargetDelegate - std::unique_ptr createRuntimeAgent( - jsinspector_modern::FrontendChannel channel, - jsinspector_modern::SessionState& sessionState) override; - std::shared_ptr callback_; std::shared_ptr nativeToJsBridge_; std::shared_ptr moduleRegistry_; @@ -185,6 +180,7 @@ class RN_EXPORT Instance : private jsinspector_modern::InstanceTargetDelegate { jsinspector_modern::PageTarget* parentInspectorTarget_{nullptr}; jsinspector_modern::InstanceTarget* inspectorTarget_{nullptr}; + jsinspector_modern::RuntimeTarget* runtimeInspectorTarget_{nullptr}; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp b/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp index dfb0b905e8b..b760e7356f5 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp +++ b/packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp @@ -35,8 +35,7 @@ double JSExecutor::performanceNow() { return duration / NANOSECONDS_IN_MILLISECOND; } -std::unique_ptr -JSExecutor::createRuntimeAgent( +std::unique_ptr JSExecutor::createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { return std::make_unique( diff --git a/packages/react-native/ReactCommon/cxxreact/JSExecutor.h b/packages/react-native/ReactCommon/cxxreact/JSExecutor.h index aed4ffb892e..cd66d0d43aa 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSExecutor.h +++ b/packages/react-native/ReactCommon/cxxreact/JSExecutor.h @@ -55,7 +55,7 @@ class JSExecutorFactory { virtual ~JSExecutorFactory() {} }; -class RN_EXPORT JSExecutor { +class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate { public: /** * Prepares the JS runtime for React Native by installing global variables. @@ -114,7 +114,7 @@ class RN_EXPORT JSExecutor { /** * Returns whether or not the underlying executor supports debugging via the * Chrome remote debugging protocol. If true, the executor should also - * override the \c createRuntimeAgent method. + * override the \c createAgent method. */ virtual bool isInspectable() { return false; @@ -143,9 +143,9 @@ class RN_EXPORT JSExecutor { /** * Create a RuntimeAgent that can be used to debug the JS VM instance. */ - virtual std::unique_ptr createRuntimeAgent( + virtual std::unique_ptr createAgent( jsinspector_modern::FrontendChannel frontendChannel, - jsinspector_modern::SessionState& sessionState); + jsinspector_modern::SessionState& sessionState) override; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp index a8859d5ac87..f23ce412fae 100644 --- a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -343,13 +343,9 @@ NativeToJsBridge::getDecoratedNativeMethodCallInvoker( m_delegate, std::move(nativeMethodCallInvoker)); } -std::unique_ptr -NativeToJsBridge::createRuntimeAgent( - jsinspector_modern::FrontendChannel frontendChannel, - jsinspector_modern::SessionState& sessionState) { - auto agent = - m_executor->createRuntimeAgent(std::move(frontendChannel), sessionState); - return agent; +jsinspector_modern::RuntimeTargetDelegate& +NativeToJsBridge::getInspectorTargetDelegate() { + return *m_executor; } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.h b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.h index a28039e6794..da3fa32068e 100644 --- a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.h +++ b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.h @@ -107,13 +107,7 @@ class NativeToJsBridge { std::shared_ptr getDecoratedNativeMethodCallInvoker( std::shared_ptr nativeInvoker) const; - /** - * Create a RuntimeAgent that can be used to debug the underlying JS VM - * instance. - */ - virtual std::unique_ptr createRuntimeAgent( - jsinspector_modern::FrontendChannel frontendChannel, - jsinspector_modern::SessionState& sessionState); + jsinspector_modern::RuntimeTargetDelegate& getInspectorTargetDelegate(); private: // This is used to avoid a race condition where a proxyCallback gets queued diff --git a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp index 409d0431a57..67fa9138ff9 100644 --- a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp +++ b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp @@ -257,8 +257,7 @@ HermesExecutor::HermesExecutor( runtime_(runtime), hermesRuntime_(hermesRuntime) {} -std::unique_ptr -HermesExecutor::createRuntimeAgent( +std::unique_ptr HermesExecutor::createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { std::shared_ptr hermesRuntimeShared(runtime_, &hermesRuntime_); diff --git a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h index 357d90f8e4a..9cc96759792 100644 --- a/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h +++ b/packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h @@ -54,7 +54,7 @@ class HermesExecutor : public JSIExecutor { RuntimeInstaller runtimeInstaller, hermes::HermesRuntime& hermesRuntime); - virtual std::unique_ptr createRuntimeAgent( + virtual std::unique_ptr createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp index 386f62109d7..b58e597d7dd 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp @@ -6,16 +6,17 @@ */ #include +#include "RuntimeTarget.h" namespace facebook::react::jsinspector_modern { InstanceAgent::InstanceAgent( FrontendChannel frontendChannel, InstanceTarget& target, - std::unique_ptr runtimeAgent) + SessionState& sessionState) : frontendChannel_(frontendChannel), target_(target), - runtimeAgent_(std::move(runtimeAgent)) { + sessionState_(sessionState) { (void)target_; } @@ -26,4 +27,12 @@ bool InstanceAgent::handleRequest(const cdp::PreparsedRequest& req) { return false; } +void InstanceAgent::setCurrentRuntime(RuntimeTarget* runtimeTarget) { + if (runtimeTarget) { + runtimeAgent_ = runtimeTarget->createAgent(frontendChannel_, sessionState_); + } else { + runtimeAgent_.reset(); + } +} + } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h index 52538786495..a67ff426b60 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.h @@ -7,9 +7,13 @@ #pragma once +#include "RuntimeTarget.h" +#include "SessionState.h" + #include #include #include + #include namespace facebook::react::jsinspector_modern { @@ -28,13 +32,12 @@ class InstanceAgent { * \param target The InstanceTarget that this agent is attached to. The * caller is responsible for ensuring that the InstanceTarget outlives this * object. - * \param runtimeAgent The RuntimeAgent that this agent will use to - * communicate with the JS runtime. + * \param sessionState The state of the session that created this agent. */ explicit InstanceAgent( FrontendChannel frontendChannel, InstanceTarget& target, - std::unique_ptr runtimeAgent); + SessionState& sessionState); /** * Handle a CDP request. The response will be sent over the provided @@ -43,10 +46,19 @@ class InstanceAgent { */ bool handleRequest(const cdp::PreparsedRequest& req); + /** + * Replace the current RuntimeAgent pageAgent_ with a new one + * connected to the new RuntimeTarget. + * \param runtime The new runtime target. May be nullptr to indicate + * there's no current debuggable runtime. + */ + void setCurrentRuntime(RuntimeTarget* runtime); + private: FrontendChannel frontendChannel_; InstanceTarget& target_; std::unique_ptr runtimeAgent_; + SessionState& sessionState_; }; } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp index 25fda576b47..90f19df8dde 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp @@ -19,12 +19,33 @@ InstanceTarget::InstanceTarget(InstanceTargetDelegate& delegate) InstanceTargetDelegate::~InstanceTargetDelegate() {} -std::unique_ptr InstanceTarget::createAgent( +std::shared_ptr InstanceTarget::createAgent( FrontendChannel channel, SessionState& sessionState) { - auto runtimeAgent = delegate_.createRuntimeAgent(channel, sessionState); - return std::make_unique( - channel, *this, std::move(runtimeAgent)); + auto instanceAgent = + std::make_shared(channel, *this, sessionState); + instanceAgent->setCurrentRuntime( + currentRuntime_.has_value() ? &*currentRuntime_ : nullptr); + agents_.push_back(instanceAgent); + return instanceAgent; +} + +RuntimeTarget& InstanceTarget::registerRuntime( + RuntimeTargetDelegate& delegate) { + assert(!currentRuntime_ && "Only one Runtime allowed"); + currentRuntime_.emplace(delegate); + forEachAgent([currentRuntime = &*currentRuntime_](InstanceAgent& agent) { + agent.setCurrentRuntime(currentRuntime); + }); + return *currentRuntime_; +} + +void InstanceTarget::unregisterRuntime(RuntimeTarget& Runtime) { + assert( + currentRuntime_.has_value() && ¤tRuntime_.value() == &Runtime && + "Invalid unregistration"); + forEachAgent([](InstanceAgent& agent) { agent.setCurrentRuntime(nullptr); }); + currentRuntime_.reset(); } } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h index 07e3f35c6d1..ae2ed58567a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h @@ -7,6 +7,7 @@ #pragma once +#include "RuntimeTarget.h" #include "SessionState.h" #include @@ -33,18 +34,6 @@ class InstanceTargetDelegate { InstanceTargetDelegate& operator=(const InstanceTargetDelegate&) = delete; InstanceTargetDelegate& operator=(InstanceTargetDelegate&&) = default; - /** - * Create a new RuntimeAgent that can be used to debug the underlying JS VM. - * The agent will be destroyed when the session ends or the InstanceTarget is - * unregistered from its PageTarget (whichever happens first). - * \param channel A thread-safe channel for sending CDP messages to the - * frontend. - * \returns The new agent, or nullptr if the target does not support JS - * debugging. - */ - virtual std::unique_ptr createRuntimeAgent( - FrontendChannel channel, - SessionState& sessionState) = 0; virtual ~InstanceTargetDelegate(); }; @@ -65,12 +54,35 @@ class InstanceTarget { InstanceTarget& operator=(const InstanceTarget&) = delete; InstanceTarget& operator=(InstanceTarget&&) = delete; - std::unique_ptr createAgent( + std::shared_ptr createAgent( FrontendChannel channel, SessionState& sessionState); + RuntimeTarget& registerRuntime(RuntimeTargetDelegate& delegate); + void unregisterRuntime(RuntimeTarget& runtime); + private: InstanceTargetDelegate& delegate_; + std::optional currentRuntime_{std::nullopt}; + std::list> agents_; + + /** + * Call the given function for every active agent, and clean up any + * references to inactive agents. + */ + template + void forEachAgent(Fn&& fn) { + for (auto it = agents_.begin(); it != agents_.end();) { + if (auto agent = it->lock()) { + fn(*agent); + ++it; + } else { + it = agents_.erase(it); + } + } + } + + void removeExpiredAgents(); }; } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.cpp index fdaa060f895..92eea3a2124 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.cpp @@ -128,7 +128,7 @@ void PageAgent::sendInfoLogEntry(std::string_view text) { } void PageAgent::setCurrentInstanceAgent( - std::unique_ptr instanceAgent) { + std::shared_ptr instanceAgent) { auto previousInstanceAgent = std::move(instanceAgent_); instanceAgent_ = std::move(instanceAgent); if (!sessionState_.isRuntimeDomainEnabled) { diff --git a/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h index f954f76d83b..a9d3b2f3d0b 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/PageAgent.h @@ -60,7 +60,7 @@ class PageAgent { * \param agent The new InstanceAgent. May be null to signify that there is * currently no active instance. */ - void setCurrentInstanceAgent(std::unique_ptr agent); + void setCurrentInstanceAgent(std::shared_ptr agent); private: /** @@ -77,7 +77,7 @@ class PageAgent { FrontendChannel frontendChannel_; PageTargetController& targetController_; const PageTarget::SessionMetadata sessionMetadata_; - std::unique_ptr instanceAgent_; + std::shared_ptr instanceAgent_; /** * A shared reference to the session's state. This is only safe to access diff --git a/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h b/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h index 1f405b5f62f..a5775819ed2 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/ReactCdp.h @@ -10,5 +10,5 @@ #include #include #include -#include +#include #include diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp new file mode 100644 index 00000000000..98364f52519 --- /dev/null +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp @@ -0,0 +1,20 @@ +/* + * 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. + */ + +#include + +namespace facebook::react::jsinspector_modern { +RuntimeTarget::RuntimeTarget(RuntimeTargetDelegate& delegate) + : delegate_(delegate) {} + +std::unique_ptr RuntimeTarget::createAgent( + FrontendChannel channel, + SessionState& 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 new file mode 100644 index 00000000000..ec0853b7cb9 --- /dev/null +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h @@ -0,0 +1,77 @@ +/* + * 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 "RuntimeAgent.h" +#include "SessionState.h" + +#include + +#ifndef JSINSPECTOR_EXPORT +#ifdef _MSC_VER +#ifdef CREATE_SHARED_LIBRARY +#define JSINSPECTOR_EXPORT __declspec(dllexport) +#else +#define JSINSPECTOR_EXPORT +#endif // CREATE_SHARED_LIBRARY +#else // _MSC_VER +#define JSINSPECTOR_EXPORT __attribute__((visibility("default"))) +#endif // _MSC_VER +#endif // !defined(JSINSPECTOR_EXPORT) + +namespace facebook::react::jsinspector_modern { + +/** + * Receives events from a RuntimeTarget. This is a shared interface that + * each React Native platform needs to implement in order to integrate with + * the debugging stack. + */ +class RuntimeTargetDelegate { + public: + virtual ~RuntimeTargetDelegate() = default; + virtual std::unique_ptr createAgent( + FrontendChannel channel, + SessionState& sessionState) = 0; +}; + +/** + * A Target corresponding to a JavaScript runtime. + */ +class JSINSPECTOR_EXPORT RuntimeTarget { + public: + /** + * \param delegate The object that will receive events from this target. + * The caller is responsible for ensuring that the delegate outlives this + * object. + */ + explicit RuntimeTarget(RuntimeTargetDelegate& delegate); + + RuntimeTarget(const RuntimeTarget&) = delete; + RuntimeTarget(RuntimeTarget&&) = delete; + RuntimeTarget& operator=(const RuntimeTarget&) = delete; + RuntimeTarget& operator=(RuntimeTarget&&) = delete; + + /** + * Create a new RuntimeAgent that can be used to debug the underlying JS VM. + * The agent will be destroyed when the session ends, the containing + * InstanceTarget is unregistered from its PageTarget, or the RuntimeAgent is + * unregistered from its InstanceTarget (whichever happens first). + * \param channel A thread-safe channel for sending CDP messages to the + * frontend. + * \returns The new agent, or nullptr if the runtime is not debuggable. + */ + std::unique_ptr createAgent( + FrontendChannel channel, + SessionState& sessionState); + + private: + RuntimeTargetDelegate& delegate_; +}; + +} // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h index 8c071b08078..c0e4596bf34 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h @@ -121,12 +121,14 @@ class MockPageTargetDelegate : public PageTargetDelegate { MOCK_METHOD(void, onReload, (const PageReloadRequest& request), (override)); }; -class MockInstanceTargetDelegate : public InstanceTargetDelegate { +class MockInstanceTargetDelegate : public InstanceTargetDelegate {}; + +class MockRuntimeTargetDelegate : public RuntimeTargetDelegate { public: - // InstanceTargetDelegate methods + // RuntimeTargetDelegate methods MOCK_METHOD( std::unique_ptr, - createRuntimeAgent, + createAgent, (FrontendChannel channel, SessionState& sessionState), (override)); }; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp index 120230ea07f..8a03588181e 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp @@ -28,7 +28,7 @@ namespace { class PageTargetTest : public Test { protected: PageTargetTest() { - EXPECT_CALL(instanceTargetDelegate_, createRuntimeAgent(_, _)) + EXPECT_CALL(runtimeTargetDelegate_, createAgent(_, _)) .WillRepeatedly( runtimeAgents_ .lazily_make_unique()); @@ -55,6 +55,7 @@ class PageTargetTest : public Test { PageTarget page_{pageTargetDelegate_}; MockInstanceTargetDelegate instanceTargetDelegate_; + MockRuntimeTargetDelegate runtimeTargetDelegate_; UniquePtrFactory> runtimeAgents_; @@ -233,11 +234,6 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredInstanceWithEvents) { InSequence s; - EXPECT_CALL(*runtimeAgents_[0], handleRequest(Eq(cdp::preparse(R"({ - "id": 1, - "method": "Runtime.enable" - })")))) - .RetiresOnSaturation(); EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({ "id": 1, "result": {} @@ -254,56 +250,14 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredInstanceWithEvents) { page_.unregisterInstance(instanceTarget); } -TEST_F(PageTargetProtocolTest, RuntimeAgentLifecycle) { - { - auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); - - EXPECT_TRUE(runtimeAgents_[0]); - - page_.unregisterInstance(instanceTarget); - } - - EXPECT_FALSE(runtimeAgents_[0]); - - { - auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); - - EXPECT_TRUE(runtimeAgents_[1]); - - page_.unregisterInstance(instanceTarget); - } - - EXPECT_FALSE(runtimeAgents_[1]); -} - -TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgent) { - InSequence s; - +TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) { auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); - ASSERT_TRUE(runtimeAgents_[0]); - EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) - .WillOnce(Return(false)) - .RetiresOnSaturation(); - EXPECT_CALL( - fromPage(), onMessage(JsonParsed(AtJsonPtr("/error/code", Eq(-32601))))) - .RetiresOnSaturation(); - toPage_->sendMessage(R"({ - "id": 1, - "method": "CustomRuntimeDomain.Foo", - "params": { - "expression": "42" - } - })"); + connect(); - page_.unregisterInstance(instanceTarget); -} - -TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgent) { InSequence s; - auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); - ASSERT_TRUE(runtimeAgents_[0]); EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) .WillOnce(Return(true)) @@ -326,6 +280,92 @@ TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgent) { .RetiresOnSaturation(); runtimeAgents_[0]->frontendChannel(kFooResponse); + instanceTarget.unregisterRuntime(runtimeTarget); + page_.unregisterInstance(instanceTarget); +} + +TEST_F(PageTargetProtocolTest, RuntimeAgentLifecycle) { + { + auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = + instanceTarget.registerRuntime(runtimeTargetDelegate_); + + EXPECT_TRUE(runtimeAgents_[0]); + + instanceTarget.unregisterRuntime(runtimeTarget); + page_.unregisterInstance(instanceTarget); + } + + EXPECT_FALSE(runtimeAgents_[0]); + + { + auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = + instanceTarget.registerRuntime(runtimeTargetDelegate_); + + EXPECT_TRUE(runtimeAgents_[1]); + + instanceTarget.unregisterRuntime(runtimeTarget); + page_.unregisterInstance(instanceTarget); + } + + EXPECT_FALSE(runtimeAgents_[1]); +} + +TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgent) { + InSequence s; + + auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); + + ASSERT_TRUE(runtimeAgents_[0]); + EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + EXPECT_CALL( + fromPage(), onMessage(JsonParsed(AtJsonPtr("/error/code", Eq(-32601))))) + .RetiresOnSaturation(); + toPage_->sendMessage(R"({ + "id": 1, + "method": "CustomRuntimeDomain.Foo", + "params": { + "expression": "42" + } + })"); + + instanceTarget.unregisterRuntime(runtimeTarget); + page_.unregisterInstance(instanceTarget); +} + +TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgent) { + InSequence s; + + auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); + + ASSERT_TRUE(runtimeAgents_[0]); + EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) + .WillOnce(Return(true)) + .RetiresOnSaturation(); + toPage_->sendMessage(R"({ + "id": 1, + "method": "CustomRuntimeDomain.Foo", + "params": { + "expression": "42" + } + })"); + + static constexpr auto kFooResponse = R"({ + "id": 1, + "result": { + "fooValue": 42 + } + })"; + EXPECT_CALL(fromPage(), onMessage(JsonEq(kFooResponse))) + .RetiresOnSaturation(); + runtimeAgents_[0]->frontendChannel(kFooResponse); + + instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } @@ -344,6 +384,7 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { })"); auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); ASSERT_TRUE(runtimeAgents_[0]); EXPECT_CALL(*runtimeAgents_[0], handleRequest(_)) @@ -367,6 +408,7 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { .RetiresOnSaturation(); runtimeAgents_[0]->frontendChannel(kFooResponse); + instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); EXPECT_FALSE(runtimeAgents_[0]); @@ -386,10 +428,11 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgent) { TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgent) { InSequence s; - EXPECT_CALL(instanceTargetDelegate_, createRuntimeAgent(_, _)) + EXPECT_CALL(runtimeTargetDelegate_, createAgent(_, _)) .WillRepeatedly(ReturnNull()); auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_); EXPECT_FALSE(runtimeAgents_[0]); @@ -404,6 +447,7 @@ TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgent) { } })"); + instanceTarget.unregisterRuntime(runtimeTarget); page_.unregisterInstance(instanceTarget); } @@ -421,7 +465,8 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentHasAccessToSessionState) { "method": "Runtime.enable" })"); - page_.registerInstance(instanceTargetDelegate_); + auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_); + instanceTarget.registerRuntime(runtimeTargetDelegate_); ASSERT_TRUE(runtimeAgents_[0]); EXPECT_TRUE(runtimeAgents_[0]->sessionState.isRuntimeDomainEnabled); diff --git a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp index 92f2ac42690..bd90687c4c3 100644 --- a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp +++ b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp @@ -18,8 +18,7 @@ JSIRuntimeHolder::JSIRuntimeHolder(std::unique_ptr runtime) assert(runtime_ != nullptr); } -std::unique_ptr -JSIRuntimeHolder::createInspectorAgent( +std::unique_ptr JSIRuntimeHolder::createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) { return std::make_unique( diff --git a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h index 61903d939ce..ce8b50afb3f 100644 --- a/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h +++ b/packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.h @@ -17,20 +17,10 @@ namespace facebook::react { /** * An interface that represents an instance of a JS VM */ -class JSRuntime { +class JSRuntime : public jsinspector_modern::RuntimeTargetDelegate { public: virtual jsi::Runtime& getRuntime() noexcept = 0; - /** - * Creates a new inspector agent for this runtime, if the runtime is - * inspectable. Returns nullptr otherwise. - * \see InspectorTargetDelegate::createRuntimeAgent - */ - virtual std::unique_ptr - createInspectorAgent( - jsinspector_modern::FrontendChannel frontendChannel, - jsinspector_modern::SessionState& sessionState) = 0; - virtual ~JSRuntime() = default; }; @@ -51,7 +41,7 @@ class JSRuntimeFactory { class JSIRuntimeHolder : public JSRuntime { public: jsi::Runtime& getRuntime() noexcept override; - std::unique_ptr createInspectorAgent( + std::unique_ptr createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override; diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 711e7e2c120..0f5fca84bab 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -39,6 +39,7 @@ ReactInstance::ReactInstance( parentInspectorTarget_(parentInspectorTarget) { if (parentInspectorTarget_) { inspectorTarget_ = &parentInspectorTarget_->registerInstance(*this); + runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(*runtime_); } auto runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), weakTimerManager = @@ -102,6 +103,8 @@ ReactInstance::ReactInstance( void ReactInstance::unregisterFromInspector() { if (inspectorTarget_) { + assert(runtimeInspectorTarget_); + inspectorTarget_->unregisterRuntime(*runtimeInspectorTarget_); assert(parentInspectorTarget_); parentInspectorTarget_->unregisterInstance(*inspectorTarget_); inspectorTarget_ = nullptr; @@ -460,12 +463,4 @@ void ReactInstance::handleMemoryPressureJs(int pressureLevel) { } } -std::unique_ptr -ReactInstance::createRuntimeAgent( - jsinspector_modern::FrontendChannel channel, - jsinspector_modern::SessionState& sessionState) { - auto agent = runtime_->createInspectorAgent(std::move(channel), sessionState); - return agent; -} - } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h index 8c28a27c9d0..50a4f17a1ed 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h @@ -72,10 +72,6 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate { void unregisterFromInspector(); private: - std::unique_ptr createRuntimeAgent( - jsinspector_modern::FrontendChannel channel, - jsinspector_modern::SessionState& sessionState) override; - std::shared_ptr runtime_; std::shared_ptr jsMessageQueueThread_; std::shared_ptr bufferedRuntimeExecutor_; @@ -88,6 +84,7 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate { std::shared_ptr hasFatalJsError_; jsinspector_modern::InstanceTarget* inspectorTarget_{nullptr}; + jsinspector_modern::RuntimeTarget* runtimeInspectorTarget_{nullptr}; jsinspector_modern::PageTarget* parentInspectorTarget_{nullptr}; }; diff --git a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp index 782ab9ab8c7..289cef1e170 100644 --- a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp @@ -104,7 +104,7 @@ class HermesJSRuntime : public JSRuntime { return *runtime_; } - std::unique_ptr createInspectorAgent( + std::unique_ptr createAgent( jsinspector_modern::FrontendChannel frontendChannel, jsinspector_modern::SessionState& sessionState) override { return std::unique_ptr(