Skip to content
Permalink
Browse files
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
  • Loading branch information
dcrousso committed Aug 31, 2022
1 parent 1978c27 commit 91f7b27a2c241b8753006ec66762aacbd2c66089
Show file tree
Hide file tree
Showing 27 changed files with 1,346 additions and 428 deletions.

Large diffs are not rendered by default.

@@ -114,7 +114,7 @@ TestPage.registerInitializer(() => {
suite.addTestCase({
name, test(resolve, reject) {
let script = window.findScript(scriptRegex);
window.loadLinesFromSourceCode(script).then(() => {
window.loadLinesFromSourceCode(script).then((lines) => {
// Set one breakpoint per line.
for (let line = script.range.startLine; line <= script.range.endLine; ++line) {
DebuggerAgent.setBreakpoint(createLocation(script, line, 0), (error, breakpointId, payload) => {
@@ -128,10 +128,19 @@ TestPage.registerInitializer(() => {
InspectorTest.log(`INSERTING AT: ${inputLocation.lineNumber}:${inputLocation.columnNumber}`);
InspectorTest.log(`PAUSES AT: ${payload.lineNumber}:${payload.columnNumber}`);
window.logResolvedBreakpointLinesWithContext(inputLocation, resolvedLocation, 3);
InspectorTest.log("");
}
});

let start = createLocation(script, line, 0);
let end = createLocation(script, line, lines[line].length);
DebuggerAgent.getBreakpointLocations(start, end, (error, locations) => {
InspectorTest.log(`LOCATIONS FROM ${start.lineNumber}:${start.columnNumber} to ${end.lineNumber}:${end.columnNumber}`);
if (error)
InspectorTest.log(`PRODUCES: ${error}`);
else
window.logResolvedBreakpointLocationsInRange(start, end, locations.map((location) => script.createSourceCodeLocation(location.lineNumber, location.columnNumber)));
});

// Clear the breakpoint we just set without knowing its breakpoint identifier.
DebuggerAgent.disable();
DebuggerAgent.enable();
@@ -106,6 +106,41 @@ TestPage.registerInitializer(() => {
}
}

window.logResolvedBreakpointLocationsInRange = function(start, end, resolvedLocations) {
InspectorTest.assert(!resolvedLocations.length || start.lineNumber < resolvedLocations.firstValue.lineNumber || start.columnNumber <= resolvedLocations.firstValue.columnNumber, "Start position should always come before first resolved location position.");
InspectorTest.assert(!resolvedLocations.length || end.lineNumber > resolvedLocations.lastValue.lineNumber || end.columnNumber > resolvedLocations.lastValue.columnNumber, "End position should always come after last resolved position.");

const inputCaret = "#";
const resolvedCaret = "|";

for (let lineNumber = start.lineNumber; lineNumber <= end.lineNumber; ++lineNumber) {
let lineContent = lines[lineNumber];
if (typeof lineContent !== "string")
continue;

if (lineNumber === start.lineNumber)
lineContent = " ".repeat(start.columnNumber) + lineContent.slice(start.columnNumber);

if (lineNumber === end.lineNumber)
lineContent = lineContent.slice(0, end.columnNumber)

for (let i = resolvedLocations.length - 1; i >= 0; --i) {
let resolvedLocation = resolvedLocations[i];

if (resolvedLocation.lineNumber !== lineNumber)
continue;

lineContent = insertCaretIntoStringAtIndex(lineContent, resolvedLocation.columnNumber, resolvedCaret);
}

if (lineContent[0] !== resolvedCaret)
lineContent = insertCaretIntoStringAtIndex(lineContent, 0, inputCaret);

let number = lineNumber.toString().padStart(3);
InspectorTest.log(` ${number} ${lineContent}`);
}
}

window.logLinesWithContext = function(location, context) {
if (location.sourceCode !== linesSourceCode && !WI.networkManager.mainFrame.mainResource.scripts.includes(location.sourceCode)) {
InspectorTest.log("--- Source Unavailable ---");
@@ -112,6 +112,10 @@ Repeating items:
["a","b","b","b","c"], ["c","b","b","a"] => [["a",-1],["c",1],["b",0],["b",0],["b",-1],["c",-1],["a",1]]
["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]]

-- Running test case: Array.prototype.firstValue
PASS: firstValue of a nonempty array should be the first value.
PASS: firstValue of an empty array should be undefined.

-- Running test case: Array.prototype.lastValue
PASS: lastValue of a nonempty array should be the last value.
PASS: lastValue of an empty array should be undefined.
@@ -226,6 +226,18 @@
}
});

suite.addTestCase({
name: "Array.prototype.firstValue",
test() {
let object1 = {};
let object2 = {};
InspectorTest.expectEqual([object1, object2].firstValue, object1, "firstValue of a nonempty array should be the first value.")
InspectorTest.expectEqual([].firstValue, undefined, "firstValue of an empty array should be undefined.")

return true;
}
});

suite.addTestCase({
name: "Array.prototype.lastValue",
test() {
@@ -5,6 +5,13 @@ PASS: Set can find a item it holds.
PASS: Set finds the first item if the given predicate matches multiple items.
PASS: Set returns 'undefined' when attempting to find an item if the given predicate doesn't match anything.

-- Running test case: Set.prototype.filter
PASS: Set can filter based on the value.
PASS: Set can filter based on the key.
PASS: Set can filter based on the set object.
PASS: Set can filter with a different this.
PASS: Set can filter based on the set object with a different this.

-- Running test case: Set.prototype.pushAll
Array:
[1,2] => [1,2,"a1","a2"]
@@ -17,6 +17,24 @@
},
});

suite.addTestCase({
name: "Set.prototype.filter",
test() {
function test(set, callback, thisArg, expected, message) {
InspectorTest.expectShallowEqual(Array.from(set.filter(callback, thisArg)), expected, message);
}

const set1 = new Set([1, 2, 3, 4]);
const set2 = new Set([2, 4]);

test(set1, function(value) { return value % 2; }, null, [1, 3], "Set can filter based on the value.");
test(set1, function(value, key) { return key % 2; }, null, [1, 3], "Set can filter based on the key.");
test(set1, function(value, key, set) { return set === set1; }, null, [1, 2, 3, 4], "Set can filter based on the set object.");
test(set1, function(value, key, set) { return this.has(key); }, set2, [2, 4], "Set can filter with a different this.");
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.");
},
});

suite.addTestCase({
name: "Set.prototype.pushAll",
test() {
@@ -465,6 +465,46 @@ DebuggerParseData& Debugger::debuggerParseData(SourceID sourceID, SourceProvider
return result.iterator->value;
}

void Debugger::forEachBreakpointLocation(SourceID sourceID, SourceProvider* sourceProvider, int startLine, int startColumn, int endLine, int endColumn, Function<void(int, int)>&& callback)
{
auto providerStartLine = sourceProvider->startPosition().m_line.oneBasedInt(); // One based to match the already adjusted line.
auto providerStartColumn = sourceProvider->startPosition().m_column.zeroBasedInt(); // Zero based so column zero is zero.

// FIXME: <https://webkit.org/b/162771> Web Inspector: Adopt TextPosition in Inspector to avoid oneBasedInt/zeroBasedInt ambiguity
// Inspector breakpoint line and column values are zero-based but the executable
// and CodeBlock line values are one-based while column is zero-based.
auto adjustedStartLine = startLine + 1;
auto adjustedStartColumn = startColumn;
auto adjustedEndLine = endLine + 1;
auto adjustedEndColumn = endColumn;

// Account for a <script>'s start position on the first line only.
if (startLine == providerStartLine && startColumn) {
ASSERT(providerStartColumn <= startColumn);
if (providerStartColumn)
adjustedStartColumn -= providerStartColumn;
}
if (endLine == providerStartLine && endColumn) {
ASSERT(providerStartColumn <= endColumn);
if (providerStartColumn)
adjustedEndColumn -= providerStartColumn;
}

auto& parseData = debuggerParseData(sourceID, sourceProvider);
parseData.pausePositions.forEachBreakpointLocation(adjustedStartLine, adjustedStartColumn, adjustedEndLine, adjustedEndColumn, [&, callback = WTFMove(callback)] (const JSTextPosition& resolvedPosition) {
auto resolvedLine = resolvedPosition.line;
auto resolvedColumn = resolvedPosition.column();

// Re-account for a <script>'s start position on the first line only.
if (resolvedLine == providerStartLine && (startColumn || (endLine == providerStartLine && endColumn))) {
if (providerStartColumn)
resolvedColumn += providerStartColumn;
}

callback(resolvedLine - 1, resolvedColumn);
});
}

bool Debugger::resolveBreakpoint(Breakpoint& breakpoint, SourceProvider* sourceProvider)
{
RELEASE_ASSERT(!breakpoint.isResolved());
@@ -60,6 +60,8 @@ class Debugger : public DoublyLinkedListNode<Debugger> {
JS_EXPORT_PRIVATE void detach(JSGlobalObject*, ReasonForDetach);
JS_EXPORT_PRIVATE bool isAttached(JSGlobalObject*);

void forEachBreakpointLocation(SourceID, SourceProvider*, int startLine, int startColumn, int endLine, int endColumn, Function<void(int, int)>&&);

bool resolveBreakpoint(Breakpoint&, SourceProvider*);
bool setBreakpoint(Breakpoint&);
bool removeBreakpoint(Breakpoint&);
@@ -30,14 +30,54 @@

namespace JSC {

std::optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column)
void DebuggerPausePositions::forEachBreakpointLocation(int startLine, int startColumn, int endLine, int endColumn, Function<void(const JSTextPosition&)>&& callback)
{
auto isAfterEnd = [&] (int line, int column) {
return (line == endLine && column >= endColumn) || line > endLine;
};

Vector<JSTextPosition> uniquePositions;
for (auto it = firstPositionAfter(startLine, startColumn); it != m_positions.end(); ++it) {
auto line = it->position.line;
auto column = it->position.column();

if (isAfterEnd(line, column))
break;

if (auto resolvedPosition = breakpointLocationForLineColumn(line, column, it)) {
if (!isAfterEnd(resolvedPosition->line, resolvedPosition->column()))
uniquePositions.appendIfNotContains(*resolvedPosition);
}
}
std::sort(uniquePositions.begin(), uniquePositions.end(), [] (const auto& a, const auto& b) {
if (a.line == b.line)
return a.column() < b.column();
return a.line < b.line;
});
for (const auto& position : uniquePositions)
callback(position);
}

DebuggerPausePositions::Positions::iterator DebuggerPausePositions::firstPositionAfter(int line, int column)
{
DebuggerPausePosition position = { DebuggerPausePositionType::Invalid, JSTextPosition(line, column, 0) };
auto it = std::lower_bound(m_positions.begin(), m_positions.end(), position, [] (const DebuggerPausePosition& a, const DebuggerPausePosition& b) {
return std::lower_bound(m_positions.begin(), m_positions.end(), position, [] (const DebuggerPausePosition& a, const DebuggerPausePosition& b) {
if (a.position.line == b.position.line)
return a.position.column() < b.position.column();
return a.position.line < b.position.line;
});
}

std::optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column)
{
return breakpointLocationForLineColumn(line, column, firstPositionAfter(line, column));
}

std::optional<JSTextPosition> DebuggerPausePositions::breakpointLocationForLineColumn(int line, int column, DebuggerPausePositions::Positions::iterator it)
{
ASSERT(line <= it->position.line);
ASSERT(line != it->position.line || column <= it->position.column());

if (it == m_positions.end())
return std::nullopt;

@@ -26,6 +26,7 @@
#pragma once

#include "ParserTokens.h"
#include <wtf/Function.h>
#include <wtf/Vector.h>

namespace JSC {
@@ -61,12 +62,19 @@ class DebuggerPausePositions {
m_positions.append({ DebuggerPausePositionType::Leave, position });
}

void forEachBreakpointLocation(int startLine, int startColumn, int endLine, int endColumn, Function<void(const JSTextPosition&)>&&);

std::optional<JSTextPosition> breakpointLocationForLineColumn(int line, int column);

void sort();

private:
Vector<DebuggerPausePosition> m_positions;
using Positions = Vector<DebuggerPausePosition>;

Positions::iterator firstPositionAfter(int line, int column);
std::optional<JSTextPosition> breakpointLocationForLineColumn(int line, int column, Positions::iterator);

Positions m_positions;
};


@@ -987,6 +987,47 @@ Protocol::ErrorStringOr<Ref<Protocol::Debugger::FunctionDetails>> InspectorDebug
return details.releaseNonNull();
}

Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Protocol::Debugger::Location>>> InspectorDebuggerAgent::getBreakpointLocations(Ref<JSON::Object>&& start, Ref<JSON::Object>&& end)
{
Protocol::ErrorString errorString;

JSC::SourceID startSourceID;
unsigned startLineNumber;
unsigned startColumnNumber;
if (!parseLocation(errorString, WTFMove(start), startSourceID, startLineNumber, startColumnNumber))
return makeUnexpected(errorString);

JSC::SourceID endSourceID;
unsigned endLineNumber;
unsigned endColumnNumber;
if (!parseLocation(errorString, WTFMove(end), endSourceID, endLineNumber, endColumnNumber))
return makeUnexpected(errorString);

if (startSourceID != endSourceID)
return makeUnexpected("Must have same scriptId for given start and given end"_s);

if (endLineNumber < startLineNumber)
return makeUnexpected("Cannot have lineNumber of given end be before lineNumber of given start"_s);

if (startLineNumber == endLineNumber && endColumnNumber < startColumnNumber)
return makeUnexpected("Cannot have columnNumber of given end be before columnNumber of given start"_s);

auto scriptIterator = m_scripts.find(startSourceID);
if (scriptIterator == m_scripts.end())
return makeUnexpected("Missing script for scriptId in given start"_s);

auto protocolLocations = JSON::ArrayOf<Protocol::Debugger::Location>::create();
m_debugger.forEachBreakpointLocation(startSourceID, scriptIterator->value.sourceProvider.get(), startLineNumber, startColumnNumber, endLineNumber, endColumnNumber, [&] (int lineNumber, int columnNumber) {
auto protocolLocation = Protocol::Debugger::Location::create()
.setScriptId(String::number(startSourceID))
.setLineNumber(lineNumber)
.release();
protocolLocation->setColumnNumber(columnNumber);
protocolLocations->addItem(WTFMove(protocolLocation));
});
return protocolLocations;
}

void InspectorDebuggerAgent::schedulePauseAtNextOpportunity(DebuggerFrontendDispatcher::Reason reason, RefPtr<JSON::Object>&& data)
{
if (m_javaScriptPauseScheduled)
@@ -90,6 +90,7 @@ class JS_EXPORT_PRIVATE InspectorDebuggerAgent
Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Protocol::GenericTypes::SearchMatch>>> searchInContent(const Protocol::Debugger::ScriptId&, const String& query, std::optional<bool>&& caseSensitive, std::optional<bool>&& isRegex) final;
Protocol::ErrorStringOr<String> getScriptSource(const Protocol::Debugger::ScriptId&) final;
Protocol::ErrorStringOr<Ref<Protocol::Debugger::FunctionDetails>> getFunctionDetails(const String& functionId) final;
Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Protocol::Debugger::Location>>> getBreakpointLocations(Ref<JSON::Object>&& start, Ref<JSON::Object>&& end) final;
Protocol::ErrorStringOr<void> setPauseOnDebuggerStatements(bool enabled, RefPtr<JSON::Object>&& options) final;
Protocol::ErrorStringOr<void> setPauseOnExceptions(const String& state, RefPtr<JSON::Object>&& options) final;
Protocol::ErrorStringOr<void> setPauseOnAssertions(bool enabled, RefPtr<JSON::Object>&& options) final;
@@ -273,6 +273,17 @@
{ "name": "details", "$ref": "FunctionDetails", "description": "Information about the function." }
]
},
{
"name": "getBreakpointLocations",
"description": "Returns a list of valid breakpoint locations within the given location range.",
"parameters": [
{ "name": "start", "$ref": "Location", "description": "Starting location to look for breakpoint locations after (inclusive). Must have same scriptId as end." },
{ "name": "end", "$ref": "Location", "description": "Ending location to look for breakpoint locations before (exclusive). Must have same scriptId as start." }
],
"returns": [
{ "name": "locations", "type": "array", "items": { "$ref": "Location" }, "description": "List of resolved breakpoint locations." }
]
},
{
"name": "setPauseOnDebuggerStatements",
"description": "Control whether the debugger pauses execution before `debugger` statements.",
@@ -100,10 +100,12 @@
"isWebInspectorInternalScript": true,
"isWebKitInternalScript": true,
"multiplicationSign": true,
"nullish": true,
"parseMIMEType": true,
"resolveDotsInPath": true,
"simpleGlobStringToRegExp": true,
"timestamp": true,
"xor": true,
"zeroWidthSpace": true,

// DOM Utilities

0 comments on commit 91f7b27

Please sign in to comment.