From 93030c2e94a1acd100e091af9bee65ab7d525f74 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:51:53 -0800 Subject: [PATCH 01/18] Wip --- dwds/lib/src/debugging/debugger.dart | 68 +++---------------- dwds/lib/src/debugging/location.dart | 16 ++++- dwds/lib/src/debugging/modules.dart | 12 +++- .../src/services/chrome_proxy_service.dart | 6 +- 4 files changed, 35 insertions(+), 67 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index b5af29092..41739bfd3 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -253,66 +253,7 @@ class Debugger extends Domain { return breakpoint; } - Future _updatedScriptRefFor(Breakpoint breakpoint) async { - final oldRef = (breakpoint.location as SourceLocation).script; - final uri = oldRef?.uri; - if (uri == null) return null; - final dartUri = DartUri(uri, _root); - return await inspector.scriptRefFor(dartUri.serverPath); - } - Future reestablishBreakpoints( - Set previousBreakpoints, - Set disabledBreakpoints, - ) async { - // Previous breakpoints were never removed from Chrome since we use - // `setBreakpointByUrl`. We simply need to update the references. - for (var breakpoint in previousBreakpoints) { - final dartBpId = breakpoint.id!; - final scriptRef = await _updatedScriptRefFor(breakpoint); - final scriptUri = scriptRef?.uri; - if (scriptRef != null && scriptUri != null) { - final jsBpId = _breakpoints.jsIdFor(dartBpId)!; - final updatedLocation = await _locations.locationForDart( - DartUri(scriptUri, _root), - _lineNumberFor(breakpoint), - _columnNumberFor(breakpoint), - ); - if (updatedLocation != null) { - final updatedBreakpoint = _breakpoints._dartBreakpoint( - scriptRef, - updatedLocation, - dartBpId, - ); - _breakpoints._note(bp: updatedBreakpoint, jsId: jsBpId); - _notifyBreakpoint(updatedBreakpoint); - } else { - logger.warning('Cannot update breakpoint $dartBpId:' - ' cannot update location.'); - } - } else { - logger.warning('Cannot update breakpoint $dartBpId:' - ' cannot find script ref.'); - } - } - - // Disabled breakpoints were actually removed from Chrome so simply add - // them back. - for (var breakpoint in disabledBreakpoints) { - final scriptRef = await _updatedScriptRefFor(breakpoint); - final scriptId = scriptRef?.id; - if (scriptId != null) { - await addBreakpoint( - scriptId, - _lineNumberFor(breakpoint), - column: _columnNumberFor(breakpoint), - ); - } else { - logger.warning('Cannot update disabled breakpoint ${breakpoint.id}:' - ' cannot find script ref.'); - } - } - } void _notifyBreakpoint(Breakpoint breakpoint) { final event = Event( @@ -858,6 +799,15 @@ class _Breakpoints extends Domain { // Prevent `Aww, snap!` errors when setting multiple breakpoints // simultaneously by serializing the requests. return _queue.run(() async { + + // final response = + // await remoteDebugger.sendCommand('Debugger.setBreakpoint', params: { + // 'location': { + // 'scriptId': location.jsLocation.scriptId, + // 'lineNumber': location.jsLocation.line - 1, + // } + + final breakPointId = await sendCommandAndValidateResult( remoteDebugger, method: 'Debugger.setBreakpointByUrl', diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 161573c6e..0b17317b9 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -29,6 +29,7 @@ class Location { ) : tokenPos = _startTokenId++; static Location from( + String scriptId, String module, TargetLineEntry lineEntry, TargetEntry entry, @@ -42,7 +43,7 @@ class Location { // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( - JsLocation.fromZeroBased(module, jsLine, jsColumn), + JsLocation.fromZeroBased(scriptId, module, jsLine, jsColumn), DartLocation.fromZeroBased(dartUri, dartLine ?? 0, dartColumn ?? 0), ); } @@ -96,6 +97,9 @@ class DartLocation { /// Location information for a JS source. class JsLocation { + /// The script ID as provided by Chrome. + final String scriptId; + final String module; /// 0 based row offset within the JS source code. @@ -105,6 +109,7 @@ class JsLocation { final int column; JsLocation._( + this.scriptId, this.module, this.line, this.column, @@ -122,8 +127,13 @@ class JsLocation { // 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); + factory JsLocation.fromZeroBased( + String scriptId, + String module, + int line, + int column, + ) => + JsLocation._(scriptId, module, line, column); } /// Contains meta data for known [Location]s. diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index 37b857af1..54855d5eb 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -21,6 +21,10 @@ class Modules { final Map _libraryToModule = {}; + // The Chrome script ID to corresponding module. + final _scriptIdToModule = {}; + + late String _entrypoint; Modules(this._root); @@ -39,6 +43,12 @@ class Modules { _entrypoint = entrypoint; } + /// Returns the module for the Chrome script ID. + String moduleForScriptId(String scriptId) => _scriptIdToModule[scriptId]; + + /// Returns the Chrome script ID for the provided module. + String scriptIdForModule(String module) => _moduleToScriptId[module]; + /// Returns the containing module for the provided Dart server path. Future moduleForSource(String serverPath) async { await _moduleMemoizer.runOnce(_initializeMapping); @@ -80,7 +90,7 @@ class Modules { final module = scriptToModule[library]!; _sourceToModule[libraryServerPath] = module; - _sourceToLibrary[libraryServerPath] = Uri.parse(library); + _sourceToLibrary[libraryServerPath] _moduleToScriptId[module] = scriptId; = Uri.parse(library); _libraryToModule[library] = module; for (var script in scripts) { diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 33b12ea11..9d0cc8f0e 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -287,10 +287,6 @@ class ChromeProxyService implements VmServiceInterface { safeUnawaited(_prewarmExpressionCompilerCache()); - await debugger.reestablishBreakpoints( - _previousBreakpoints, - _disabledBreakpoints, - ); _disabledBreakpoints.clear(); safeUnawaited( @@ -393,6 +389,7 @@ class ChromeProxyService implements VmServiceInterface { int line, { int? column, }) { + print('ADD BREAKPOINT: $line'); return wrapInErrorHandlerAsync( 'addBreakpoint', () => _addBreakpoint(isolateId, scriptId, line), @@ -438,6 +435,7 @@ class ChromeProxyService implements VmServiceInterface { int line, { int? column, }) async { + print('add breakpoint with script uri $line'); await isInitialized; _checkIsolate('addBreakpointWithScriptUri', isolateId); if (Uri.parse(scriptUri).scheme == 'dart') { From 77cffd45a314a880773c5d16f713e426a2be9354 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:53:56 -0800 Subject: [PATCH 02/18] Forgot to save --- dwds/lib/src/debugging/modules.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index 54855d5eb..debacebe3 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -90,7 +90,7 @@ class Modules { final module = scriptToModule[library]!; _sourceToModule[libraryServerPath] = module; - _sourceToLibrary[libraryServerPath] _moduleToScriptId[module] = scriptId; = Uri.parse(library); + _sourceToLibrary[libraryServerPath] = Uri.parse(library); _libraryToModule[library] = module; for (var script in scripts) { From ad6ebd5fc456edbb4afa478a1c6000409c5eb758 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 31 Jan 2024 11:23:03 -0800 Subject: [PATCH 03/18] Switch from setBreakpointByUrl to setBreakpoint --- dwds/lib/src/debugging/debugger.dart | 19 ++++--------------- dwds/lib/src/debugging/location.dart | 10 ++-------- dwds/lib/src/debugging/modules.dart | 6 ------ dwds/lib/src/servers/extension_debugger.dart | 1 + 4 files changed, 7 insertions(+), 29 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 41739bfd3..262efa93d 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -742,7 +742,7 @@ class _Breakpoints extends Domain { try { final dartBreakpoint = _dartBreakpoint(dartScript!, location, id); - final jsBreakpointId = await _setJsBreakpoint(location); + final jsBreakpointId = await _setJsBreakpoint(location, scriptId); if (jsBreakpointId == null) { _logger.fine('Failed to set breakpoint $id ' '($scriptId:$line:$column): ' @@ -793,27 +793,16 @@ class _Breakpoints extends Domain { } /// Calls the Chrome protocol setBreakpoint and returns the remote ID. - Future _setJsBreakpoint(Location location) { - // The module can be loaded from a nested path and contain an ETAG suffix. - final urlRegex = '.*${location.jsLocation.module}.*'; + Future _setJsBreakpoint(Location location, String scriptId) { // Prevent `Aww, snap!` errors when setting multiple breakpoints // simultaneously by serializing the requests. return _queue.run(() async { - - // final response = - // await remoteDebugger.sendCommand('Debugger.setBreakpoint', params: { - // 'location': { - // 'scriptId': location.jsLocation.scriptId, - // 'lineNumber': location.jsLocation.line - 1, - // } - - final breakPointId = await sendCommandAndValidateResult( remoteDebugger, - method: 'Debugger.setBreakpointByUrl', + method: 'Debugger.setBreakpoint', resultField: 'breakpointId', params: { - 'urlRegex': urlRegex, + 'scriptId': scriptId, 'lineNumber': location.jsLocation.line, 'columnNumber': location.jsLocation.column, }, diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 0b17317b9..a56037d12 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -29,7 +29,6 @@ class Location { ) : tokenPos = _startTokenId++; static Location from( - String scriptId, String module, TargetLineEntry lineEntry, TargetEntry entry, @@ -43,7 +42,7 @@ class Location { // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( - JsLocation.fromZeroBased(scriptId, module, jsLine, jsColumn), + JsLocation.fromZeroBased(module, jsLine, jsColumn), DartLocation.fromZeroBased(dartUri, dartLine ?? 0, dartColumn ?? 0), ); } @@ -97,9 +96,6 @@ class DartLocation { /// Location information for a JS source. class JsLocation { - /// The script ID as provided by Chrome. - final String scriptId; - final String module; /// 0 based row offset within the JS source code. @@ -109,7 +105,6 @@ class JsLocation { final int column; JsLocation._( - this.scriptId, this.module, this.line, this.column, @@ -128,12 +123,11 @@ class JsLocation { // JS Location is 0 based according to: // https://chromedevtools.github.io/devtools-protocol/tot/Debugger#type-Location factory JsLocation.fromZeroBased( - String scriptId, String module, int line, int column, ) => - JsLocation._(scriptId, module, line, column); + JsLocation._(module, line, column); } /// Contains meta data for known [Location]s. diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index debacebe3..d67158c75 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -43,12 +43,6 @@ class Modules { _entrypoint = entrypoint; } - /// Returns the module for the Chrome script ID. - String moduleForScriptId(String scriptId) => _scriptIdToModule[scriptId]; - - /// Returns the Chrome script ID for the provided module. - String scriptIdForModule(String module) => _moduleToScriptId[module]; - /// Returns the containing module for the provided Dart server path. Future moduleForSource(String serverPath) async { await _moduleMemoizer.runOnce(_initializeMapping); diff --git a/dwds/lib/src/servers/extension_debugger.dart b/dwds/lib/src/servers/extension_debugger.dart index 6c0736198..17f7db2fa 100644 --- a/dwds/lib/src/servers/extension_debugger.dart +++ b/dwds/lib/src/servers/extension_debugger.dart @@ -131,6 +131,7 @@ class ExtensionDebugger implements RemoteDebugger { }); // Listens for a page reload. onGlobalObjectCleared.listen((_) { + // Note: Can listen for page-reload here. _scripts.clear(); }); } From a060accea3d3e718f625fa5b797e82a1c8aec51b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 31 Jan 2024 13:33:21 -0800 Subject: [PATCH 04/18] Add some logging --- dwds/lib/src/services/chrome_proxy_service.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 9d0cc8f0e..fb0142a33 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -314,6 +314,7 @@ class ChromeProxyService implements VmServiceInterface { isolate: isolateRef, ), ); + print('==== NOTIFY kIsolateRunnable!!!!'); _streamNotify( 'Isolate', Event( From 5e7d792c72cc9432fd864739db426033fd479df9 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 1 Feb 2024 10:03:12 -0800 Subject: [PATCH 05/18] Wip --- dwds/lib/src/debugging/debugger.dart | 19 ++++++++++++++++++- dwds/lib/src/debugging/location.dart | 14 ++++++++++++-- dwds/lib/src/debugging/modules.dart | 22 ++++++++++++++++++++-- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 262efa93d..6fdf5869d 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -207,6 +207,7 @@ class Debugger extends Domain { // miss events. // Allow a null debugger/connection for unit tests. runZonedGuarded(() { + _remoteDebugger.onScriptParsed.listen(_scriptParsedHandler); _remoteDebugger.onPaused.listen(_pauseHandler); _remoteDebugger.onResumed.listen(_resumeHandler); _remoteDebugger.onTargetCrashed.listen(_crashHandler); @@ -461,6 +462,11 @@ class Debugger extends Domain { return dartFrame; } + void _scriptParsedHandler(ScriptParsedEvent e) { + final script = e.script; + print('=== GOT SCRIPT: ${script.scriptId} -> ${script.url}'); + } + /// Handles pause events coming from the Chrome connection. Future _pauseHandler(DebuggerPausedEvent e) async { final isolate = inspector.isolate; @@ -796,18 +802,29 @@ class _Breakpoints extends Domain { Future _setJsBreakpoint(Location location, String scriptId) { // Prevent `Aww, snap!` errors when setting multiple breakpoints // simultaneously by serializing the requests. + + // The module can be loaded from a nested path and contain an ETAG suffix. + final urlRegex = '.*${location.jsLocation.module}.*'; + return _queue.run(() async { + final chromeScriptId = location.jsLocation.chromeScriptId; + if (chromeScriptId != null) { final breakPointId = await sendCommandAndValidateResult( remoteDebugger, method: 'Debugger.setBreakpoint', resultField: 'breakpointId', params: { - 'scriptId': scriptId, + 'scriptId': location.jsLocation.chromeScriptId, 'lineNumber': location.jsLocation.line, 'columnNumber': location.jsLocation.column, }, ); return breakPointId; + } else { + print('CHROME SCRIPT ID IS NULL FOR $scriptId'); + return null; + } + }); } diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index a56037d12..d2cbc8f47 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -33,6 +33,7 @@ class Location { TargetLineEntry lineEntry, TargetEntry entry, DartUri dartUri, + String? chromeScriptId, ) { final dartLine = entry.sourceLine; final dartColumn = entry.sourceColumn; @@ -42,7 +43,7 @@ class Location { // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( - JsLocation.fromZeroBased(module, jsLine, jsColumn), + JsLocation.fromZeroBased(module, jsLine, jsColumn, chromeScriptId), DartLocation.fromZeroBased(dartUri, dartLine ?? 0, dartColumn ?? 0), ); } @@ -104,10 +105,13 @@ class JsLocation { /// 0 based column offset within the JS source code. final int column; + String? chromeScriptId; + JsLocation._( this.module, this.line, this.column, + this.chromeScriptId, ); int compareTo(JsLocation other) => compareToLine(other.line, other.column); @@ -126,8 +130,9 @@ class JsLocation { String module, int line, int column, + String? chromeScriptId, ) => - JsLocation._(module, line, column); + JsLocation._(module, line, column, chromeScriptId); } /// Contains meta data for known [Location]s. @@ -315,6 +320,9 @@ class Locations { } final sourceMapPath = await globalToolConfiguration.loadStrategy .sourceMapPathForModule(_entrypoint, module); + final chromeScriptId = + await _modules.getScriptIdForModule(_entrypoint, module); + print('=== CHROME SCRIPT ID IS $chromeScriptId'); if (sourceMapPath == null) { _logger.warning('No sourceMap path for module: $module'); return result; @@ -343,12 +351,14 @@ class Locations { ); final dartUri = DartUri(path, _root); + result.add( Location.from( modulePath, lineEntry, entry, dartUri, + chromeScriptId, ), ); } diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index d67158c75..4230a8e67 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -21,9 +21,11 @@ class Modules { final Map _libraryToModule = {}; - // The Chrome script ID to corresponding module. - final _scriptIdToModule = {}; + // The Chrome script ID to corresponding url. + final _scriptIdToScriptUrl = {}; + // The Chrome script url to corresponding Chrome script ID. + final _scriptUrlToScriptId = {}; late String _entrypoint; @@ -66,6 +68,22 @@ class Modules { return _sourceToModule; } + void saveScriptId( + String scriptId, { + required String scriptUrl, + }) { + _scriptIdToScriptUrl[scriptUrl] = scriptId; + _scriptUrlToScriptId[scriptId] = scriptUrl; + } + + Future getScriptIdForModule(String entrypoint, String module) async { + final serverPath = await globalToolConfiguration.loadStrategy + .serverPathForModule(entrypoint, module); + final scriptId = _scriptUrlToScriptId[serverPath]; + print('found $scriptId for $module'); + return scriptId; + } + /// Initializes [_sourceToModule] and [_sourceToLibrary]. Future _initializeMapping() async { final provider = From 9ce892fc9202cc80a334300bc9357798834d4c0f Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:02:03 -0800 Subject: [PATCH 06/18] Setting breakpoints works --- dwds/lib/src/debugging/debugger.dart | 47 ++++++++++++++++++---------- dwds/lib/src/debugging/modules.dart | 18 ++--------- dwds/lib/src/shared/globals.dart | 14 +++++++++ 3 files changed, 48 insertions(+), 31 deletions(-) create mode 100644 dwds/lib/src/shared/globals.dart diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 6fdf5869d..a13109cd7 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -21,6 +21,7 @@ import 'package:logging/logging.dart'; import 'package:vm_service/vm_service.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' hide StackTrace; +import 'package:path/path.dart' as p; /// Adds [event] to the stream with [streamId] if there is anybody listening /// on that stream. @@ -36,6 +37,9 @@ const _pauseModePauseStates = { 'unhandled': PauseState.uncaught, }; +final chromeScriptIdToUrl = {}; +final chromeScriptUrlToId = {}; + class Debugger extends Domain { static final logger = Logger('Debugger'); @@ -254,8 +258,6 @@ class Debugger extends Domain { return breakpoint; } - - void _notifyBreakpoint(Breakpoint breakpoint) { final event = Event( kind: EventKind.kBreakpointAdded, @@ -464,7 +466,18 @@ class Debugger extends Domain { void _scriptParsedHandler(ScriptParsedEvent e) { final script = e.script; - print('=== GOT SCRIPT: ${script.scriptId} -> ${script.url}'); + final scriptId = script.scriptId; + final scriptPath = p.joinAll(Uri.parse(script.url).pathSegments); + print('received script parsed event: $scriptPath'); + _saveScriptId(scriptId, scriptUrl: scriptPath); + } + + void _saveScriptId( + String scriptId, { + required String scriptUrl, + }) { + chromeScriptIdToUrl[scriptId] = scriptUrl; + chromeScriptUrlToId[scriptUrl] = scriptId; } /// Handles pause events coming from the Chrome connection. @@ -805,26 +818,28 @@ class _Breakpoints extends Domain { // The module can be loaded from a nested path and contain an ETAG suffix. final urlRegex = '.*${location.jsLocation.module}.*'; - + return _queue.run(() async { final chromeScriptId = location.jsLocation.chromeScriptId; + print('requesting breakpoint with id $chromeScriptId'); if (chromeScriptId != null) { - final breakPointId = await sendCommandAndValidateResult( - remoteDebugger, - method: 'Debugger.setBreakpoint', - resultField: 'breakpointId', - params: { - 'scriptId': location.jsLocation.chromeScriptId, - 'lineNumber': location.jsLocation.line, - 'columnNumber': location.jsLocation.column, - }, - ); - return breakPointId; + final breakPointId = await sendCommandAndValidateResult( + remoteDebugger, + method: 'Debugger.setBreakpoint', + resultField: 'breakpointId', + params: { + 'location': { + 'lineNumber': location.jsLocation.line, + 'columnNumber': location.jsLocation.column, + 'scriptId': location.jsLocation.chromeScriptId, + }, + }, + ); + return breakPointId; } else { print('CHROME SCRIPT ID IS NULL FOR $scriptId'); return null; } - }); } diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index 4230a8e67..754293598 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -6,6 +6,7 @@ import 'package:async/async.dart'; import 'package:dwds/src/config/tool_configuration.dart'; import 'package:dwds/src/utilities/dart_uri.dart'; import 'package:logging/logging.dart'; +import 'package:dwds/src/debugging/debugger.dart'; /// Tracks modules for the compiled application. class Modules { @@ -21,12 +22,6 @@ class Modules { final Map _libraryToModule = {}; - // The Chrome script ID to corresponding url. - final _scriptIdToScriptUrl = {}; - - // The Chrome script url to corresponding Chrome script ID. - final _scriptUrlToScriptId = {}; - late String _entrypoint; Modules(this._root); @@ -68,18 +63,11 @@ class Modules { return _sourceToModule; } - void saveScriptId( - String scriptId, { - required String scriptUrl, - }) { - _scriptIdToScriptUrl[scriptUrl] = scriptId; - _scriptUrlToScriptId[scriptId] = scriptUrl; - } - Future getScriptIdForModule(String entrypoint, String module) async { final serverPath = await globalToolConfiguration.loadStrategy .serverPathForModule(entrypoint, module); - final scriptId = _scriptUrlToScriptId[serverPath]; + print('looking for server path $serverPath'); + final scriptId = chromeScriptUrlToId[serverPath]; print('found $scriptId for $module'); return scriptId; } diff --git a/dwds/lib/src/shared/globals.dart b/dwds/lib/src/shared/globals.dart new file mode 100644 index 000000000..cef0ca0b3 --- /dev/null +++ b/dwds/lib/src/shared/globals.dart @@ -0,0 +1,14 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:async/async.dart'; +import 'package:dwds/src/config/tool_configuration.dart'; +import 'package:dwds/src/utilities/dart_uri.dart'; +import 'package:logging/logging.dart'; + +/// The tool configuration for the connected app. +/// +/// TODO(elliette): Consider making this final (would require updating tests +/// that currently depend on changing the configuration between test cases). +{} globalChromeScriptIdToUrl; \ No newline at end of file From d11d41ccfe38a3ab05471de232fe057bda747d5d Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 5 Feb 2024 13:33:02 -0800 Subject: [PATCH 07/18] Clean up --- dwds/lib/src/debugging/debugger.dart | 46 ++++++++----------- dwds/lib/src/debugging/location.dart | 24 ++++++---- dwds/lib/src/debugging/modules.dart | 10 ++-- .../src/services/chrome_proxy_service.dart | 3 -- dwds/lib/src/shared/globals.dart | 14 ------ 5 files changed, 37 insertions(+), 60 deletions(-) delete mode 100644 dwds/lib/src/shared/globals.dart diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index a13109cd7..57b8b4960 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -37,8 +37,13 @@ const _pauseModePauseStates = { 'unhandled': PauseState.uncaught, }; -final chromeScriptIdToUrl = {}; -final chromeScriptUrlToId = {}; +/// Mapping from the path of a script in Chrome to the Runtime.ScriptId Chrome +/// uses to reference it. +/// +/// See https://chromedevtools.github.io/devtools-protocol/tot/Runtime/#type-ScriptId +/// +/// e.g. 'packages/myapp/main.dart.lib.js' -> '12' +final chromePathToRuntimeScriptId = {}; class Debugger extends Domain { static final logger = Logger('Debugger'); @@ -465,19 +470,10 @@ class Debugger extends Domain { } void _scriptParsedHandler(ScriptParsedEvent e) { - final script = e.script; - final scriptId = script.scriptId; - final scriptPath = p.joinAll(Uri.parse(script.url).pathSegments); - print('received script parsed event: $scriptPath'); - _saveScriptId(scriptId, scriptUrl: scriptPath); - } - - void _saveScriptId( - String scriptId, { - required String scriptUrl, - }) { - chromeScriptIdToUrl[scriptId] = scriptUrl; - chromeScriptUrlToId[scriptUrl] = scriptId; + final scriptPath = p.joinAll(Uri.parse(e.script.url).pathSegments); + if (scriptPath.isNotEmpty) { + chromePathToRuntimeScriptId[scriptPath] = e.script.scriptId; + } } /// Handles pause events coming from the Chrome connection. @@ -761,7 +757,7 @@ class _Breakpoints extends Domain { try { final dartBreakpoint = _dartBreakpoint(dartScript!, location, id); - final jsBreakpointId = await _setJsBreakpoint(location, scriptId); + final jsBreakpointId = await _setJsBreakpoint(location); if (jsBreakpointId == null) { _logger.fine('Failed to set breakpoint $id ' '($scriptId:$line:$column): ' @@ -812,18 +808,13 @@ class _Breakpoints extends Domain { } /// Calls the Chrome protocol setBreakpoint and returns the remote ID. - Future _setJsBreakpoint(Location location, String scriptId) { + Future _setJsBreakpoint(Location location) { // Prevent `Aww, snap!` errors when setting multiple breakpoints // simultaneously by serializing the requests. - - // The module can be loaded from a nested path and contain an ETAG suffix. - final urlRegex = '.*${location.jsLocation.module}.*'; - return _queue.run(() async { - final chromeScriptId = location.jsLocation.chromeScriptId; - print('requesting breakpoint with id $chromeScriptId'); - if (chromeScriptId != null) { - final breakPointId = await sendCommandAndValidateResult( + final scriptId = location.jsLocation.runtimeScriptId; + if (scriptId != null) { + return sendCommandAndValidateResult( remoteDebugger, method: 'Debugger.setBreakpoint', resultField: 'breakpointId', @@ -831,13 +822,12 @@ class _Breakpoints extends Domain { 'location': { 'lineNumber': location.jsLocation.line, 'columnNumber': location.jsLocation.column, - 'scriptId': location.jsLocation.chromeScriptId, + 'scriptId': scriptId, }, }, ); - return breakPointId; } else { - print('CHROME SCRIPT ID IS NULL FOR $scriptId'); + _logger.fine('No runtime script ID for location $location'); return null; } }); diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index d2cbc8f47..c055dee69 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -33,7 +33,7 @@ class Location { TargetLineEntry lineEntry, TargetEntry entry, DartUri dartUri, - String? chromeScriptId, + String? runtimeScriptId, ) { final dartLine = entry.sourceLine; final dartColumn = entry.sourceColumn; @@ -43,7 +43,7 @@ class Location { // lineEntry data is 0 based according to: // https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k return Location._( - JsLocation.fromZeroBased(module, jsLine, jsColumn, chromeScriptId), + JsLocation.fromZeroBased(module, jsLine, jsColumn, runtimeScriptId), DartLocation.fromZeroBased(dartUri, dartLine ?? 0, dartColumn ?? 0), ); } @@ -105,13 +105,16 @@ class JsLocation { /// 0 based column offset within the JS source code. final int column; - String? chromeScriptId; + /// The Runtime.ScriptId of a script in Chrome. + /// + /// See https://chromedevtools.github.io/devtools-protocol/tot/Runtime/#type-ScriptId + String? runtimeScriptId; JsLocation._( this.module, this.line, this.column, - this.chromeScriptId, + this.runtimeScriptId, ); int compareTo(JsLocation other) => compareToLine(other.line, other.column); @@ -130,9 +133,9 @@ class JsLocation { String module, int line, int column, - String? chromeScriptId, + String? runtimeScriptId, ) => - JsLocation._(module, line, column, chromeScriptId); + JsLocation._(module, line, column, runtimeScriptId); } /// Contains meta data for known [Location]s. @@ -320,9 +323,6 @@ class Locations { } final sourceMapPath = await globalToolConfiguration.loadStrategy .sourceMapPathForModule(_entrypoint, module); - final chromeScriptId = - await _modules.getScriptIdForModule(_entrypoint, module); - print('=== CHROME SCRIPT ID IS $chromeScriptId'); if (sourceMapPath == null) { _logger.warning('No sourceMap path for module: $module'); return result; @@ -333,6 +333,10 @@ class Locations { p.url.dirname('/${stripLeadingSlashes(modulePath)}'); if (sourceMapContents == null) return result; + + final runtimeScriptId = + await _modules.getRuntimeScriptIdForModule(_entrypoint, module); + // This happens to be a [SingleMapping] today in DDC. final mapping = parse(sourceMapContents); if (mapping is SingleMapping) { @@ -358,7 +362,7 @@ class Locations { lineEntry, entry, dartUri, - chromeScriptId, + runtimeScriptId, ), ); } diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index 754293598..649c32be3 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -63,13 +63,13 @@ class Modules { return _sourceToModule; } - Future getScriptIdForModule(String entrypoint, String module) async { + Future getRuntimeScriptIdForModule( + String entrypoint, + String module, + ) async { final serverPath = await globalToolConfiguration.loadStrategy .serverPathForModule(entrypoint, module); - print('looking for server path $serverPath'); - final scriptId = chromeScriptUrlToId[serverPath]; - print('found $scriptId for $module'); - return scriptId; + return chromePathToRuntimeScriptId[serverPath]; } /// Initializes [_sourceToModule] and [_sourceToLibrary]. diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index fb0142a33..682965c85 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -314,7 +314,6 @@ class ChromeProxyService implements VmServiceInterface { isolate: isolateRef, ), ); - print('==== NOTIFY kIsolateRunnable!!!!'); _streamNotify( 'Isolate', Event( @@ -390,7 +389,6 @@ class ChromeProxyService implements VmServiceInterface { int line, { int? column, }) { - print('ADD BREAKPOINT: $line'); return wrapInErrorHandlerAsync( 'addBreakpoint', () => _addBreakpoint(isolateId, scriptId, line), @@ -436,7 +434,6 @@ class ChromeProxyService implements VmServiceInterface { int line, { int? column, }) async { - print('add breakpoint with script uri $line'); await isInitialized; _checkIsolate('addBreakpointWithScriptUri', isolateId); if (Uri.parse(scriptUri).scheme == 'dart') { diff --git a/dwds/lib/src/shared/globals.dart b/dwds/lib/src/shared/globals.dart deleted file mode 100644 index cef0ca0b3..000000000 --- a/dwds/lib/src/shared/globals.dart +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'package:async/async.dart'; -import 'package:dwds/src/config/tool_configuration.dart'; -import 'package:dwds/src/utilities/dart_uri.dart'; -import 'package:logging/logging.dart'; - -/// The tool configuration for the connected app. -/// -/// TODO(elliette): Consider making this final (would require updating tests -/// that currently depend on changing the configuration between test cases). -{} globalChromeScriptIdToUrl; \ No newline at end of file From 8f84138d70ebbed5220062d9469c16a3df3d8c84 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 5 Feb 2024 13:35:15 -0800 Subject: [PATCH 08/18] More clean up --- dwds/lib/src/servers/extension_debugger.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/dwds/lib/src/servers/extension_debugger.dart b/dwds/lib/src/servers/extension_debugger.dart index 17f7db2fa..6c0736198 100644 --- a/dwds/lib/src/servers/extension_debugger.dart +++ b/dwds/lib/src/servers/extension_debugger.dart @@ -131,7 +131,6 @@ class ExtensionDebugger implements RemoteDebugger { }); // Listens for a page reload. onGlobalObjectCleared.listen((_) { - // Note: Can listen for page-reload here. _scripts.clear(); }); } From ef2fcacf02918a6fe01cee7316c9c43fe934ca8b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 5 Feb 2024 14:16:45 -0800 Subject: [PATCH 09/18] Resolve analyzer errors --- dwds/lib/src/debugging/debugger.dart | 7 +++---- dwds/lib/src/debugging/modules.dart | 2 +- dwds/test/fixtures/fakes.dart | 7 +++++++ dwds/test/location_test.dart | 21 ++++++++++++++------- dwds/test/skip_list_test.dart | 17 ++++++++++++----- 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 57b8b4960..21444eaeb 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -18,10 +18,10 @@ import 'package:dwds/src/utilities/server.dart'; import 'package:dwds/src/utilities/shared.dart'; import 'package:dwds/src/utilities/synchronized.dart'; import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; import 'package:vm_service/vm_service.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' hide StackTrace; -import 'package:path/path.dart' as p; /// Adds [event] to the stream with [streamId] if there is anybody listening /// on that stream. @@ -53,18 +53,17 @@ class Debugger extends Domain { final StreamNotify _streamNotify; final Locations _locations; final SkipLists _skipLists; - final String _root; Debugger._( this._remoteDebugger, this._streamNotify, this._locations, this._skipLists, - this._root, + root, ) : _breakpoints = _Breakpoints( locations: _locations, remoteDebugger: _remoteDebugger, - root: _root, + root: root, ); /// The breakpoints we have set so far, indexable by either diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index 649c32be3..5ec0afdc3 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -4,9 +4,9 @@ import 'package:async/async.dart'; import 'package:dwds/src/config/tool_configuration.dart'; +import 'package:dwds/src/debugging/debugger.dart'; import 'package:dwds/src/utilities/dart_uri.dart'; import 'package:logging/logging.dart'; -import 'package:dwds/src/debugging/debugger.dart'; /// Tracks modules for the compiled application. class Modules { diff --git a/dwds/test/fixtures/fakes.dart b/dwds/test/fixtures/fakes.dart index a24381d7a..50e5eba81 100644 --- a/dwds/test/fixtures/fakes.dart +++ b/dwds/test/fixtures/fakes.dart @@ -169,6 +169,13 @@ class FakeModules implements Modules { @override Future moduleForLibrary(String libraryUri) async => _module; + + @override + Future getRuntimeScriptIdForModule( + String entrypoint, + String module, + ) async => + null; } class FakeWebkitDebugger implements WebkitDebugger { diff --git a/dwds/test/location_test.dart b/dwds/test/location_test.dart index bfa0ecfda..28cf8323f 100644 --- a/dwds/test/location_test.dart +++ b/dwds/test/location_test.dart @@ -36,17 +36,24 @@ void main() { locations.initialize('fake_entrypoint'); group('JS locations |', () { + const fakeRuntimeScriptId = '12'; + group('location |', () { - test('is zero based', () async { - final loc = JsLocation.fromZeroBased(_module, 0, 0); + test('is zero based', () { + final loc = + JsLocation.fromZeroBased(_module, 0, 0, fakeRuntimeScriptId); expect(loc, _matchJsLocation(0, 0)); }); - test('can compare to other location', () async { - final loc00 = JsLocation.fromZeroBased(_module, 0, 0); - final loc01 = JsLocation.fromZeroBased(_module, 0, 1); - final loc10 = JsLocation.fromZeroBased(_module, 1, 0); - final loc11 = JsLocation.fromZeroBased(_module, 1, 1); + test('can compare to other location', () { + final loc00 = + JsLocation.fromZeroBased(_module, 0, 0, fakeRuntimeScriptId); + final loc01 = + JsLocation.fromZeroBased(_module, 0, 1, fakeRuntimeScriptId); + final loc10 = + JsLocation.fromZeroBased(_module, 1, 0, fakeRuntimeScriptId); + final loc11 = + JsLocation.fromZeroBased(_module, 1, 1, fakeRuntimeScriptId); expect(loc00.compareTo(loc01), isNegative); expect(loc00.compareTo(loc10), isNegative); diff --git a/dwds/test/skip_list_test.dart b/dwds/test/skip_list_test.dart index f4bb01b12..99f09e1d8 100644 --- a/dwds/test/skip_list_test.dart +++ b/dwds/test/skip_list_test.dart @@ -16,24 +16,27 @@ void main() { setGlobalsForTesting(); late SkipLists skipLists; final dartUri = DartUri('org-dartlang-app://web/main.dart'); + const fakeRuntimeScriptId = '12'; group('SkipLists', () { setUp(() { skipLists = SkipLists(); }); - test('do not include known ranges', () async { + test('do not include known ranges', () { final skipList = skipLists.compute('123', { Location.from( 'foo', TargetLineEntry(1, []), TargetEntry(2, 0, 0, 0), dartUri, + fakeRuntimeScriptId, ), Location.from( 'foo', TargetLineEntry(10, []), TargetEntry(20, 0, 0, 0), dartUri, + fakeRuntimeScriptId, ), }); expect(skipList.length, 3); @@ -42,19 +45,21 @@ void main() { _validateRange(skipList.last, 10, 21, maxValue, maxValue); }); - test('do not include start of the file', () async { + test('do not include start of the file', () { final skipList = skipLists.compute('123', { Location.from( 'foo', TargetLineEntry(0, []), TargetEntry(0, 0, 0, 0), dartUri, + fakeRuntimeScriptId, ), Location.from( 'foo', TargetLineEntry(10, []), TargetEntry(20, 0, 0, 0), dartUri, + fakeRuntimeScriptId, ), }); expect(skipList.length, 2); @@ -62,19 +67,21 @@ void main() { _validateRange(skipList.last, 10, 21, maxValue, maxValue); }); - test('does not depend on order of locations', () async { + test('does not depend on order of locations', () { final skipList = skipLists.compute('123', { Location.from( 'foo', TargetLineEntry(10, []), TargetEntry(20, 0, 0, 0), dartUri, + fakeRuntimeScriptId, ), Location.from( 'foo', TargetLineEntry(0, []), TargetEntry(0, 0, 0, 0), dartUri, + fakeRuntimeScriptId, ), }); expect(skipList.length, 2); @@ -82,7 +89,7 @@ void main() { _validateRange(skipList.last, 10, 21, maxValue, maxValue); }); - test('contains the provided id', () async { + test('contains the provided id', () { final id = '123'; final skipList = skipLists.compute(id, {}); for (var range in skipList) { @@ -90,7 +97,7 @@ void main() { } }); - test('ignores the whole file if provided no locations', () async { + test('ignores the whole file if provided no locations', () { final skipList = skipLists.compute('123', {}); expect(skipList.length, 1); _validateRange(skipList.first, 0, 0, maxValue, maxValue); From 9c33253b2adf5a33301482b7b3f7955691975548 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 5 Feb 2024 15:09:33 -0800 Subject: [PATCH 10/18] Make this work for g3 directories --- dwds/lib/src/debugging/debugger.dart | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 21444eaeb..cce22c403 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -469,12 +469,24 @@ class Debugger extends Domain { } void _scriptParsedHandler(ScriptParsedEvent e) { - final scriptPath = p.joinAll(Uri.parse(e.script.url).pathSegments); - if (scriptPath.isNotEmpty) { + final scriptPath = _packagesPathForChromeScript(e.script.url); + if (scriptPath != null) { chromePathToRuntimeScriptId[scriptPath] = e.script.scriptId; } } + String? _packagesPathForChromeScript(String scriptUrl) { + final scriptPathSegments = Uri.parse(scriptUrl).pathSegments; + const packagesDir = 'packages'; + if (scriptPathSegments.isEmpty || + !scriptPathSegments.contains(packagesDir)) { + return null; + } + + final packagesIdx = scriptPathSegments.indexOf(packagesDir); + return p.joinAll(scriptPathSegments.sublist(packagesIdx)); + } + /// Handles pause events coming from the Chrome connection. Future _pauseHandler(DebuggerPausedEvent e) async { final isolate = inspector.isolate; From dbc9431cb8398ed037cda82d91917880e9d95d4b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 5 Feb 2024 15:15:18 -0800 Subject: [PATCH 11/18] Analyzer errors --- dwds/lib/src/debugging/debugger.dart | 8 -------- 1 file changed, 8 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index cce22c403..4828c25db 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -705,14 +705,6 @@ Future sendCommandAndValidateResult( return result; } -/// Returns the Dart line number for the provided breakpoint. -int _lineNumberFor(Breakpoint breakpoint) => - 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, int column) => From 56213c3c85b5925d75d5748b83f7b72526880cbc Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:22:02 -0800 Subject: [PATCH 12/18] Remove dead code --- dwds/lib/src/services/chrome_proxy_service.dart | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 682965c85..2908415d9 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -91,9 +91,6 @@ class ChromeProxyService implements VmServiceInterface { StreamSubscription? _consoleSubscription; - final _disabledBreakpoints = {}; - final _previousBreakpoints = {}; - final _logger = Logger('ChromeProxyService'); final ExpressionCompiler? _compiler; @@ -287,8 +284,6 @@ class ChromeProxyService implements VmServiceInterface { safeUnawaited(_prewarmExpressionCompilerCache()); - _disabledBreakpoints.clear(); - safeUnawaited( appConnection.onStart.then((_) { debugger.resumeFromStart(); @@ -364,19 +359,15 @@ class ChromeProxyService implements VmServiceInterface { ); _vm.isolates?.removeWhere((ref) => ref.id == isolate.id); _inspector = null; - _previousBreakpoints.clear(); - _previousBreakpoints.addAll(isolate.breakpoints ?? []); _expressionEvaluator?.close(); _consoleSubscription?.cancel(); _consoleSubscription = null; } Future disableBreakpoints() async { - _disabledBreakpoints.clear(); if (!_isIsolateRunning) return; final isolate = inspector.isolate; - _disabledBreakpoints.addAll(isolate.breakpoints ?? []); for (var breakpoint in isolate.breakpoints?.toList() ?? []) { await (await debuggerFuture).removeBreakpoint(breakpoint.id); } @@ -1109,8 +1100,6 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer. ) async { await isInitialized; _checkIsolate('removeBreakpoint', isolateId); - _disabledBreakpoints - .removeWhere((breakpoint) => breakpoint.id == breakpointId); return (await debuggerFuture).removeBreakpoint(breakpointId); } From 1eaebe0d8fc293bfafbf21387114844f3be42857 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:22:12 -0800 Subject: [PATCH 13/18] Get tests to pass --- dwds/lib/src/debugging/debugger.dart | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 4828c25db..5e3507519 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -469,22 +469,26 @@ class Debugger extends Domain { } void _scriptParsedHandler(ScriptParsedEvent e) { - final scriptPath = _packagesPathForChromeScript(e.script.url); + final scriptPath = _pathForChromeScript(e.script.url); if (scriptPath != null) { chromePathToRuntimeScriptId[scriptPath] = e.script.scriptId; } } - String? _packagesPathForChromeScript(String scriptUrl) { + String? _pathForChromeScript(String scriptUrl) { final scriptPathSegments = Uri.parse(scriptUrl).pathSegments; - const packagesDir = 'packages'; - if (scriptPathSegments.isEmpty || - !scriptPathSegments.contains(packagesDir)) { + if (scriptPathSegments.isEmpty) { return null; } - final packagesIdx = scriptPathSegments.indexOf(packagesDir); - return p.joinAll(scriptPathSegments.sublist(packagesIdx)); + final isInternal = globalToolConfiguration.appMetadata.isInternalBuild; + const packagesDir = 'packages'; + if (isInternal && scriptUrl.contains(packagesDir)) { + final packagesIdx = scriptPathSegments.indexOf(packagesDir); + return p.joinAll(scriptPathSegments.sublist(packagesIdx)); + } + + return p.joinAll(scriptPathSegments); } /// Handles pause events coming from the Chrome connection. From e5f0da017b4a98ff01b1028ca80cb2e687f3848e Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:23:57 -0800 Subject: [PATCH 14/18] Update CHANGELOG --- dwds/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 42179d692..4046391b3 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,6 +1,7 @@ ## 23.4.0-wip -- Adding tests for constants in DDC after a hot restart - [#2349](https://github.com/dart-lang/webdev/pull/2349) +- Adding tests for constants in DDC after a hot restart. - [#2349](https://github.com/dart-lang/webdev/pull/2349) +- Do not try to persist breakpoints across hot restarts or page reloads. - [#2371](https://github.com/dart-lang/webdev/pull/2371) ## 23.3.0 From 7477820280409af75421bcfa211cba31f90058e2 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:24:38 -0800 Subject: [PATCH 15/18] CHANGELOG wording --- dwds/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 4046391b3..2436edd40 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,7 +1,7 @@ ## 23.4.0-wip - Adding tests for constants in DDC after a hot restart. - [#2349](https://github.com/dart-lang/webdev/pull/2349) -- Do not try to persist breakpoints across hot restarts or page reloads. - [#2371](https://github.com/dart-lang/webdev/pull/2371) +- Do not persist breakpoints across hot restarts or page reloads. - [#2371](https://github.com/dart-lang/webdev/pull/2371) ## 23.3.0 From 706e57bd0d34b330201d3ad8452e12c810e74cc9 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 6 Feb 2024 15:20:04 -0800 Subject: [PATCH 16/18] Delete restore_breakpoints_test --- dwds/test/restore_breakpoints_test.dart | 124 ------------------------ 1 file changed, 124 deletions(-) delete mode 100644 dwds/test/restore_breakpoints_test.dart diff --git a/dwds/test/restore_breakpoints_test.dart b/dwds/test/restore_breakpoints_test.dart deleted file mode 100644 index 5070c47e1..000000000 --- a/dwds/test/restore_breakpoints_test.dart +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -@TestOn('vm') -@Timeout(Duration(minutes: 2)) -import 'dart:async'; - -import 'package:test/test.dart'; -import 'package:test_common/logging.dart'; -import 'package:test_common/test_sdk_configuration.dart'; -import 'package:vm_service/vm_service.dart'; -import 'package:vm_service_interface/vm_service_interface.dart'; - -import 'fixtures/context.dart'; -import 'fixtures/project.dart'; - -void main() { - final provider = TestSdkConfigurationProvider(); - tearDownAll(provider.dispose); - - final context = TestContext(TestProject.testWithSoundNullSafety, provider); - - setUpAll(() async { - setCurrentLogWriter(); - await context.setUp(); - }); - - tearDownAll(() async { - await context.tearDown(); - }); - - group('breakpoints', () { - late VmServiceInterface service; - VM vm; - late Isolate isolate; - ScriptList scripts; - late ScriptRef mainScript; - late Stream isolateEventStream; - - setUp(() async { - setCurrentLogWriter(); - service = context.service; - vm = await service.getVM(); - isolate = await service.getIsolate(vm.isolates!.first.id!); - scripts = await service.getScripts(isolate.id!); - mainScript = scripts.scripts! - .firstWhere((each) => each.uri!.contains('main.dart')); - isolateEventStream = service.onEvent('Isolate'); - }); - - tearDown(() async { - // Remove breakpoints so they don't impact other tests. - for (var breakpoint in isolate.breakpoints!.toList()) { - await service.removeBreakpoint(isolate.id!, breakpoint.id!); - } - }); - - test( - 'restore after refresh', - () async { - final firstBp = - await service.addBreakpoint(isolate.id!, mainScript.id!, 23); - expect(firstBp, isNotNull); - expect(firstBp.id, isNotNull); - - final eventsDone = expectLater( - isolateEventStream, - emitsThrough( - emitsInOrder([ - predicate((Event event) => event.kind == EventKind.kIsolateExit), - predicate((Event event) => event.kind == EventKind.kIsolateStart), - predicate( - (Event event) => event.kind == EventKind.kIsolateRunnable, - ), - ]), - ), - ); - - await context.webDriver.refresh(); - await eventsDone; - - vm = await service.getVM(); - isolate = await service.getIsolate(vm.isolates!.first.id!); - - expect(isolate.breakpoints!.length, equals(1)); - }, - timeout: const Timeout.factor(2), - ); - - test( - 'restore after hot restart', - () async { - final firstBp = - await service.addBreakpoint(isolate.id!, mainScript.id!, 23); - expect(firstBp, isNotNull); - expect(firstBp.id, isNotNull); - - final eventsDone = expectLater( - isolateEventStream, - emits( - emitsInOrder([ - predicate((Event event) => event.kind == EventKind.kIsolateExit), - predicate((Event event) => event.kind == EventKind.kIsolateStart), - predicate( - (Event event) => event.kind == EventKind.kIsolateRunnable, - ), - ]), - ), - ); - - await context.debugConnection.vmService - .callServiceExtension('hotRestart'); - await eventsDone; - - vm = await service.getVM(); - isolate = await service.getIsolate(vm.isolates!.first.id!); - - expect(isolate.breakpoints!.length, equals(1)); - }, - timeout: const Timeout.factor(2), - ); - }); -} From d150821cc967c26df6c4b8ae5d78b08450e6c0d0 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 13 Feb 2024 14:43:11 -0800 Subject: [PATCH 17/18] Fix reload_test --- dwds/test/reload_test.dart | 58 ++++---------------------------------- 1 file changed, 6 insertions(+), 52 deletions(-) diff --git a/dwds/test/reload_test.dart b/dwds/test/reload_test.dart index 32b969326..2961273e7 100644 --- a/dwds/test/reload_test.dart +++ b/dwds/test/reload_test.dart @@ -376,68 +376,22 @@ void main() { isolateId = vm.isolates!.first.id!; final isolate = await client.getIsolate(isolateId); - // Previous breakpoint should still exist. - expect(isolate.breakpoints!.isNotEmpty, isTrue); - final bp = isolate.breakpoints!.first; - - // Should pause eventually. - await stream - .firstWhere((event) => event.kind == EventKind.kPauseBreakpoint); - - expect( - await client.removeBreakpoint(isolate.id!, bp.id!), - isA(), - ); - expect(await client.resume(isolate.id!), isA()); + // Previous breakpoint should be cleared. + expect(isolate.breakpoints!.isEmpty, isTrue); }); - test('can evaluate expressions after hot restart ', () async { + test('can evaluate expressions after hot restart', () async { final client = context.debugConnection.vmService; - var vm = await client.getVM(); - var isolateId = vm.isolates!.first.id!; - await client.streamListen('Debug'); - final stream = client.onEvent('Debug'); - final scriptList = await client.getScripts(isolateId); - final main = scriptList.scripts! - .firstWhere((script) => script.uri!.contains('main.dart')); - final bpLine = - await context.findBreakpointLine('printCount', isolateId, main); - await client.addBreakpoint(isolateId, main.id!, bpLine); - await stream - .firstWhere((event) => event.kind == EventKind.kPauseBreakpoint); await client.callServiceExtension('hotRestart'); - vm = await client.getVM(); - isolateId = vm.isolates!.first.id!; + final vm = await client.getVM(); + final isolateId = vm.isolates!.first.id!; final isolate = await client.getIsolate(isolateId); final library = isolate.rootLib!.uri!; - final bp = isolate.breakpoints!.first; - - // Should pause eventually. - final event = await stream - .firstWhere((event) => event.kind == EventKind.kPauseBreakpoint); - - // Expression evaluation while paused on a breakpoint should work. - var result = await client.evaluateInFrame( - isolate.id!, - event.topFrame!.index!, - 'count', - ); - expect( - result, - isA().having( - (instance) => instance.valueAsString, - 'valueAsString', - greaterThanOrEqualTo('0'), - ), - ); - - await client.removeBreakpoint(isolateId, bp.id!); - await client.resume(isolateId); // Expression evaluation while running should work. - result = await client.evaluate(isolateId, library, 'true'); + final result = await client.evaluate(isolateId, library, 'true'); expect( result, isA().having( From 2a5e41725cc8a435535bcd31d8120a04ebb6a01e Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:43:22 -0800 Subject: [PATCH 18/18] Hopefully fix failing window tests --- dwds/lib/src/debugging/debugger.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 5e3507519..c2e934605 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -488,7 +488,9 @@ class Debugger extends Domain { return p.joinAll(scriptPathSegments.sublist(packagesIdx)); } - return p.joinAll(scriptPathSegments); + // Note: Replacing "\" with "/" is necessary because `joinAll` uses "\" if + // the platform is Windows. However, only "/" is expected by the browser. + return p.joinAll(scriptPathSegments).replaceAll('\\', '/'); } /// Handles pause events coming from the Chrome connection. @@ -742,8 +744,10 @@ class _Breakpoints extends Domain { int line, int column, ) async { + print('creating breakpoint at $scriptId:$line:$column)'); final dartScript = inspector.scriptWithId(scriptId); final dartScriptUri = dartScript?.uri; + print('dart script uri is $dartScriptUri'); Location? location; if (dartScriptUri != null) { final dartUri = DartUri(dartScriptUri, root);