[inspector] Change ScriptBreakpoint to include scriptId

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

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/inspector/v8-debugger-agent-impl.h"
 
 #include <algorithm>
 
 #include "src/debug/debug-interface.h"
 #include "src/inspector/injected-script.h"
@@ skipping 56 matching lines @@
       break;
     case V8DebuggerAgentImpl::DebugCommandBreakpointSource:
       return ":debug";
     case V8DebuggerAgentImpl::MonitorCommandBreakpointSource:
       return ":monitor";
   }
   return String16();
 }
 
 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));
 }
 
 static bool positionComparator(const std::pair<int, int>& a,
                                const std::pair<int, int>& b) {
   if (a.first != b.first) return a.first < b.first;
   return a.second < b.second;
 }
 
 static std::unique_ptr<protocol::Debugger::Location> buildProtocolLocation(
@@ skipping 188 matching lines @@
     m_state->setObject(DebuggerAgentState::javaScriptBreakpoints,
                        std::move(newValue));
   }
   if (breakpointsCookie->get(breakpointId))
     return Response::Error("Breakpoint at specified location already exists.");
 
   breakpointsCookie->setObject(
       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));
   }
 
   *outBreakpointId = breakpointId;
   return Response::OK();
 }
 
 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();
 }
 
 Response V8DebuggerAgentImpl::removeBreakpoint(const String16& breakpointId) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
   protocol::DictionaryValue* breakpointsCookie =
       m_state->getObject(DebuggerAgentState::javaScriptBreakpoints);
   if (breakpointsCookie) breakpointsCookie->remove(breakpointId);
@@ skipping 62 matching lines @@
 }
 
 Response V8DebuggerAgentImpl::continueToLocation(
     std::unique_ptr<protocol::Debugger::Location> location) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
   if (!m_continueToLocationBreakpointId.isEmpty()) {
     m_debugger->removeBreakpoint(m_continueToLocationBreakpointId);
     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();
 }
 
 bool V8DebuggerAgentImpl::isCurrentCallStackEmptyOrBlackboxed() {
   DCHECK(enabled());
   JavaScriptCallFrames callFrames = m_debugger->currentCallFrames();
   for (size_t index = 0; index < callFrames.size(); ++index) {
     if (!isCallFrameWithUnknownScriptOrBlackboxed(callFrames[index].get()))
       return false;
   }
@@ skipping 61 matching lines @@
   if (m_skippedStepFrameCount >= kMaxSkipStepFrameCount) return RequestStepOut;
 
   if (!m_skippedStepFrameCount) m_recursionLevelForStepFrame = 1;
 
   ++m_skippedStepFrameCount;
   return RequestStepFrame;
 }
 
 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] =
       std::make_pair(breakpointId, source);
   CHECK(!breakpointId.isEmpty());
 
   m_breakpointIdToDebuggerBreakpointIds[breakpointId].push_back(
       debuggerBreakpointId);
-  return buildProtocolLocation(scriptId, actualLineNumber, actualColumnNumber);
+  return buildProtocolLocation(breakpoint.script_id, actualLineNumber,
+                               actualColumnNumber);
 }
 
 Response V8DebuggerAgentImpl::searchInContent(
     const String16& scriptId, const String16& query,
     Maybe<bool> optionalCaseSensitive, Maybe<bool> optionalIsRegex,
     std::unique_ptr<Array<protocol::Debugger::SearchMatch>>* results) {
   v8::HandleScope handles(m_isolate);
   ScriptsMap::iterator it = m_scripts.find(scriptId);
   if (it == m_scripts.end())
     return Response::Error("No script for id: " + scriptId);
@@ skipping 530 matching lines @@
   for (size_t i = 0; i < breakpointsCookie->size(); ++i) {
     auto cookie = breakpointsCookie->at(i);
     protocol::DictionaryValue* breakpointObject =
         protocol::DictionaryValue::cast(cookie.second);
     bool isRegex;
     breakpointObject->getBoolean(DebuggerAgentState::isRegex, &isRegex);
     String16 url;
     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));
   }
 }
 
 V8DebuggerAgentImpl::SkipPauseRequest V8DebuggerAgentImpl::didPause(
     v8::Local<v8::Context> context, v8::Local<v8::Value> exception,
     const std::vector<String16>& hitBreakpoints, bool isPromiseRejection,
     bool isUncaught) {
   JavaScriptCallFrames callFrames = m_debugger->currentCallFrames(1);
@@ skipping 118 matching lines @@
 
 void V8DebuggerAgentImpl::clearBreakDetails() {
   m_breakReason = protocol::Debugger::Paused::ReasonEnum::Other;
   m_breakAuxData = nullptr;
 }
 
 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() {
   if (!enabled()) return;
   m_scheduledDebuggerStep = NoStep;
   m_scripts.clear();
   m_blackboxedPositions.clear();
   m_breakpointIdToDebuggerBreakpointIds.clear();
 }
 
 }  // namespace v8_inspector