Implement Debugger.setBreakpoint and removeBreakpoint

Summary: Implement set and remove breakpoint messages.

Reviewed By: mattbfb

Differential Revision: D53340313

fbshipit-source-id: f1eb0b407dfc50140b1eb01a3631d3121316072c

API/hermes/cdp/CDPAgent.cpp

@@ -210,6 +210,12 @@ void CDPAgentImpl::DomainAgents::handleCommand(
   } else if (command->method == "Debugger.evaluateOnCallFrame") {
     debuggerAgent_->evaluateOnCallFrame(
         static_cast<m::debugger::EvaluateOnCallFrameRequest &>(*command));
+  } else if (command->method == "Debugger.setBreakpoint") {
+    debuggerAgent_->setBreakpoint(
+        static_cast<m::debugger::SetBreakpointRequest &>(*command));
+  } else if (command->method == "Debugger.removeBreakpoint") {
+    debuggerAgent_->removeBreakpoint(
+        static_cast<m::debugger::RemoveBreakpointRequest &>(*command));
   } else if (command->method == "Runtime.enable") {
     runtimeAgent_->enable(static_cast<m::runtime::EnableRequest &>(*command));
   } else if (command->method == "Runtime.disable") {

API/hermes/cdp/DebuggerDomainAgent.cpp

@@ -55,15 +55,8 @@ void DebuggerDomainAgent::handleDebuggerEvent(
       }
       break;
     case DebuggerEventType::DebuggerStatement:
-      paused_ = true;
-      sendPausedNotificationToClient();
-      break;
     case DebuggerEventType::Breakpoint:
-      break;
     case DebuggerEventType::StepFinish:
-      paused_ = true;
-      sendPausedNotificationToClient();
-      break;
     case DebuggerEventType::ExplicitPause:
       paused_ = true;
       sendPausedNotificationToClient();
@@ -229,6 +222,65 @@ void DebuggerDomainAgent::evaluateOnCallFrame(
       });
 }
 
+void DebuggerDomainAgent::setBreakpoint(
+    const m::debugger::SetBreakpointRequest &req) {
+  CDPBreakpointDescription description;
+  description.line = req.location.lineNumber;
+  description.column = req.location.columnNumber;
+  description.condition = req.location.scriptId;
+
+  auto scriptID = std::stoull(req.location.scriptId);
+
+  // Create the Hermes breakpoint
+  std::optional<HermesBreakpointLocation> hermesBreakpoint =
+      createHermesBreakpont(
+          static_cast<debugger::ScriptID>(scriptID), description);
+  if (!hermesBreakpoint) {
+    sendResponseToClient(m::makeErrorResponse(
+        req.id, m::ErrorCode::ServerError, "Breakpoint creation failed"));
+    return;
+  }
+
+  // Create the CDP breakpoint
+  auto [breakpointID, breakpoint] = createCDPBreakpoint(
+      std::move(description),
+      HermesBreakpoint{
+          hermesBreakpoint.value().id,
+          static_cast<debugger::ScriptID>(scriptID)});
+
+  // Send the response
+  m::debugger::SetBreakpointResponse resp;
+  resp.id = req.id;
+  resp.breakpointId = std::to_string(breakpointID);
+  resp.actualLocation =
+      m::debugger::makeLocation(hermesBreakpoint.value().location);
+
+  sendResponseToClient(resp);
+}
+
+void DebuggerDomainAgent::removeBreakpoint(
+    const m::debugger::RemoveBreakpointRequest &req) {
+  auto cdpID = std::stoull(req.breakpointId);
+  auto cdpBreakpoint = cdpBreakpoints_.find(cdpID);
+  if (cdpBreakpoint == cdpBreakpoints_.end()) {
+    sendResponseToClient(m::makeErrorResponse(
+        req.id,
+        m::ErrorCode::InvalidRequest,
+        "Unknown breakpoint ID: " + req.breakpointId));
+    return;
+  }
+
+  // Remove all the Hermes breakpoints implied by the CDP breakpoint
+  for (HermesBreakpoint hermesBreakpoint :
+       cdpBreakpoint->second.hermesBreakpoints) {
+    runtime_.getDebugger().deleteBreakpoint(hermesBreakpoint.breakpointID);
+  }
+
+  // Remove the CDP breakpoint
+  cdpBreakpoints_.erase(cdpBreakpoint);
+  sendResponseToClient(m::makeOkResponse(req.id));
+}
+
 void DebuggerDomainAgent::sendPausedNotificationToClient() {
   m::debugger::PausedNotification note;
   note.reason = "other";
@@ -279,6 +331,71 @@ void DebuggerDomainAgent::processNewLoadedScript() {
   }
 }
 
