[inspector] Expose GetPossibleBreakpoints for wasm

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 455 matching lines @@
   DCHECK(enabled());
   // FIXME: remove these checks once crbug.com/520702 is resolved.
   CHECK(!breakpointId.isEmpty());
   CHECK(!breakpoint.script_id.isEmpty());
   ScriptsMap::iterator scriptIterator = m_scripts.find(breakpoint.script_id);
   if (scriptIterator == m_scripts.end()) return nullptr;
   if (breakpoint.line_number < scriptIterator->second->startLine() ||
       scriptIterator->second->endLine() < breakpoint.line_number)
     return nullptr;
 
+  // Translate from protocol location to v8 location for the debugger.
   ScriptBreakpoint translatedBreakpoint = breakpoint;
   m_debugger->wasmTranslation()->TranslateProtocolLocationToWasmScriptLocation(
       &translatedBreakpoint.script_id, &translatedBreakpoint.line_number,
       &translatedBreakpoint.column_number);
 
-  int actualLineNumber;
-  int actualColumnNumber;
   String16 debuggerBreakpointId = m_debugger->setBreakpoint(
-      translatedBreakpoint, &actualLineNumber, &actualColumnNumber);
+      translatedBreakpoint, &translatedBreakpoint.line_number,

[dgozman, 2017/01/26 06:24:34]

drive-by: it doesn't look safe to pass a part of input parameter as an output
parameter.

[Clemens Hammacher, 2017/01/26 09:57:47]

Yes, it looks weird. I checked that it works, but anyway undid this change now.
Two stack variables don't cost us too much ;)

+      &translatedBreakpoint.column_number);
   if (debuggerBreakpointId.isEmpty()) return nullptr;
 
+  // Translate back from v8 location to protocol location for the return value.
+  m_debugger->wasmTranslation()->TranslateWasmScriptLocationToProtocolLocation(

[dgozman, 2017/01/26 06:24:34]

drive-by: why don't we pass ScriptBreakpoint* around so these methods can modify
it directly?

[Clemens Hammacher, 2017/01/26 09:57:48]

It's not only ScriptBreakpoint object which need to be translated. It's also
Locations and StackFrames. All of them have a script id, line and column though,
that's why the interface is modeled this way.

+      &translatedBreakpoint.script_id, &translatedBreakpoint.line_number,
+      &translatedBreakpoint.column_number);
+
   m_serverBreakpoints[debuggerBreakpointId] =
       std::make_pair(breakpointId, source);
   CHECK(!breakpointId.isEmpty());
 
   m_breakpointIdToDebuggerBreakpointIds[breakpointId].push_back(
       debuggerBreakpointId);
-  return buildProtocolLocation(translatedBreakpoint.script_id, actualLineNumber,
-                               actualColumnNumber);
+  return buildProtocolLocation(translatedBreakpoint.script_id,
+                               translatedBreakpoint.line_number,
+                               translatedBreakpoint.column_number);
 }
 
 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 676 matching lines @@
 void V8DebuggerAgentImpl::reset() {
   if (!enabled()) return;
   m_scheduledDebuggerStep = NoStep;
   m_blackboxedPositions.clear();
   resetBlackboxedStateCache();
   m_scripts.clear();
   m_breakpointIdToDebuggerBreakpointIds.clear();
 }
 
 }  // namespace v8_inspector