Skip to content

Commit 91f7b27

Browse files
dcroussodcrousso
committed
Web Inspector: Implement inline breakpoints.
https://bugs.webkit.org/show_bug.cgi?id=197977 <rdar://problem/50903783> Reviewed by Patrick Angle. This will allow developers more flexibility with JS breakpoints since JS can frequently pack lots of logic onto a single line. For example, ``` |setTimeout(() => { |a(); |b(); |}) ``` has pause locations at each `|`, so developers should be able to set a breakpoint there. Previously, only the first `|` was a valid pause location (unless the file is pretty printed such that the nested inline function is reformatted onto multiple lines) because adding a breakpoint via the text editor gutter will only attempt to add a breakpoint on the first column of that line. * Source/JavaScriptCore/inspector/protocol/Debugger.json: * Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h: * Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp: (Inspector::InspectorDebuggerAgent::getBreakpointLocations): Added. * Source/JavaScriptCore/debugger/Debugger.h: * Source/JavaScriptCore/debugger/Debugger.cpp: (JSC::Debugger::forEachBreakpointLocation): Added. * Source/JavaScriptCore/debugger/DebuggerParseData.h: * Source/JavaScriptCore/debugger/DebuggerParseData.cpp: (JSC::DebuggerPausePositions::forEachBreakpointLocation): Added. (JSC::DebuggerPausePositions::firstPositionAfter): Added. (JSC::DebuggerPausePositions::breakpointLocationForLineColumn): * Source/WebInspectorUI/UserInterface/Models/Script.js: (WI.Script.prototype.async breakpointLocations): Added. Add a way for the frontend to get all breakpoint locations between two positions. * Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js: (WI.SourceCodeTextEditor): (WI.SourceCodeTextEditor.prototype._editorPositionForSourceCodeLocation): Added. (WI.SourceCodeTextEditor.prototype._editorLineInfoForSourceCodeLocation): (WI.SourceCodeTextEditor.prototype._editorLineInfoForEditorPosition): Added. (WI.SourceCodeTextEditor.prototype._addBreakpointWithEditorLineInfo): (WI.SourceCodeTextEditor.prototype._removeBreakpointWithEditorLineInfo): (WI.SourceCodeTextEditor.prototype._prepareEditorForInitialContent): (WI.SourceCodeTextEditor.prototype.async _addBreakpointWidgetsForLine): Added. (WI.SourceCodeTextEditor.prototype._removeBreakpointWidgetsForLine): Added. (WI.SourceCodeTextEditor.prototype._removeBreakpointWidgets): Added. (WI.SourceCodeTextEditor.prototype.textEditorBreakpointRemoved): (WI.SourceCodeTextEditor.prototype.textEditorBreakpointClicked): (WI.SourceCodeTextEditor.prototype._handleFormatterDidChange): * Source/WebInspectorUI/UserInterface/Views/BreakpointInlineWidget.js: Added. (WI.BreakpointInlineWidget): (WI.BreakpointInlineWidget.prototype.get element): (WI.BreakpointInlineWidget.prototype.get sourceCodeLocation): (WI.BreakpointInlineWidget.prototype.get breakpoint): (WI.BreakpointInlineWidget.prototype._update): (WI.BreakpointInlineWidget.prototype._handleClick): * Source/WebInspectorUI/UserInterface/Views/BreakpointInlineWidget.css: Added. (.inline-widget.breakpoint): (.inline-widget.breakpoint:not(.disabled)): (.inline-widget.breakpoint.disabled): Add inline widgets for each breakpoint location on a given line after the first breakpoint is added for that line. Clicking these widgets will add/remove a breakpoint at that location, until there are no breakpoints left at which point all of the widgets are removed from the line. Reformatting will recalculate the definition of "line". * Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js: (WI.DebuggerManager.prototype.breakpointsForSourceCodeLocation): Added. (WI.DebuggerManager.prototype.breakpointResolved): (WI.DebuggerManager.prototype.globalObjectCleared): (WI.DebuggerManager.prototype._setBreakpoint): * Source/WebInspectorUI/UserInterface/Models/JavaScriptBreakpoint.js: (WI.JavaScriptBreakpoint.prototype.get resolvedLocations): Added. (WI.JavaScriptBreakpoint.prototype.addResolvedLocation): Added. (WI.JavaScriptBreakpoint.prototype.hasResolvedLocation): Added. (WI.JavaScriptBreakpoint.prototype.clearResolvedLocations): Added. Keep track of all resolved locations each time a JS breakpoint is set. This allows for inline widgets to see if there are any existing breakponts that match the related location, using that instead of creating a new breakpoint. * Source/WebInspectorUI/UserInterface/Models/SourceCodeLocation.js: (WI.SourceCodeLocation.prototype.isEqual): (WI.SourceCodeLocation.prototype.isEqual.resolveSourceCode): Added. * Source/WebInspectorUI/UserInterface/Models/LazySourceCodeLocation.js: (WI.LazySourceCodeLocation.prototype.isEqual): Deleted. If a `WI.Script` is mapped to a `WI.Resource` it should be considered as equal. This allows for the initial breakpoint set on a line to map to the inline widget that is added for the resolved location, since the initial breakpoint can be set with a `WI.Resource` while all breakpoint locations will use the first related `WI.Script`. * Source/WebInspectorUI/UserInterface/Views/TextEditor.js: (WI.TextEditor.prototype._gutterMouseDown): (WI.TextEditor.prototype._documentMouseMoved): (WI.TextEditor.prototype._documentMouseUp): Allow for clicking/dragging the gutter icon when there are multiple breakpoints on the same line. Unlike when there's only a single breakpoint, however, dragging to another line will instead remove all breakpoints for that line. * Source/WebInspectorUI/UserInterface/Base/Utilities.js: (nullish): Added. (Array.prototype.firstValue): Added. (Set.prototype.filter): Added. * Source/WebInspectorUI/.eslintrc: * LayoutTests/inspector/unit-tests/array-utilities.html: * LayoutTests/inspector/unit-tests/array-utilities-expected.txt: * LayoutTests/inspector/unit-tests/set-utilities.html: * LayoutTests/inspector/unit-tests/set-utilities-expected.txt: Add utility functions for `Array.prototype.firstValue` and `Set.prototype.filter` to make it easier to use `Array` and `Set` interchangeably. * Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js: (WI.SourceCodePosition): Drive-by: Fix typo. * Source/WebInspectorUI/UserInterface/Main.html: * LayoutTests/inspector/debugger/breakpoints/resources/dump.js: (TestPage.registerInitializer.window.addDumpEachLinePauseLocationTestCase): Added. * LayoutTests/inspector/debugger/resources/log-pause-location.js: (TestPage.registerInitializer.window.logResolvedBreakpointLocationsInRange): Added. * LayoutTests/inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt: Canonical link: https://commits.webkit.org/254015@main
1 parent 1978c27 commit 91f7b27

