[inspector] Change ScriptBreakpoint to include scriptId

src/inspector/v8-debugger-agent-impl.cc

Index: src/inspector/v8-debugger-agent-impl.cc
diff --git a/src/inspector/v8-debugger-agent-impl.cc b/src/inspector/v8-debugger-agent-impl.cc
index ae7cad284588e9a822112d8a310963f870e301e5..8bbbc24d9f9b766e38fed0d8fa7327f7b4f2f5e0 100644
--- a/src/inspector/v8-debugger-agent-impl.cc
+++ b/src/inspector/v8-debugger-agent-impl.cc
@@ -74,9 +74,10 @@ static String16 breakpointIdSuffix(
 }
 
 static String16 generateBreakpointId(
-    const String16& scriptId, int lineNumber, int columnNumber,
+    const ScriptBreakpoint& breakpoint,
     V8DebuggerAgentImpl::BreakpointSource source) {
-  return String16::concat(scriptId, ':', lineNumber, ':', columnNumber,
+  return String16::concat(breakpoint.script_id, ':', breakpoint.line_number,

[kozy, 2016/11/15 23:29:47]

Let's introduce id method on ScriptBreakpoint.

[Clemens Hammacher, 2016/11/16 12:06:24]

Doing that would require ScriptBreakpoint to become a class according to the
style guide. This would make things like breakpointObject->getInteger(...,
&breakpoint.line_number) much more bloated.

Let's keep it this way for now, and get this CL through, since it's blocking the
larger CL which should land before the branch point for version 56 (tomorrow).
Let me know if you want me to fix this in a separate CL.

+                          ':', breakpoint.column_number,
                           breakpointIdSuffix(source));
 }
 
@@ -285,12 +286,13 @@ Response V8DebuggerAgentImpl::setBreakpointByUrl(
       breakpointId, buildObjectForBreakpointCookie(
                         url, lineNumber, columnNumber, condition, isRegex));
 
-  ScriptBreakpoint breakpoint(lineNumber, columnNumber, condition);
+  ScriptBreakpoint breakpoint(String16(), lineNumber, columnNumber, condition);
   for (const auto& script : m_scripts) {
     if (!matches(m_inspector, script.second->sourceURL(), url, isRegex))
       continue;
-    std::unique_ptr<protocol::Debugger::Location> location = resolveBreakpoint(
-        breakpointId, script.first, breakpoint, UserBreakpointSource);
+    breakpoint.script_id = script.first;
+    std::unique_ptr<protocol::Debugger::Location> location =
+        resolveBreakpoint(breakpointId, breakpoint, UserBreakpointSource);
     if (location) (*locations)->addItem(std::move(location));
   }
 
@@ -302,21 +304,18 @@ Response V8DebuggerAgentImpl::setBreakpoint(
     std::unique_ptr<protocol::Debugger::Location> location,
     Maybe<String16> optionalCondition, String16* outBreakpointId,
     std::unique_ptr<protocol::Debugger::Location>* actualLocation) {
-  String16 scriptId = location->getScriptId();
-  int lineNumber = location->getLineNumber();
-  int columnNumber = location->getColumnNumber(0);
+  ScriptBreakpoint breakpoint(
+      location->getScriptId(), location->getLineNumber(),
+      location->getColumnNumber(0), optionalCondition.fromMaybe(String16()));
 
-  String16 condition = optionalCondition.fromMaybe("");
-
-  String16 breakpointId = generateBreakpointId(
-      scriptId, lineNumber, columnNumber, UserBreakpointSource);
+  String16 breakpointId =
+      generateBreakpointId(breakpoint, UserBreakpointSource);
   if (m_breakpointIdToDebuggerBreakpointIds.find(breakpointId) !=
       m_breakpointIdToDebuggerBreakpointIds.end()) {
     return Response::Error("Breakpoint at specified location already exists.");
   }
-  ScriptBreakpoint breakpoint(lineNumber, columnNumber, condition);
-  *actualLocation = resolveBreakpoint(breakpointId, scriptId, breakpoint,
-                                      UserBreakpointSource);
+  *actualLocation =
+      resolveBreakpoint(breakpointId, breakpoint, UserBreakpointSource);
   if (!*actualLocation) return Response::Error("Could not resolve breakpoint");
   *outBreakpointId = breakpointId;
   return Response::OK();
@@ -399,13 +398,12 @@ Response V8DebuggerAgentImpl::continueToLocation(
     m_continueToLocationBreakpointId = "";
   }
 
-  String16 scriptId = location->getScriptId();
-  int lineNumber = location->getLineNumber();
-  int columnNumber = location->getColumnNumber(0);
+  ScriptBreakpoint breakpoint(location->getScriptId(),
+                              location->getLineNumber(),
+                              location->getColumnNumber(0), String16());
 
-  ScriptBreakpoint breakpoint(lineNumber, columnNumber, "");
-  m_continueToLocationBreakpointId = m_debugger->setBreakpoint(
-      scriptId, breakpoint, &lineNumber, &columnNumber);
+  m_continueToLocationBreakpointId =
+      m_debugger->setBreakpoint(breakpoint, nullptr, nullptr);
   return resume();
 }
 
@@ -487,23 +485,22 @@ V8DebuggerAgentImpl::SkipPauseRequest V8DebuggerAgentImpl::shouldSkipStepPause(
 
 std::unique_ptr<protocol::Debugger::Location>
 V8DebuggerAgentImpl::resolveBreakpoint(const String16& breakpointId,
-                                       const String16& scriptId,
                                        const ScriptBreakpoint& breakpoint,
                                        BreakpointSource source) {
   DCHECK(enabled());
   // FIXME: remove these checks once crbug.com/520702 is resolved.
   CHECK(!breakpointId.isEmpty());
-  CHECK(!scriptId.isEmpty());
-  ScriptsMap::iterator scriptIterator = m_scripts.find(scriptId);
+  CHECK(!breakpoint.script_id.isEmpty());
+  ScriptsMap::iterator scriptIterator = m_scripts.find(breakpoint.script_id);
   if (scriptIterator == m_scripts.end()) return nullptr;
-  if (breakpoint.lineNumber < scriptIterator->second->startLine() ||
-      scriptIterator->second->endLine() < breakpoint.lineNumber)
+  if (breakpoint.line_number < scriptIterator->second->startLine() ||
+      scriptIterator->second->endLine() < breakpoint.line_number)
     return nullptr;
 
   int actualLineNumber;
   int actualColumnNumber;
   String16 debuggerBreakpointId = m_debugger->setBreakpoint(
-      scriptId, breakpoint, &actualLineNumber, &actualColumnNumber);
+      breakpoint, &actualLineNumber, &actualColumnNumber);
   if (debuggerBreakpointId.isEmpty()) return nullptr;
 
   m_serverBreakpoints[debuggerBreakpointId] =
@@ -512,7 +509,8 @@ V8DebuggerAgentImpl::resolveBreakpoint(const String16& breakpointId,
 
   m_breakpointIdToDebuggerBreakpointIds[breakpointId].push_back(
       debuggerBreakpointId);
-  return buildProtocolLocation(scriptId, actualLineNumber, actualColumnNumber);
+  return buildProtocolLocation(breakpoint.script_id, actualLineNumber,
+                               actualColumnNumber);
 }
 
 Response V8DebuggerAgentImpl::searchInContent(
@@ -1063,14 +1061,15 @@ void V8DebuggerAgentImpl::didParseSource(
     breakpointObject->getString(DebuggerAgentState::url, &url);
     if (!matches(m_inspector, scriptURL, url, isRegex)) continue;
     ScriptBreakpoint breakpoint;
+    breakpoint.script_id = scriptId;
     breakpointObject->getInteger(DebuggerAgentState::lineNumber,
-                                 &breakpoint.lineNumber);
+                                 &breakpoint.line_number);
     breakpointObject->getInteger(DebuggerAgentState::columnNumber,
-                                 &breakpoint.columnNumber);
+                                 &breakpoint.column_number);
     breakpointObject->getString(DebuggerAgentState::condition,
                                 &breakpoint.condition);
-    std::unique_ptr<protocol::Debugger::Location> location = resolveBreakpoint(
-        cookie.first, scriptId, breakpoint, UserBreakpointSource);
+    std::unique_ptr<protocol::Debugger::Location> location =
+        resolveBreakpoint(cookie.first, breakpoint, UserBreakpointSource);
     if (location)
       m_frontend.breakpointResolved(cookie.first, std::move(location));
   }
@@ -1209,17 +1208,17 @@ void V8DebuggerAgentImpl::setBreakpointAt(const String16& scriptId,
                                           int lineNumber, int columnNumber,
                                           BreakpointSource source,
                                           const String16& condition) {
-  String16 breakpointId =
-      generateBreakpointId(scriptId, lineNumber, columnNumber, source);
-  ScriptBreakpoint breakpoint(lineNumber, columnNumber, condition);
-  resolveBreakpoint(breakpointId, scriptId, breakpoint, source);
+  ScriptBreakpoint breakpoint(scriptId, lineNumber, columnNumber, condition);
+  String16 breakpointId = generateBreakpointId(breakpoint, source);
+  resolveBreakpoint(breakpointId, breakpoint, source);
 }
 
 void V8DebuggerAgentImpl::removeBreakpointAt(const String16& scriptId,
                                              int lineNumber, int columnNumber,
                                              BreakpointSource source) {
-  removeBreakpointImpl(
-      generateBreakpointId(scriptId, lineNumber, columnNumber, source));
+  removeBreakpointImpl(generateBreakpointId(
+      ScriptBreakpoint(scriptId, lineNumber, columnNumber, String16()),
+      source));
 }
 
 void V8DebuggerAgentImpl::reset() {