diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index d62fd0d31..a657e6eef 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -11,9 +11,19 @@ - Fix breakpoints not hitting after changing a base in index.html. - Find best locations for call frames, breakpoints, or expression evaluation. - Close the SSE connection when a DebugExtension.detached event is received. +- Fix issues discovered when using legacy module system, debug extension, + and JIT modules: + - Improve step-into times by not stepping into library loading code. + - Fix incorrect skip lists due to unsorted locations. + - Fix memory leak in extension debugger by removing stale script IDs. + - Allow mapping JS locations to Dart locations matching other JS lines, + to match the behavior of Chrome DevTools. + - Fix expression evaluation failure if debugger is stopped in the middle + of a variable definition. **Breaking changes:** - Add `basePath` parameter to `FrontendServerRequireStrategy`. +- Add `loadLibrariesModule` getter to `LoadStrategy` interface. ## 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 f1332bf74..b761879cb 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -332,9 +332,16 @@ class Debugger extends Domain { var column = location['columnNumber'] as int; var url = urlForScriptId(scriptId); + if (url == null) return null; return _locations.locationForJs(url, line, column); } + /// Returns script ID for the paused event. + String _frameScriptId(DebuggerPausedEvent e) { + var frame = e.params['callFrames'][0]; + return frame['location']['scriptId'] as String; + } + /// The variables visible in a frame in Dart protocol [BoundVariable] form. Future> variablesFor(WipCallFrame frame) async { // TODO(alanknight): Can these be moved to dart_scope.dart? @@ -470,7 +477,7 @@ class Debugger extends Domain { var url = urlForScriptId(location.scriptId); if (url == null) { logger.severe('Failed to create dart frame for ${frame.functionName}: ' - 'cannot find location for script ${location.scriptId}'); + 'cannot find url for script ${location.scriptId}'); return null; } @@ -567,26 +574,32 @@ class Debugger extends Domain { exception: exception, ); } else { - // If we don't have source location continue stepping. - if (_isStepping && (await _sourceLocation(e)) == null) { - var frame = e.params['callFrames'][0]; - var scriptId = '${frame["location"]["scriptId"]}'; - + // Continue stepping until we hit a dart location, + // avoiding stepping through library loading code. + if (_isStepping) { + var scriptId = _frameScriptId(e); var url = urlForScriptId(scriptId); + if (url == null) { logger.severe('Stepping failed: ' - 'cannot find location for script $scriptId'); + 'cannot find url for script $scriptId'); + throw StateError('Stepping failed in script $scriptId'); } - // TODO(grouma) - In the future we should send all previously computed - // skipLists. - await _remoteDebugger.stepInto(params: { - 'skipList': await _skipLists.compute( - scriptId, - await _locations.locationsForUrl(url), - ) - }); - return; + if (url.contains(globalLoadStrategy.loadLibrariesModule)) { + await _remoteDebugger.stepOut(); + return; + } else if ((await _sourceLocation(e)) == null) { + // TODO(grouma) - In the future we should send all previously computed + // skipLists. + await _remoteDebugger.stepInto(params: { + 'skipList': await _skipLists.compute( + scriptId, + await _locations.locationsForUrl(url), + ) + }); + return; + } } event = Event( kind: EventKind.kPauseInterrupted, diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index acdd19b38..48f62bef8 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -69,6 +69,13 @@ class DartLocation { this.column, ); + int compareTo(DartLocation other) => compareToLine(other.line, other.column); + + int compareToLine(int otherLine, int otherColumn) { + var result = line.compareTo(otherLine); + return result == 0 ? column.compareTo(otherColumn) : result; + } + @override String toString() => '[${uri.serverPath}:$line:$column]'; @@ -92,6 +99,13 @@ class JsLocation { this.column, ); + int compareTo(JsLocation other) => compareToLine(other.line, other.column); + + int compareToLine(int otherLine, int otherColumn) { + var result = line.compareTo(otherLine); + return result == 0 ? column.compareTo(otherColumn) : result; + } + @override String toString() => '[$module:$line:$column]'; @@ -177,7 +191,7 @@ class Locations { if (location.dartLocation.line == line && location.dartLocation.column >= column) { bestLocation ??= location; - if (location.dartLocation.column < bestLocation.dartLocation.column) { + if (location.dartLocation.compareTo(bestLocation.dartLocation) < 0) { bestLocation = location; } } @@ -188,76 +202,23 @@ class Locations { /// Find closest existing JavaScript location for the line and column. /// /// Some JS locations are not stored in the source maps, so we find the - /// closest existing location, preferring the one coming after the given - /// column, if the current break is at an expression statement, or the - /// one coming before if the current break is at a function call. + /// closest existing location coming before the given column. /// /// This is a known problem that other code bases solve using by finding /// the closest location to the current one: /// /// https://github.com/microsoft/vscode-js-debug/blob/536f96bae61a3d87546b61bc7916097904c81429/src/common/sourceUtils.ts#L286 - /// - /// Unfortunately, this approach fails for Flutter code too often, as it - /// frequently contains multi-line statements with nested objects. - /// - /// For example: - /// - /// - `t33 = main.doSomething()` in top frame: - /// Current column is at `t33`. Return existing location starting - /// at `main`. - /// - `main.doSomething()` in top frame: - /// Current column is at `main`. Return existing location starting - /// at `main`. - /// - `main.doSomething()` in a frame down the stack: - /// Current column is at `doSomething`. Source map does not have a - /// location stored that starts at `doSomething()`. Return existing - /// location starting at `main`. Location _bestJsLocation(Iterable locations, int line, int column) { - Location bestLocationBefore; - Location bestLocationAfter; - - var locationsAfter = locations.where((location) => - location.jsLocation.line == line && - location.jsLocation.column >= column); - var locationsBefore = locations.where((location) => - location.jsLocation.line == line && - location.jsLocation.column < column); - - for (var location in locationsAfter) { - bestLocationAfter ??= location; - if (location.jsLocation.column < bestLocationAfter.jsLocation.column) { - bestLocationAfter = location; - } - } - for (var location in locationsBefore) { - bestLocationBefore ??= location; - if (location.jsLocation.column > bestLocationBefore.jsLocation.column) { - bestLocationBefore = location; + Location bestLocation; + for (var location in locations) { + if (location.jsLocation.compareToLine(line, column) <= 0) { + bestLocation ??= location; + if (location.jsLocation.compareTo(bestLocation.jsLocation) > 0) { + bestLocation = location; + } } } - if (bestLocationAfter == null) return bestLocationBefore; - if (bestLocationBefore == null) return bestLocationAfter; - - if (bestLocationAfter.jsLocation.line == line && - bestLocationAfter.jsLocation.column == column) { - // Prefer exact match. - return bestLocationAfter; - } - - // Return the closest location after the current if the current location - // is at the beginning of the line (i.e. on expression statement). - // Return the closest location before the current if the current location - // is in the middle of the line (i.e. on function call). - if (locationsBefore.length == 1 && - locationsBefore.first.jsLocation.column == 0) { - // Best guess on whether the the current location is at the beginning of - // the line (i.e. expression statement): - // The only location on the left has column 0, so only spaces are on the - // left. - return bestLocationAfter; - } - // Current column is in the middle of the line (i.e .function call). - return bestLocationBefore; + return bestLocation; } /// 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 07bc583cf..7a34fcc36 100644 --- a/dwds/lib/src/debugging/skip_list.dart +++ b/dwds/lib/src/debugging/skip_list.dart @@ -26,11 +26,13 @@ class SkipLists { ) async { if (_idToList.containsKey(scriptId)) return _idToList[scriptId]; + var sortedLocations = locations.toList() + ..sort((a, b) => a.jsLocation.compareTo(b.jsLocation)); + var ranges = >[]; var startLine = 0; var startColumn = 0; - for (var location in locations) { - // Account for 1 based. + for (var location in sortedLocations) { var endLine = location.jsLocation.line; var endColumn = location.jsLocation.column; // Stop before the known location. @@ -40,8 +42,10 @@ class SkipLists { endColumn = maxValue; } if (endLine > startLine || endColumn > startColumn) { - ranges.add( - _rangeFor(scriptId, startLine, startColumn, endLine, endColumn)); + if (endLine >= startLine) { + ranges.add( + _rangeFor(scriptId, startLine, startColumn, endLine, endColumn)); + } startLine = location.jsLocation.line; startColumn = location.jsLocation.column + 1; } diff --git a/dwds/lib/src/loaders/legacy.dart b/dwds/lib/src/loaders/legacy.dart index e712ab58c..df4c02206 100644 --- a/dwds/lib/src/loaders/legacy.dart +++ b/dwds/lib/src/loaders/legacy.dart @@ -79,6 +79,9 @@ class LegacyStrategy extends LoadStrategy { @override String get moduleFormat => 'ddc'; + @override + String get loadLibrariesModule => 'dart_library.ddk.js'; + @override String get loadLibrariesSnippet => 'for(let module of dart_library.libraries()) {\n' diff --git a/dwds/lib/src/loaders/require.dart b/dwds/lib/src/loaders/require.dart index 15f134ec5..d19bc5935 100644 --- a/dwds/lib/src/loaders/require.dart +++ b/dwds/lib/src/loaders/require.dart @@ -169,6 +169,9 @@ class RequireStrategy extends LoadStrategy { @override String get moduleFormat => 'amd'; + @override + String get loadLibrariesModule => 'require.js'; + @override String get loadLibrariesSnippet => 'let libs = $loadModuleSnippet("dart_sdk").dart.getLibraries();\n'; diff --git a/dwds/lib/src/loaders/strategy.dart b/dwds/lib/src/loaders/strategy.dart index 801266aa4..5930e80bd 100644 --- a/dwds/lib/src/loaders/strategy.dart +++ b/dwds/lib/src/loaders/strategy.dart @@ -36,6 +36,11 @@ abstract class LoadStrategy { /// expression evaluation. String get moduleFormat; + /// Module containing code for loading libraries. + /// + /// Used for preventing stepping into the library loading code. + String get loadLibrariesModule; + /// Returns a snippet of JS code that loads all Dart libraries into a `libs` /// variable. String get loadLibrariesSnippet; diff --git a/dwds/lib/src/servers/extension_debugger.dart b/dwds/lib/src/servers/extension_debugger.dart index 385f64e0b..854b2c704 100644 --- a/dwds/lib/src/servers/extension_debugger.dart +++ b/dwds/lib/src/servers/extension_debugger.dart @@ -64,6 +64,7 @@ class ExtensionDebugger implements RemoteDebugger { (WipEvent event) => ExceptionThrownEvent(event.json)); final _scripts = {}; + final _scriptIds = {}; ExtensionDebugger(this.sseConnection) { sseConnection.stream.listen((data) { @@ -113,7 +114,13 @@ class ExtensionDebugger implements RemoteDebugger { close(); }, onDone: close); onScriptParsed.listen((event) { + // Remove stale scripts from cache. + if (event.script.url.isNotEmpty && + _scriptIds.containsKey(event.script.url)) { + _scripts.remove(_scriptIds[event.script.url]); + } _scripts[event.script.scriptId] = event.script; + _scriptIds[event.script.url] = event.script.scriptId; }); // Listens for a page reload. onGlobalObjectCleared.listen((_) { diff --git a/dwds/lib/src/services/expression_evaluator.dart b/dwds/lib/src/services/expression_evaluator.dart index 9fabccc9a..4899f7d34 100644 --- a/dwds/lib/src/services/expression_evaluator.dart +++ b/dwds/lib/src/services/expression_evaluator.dart @@ -229,6 +229,8 @@ class ExpressionEvaluator { return ret; } + bool _isUndefined(RemoteObject value) => value.type == 'undefined'; + RemoteObject _formatCompilationError(String error) { // Frontend currently gives a text message including library name // and function name on compilation error. Strip this information @@ -284,6 +286,7 @@ class ExpressionEvaluator { for (var p in variables) { var name = p.name; var value = p.value; + if (_isUndefined(value)) continue; if (scopeType == 'closure') { // Substitute potentially unavailable captures with their values from diff --git a/dwds/test/build_daemon_callstack_test.dart b/dwds/test/build_daemon_callstack_test.dart index 7241b54c8..21a615229 100644 --- a/dwds/test/build_daemon_callstack_test.dart +++ b/dwds/test/build_daemon_callstack_test.dart @@ -53,7 +53,8 @@ void main() { group('${soundNullSafety ? "sound" : "weak"} null safety |', () { setUpAll(() async { setCurrentLogWriter(debug: debug); - await context.setUp(enableExpressionEvaluation: true); + await context.setUp( + enableExpressionEvaluation: true, verboseCompiler: debug); }); tearDownAll(() async { @@ -66,6 +67,7 @@ void main() { Isolate isolate; ScriptList scripts; ScriptRef mainScript; + ScriptRef testLibraryScript; Stream stream; setUp(() async { @@ -78,25 +80,32 @@ void main() { await service.streamListen('Debug'); stream = service.onEvent('Debug'); + var testPackage = + soundNullSafety ? '_test_package_sound' : '_test_package'; + mainScript = scripts.scripts .firstWhere((each) => each.uri.contains('main.dart')); + testLibraryScript = scripts.scripts.firstWhere((each) => + each.uri.contains('package:$testPackage/test_library.dart')); }); tearDown(() async { await service.resume(isolate.id); }); - Future onBreakPoint(ScriptRef script, String breakPointId, + Future onBreakPoint(BreakpointTestData breakpoint, Future Function() body) async { Breakpoint bp; try { - var line = await context.findBreakpointLine( - breakPointId, isolate.id, script); + var bpId = breakpoint.bpId; + var script = breakpoint.script; + var line = + await context.findBreakpointLine(bpId, isolate.id, script); bp = await setup.service .addBreakpointWithScriptUri(isolate.id, script.uri, line); expect(bp, isNotNull); - expect(bp.location, _matchBpLocation(mainScript, line, 0)); + expect(bp.location, _matchBpLocation(script, line, 0)); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseBreakpoint); @@ -110,11 +119,11 @@ void main() { } } - Future testCallStack( - List bpIds, List functions) async { + Future testCallStack(List breakpoints, + {int frameIndex = 1}) async { // Find lines the breakpoints are located on. - var lines = await Future.wait(bpIds.map((bpId) => - context.findBreakpointLine(bpId, isolate.id, mainScript))); + var lines = await Future.wait(breakpoints.map((frame) => context + .findBreakpointLine(frame.bpId, isolate.id, frame.script))); // Get current stack. var stack = await service.getStack(isolate.id); @@ -123,111 +132,169 @@ void main() { expect(stack.frames.length, greaterThanOrEqualTo(lines.length)); var expected = [ for (var i = 0; i < lines.length; i++) - _matchFrame(mainScript, functions[i], lines[i]) + _matchFrame( + breakpoints[i].script, breakpoints[i].function, lines[i]) ]; expect(stack.frames, containsAll(expected)); // Verify that expression evaluation is not failing. - var instance = await service.evaluateInFrame(isolate.id, 1, 'main'); + var instance = + await service.evaluateInFrame(isolate.id, frameIndex, 'true'); expect(instance, isA()); } test('breakpoint succeeds with correct callstack', () async { - // Breakpoint IDs. - var bpIds = [ - 'printEnclosingObject', - 'printEnclosingFunctionMultiLine', - 'callPrintEnclosingFunctionMultiLine', - ]; - // Functions breakpoints are located in. - var functions = [ - 'printEnclosingObject', - 'printNestedObjectsMultiLine', - '', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'printEnclosingObject', + 'printEnclosingObject', + mainScript, + ), + BreakpointTestData( + 'printEnclosingFunctionMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; await onBreakPoint( - mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + breakpoints[0], () => testCallStack(breakpoints)); }); - test('breakpoint inside a line gives correct callstack', () async { - // Breakpoint IDs. - var bpIds = [ - 'newEnclosedClass', - 'printNestedObjectMultiLine', - 'callPrintEnclosingFunctionMultiLine', + test('expression evaluation succeeds on parent frame', () async { + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'testLibraryClassConstructor', + 'new', + testLibraryScript, + ), + BreakpointTestData( + 'createLibraryObject', + 'printFieldFromLibraryClass', + mainScript, + ), + BreakpointTestData( + 'callPrintFieldFromLibraryClass', + '', + mainScript, + ), ]; - // Functions breakpoints are located in. - var functions = [ - 'new', - 'printNestedObjectsMultiLine', - '', + await onBreakPoint(breakpoints[0], + () => testCallStack(breakpoints, frameIndex: 2)); + }); + + test('breakpoint inside a line gives correct callstack', () async { + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'newEnclosedClass', + 'new', + mainScript, + ), + BreakpointTestData( + 'printNestedObjectMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; await onBreakPoint( - mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + breakpoints[0], () => testCallStack(breakpoints)); }); test('breakpoint gives correct callstack after step out', () async { - // Breakpoint IDs. - var bpIds = [ - 'printEnclosingObjectMultiLine', - 'callPrintEnclosingFunctionMultiLine', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'newEnclosedClass', + 'new', + mainScript, + ), + BreakpointTestData( + 'printEnclosingObjectMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; - // Functions breakpoints are located in. - var functions = [ - 'printNestedObjectsMultiLine', - '', - ]; - await onBreakPoint(mainScript, 'newEnclosedClass', () async { + await onBreakPoint(breakpoints[0], () async { await service.resume(isolate.id, step: 'Out'); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseInterrupted); - return testCallStack(bpIds, functions); + return testCallStack([breakpoints[1], breakpoints[2]]); }); }); test('breakpoint gives correct callstack after step in', () async { - // Breakpoint IDs. - var bpIds = [ - 'newEnclosedClass', - 'printNestedObjectMultiLine', - 'callPrintEnclosingFunctionMultiLine', - ]; - // Functions breakpoints are located in. - var functions = [ - 'new', - 'printNestedObjectsMultiLine', - '', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'newEnclosedClass', + 'new', + mainScript, + ), + BreakpointTestData( + 'printNestedObjectMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; - await onBreakPoint(mainScript, 'printNestedObjectMultiLine', - () async { + await onBreakPoint(breakpoints[1], () async { await service.resume(isolate.id, step: 'Into'); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseInterrupted); - return testCallStack(bpIds, functions); + return testCallStack(breakpoints); }); }); test('breakpoint gives correct callstack after step into chain calls', () async { - // Breakpoint IDs. - var bpIds = [ - 'createObjectWithMethod', - // This is currently incorrect, should be printObjectMultiLine. - // See issue: https://github.com/dart-lang/sdk/issues/48874 - 'printMultiLine', - 'callPrintObjectMultiLine', - ]; - // Functions breakpoints are located in. - var functions = [ - 'createObject', - 'printObjectMultiLine', - '', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'createObjectWithMethod', + 'createObject', + mainScript, + ), + BreakpointTestData( + // This is currently incorrect, should be printObjectMultiLine. + // See issue: https://github.com/dart-lang/sdk/issues/48874 + 'printMultiLine', + 'printObjectMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintObjectMultiLine', + '', + mainScript, + ), ]; - await onBreakPoint(mainScript, 'printMultiLine', () async { + var bp = BreakpointTestData( + 'printMultiLine', 'printObjectMultiLine', mainScript); + await onBreakPoint(bp, () async { await service.resume(isolate.id, step: 'Into'); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseInterrupted); - return testCallStack(bpIds, functions); + return testCallStack(breakpoints); }); }); }); @@ -250,3 +317,11 @@ Matcher _matchBpLocation(ScriptRef script, int line, int column) => Matcher _matchFrameLocation(ScriptRef script, int line) => isA() .having((loc) => loc.script, 'script', equals(script)) .having((loc) => loc.line, 'line', equals(line)); + +class BreakpointTestData { + String bpId; + String function; + ScriptRef script; + + BreakpointTestData(this.bpId, this.function, this.script); +} diff --git a/dwds/test/fixtures/fakes.dart b/dwds/test/fixtures/fakes.dart index d26534724..d3c57a4f5 100644 --- a/dwds/test/fixtures/fakes.dart +++ b/dwds/test/fixtures/fakes.dart @@ -275,6 +275,9 @@ class FakeStrategy implements LoadStrategy { @override String get moduleFormat => 'dummy-format'; + @override + String get loadLibrariesModule => ''; + @override String get loadLibrariesSnippet => ''; diff --git a/dwds/test/frontend_server_callstack_test.dart b/dwds/test/frontend_server_callstack_test.dart index 79dce043e..8ff5b8457 100644 --- a/dwds/test/frontend_server_callstack_test.dart +++ b/dwds/test/frontend_server_callstack_test.dart @@ -50,14 +50,14 @@ void main() { var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); var context = setup.context; - group('${soundNullSafety ? "" : "no "}sound null safety |', () { + group('${soundNullSafety ? "sound" : "weak"} null safety |', () { setUpAll(() async { setCurrentLogWriter(debug: debug); await context.setUp( - enableExpressionEvaluation: true, - compilationMode: CompilationMode.frontendServer, - soundNullSafety: soundNullSafety, - ); + compilationMode: CompilationMode.frontendServer, + soundNullSafety: soundNullSafety, + enableExpressionEvaluation: true, + verboseCompiler: debug); }); tearDownAll(() async { @@ -70,6 +70,7 @@ void main() { Isolate isolate; ScriptList scripts; ScriptRef mainScript; + ScriptRef testLibraryScript; Stream stream; setUp(() async { @@ -82,25 +83,32 @@ void main() { await service.streamListen('Debug'); stream = service.onEvent('Debug'); + var testPackage = + soundNullSafety ? '_test_package_sound' : '_test_package'; + mainScript = scripts.scripts .firstWhere((each) => each.uri.contains('main.dart')); + testLibraryScript = scripts.scripts.firstWhere((each) => + each.uri.contains('package:$testPackage/test_library.dart')); }); tearDown(() async { await service.resume(isolate.id); }); - Future onBreakPoint(ScriptRef script, String breakPointId, + Future onBreakPoint(BreakpointTestData breakpoint, Future Function() body) async { Breakpoint bp; try { - var line = await context.findBreakpointLine( - breakPointId, isolate.id, script); + var bpId = breakpoint.bpId; + var script = breakpoint.script; + var line = + await context.findBreakpointLine(bpId, isolate.id, script); bp = await setup.service .addBreakpointWithScriptUri(isolate.id, script.uri, line); expect(bp, isNotNull); - expect(bp.location, _matchBpLocation(mainScript, line, 0)); + expect(bp.location, _matchBpLocation(script, line, 0)); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseBreakpoint); @@ -114,11 +122,11 @@ void main() { } } - Future testCallStack( - List bpIds, List functions) async { + Future testCallStack(List breakpoints, + {int frameIndex = 1}) async { // Find lines the breakpoints are located on. - var lines = await Future.wait(bpIds.map((bpId) => - context.findBreakpointLine(bpId, isolate.id, mainScript))); + var lines = await Future.wait(breakpoints.map((frame) => context + .findBreakpointLine(frame.bpId, isolate.id, frame.script))); // Get current stack. var stack = await service.getStack(isolate.id); @@ -127,117 +135,174 @@ void main() { expect(stack.frames.length, greaterThanOrEqualTo(lines.length)); var expected = [ for (var i = 0; i < lines.length; i++) - _matchFrame(mainScript, functions[i], lines[i]) + _matchFrame( + breakpoints[i].script, breakpoints[i].function, lines[i]) ]; expect(stack.frames, containsAll(expected)); // Verify that expression evaluation is not failing. - var instance = await service.evaluateInFrame(isolate.id, 1, 'main'); + var instance = + await service.evaluateInFrame(isolate.id, frameIndex, 'true'); expect(instance, isA()); } test('breakpoint succeeds with correct callstack', () async { - // Breakpoint IDs. - var bpIds = [ - 'printEnclosingObject', - 'printEnclosingFunctionMultiLine', - 'callPrintEnclosingFunctionMultiLine', - ]; - // Functions breakpoints are located in. - var functions = [ - 'printEnclosingObject', - 'printNestedObjectsMultiLine', - '', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'printEnclosingObject', + 'printEnclosingObject', + mainScript, + ), + BreakpointTestData( + 'printEnclosingFunctionMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; await onBreakPoint( - mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + breakpoints[0], () => testCallStack(breakpoints)); }); - test('breakpoint inside a line gives correct callstack', () async { - // Breakpoint IDs. - var bpIds = [ - 'newEnclosedClass', - 'printNestedObjectMultiLine', - 'callPrintEnclosingFunctionMultiLine', + test('expression evaluation succeeds on parent frame', () async { + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'testLibraryClassConstructor', + 'new', + testLibraryScript, + ), + BreakpointTestData( + 'createLibraryObject', + 'printFieldFromLibraryClass', + mainScript, + ), + BreakpointTestData( + 'callPrintFieldFromLibraryClass', + '', + mainScript, + ), ]; - // Functions breakpoints are located in. - var functions = [ - 'new', - 'printNestedObjectsMultiLine', - '', + await onBreakPoint(breakpoints[0], + () => testCallStack(breakpoints, frameIndex: 2)); + }); + + test('breakpoint inside a line gives correct callstack', () async { + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'newEnclosedClass', + 'new', + mainScript, + ), + BreakpointTestData( + 'printNestedObjectMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; await onBreakPoint( - mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + breakpoints[0], () => testCallStack(breakpoints)); }); test('breakpoint gives correct callstack after step out', () async { - // Breakpoint IDs. - var bpIds = [ - 'printEnclosingObjectMultiLine', - 'callPrintEnclosingFunctionMultiLine', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'newEnclosedClass', + 'new', + mainScript, + ), + BreakpointTestData( + 'printEnclosingObjectMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; - // Functions breakpoints are located in. - var functions = [ - 'printNestedObjectsMultiLine', - '', - ]; - await onBreakPoint(mainScript, 'newEnclosedClass', () async { + await onBreakPoint(breakpoints[0], () async { await service.resume(isolate.id, step: 'Out'); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseInterrupted); - return testCallStack(bpIds, functions); + return testCallStack([breakpoints[1], breakpoints[2]]); }); }); test('breakpoint gives correct callstack after step in', () async { - // Breakpoint IDs. - var bpIds = [ - 'newEnclosedClass', - 'printNestedObjectMultiLine', - 'callPrintEnclosingFunctionMultiLine', - ]; - // Functions breakpoints are located in. - var functions = [ - 'new', - 'printNestedObjectsMultiLine', - '', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'newEnclosedClass', + 'new', + mainScript, + ), + BreakpointTestData( + 'printNestedObjectMultiLine', + 'printNestedObjectsMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintEnclosingFunctionMultiLine', + '', + mainScript, + ), ]; - await onBreakPoint(mainScript, 'printNestedObjectMultiLine', - () async { + await onBreakPoint(breakpoints[1], () async { await service.resume(isolate.id, step: 'Into'); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseInterrupted); - return testCallStack(bpIds, functions); + return testCallStack(breakpoints); }); }); test('breakpoint gives correct callstack after step into chain calls', () async { - // Breakpoint IDs. - var bpIds = [ - 'createObjectWithMethod', - // This is currently incorrect, should be printObjectMultiLine. - // See issue: https://github.com/dart-lang/sdk/issues/48874 - 'printMultiLine', - 'callPrintObjectMultiLine', - ]; - // Functions breakpoints are located in. - var functions = [ - 'createObject', - 'printObjectMultiLine', - '', + // Expected breakpoints on the stack + var breakpoints = [ + BreakpointTestData( + 'createObjectWithMethod', + 'createObject', + mainScript, + ), + BreakpointTestData( + // This is currently incorrect, should be printObjectMultiLine. + // See issue: https://github.com/dart-lang/sdk/issues/48874 + 'printMultiLine', + 'printObjectMultiLine', + mainScript, + ), + BreakpointTestData( + 'callPrintObjectMultiLine', + '', + mainScript, + ), ]; - await onBreakPoint(mainScript, 'printMultiLine', () async { + var bp = BreakpointTestData( + 'printMultiLine', 'printObjectMultiLine', mainScript); + await onBreakPoint(bp, () async { await service.resume(isolate.id, step: 'Into'); await stream.firstWhere( (Event event) => event.kind == EventKind.kPauseInterrupted); - return testCallStack(bpIds, functions); + return testCallStack(breakpoints); }); }); }); - }, - skip: - soundNullSafety); // https://github.com/dart-lang/webdev/issues/1591 + }, // https://github.com/dart-lang/webdev/issues/1591 + skip: soundNullSafety); } }); } @@ -256,3 +321,11 @@ Matcher _matchBpLocation(ScriptRef script, int line, int column) => Matcher _matchFrameLocation(ScriptRef script, int line) => isA() .having((loc) => loc.script, 'script', equals(script)) .having((loc) => loc.line, 'line', equals(line)); + +class BreakpointTestData { + String bpId; + String function; + ScriptRef script; + + BreakpointTestData(this.bpId, this.function, this.script); +} diff --git a/dwds/test/location_test.dart b/dwds/test/location_test.dart new file mode 100644 index 000000000..c4de47d0b --- /dev/null +++ b/dwds/test/location_test.dart @@ -0,0 +1,259 @@ +// Copyright 2020 The Dart Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @dart = 2.9 + +import 'package:dwds/dwds.dart'; +import 'package:dwds/src/debugging/location.dart'; +import 'package:dwds/src/debugging/modules.dart'; +import 'package:dwds/src/loaders/strategy.dart'; +import 'package:dwds/src/utilities/dart_uri.dart'; +import 'package:shelf/shelf.dart' as shelf; +import 'package:test/test.dart'; + +import 'debugger_test.dart'; + +void main() { + const lines = 100; + const lineLength = 150; + + globalLoadStrategy = MockLoadStrategy(); + var dartUri = DartUri('org-dartlang://web/main.dart'); + + var assetReader = FakeAssetReader(); + var modules = MockModules(); + var locations = Locations(assetReader, modules, ''); + + group('JS locations |', () { + group('location |', () { + test('is zero based', () async { + var loc = JsLocation.fromZeroBased(_module, 0, 0); + expect(loc, _matchJsLocation(0, 0)); + }); + + test('can compare to other location', () async { + var loc00 = JsLocation.fromZeroBased(_module, 0, 0); + var loc01 = JsLocation.fromZeroBased(_module, 0, 1); + var loc10 = JsLocation.fromZeroBased(_module, 1, 0); + var loc11 = JsLocation.fromZeroBased(_module, 1, 1); + + expect(loc00.compareTo(loc01), isNegative); + expect(loc00.compareTo(loc10), isNegative); + expect(loc00.compareTo(loc10), isNegative); + expect(loc00.compareTo(loc11), isNegative); + + expect(loc00.compareTo(loc00), isZero); + expect(loc11.compareTo(loc00), isPositive); + }); + }); + + group('best location |', () { + test('does not return location for setup code', () async { + var location = await locations.locationForJs(_module, 0, 0); + expect(location, isNull); + }); + + test('prefers precise match', () async { + var location = await locations.locationForJs(_module, 37, 0); + expect(location, _matchLocationForJs(37, 0)); + }); + + test('finds a match in the beginning of the line', () async { + var location = await locations.locationForJs(_module, 39, 4); + expect(location, _matchLocationForJs(39, 0)); + }); + + test('finds a match in the middle of the line', () async { + var location = await locations.locationForJs(_module, 39, 10); + expect(location, _matchLocationForJs(39, 6)); + }); + + test('finds a match on a previous line', () async { + var location = await locations.locationForJs(_module, 44, 0); + expect(location, _matchLocationForJs(43, 18)); + }); + + test('finds a match on a previous line with a closer match after', + () async { + var location = + await locations.locationForJs(_module, 44, lineLength - 1); + expect(location, _matchLocationForJs(43, 18)); + }); + + test('finds a match on the last line', () async { + var location = + await locations.locationForJs(_module, lines - 1, lineLength - 1); + expect(location, _matchLocationForJs(50, 2)); + }); + + test('finds a match on invalid line', () async { + var location = + await locations.locationForJs(_module, lines, lineLength - 1); + expect(location, _matchLocationForJs(50, 2)); + }); + + test('finds a match on invalid column on the same line', () async { + var location = await locations.locationForJs(_module, 50, lineLength); + expect(location, _matchLocationForJs(50, 2)); + }); + + test('finds a match on invalid column on a previous line', () async { + var location = + await locations.locationForJs(_module, lines - 1, lineLength); + expect(location, _matchLocationForJs(50, 2)); + }); + }); + }); + + group('Dart locations |', () { + group('location |', () { + test('is one based', () async { + var loc = DartLocation.fromZeroBased(dartUri, 0, 0); + expect(loc, _matchDartLocation(1, 1)); + }); + + test('can compare to other locations', () async { + var loc00 = DartLocation.fromZeroBased(dartUri, 0, 0); + var loc01 = DartLocation.fromZeroBased(dartUri, 0, 1); + var loc10 = DartLocation.fromZeroBased(dartUri, 1, 0); + var loc11 = DartLocation.fromZeroBased(dartUri, 1, 1); + + expect(loc00.compareTo(loc01), isNegative); + expect(loc00.compareTo(loc10), isNegative); + expect(loc00.compareTo(loc10), isNegative); + expect(loc00.compareTo(loc11), isNegative); + + expect(loc00.compareTo(loc00), isZero); + expect(loc11.compareTo(loc00), isPositive); + }); + }); + + group('best location |', () { + test('does not return location for dart lines not mapped to JS', + () async { + var location = await locations.locationForDart(dartUri, 0, 0); + expect(location, isNull); + }); + + test('returns location after on the same line', () async { + var location = await locations.locationForDart(dartUri, 11, 0); + expect(location, _matchLocationForDart(11, 3)); + }); + + test('return null on invalid line', () async { + var location = await locations.locationForDart(dartUri, lines, 0); + expect(location, isNull); + }); + + test('return null on invalid column', () async { + var location = + await locations.locationForDart(dartUri, lines - 1, lineLength); + expect(location, isNull); + }); + }); + }); +} + +Matcher _matchLocationForDart(int line, int column) => isA().having( + (l) => l.dartLocation, 'dartLocation', _matchDartLocation(line, column)); + +Matcher _matchLocationForJs(int line, int column) => isA() + .having((l) => l.jsLocation, 'jsLocation', _matchJsLocation(line, column)); + +Matcher _matchDartLocation(int line, int column) => isA() + .having((l) => l.line, 'line', line) + .having((l) => l.column, 'column', column); + +Matcher _matchJsLocation(int line, int column) => isA() + .having((l) => l.line, 'line', line) + .having((l) => l.column, 'column', column); + +const _module = 'packages/module'; +const _serverPath = 'package/module.js'; +const _sourceMapPath = 'packages/module.js.map'; + +class MockLoadStrategy implements LoadStrategy { + @override + Future bootstrapFor(String entrypoint) async => 'dummy_bootstrap'; + + @override + shelf.Handler get handler => + (request) => (request.url.path == 'someDummyPath') + ? shelf.Response.ok('some dummy response') + : null; + + @override + String get id => 'dummy-id'; + + @override + String get moduleFormat => 'dummy-format'; + + @override + String get loadLibrariesModule => ''; + + @override + String get loadLibrariesSnippet => ''; + + @override + String loadLibrarySnippet(String libraryUri) => ''; + + @override + String get loadModuleSnippet => ''; + + @override + ReloadConfiguration get reloadConfiguration => ReloadConfiguration.none; + + @override + String loadClientSnippet(String clientScript) => 'dummy-load-client-snippet'; + + @override + Future moduleForServerPath( + String entrypoint, String serverPath) async => + _module; + + @override + Future serverPathForModule(String entrypoint, String module) async => + _serverPath; + + @override + Future sourceMapPathForModule( + String entrypoint, String module) async => + _sourceMapPath; + + @override + String serverPathForAppUri(String appUri) => _serverPath; + + @override + MetadataProvider metadataProviderFor(String entrypoint) => null; + + @override + void trackEntrypoint(String entrypoint) {} + + @override + Future> moduleInfoForEntrypoint(String entrypoint) => + throw UnimplementedError(); +} + +class MockModules implements Modules { + @override + void initialize(String entrypoint) {} + + @override + Future libraryForSource(String serverPath) { + throw UnimplementedError(); + } + + @override + Future moduleForSource(String serverPath) async => _module; + + @override + Future> modules() { + throw UnimplementedError(); + } + + @override + Future moduleForlibrary(String libraryUri) { + throw UnimplementedError(); + } +} diff --git a/dwds/test/skip_list_test.dart b/dwds/test/skip_list_test.dart index ea73411f1..5fc96cadd 100644 --- a/dwds/test/skip_list_test.dart +++ b/dwds/test/skip_list_test.dart @@ -29,6 +29,30 @@ void main() { _validateRange(skipList.last, 10, 21, maxValue, maxValue); }); + test('do not include start of the file', () async { + var skipList = await skipLists.compute('123', { + Location.from( + 'foo', TargetLineEntry(0, []), TargetEntry(0, 0, 0, 0), null), + Location.from( + 'foo', TargetLineEntry(10, []), TargetEntry(20, 0, 0, 0), null), + }); + expect(skipList.length, 2); + _validateRange(skipList[0], 0, 1, 10, 19); + _validateRange(skipList.last, 10, 21, maxValue, maxValue); + }); + + test('does not depend on order of locations', () async { + var skipList = await skipLists.compute('123', { + Location.from( + 'foo', TargetLineEntry(10, []), TargetEntry(20, 0, 0, 0), null), + Location.from( + 'foo', TargetLineEntry(0, []), TargetEntry(0, 0, 0, 0), null), + }); + expect(skipList.length, 2); + _validateRange(skipList[0], 0, 1, 10, 19); + _validateRange(skipList.last, 10, 21, maxValue, maxValue); + }); + test('contains the provided id', () async { var id = '123'; var skipList = await skipLists.compute(id, {}); diff --git a/fixtures/_testPackage/web/main.dart b/fixtures/_testPackage/web/main.dart index 56fe7511c..2ac7b802f 100644 --- a/fixtures/_testPackage/web/main.dart +++ b/fixtures/_testPackage/web/main.dart @@ -31,7 +31,7 @@ void main() { // for evaluation Timer.periodic(const Duration(seconds: 1), (_) { printLocal(); - printFieldFromLibraryClass(); + printFieldFromLibraryClass(); // Breakpoint: callPrintFieldFromLibraryClass printFieldFromLibraryPartClass(); printFieldMain(); printGlobal(); @@ -53,7 +53,7 @@ void printLocal() { } void printFieldFromLibraryClass() { - var instance = TestLibraryClass(1, 2); + var instance = TestLibraryClass(1, 2); // Breakpoint: createLibraryObject print('$instance'); // Breakpoint: printFieldFromLibraryClass } @@ -109,7 +109,7 @@ void printNestedObjectsMultiLine() { } void printObjectMultiLine() { - print(// Breakpoint: printMultiLine + print( // Breakpoint: printMultiLine createObject() // Breakpoint: printObjectMultiLine ..initialize(), ); diff --git a/fixtures/_testPackageSound/web/main.dart b/fixtures/_testPackageSound/web/main.dart index 1f19889e0..1552cb314 100644 --- a/fixtures/_testPackageSound/web/main.dart +++ b/fixtures/_testPackageSound/web/main.dart @@ -30,7 +30,7 @@ void main() async { Timer.periodic(const Duration(seconds: 1), (_) async { await asyncMethod(); printLocal(); - printFieldFromLibraryClass(); + printFieldFromLibraryClass(); // Breakpoint: callPrintFieldFromLibraryClass printFieldFromLibraryPartClass(); printFieldMain(); printGlobal(); @@ -62,7 +62,7 @@ void printLocal() { } void printFieldFromLibraryClass() { - var instance = TestLibraryClass(1, 2); + var instance = TestLibraryClass(1, 2); // Breakpoint: createLibraryObject print('$instance'); // Breakpoint: printFieldFromLibraryClass } @@ -118,7 +118,7 @@ void printNestedObjectsMultiLine() { } void printObjectMultiLine() { - print(// Breakpoint: printMultiLine + print( // Breakpoint: printMultiLine createObject() // Breakpoint: printObjectMultiLine ..initialize(), );