Skip to content

Commit eef87ca

Browse files
huntiefacebook-github-bot
huntie
authored and
facebook-github-bot
committed
Implement previouslyExportedState on HermesRuntimeAgentDelegateNew (facebook#43392)
Summary: Pull Request resolved: facebook#43392 ## Context We are migrating to the new Hermes `CDPAgent` and `CDPDebugAPI` APIs in the modern CDP server (previously `HermesCDPHandler`). ## This diff Wires up `previouslyExportedState` with `CDPAgent`, and re-enables the `ResolveBreakpointAfterReload` integration test. Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D54369985 fbshipit-source-id: 5dcb4fe59b8b36b2db9f0385e8487097822e5704
1 parent 55ed1c2 commit eef87ca

5 files changed

Lines changed: 51 additions & 22 deletions

File tree

‎.circleci/configurations/top_level.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ references:
9292
# Cocoapods - RNTester
9393
pods_cache_key: &pods_cache_key v11-pods-{{ .Environment.CIRCLE_JOB }}-{{ checksum "packages/rn-tester/Podfile.lock.bak" }}-{{ checksum "packages/rn-tester/Podfile" }}
9494
cocoapods_cache_key: &cocoapods_cache_key v11-cocoapods-{{ .Environment.CIRCLE_JOB }}-{{ checksum "packages/rn-tester/Podfile.lock" }}-{{ checksum "packages/rn-tester/Podfile" }}-{{ checksum "/tmp/hermes/hermesversion" }}
95-
rntester_podfile_lock_cache_key: &rntester_podfile_lock_cache_key v9-podfilelock-{{ .Environment.CIRCLE_JOB }}-{{ checksum "packages/rn-tester/Podfile" }}-{{ checksum "/tmp/week_year" }}-{{ checksum "/tmp/hermes/hermesversion" }}
95+
rntester_podfile_lock_cache_key: &rntester_podfile_lock_cache_key v10-podfilelock-{{ .Environment.CIRCLE_JOB }}-{{ checksum "packages/rn-tester/Podfile" }}-{{ checksum "/tmp/week_year" }}-{{ checksum "/tmp/hermes/hermesversion" }}
9696

9797
# Cocoapods - Template
9898
template_cocoapods_cache_key: &template_cocoapods_cache_key v6-cocoapods-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/iOSTemplateProject/ios/Podfile.lock" }}-{{ checksum "/tmp/iOSTemplateProject/ios/Podfile" }}-{{ checksum "/tmp/hermes/hermesversion" }}-{{ checksum "packages/rn-tester/Podfile.lock" }}

‎.github/workflows/ios-tests.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ jobs:
3636
uses: actions/cache@v3
3737
with:
3838
path: packages/rn-tester/Pods
39-
key: v2-${{ runner.os }}-RNTesterPods-${{ hashFiles('packages/rn-tester/Podfile.lock') }}-${{ hashFiles('packages/rn-tester/Podfile') }}-${{ hashFiles('tmp/hermes/hermesversion') }}
39+
key: v3-${{ runner.os }}-RNTesterPods-${{ hashFiles('packages/rn-tester/Podfile.lock') }}-${{ hashFiles('packages/rn-tester/Podfile') }}-${{ hashFiles('tmp/hermes/hermesversion') }}
4040
- name: Pod Install
4141
run: |
4242
cd packages/rn-tester

‎packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegateNew.cpp‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,25 @@ using namespace facebook::hermes;
2424
namespace facebook::react::jsinspector_modern {
2525

2626
class HermesRuntimeAgentDelegateNew::Impl final : public RuntimeAgentDelegate {
27+
using HermesState = hermes::cdp::State;
28+
29+
struct HermesStateWrapper : public ExportedState {
30+
explicit HermesStateWrapper(HermesState state) : state_(std::move(state)) {}
31+
32+
static HermesState unwrapDestructively(ExportedState* wrapper) {
33+
if (!wrapper) {
34+
return {};
35+
}
36+
if (auto* typedWrapper = dynamic_cast<HermesStateWrapper*>(wrapper)) {
37+
return std::move(typedWrapper->state_);
38+
}
39+
return {};
40+
}
41+
42+
private:
43+
HermesState state_;
44+
};
45+
2746
public:
2847
Impl(
2948
FrontendChannel frontendChannel,
@@ -44,24 +63,15 @@ class HermesRuntimeAgentDelegateNew::Impl final : public RuntimeAgentDelegate {
4463
runtimeExecutor(
4564
[&runtime, fn = std::move(fn)](auto&) { fn(runtime); });
4665
},
47-
std::move(frontendChannel))) {
48-
// TODO(T178858701): Pass previouslyExportedState to CDPAgent
49-
(void)previouslyExportedState;
50-
}
66+
std::move(frontendChannel),
67+
HermesStateWrapper::unwrapDestructively(
68+
previouslyExportedState.get()))) {}
5169

