[inspector] restore provisional breakpoints smarter

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 25 matching lines @@
 using protocol::Runtime::ScriptId;
 using protocol::Runtime::StackTrace;
 using protocol::Runtime::RemoteObject;
 
 namespace DebuggerAgentState {
 static const char javaScriptBreakpoints[] = "javaScriptBreakopints";
 static const char pauseOnExceptionsState[] = "pauseOnExceptionsState";
 static const char asyncCallStackDepth[] = "asyncCallStackDepth";
 static const char blackboxPattern[] = "blackboxPattern";
 static const char debuggerEnabled[] = "debuggerEnabled";
+static const char skipAllPauses[] = "skipAllPauses";
 
 // Breakpoint properties.
 static const char url[] = "url";
 static const char isRegex[] = "isRegex";
 static const char lineNumber[] = "lineNumber";
 static const char columnNumber[] = "columnNumber";
 static const char condition[] = "condition";
-static const char skipAllPauses[] = "skipAllPauses";
+static const char hint[] = "hint";
 
 }  // namespace DebuggerAgentState
 
 static const char kBacktraceObjectGroup[] = "backtrace";
 static const char kDebuggerNotEnabled[] = "Debugger agent is not enabled";
 static const char kDebuggerNotPaused[] =
     "Can only perform operation while paused.";
 
