[inspector] Change ScriptBreakpoint to include scriptId

The ScriptBreakpoint struct was before just holding line, column and
condition. It now additionally holds the scriptId.
This encapsulates information nicer, and allows for easier translation
of wasm locations, since one struct now holds all information needed
for the translation.

BUG=chromium:659715
R=yangguo@chromium.org, kozyatinskiy@chromium.org

Review-Url: https://codereview.chromium.org/2491133003
Cr-Commit-Position: refs/heads/master@{#41044}

src/inspector/script-breakpoint.h

@@ -35,15 +35,18 @@
 namespace v8_inspector {
 
 struct ScriptBreakpoint {
-  ScriptBreakpoint() : ScriptBreakpoint(0, 0, String16()) {}
-
-  ScriptBreakpoint(int lineNumber, int columnNumber, const String16& condition)
-      : lineNumber(lineNumber),
-        columnNumber(columnNumber),
-        condition(condition) {}
-
-  int lineNumber;
-  int columnNumber;
+  ScriptBreakpoint() {}
+
+  ScriptBreakpoint(String16 script_id, int line_number, int column_number,
+                   String16 condition)
+      : script_id(std::move(script_id)),
+        line_number(line_number),
+        column_number(column_number),
+        condition(std::move(condition)) {}
+
+  String16 script_id;
+  int line_number = 0;
+  int column_number = 0;
   String16 condition;
 };
 

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

@@ -74,14 +74,14 @@ static String16 breakpointIdSuffix(
 }
 
 static String16 generateBreakpointId(
-    const String16& scriptId, int lineNumber, int columnNumber,
+    const ScriptBreakpoint& breakpoint,
     V8DebuggerAgentImpl::BreakpointSource source) {
   String16Builder builder;
-  builder.append(scriptId);
+  builder.append(breakpoint.script_id);
   builder.append(':');
-  builder.appendNumber(lineNumber);
+  builder.appendNumber(breakpoint.line_number);
   builder.append(':');
-  builder.appendNumber(columnNumber);
+  builder.appendNumber(breakpoint.column_number);
   builder.append(breakpointIdSuffix(source));
   return builder.toString();
 }
@@ -291,12 +291,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));
   }
 
@@ -308,21 +309,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();
@@ -405,13 +403,13 @@ 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);
+      breakpoint, &breakpoint.line_number, &breakpoint.column_number);
+  // TODO(kozyatinskiy): Return actual line and column number.
   return resume();
 }
 
@@ -493,23 +491,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] =
@@ -518,7 +515,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(
@@ -1075,14 +1073,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));
   }
@@ -1221,17 +1220,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() {

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

@@ -162,8 +162,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
   void setPauseOnExceptionsImpl(int);
 
   std::unique_ptr<protocol::Debugger::Location> resolveBreakpoint(
-      const String16& breakpointId, const String16& scriptId,
-      const ScriptBreakpoint&, BreakpointSource);
+      const String16& breakpointId, const ScriptBreakpoint&, BreakpointSource);
   void removeBreakpointImpl(const String16& breakpointId);
   void clearBreakDetails();
 

src/inspector/v8-debugger.cc

@@ -135,8 +135,7 @@ void V8Debugger::getCompiledScripts(
   }
 }
 
-String16 V8Debugger::setBreakpoint(const String16& sourceID,
-                                   const ScriptBreakpoint& scriptBreakpoint,
+String16 V8Debugger::setBreakpoint(const ScriptBreakpoint& breakpoint,
                                    int* actualLineNumber,
                                    int* actualColumnNumber) {
   v8::HandleScope scope(m_isolate);
@@ -146,20 +145,20 @@ String16 V8Debugger::setBreakpoint(const String16& sourceID,
   v8::Local<v8::Object> info = v8::Object::New(m_isolate);
   bool success = false;
   success = info->Set(context, toV8StringInternalized(m_isolate, "sourceID"),
-                      toV8String(m_isolate, sourceID))
+                      toV8String(m_isolate, breakpoint.script_id))
                 .FromMaybe(false);
   DCHECK(success);
   success = info->Set(context, toV8StringInternalized(m_isolate, "lineNumber"),
-                      v8::Integer::New(m_isolate, scriptBreakpoint.lineNumber))
+                      v8::Integer::New(m_isolate, breakpoint.line_number))
                 .FromMaybe(false);
   DCHECK(success);
   success =
       info->Set(context, toV8StringInternalized(m_isolate, "columnNumber"),
-                v8::Integer::New(m_isolate, scriptBreakpoint.columnNumber))
+                v8::Integer::New(m_isolate, breakpoint.column_number))
           .FromMaybe(false);
   DCHECK(success);
   success = info->Set(context, toV8StringInternalized(m_isolate, "condition"),
-                      toV8String(m_isolate, scriptBreakpoint.condition))
+                      toV8String(m_isolate, breakpoint.condition))
                 .FromMaybe(false);
   DCHECK(success);
 

src/inspector/v8-debugger.h

@@ -36,8 +36,8 @@ class V8Debugger {
 
   bool enabled() const;
 
-  String16 setBreakpoint(const String16& sourceID, const ScriptBreakpoint&,
-                         int* actualLineNumber, int* actualColumnNumber);
+  String16 setBreakpoint(const ScriptBreakpoint&, int* actualLineNumber,
+                         int* actualColumnNumber);
   void removeBreakpoint(const String16& breakpointId);
   void setBreakpointsActivated(bool);
   bool breakpointsActivated() const { return m_breakpointsActivated; }