[inspector] split DebuggerAgent::breakpointsCookie

This split is required for adding scriptHash argument.

R=dgozman@chromium.org
TBR=machenbach@chromium.org

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

src/inspector/BUILD.gn

@@ -120,7 +120,6 @@ v8_source_set("inspector") {
     "inspected-context.h",
     "remote-object-id.cc",
     "remote-object-id.h",
-    "script-breakpoint.h",
     "search-util.cc",
     "search-util.h",
     "string-16.cc",

src/inspector/inspector.gypi

@@ -39,7 +39,6 @@
       'inspector/inspected-context.h',
       'inspector/remote-object-id.cc',
       'inspector/remote-object-id.h',
-      'inspector/script-breakpoint.h',
       'inspector/search-util.cc',
       'inspector/search-util.h',
       'inspector/string-16.cc',

src/inspector/script-breakpoint.h

-/*
- * Copyright (C) 2009 Apple Inc. All rights reserved.
- * Copyright (C) 2009 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1.  Redistributions of source code must retain the above copyright
- *     notice, this list of conditions and the following disclaimer.
- * 2.  Redistributions in binary form must reproduce the above copyright
- *     notice, this list of conditions and the following disclaimer in the
- *     documentation and/or other materials provided with the distribution.
- * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
- *     its contributors may be used to endorse or promote products derived
- *     from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
- * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
- * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef V8_INSPECTOR_SCRIPTBREAKPOINT_H_
-#define V8_INSPECTOR_SCRIPTBREAKPOINT_H_
-
-#include "src/inspector/string-16.h"
-
-namespace v8_inspector {
-
-struct ScriptBreakpoint {
-  ScriptBreakpoint() {}
-
-  ScriptBreakpoint(String16 script_id, int line_number, int column_number,
-                   String16 condition)
-      : script_id(std::move(script_id)),
-        line_number(line_number),
-        column_number(column_number),
-        condition(std::move(condition)) {}
-
-  String16 script_id;
-  int line_number = 0;
-  int column_number = 0;
-  String16 condition;
-};
-
-}  // namespace v8_inspector
-
-#endif  // V8_INSPECTOR_SCRIPTBREAKPOINT_H_

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

@@ -16,8 +16,6 @@
 namespace v8_inspector {
 
 struct ScriptBreakpoint;
-class JavaScriptCallFrame;
-class PromiseTracker;
 class V8Debugger;
 class V8DebuggerScript;
 class V8InspectorImpl;
@@ -150,9 +148,9 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
 
   void setPauseOnExceptionsImpl(int);
 
-  std::unique_ptr<protocol::Debugger::Location> resolveBreakpoint(
-      const String16& breakpointId, const ScriptBreakpoint&, BreakpointSource,
-      const String16& hint);
+  std::unique_ptr<protocol::Debugger::Location> setBreakpointImpl(
+      const String16& breakpointId, const String16& scriptId,
+      const String16& condition, int lineNumber, int columnNumber);
   void removeBreakpointImpl(const String16& breakpointId);
   void clearBreakDetails();
 
@@ -168,10 +166,8 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
       protocol::HashMap<String16, std::unique_ptr<V8DebuggerScript>>;
   using BreakpointIdToDebuggerBreakpointIdsMap =
       protocol::HashMap<String16, std::vector<v8::debug::BreakpointId>>;
-  using DebugServerBreakpointToBreakpointIdAndSourceMap =
-      protocol::HashMap<v8::debug::BreakpointId,
-                        std::pair<String16, BreakpointSource>>;
-  using MuteBreakpoins = protocol::HashMap<String16, std::pair<String16, int>>;
+  using DebuggerBreakpointIdToBreakpointIdMap =
+      protocol::HashMap<v8::debug::BreakpointId, String16>;
 
   V8InspectorImpl* m_inspector;
   V8Debugger* m_debugger;
@@ -182,7 +178,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
   v8::Isolate* m_isolate;
   ScriptsMap m_scripts;
   BreakpointIdToDebuggerBreakpointIdsMap m_breakpointIdToDebuggerBreakpointIds;
-  DebugServerBreakpointToBreakpointIdAndSourceMap m_serverBreakpoints;
+  DebuggerBreakpointIdToBreakpointIdMap m_debuggerBreakpointIdToBreakpointId;
 
   using BreakReason =
       std::pair<String16, std::unique_ptr<protocol::DictionaryValue>>;

src/inspector/v8-debugger-script.cc

@@ -229,6 +229,13 @@ class ActualScript : public V8DebuggerScript {
     return m_script.Get(m_isolate)->GetSourceLocation(offset);
   }
 
+  bool setBreakpoint(const String16& condition, v8::debug::Location* location,
+                     int* id) const override {
+    v8::HandleScope scope(m_isolate);
+    return script()->SetBreakpoint(toV8String(m_isolate, condition), location,
+                                   id);
+  }
+
  private:
   String16 GetNameOrSourceUrl(v8::Local<v8::debug::Script> script) {
     v8::Local<v8::String> name;
@@ -318,6 +325,22 @@ class WasmVirtualScript : public V8DebuggerScript {
     return v8::debug::Location();
   }
 
+  bool setBreakpoint(const String16& condition, v8::debug::Location* location,
+                     int* id) const override {
+    v8::HandleScope scope(m_isolate);
+    v8::Local<v8::debug::Script> script = m_script.Get(m_isolate);
+    String16 v8ScriptId = String16::fromInteger(script->Id());
+
+    TranslateProtocolLocationToV8Location(m_wasmTranslation, location,
+                                          scriptId(), v8ScriptId);
+    if (location->IsEmpty()) return false;
+    if (!script->SetBreakpoint(toV8String(m_isolate, condition), location, id))
+      return false;
+    TranslateV8LocationToProtocolLocation(m_wasmTranslation, location,
+                                          v8ScriptId, scriptId());
+    return true;
+  }
+
  private:
   static const String16& emptyString() {
     static const String16 singleEmptyString;

src/inspector/v8-debugger-script.h

@@ -84,8 +84,8 @@ class V8DebuggerScript {
   virtual int offset(int lineNumber, int columnNumber) const = 0;
   virtual v8::debug::Location location(int offset) const = 0;
 
-  bool setBreakpoint(const String16& condition, v8::debug::Location* location,
-                     int* id) const;
+  virtual bool setBreakpoint(const String16& condition,
+                             v8::debug::Location* location, int* id) const = 0;
 
  protected:
   V8DebuggerScript(v8::Isolate*, String16 id, String16 url);

src/inspector/v8-debugger.cc

@@ -6,7 +6,6 @@
 
 #include "src/inspector/inspected-context.h"
 #include "src/inspector/protocol/Protocol.h"
-#include "src/inspector/script-breakpoint.h"
 #include "src/inspector/string-util.h"
 #include "src/inspector/v8-debugger-agent-impl.h"
 #include "src/inspector/v8-inspector-impl.h"

src/inspector/v8-debugger.h

@@ -21,7 +21,6 @@
 namespace v8_inspector {
 
 class AsyncStackTrace;
-struct ScriptBreakpoint;
 class StackFrame;
 class V8Debugger;
 class V8DebuggerAgentImpl;

src/inspector/wasm-translation.cc

@@ -7,7 +7,6 @@
 #include <algorithm>
 
 #include "src/debug/debug-interface.h"
-#include "src/inspector/script-breakpoint.h"
 #include "src/inspector/string-util.h"
 #include "src/inspector/v8-debugger-agent-impl.h"
 #include "src/inspector/v8-debugger-script.h"

test/inspector/debugger/breakpoints-expected.txt

+Checks breakpoints.
+
+Running test: testRemoveBreakpoint
+Debugger.removeBreakpoint when agent is disabled:
+{
+    error : {
+        code : -32000
+        message : Debugger agent is not enabled
+    }
+    id : <messageId>
+}
+Remove breakpoint with invalid breakpoint id:
+{
+    id : <messageId>
+    result : {
+    }
+}
+{
+    id : <messageId>
+    result : {
+    }
+}
+
+Running test: testSetBreakpointByUrl
+Adding conditional (arg === 1) breakpoint
+evaluating foo1(0):
+  not paused
+evaluating foo1(1):
+  hit expected breakpoint
+
+Evaluating another script with the same url
+evaluating foo2(0):
+  not paused
+evaluating foo2(1):
+  hit expected breakpoint
+
+Removing breakpoint
+evaluating foo1(1):
+  not paused
+evaluating foo2(1):
+  not paused
+
+Adding breakpoint back
+evaluating foo1(0):
+  not paused
+evaluating foo1(1):
+  hit expected breakpoint
+
+Disabling debugger agent
+evaluating foo1(1):
+  not paused
+evaluating foo2(1):
+  not paused
+
+Enabling debugger agent
+evaluating foo1(1):
+  not paused
+evaluating foo2(1):
+  not paused
+
+Running test: testSetBreakpointInScriptsWithDifferentOffsets
+Adding breakpoint
+evaluating foo1(0):
+  hit expected breakpoint
+evaluating foo2(0):
+  not paused

test/inspector/debugger/breakpoints.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 breakpoints.');
+
+session.setupScriptMap();
+InspectorTest.runAsyncTestSuite([
+  async function testRemoveBreakpoint() {
+    InspectorTest.log('Debugger.removeBreakpoint when agent is disabled:');
+    InspectorTest.logMessage(await Protocol.Debugger.removeBreakpoint({
+      breakpointId: '1:test.js:0:0'
+    }));
+    Protocol.Debugger.enable();
+    InspectorTest.log('Remove breakpoint with invalid breakpoint id:')
+    InspectorTest.logMessage(await Protocol.Debugger.removeBreakpoint({
+      breakpointId: ''
+    }));
+    InspectorTest.logMessage(await Protocol.Debugger.removeBreakpoint({
+      breakpointId: ':::'
+    }));
+    await Protocol.Debugger.disable();
+  },
+
+  async function testSetBreakpointByUrl() {
+    await Protocol.Debugger.enable();
+    Protocol.Runtime.evaluate({expression: `
+function foo1(arg) {
+  return arg;
+}
+//# sourceURL=testSetBreakpointByUrl.js`});
+    InspectorTest.log('Adding conditional (arg === 1) breakpoint');
+    let {result:{breakpointId}} = await Protocol.Debugger.setBreakpointByUrl({
+      lineNumber: 2,
+      url: 'testSetBreakpointByUrl.js',
+      columnNumber: 2,
+      condition: 'arg === 1'
+    });
+    await evaluate('foo1(0)');
+    await evaluate('foo1(1)', breakpointId);
+
+    InspectorTest.log('\nEvaluating another script with the same url')
+    Protocol.Runtime.evaluate({expression: `
+function foo2(arg) {
+  return arg;
+}
+//# sourceURL=testSetBreakpointByUrl.js`});
+    await evaluate('foo2(0)');
+    await evaluate('foo2(1)', breakpointId);
+
+    InspectorTest.log('\nRemoving breakpoint');
+    await Protocol.Debugger.removeBreakpoint({breakpointId});
+    await evaluate('foo1(1)');
+    await evaluate('foo2(1)');
+
+    InspectorTest.log('\nAdding breakpoint back');
+    ({result:{breakpointId}} = await Protocol.Debugger.setBreakpointByUrl({
+      lineNumber: 2,
+      url: 'testSetBreakpointByUrl.js',
+      columnNumber: 2,
+      condition: 'arg === 1'
+    }));
+    await evaluate('foo1(0)');
+    await evaluate('foo1(1)', breakpointId);
+
+    InspectorTest.log('\nDisabling debugger agent');
+    await Protocol.Debugger.disable();
+    await evaluate('foo1(1)');
+    await evaluate('foo2(1)');
+
+    InspectorTest.log('\nEnabling debugger agent');
+    await Protocol.Debugger.enable();
+    await evaluate('foo1(1)');
+    await evaluate('foo2(1)');
+  },
+
+  async function testSetBreakpointInScriptsWithDifferentOffsets() {
+    await Protocol.Debugger.enable();
+    InspectorTest.log('Adding breakpoint');
+    let {result:{breakpointId}} = await Protocol.Debugger.setBreakpointByUrl({
+      lineNumber: 2,
+      url: 'test2.js',
+      columnNumber: 2,
+    });
+    contextGroup.addScript(`
+function foo1(arg) {
+  return arg;
+}
+//# sourceURL=test2.js`);
+    contextGroup.addScript(`
+function foo2(arg) {
+  return arg;
+}
+//# sourceURL=test2.js`, 5);
+    await evaluate('foo1(0)', breakpointId);
+    await evaluate('foo2(0)');
+}
+]);
+
+async function evaluate(expression, expectedBreakpoint) {
+  InspectorTest.log('evaluating ' + expression + ':');
+  let paused = Protocol.Debugger.oncePaused();
+  let evaluate = Protocol.Runtime.evaluate({expression});
+  let result = await Promise.race([paused, evaluate]);
+  if (result.method === 'Debugger.paused') {
+    if (result.params.hitBreakpoints) {
+      if (result.params.hitBreakpoints.find(b => b === expectedBreakpoint)) {
+        InspectorTest.log('  hit expected breakpoint')
+      } else {
+        InspectorTest.log('  hit unexpected breakpoint');
+      }
+    }
+    await Protocol.Debugger.resume();
+  } else {
+    InspectorTest.log('  not paused');
+  }
+}

test/inspector/debugger/restore-breakpoint.js

@@ -5,6 +5,7 @@
 let {session, contextGroup, Protocol} = InspectorTest.start('Checks that debugger agent uses source content to restore breakpoints.');
 
 Protocol.Debugger.enable();
+var finishedTests = 0;
 InspectorTest.runTestSuite([
   function testSameSource(next) {
     var source = 'function foo() {\nboo();\n}';
@@ -43,27 +44,28 @@ InspectorTest.runTestSuite([
   }
 ]);
 
-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');
+  function dumpSourceWithBreakpoint(source, location) {
+    var lineNumber = location.lineNumber;
+    var columnNumber = location.columnNumber;
+    var lines = source.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;
-  });
+  }
+
+  Protocol.Debugger.onBreakpointResolved(message => {
+    dumpSourceWithBreakpoint(newSource, message.params.location);
+  })
 
   var sourceURL = `test${++finishedTests}.js`;
-  await Protocol.Debugger.setBreakpointByUrl({
+  await Protocol.Runtime.evaluate({ expression: `${source}\n//# sourceURL=${sourceURL}` });
+  let {result:{locations}} = await Protocol.Debugger.setBreakpointByUrl({
     url: sourceURL,
     lineNumber: location.lineNumber,
     columnNumber: location.columnNumber
   });
-  await Protocol.Runtime.evaluate({ expression: `${source}\n//# sourceURL=${sourceURL}` });
+  dumpSourceWithBreakpoint(source, locations[0]);
   await Protocol.Runtime.evaluate({ expression: `${newSource}\n//# sourceURL=${sourceURL}` });
   next();
 }

test/inspector/protocol-test.js

@@ -45,8 +45,8 @@ 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(/\d+:\d+:\d+:debug/))
-        object[key] = object[key].replace(/\d+/, '<scriptId>');
+      else if (typeof object[key] === "string" && object[key].match(/4:\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] : <scriptId>:0:12:debug
+    [0] : 4:0:12:<scriptId>
 ]
 foo (:0:16)
 (anonymous) (:0:0)
 [
-    [0] : <scriptId>:0:12:debug
+    [0] : 4:0:12:<scriptId>
 ]
 
 Running test: testMonitor

test/inspector/sessions/debugger-stepping-and-breakpoints-expected.txt

@@ -20,12 +20,12 @@ Setting breakpoints in 2
 Evaluating common breakpoint in 1
 Paused in 1:
   reason: other
-  hit breakpoints: test.js:11:0
+  hit breakpoints: 1:11:0:test.js
   location: foo@11
   data: null
 Paused in 2:
   reason: other
-  hit breakpoints: test.js:11:0
+  hit breakpoints: 1:11:0:test.js
   location: foo@11
   data: null
 Resuming in 1
@@ -48,7 +48,7 @@ Resumed in 2
 Evaluating exclusive breakpoint in 1
 Paused in 1:
   reason: other
-  hit breakpoints: test.js:14:0
+  hit breakpoints: 1:14:0:test.js
   location: baz@14
   data: null
 Paused in 2:
@@ -62,12 +62,12 @@ Resumed in 2
 Evaluating common breakpoint in 2
 Paused in 1:
   reason: other
-  hit breakpoints: test.js:11:0
+  hit breakpoints: 1:11:0:test.js
   location: foo@11
   data: null
 Paused in 2:
   reason: other
-  hit breakpoints: test.js:11:0
+  hit breakpoints: 1:11:0:test.js
   location: foo@11
   data: null
 Resuming in 2
@@ -90,7 +90,7 @@ Resumed in 2
 Evaluating exclusive breakpoint in 2
 Paused in 1:
   reason: other
-  hit breakpoints: test.js:14:0
+  hit breakpoints: 1:14:0:test.js
   location: baz@14
   data: null
 Paused in 2:
@@ -197,7 +197,7 @@ Skipping pauses in 1
 Evaluating common breakpoint in 1
 Paused in 2:
   reason: other
-  hit breakpoints: test.js:11:0
+  hit breakpoints: 1:11:0:test.js
   location: foo@11
   data: null
 Resuming in 2
@@ -210,7 +210,7 @@ Deactivating breakpoints in 1
 Evaluating common breakpoint in 1
 Paused in 2:
   reason: other
-  hit breakpoints: test.js:11:0
+  hit breakpoints: 1:11:0:test.js
   location: foo@11
   data: null
 Resuming in 2

tools/presubmit.py

@@ -366,7 +366,6 @@ class SourceProcessor(SourceFileProcessor):
                        'regexp-pcre.js',
                        'resources-123.js',
                        'rjsmin.py',
-                       'script-breakpoint.h',
                        'sqlite.js',
                        'sqlite-change-heap.js',
                        'sqlite-pointer-masking.js',