+/// Create a CDP breakpoint with a \p description of where to break, and
+/// (optionally) a \p hermesBreakpointID that has already been applied.
+/// Returns a pair containing the newly created breakpoint ID and value.
+std::pair<unsigned int, CDPBreakpoint &>
+DebuggerDomainAgent::createCDPBreakpoint(
+    CDPBreakpointDescription &&description,
+    std::optional<HermesBreakpoint> hermesBreakpoint) {
+  unsigned int breakpointID = nextBreakpointID_++;
+  CDPBreakpoint &breakpoint =
+      cdpBreakpoints_.emplace(breakpointID, CDPBreakpoint(description))
+          .first->second;
+
+  if (hermesBreakpoint) {
+    breakpoint.hermesBreakpoints.push_back(hermesBreakpoint.value());
+  }
+
+  return {breakpointID, breakpoint};
+}
+
+/// Attempt to create a breakpoint in the Hermes script identified by
+/// \p scriptID at the location described by \p description.
+std::optional<HermesBreakpointLocation>
+DebuggerDomainAgent::createHermesBreakpont(
+    debugger::ScriptID scriptID,
+    const CDPBreakpointDescription &description) {
+  // Convert the location description to a Hermes location
+  debugger::SourceLocation hermesLoc;
+  hermesLoc.fileId = scriptID;
+  // CDP Locations are 0-based, Hermes lines/columns are 1-based
+  hermesLoc.line = description.line + 1;
+
+  if (description.column.has_value()) {
+    if (description.column.value() == 0) {
+      // TODO: When CDTP sends a column number of 0, we send Hermes a column
+      // number of 1. For some reason, this causes Hermes to not be
+      // able to resolve breakpoints.
+      hermesLoc.column = debugger::kInvalidLocation;
+    } else {
+      hermesLoc.column = description.column.value() + 1;
+    }
+  }
+
+  // Set the breakpoint in Hermes
+  debugger::BreakpointID breakpointID =
+      runtime_.getDebugger().setBreakpoint(hermesLoc);
+  if (breakpointID == debugger::kInvalidBreakpoint) {
+    return {};
+  }
+
+  debugger::BreakpointInfo info =
+      runtime_.getDebugger().getBreakpointInfo(breakpointID);
+  if (!info.resolved) {
+    // Only accept immediately-resolvable breakpoints
+    return {};
+  }
+
+  // Apply any break conditions
+  if (description.condition) {
+    runtime_.getDebugger().setBreakpointCondition(
+        breakpointID, description.condition.value());
+  }
+
+  return HermesBreakpointLocation{breakpointID, info.resolvedLocation};
+}
+
 bool DebuggerDomainAgent::checkDebuggerEnabled(const m::Request &req) {
   if (!enabled_) {
     sendResponseToClient(m::makeErrorResponse(

API/hermes/cdp/DebuggerDomainAgent.h

@@ -25,6 +25,52 @@ namespace cdp {
 namespace m = ::facebook::hermes::inspector_modern::chrome::message;
 namespace old_cdp = ::facebook::hermes::inspector_modern::chrome;
 
+namespace {
+/// Details about a single Hermes breakpoint, implied by a CDP breakpoint.
+struct HermesBreakpoint {
+  debugger::BreakpointID breakpointID;
+  debugger::ScriptID scriptID;
+};
+
+/// Type used to store CDP breakpoint identifiers. These IDs are generated by
+/// the CDP Handler, so we can constrain them to a specific range.
+using CDPBreakpointID = uint32_t;
+
+/// Description of where breakpoints should be created.
+struct CDPBreakpointDescription {
+  /// Determines whether this breakpoint can be persisted across sessions
+  bool persistable() const {
+    // Only persist breakpoints that can apply to future scripts (i.e.
+    // breakpoints set on a set of files specified by script URL, not
+    // breakpoints set on an exact, session-specific script ID).
+    return url.has_value();
+  }
+
+  std::optional<std::string> url;
+  long long line;
+  std::optional<long long> column;
+  std::optional<std::string> condition;
+};
+
+/// Details of each existing CDP breakpoint, which may correspond to multiple
+/// Hermes breakpoints.
+struct CDPBreakpoint {
+  explicit CDPBreakpoint(CDPBreakpointDescription description)
+      : description(description) {}
+
+  // Description of where the breakpoint should be applied
+  CDPBreakpointDescription description;
+
+  // Registered breakpoints in Hermes
+  std::vector<HermesBreakpoint> hermesBreakpoints;
+};
+
+struct HermesBreakpointLocation {
+  debugger::BreakpointID id;
+  debugger::SourceLocation location;
+};
+} // namespace
+
 /// Handler for the "Debugger" domain of CDP. Accepts events from the runtime,
 /// and CDP requests from the debug client belonging to the "Debugger" domain.
 /// Produces CDP responses and events belonging to the "Debugger" domain. All
@@ -48,19 +94,26 @@ class DebuggerDomainAgent : public DomainAgent {
   /// Handles Debugger.resume request
   void resume(const m::debugger::ResumeRequest &req);
 
-  // Handles Debugger.stepInto request
+  /// Handles Debugger.stepInto request
   void stepInto(const m::debugger::StepIntoRequest &req);
-  // Handles Debugger.stepOut request
+  /// Handles Debugger.stepOut request
   void stepOut(const m::debugger::StepOutRequest &req);
-  // Handles Debugger.stepOver request
+  /// Handles Debugger.stepOver request
   void stepOver(const m::debugger::StepOverRequest &req);
 
-  // Handles Debugger.setPauseOnExceptions
+  /// Handles Debugger.setPauseOnExceptions
   void setPauseOnExceptions(
       const m::debugger::SetPauseOnExceptionsRequest &req);
 
+  /// Handles Debugger.evaluateOnCallFrame
   void evaluateOnCallFrame(const m::debugger::EvaluateOnCallFrameRequest &req);
 
+  /// Debugger.setBreakpoint creates a CDP breakpoint that applies to exactly
+  /// one script (identified by script ID) that does not survive reloads.
+  void setBreakpoint(const m::debugger::SetBreakpointRequest &req);
+  /// Handles Debugger.removeBreakpoint
+  void removeBreakpoint(const m::debugger::RemoveBreakpointRequest &req);
+
  private:
   /// Handle an event originating from the runtime.
   void handleDebuggerEvent(
@@ -82,6 +135,14 @@ class DebuggerDomainAgent : public DomainAgent {
   /// debug client
   void processNewLoadedScript();
 
+  std::pair<unsigned int, CDPBreakpoint &> createCDPBreakpoint(
+      CDPBreakpointDescription &&description,
+      std::optional<HermesBreakpoint> hermesBreakpoint = std::nullopt);
+
+  std::optional<HermesBreakpointLocation> createHermesBreakpont(
+      debugger::ScriptID scriptID,
+      const CDPBreakpointDescription &description);
+
   bool checkDebuggerEnabled(const m::Request &req);
   bool checkDebuggerPaused(const m::Request &req);
 
@@ -93,6 +154,14 @@ class DebuggerDomainAgent : public DomainAgent {
 
   old_cdp::RemoteObjectsTable objTable_{};
 
+  /// Details of each CDP breakpoint that has been created, and not
+  /// yet destroyed.
+  std::unordered_map<CDPBreakpointID, CDPBreakpoint> cdpBreakpoints_{};
+
+  /// CDP breakpoint IDs are assigned by the DebuggerDomainAgent. Keep track of
+  /// the next available ID.
+  CDPBreakpointID nextBreakpointID_ = 1;
+
   /// Whether Debugger.enable was received and wasn't disabled by receiving
   /// Debugger.disable
   bool enabled_;

API/hermes/inspector/chrome/tests/ConnectionTests.cpp

@@ -1076,6 +1076,7 @@ TEST_F(ConnectionTests, testApplyBreakpointsToLoadedScripts) {
   expectNothing(conn);
 }
 
+// Also implemented as CDPAgentTest::TestSetBreakpointById
 TEST_F(ConnectionTests, testSetBreakpointById) {
   int msgId = 1;
 
@@ -1359,6 +1360,7 @@ TEST_F(ConnectionTests, testSetBreakpointConditional) {
   expectNotification<m::debugger::ResumedNotification>(conn);
 }
 
+// Also implemented as CDPAgentTest::TestRemoveBreakpoint
 TEST_F(ConnectionTests, testRemoveBreakpoint) {
   int msgId = 1;