52-
/**
53-
* Handle a CDP request. The response will be sent over the provided
54-
* \c FrontendChannel synchronously or asynchronously.
55-
* \param req The parsed request.
56-
* \returns true if this agent has responded, or will respond asynchronously,
57-
* to the request (with either a success or error message). False if the
58-
* agent expects another agent to respond to the request instead.
59-
*/
6070
bool handleRequest(const cdp::PreparsedRequest& req) override {
6171
// TODO: Change to string::starts_with when we're on C++20.
6272
if (req.method.rfind("Log.", 0) == 0) {
63-
// Since we know Hermes doesn't do anything useful with Log messages, but
64-
// our containing PageAgent will, just bail out early.
73+
// Since we know Hermes doesn't do anything useful with Log messages,
74+
// but our containing HostAgent will, bail out early.
6575
// TODO: We need a way to negotiate this more dynamically with Hermes
6676
// through the API.
6777
return false;
@@ -73,6 +83,10 @@ class HermesRuntimeAgentDelegateNew::Impl final : public RuntimeAgentDelegate {
7383
return true;
7484
}
7585

86+
std::unique_ptr<ExportedState> getExportedState() override {
87+
return std::make_unique<HermesStateWrapper>(hermes_->getState());
88+
}
89+
7690
private:
7791
std::unique_ptr<hermes::cdp::CDPAgent> hermes_;
7892
};
@@ -100,6 +114,11 @@ bool HermesRuntimeAgentDelegateNew::handleRequest(
100114
return impl_->handleRequest(req);
101115
}
102116

117+
std::unique_ptr<RuntimeAgentDelegate::ExportedState>
118+
HermesRuntimeAgentDelegateNew::getExportedState() {
119+
return impl_->getExportedState();
120+
}
121+
103122
} // namespace facebook::react::jsinspector_modern
104123

105124
#endif // HERMES_ENABLE_DEBUGGER

‎packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegateNew.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ class HermesRuntimeAgentDelegateNew : public RuntimeAgentDelegate {
6666
*/
6767
bool handleRequest(const cdp::PreparsedRequest& req) override;
6868

69+
std::unique_ptr<RuntimeAgentDelegate::ExportedState> getExportedState()
70+
override;
71+
6972
private:
7073
class Impl;
7174

‎packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -559,28 +559,35 @@ TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpressionInExecutionContext) {
559559
std::to_string(executionContextId)));
560560
}
561561

562-
// TODO(T178858701): Restore breakpoint reload persistence under
563-
// HermesRuntimeAgentDelegateNew
564-
TYPED_TEST(JsiIntegrationHermesLegacyTest, ResolveBreakpointAfterReload) {
562+
TYPED_TEST(JsiIntegrationHermesTest, ResolveBreakpointAfterReload) {
565563
this->connect();
566564

567565
InSequence s;
568566

569-
this->expectMessageFromPage(JsonParsed(AtJsonPtr("/id", 1)));
567+
this->expectMessageFromPage(JsonEq(R"({
568+
"id": 1,
569+
"result": {}
570+
})"));
570571
this->toPage_->sendMessage(R"({
571572
"id": 1,
573+
"method": "Debugger.enable"
574+
})");
575+
576+
this->expectMessageFromPage(JsonParsed(AtJsonPtr("/id", 2)));
577+
this->toPage_->sendMessage(R"({
578+
"id": 2,
572579
"method": "Debugger.setBreakpointByUrl",
573580
"params": {"lineNumber": 2, "url": "breakpointTest.js"}
574581
})");
575582

576583
this->reload();
577584

578585
this->expectMessageFromPage(JsonEq(R"({
579-
"id": 2,
586+
"id": 3,
580587
"result": {}
581588
})"));
582589
this->toPage_->sendMessage(R"({
583-
"id": 2,
590+
"id": 3,
584591
"method": "Debugger.enable"
585592
})");
586593

0 commit comments

Comments
 (0)