diff --git a/ReactCommon/hermes/inspector/Inspector.cpp b/ReactCommon/hermes/inspector/Inspector.cpp index 14cb76cad3f..21b7bd2108d 100644 --- a/ReactCommon/hermes/inspector/Inspector.cpp +++ b/ReactCommon/hermes/inspector/Inspector.cpp @@ -351,6 +351,9 @@ folly::Future Inspector::setBreakpoint( debugger::SourceLocation loc, folly::Optional condition) { auto promise = std::make_shared>(); + // Automatically re-enable breakpoints since the user presumably wants this + // to start triggering. + breakpointsActive_ = true; executor_->add([this, loc, condition, promise] { setBreakpointOnExecutor(loc, condition, promise); @@ -453,6 +456,14 @@ folly::Future Inspector::setPauseOnLoads( return promise->getFuture(); }; +folly::Future Inspector::setBreakpointsActive(bool active) { + // Same logic as setPauseOnLoads. + auto promise = std::make_shared>(); + breakpointsActive_ = active; + promise->setValue(); + return promise->getFuture(); +}; + bool Inspector::shouldPauseOnThisScriptLoad() { switch (pauseOnLoadMode_) { case None: diff --git a/ReactCommon/hermes/inspector/Inspector.h b/ReactCommon/hermes/inspector/Inspector.h index ac00e670e74..5d98588286d 100644 --- a/ReactCommon/hermes/inspector/Inspector.h +++ b/ReactCommon/hermes/inspector/Inspector.h @@ -208,6 +208,12 @@ class Inspector : public facebook::hermes::debugger::EventObserver, */ folly::Future setPauseOnLoads(const PauseOnLoadMode mode); + /** + * Set whether breakpoints are active (pause when hit). This does not require + * runtime modifications, but returns a future for consistency. + */ + folly::Future setBreakpointsActive(bool active); + /** * If called during a script load event, return true if we should pause. * Assumed to be called from a script load event where we already hold @@ -326,6 +332,9 @@ class Inspector : public facebook::hermes::debugger::EventObserver, // Whether we should enter a paused state when a script loads. PauseOnLoadMode pauseOnLoadMode_ = PauseOnLoadMode::None; + // Whether or not we should pause on breakpoints. + bool breakpointsActive_ = true; + // All scripts loaded in to the VM, along with whether we've notified the // client about the script yet. struct LoadedScriptInfo { diff --git a/ReactCommon/hermes/inspector/InspectorState.cpp b/ReactCommon/hermes/inspector/InspectorState.cpp index 38042872472..07e005ede5b 100644 --- a/ReactCommon/hermes/inspector/InspectorState.cpp +++ b/ReactCommon/hermes/inspector/InspectorState.cpp @@ -254,6 +254,12 @@ std::pair InspectorState::Running::didPause( pendingEvalPromise_->setValue( inspector_.debugger_.getProgramState().getEvalResult()); pendingEvalPromise_.reset(); + } else if ( + reason == debugger::PauseReason::Breakpoint && + !inspector_.breakpointsActive_) { + // We hit a user defined breakpoint, but breakpoints have been deactivated. + return std::make_pair( + nullptr, makeContinueCommand()); } else /* other cases imply a transition to Pause */ { return std::make_pair( InspectorState::Paused::make(inspector_), nullptr); diff --git a/ReactCommon/hermes/inspector/chrome/Connection.cpp b/ReactCommon/hermes/inspector/chrome/Connection.cpp index 43dacde5bf0..e8653e592c1 100644 --- a/ReactCommon/hermes/inspector/chrome/Connection.cpp +++ b/ReactCommon/hermes/inspector/chrome/Connection.cpp @@ -81,6 +81,7 @@ class Connection::Impl : public inspector::InspectorObserver, void handle(const m::debugger::ResumeRequest &req) override; void handle(const m::debugger::SetBreakpointRequest &req) override; void handle(const m::debugger::SetBreakpointByUrlRequest &req) override; + void handle(const m::debugger::SetBreakpointsActiveRequest &req) override; void handle( const m::debugger::SetInstrumentationBreakpointRequest &req) override; void handle(const m::debugger::SetPauseOnExceptionsRequest &req) override; @@ -659,6 +660,16 @@ void Connection::Impl::handle( .thenError(sendErrorToClient(req.id)); } +void Connection::Impl::handle( + const m::debugger::SetBreakpointsActiveRequest &req) { + inspector_->setBreakpointsActive(req.active) + .via(executor_.get()) + .thenValue([this, id = req.id](const Unit &unit) { + sendResponseToClient(m::makeOkResponse(id)); + }) + .thenError(sendErrorToClient(req.id)); +} + bool Connection::Impl::isVirtualBreakpointId(const std::string &id) { return id.rfind(kVirtualBreakpointPrefix, 0) == 0; } diff --git a/ReactCommon/hermes/inspector/chrome/MessageTypes.cpp b/ReactCommon/hermes/inspector/chrome/MessageTypes.cpp index e136e1c8f19..fa97e98ed87 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<> +// @generated SignedSource<> #include "MessageTypes.h" @@ -35,6 +35,8 @@ std::unique_ptr Request::fromJsonThrowOnError(const std::string &str) { {"Debugger.setBreakpoint", makeUnique}, {"Debugger.setBreakpointByUrl", makeUnique}, + {"Debugger.setBreakpointsActive", + makeUnique}, {"Debugger.setInstrumentationBreakpoint", makeUnique}, {"Debugger.setPauseOnExceptions", @@ -509,6 +511,35 @@ void debugger::SetBreakpointByUrlRequest::accept( handler.handle(*this); } +debugger::SetBreakpointsActiveRequest::SetBreakpointsActiveRequest() + : Request("Debugger.setBreakpointsActive") {} + +debugger::SetBreakpointsActiveRequest::SetBreakpointsActiveRequest( + const dynamic &obj) + : Request("Debugger.setBreakpointsActive") { + assign(id, obj, "id"); + assign(method, obj, "method"); + + dynamic params = obj.at("params"); + assign(active, params, "active"); +} + +dynamic debugger::SetBreakpointsActiveRequest::toDynamic() const { + dynamic params = dynamic::object; + put(params, "active", active); + + dynamic obj = dynamic::object; + put(obj, "id", id); + put(obj, "method", method); + put(obj, "params", std::move(params)); + return obj; +} + +void debugger::SetBreakpointsActiveRequest::accept( + RequestHandler &handler) const { + handler.handle(*this); +} + debugger::SetInstrumentationBreakpointRequest:: SetInstrumentationBreakpointRequest() : Request("Debugger.setInstrumentationBreakpoint") {} diff --git a/ReactCommon/hermes/inspector/chrome/MessageTypes.h b/ReactCommon/hermes/inspector/chrome/MessageTypes.h index d7e38d81a49..fdb6270f11a 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<<356df52df2a053b5254f0e039cc36a7b>> +// @generated SignedSource<<0563169b47d73a70d7540528f28d1d13>> #pragma once @@ -38,6 +38,7 @@ struct SetBreakpointByUrlRequest; struct SetBreakpointByUrlResponse; struct SetBreakpointRequest; struct SetBreakpointResponse; +struct SetBreakpointsActiveRequest; struct SetInstrumentationBreakpointRequest; struct SetInstrumentationBreakpointResponse; struct SetPauseOnExceptionsRequest; @@ -89,6 +90,7 @@ struct RequestHandler { virtual void handle(const debugger::ResumeRequest &req) = 0; virtual void handle(const debugger::SetBreakpointRequest &req) = 0; virtual void handle(const debugger::SetBreakpointByUrlRequest &req) = 0; + virtual void handle(const debugger::SetBreakpointsActiveRequest &req) = 0; virtual void handle( const debugger::SetInstrumentationBreakpointRequest &req) = 0; virtual void handle(const debugger::SetPauseOnExceptionsRequest &req) = 0; @@ -116,6 +118,7 @@ struct NoopRequestHandler : public RequestHandler { void handle(const debugger::ResumeRequest &req) override {} void handle(const debugger::SetBreakpointRequest &req) override {} void handle(const debugger::SetBreakpointByUrlRequest &req) override {} + void handle(const debugger::SetBreakpointsActiveRequest &req) override {} void handle( const debugger::SetInstrumentationBreakpointRequest &req) override {} void handle(const debugger::SetPauseOnExceptionsRequest &req) override {} @@ -354,6 +357,16 @@ struct debugger::SetBreakpointByUrlRequest : public Request { folly::Optional condition; }; +struct debugger::SetBreakpointsActiveRequest : public Request { + SetBreakpointsActiveRequest(); + explicit SetBreakpointsActiveRequest(const folly::dynamic &obj); + + folly::dynamic toDynamic() const override; + void accept(RequestHandler &handler) const override; + + bool active{}; +}; + struct debugger::SetInstrumentationBreakpointRequest : public Request { SetInstrumentationBreakpointRequest(); explicit SetInstrumentationBreakpointRequest(const folly::dynamic &obj); diff --git a/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp b/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp index 9d5abc5962a..b97bf2cccd4 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp +++ b/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp @@ -750,6 +750,70 @@ TEST(ConnectionTests, testSetBreakpointById) { expectNotification(conn); } +TEST(ConnectionTests, testActivateBreakpoints) { + TestContext context; + AsyncHermesRuntime &asyncRuntime = context.runtime(); + SyncConnection &conn = context.conn(); + int msgId = 1; + + asyncRuntime.executeScriptAsync(R"( + debugger; // line 1 + x=100 // 2 + debugger; // 3 + x=101; // 4 + )"); + + send(conn, ++msgId); + expectExecutionContextCreated(conn); + auto script = expectNotification(conn); + + expectPaused(conn, "other", {{"global", 1, 1}}); + + // Set breakpoint #1 + m::debugger::SetBreakpointRequest req; + req.id = ++msgId; + req.location.scriptId = script.scriptId; + req.location.lineNumber = 2; + conn.send(req.toJson()); + expectResponse(conn, req.id); + + // Set breakpoint #2 + req.id = ++msgId; + req.location.scriptId = script.scriptId; + req.location.lineNumber = 4; + conn.send(req.toJson()); + expectResponse(conn, req.id); + + // Disable breakpoints + m::debugger::SetBreakpointsActiveRequest activeReq; + activeReq.id = ++msgId; + activeReq.active = false; + conn.send(activeReq.toJson()); + expectResponse(conn, activeReq.id); + + // Resume + send(conn, ++msgId); + expectNotification(conn); + + // Expect first breakpoint to be skipped, now hitting line #3 + expectPaused(conn, "other", {{"global", 3, 1}}); + + // Re-enable breakpoints + activeReq.id = ++msgId; + activeReq.active = true; + conn.send(activeReq.toJson()); + expectResponse(conn, activeReq.id); + + // Resume and expect breakpoints to trigger again + send(conn, ++msgId); + expectNotification(conn); + expectPaused(conn, "other", {{"global", 4, 1}}); + + // Continue and exit + send(conn, ++msgId); + expectNotification(conn); +} + TEST(ConnectionTests, testSetBreakpointByIdWithColumnInIndenting) { TestContext context; AsyncHermesRuntime &asyncRuntime = context.runtime(); diff --git a/ReactCommon/hermes/inspector/tools/message_types.txt b/ReactCommon/hermes/inspector/tools/message_types.txt index 02efcce5f1b..7807ac5b017 100644 --- a/ReactCommon/hermes/inspector/tools/message_types.txt +++ b/ReactCommon/hermes/inspector/tools/message_types.txt @@ -10,6 +10,7 @@ Debugger.resumed Debugger.scriptParsed Debugger.setBreakpoint Debugger.setBreakpointByUrl +Debugger.setBreakpointsActive Debugger.setInstrumentationBreakpoint Debugger.setPauseOnExceptions Debugger.stepInto