Add support for Debugger.runIfWaitingForDebugger

Summary:
This call is used to continue execution when the app has just been
started in a "wait for debugger" mode. This is the only case
in which it has an effect.

Notably, it should do nothing in the following cases, which a layperson
may be tempted to classify as "WaitingForDebugger":

* The app was running detached and hit a 'debugger;' statement
* The app is paused because of a breakpoint or hitting the Pause button
* The app stopped on an instrumentation breakpoint, and expects
  the debugger to collect data and potentially auto-resume.

Changelog: [Internal] Add Hermes support for Debugger.runIfWaitingForDebugger

Reviewed By: mhorowitz

Differential Revision: D21557446

fbshipit-source-id: 790cec7444ddc61908d2ef9d92e4649b535d678f

ReactCommon/hermes/inspector/Inspector.cpp

@@ -120,6 +120,7 @@ Inspector::Inspector(
     std::lock_guard<std::mutex> lock(mutex_);
 
     if (pauseOnFirstStatement) {
+      awaitingDebuggerOnStart_ = true;
       TRANSITION(std::make_unique<InspectorState::RunningWaitEnable>(*this));
     } else {
       TRANSITION(std::make_unique<InspectorState::RunningDetached>(*this));
@@ -679,6 +680,10 @@ bool Inspector::isExecutingSupersededFile() {
   return false;
 }
 
+bool Inspector::isAwaitingDebuggerOnStart() {
+  return awaitingDebuggerOnStart_;
+}
+
 } // namespace inspector
 } // namespace hermes
 } // namespace facebook

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

ReactCommon/hermes/inspector/InspectorState.cpp

@@ -51,6 +51,10 @@ std::pair<NextStatePtr, CommandPtr> InspectorState::RunningDetached::didPause(
       nullptr, makeContinueCommand());
 }
 
+void InspectorState::RunningDetached::onEnter(InspectorState *previous) {
+  inspector_.awaitingDebuggerOnStart_ = false;
+}
+
 std::pair<NextStatePtr, bool> InspectorState::RunningDetached::enable() {
   return std::make_pair<NextStatePtr, bool>(
       InspectorState::Running::make(inspector_), true);
@@ -172,6 +176,8 @@ void InspectorState::Running::onEnter(InspectorState *prevState) {
       inspector_.notifyScriptsLoaded();
     }
   }
+
+  inspector_.awaitingDebuggerOnStart_ = false;
 }
 
 void InspectorState::Running::detach(

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<folly::Promise<folly::Unit>> 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<NextStatePtr, CommandPtr> didPause(MonitorLock &lock) override;
   std::pair<NextStatePtr, bool> enable() override;
 
+  void onEnter(InspectorState *prevState) override;
+
   bool isRunningDetached() const override {
     return true;
   }

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<m::runtime::PropertyDescriptor> 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<m::debugger::BreakpointId>();
 
@@ -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<std::exception>(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
  */

ReactCommon/hermes/inspector/chrome/MessageTypes.cpp

@@ -1,5 +1,5 @@
 // Copyright 2004-present Facebook. All Rights Reserved.
-// @generated SignedSource<<575de63c36edd2a9a0f191501fd4f662>>
+// @generated SignedSource<<f251363b69dd291d2c70e893c3fed04d>>
 
 #include "MessageTypes.h"
 
@@ -50,6 +50,8 @@ std::unique_ptr<Request> Request::fromJsonThrowOnError(const std::string &str) {
        makeUnique<heapProfiler::TakeHeapSnapshotRequest>},
       {"Runtime.evaluate", makeUnique<runtime::EvaluateRequest>},
       {"Runtime.getProperties", makeUnique<runtime::GetPropertiesRequest>},
+      {"Runtime.runIfWaitingForDebugger",
+       makeUnique<runtime::RunIfWaitingForDebuggerRequest>},
   };
 
   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");

ReactCommon/hermes/inspector/chrome/MessageTypes.h

@@ -1,5 +1,5 @@
 // Copyright 2004-present Facebook. All Rights Reserved.
-// @generated SignedSource<<d3d8ca811a2e56cf4794f5dc4171bbdb>>
+// @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<bool> 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;

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<m::debugger::ResumedNotification>(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<m::debugger::EnableRequest>(conn, ++msgId);
+  expectExecutionContextCreated(conn);
+  expectNotification<m::debugger::ScriptParsedNotification>(conn);
+  expectNotification<m::debugger::PausedNotification>(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<m::runtime::RunIfWaitingForDebuggerRequest>(conn, ++msgId);
+  expectNotification<m::debugger::ResumedNotification>(conn);
+
+  // We should immediately hit the 'debugger;' statement
+  expectNotification<m::debugger::PausedNotification>(conn);
+  EXPECT_EQ(1, asyncRuntime.awaitStoredValue().asNumber());
+
+  // RunIfWaitingForDebuggerResponse should be accepted but have no effect
+  send<m::runtime::RunIfWaitingForDebuggerRequest>(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<m::debugger::ResumeRequest>(conn, msgId++);
+  expectNotification<m::debugger::ResumedNotification>(conn);
+}
+
 } // namespace chrome
 } // namespace inspector
 } // namespace hermes

ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp

@@ -52,12 +52,15 @@ class SyncConnection::RemoteConnnection : public IRemoteConnection {
   SyncConnection &conn_;
 };
 
-SyncConnection::SyncConnection(std::shared_ptr<HermesRuntime> runtime)
+SyncConnection::SyncConnection(
+    std::shared_ptr<HermesRuntime> runtime,
+    bool waitForDebugger)
     : connection_(
           std::make_unique<SharedRuntimeAdapter>(
               runtime,
               runtime->getDebugger()),
-          "testConn") {
+          "testConn",
+          waitForDebugger) {
   connection_.connect(std::make_unique<RemoteConnnection>(*this));
 }
 

ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h

@@ -28,7 +28,9 @@ namespace chrome {
  */
 class SyncConnection {
  public:
-  SyncConnection(std::shared_ptr<HermesRuntime> runtime);
+  SyncConnection(
+      std::shared_ptr<HermesRuntime> runtime,
+      bool waitForDebugger = false);
   ~SyncConnection() = default;
 
   /// sends a message to the debugger

ReactCommon/hermes/inspector/tools/message_types.txt

@@ -24,3 +24,4 @@ Runtime.consoleAPICalled
 Runtime.evaluate
 Runtime.executionContextCreated
 Runtime.getProperties
+Runtime.runIfWaitingForDebugger