Skip to content

Commit b3a7a13

Browse files
motiz88facebook-github-bot
authored andcommitted
RuntimeTarget refactor - Add RuntimeExecutor to RuntimeTarget
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`. ~~ * ~~Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`). ~~ * ~~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. *← This diff* * 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: D53266710 fbshipit-source-id: df3a181fcc8e033c37a7f4f430f23a29b326b56a
1 parent 496724f commit b3a7a13

File tree

9 files changed

+47
-21
lines changed

9 files changed

+47
-21
lines changed

packages/react-native/ReactCommon/cxxreact/Instance.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ void Instance::initializeBridge(
6767

6868
if (parentInspectorTarget) {
6969
inspectorTarget_ = &parentInspectorTarget->registerInstance(*this);
70+
RuntimeExecutor runtimeExecutorIfJsi = getRuntimeExecutor();
7071
runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(
71-
nativeToJsBridge_->getInspectorTargetDelegate());
72+
nativeToJsBridge_->getInspectorTargetDelegate(),
73+
runtimeExecutorIfJsi ? runtimeExecutorIfJsi : [](auto) {});
7274
}
7375

7476
/**

packages/react-native/ReactCommon/jsinspector-modern/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@ target_link_libraries(jsinspector
2222
folly_runtime
2323
glog
2424
react_featureflags
25+
runtimeexecutor
2526
)

packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,10 @@ InstanceTarget::~InstanceTarget() {
4646
}
4747

4848
RuntimeTarget& InstanceTarget::registerRuntime(
49-
RuntimeTargetDelegate& delegate) {
49+
RuntimeTargetDelegate& delegate,
50+
RuntimeExecutor executor) {
5051
assert(!currentRuntime_ && "Only one Runtime allowed");
51-
currentRuntime_.emplace(delegate);
52+
currentRuntime_.emplace(delegate, executor);
5253
forEachAgent([currentRuntime = &*currentRuntime_](InstanceAgent& agent) {
5354
agent.setCurrentRuntime(currentRuntime);
5455
});

packages/react-native/ReactCommon/jsinspector-modern/InstanceTarget.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ class InstanceTarget final {
5959
FrontendChannel channel,
6060
SessionState& sessionState);
6161

62-
RuntimeTarget& registerRuntime(RuntimeTargetDelegate& delegate);
62+
RuntimeTarget& registerRuntime(
63+
RuntimeTargetDelegate& delegate,
64+
RuntimeExecutor executor);
6365
void unregisterRuntime(RuntimeTarget& runtime);
6466

6567
private:

packages/react-native/ReactCommon/jsinspector-modern/React-jsinspector.podspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,5 @@ Pod::Spec.new do |s|
5151
s.dependency "RCT-Folly", folly_version
5252
s.dependency "React-featureflags"
5353
s.dependency "DoubleConversion"
54+
s.dependency "React-runtimeexecutor", version
5455
end

packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
#include <jsinspector-modern/RuntimeTarget.h>
99

1010
namespace facebook::react::jsinspector_modern {
11-
RuntimeTarget::RuntimeTarget(RuntimeTargetDelegate& delegate)
12-
: delegate_(delegate) {}
11+
12+
RuntimeTarget::RuntimeTarget(
13+
RuntimeTargetDelegate& delegate,
14+
RuntimeExecutor executor)
15+
: delegate_(delegate), executor_(executor) {}
1316

1417
std::shared_ptr<RuntimeAgent> RuntimeTarget::createAgent(
1518
FrontendChannel channel,

packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#pragma once
99

10+
#include <ReactCommon/RuntimeExecutor.h>
1011
#include "InspectorInterfaces.h"
1112
#include "RuntimeAgent.h"
1213
#include "SessionState.h"
@@ -53,8 +54,12 @@ class JSINSPECTOR_EXPORT RuntimeTarget final {
5354
* \param delegate The object that will receive events from this target.
5455
* The caller is responsible for ensuring that the delegate outlives this
5556
* object.
57+
* \param executor A RuntimeExecutor that can be used to schedule work on
58+
* the JS runtime's thread. The executor's queue should be empty when
59+
* RuntimeTarget is constructed (i.e. anything scheduled during the
60+
* constructor should be executed before any user code is run).
5661
*/
57-
explicit RuntimeTarget(RuntimeTargetDelegate& delegate);
62+
RuntimeTarget(RuntimeTargetDelegate& delegate, RuntimeExecutor executor);
5863

