Commit 81b91591 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[inspector] Properly filter by column number for inline scripts.

Previously `setBreakpointByUrl` and friends would only filter based on
line number to find matching scripts. But that didn't work when there
were multiple scripts in the same line (i.e. minified HTML), and we'd
end up setting multiple breakpoints in different inline scripts, looking
for the next possible break location in each of them individually.

Fixed: chromium:1183664
Also-By: pfaffe@chromium.org, kimanh@chromium.org
Change-Id: I957811d30aa71609a38da75f33a24c0f720116f6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2749155
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarKim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73332}
parent 1691b1f6
......@@ -194,6 +194,14 @@ void adjustBreakpointLocation(const V8DebuggerScript& script,
int* columnNumber) {
if (*lineNumber < script.startLine() || *lineNumber > script.endLine())
return;
if (*lineNumber == script.startLine() &&
*columnNumber < script.startColumn()) {
return;
}
if (*lineNumber == script.endLine() && script.endColumn() < *columnNumber) {
return;
}
if (hint.isEmpty()) return;
intptr_t sourceOffset = script.offset(*lineNumber, *columnNumber);
if (sourceOffset == V8DebuggerScript::kNoOffset) return;
......@@ -915,6 +923,13 @@ V8DebuggerAgentImpl::setBreakpointImpl(const String16& breakpointId,
if (lineNumber < script->startLine() || script->endLine() < lineNumber) {
return nullptr;
}
if (lineNumber == script->startLine() &&
columnNumber < script->startColumn()) {
return nullptr;
}
if (lineNumber == script->endLine() && script->endColumn() < columnNumber) {
return nullptr;
}
v8::debug::BreakpointId debuggerBreakpointId;
v8::debug::Location location(lineNumber, columnNumber);
......
Regression test for crbug.com/1183664
Running test: testMultipleScriptsInSameLineWithSameURL
Setting breakpoint in first script
[
[0] : {
columnNumber : 1
lineNumber : 0
scriptId : <scriptId>
}
]
Setting breakpoint in second script
[
[0] : {
columnNumber : 65
lineNumber : 0
scriptId : <scriptId>
}
]
// Copyright 2021 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('Regression test for crbug.com/1183664');
const url = 'test://foo.js';
const lineNumber = 0;
const columnNumber1 = 1;
contextGroup.addScript(`console.log("FIRST")`, lineNumber, columnNumber1, url);
const columnNumber2 = 65;
contextGroup.addScript(`console.log("SECOND")`, lineNumber, columnNumber2, url);
InspectorTest.runAsyncTestSuite([
async function testMultipleScriptsInSameLineWithSameURL() {
await Protocol.Debugger.enable();
InspectorTest.logMessage('Setting breakpoint in first script')
{
const {result: {breakpointId, locations}} = await Protocol.Debugger.setBreakpointByUrl({
url,
lineNumber,
columnNumber: columnNumber1,
});
InspectorTest.logMessage(locations);
await Protocol.Debugger.removeBreakpoint({breakpointId});
}
InspectorTest.logMessage('Setting breakpoint in second script')
{
const {result: {breakpointId, locations}} = await Protocol.Debugger.setBreakpointByUrl({
url,
lineNumber,
columnNumber: columnNumber2,
});
InspectorTest.logMessage(locations);
await Protocol.Debugger.removeBreakpoint({breakpointId});
}
}
]);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment