[inspector] restore provisional breakpoints smarter

For breakpoints which are set by setBreakpointByUrl(url:..) backend calculates source hint on first related breakpoints resolved event and then uses this hint to adjust breakpoint position in later arrived scripts with the same url or on page reload.

Doc: https://docs.google.com/a/google.com/document/d/1VtWo_-jelzEXSNbjESGTtruZngzXgbHLexfTzxNlnjE/edit?usp=sharing

BUG=chromium:688776
R=pfeldman@chromium.org, alph@chromium.org

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

src/api.cc

@@ -9257,13 +9257,10 @@ bool debug::Script::GetPossibleBreakpoints(
       i::Handle<i::FixedArray>::cast(i::handle(script->line_ends(), isolate));
   CHECK(line_ends->length());
 
-  int start_offset = GetSourcePosition(start);
-  int end_offset;
-  if (end.IsEmpty()) {
-    end_offset = GetSmiValue(line_ends, line_ends->length() - 1) + 1;
-  } else {
-    end_offset = GetSourcePosition(end);
-  }
+  int start_offset = GetSourceOffset(start);
+  int end_offset = end.IsEmpty()
+                       ? GetSmiValue(line_ends, line_ends->length() - 1) + 1
+                       : GetSourceOffset(end);
   if (start_offset >= end_offset) return true;
 
   std::set<int> offsets;
@@ -9292,7 +9289,7 @@ bool debug::Script::GetPossibleBreakpoints(
   return true;
 }
 
-int debug::Script::GetSourcePosition(const debug::Location& location) const {
+int debug::Script::GetSourceOffset(const debug::Location& location) const {
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   if (script->type() == i::Script::TYPE_WASM) {
     // TODO(clemensh): Return the proper thing for wasm.
@@ -9318,6 +9315,17 @@ int debug::Script::GetSourcePosition(const debug::Location& location) const {
   return std::min(prev_line_offset + column + 1, line_offset);
 }
 
+v8::debug::Location debug::Script::GetSourceLocation(int offset) const {
+  i::Handle<i::Script> script = Utils::OpenHandle(this);
+  if (script->type() == i::Script::TYPE_WASM) {
+    // TODO(clemensh): Return the proper thing for wasm.
+    return v8::debug::Location();
+  }
+  i::Script::PositionInfo info;
+  i::Script::GetPositionInfo(script, offset, &info, i::Script::WITH_OFFSET);
+  return debug::Location(info.line, info.column);
+}
+
 debug::WasmScript* debug::WasmScript::Cast(debug::Script* script) {
   CHECK(script->IsWasm());
   return static_cast<WasmScript*>(script);

src/debug/debug-interface.h

@@ -139,9 +139,8 @@ class V8_EXPORT_PRIVATE Script {
                               const debug::Location& end,
                               bool restrict_to_function,
                               std::vector<debug::Location>* locations) const;
-
- private:
-  int GetSourcePosition(const debug::Location& location) const;
+  int GetSourceOffset(const debug::Location& location) const;
+  v8::debug::Location GetSourceLocation(int offset) const;
 };
 
 // Specialization for wasm Scripts.

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

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

src/inspector/v8-debugger-script.cc

@@ -133,10 +133,9 @@ class ActualScript : public V8DebuggerScript {
     }
 
     if (script->Source().ToLocal(&tmp)) {
-      m_sourceObj.Reset(m_isolate, tmp);
-      String16 source = toProtocolString(tmp);
+      m_source = toProtocolString(tmp);
       // V8 will not count last line if script source ends with \n.
-      if (source.length() > 1 && source[source.length() - 1] == '\n') {
+      if (m_source.length() > 0 && m_source[m_source.length() - 1] == '\n') {
         m_endLine++;
         m_endColumn = 0;
       }
@@ -154,22 +153,10 @@ class ActualScript : public V8DebuggerScript {
     return m_sourceMappingURL;
   }
 
-  String16 source(v8::Isolate* isolate) const override {
-    if (!m_sourceObj.IsEmpty())
-      return toProtocolString(m_sourceObj.Get(isolate));
-    return V8DebuggerScript::source(isolate);
-  }
-
   void setSourceMappingURL(const String16& sourceMappingURL) override {
     m_sourceMappingURL = sourceMappingURL;
   }
 
-  void setSource(v8::Local<v8::String> source) override {
-    m_source = String16();
-    m_sourceObj.Reset(m_isolate, source);
-    m_hash = String16();
-  }
-
   bool getPossibleBreakpoints(
       const v8::debug::Location& start, const v8::debug::Location& end,
       bool restrictToFunction,
@@ -185,6 +172,17 @@ class ActualScript : public V8DebuggerScript {
     v8::debug::ResetBlackboxedStateCache(m_isolate, m_script.Get(m_isolate));
   }
 
+  int offset(int lineNumber, int columnNumber) const override {
+    v8::HandleScope scope(m_isolate);
+    return m_script.Get(m_isolate)->GetSourceOffset(
+        v8::debug::Location(lineNumber, columnNumber));
+  }
+
+  v8::debug::Location location(int offset) const override {
+    v8::HandleScope scope(m_isolate);
+    return m_script.Get(m_isolate)->GetSourceLocation(offset);
+  }
+
  private:
   String16 GetNameOrSourceUrl(v8::Local<v8::debug::Script> script) {
     v8::Local<v8::String> name;
@@ -194,7 +192,6 @@ class ActualScript : public V8DebuggerScript {
   }
 
   String16 m_sourceMappingURL;
-  v8::Global<v8::String> m_sourceObj;
   bool m_isLiveEdit = false;
   bool m_isModule = false;
   v8::Global<v8::debug::Script> m_script;
@@ -261,6 +258,14 @@ class WasmVirtualScript : public V8DebuggerScript {
 
   void resetBlackboxedStateCache() override {}
 
+  int offset(int lineNumber, int columnNumber) const override {
+    return kNoOffset;
+  }
+
+  v8::debug::Location location(int offset) const override {
+    return v8::debug::Location();
+  }
+
  private:
   static const String16& emptyString() {
     static const String16 singleEmptyString;
@@ -299,8 +304,8 @@ const String16& V8DebuggerScript::sourceURL() const {
   return m_sourceURL.isEmpty() ? m_url : m_sourceURL;
 }
 
-const String16& V8DebuggerScript::hash(v8::Isolate* isolate) const {
-  if (m_hash.isEmpty()) m_hash = calculateHash(source(isolate));
+const String16& V8DebuggerScript::hash() const {
+  if (m_hash.isEmpty()) m_hash = calculateHash(source());
   DCHECK(!m_hash.isEmpty());
   return m_hash;
 }

src/inspector/v8-debugger-script.h

@@ -59,8 +59,8 @@ class V8DebuggerScript {
   bool hasSourceURL() const { return !m_sourceURL.isEmpty(); }
   const String16& sourceURL() const;
   virtual const String16& sourceMappingURL() const = 0;
-  virtual String16 source(v8::Isolate*) const { return m_source; }
-  const String16& hash(v8::Isolate*) const;
+  const String16& source() const { return m_source; }
+  const String16& hash() const;
   int startLine() const { return m_startLine; }
   int startColumn() const { return m_startColumn; }
   int endLine() const { return m_endLine; }
@@ -71,8 +71,9 @@ class V8DebuggerScript {
 
   void setSourceURL(const String16&);
   virtual void setSourceMappingURL(const String16&) = 0;
-  virtual void setSource(v8::Local<v8::String> source) {
-    m_source = toProtocolString(source);
+  void setSource(const String16& source) {
+    m_source = source;
+    m_hash = String16();
   }
 
   virtual bool getPossibleBreakpoints(
@@ -81,6 +82,10 @@ class V8DebuggerScript {
       std::vector<v8::debug::Location>* locations) = 0;
   virtual void resetBlackboxedStateCache() = 0;
 
+  static const int kNoOffset = -1;
+  virtual int offset(int lineNumber, int columnNumber) const = 0;
+  virtual v8::debug::Location location(int offset) const = 0;
+
  protected:
   V8DebuggerScript(v8::Isolate*, String16 id, String16 url);
 

test/inspector/debugger/restore-breakpoint-expected.txt

+Checks that debugger agent uses source content to restore breakpoints.
+
+Running test: testSameSource
+function foo() {
+#boo();
+}
+function foo() {
+#boo();
+}
+
+Running test: testOneLineOffset
+function foo() {
+#boo();
+}
+function foo() {
+#boo();
+boo();
+}
+
+Running test: testTwoSimilarLinesCloseToOriginalLocation1
+function foo() {
+
+
+#boo();
+}
+function foo() {
+#boo();
+
+newCode();
+boo();
+
+
+
+boo();
+}
+
+Running test: testTwoSimilarLinesCloseToOriginalLocation2
+function foo() {
+
+
+#boo();
+}
+function foo() {
+boo();
+newLongCode();
+newCode();
+#boo();
+
+
+
+boo();
+}
+
+Running test: testHintIgnoreWhiteSpaces
+function foo() {
+
+
+
+#boo();
+}
+function foo() {
+foo();
+
+
+#boo();
+}
+
+Running test: testCheckOnlyLimitedOffsets
+function foo() {
+#boo();
+}
+function foo() {
+#newCode();
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;...
+boo();
+}

test/inspector/debugger/restore-breakpoint.js

+// Copyright 2017 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.
+
+print('Checks that debugger agent uses source content to restore breakpoints.');
+
+Protocol.Debugger.enable();
+InspectorTest.runTestSuite([
+  function testSameSource(next) {
+    var source = 'function foo() {\nboo();\n}';
+    test(source, source, { lineNumber: 1, columnNumber: 0 }, next);
+  },
+
+  function testOneLineOffset(next) {
+    var source = 'function foo() {\nboo();\n}';
+    var newSource = 'function foo() {\nboo();\nboo();\n}';
+    test(source, newSource, { lineNumber: 1, columnNumber: 0 }, next);
+  },
+
+  function testTwoSimilarLinesCloseToOriginalLocation1(next) {
+    var source = 'function foo() {\n\n\nboo();\n}';
+    var newSource = 'function foo() {\nboo();\n\nnewCode();\nboo();\n\n\n\nboo();\n}';
+    test(source, newSource, { lineNumber: 3, columnNumber: 0 }, next);
+  },
+
+  function testTwoSimilarLinesCloseToOriginalLocation2(next) {
+    var source = 'function foo() {\n\n\nboo();\n}';
+    var newSource = 'function foo() {\nboo();\nnewLongCode();\nnewCode();\nboo();\n\n\n\nboo();\n}';
+    test(source, newSource, { lineNumber: 3, columnNumber: 0 }, next);
+  },
+
+  function testHintIgnoreWhiteSpaces(next) {
+    var source = 'function foo() {\n\n\n\nboo();\n}';
+    var newSource = 'function foo() {\nfoo();\n\n\nboo();\n}';
+    test(source, newSource, { lineNumber: 1, columnNumber: 0 }, next);
+  },
+
+  function testCheckOnlyLimitedOffsets(next) {
+    var source = 'function foo() {\nboo();\n}';
+    var longString = ';'.repeat(1000);
+    var newSource = `function foo() {\nnewCode();\n${longString};\nboo();\n}`;
+    test(source, newSource, { lineNumber: 1, columnNumber: 0 }, next);
+  }
+]);
+
+var finishedTests = 0;
+async function test(source, newSource, location, next) {
+  var firstBreakpoint = true;
+  Protocol.Debugger.onBreakpointResolved(message => {
+    var lineNumber = message.params.location.lineNumber;
+    var columnNumber = message.params.location.columnNumber;
+    var currentSource = firstBreakpoint ? source : newSource;
+    var lines = currentSource.split('\n');
+    lines = lines.map(line => line.length > 80 ? line.substring(0, 77) + '...' : line);
+    lines[lineNumber] = lines[lineNumber].slice(0, columnNumber) + '#' + lines[lineNumber].slice(columnNumber);
+    InspectorTest.log(lines.join('\n'));
+    firstBreakpoint = false;
+  });
+
+  var sourceURL = `test${++finishedTests}.js`;
+  await Protocol.Debugger.setBreakpointByUrl({
+    url: sourceURL,
+    lineNumber: location.lineNumber,
+    columnNumber: location.columnNumber
+  });
+  await Protocol.Runtime.evaluate({ expression: `${source}\n//# sourceURL=${sourceURL}` });
+  await Protocol.Runtime.evaluate({ expression: `${newSource}\n//# sourceURL=${sourceURL}` });
+  next();
+}