+static const size_t kBreakpointHintMaxLength = 128;
+static const size_t kBreakpointHintMaxSearchOffset = 80 * 10;
+
 namespace {
 
 void TranslateWasmStackTraceLocations(Array<CallFrame>* stackTrace,
                                       WasmTranslation* wasmTranslation) {
   for (size_t i = 0, e = stackTrace->length(); i != e; ++i) {
     protocol::Debugger::Location* location = stackTrace->get(i)->getLocation();
     String16 scriptId = location->getScriptId();
     int lineNumber = location->getLineNumber();
     int columnNumber = location->getColumnNumber(-1);
 
@@ skipping 40 matching lines @@
 
 std::unique_ptr<protocol::Debugger::Location> buildProtocolLocation(
     const String16& scriptId, int lineNumber, int columnNumber) {
   return protocol::Debugger::Location::create()
       .setScriptId(scriptId)
       .setLineNumber(lineNumber)
       .setColumnNumber(columnNumber)
       .build();
 }
 
+String16 breakpointHint(const V8DebuggerScript& script,
+                        const ScriptBreakpoint& breakpoint) {
+  int offset = script.offset(breakpoint.line_number, breakpoint.column_number);
+  if (offset == V8DebuggerScript::kNoOffset) return String16();
+  const String16& source = script.source();
+  String16 hint = source.substring(offset, offset + kBreakpointHintMaxLength)

[alph, 2017/02/27 22:01:29]

the second arg of substring is length

[kozy, 2017/02/27 23:16:35]

ooops, fixed.

+                      .stripWhiteSpace();
+  for (size_t i = 0; i < hint.length(); ++i) {
+    if (hint[i] == '\r' || hint[i] == '\n' || hint[i] == ';') {
+      return hint.substring(0, i);
+    }
+  }
+  return hint;
+}
+
+void adjustBreakpointLocation(const V8DebuggerScript& script,
+                              const String16& hint,
+                              ScriptBreakpoint* breakpoint) {
+  if (hint.isEmpty()) return;
+  size_t offset = static_cast<size_t>(
+      script.offset(breakpoint->line_number, breakpoint->column_number));
+  if (offset == static_cast<size_t>(V8DebuggerScript::kNoOffset)) return;
+  size_t searchAreaOffset = 0;
+  if (offset > kBreakpointHintMaxSearchOffset)
+    searchAreaOffset = offset - kBreakpointHintMaxSearchOffset;
+  String16 searchArea =
+      script.source()
+          .substring(searchAreaOffset, offset + kBreakpointHintMaxSearchOffset)

[alph, 2017/02/27 22:01:29]

The second arg of substring is length.
Also you may want to use 2 * kBreakpointHintMaxSearchOffset

[kozy, 2017/02/27 23:16:35]

Done.

+          .stripWhiteSpace();

[alph, 2017/02/27 22:01:29]

As long as it strips whitespaces from the beginning, you can't map the offsets
to original script anymore.

[kozy, 2017/02/27 23:16:35]

Done.

+  size_t hintOffset = searchArea.find(hint);
+  if (hintOffset == String16::kNotFound) return;
+  size_t bestMatch = hintOffset + searchAreaOffset;
+  size_t bestDistance =
+      bestMatch > offset ? bestMatch - offset : offset - bestMatch;

[alph, 2017/02/27 22:01:29]

I wonder if you can use intptr_t instead of size_t to simplify arithmetic with
negative numbers.

[kozy, 2017/02/27 23:16:35]

Done.

+  while (hintOffset != String16::kNotFound) {

[alph, 2017/02/27 22:01:29]

the condition always holds

[kozy, 2017/02/27 23:16:35]

Done.

+    hintOffset = searchArea.find(hint, hintOffset + 1);
+    if (hintOffset == String16::kNotFound) break;
+    size_t hintDistance = hintOffset + searchAreaOffset > offset

[alph, 2017/02/27 22:01:29]

You don't need to do it in a loop. Just two checks with find and reverseFind
should be enough. right?

[kozy, 2017/02/27 23:16:35]

Done.

+                              ? hintOffset + searchAreaOffset - offset
+                              : offset - (hintOffset + searchAreaOffset);
+    if (hintDistance < bestDistance) {
+      bestMatch = hintOffset + searchAreaOffset;
+      bestDistance = hintDistance;
+    } else {
+      break;
+    }
+  }
+  if (bestMatch == String16::kNotFound) return;

[alph, 2017/02/27 22:01:29]

always false

[kozy, 2017/02/27 23:16:35]

Done.

+  v8::debug::Location hintPosition = script.location(bestMatch);
+  if (hintPosition.IsEmpty()) return;
+  breakpoint->line_number = hintPosition.GetLineNumber();
+  breakpoint->column_number = hintPosition.GetColumnNumber();
+}
 }  // namespace
 
 V8DebuggerAgentImpl::V8DebuggerAgentImpl(
     V8InspectorSessionImpl* session, protocol::FrontendChannel* frontendChannel,
     protocol::DictionaryValue* state)
     : m_inspector(session->inspector()),
       m_debugger(m_inspector->debugger()),
       m_session(session),
       m_enabled(false),
       m_state(state),
@@ skipping 100 matching lines @@
 
 Response V8DebuggerAgentImpl::setSkipAllPauses(bool skip) {
   m_state->setBoolean(DebuggerAgentState::skipAllPauses, skip);
   m_skipAllPauses = skip;
   return Response::OK();
 }
 
 static std::unique_ptr<protocol::DictionaryValue>
 buildObjectForBreakpointCookie(const String16& url, int lineNumber,
                                int columnNumber, const String16& condition,
-                               bool isRegex) {
+                               bool isRegex, const String16& hint) {
   std::unique_ptr<protocol::DictionaryValue> breakpointObject =
       protocol::DictionaryValue::create();
   breakpointObject->setString(DebuggerAgentState::url, url);
   breakpointObject->setInteger(DebuggerAgentState::lineNumber, lineNumber);
   breakpointObject->setInteger(DebuggerAgentState::columnNumber, columnNumber);
   breakpointObject->setString(DebuggerAgentState::condition, condition);
   breakpointObject->setBoolean(DebuggerAgentState::isRegex, isRegex);
+  if (!hint.isEmpty()) {
+    breakpointObject->setString(DebuggerAgentState::hint, hint);
+  }
   return breakpointObject;
 }
 
 static bool matches(V8InspectorImpl* inspector, const String16& url,
                     const String16& pattern, bool isRegex) {
   if (isRegex) {
     V8Regex regex(inspector, pattern, true);
     return regex.match(url) != -1;
   }
   return url == pattern;
@@ skipping 26 matching lines @@
   if (!breakpointsCookie) {
     std::unique_ptr<protocol::DictionaryValue> newValue =
         protocol::DictionaryValue::create();
     breakpointsCookie = newValue.get();
     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));
-
+  String16 hint;
   ScriptBreakpoint breakpoint(String16(), lineNumber, columnNumber, condition);
   for (const auto& script : m_scripts) {
     if (!matches(m_inspector, script.second->sourceURL(), url, isRegex))
       continue;
     breakpoint.script_id = script.first;
     std::unique_ptr<protocol::Debugger::Location> location =
-        resolveBreakpoint(breakpointId, breakpoint, UserBreakpointSource);
+        resolveBreakpoint(breakpointId, breakpoint, UserBreakpointSource, hint);
+    if (!isRegex) hint = breakpointHint(*script.second, breakpoint);
     if (location) (*locations)->addItem(std::move(location));
   }
 
+  breakpointsCookie->setObject(
+      breakpointId,
+      buildObjectForBreakpointCookie(url, lineNumber, columnNumber, condition,
+                                     isRegex, hint));
+
   *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) {
   ScriptBreakpoint breakpoint(
       location->getScriptId(), location->getLineNumber(),
       location->getColumnNumber(0), optionalCondition.fromMaybe(String16()));
 
   String16 breakpointId =
       generateBreakpointId(breakpoint, UserBreakpointSource);
   if (m_breakpointIdToDebuggerBreakpointIds.find(breakpointId) !=
       m_breakpointIdToDebuggerBreakpointIds.end()) {
     return Response::Error("Breakpoint at specified location already exists.");
   }
-  *actualLocation =
-      resolveBreakpoint(breakpointId, breakpoint, UserBreakpointSource);
+  *actualLocation = resolveBreakpoint(
+      breakpointId, breakpoint, UserBreakpointSource, /* hint */ String16());
   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 110 matching lines @@
   // Ranges array contains positions in script where blackbox state is changed.
   // [(0,0) ... ranges[0]) isn't blackboxed, [ranges[0] ... ranges[1]) is
   // blackboxed...
   return itStartRange == itEndRange &&
          std::distance(ranges.begin(), itStartRange) % 2;
 }
 
 std::unique_ptr<protocol::Debugger::Location>
 V8DebuggerAgentImpl::resolveBreakpoint(const String16& breakpointId,
                                        const ScriptBreakpoint& breakpoint,
-                                       BreakpointSource source) {
+                                       BreakpointSource source,
+                                       const String16& hint) {
   v8::HandleScope handles(m_isolate);
   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;
+  adjustBreakpointLocation(*scriptIterator->second, hint,
+                           &translatedBreakpoint);
   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);
   if (debuggerBreakpointId.isEmpty()) return nullptr;
 
@@ skipping 14 matching lines @@
 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);
 
   std::vector<std::unique_ptr<protocol::Debugger::SearchMatch>> matches =
