Skip to content

Commit

Permalink
Stop sending execution context notification
Browse files Browse the repository at this point in the history
Summary:
RuntimeDomainAgent should not be emitting execution context
notification. We agreed the integrator should emit those notifications
instead.

Reviewed By: mattbfb

Differential Revision: D54134227

fbshipit-source-id: 110febb60b3921d419593c00d4adf2131b24d08b
  • Loading branch information
dannysu authored and facebook-github-bot committed Feb 23, 2024
1 parent 0c233be commit 686c65d
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 34 deletions.
4 changes: 0 additions & 4 deletions API/hermes/cdp/DomainAgent.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ namespace cdp {

namespace m = ::facebook::hermes::cdp::message;

/// Fixed execution context ID because Hermes doesn't currently support realms
/// or Web Workers.
static constexpr int32_t kHermesExecutionContextId = 1;

/// A wrapper around std::function<void(...)> to make it safe to use from
/// multiple threads. The wrapper implements an invalidate function so that one
/// thread can clean up the underlying std::function in a thread-safe way.
Expand Down
8 changes: 1 addition & 7 deletions API/hermes/cdp/RuntimeDomainAgent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,6 @@ void RuntimeDomainAgent::enable(const m::runtime::EnableRequest &req) {
// Enable
enabled_ = true;
sendResponseToClient(m::makeOkResponse(req.id));

// Notify the client about the hard-coded Hermes execution context.
m::runtime::ExecutionContextCreatedNotification note;
note.context.id = kHermesExecutionContextId;
note.context.name = "hermes";
sendNotificationToClient(note);
}

void RuntimeDomainAgent::disable(const m::runtime::DisableRequest &req) {
Expand Down Expand Up @@ -449,7 +443,7 @@ void RuntimeDomainAgent::callFunctionOn(
assert(
req.executionContextId &&
"should not be here if both object id and execution context id are missing");
if (*req.executionContextId != kHermesExecutionContextId) {
if (*req.executionContextId != executionContextID_) {
sendResponseToClient(m::makeErrorResponse(
req.id,
m::ErrorCode::InvalidRequest,
Expand Down
25 changes: 2 additions & 23 deletions unittests/API/CDPAgentTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1473,22 +1473,13 @@ TEST_F(CDPAgentTest, TestRuntimeEnable) {
// Verify enable gets an "OK" response
sendAndCheckResponse("Runtime.enable", msgId++);

// Verify the hard-coded execution context is announced.
auto note = expectNotification("Runtime.executionContextCreated");
EXPECT_EQ(
jsonScope_.getNumber(note, {"params", "context", "id"}),
kHermesExecutionContextId);
EXPECT_EQ(
jsonScope_.getString(note, {"params", "context", "name"}), "hermes");

// Verify disable gets an "OK" response
sendAndCheckResponse("Runtime.disable", msgId++);
}

TEST_F(CDPAgentTest, RefuseDoubleRuntimeEnable) {
int msgId = 1;
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");

// Verify enabling a second time fails
sendParameterlessRequest("Runtime.enable", msgId);
Expand All @@ -1515,7 +1506,6 @@ TEST_F(CDPAgentTest, GetHeapUsage) {
int msgId = 1;

sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");

scheduleScript(R"(
// Allocate some objects
Expand Down Expand Up @@ -1556,7 +1546,6 @@ TEST_F(CDPAgentTest, RuntimeGlobalLexicalScopeNames) {
int msgId = 1;

sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");

scheduleScript(R"(
// Declare some globals to get the names of
Expand Down Expand Up @@ -1593,7 +1582,7 @@ TEST_F(CDPAgentTest, RuntimeGlobalLexicalScopeNames) {
"Runtime.globalLexicalScopeNames",
msgId,
[](::hermes::JSONEmitter &json) {
json.emitKeyValue("executionContextId", kHermesExecutionContextId);
json.emitKeyValue("executionContextId", kTestExecutionContextId);
});

auto resp = expectResponse(std::nullopt, msgId++);
Expand All @@ -1615,7 +1604,6 @@ TEST_F(CDPAgentTest, RuntimeCompileScript) {
int msgId = 1;

sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");

// Compile a valid script
sendRequest("Runtime.compileScript", msgId, [](::hermes::JSONEmitter &json) {
Expand All @@ -1633,7 +1621,6 @@ TEST_F(CDPAgentTest, RuntimeCompileScriptParseError) {
int msgId = 1;

sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");

// Compile an invalid script
sendRequest("Runtime.compileScript", msgId, [](::hermes::JSONEmitter &json) {
Expand All @@ -1655,7 +1642,6 @@ TEST_F(CDPAgentTest, GetProperties) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
sendAndCheckResponse("Debugger.enable", msgId++);
scheduleScript(R"(
function foo() {
Expand Down Expand Up @@ -1752,7 +1738,6 @@ TEST_F(CDPAgentTest, GetPropertiesOnlyOwn) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
sendAndCheckResponse("Debugger.enable", msgId++);
scheduleScript(R"(
function foo() {
Expand Down Expand Up @@ -1811,7 +1796,6 @@ TEST_F(CDPAgentTest, RuntimeEvaluate) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
scheduleScript(R"(
var globalVar = "omega";
var booleanVar = true;
Expand Down Expand Up @@ -1881,7 +1865,6 @@ TEST_F(CDPAgentTest, RuntimeEvaluateWhilePaused) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
sendAndCheckResponse("Debugger.enable", msgId++);
scheduleScript(R"(
var inGlobalScope = 123;
Expand Down Expand Up @@ -1925,7 +1908,6 @@ TEST_F(CDPAgentTest, RuntimeEvaluateReturnByValue) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
scheduleScript(R"(while(!shouldStop());)");

// We expect this JSON object to be evaluated and return by value, so
Expand Down Expand Up @@ -1962,7 +1944,6 @@ TEST_F(CDPAgentTest, RuntimeEvaluateException) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
scheduleScript(R"(while(!shouldStop()) {})");

// Evaluate something that throws
Expand Down Expand Up @@ -2027,7 +2008,6 @@ TEST_F(CDPAgentTest, RuntimeCallFunctionOnObject) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
sendAndCheckResponse("Debugger.enable", msgId++);
scheduleScript(R"(debugger;)");
expectNotification("Debugger.scriptParsed");
Expand Down Expand Up @@ -2147,7 +2127,6 @@ TEST_F(CDPAgentTest, RuntimeCallFunctionOnExecutionContext) {

// Start a script
sendAndCheckResponse("Runtime.enable", msgId++);
ensureNotification(waitForMessage(), "Runtime.executionContextCreated");
sendAndCheckResponse("Debugger.enable", msgId++);
scheduleScript(R"(debugger;)");
expectNotification("Debugger.scriptParsed");
Expand Down Expand Up @@ -2210,7 +2189,7 @@ TEST_F(CDPAgentTest, RuntimeCallFunctionOnExecutionContext) {
// Don't have an easy way to copy these, so...
req.arguments = std::vector<m::runtime::CallArgument>{};
req.arguments->push_back(std::move(ca));
req.executionContextId = 1;
req.executionContextId = kTestExecutionContextId;

cdpAgent_->handleCommand(
serializeRuntimeCallFunctionOnRequest(std::move(req)));
Expand Down

0 comments on commit 686c65d

Please sign in to comment.