diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 20cec2387..66c6bfff4 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,3 +1,6 @@ +## 13.1.1-dev +- Add column information to breakpoints to allow precise breakpoint placement. + ## 13.1.0 - Update _fe_analyzer_shared to version ^38.0.0. diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index f1b4a6fba..ddc9ed0df 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -228,8 +228,9 @@ class Debugger extends Domain { int line, { int column, }) async { + column ??= 0; checkIsolate('addBreakpoint', isolateId); - final breakpoint = await _breakpoints.add(scriptId, line); + final breakpoint = await _breakpoints.add(scriptId, line, column); _notifyBreakpoint(breakpoint); return breakpoint; } @@ -249,7 +250,9 @@ class Debugger extends Domain { for (var breakpoint in previousBreakpoints) { var scriptRef = await _updatedScriptRefFor(breakpoint); var updatedLocation = await _locations.locationForDart( - DartUri(scriptRef.uri, _root), _lineNumberFor(breakpoint)); + DartUri(scriptRef.uri, _root), + _lineNumberFor(breakpoint), + _columnNumberFor(breakpoint)); var updatedBreakpoint = _breakpoints._dartBreakpoint( scriptRef, updatedLocation, breakpoint.id); _breakpoints._note( @@ -263,7 +266,8 @@ class Debugger extends Domain { await addBreakpoint( inspector.isolate.id, (await _updatedScriptRefFor(breakpoint)).id, - _lineNumberFor(breakpoint)); + _lineNumberFor(breakpoint), + column: _columnNumberFor(breakpoint)); } } @@ -324,10 +328,11 @@ class Debugger extends Domain { var frame = e.params['callFrames'][0]; var location = frame['location']; var scriptId = location['scriptId'] as String; - var lineNumber = location['lineNumber'] as int; + var line = location['lineNumber'] as int; + var column = location['columnNumber'] as int; var url = _urlForScriptId(scriptId); - return _locations.locationForJs(url, lineNumber + 1); + return _locations.locationForJs(url, line, column); } /// The variables visible in a frame in Dart protocol [BoundVariable] form. @@ -459,9 +464,8 @@ class Debugger extends Domain { bool populateVariables = true, }) async { var location = frame.location; - // Chrome is 0 based. Account for this. - var line = location.lineNumber + 1; - var column = location.columnNumber + 1; + var line = location.lineNumber; + var column = location.columnNumber; var url = _urlForScriptId(location.scriptId); if (url == null) { @@ -723,14 +727,20 @@ bool isNativeJsObject(InstanceRef instanceRef) { /// Returns the Dart line number for the provided breakpoint. int _lineNumberFor(Breakpoint breakpoint) => - int.parse(breakpoint.id.split('#').last); + int.parse(breakpoint.id.split('#').last.split(':').first); + +/// Returns the Dart column number for the provided breakpoint. +int _columnNumberFor(Breakpoint breakpoint) => + int.parse(breakpoint.id.split('#').last.split(':').last); /// Returns the breakpoint ID for the provided Dart script ID and Dart line /// number. -String breakpointIdFor(String scriptId, int line) => 'bp/$scriptId#$line'; +String breakpointIdFor(String scriptId, int line, int column) => + 'bp/$scriptId#$line:$column'; /// Keeps track of the Dart and JS breakpoint Ids that correspond. class _Breakpoints extends Domain { + final logger = Logger('Breakpoints'); final _dartIdByJsId = {}; final _jsIdByDartId = {}; @@ -752,13 +762,16 @@ class _Breakpoints extends Domain { }) : super(provider); Future _createBreakpoint( - String id, String scriptId, int line) async { + String id, String scriptId, int line, int column) async { var dartScript = inspector.scriptWithId(scriptId); var dartUri = DartUri(dartScript.uri, root); - var location = await locations.locationForDart(dartUri, line); - + var location = await locations.locationForDart(dartUri, line, column); // TODO: Handle cases where a breakpoint can't be set exactly at that line. if (location == null) { + logger + .fine('Failed to set breakpoint at ${dartScript.uri}:$line:$column: ' + 'Dart location not found for scriptId: $scriptId, ' + 'server path: ${dartUri.serverPath}, root:$root'); throw RPCError( 'addBreakpoint', 102, @@ -779,10 +792,10 @@ class _Breakpoints extends Domain { /// Adds a breakpoint at [scriptId] and [line] or returns an existing one if /// present. - Future add(String scriptId, int line) async { - final id = breakpointIdFor(scriptId, line); + Future add(String scriptId, int line, int column) async { + final id = breakpointIdFor(scriptId, line, column); return _bpByDartId.putIfAbsent( - id, () => _createBreakpoint(id, scriptId, line)); + id, () => _createBreakpoint(id, scriptId, line, column)); } /// Create a Dart breakpoint at [location] in [dartScript] with [id]. @@ -792,7 +805,12 @@ class _Breakpoints extends Domain { id: id, breakpointNumber: int.parse(createId()), resolved: true, - location: SourceLocation(script: dartScript, tokenPos: location.tokenPos), + location: SourceLocation( + script: dartScript, + tokenPos: location.tokenPos, + line: location.dartLocation.line, + column: location.dartLocation.column, + ), enabled: true, )..id = id; return breakpoint; @@ -800,9 +818,6 @@ class _Breakpoints extends Domain { /// Calls the Chrome protocol setBreakpoint and returns the remote ID. Future _setJsBreakpoint(Location location) async { - // Location is 0 based according to: - // https://chromedevtools.github.io/devtools-protocol/tot/Debugger#type-Location - // The module can be loaded from a nested path and contain an ETAG suffix. var urlRegex = '.*${location.jsLocation.module}.*'; // Prevent `Aww, snap!` errors when setting multiple breakpoints @@ -811,7 +826,8 @@ class _Breakpoints extends Domain { var response = await remoteDebugger .sendCommand('Debugger.setBreakpointByUrl', params: { 'urlRegex': urlRegex, - 'lineNumber': location.jsLocation.line - 1, + 'lineNumber': location.jsLocation.line, + 'columnNumber': location.jsLocation.column, }); return response.result['breakpointId'] as String; }); diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index ed66fc18e..1dad90baa 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -40,6 +40,7 @@ class Location { var dartColumn = entry.sourceColumn; var jsLine = lineEntry.line; var jsColumn = entry.column; + // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( @@ -71,21 +72,18 @@ class DartLocation { @override String toString() => '[${uri.serverPath}:$line:$column]'; - static DartLocation fromZeroBased(DartUri uri, int line, int column) => + factory DartLocation.fromZeroBased(DartUri uri, int line, int column) => DartLocation._(uri, line + 1, column + 1); - - static DartLocation fromOneBased(DartUri uri, int line, int column) => - DartLocation._(uri, line, column); } /// Location information for a JS source. class JsLocation { final String module; - /// 1 based row offset within the JS source code. + /// 0 based row offset within the JS source code. final int line; - /// 1 based column offset within the JS source code. + /// 0 based column offset within the JS source code. final int column; JsLocation._( @@ -97,10 +95,9 @@ class JsLocation { @override String toString() => '[$module:$line:$column]'; - static JsLocation fromZeroBased(String module, int line, int column) => - JsLocation._(module, line + 1, column + 1); - - static JsLocation fromOneBased(String module, int line, int column) => + // JS Location is 0 based according to: + // https://chromedevtools.github.io/devtools-protocol/tot/Debugger#type-Location + factory JsLocation.fromZeroBased(String module, int line, int column) => JsLocation._(module, line, column); } @@ -155,17 +152,21 @@ class Locations { /// Find the [Location] for the given Dart source position. /// /// The [line] number is 1-based. - Future locationForDart(DartUri uri, int line) async => + Future locationForDart(DartUri uri, int line, int column) async => (await locationsForDart(uri.serverPath)).firstWhere( - (location) => location.dartLocation.line == line, + (location) => + location.dartLocation.line == line && + location.dartLocation.column >= column, orElse: () => null); /// Find the [Location] for the given JS source position. /// - /// The [line] number is 1-based. - Future locationForJs(String url, int line) async => + /// The [line] number is 0-based. + Future locationForJs(String url, int line, int column) async => (await locationsForUrl(url)).firstWhere( - (location) => location.jsLocation.line == line, + (location) => + location.jsLocation.line == line && + location.jsLocation.column >= column, orElse: () => null); /// Returns the tokenPosTable for the provided Dart script path as defined diff --git a/dwds/lib/src/debugging/skip_list.dart b/dwds/lib/src/debugging/skip_list.dart index 994c7bee6..07bc583cf 100644 --- a/dwds/lib/src/debugging/skip_list.dart +++ b/dwds/lib/src/debugging/skip_list.dart @@ -31,8 +31,8 @@ class SkipLists { var startColumn = 0; for (var location in locations) { // Account for 1 based. - var endLine = location.jsLocation.line - 1; - var endColumn = location.jsLocation.column - 1; + var endLine = location.jsLocation.line; + var endColumn = location.jsLocation.column; // Stop before the known location. endColumn -= 1; if (endColumn < 0) { @@ -42,8 +42,8 @@ class SkipLists { if (endLine > startLine || endColumn > startColumn) { ranges.add( _rangeFor(scriptId, startLine, startColumn, endLine, endColumn)); - startLine = location.jsLocation.line - 1; - startColumn = location.jsLocation.column; + startLine = location.jsLocation.line; + startColumn = location.jsLocation.column + 1; } } ranges.add(_rangeFor(scriptId, startLine, startColumn, maxValue, maxValue)); diff --git a/dwds/lib/src/services/expression_evaluator.dart b/dwds/lib/src/services/expression_evaluator.dart index 211e9f940..8c1494eef 100644 --- a/dwds/lib/src/services/expression_evaluator.dart +++ b/dwds/lib/src/services/expression_evaluator.dart @@ -159,8 +159,9 @@ class ExpressionEvaluator { } var functionName = jsFrame.functionName; - var jsLine = jsFrame.location.lineNumber + 1; + var jsLine = jsFrame.location.lineNumber; var jsScriptId = jsFrame.location.scriptId; + var jsColumn = jsFrame.location.columnNumber; var jsScope = await _collectLocalJsScope(jsFrame); // Find corresponding dart location and scope. @@ -171,7 +172,7 @@ class ExpressionEvaluator { // cases. Invent location matching strategy for those cases. // [issue 890](https://github.com/dart-lang/webdev/issues/890) var url = _urlForScriptId(jsScriptId); - var locationMap = await _locations.locationForJs(url, jsLine); + var locationMap = await _locations.locationForJs(url, jsLine, jsColumn); if (locationMap == null) { return _createError( ErrorKind.internal, diff --git a/dwds/lib/src/version.dart b/dwds/lib/src/version.dart index 780a52f21..06dc05173 100644 --- a/dwds/lib/src/version.dart +++ b/dwds/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '13.1.0'; +const packageVersion = '13.1.1-dev'; diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index 4af89c4eb..dfc8dbbc1 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -1,6 +1,6 @@ name: dwds # Every time this changes you need to run `dart run build_runner build`. -version: 13.1.0 +version: 13.1.1-dev homepage: https://github.com/dart-lang/webdev/tree/master/dwds description: >- A service that proxies between the Chrome debug protocol and the Dart VM diff --git a/dwds/test/build_daemon_breakpoint_test.dart b/dwds/test/build_daemon_breakpoint_test.dart index 847adedf9..876383a5e 100644 --- a/dwds/test/build_daemon_breakpoint_test.dart +++ b/dwds/test/build_daemon_breakpoint_test.dart @@ -74,6 +74,28 @@ void main() { await service.removeBreakpoint(isolate.id, bp.id); }); + test('set breakpoint inside a JavaScript line succeeds on', () async { + var line = await context.findBreakpointLine( + 'printNestedObjectMultiLine', isolate.id, mainScript); + var column = 0; + var bp = await service.addBreakpointWithScriptUri( + isolate.id, mainScript.uri, line, + column: column); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseBreakpoint); + + expect(bp, isNotNull); + expect( + bp.location, + isA() + .having((loc) => loc.script, 'script', equals(mainScript)) + .having((loc) => loc.line, 'line', equals(line)) + .having((loc) => loc.column, 'column', greaterThan(column))); + + // Remove breakpoint so it doesn't impact other tests. + await service.removeBreakpoint(isolate.id, bp.id); + }); + test('set breakpoint again', () async { var line = await context.findBreakpointLine( 'printLocal', isolate.id, mainScript); diff --git a/dwds/test/frontend_server_breakpoint_test.dart b/dwds/test/frontend_server_breakpoint_test.dart index 54cfe877b..6e2cef23f 100644 --- a/dwds/test/frontend_server_breakpoint_test.dart +++ b/dwds/test/frontend_server_breakpoint_test.dart @@ -104,6 +104,29 @@ void main() { // Remove breakpoint so it doesn't impact other tests. await service.removeBreakpoint(isolate.id, bp.id); }); + + test('set breakpoint inside a JavaScript line succeeds', () async { + var line = await context.findBreakpointLine( + 'printNestedObjectMultiLine', isolate.id, mainScript); + var column = 0; + var bp = await service.addBreakpointWithScriptUri( + isolate.id, mainScript.uri, line, + column: column); + + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseBreakpoint); + + expect(bp, isNotNull); + expect( + bp.location, + isA() + .having((loc) => loc.script, 'script', equals(mainScript)) + .having((loc) => loc.line, 'line', equals(line)) + .having((loc) => loc.column, 'column', greaterThan(column))); + + // Remove breakpoint so it doesn't impact other tests. + await service.removeBreakpoint(isolate.id, bp.id); + }); }); }); } diff --git a/fixtures/_testPackage/web/main.dart b/fixtures/_testPackage/web/main.dart index d966da70d..0d82f4b12 100644 --- a/fixtures/_testPackage/web/main.dart +++ b/fixtures/_testPackage/web/main.dart @@ -40,6 +40,7 @@ void main() { printFromTestPackage(); printCallExtension(); printLoopVariable(); + printNestedObjectsMultiLine(); }); document.body.appendText(concatenate('Program', ' is running!')); @@ -99,6 +100,13 @@ Future printDeferred() async { d.deferredPrintLocal(); } +void printNestedObjectsMultiLine() { + print(// Breakpoint: printEnclosingFunctionMultiLine + EnclosingMainClass(// Breakpoint: printEnclosingObjectMultiLine + MainClass(0) // Breakpoint: printNestedObjectMultiLine + )); +} + class MainClass { final int _field; MainClass(this._field); @@ -106,3 +114,11 @@ class MainClass { @override String toString() => '$_field'; } + +class EnclosingMainClass { + final MainClass _field; + EnclosingMainClass(this._field); + + @override + String toString() => '$_field'; +} diff --git a/fixtures/_testPackageSound/web/main.dart b/fixtures/_testPackageSound/web/main.dart index 54aeaa1b2..5305c9639 100644 --- a/fixtures/_testPackageSound/web/main.dart +++ b/fixtures/_testPackageSound/web/main.dart @@ -40,6 +40,7 @@ void main() async { printCallExtension(); printLoopVariable(); printGeneric(0); + printNestedObjectsMultiLine(); }); document.body?.appendText(concatenate('Program', ' is running!')); @@ -108,6 +109,13 @@ Future printDeferred() async { d.deferredPrintLocal(); } +void printNestedObjectsMultiLine() { + print(// Breakpoint: printEnclosingFunctionMultiLine + EnclosingMainClass(// Breakpoint: printEnclosingObjectMultiLine + MainClass(0) // Breakpoint: printNestedObjectMultiLine + )); +} + class MainClass { final int _field; MainClass(this._field); @@ -115,3 +123,11 @@ class MainClass { @override String toString() => '$_field'; } + +class EnclosingMainClass { + final MainClass _field; + EnclosingMainClass(this._field); + + @override + String toString() => '$_field'; +}