5964
RuntimeTarget(const RuntimeTarget&) = delete;
6065
RuntimeTarget(RuntimeTarget&&) = delete;
@@ -77,6 +82,7 @@ class JSINSPECTOR_EXPORT RuntimeTarget final {
7782

7883
private:
7984
RuntimeTargetDelegate& delegate_;
85+
RuntimeExecutor executor_;
8086
std::list<std::weak_ptr<RuntimeAgent>> agents_;
8187

8288
/**

packages/react-native/ReactCommon/jsinspector-modern/tests/PageTargetTest.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ class PageTargetTest : public Test {
5656

5757
MockInstanceTargetDelegate instanceTargetDelegate_;
5858
MockRuntimeTargetDelegate runtimeTargetDelegate_;
59+
// We don't have access to a jsi::Runtime in these tests, so just use an
60+
// executor that never runs the scheduled callbacks.
61+
RuntimeExecutor runtimeExecutor_ = [](auto) {};
5962

6063
UniquePtrFactory<StrictMock<MockRuntimeAgentDelegate>> runtimeAgentDelegates_;
6164

@@ -252,7 +255,8 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredInstanceWithEvents) {
252255

253256
TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) {
254257
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
255-
auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_);
258+
auto& runtimeTarget =
259+
instanceTarget.registerRuntime(runtimeTargetDelegate_, runtimeExecutor_);
256260

257261
connect();
258262

@@ -287,8 +291,8 @@ TEST_F(PageTargetTest, ConnectToAlreadyRegisteredRuntimeWithEvents) {
287291
TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateLifecycle) {
288292
{
289293
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
290-
auto& runtimeTarget =
291-
instanceTarget.registerRuntime(runtimeTargetDelegate_);
294+
auto& runtimeTarget = instanceTarget.registerRuntime(
295+
runtimeTargetDelegate_, runtimeExecutor_);
292296

293297
EXPECT_TRUE(runtimeAgentDelegates_[0]);
294298

@@ -300,8 +304,8 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateLifecycle) {
300304

301305
{
302306
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
303-
auto& runtimeTarget =
304-
instanceTarget.registerRuntime(runtimeTargetDelegate_);
307+
auto& runtimeTarget = instanceTarget.registerRuntime(
308+
runtimeTargetDelegate_, runtimeExecutor_);
305309

306310
EXPECT_TRUE(runtimeAgentDelegates_[1]);
307311

@@ -316,7 +320,8 @@ TEST_F(PageTargetProtocolTest, MethodNotHandledByRuntimeAgentDelegate) {
316320
InSequence s;
317321

318322
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
319-
auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_);
323+
auto& runtimeTarget =
324+
instanceTarget.registerRuntime(runtimeTargetDelegate_, runtimeExecutor_);
320325

321326
ASSERT_TRUE(runtimeAgentDelegates_[0]);
322327
EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_))
@@ -341,7 +346,8 @@ TEST_F(PageTargetProtocolTest, MethodHandledByRuntimeAgentDelegate) {
341346
InSequence s;
342347

343348
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
344-
auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_);
349+
auto& runtimeTarget =
350+
instanceTarget.registerRuntime(runtimeTargetDelegate_, runtimeExecutor_);
345351

346352
ASSERT_TRUE(runtimeAgentDelegates_[0]);
347353
EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_))
@@ -384,7 +390,8 @@ TEST_F(PageTargetProtocolTest, MessageRoutingWhileNoRuntimeAgentDelegate) {
384390
})");
385391

386392
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
387-
auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_);
393+
auto& runtimeTarget =
394+
instanceTarget.registerRuntime(runtimeTargetDelegate_, runtimeExecutor_);
388395

389396
ASSERT_TRUE(runtimeAgentDelegates_[0]);
390397
EXPECT_CALL(*runtimeAgentDelegates_[0], handleRequest(_))
@@ -432,7 +439,8 @@ TEST_F(PageTargetProtocolTest, InstanceWithNullRuntimeAgentDelegate) {
432439
.WillRepeatedly(ReturnNull());
433440

434441
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
435-
auto& runtimeTarget = instanceTarget.registerRuntime(runtimeTargetDelegate_);
442+
auto& runtimeTarget =
443+
instanceTarget.registerRuntime(runtimeTargetDelegate_, runtimeExecutor_);
436444

437445
EXPECT_FALSE(runtimeAgentDelegates_[0]);
438446

@@ -466,7 +474,7 @@ TEST_F(PageTargetProtocolTest, RuntimeAgentDelegateHasAccessToSessionState) {
466474
})");
467475

468476
auto& instanceTarget = page_.registerInstance(instanceTargetDelegate_);
469-
instanceTarget.registerRuntime(runtimeTargetDelegate_);
477+
instanceTarget.registerRuntime(runtimeTargetDelegate_, runtimeExecutor_);
470478
ASSERT_TRUE(runtimeAgentDelegates_[0]);
471479

472480
EXPECT_TRUE(runtimeAgentDelegates_[0]->sessionState.isRuntimeDomainEnabled);

packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ ReactInstance::ReactInstance(
3737
jsErrorHandler_(jsErrorHandlingFunc),
3838
hasFatalJsError_(std::make_shared<bool>(false)),
3939
parentInspectorTarget_(parentInspectorTarget) {
40-
if (parentInspectorTarget_) {
41-
inspectorTarget_ = &parentInspectorTarget_->registerInstance(*this);
42-
runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(*runtime_);
43-
}
4440
auto runtimeExecutor = [weakRuntime = std::weak_ptr<JSRuntime>(runtime_),
4541
weakTimerManager =
4642
std::weak_ptr<TimerManager>(timerManager_),
@@ -88,6 +84,12 @@ ReactInstance::ReactInstance(
8884
}
8985
};
9086

87+
if (parentInspectorTarget_) {
88+
inspectorTarget_ = &parentInspectorTarget_->registerInstance(*this);
89+
runtimeInspectorTarget_ =
90+
&inspectorTarget_->registerRuntime(*runtime_, runtimeExecutor);
91+
}
92+
9193
runtimeScheduler_ =
9294
std::make_shared<RuntimeScheduler>(std::move(runtimeExecutor));
9395

0 commit comments

Comments
 (0)