-      searchInTextByLinesImpl(m_session, it->second->source(m_isolate), query,
+      searchInTextByLinesImpl(m_session, it->second->source(), query,
                               optionalCaseSensitive.fromMaybe(false),
                               optionalIsRegex.fromMaybe(false));
   *results = protocol::Array<protocol::Debugger::SearchMatch>::create();
   for (size_t i = 0; i < matches.size(); ++i)
     (*results)->addItem(std::move(matches[i]));
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::setScriptSource(
     const String16& scriptId, const String16& newContent, Maybe<bool> dryRun,
@@ skipping 12 matching lines @@
   }
 
   v8::HandleScope handles(m_isolate);
   v8::Local<v8::String> newSource = toV8String(m_isolate, newContent);
   bool compileError = false;
   Response response = m_debugger->setScriptSource(
       scriptId, newSource, dryRun.fromMaybe(false), optOutCompileError,
       &m_pausedCallFrames, stackChanged, &compileError);
   if (!response.isSuccess() || compileError) return response;
 
-  it->second->setSource(newSource);
+  it->second->setSource(newContent);
   std::unique_ptr<Array<CallFrame>> callFrames;
   response = currentCallFrames(&callFrames);
   if (!response.isSuccess()) return response;
   *newCallFrames = std::move(callFrames);
   *asyncStackTrace = currentAsyncStackTrace();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::restartFrame(
     const String16& callFrameId,
@@ skipping 24 matching lines @@
   *asyncStackTrace = currentAsyncStackTrace();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::getScriptSource(const String16& scriptId,
                                               String16* scriptSource) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
   ScriptsMap::iterator it = m_scripts.find(scriptId);
   if (it == m_scripts.end())
     return Response::Error("No script for id: " + scriptId);
-  v8::HandleScope handles(m_isolate);
-  *scriptSource = it->second->source(m_isolate);
+  *scriptSource = it->second->source();
   return Response::OK();
 }
 
 void V8DebuggerAgentImpl::pushBreakDetails(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> breakAuxData) {
   m_breakReason.push_back(std::make_pair(breakReason, std::move(breakAuxData)));
 }
 
 void V8DebuggerAgentImpl::popBreakDetails() {
@@ skipping 388 matching lines @@
   V8StackTraceImpl* stackTrace = m_debugger->currentAsyncCallChain();
   return stackTrace ? stackTrace->buildInspectorObjectForTail(m_debugger)
                     : nullptr;
 }
 
 bool V8DebuggerAgentImpl::isPaused() const { return m_debugger->isPaused(); }
 
 void V8DebuggerAgentImpl::didParseSource(
     std::unique_ptr<V8DebuggerScript> script, bool success) {
   v8::HandleScope handles(m_isolate);
-  String16 scriptSource = script->source(m_isolate);
+  String16 scriptSource = script->source();
   if (!success) script->setSourceURL(findSourceURL(scriptSource, false));
   if (!success)
     script->setSourceMappingURL(findSourceMapURL(scriptSource, false));
 
   int contextId = script->executionContextId();
   int contextGroupId = m_inspector->contextGroupId(contextId);
   InspectedContext* inspected =
       m_inspector->getContext(contextGroupId, contextId);
   std::unique_ptr<protocol::DictionaryValue> executionContextAuxData;
   if (inspected) {
@@ skipping 22 matching lines @@
   Maybe<String16> sourceMapURLParam = scriptRef->sourceMappingURL();
   Maybe<protocol::DictionaryValue> executionContextAuxDataParam(
       std::move(executionContextAuxData));
   const bool* isLiveEditParam = isLiveEdit ? &isLiveEdit : nullptr;
   const bool* hasSourceURLParam = hasSourceURL ? &hasSourceURL : nullptr;
   const bool* isModuleParam = isModule ? &isModule : nullptr;
   if (success)
     m_frontend.scriptParsed(
         scriptId, scriptURL, scriptRef->startLine(), scriptRef->startColumn(),
         scriptRef->endLine(), scriptRef->endColumn(), contextId,
-        scriptRef->hash(m_isolate), std::move(executionContextAuxDataParam),
+        scriptRef->hash(), std::move(executionContextAuxDataParam),
         isLiveEditParam, std::move(sourceMapURLParam), hasSourceURLParam,
         isModuleParam);
   else
     m_frontend.scriptFailedToParse(
         scriptId, scriptURL, scriptRef->startLine(), scriptRef->startColumn(),
         scriptRef->endLine(), scriptRef->endColumn(), contextId,
-        scriptRef->hash(m_isolate), std::move(executionContextAuxDataParam),
+        scriptRef->hash(), std::move(executionContextAuxDataParam),
         std::move(sourceMapURLParam), hasSourceURLParam, isModuleParam);
 
   if (scriptURL.isEmpty() || !success) return;
 
   protocol::DictionaryValue* breakpointsCookie =
       m_state->getObject(DebuggerAgentState::javaScriptBreakpoints);
   if (!breakpointsCookie) return;
 
   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.line_number);
     breakpointObject->getInteger(DebuggerAgentState::columnNumber,
                                  &breakpoint.column_number);
     breakpointObject->getString(DebuggerAgentState::condition,
                                 &breakpoint.condition);
+    String16 hint;
+    bool hasHint = breakpointObject->getString(DebuggerAgentState::hint, &hint);
     std::unique_ptr<protocol::Debugger::Location> location =
-        resolveBreakpoint(cookie.first, breakpoint, UserBreakpointSource);
+        resolveBreakpoint(cookie.first, breakpoint, UserBreakpointSource, hint);
+    if (!hasHint) {
+      hint = breakpointHint(*scriptRef, breakpoint);
+      if (!hint.isEmpty())
+        breakpointObject->setString(DebuggerAgentState::hint, hint);
+    }
     if (location)
       m_frontend.breakpointResolved(cookie.first, std::move(location));
   }
 }
 
 void V8DebuggerAgentImpl::didPause(int contextId,
                                    v8::Local<v8::Value> exception,
                                    const std::vector<String16>& hitBreakpoints,
                                    bool isPromiseRejection, bool isUncaught,
                                    bool isOOMBreak) {
@@ skipping 118 matching lines @@
     return;
   breakProgram(breakReason, std::move(data));
 }
 
 void V8DebuggerAgentImpl::setBreakpointAt(const String16& scriptId,
                                           int lineNumber, int columnNumber,
                                           BreakpointSource source,
                                           const String16& condition) {
   ScriptBreakpoint breakpoint(scriptId, lineNumber, columnNumber, condition);
   String16 breakpointId = generateBreakpointId(breakpoint, source);
-  resolveBreakpoint(breakpointId, breakpoint, source);
+  resolveBreakpoint(breakpointId, breakpoint, source, /* hint */ String16());
 }
 
 void V8DebuggerAgentImpl::removeBreakpointAt(const String16& scriptId,
                                              int lineNumber, int columnNumber,
                                              BreakpointSource source) {
   removeBreakpointImpl(generateBreakpointId(
       ScriptBreakpoint(scriptId, lineNumber, columnNumber, String16()),
       source));
 }
 
 void V8DebuggerAgentImpl::reset() {
   if (!enabled()) return;
   m_scheduledDebuggerStep = NoStep;
   m_blackboxedPositions.clear();
   resetBlackboxedStateCache();
   m_scripts.clear();
   m_breakpointIdToDebuggerBreakpointIds.clear();
 }
 
 }  // namespace v8_inspector