27 files changed

+1346
-428
lines changed

‎LayoutTests/inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt‎

Lines changed: 750 additions & 366 deletions
Large diffs are not rendered by default.

‎LayoutTests/inspector/debugger/breakpoints/resources/dump.js‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ TestPage.registerInitializer(() => {
114114
suite.addTestCase({
115115
name, test(resolve, reject) {
116116
let script = window.findScript(scriptRegex);
117-
window.loadLinesFromSourceCode(script).then(() => {
117+
window.loadLinesFromSourceCode(script).then((lines) => {
118118
// Set one breakpoint per line.
119119
for (let line = script.range.startLine; line <= script.range.endLine; ++line) {
120120
DebuggerAgent.setBreakpoint(createLocation(script, line, 0), (error, breakpointId, payload) => {
@@ -128,10 +128,19 @@ TestPage.registerInitializer(() => {
128128
InspectorTest.log(`INSERTING AT: ${inputLocation.lineNumber}:${inputLocation.columnNumber}`);
129129
InspectorTest.log(`PAUSES AT: ${payload.lineNumber}:${payload.columnNumber}`);
130130
window.logResolvedBreakpointLinesWithContext(inputLocation, resolvedLocation, 3);
131-
InspectorTest.log("");
132131
}
133132
});
134133

134+
let start = createLocation(script, line, 0);
135+
let end = createLocation(script, line, lines[line].length);
136+
DebuggerAgent.getBreakpointLocations(start, end, (error, locations) => {
137+
InspectorTest.log(`LOCATIONS FROM ${start.lineNumber}:${start.columnNumber} to ${end.lineNumber}:${end.columnNumber}`);
138+
if (error)
139+
InspectorTest.log(`PRODUCES: ${error}`);
140+
else
141+
window.logResolvedBreakpointLocationsInRange(start, end, locations.map((location) => script.createSourceCodeLocation(location.lineNumber, location.columnNumber)));
142+
});
143+
135144
// Clear the breakpoint we just set without knowing its breakpoint identifier.
136145
DebuggerAgent.disable();
137146
DebuggerAgent.enable();

‎LayoutTests/inspector/debugger/resources/log-pause-location.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,41 @@ TestPage.registerInitializer(() => {
106106
}
107107
}
108108

109+
window.logResolvedBreakpointLocationsInRange = function(start, end, resolvedLocations) {
110+
InspectorTest.assert(!resolvedLocations.length || start.lineNumber < resolvedLocations.firstValue.lineNumber || start.columnNumber <= resolvedLocations.firstValue.columnNumber, "Start position should always come before first resolved location position.");
111+
InspectorTest.assert(!resolvedLocations.length || end.lineNumber > resolvedLocations.lastValue.lineNumber || end.columnNumber > resolvedLocations.lastValue.columnNumber, "End position should always come after last resolved position.");
112+
113+
const inputCaret = "#";
114+
const resolvedCaret = "|";
115+
116+
for (let lineNumber = start.lineNumber; lineNumber <= end.lineNumber; ++lineNumber) {
117+
let lineContent = lines[lineNumber];
118+
if (typeof lineContent !== "string")
119+
continue;
120+
121+
if (lineNumber === start.lineNumber)
122+
lineContent = " ".repeat(start.columnNumber) + lineContent.slice(start.columnNumber);
123+
124+
if (lineNumber === end.lineNumber)
125+
lineContent = lineContent.slice(0, end.columnNumber)
126+
127+
for (let i = resolvedLocations.length - 1; i >= 0; --i) {
128+
let resolvedLocation = resolvedLocations[i];
129+
130+
if (resolvedLocation.lineNumber !== lineNumber)
131+
continue;
132+
133+
lineContent = insertCaretIntoStringAtIndex(lineContent, resolvedLocation.columnNumber, resolvedCaret);
134+
}
135+
136+
if (lineContent[0] !== resolvedCaret)
137+
lineContent = insertCaretIntoStringAtIndex(lineContent, 0, inputCaret);
138+
139+
let number = lineNumber.toString().padStart(3);
140+
InspectorTest.log(` ${number} ${lineContent}`);
141+
}
142+
}
143+
109144
window.logLinesWithContext = function(location, context) {
110145
if (location.sourceCode !== linesSourceCode && !WI.networkManager.mainFrame.mainResource.scripts.includes(location.sourceCode)) {
111146
InspectorTest.log("--- Source Unavailable ---");

‎LayoutTests/inspector/unit-tests/array-utilities-expected.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ Repeating items:
112112
["a","b","b","b","c"], ["c","b","b","a"] => [["a",-1],["c",1],["b",0],["b",0],["b",-1],["c",-1],["a",1]]
113113
["a","a","b","b","c","c"], ["b","b","c","c","a","a"] => [["a",-1],["a",-1],["b",0],["b",0],["c",0],["c",0],["a",1],["a",1]]
114114

115+
-- Running test case: Array.prototype.firstValue
116+
PASS: firstValue of a nonempty array should be the first value.
117+
PASS: firstValue of an empty array should be undefined.
118+
115119
-- Running test case: Array.prototype.lastValue
116120
PASS: lastValue of a nonempty array should be the last value.
117121
PASS: lastValue of an empty array should be undefined.

‎LayoutTests/inspector/unit-tests/array-utilities.html‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,18 @@
226226
}
227227
});
228228

229+
suite.addTestCase({
230+
name: "Array.prototype.firstValue",
231+
test() {
232+
let object1 = {};
233+
let object2 = {};
234+
InspectorTest.expectEqual([object1, object2].firstValue, object1, "firstValue of a nonempty array should be the first value.")
235+
InspectorTest.expectEqual([].firstValue, undefined, "firstValue of an empty array should be undefined.")
236+
237+
return true;
238+
}
239+
});
240+
229241
suite.addTestCase({
230242
name: "Array.prototype.lastValue",
231243
test() {

‎LayoutTests/inspector/unit-tests/set-utilities-expected.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ PASS: Set can find a item it holds.
55
PASS: Set finds the first item if the given predicate matches multiple items.
66
PASS: Set returns 'undefined' when attempting to find an item if the given predicate doesn't match anything.
77

8+
-- Running test case: Set.prototype.filter
9+
PASS: Set can filter based on the value.
10+
PASS: Set can filter based on the key.
11+
PASS: Set can filter based on the set object.
12+
PASS: Set can filter with a different this.
13+
PASS: Set can filter based on the set object with a different this.
14+
815
-- Running test case: Set.prototype.pushAll
916
Array:
1017
[1,2] => [1,2,"a1","a2"]

‎LayoutTests/inspector/unit-tests/set-utilities.html‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,24 @@
1717
},
1818
});
1919

20+
suite.addTestCase({
21+
name: "Set.prototype.filter",
22+
test() {
23+
function test(set, callback, thisArg, expected, message) {
24+
InspectorTest.expectShallowEqual(Array.from(set.filter(callback, thisArg)), expected, message);
25+
}
26+
27+
const set1 = new Set([1, 2, 3, 4]);
28+
const set2 = new Set([2, 4]);
29+
30+
test(set1, function(value) { return value % 2; }, null, [1, 3], "Set can filter based on the value.");
31+
test(set1, function(value, key) { return key % 2; }, null, [1, 3], "Set can filter based on the key.");
32+
test(set1, function(value, key, set) { return set === set1; }, null, [1, 2, 3, 4], "Set can filter based on the set object.");
33+
test(set1, function(value, key, set) { return this.has(key); }, set2, [2, 4], "Set can filter with a different this.");
34+
test(set1, function(value, key, set) { return this !== set; }, set2, [1, 2, 3, 4], "Set can filter based on the set object with a different this.");
35+
},
36+
});
37+
2038
suite.addTestCase({
2139
name: "Set.prototype.pushAll",
2240
test() {

‎Source/JavaScriptCore/debugger/Debugger.cpp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,46 @@ DebuggerParseData& Debugger::debuggerParseData(SourceID sourceID, SourceProvider
465465
return result.iterator->value;
466466
}
467467

468+
void Debugger::forEachBreakpointLocation(SourceID sourceID, SourceProvider* sourceProvider, int startLine, int startColumn, int endLine, int endColumn, Function<void(int, int)>&& callback)
469+
{
470+
auto providerStartLine = sourceProvider->startPosition().m_line.oneBasedInt(); // One based to match the already adjusted line.
471+
auto providerStartColumn = sourceProvider->startPosition().m_column.zeroBasedInt(); // Zero based so column zero is zero.
472+
473+
// FIXME: <https://webkit.org/b/162771> Web Inspector: Adopt TextPosition in Inspector to avoid oneBasedInt/zeroBasedInt ambiguity
474+
// Inspector breakpoint line and column values are zero-based but the executable
475+
// and CodeBlock line values are one-based while column is zero-based.
476+
auto adjustedStartLine = startLine + 1;
477+
auto adjustedStartColumn = startColumn;
478+
auto adjustedEndLine = endLine + 1;
479+
auto adjustedEndColumn = endColumn;
480+
481+
// Account for a <script>'s start position on the first line only.
482+
if (startLine == providerStartLine && startColumn) {
483+
ASSERT(providerStartColumn <= startColumn);
484+
if (providerStartColumn)
485+
adjustedStartColumn -= providerStartColumn;
486+
}
487+
if (endLine == providerStartLine && endColumn) {
488+
ASSERT(providerStartColumn <= endColumn);
489+
if (providerStartColumn)
490+
adjustedEndColumn -= providerStartColumn;
491+
}
492+
493+
auto& parseData = debuggerParseData(sourceID, sourceProvider);
494+
parseData.pausePositions.forEachBreakpointLocation(adjustedStartLine, adjustedStartColumn, adjustedEndLine, adjustedEndColumn, [&, callback = WTFMove(callback)] (const JSTextPosition& resolvedPosition) {
495+
auto resolvedLine = resolvedPosition.line;
496+
auto resolvedColumn = resolvedPosition.column();
497+
498+
// Re-account for a <script>'s start position on the first line only.
499+
if (resolvedLine == providerStartLine && (startColumn || (endLine == providerStartLine && endColumn))) {
500+
if (providerStartColumn)
501+
resolvedColumn += providerStartColumn;
502+
}
503+
504+
callback(resolvedLine - 1, resolvedColumn);
505+
});
506+
}
507+
468508
bool Debugger::resolveBreakpoint(Breakpoint& breakpoint, SourceProvider* sourceProvider)
469509
{
470510
RELEASE_ASSERT(!breakpoint.isResolved());

‎Source/JavaScriptCore/debugger/Debugger.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ class Debugger : public DoublyLinkedListNode<Debugger> {
6060
JS_EXPORT_PRIVATE void detach(JSGlobalObject*, ReasonForDetach);
6161
JS_EXPORT_PRIVATE bool isAttached(JSGlobalObject*);
6262

63+
void forEachBreakpointLocation(SourceID, SourceProvider*, int startLine, int startColumn, int endLine, int endColumn, Function<void(int, int)>&&);
64+
6365
bool resolveBreakpoint(Breakpoint&, SourceProvider*);
6466
bool setBreakpoint(Breakpoint&);
6567
bool removeBreakpoint(Breakpoint&);

‎Source/JavaScriptCore/debugger/DebuggerParseData.cpp‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,54 @@
3030

3131
namespace JSC {
3232

33-
std::optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column)
33+
void DebuggerPausePositions::forEachBreakpointLocation(int startLine, int startColumn, int endLine, int endColumn, Function<void(const JSTextPosition&)>&& callback)
34+
{
35+
auto isAfterEnd = [&] (int line, int column) {
36+
return (line == endLine && column >= endColumn) || line > endLine;
37+
};
38+
39+
Vector<JSTextPosition> uniquePositions;
40+
for (auto it = firstPositionAfter(startLine, startColumn); it != m_positions.end(); ++it) {
41+
auto line = it->position.line;
42+
auto column = it->position.column();
43+
44+
if (isAfterEnd(line, column))
45+
break;
46+
47+
if (auto resolvedPosition = breakpointLocationForLineColumn(line, column, it)) {
48+
if (!isAfterEnd(resolvedPosition->line, resolvedPosition->column()))
49+
uniquePositions.appendIfNotContains(*resolvedPosition);
50+
}
51+
}
52+
std::sort(uniquePositions.begin(), uniquePositions.end(), [] (const auto& a, const auto& b) {
53+
if (a.line == b.line)
54+
return a.column() < b.column();
55+
return a.line < b.line;
56+
});
57+
for (const auto& position : uniquePositions)
58+
callback(position);
59+
}
60+
61+
DebuggerPausePositions::Positions::iterator DebuggerPausePositions::firstPositionAfter(int line, int column)
3462
{
3563
DebuggerPausePosition position = { DebuggerPausePositionType::Invalid, JSTextPosition(line, column, 0) };
36-
auto it = std::lower_bound(m_positions.begin(), m_positions.end(), position, [] (const DebuggerPausePosition& a, const DebuggerPausePosition& b) {
64+
return std::lower_bound(m_positions.begin(), m_positions.end(), position, [] (const DebuggerPausePosition& a, const DebuggerPausePosition& b) {
3765
if (a.position.line == b.position.line)
3866
return a.position.column() < b.position.column();
3967
return a.position.line < b.position.line;
4068
});
69+
}
70+
71+
std::optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column)
72+
{
73+
return breakpointLocationForLineColumn(line, column, firstPositionAfter(line, column));
74+
}
75+
76+
std::optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column, DebuggerPausePositions::Positions::iterator it)
77+
{
78+
ASSERT(line <= it->position.line);
79+
ASSERT(line != it->position.line || column <= it->position.column());
80+
4181
if (it == m_positions.end())
4282
return std::nullopt;
4383

0 commit comments

Comments
 (0)