[inspector] provisional breakpoints for anonymous script

Use case: anonymous script with sourceMappingUrl. User can set
breakpoint in source with sourceUrl from sourceMap, we persist this
breakpoint in DevTools and on page reload breakpoint should be restored
correctly.

Debugger.setBreakpointByUrl method provides capabilities to set
provisional breakpoints and looks like best candidate for new "scriptHash"
argument.

I considered other options such as replacing scriptId with something
more persistent like "script-hash:script-with-this-hash-number" but it
looks more complicated and doesn't provide clear advantages.

One pager: http://bit.ly/2wkRHnt

R=pfeldman@chromium.org

Bug: chromium:459499
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0e2833fceffe6b04afac01d1a4522d6874b6067a
Reviewed-on: https://chromium-review.googlesource.com/683597
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48357}

src/inspector/js_protocol.json

@@ -527,6 +527,7 @@
                     { "name": "lineNumber", "type": "integer", "description": "Line number to set breakpoint at." },
                     { "name": "url", "type": "string", "optional": true, "description": "URL of the resources to set breakpoint on." },
                     { "name": "urlRegex", "type": "string", "optional": true, "description": "Regex pattern for the URLs of the resources to set breakpoints on. Either <code>url</code> or <code>urlRegex</code> must be specified." },
+                    { "name": "scriptHash", "type": "string", "optional": true, "experimental": true, "description": "Script hash of the resources to set breakpoint on." },
                     { "name": "columnNumber", "type": "integer", "optional": true, "description": "Offset in the line to set breakpoint at." },
                     { "name": "condition", "type": "string", "optional": true, "description": "Expression to use as a breakpoint condition. When specified, debugger will only stop on the breakpoint if this expression evaluates to true." }
                 ],

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

@@ -44,6 +44,7 @@ static const char skipAllPauses[] = "skipAllPauses";
 
 static const char breakpointsByRegex[] = "breakpointsByRegex";
 static const char breakpointsByUrl[] = "breakpointsByUrl";
+static const char breakpointsByScriptHash[] = "breakpointsByScriptHash";
 static const char breakpointHints[] = "breakpointHints";
 
 }  // namespace DebuggerAgentState
