From 8a5228553d3e49e2734024cb7054830857a74ef5 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 4 Apr 2022 17:45:10 -0700 Subject: [PATCH 1/7] Add columns to breakpoints --- dwds/lib/src/debugging/debugger.dart | 42 +++++++++++++------ dwds/lib/src/debugging/location.dart | 13 ++++-- .../src/services/expression_evaluator.dart | 3 +- .../test/frontend_server_breakpoint_test.dart | 2 +- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index f1b4a6fba..f120c0bca 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 + 1, column + 1); } /// The variables visible in a frame in Dart protocol [BoundVariable] form. @@ -723,14 +728,23 @@ 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 line 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) { + var id = 'bp/$scriptId#$line:$column'; + print('breakpointIdFor: $scriptId, $line, $column: $id'); + return id; +} /// Keeps track of the Dart and JS breakpoint Ids that correspond. class _Breakpoints extends Domain { + final _logger = Logger('_Breakpoints'); final _dartIdByJsId = {}; final _jsIdByDartId = {}; @@ -752,11 +766,12 @@ 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); + _logger.warning( + '_createBreakpoint: uri: $dartUri, line: $line, column :$column, location: $location'); // TODO: Handle cases where a breakpoint can't be set exactly at that line. if (location == null) { throw RPCError( @@ -779,10 +794,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]. @@ -812,6 +827,7 @@ class _Breakpoints extends Domain { .sendCommand('Debugger.setBreakpointByUrl', params: { 'urlRegex': urlRegex, 'lineNumber': location.jsLocation.line - 1, + 'columnNumber': location.jsLocation.column - 1, }); return response.result['breakpointId'] as String; }); diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index ed66fc18e..562b8e1db 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; + print('Location: $jsLine:$jsColumn'); // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( @@ -155,17 +156,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 => + 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/services/expression_evaluator.dart b/dwds/lib/src/services/expression_evaluator.dart index 211e9f940..1f361f7dd 100644 --- a/dwds/lib/src/services/expression_evaluator.dart +++ b/dwds/lib/src/services/expression_evaluator.dart @@ -161,6 +161,7 @@ class ExpressionEvaluator { var functionName = jsFrame.functionName; var jsLine = jsFrame.location.lineNumber + 1; var jsScriptId = jsFrame.location.scriptId; + var jsColumn = jsFrame.location.columnNumber + 1; 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/test/frontend_server_breakpoint_test.dart b/dwds/test/frontend_server_breakpoint_test.dart index 54cfe877b..26f171015 100644 --- a/dwds/test/frontend_server_breakpoint_test.dart +++ b/dwds/test/frontend_server_breakpoint_test.dart @@ -88,7 +88,7 @@ void main() { // Remove breakpoint so it doesn't impact other tests. await service.removeBreakpoint(isolate.id, bp.id); - }); + }, solo: true); test('set breakpoint again', () async { var line = await context.findBreakpointLine( From fa4e3eba3960261894dee97a2f2133526f8c2fe2 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 8 Apr 2022 16:18:13 -0700 Subject: [PATCH 2/7] Added tests and updated version --- dwds/CHANGELOG.md | 3 +++ dwds/lib/src/debugging/debugger.dart | 14 +++++++----- dwds/lib/src/debugging/location.dart | 2 +- dwds/lib/src/injected/client.js | 20 +++++++++-------- dwds/lib/src/version.dart | 2 +- dwds/pubspec.yaml | 2 +- dwds/test/build_daemon_breakpoint_test.dart | 20 +++++++++++++++++ .../test/frontend_server_breakpoint_test.dart | 22 ++++++++++++++++++- fixtures/_testPackage/web/main.dart | 16 ++++++++++++++ fixtures/_testPackageSound/web/main.dart | 16 ++++++++++++++ 10 files changed, 98 insertions(+), 19 deletions(-) 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 f120c0bca..51df5a267 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -736,11 +736,8 @@ int _columnNumberFor(Breakpoint breakpoint) => /// Returns the breakpoint ID for the provided Dart script ID and Dart line /// number. -String breakpointIdFor(String scriptId, int line, int column) { - var id = 'bp/$scriptId#$line:$column'; - print('breakpointIdFor: $scriptId, $line, $column: $id'); - return id; -} +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 { @@ -807,7 +804,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; diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 562b8e1db..02f67b86d 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -40,7 +40,7 @@ class Location { var dartColumn = entry.sourceColumn; var jsLine = lineEntry.line; var jsColumn = entry.column; - print('Location: $jsLine:$jsColumn'); + // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( diff --git a/dwds/lib/src/injected/client.js b/dwds/lib/src/injected/client.js index 29409e86e..8e6635ca1 100644 --- a/dwds/lib/src/injected/client.js +++ b/dwds/lib/src/injected/client.js @@ -1,4 +1,4 @@ -// Generated by dart2js (NullSafetyMode.unsound, csp), the Dart to JavaScript compiler version: 2.17.0-282.0.dev. +// Generated by dart2js (NullSafetyMode.unsound, csp), the Dart to JavaScript compiler version: 2.17.0-266.0.dev. // The code supports the following hooks: // dartPrint(message): // if this function is defined it is called instead of the Dart [print] @@ -9389,11 +9389,6 @@ for (i = 0; i < len; ++i) receiver.push(array[i]); }, - clear$0(receiver) { - if (!!receiver.fixed$length) - A.throwExpression(A.UnsupportedError$("clear")); - receiver.length = 0; - }, forEach$1(receiver, f) { var end, i; A._arrayInstanceType(receiver)._eval$1("~(1)")._as(f); @@ -9547,6 +9542,13 @@ get$length(receiver) { return receiver.length; }, + set$length(receiver, newLength) { + if (!!receiver.fixed$length) + A.throwExpression(A.UnsupportedError$("set length")); + if (newLength < 0) + throw A.wrapException(A.RangeError$range(newLength, 0, null, "newLength", null)); + receiver.length = newLength; + }, $index(receiver, index) { if (!A._isInt(index)) throw A.wrapException(A.diagnoseIndexError(receiver, index)); @@ -14650,7 +14652,7 @@ return false; if (_this._splayCount !== t2._splayCount) { t3 = _this.$ti._eval$1("_SplayTreeIterator.K")._as(B.JSArray_methods.get$last(t1).key); - B.JSArray_methods.clear$0(t1); + B.JSArray_methods.set$length(t1, 0); t2._splay$1(t3); t3 = t2._root; t3.toString; @@ -14857,7 +14859,7 @@ if (t1 === 0) B.JSArray_methods.add$1(keys, ""); else - B.JSArray_methods.clear$0(keys); + B.JSArray_methods.set$length(keys, 0); _this._original = _this._processed = null; return _this._data = result; }, @@ -24188,7 +24190,7 @@ t3.lastPendingEvent = t4; } } - B.JSArray_methods.clear$0(buffer); + B.JSArray_methods.set$length(buffer, 0); } lastSendTime = lastSendTime0; } 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..ded2cb073 100644 --- a/dwds/test/build_daemon_breakpoint_test.dart +++ b/dwds/test/build_daemon_breakpoint_test.dart @@ -74,6 +74,26 @@ void main() { await service.removeBreakpoint(isolate.id, bp.id); }); + test('set breakpoint inside a JavaScript line succeeds', () async { + var line = await context.findBreakpointLine( + 'printNestedObjectsMultiLine', isolate.id, mainScript); + var bp = await service.addBreakpointWithScriptUri( + isolate.id, mainScript.uri, line); + + 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))); + + // 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 26f171015..88c102b0e 100644 --- a/dwds/test/frontend_server_breakpoint_test.dart +++ b/dwds/test/frontend_server_breakpoint_test.dart @@ -88,7 +88,7 @@ void main() { // Remove breakpoint so it doesn't impact other tests. await service.removeBreakpoint(isolate.id, bp.id); - }, solo: true); + }); test('set breakpoint again', () async { var line = await context.findBreakpointLine( @@ -104,6 +104,26 @@ 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( + 'printNestedObjectsMultiLine', isolate.id, mainScript); + var bp = await service.addBreakpointWithScriptUri( + isolate.id, mainScript.uri, line); + + 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))); + + // 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..ba8e9ebc1 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( + EnclosingMainClass( + MainClass(0) // Breakpoint: printNestedObjectsMultiLine + )); +} + 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'; +} \ No newline at end of file diff --git a/fixtures/_testPackageSound/web/main.dart b/fixtures/_testPackageSound/web/main.dart index 54aeaa1b2..cb5af6254 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( + EnclosingMainClass( + MainClass(0) // Breakpoint: printNestedObjectsMultiLine + )); +} + 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'; +} From 0ed66e31f51eaf4ad80a63ee379da66281cee18b Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 8 Apr 2022 16:34:08 -0700 Subject: [PATCH 3/7] Remove debug printing --- dwds/lib/src/debugging/debugger.dart | 3 --- fixtures/_testPackage/web/main.dart | 7 +++---- fixtures/_testPackageSound/web/main.dart | 5 ++--- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 51df5a267..083cfa924 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -741,7 +741,6 @@ String breakpointIdFor(String scriptId, int line, int column) => /// Keeps track of the Dart and JS breakpoint Ids that correspond. class _Breakpoints extends Domain { - final _logger = Logger('_Breakpoints'); final _dartIdByJsId = {}; final _jsIdByDartId = {}; @@ -767,8 +766,6 @@ class _Breakpoints extends Domain { var dartScript = inspector.scriptWithId(scriptId); var dartUri = DartUri(dartScript.uri, root); var location = await locations.locationForDart(dartUri, line, column); - _logger.warning( - '_createBreakpoint: uri: $dartUri, line: $line, column :$column, location: $location'); // TODO: Handle cases where a breakpoint can't be set exactly at that line. if (location == null) { throw RPCError( diff --git a/fixtures/_testPackage/web/main.dart b/fixtures/_testPackage/web/main.dart index ba8e9ebc1..e2b25a6a8 100644 --- a/fixtures/_testPackage/web/main.dart +++ b/fixtures/_testPackage/web/main.dart @@ -102,9 +102,8 @@ Future printDeferred() async { void printNestedObjectsMultiLine() { print( - EnclosingMainClass( - MainClass(0) // Breakpoint: printNestedObjectsMultiLine - )); + EnclosingMainClass(MainClass(0) // Breakpoint: printNestedObjectsMultiLine + )); } class MainClass { @@ -121,4 +120,4 @@ class EnclosingMainClass { @override String toString() => '$_field'; -} \ No newline at end of file +} diff --git a/fixtures/_testPackageSound/web/main.dart b/fixtures/_testPackageSound/web/main.dart index cb5af6254..240f00638 100644 --- a/fixtures/_testPackageSound/web/main.dart +++ b/fixtures/_testPackageSound/web/main.dart @@ -111,9 +111,8 @@ Future printDeferred() async { void printNestedObjectsMultiLine() { print( - EnclosingMainClass( - MainClass(0) // Breakpoint: printNestedObjectsMultiLine - )); + EnclosingMainClass(MainClass(0) // Breakpoint: printNestedObjectsMultiLine + )); } class MainClass { From 571acab438f5a0f5288a19b801e6960d8ce1cc86 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 11 Apr 2022 16:44:20 -0700 Subject: [PATCH 4/7] Removed adjusting JS locations from 0-based to 1-based and back --- dwds/lib/src/debugging/debugger.dart | 16 ++++++---------- dwds/lib/src/debugging/location.dart | 16 ++++++---------- dwds/lib/src/services/expression_evaluator.dart | 4 ++-- dwds/test/build_daemon_breakpoint_test.dart | 10 ++++++---- dwds/test/frontend_server_breakpoint_test.dart | 7 +++++-- 5 files changed, 25 insertions(+), 28 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 083cfa924..0f1a14c43 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -332,7 +332,7 @@ class Debugger extends Domain { var column = location['columnNumber'] as int; var url = _urlForScriptId(scriptId); - return _locations.locationForJs(url, line + 1, column + 1); + return _locations.locationForJs(url, line, column); } /// The variables visible in a frame in Dart protocol [BoundVariable] form. @@ -464,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) { @@ -730,7 +729,7 @@ bool isNativeJsObject(InstanceRef instanceRef) { int _lineNumberFor(Breakpoint breakpoint) => int.parse(breakpoint.id.split('#').last.split(':').first); -/// Returns the Dart line number for the provided breakpoint. +/// Returns the Dart column number for the provided breakpoint. int _columnNumberFor(Breakpoint breakpoint) => int.parse(breakpoint.id.split('#').last.split(':').last); @@ -814,9 +813,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 @@ -825,8 +821,8 @@ class _Breakpoints extends Domain { var response = await remoteDebugger .sendCommand('Debugger.setBreakpointByUrl', params: { 'urlRegex': urlRegex, - 'lineNumber': location.jsLocation.line - 1, - 'columnNumber': location.jsLocation.column - 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 02f67b86d..245f3f28f 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -72,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._( @@ -98,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); } diff --git a/dwds/lib/src/services/expression_evaluator.dart b/dwds/lib/src/services/expression_evaluator.dart index 1f361f7dd..8c1494eef 100644 --- a/dwds/lib/src/services/expression_evaluator.dart +++ b/dwds/lib/src/services/expression_evaluator.dart @@ -159,9 +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 + 1; + var jsColumn = jsFrame.location.columnNumber; var jsScope = await _collectLocalJsScope(jsFrame); // Find corresponding dart location and scope. diff --git a/dwds/test/build_daemon_breakpoint_test.dart b/dwds/test/build_daemon_breakpoint_test.dart index ded2cb073..e7c12b8c0 100644 --- a/dwds/test/build_daemon_breakpoint_test.dart +++ b/dwds/test/build_daemon_breakpoint_test.dart @@ -74,12 +74,13 @@ void main() { await service.removeBreakpoint(isolate.id, bp.id); }); - test('set breakpoint inside a JavaScript line succeeds', () async { + test('set breakpoint inside a JavaScript line succeeds on', () async { var line = await context.findBreakpointLine( 'printNestedObjectsMultiLine', isolate.id, mainScript); + var column = 0; var bp = await service.addBreakpointWithScriptUri( - isolate.id, mainScript.uri, line); - + isolate.id, mainScript.uri, line, + column: column); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseBreakpoint); @@ -88,7 +89,8 @@ void main() { bp.location, isA() .having((loc) => loc.script, 'script', equals(mainScript)) - .having((loc) => loc.line, 'line', equals(line))); + .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/dwds/test/frontend_server_breakpoint_test.dart b/dwds/test/frontend_server_breakpoint_test.dart index 88c102b0e..b45f5ad4f 100644 --- a/dwds/test/frontend_server_breakpoint_test.dart +++ b/dwds/test/frontend_server_breakpoint_test.dart @@ -108,8 +108,10 @@ void main() { test('set breakpoint inside a JavaScript line succeeds', () async { var line = await context.findBreakpointLine( 'printNestedObjectsMultiLine', isolate.id, mainScript); + var column = 0; var bp = await service.addBreakpointWithScriptUri( - isolate.id, mainScript.uri, line); + isolate.id, mainScript.uri, line, + column: column); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseBreakpoint); @@ -119,7 +121,8 @@ void main() { bp.location, isA() .having((loc) => loc.script, 'script', equals(mainScript)) - .having((loc) => loc.line, 'line', equals(line))); + .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); From b5b2ab147c1f9be1ed69d8b2c904d72905283a57 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Tue, 12 Apr 2022 10:18:26 -0700 Subject: [PATCH 5/7] Add debug information on failure to set breakpoints --- dwds/lib/src/debugging/debugger.dart | 5 +++++ dwds/lib/src/debugging/location.dart | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 0f1a14c43..ddc9ed0df 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -740,6 +740,7 @@ String breakpointIdFor(String scriptId, int line, int column) => /// Keeps track of the Dart and JS breakpoint Ids that correspond. class _Breakpoints extends Domain { + final logger = Logger('Breakpoints'); final _dartIdByJsId = {}; final _jsIdByDartId = {}; @@ -767,6 +768,10 @@ class _Breakpoints extends Domain { 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, diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 245f3f28f..1dad90baa 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -161,7 +161,7 @@ class Locations { /// Find the [Location] for the given JS source position. /// - /// The [line] number is 1-based. + /// The [line] number is 0-based. Future locationForJs(String url, int line, int column) async => (await locationsForUrl(url)).firstWhere( (location) => From 65d6f48f5c18ba0c66d2fe0fa7efef0095596196 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Tue, 12 Apr 2022 10:53:23 -0700 Subject: [PATCH 6/7] Update tests --- dwds/test/build_daemon_breakpoint_test.dart | 2 +- dwds/test/frontend_server_breakpoint_test.dart | 2 +- fixtures/_testPackage/web/main.dart | 5 +++-- fixtures/_testPackageSound/web/main.dart | 5 +++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/dwds/test/build_daemon_breakpoint_test.dart b/dwds/test/build_daemon_breakpoint_test.dart index e7c12b8c0..876383a5e 100644 --- a/dwds/test/build_daemon_breakpoint_test.dart +++ b/dwds/test/build_daemon_breakpoint_test.dart @@ -76,7 +76,7 @@ void main() { test('set breakpoint inside a JavaScript line succeeds on', () async { var line = await context.findBreakpointLine( - 'printNestedObjectsMultiLine', isolate.id, mainScript); + 'printNestedObjectMultiLine', isolate.id, mainScript); var column = 0; var bp = await service.addBreakpointWithScriptUri( isolate.id, mainScript.uri, line, diff --git a/dwds/test/frontend_server_breakpoint_test.dart b/dwds/test/frontend_server_breakpoint_test.dart index b45f5ad4f..6e2cef23f 100644 --- a/dwds/test/frontend_server_breakpoint_test.dart +++ b/dwds/test/frontend_server_breakpoint_test.dart @@ -107,7 +107,7 @@ void main() { test('set breakpoint inside a JavaScript line succeeds', () async { var line = await context.findBreakpointLine( - 'printNestedObjectsMultiLine', isolate.id, mainScript); + 'printNestedObjectMultiLine', isolate.id, mainScript); var column = 0; var bp = await service.addBreakpointWithScriptUri( isolate.id, mainScript.uri, line, diff --git a/fixtures/_testPackage/web/main.dart b/fixtures/_testPackage/web/main.dart index e2b25a6a8..0d82f4b12 100644 --- a/fixtures/_testPackage/web/main.dart +++ b/fixtures/_testPackage/web/main.dart @@ -101,8 +101,9 @@ Future printDeferred() async { } void printNestedObjectsMultiLine() { - print( - EnclosingMainClass(MainClass(0) // Breakpoint: printNestedObjectsMultiLine + print(// Breakpoint: printEnclosingFunctionMultiLine + EnclosingMainClass(// Breakpoint: printEnclosingObjectMultiLine + MainClass(0) // Breakpoint: printNestedObjectMultiLine )); } diff --git a/fixtures/_testPackageSound/web/main.dart b/fixtures/_testPackageSound/web/main.dart index 240f00638..5305c9639 100644 --- a/fixtures/_testPackageSound/web/main.dart +++ b/fixtures/_testPackageSound/web/main.dart @@ -110,8 +110,9 @@ Future printDeferred() async { } void printNestedObjectsMultiLine() { - print( - EnclosingMainClass(MainClass(0) // Breakpoint: printNestedObjectsMultiLine + print(// Breakpoint: printEnclosingFunctionMultiLine + EnclosingMainClass(// Breakpoint: printEnclosingObjectMultiLine + MainClass(0) // Breakpoint: printNestedObjectMultiLine )); } From 5b479da0bb9e599726e03035f805a44aeeee2ad2 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Wed, 13 Apr 2022 10:22:08 -0700 Subject: [PATCH 7/7] Update skiplist tests --- dwds/lib/src/debugging/skip_list.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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));