diff --git a/ReactCommon/hermes/inspector/Inspector.cpp b/ReactCommon/hermes/inspector/Inspector.cpp index 60a31bef749..c2b58712119 100644 --- a/ReactCommon/hermes/inspector/Inspector.cpp +++ b/ReactCommon/hermes/inspector/Inspector.cpp @@ -120,6 +120,7 @@ Inspector::Inspector( std::lock_guard lock(mutex_); if (pauseOnFirstStatement) { + awaitingDebuggerOnStart_ = true; TRANSITION(std::make_unique(*this)); } else { TRANSITION(std::make_unique(*this)); @@ -679,6 +680,10 @@ bool Inspector::isExecutingSupersededFile() { return false; } +bool Inspector::isAwaitingDebuggerOnStart() { + return awaitingDebuggerOnStart_; +} + } // namespace inspector } // namespace hermes } // namespace facebook diff --git a/ReactCommon/hermes/inspector/Inspector.h b/ReactCommon/hermes/inspector/Inspector.h index ad77ba54467..ac00e670e74 100644 --- a/ReactCommon/hermes/inspector/Inspector.h +++ b/ReactCommon/hermes/inspector/Inspector.h @@ -229,6 +229,16 @@ class Inspector : public facebook::hermes::debugger::EventObserver, facebook::hermes::debugger::Debugger &debugger, facebook::hermes::debugger::BreakpointID breakpointId) override; + /** + * Get whether we started with pauseOnFirstStatement, and have not yet had a + * debugger attach and ask to resume from that point. This matches the + * semantics of when CDP Debugger.runIfWaitingForDebugger should resume. + * + * It's not named "isPausedOnStart" because the VM and inspector is not + * necessarily paused; we could be in a RunningWaitPause state. + */ + bool isAwaitingDebuggerOnStart(); + private: friend class InspectorState; @@ -334,6 +344,10 @@ class Inspector : public facebook::hermes::debugger::EventObserver, // Trigger a fake console.log if we're currently in a superseded file. void alertIfPausedInSupersededFile(); + + // Are we currently waiting for a debugger to attach, because we + // requested 'pauseOnFirstStatement'? + bool awaitingDebuggerOnStart_; }; } // namespace inspector diff --git a/ReactCommon/hermes/inspector/InspectorState.cpp b/ReactCommon/hermes/inspector/InspectorState.cpp index 7d6c35e05bb..38042872472 100644 --- a/ReactCommon/hermes/inspector/InspectorState.cpp +++ b/ReactCommon/hermes/inspector/InspectorState.cpp @@ -51,6 +51,10 @@ std::pair InspectorState::RunningDetached::didPause( nullptr, makeContinueCommand()); } +void InspectorState::RunningDetached::onEnter(InspectorState *previous) { + inspector_.awaitingDebuggerOnStart_ = false; +} + std::pair InspectorState::RunningDetached::enable() { return std::make_pair( InspectorState::Running::make(inspector_), true); @@ -172,6 +176,8 @@ void InspectorState::Running::onEnter(InspectorState *prevState) { inspector_.notifyScriptsLoaded(); } } + + inspector_.awaitingDebuggerOnStart_ = false; } void InspectorState::Running::detach( diff --git a/ReactCommon/hermes/inspector/InspectorState.h b/ReactCommon/hermes/inspector/InspectorState.h index 99cbff7e372..827f3149f43 100644 --- a/ReactCommon/hermes/inspector/InspectorState.h +++ b/ReactCommon/hermes/inspector/InspectorState.h @@ -52,7 +52,7 @@ class InspectorState { */ /** - * detach clears all debuger state and transitions to RunningDetached. + * detach clears all debugger state and transitions to RunningDetached. */ virtual void detach(std::shared_ptr> promise) { // As we're not attached we'd like for the operation to be idempotent @@ -186,6 +186,8 @@ class InspectorState::RunningDetached : public InspectorState { std::pair didPause(MonitorLock &lock) override; std::pair enable() override; + void onEnter(InspectorState *prevState) override; + bool isRunningDetached() const override { return true; } diff --git a/ReactCommon/hermes/inspector/chrome/Connection.cpp b/ReactCommon/hermes/inspector/chrome/Connection.cpp index 4915395d789..56851a0aa2d 100644 --- a/ReactCommon/hermes/inspector/chrome/Connection.cpp +++ b/ReactCommon/hermes/inspector/chrome/Connection.cpp @@ -94,6 +94,7 @@ class Connection::Impl : public inspector::InspectorObserver, const m::heapProfiler::StopTrackingHeapObjectsRequest &req) override; void handle(const m::runtime::EvaluateRequest &req) override; void handle(const m::runtime::GetPropertiesRequest &req) override; + void handle(const m::runtime::RunIfWaitingForDebuggerRequest &req) override; private: std::vector makePropsFromScope( @@ -312,6 +313,9 @@ void Connection::Impl::onPause( note.reason = "exception"; break; case debugger::PauseReason::ScriptLoaded: { + // This case covers both wait-for-debugger and instrumentation + // breakpoints, since both are implemented as pauses on script load. + note.reason = "other"; note.hitBreakpoints = std::vector(); @@ -325,7 +329,8 @@ void Connection::Impl::onPause( // in the extremely unlikely event that it did *and* did it exactly // between us 1. checking that we should stop, and 2. adding the stop // reason here, then just resume and skip sending a pause notification. - if (note.hitBreakpoints->empty()) { + if (!inspector_->isAwaitingDebuggerOnStart() && + note.hitBreakpoints->empty()) { sendNotification = false; inspector_->resume(); } @@ -866,6 +871,16 @@ void Connection::Impl::handle(const m::runtime::GetPropertiesRequest &req) { .thenError(sendErrorToClient(req.id)); } +void Connection::Impl::handle( + const m::runtime::RunIfWaitingForDebuggerRequest &req) { + if (inspector_->isAwaitingDebuggerOnStart()) { + sendResponseToClientViaExecutor(inspector_->resume(), req.id); + } else { + // We weren't awaiting a debugger. Just send an 'ok'. + sendResponseToClientViaExecutor(req.id); + } +} + /* * Send-to-client methods */ diff --git a/ReactCommon/hermes/inspector/chrome/MessageTypes.cpp b/ReactCommon/hermes/inspector/chrome/MessageTypes.cpp index d40c05e1472..e136e1c8f19 100644 --- a/ReactCommon/hermes/inspector/chrome/MessageTypes.cpp +++ b/ReactCommon/hermes/inspector/chrome/MessageTypes.cpp @@ -1,5 +1,5 @@ // Copyright 2004-present Facebook. All Rights Reserved. -// @generated SignedSource<<575de63c36edd2a9a0f191501fd4f662>> +// @generated SignedSource<> #include "MessageTypes.h" @@ -50,6 +50,8 @@ std::unique_ptr Request::fromJsonThrowOnError(const std::string &str) { makeUnique}, {"Runtime.evaluate", makeUnique}, {"Runtime.getProperties", makeUnique}, + {"Runtime.runIfWaitingForDebugger", + makeUnique}, }; dynamic obj = folly::parseJson(str); @@ -783,6 +785,28 @@ void runtime::GetPropertiesRequest::accept(RequestHandler &handler) const { handler.handle(*this); } +runtime::RunIfWaitingForDebuggerRequest::RunIfWaitingForDebuggerRequest() + : Request("Runtime.runIfWaitingForDebugger") {} + +runtime::RunIfWaitingForDebuggerRequest::RunIfWaitingForDebuggerRequest( + const dynamic &obj) + : Request("Runtime.runIfWaitingForDebugger") { + assign(id, obj, "id"); + assign(method, obj, "method"); +} + +dynamic runtime::RunIfWaitingForDebuggerRequest::toDynamic() const { + dynamic obj = dynamic::object; + put(obj, "id", id); + put(obj, "method", method); + return obj; +} + +void runtime::RunIfWaitingForDebuggerRequest::accept( + RequestHandler &handler) const { + handler.handle(*this); +} + /// Responses ErrorResponse::ErrorResponse(const dynamic &obj) { assign(id, obj, "id"); diff --git a/ReactCommon/hermes/inspector/chrome/MessageTypes.h b/ReactCommon/hermes/inspector/chrome/MessageTypes.h index 931e19fb41a..d7e38d81a49 100644 --- a/ReactCommon/hermes/inspector/chrome/MessageTypes.h +++ b/ReactCommon/hermes/inspector/chrome/MessageTypes.h @@ -1,5 +1,5 @@ // Copyright 2004-present Facebook. All Rights Reserved. -// @generated SignedSource<> +// @generated SignedSource<<356df52df2a053b5254f0e039cc36a7b>> #pragma once @@ -61,6 +61,7 @@ struct InternalPropertyDescriptor; struct PropertyDescriptor; struct RemoteObject; using RemoteObjectId = std::string; +struct RunIfWaitingForDebuggerRequest; using ScriptId = std::string; struct StackTrace; using Timestamp = double; @@ -101,6 +102,7 @@ struct RequestHandler { virtual void handle(const heapProfiler::TakeHeapSnapshotRequest &req) = 0; virtual void handle(const runtime::EvaluateRequest &req) = 0; virtual void handle(const runtime::GetPropertiesRequest &req) = 0; + virtual void handle(const runtime::RunIfWaitingForDebuggerRequest &req) = 0; }; /// NoopRequestHandler can be subclassed to only handle some requests. @@ -127,6 +129,7 @@ struct NoopRequestHandler : public RequestHandler { void handle(const heapProfiler::TakeHeapSnapshotRequest &req) override {} void handle(const runtime::EvaluateRequest &req) override {} void handle(const runtime::GetPropertiesRequest &req) override {} + void handle(const runtime::RunIfWaitingForDebuggerRequest &req) override {} }; /// Types @@ -455,6 +458,14 @@ struct runtime::GetPropertiesRequest : public Request { folly::Optional ownProperties; }; +struct runtime::RunIfWaitingForDebuggerRequest : public Request { + RunIfWaitingForDebuggerRequest(); + explicit RunIfWaitingForDebuggerRequest(const folly::dynamic &obj); + + folly::dynamic toDynamic() const override; + void accept(RequestHandler &handler) const override; +}; + /// Responses struct ErrorResponse : public Response { ErrorResponse() = default; diff --git a/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp b/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp index b55194a1869..9d5abc5962a 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp +++ b/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp @@ -44,7 +44,8 @@ namespace { // the already-deallocated connection. class TestContext { public: - TestContext() : conn_(runtime_.runtime()) {} + TestContext(bool waitForDebugger = false) + : conn_(runtime_.runtime(), waitForDebugger) {} ~TestContext() { runtime_.wait(); } @@ -85,6 +86,7 @@ NotificationType expectNotification(SyncConnection &conn) { note = NotificationType(folly::parseJson(str)); } catch (const std::exception &e) { parseError = e.what(); + parseError += " (json: " + str + ")"; } EXPECT_EQ(parseError, ""); @@ -2349,6 +2351,44 @@ TEST(ConnectionTests, wontStopOnFilesWithoutSourceMaps) { expectNotification(conn); } +TEST(ConnectionTests, runIfWaitingForDebugger) { + TestContext context(true); + AsyncHermesRuntime &asyncRuntime = context.runtime(); + SyncConnection &conn = context.conn(); + int msgId = 0; + + asyncRuntime.executeScriptAsync(R"( + storeValue(1); debugger; + )"); + + send(conn, ++msgId); + expectExecutionContextCreated(conn); + expectNotification(conn); + expectNotification(conn); + + // We should now be paused on load. Verify that we didn't run code. + ASSERT_FALSE(asyncRuntime.hasStoredValue()); + + // RunIfWaitingForDebugger should cause us to resume + send(conn, ++msgId); + expectNotification(conn); + + // We should immediately hit the 'debugger;' statement + expectNotification(conn); + EXPECT_EQ(1, asyncRuntime.awaitStoredValue().asNumber()); + + // RunIfWaitingForDebuggerResponse should be accepted but have no effect + send(conn, ++msgId); + + // Do a dummy call so we can expect something other than a ResumeRequest + sendRuntimeEvalRequest(conn, ++msgId, "true"); + expectEvalResponse(conn, msgId, true); + + // Finally explicitly continue and exit + send(conn, msgId++); + expectNotification(conn); +} + } // namespace chrome } // namespace inspector } // namespace hermes diff --git a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp index d30b044d216..0514db17fe9 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp +++ b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp @@ -52,12 +52,15 @@ class SyncConnection::RemoteConnnection : public IRemoteConnection { SyncConnection &conn_; }; -SyncConnection::SyncConnection(std::shared_ptr runtime) +SyncConnection::SyncConnection( + std::shared_ptr runtime, + bool waitForDebugger) : connection_( std::make_unique( runtime, runtime->getDebugger()), - "testConn") { + "testConn", + waitForDebugger) { connection_.connect(std::make_unique(*this)); } diff --git a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h index 40f97ee7d0d..97da2ec8657 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h +++ b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h @@ -28,7 +28,9 @@ namespace chrome { */ class SyncConnection { public: - SyncConnection(std::shared_ptr runtime); + SyncConnection( + std::shared_ptr runtime, + bool waitForDebugger = false); ~SyncConnection() = default; /// sends a message to the debugger diff --git a/ReactCommon/hermes/inspector/tools/message_types.txt b/ReactCommon/hermes/inspector/tools/message_types.txt index 0db4a6b7709..02efcce5f1b 100644 --- a/ReactCommon/hermes/inspector/tools/message_types.txt +++ b/ReactCommon/hermes/inspector/tools/message_types.txt @@ -24,3 +24,4 @@ Runtime.consoleAPICalled Runtime.evaluate Runtime.executionContextCreated Runtime.getProperties +Runtime.runIfWaitingForDebugger