@@ -74,6 +75,7 @@ void TranslateLocation(protocol::Debugger::Location* location,
 enum class BreakpointType {
   kByUrl = 1,
   kByUrlRegex,
+  kByScriptHash,
   kByScriptId,
   kDebugCommand,
   kMonitorCommand
@@ -383,6 +385,7 @@ Response V8DebuggerAgentImpl::disable() {
 
   m_state->remove(DebuggerAgentState::breakpointsByRegex);
   m_state->remove(DebuggerAgentState::breakpointsByUrl);
+  m_state->remove(DebuggerAgentState::breakpointsByScriptHash);
   m_state->remove(DebuggerAgentState::breakpointHints);
 
   m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState,
@@ -460,45 +463,80 @@ Response V8DebuggerAgentImpl::setSkipAllPauses(bool skip) {
   return Response::OK();
 }
 
-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;
+static bool matches(V8InspectorImpl* inspector, const V8DebuggerScript& script,
+                    BreakpointType type, const String16& selector) {
+  switch (type) {
+    case BreakpointType::kByUrl:
+      return script.sourceURL() == selector;
+    case BreakpointType::kByScriptHash:
+      return script.hash() == selector;
+    case BreakpointType::kByUrlRegex: {
+      V8Regex regex(inspector, selector, true);
+      return regex.match(script.sourceURL()) != -1;
+    }
+    default:
+      UNREACHABLE();
+      return false;
   }
-  return url == pattern;
 }
 
 Response V8DebuggerAgentImpl::setBreakpointByUrl(
     int lineNumber, Maybe<String16> optionalURL,
-    Maybe<String16> optionalURLRegex, Maybe<int> optionalColumnNumber,
-    Maybe<String16> optionalCondition, String16* outBreakpointId,
+    Maybe<String16> optionalURLRegex, Maybe<String16> optionalScriptHash,
+    Maybe<int> optionalColumnNumber, Maybe<String16> optionalCondition,
+    String16* outBreakpointId,
     std::unique_ptr<protocol::Array<protocol::Debugger::Location>>* locations) {
   *locations = Array<protocol::Debugger::Location>::create();
-  if (optionalURL.isJust() == optionalURLRegex.isJust())
-    return Response::Error("Either url or urlRegex must be specified.");
 
-  String16 url = optionalURL.isJust() ? optionalURL.fromJust()
-                                      : optionalURLRegex.fromJust();
+  int specified = (optionalURL.isJust() ? 1 : 0) +
+                  (optionalURLRegex.isJust() ? 1 : 0) +
+                  (optionalScriptHash.isJust() ? 1 : 0);
+  if (specified != 1) {
+    return Response::Error(
+        "Either url or urlRegex or scriptHash must be specified.");
+  }
   int columnNumber = 0;
   if (optionalColumnNumber.isJust()) {
     columnNumber = optionalColumnNumber.fromJust();
     if (columnNumber < 0) return Response::Error("Incorrect column number");
   }
-  String16 condition = optionalCondition.fromMaybe("");
-  bool isRegex = optionalURLRegex.isJust();
 
-  String16 breakpointId = generateBreakpointId(
-      isRegex ? BreakpointType::kByUrlRegex : BreakpointType::kByUrl, url,
-      lineNumber, columnNumber);
+  BreakpointType type = BreakpointType::kByUrl;
+  String16 selector;
+  if (optionalURLRegex.isJust()) {
+    selector = optionalURLRegex.fromJust();
+    type = BreakpointType::kByUrlRegex;
+  } else if (optionalURL.isJust()) {
+    selector = optionalURL.fromJust();
+    type = BreakpointType::kByUrl;
+  } else if (optionalScriptHash.isJust()) {
+    selector = optionalScriptHash.fromJust();
+    type = BreakpointType::kByScriptHash;
+  }
+
+  String16 condition = optionalCondition.fromMaybe(String16());
+  String16 breakpointId =
+      generateBreakpointId(type, selector, lineNumber, columnNumber);
   protocol::DictionaryValue* breakpoints;
-  if (isRegex) {
-    breakpoints =
-        getOrCreateObject(m_state, DebuggerAgentState::breakpointsByRegex);
-  } else {
-    protocol::DictionaryValue* breakpointsByUrl =
-        getOrCreateObject(m_state, DebuggerAgentState::breakpointsByUrl);
-    breakpoints = getOrCreateObject(breakpointsByUrl, url);
+  switch (type) {
+    case BreakpointType::kByUrlRegex:
+      breakpoints =
+          getOrCreateObject(m_state, DebuggerAgentState::breakpointsByRegex);
+      break;
+    case BreakpointType::kByUrl:
+      breakpoints = getOrCreateObject(
+          getOrCreateObject(m_state, DebuggerAgentState::breakpointsByUrl),
+          selector);
+      break;
+    case BreakpointType::kByScriptHash:
+      breakpoints = getOrCreateObject(
+          getOrCreateObject(m_state,
+                            DebuggerAgentState::breakpointsByScriptHash),
+          selector);
+      break;
+    default:
+      UNREACHABLE();
+      break;
   }
   if (breakpoints->get(breakpointId)) {
     return Response::Error("Breakpoint at specified location already exists.");
@@ -506,16 +544,16 @@ Response V8DebuggerAgentImpl::setBreakpointByUrl(
 
   String16 hint;
   for (const auto& script : m_scripts) {
-    if (!matches(m_inspector, script.second->sourceURL(), url, isRegex))
-      continue;
+    if (!matches(m_inspector, *script.second, type, selector)) continue;
     if (!hint.isEmpty()) {
       adjustBreakpointLocation(*script.second, hint, &lineNumber,
                                &columnNumber);
     }
     std::unique_ptr<protocol::Debugger::Location> location = setBreakpointImpl(
         breakpointId, script.first, condition, lineNumber, columnNumber);
-    if (!isRegex)
+    if (type != BreakpointType::kByUrlRegex) {
       hint = breakpointHint(*script.second, lineNumber, columnNumber);
+    }
     if (location) (*locations)->addItem(std::move(location));
   }
   breakpoints->setString(breakpointId, condition);
@@ -556,14 +594,26 @@ Response V8DebuggerAgentImpl::removeBreakpoint(const String16& breakpointId) {
     return Response::OK();
   }
   protocol::DictionaryValue* breakpoints = nullptr;
-  if (type == BreakpointType::kByUrl) {
-    protocol::DictionaryValue* breakpointsByUrl =
-        m_state->getObject(DebuggerAgentState::breakpointsByUrl);
-    if (breakpointsByUrl) {
-      breakpoints = breakpointsByUrl->getObject(selector);
-    }
-  } else if (type == BreakpointType::kByUrlRegex) {
-    breakpoints = m_state->getObject(DebuggerAgentState::breakpointsByRegex);
+  switch (type) {
+    case BreakpointType::kByUrl: {
+      protocol::DictionaryValue* breakpointsByUrl =
+          m_state->getObject(DebuggerAgentState::breakpointsByUrl);
+      if (breakpointsByUrl) {
+        breakpoints = breakpointsByUrl->getObject(selector);
+      }
+    } break;
+    case BreakpointType::kByScriptHash: {
+      protocol::DictionaryValue* breakpointsByScriptHash =
+          m_state->getObject(DebuggerAgentState::breakpointsByScriptHash);
+      if (breakpointsByScriptHash) {
+        breakpoints = breakpointsByScriptHash->getObject(selector);
+      }
+    } break;
+    case BreakpointType::kByUrlRegex:
+      breakpoints = m_state->getObject(DebuggerAgentState::breakpointsByRegex);
+      break;
+    default:
+      break;
   }
   if (breakpoints) breakpoints->remove(breakpointId);
   protocol::DictionaryValue* breakpointHints =
@@ -1266,16 +1316,24 @@ void V8DebuggerAgentImpl::didParseSource(
         static_cast<int>(scriptRef->source().length()), std::move(stackTrace));
   }
 
-  if (scriptURL.isEmpty() || !success) return;
+  if (!success) return;
 
   std::vector<protocol::DictionaryValue*> potentialBreakpoints;
-  protocol::DictionaryValue* breakpointsByUrl =
-      m_state->getObject(DebuggerAgentState::breakpointsByUrl);
-  if (breakpointsByUrl) {
-    potentialBreakpoints.push_back(breakpointsByUrl->getObject(scriptURL));
+  if (!scriptURL.isEmpty()) {
+    protocol::DictionaryValue* breakpointsByUrl =
+        m_state->getObject(DebuggerAgentState::breakpointsByUrl);
+    if (breakpointsByUrl) {
+      potentialBreakpoints.push_back(breakpointsByUrl->getObject(scriptURL));
+    }
+    potentialBreakpoints.push_back(
+        m_state->getObject(DebuggerAgentState::breakpointsByRegex));
+  }
+  protocol::DictionaryValue* breakpointsByScriptHash =
+      m_state->getObject(DebuggerAgentState::breakpointsByScriptHash);
+  if (breakpointsByScriptHash) {
+    potentialBreakpoints.push_back(
+        breakpointsByScriptHash->getObject(scriptRef->hash()));
   }
-  potentialBreakpoints.push_back(
-      m_state->getObject(DebuggerAgentState::breakpointsByRegex));
   protocol::DictionaryValue* breakpointHints =
       m_state->getObject(DebuggerAgentState::breakpointHints);
   for (auto breakpoints : potentialBreakpoints) {
@@ -1291,9 +1349,7 @@ void V8DebuggerAgentImpl::didParseSource(
       parseBreakpointId(breakpointId, &type, &selector, &lineNumber,
                         &columnNumber);
 
-      bool isRegex = type == BreakpointType::kByUrlRegex;
-      if (!matches(m_inspector, scriptURL, selector, isRegex)) continue;
-
+      if (!matches(m_inspector, *scriptRef, type, selector)) continue;
       String16 condition;
       breakpointWithCondition.second->asString(&condition);
       String16 hint;

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

@@ -45,8 +45,9 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
   Response setSkipAllPauses(bool skip) override;
   Response setBreakpointByUrl(
       int lineNumber, Maybe<String16> optionalURL,
-      Maybe<String16> optionalURLRegex, Maybe<int> optionalColumnNumber,
-      Maybe<String16> optionalCondition, String16*,
+      Maybe<String16> optionalURLRegex, Maybe<String16> optionalScriptHash,
+      Maybe<int> optionalColumnNumber, Maybe<String16> optionalCondition,
+      String16*,
       std::unique_ptr<protocol::Array<protocol::Debugger::Location>>* locations)
       override;
   Response setBreakpoint(

test/inspector/debugger/provisional-breakpoint-for-anonymous-script-expected.txt

+Checks provisional breakpoints by hash in anonymous scripts
+
+Running test: testNextScriptParsed
+function foo(){#}
+
+
+Running test: testPreviousScriptParsed
+var list = list ? list.concat(foo) : [foo]; function foo(){#}
+
+var list = list ? list.concat(foo) : [foo]; function foo(){#}
+

test/inspector/debugger/provisional-breakpoint-for-anonymous-script.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.
+
+let {session, contextGroup, Protocol} =
+  InspectorTest.start('Checks provisional breakpoints by hash in anonymous scripts');
+session.setupScriptMap();
+
+InspectorTest.runAsyncTestSuite([
+  async function testNextScriptParsed() {
+    await Protocol.Debugger.enable();
+    // set breakpoint in anonymous script..
+    Protocol.Runtime.evaluate({expression: 'function foo(){}'});
+    let {params:{hash}} = await Protocol.Debugger.onceScriptParsed();
+    let {result:{breakpointId}} = await Protocol.Debugger.setBreakpointByUrl({
+      scriptHash: hash,
+      lineNumber: 0,
+      columnNumber: 15
+    });
+    // evaluate the same anonymous script again..
+    Protocol.Runtime.evaluate({expression: 'function foo(){}'});
+    // run function and check Debugger.paused event..
+    let evaluation = Protocol.Runtime.evaluate({expression: 'foo()'});
+    let result = await Promise.race([evaluation, Protocol.Debugger.oncePaused()]);
+    if (result.method !== 'Debugger.paused') {
+      InspectorTest.log('FAIL: breakpoint was ignored');
+    } else {
+      await session.logSourceLocation(result.params.callFrames[0].location);
+    }
+    // remove breakpoint and run again..
+    await Protocol.Debugger.removeBreakpoint({breakpointId});
+    evaluation = Protocol.Runtime.evaluate({expression: 'foo()'});
+    result = await Promise.race([evaluation, Protocol.Debugger.oncePaused()]);
+    if (result.method === 'Debugger.paused') {
+      InspectorTest.log('FAIL: breakpoint was not removed');
+    }
+    await Protocol.Debugger.disable();
+  },
+  async function testPreviousScriptParsed() {
+    await Protocol.Debugger.enable();
+    // run script and store function to global list..
+    await Protocol.Runtime.evaluate({expression: 'var list = list ? list.concat(foo) : [foo]; function foo(){}'});
+    // run same script again..
+    Protocol.Runtime.evaluate({expression: 'var list = list ? list.concat(foo) : [foo]; function foo(){}'});
+    let {params:{hash}} = await Protocol.Debugger.onceScriptParsed();
+    // set breakpoint by hash of latest script..
+    let {result:{breakpointId}} = await Protocol.Debugger.setBreakpointByUrl({
+      scriptHash: hash,
+      lineNumber: 0,
+      columnNumber: 49
+    });
+    // call each function in global list and wait for Debugger.paused events..
+    let evaluation = Protocol.Runtime.evaluate({expression: 'list.forEach(x => x())'});
+    let result = await Promise.race([evaluation, Protocol.Debugger.oncePaused()]);
+    while (result.method === 'Debugger.paused') {
+      await session.logSourceLocation(result.params.callFrames[0].location);
+      Protocol.Debugger.resume();
+      result = await Promise.race([evaluation, Protocol.Debugger.oncePaused()]);
+    }
+    // remove breakpoint and call functions again..
+    await Protocol.Debugger.removeBreakpoint({breakpointId});
+    evaluation = Protocol.Runtime.evaluate({expression: 'foo()'});
+    result = await Promise.race([evaluation, Protocol.Debugger.oncePaused()]);
+    if (result.method === 'Debugger.paused') {
+      InspectorTest.log('FAIL: breakpoint was not removed');
+    }
+    await Protocol.Debugger.disable();
+  }
+]);

test/inspector/protocol-test.js

@@ -45,7 +45,7 @@ InspectorTest.logMessage = function(originalMessage) {
     for (var key in object) {
       if (nonStableFields.has(key))
         object[key] = `<${key}>`;
-      else if (typeof object[key] === "string" && object[key].match(/4:\d+:\d+:\d+/))
+      else if (typeof object[key] === "string" && object[key].match(/\d+:\d+:\d+:\d+/))
         object[key] = object[key].substring(0, object[key].lastIndexOf(':')) + ":<scriptId>";
       else if (typeof object[key] === "object")
         objects.push(object[key]);

test/inspector/runtime/command-line-api-expected.txt

@@ -255,12 +255,12 @@ Running test: testDebug
 foo (:0:16)
 (anonymous) (:0:0)
 [
-    [0] : 4:0:12:<scriptId>
+    [0] : 5:0:12:<scriptId>
 ]
 foo (:0:16)
 (anonymous) (:0:0)
 [
-    [0] : 4:0:12:<scriptId>
+    [0] : 5:0:12:<scriptId>
 ]
 
 Running test: testMonitor