Skip to content

Set breakpoints by regex URL #1078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

**Breaking Changes:**
- Require access to the `.ddc_merged_metadata` file.
- Remove deprecated parameter `restoreBreakpoints` as breakpoints are now
set by regex URL and Chrome automatically reestablishes them.

## 5.0.0

Expand Down
3 changes: 0 additions & 3 deletions dwds/lib/dwds.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class Dwds {
// TODO(annagrin): make expressionCompiler argument required
// [issue 881](https://github.com/dart-lang/webdev/issues/881)
ExpressionCompiler expressionCompiler,
@deprecated bool restoreBreakpoints,
}) async {
hostname ??= 'localhost';
enableDebugging ??= true;
Expand All @@ -104,7 +103,6 @@ class Dwds {
logWriter ??= (level, message) => print(message);
verbose ??= false;
globalLoadStrategy = loadStrategy;
restoreBreakpoints ??= false;

DevTools devTools;
String extensionUri;
Expand Down Expand Up @@ -148,7 +146,6 @@ class Dwds {
logWriter,
extensionBackend,
urlEncoder,
restoreBreakpoints,
useSseForDebugProxy,
serveDevTools,
expressionCompiler,
Expand Down
62 changes: 48 additions & 14 deletions dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Debugger extends Domain {
final AssetReader _assetReader;
final Modules _modules;
final Locations _locations;
final String _root;

Debugger._(
this._remoteDebugger,
Expand All @@ -56,12 +57,12 @@ class Debugger extends Domain {
this._assetReader,
this._modules,
this._locations,
String root,
this._root,
) : _breakpoints = _Breakpoints(
locations: _locations,
provider: provider,
remoteDebugger: _remoteDebugger,
root: root),
root: _root),
super(provider);

/// The breakpoints we have set so far, indexable by either
Expand Down Expand Up @@ -269,18 +270,52 @@ class Debugger extends Domain {
int column,
}) async {
checkIsolate('addBreakpoint', isolateId);

final breakpoint = await _breakpoints.add(scriptId, line);
_notifyBreakpoint(breakpoint);
return breakpoint;
}

Future<ScriptRef> _updatedScriptRefFor(Breakpoint breakpoint) async {
var oldRef = (breakpoint.location as SourceLocation).script;
var dartUri = DartUri(oldRef.uri, _root);
return await inspector.scriptRefFor(dartUri.serverPath);
}

Future<void> reestablishBreakpoints(
Set<Breakpoint> previousBreakpoints,
Set<Breakpoint> 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) {
var scriptRef = await _updatedScriptRefFor(breakpoint);
var updatedLocation = await _locations.locationForDart(
DartUri(scriptRef.uri, _root), _lineNumberFor(breakpoint));
var updatedBreakpoint = _breakpoints._dartBreakpoint(
scriptRef, updatedLocation, breakpoint.id);
_breakpoints._note(
bp: updatedBreakpoint,
jsId: _breakpoints._jsIdByDartId[updatedBreakpoint.id]);
_notifyBreakpoint(updatedBreakpoint);
}
// Disabled breakpoints were actually removed from Chrome so simply add
// them back.
for (var breakpoint in disabledBreakpoints) {
await addBreakpoint(
inspector.isolate.id,
(await _updatedScriptRefFor(breakpoint)).id,
_lineNumberFor(breakpoint));
}
}

void _notifyBreakpoint(Breakpoint breakpoint) {
final event = Event(
kind: EventKind.kBreakpointAdded,
timestamp: DateTime.now().millisecondsSinceEpoch,
isolate: inspector.isolateRef,
);
event.breakpoint = breakpoint;
_streamNotify('Debug', event);

return breakpoint;
}

/// Remove a Dart breakpoint.
Expand Down Expand Up @@ -624,7 +659,7 @@ class Debugger extends Domain {
}

/// Returns the Dart line number for the provided breakpoint.
int lineNumberFor(Breakpoint breakpoint) =>
int _lineNumberFor(Breakpoint breakpoint) =>
int.parse(breakpoint.id.split('#').last);

/// Returns the breakpoint ID for the provided Dart script ID and Dart line
Expand Down Expand Up @@ -699,12 +734,13 @@ class _Breakpoints extends Domain {
Future<String> _setJsBreakpoint(Location location) async {
// Location is 0 based according to:
// https://chromedevtools.github.io/devtools-protocol/tot/Debugger#type-Location
var response =
await remoteDebugger.sendCommand('Debugger.setBreakpoint', params: {
'location': {
'scriptId': location.jsLocation.scriptId,
'lineNumber': location.jsLocation.line - 1,
}

// The module can be loaded from a nested path and contain an ETAG suffix.
var urlRegex = '.*${location.modulePath}.*';
var response = await remoteDebugger
.sendCommand('Debugger.setBreakpointByUrl', params: {
'urlRegex': urlRegex,
'lineNumber': location.jsLocation.line - 1,
});
return response.result['breakpointId'] as String;
}
Expand All @@ -729,8 +765,6 @@ class _Breakpoints extends Domain {
return _bpByDartId.remove(dartId);
}

String dartId(String jsId) => _dartIdByJsId[jsId];

String jsId(String dartId) => _jsIdByDartId[dartId];
}

Expand Down
12 changes: 10 additions & 2 deletions dwds/lib/src/debugging/location.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@ class Location {

final DartLocation dartLocation;

final String modulePath;

/// An arbitrary integer value used to represent this location.
final int tokenPos;

Location._(
this.jsLocation,
this.dartLocation,
this.modulePath,
) : tokenPos = _startTokenId++;

static Location from(
String scriptId,
String modulePath,
TargetLineEntry lineEntry,
TargetEntry entry,
DartUri dartUri,
Expand All @@ -41,8 +45,11 @@ class Location {
var jsColumn = entry.column;
// lineEntry data is 0 based according to:
// https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k
return Location._(JsLocation.fromZeroBased(scriptId, jsLine, jsColumn),
DartLocation.fromZeroBased(dartUri, dartLine, dartColumn));
return Location._(
JsLocation.fromZeroBased(scriptId, jsLine, jsColumn),
DartLocation.fromZeroBased(dartUri, dartLine, dartColumn),
modulePath,
);
}

@override
Expand Down Expand Up @@ -259,6 +266,7 @@ class Locations {
var dartUri = DartUri(path, _root);
result.add(Location.from(
scriptId,
modulePath,
lineEntry,
entry,
dartUri,
Expand Down
4 changes: 0 additions & 4 deletions dwds/lib/src/handlers/dev_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ class DevHandler {
final StreamController<DebugConnection> extensionDebugConnections =
StreamController<DebugConnection>();
final UrlEncoder _urlEncoder;
final bool _restoreBreakpoints;
final bool _useSseForDebugProxy;
final bool _serveDevTools;
final ExpressionCompiler _expressionCompiler;
Expand All @@ -87,7 +86,6 @@ class DevHandler {
this._logWriter,
this._extensionBackend,
this._urlEncoder,
this._restoreBreakpoints,
this._useSseForDebugProxy,
this._serveDevTools,
this._expressionCompiler,
Expand Down Expand Up @@ -197,7 +195,6 @@ class DevHandler {
metadataProvider,
appConnection,
_logWriter,
_restoreBreakpoints,
onResponse: (response) {
if (_verbose) {
if (response['error'] == null) return;
Expand Down Expand Up @@ -473,7 +470,6 @@ class DevHandler {
metadataProvider,
connection,
_logWriter,
_restoreBreakpoints,
onResponse: _verbose
? (response) {
if (response['error'] == null) return;
Expand Down
36 changes: 7 additions & 29 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ class ChromeProxyService implements VmServiceInterface {

StreamSubscription<ConsoleAPIEvent> _consoleSubscription;

/// If breakpoints should be restored upon a reload / hot restart.
final bool _restoreBreakpoints;
final _disabledBreakpoints = <Breakpoint>{};
final _previousBreakpoints = <Breakpoint>{};

Expand All @@ -96,7 +94,6 @@ class ChromeProxyService implements VmServiceInterface {
this._metadataProvider,
this._modules,
this._locations,
this._restoreBreakpoints,
this.executionContext,
this._logWriter,
) {
Expand All @@ -119,7 +116,6 @@ class ChromeProxyService implements VmServiceInterface {
MetadataProvider metadataProvider,
AppConnection appConnection,
LogWriter logWriter,
bool restoreBreakpoints,
ExecutionContext executionContext,
ExpressionCompiler expressionCompiler) async {
final vm = VM(
Expand All @@ -137,17 +133,8 @@ class ChromeProxyService implements VmServiceInterface {

var modules = Modules(metadataProvider, tabUrl);
var locations = Locations(assetReader, modules, tabUrl);
var service = ChromeProxyService._(
vm,
tabUrl,
assetReader,
remoteDebugger,
metadataProvider,
modules,
locations,
restoreBreakpoints,
executionContext,
logWriter);
var service = ChromeProxyService._(vm, tabUrl, assetReader, remoteDebugger,
metadataProvider, modules, locations, executionContext, logWriter);
unawaited(service.createIsolate(appConnection));
await service.createEvaluator(expressionCompiler);
return service;
Expand Down Expand Up @@ -192,14 +179,9 @@ class ChromeProxyService implements VmServiceInterface {
executionContext,
);

for (var breakpoint in _previousBreakpoints) {
var lineNumber = lineNumberFor(breakpoint);
var oldRef = (breakpoint.location as SourceLocation).script;
var dartUri = DartUri(oldRef.uri, uri);
var newRef = await _inspector.scriptRefFor(dartUri.serverPath);
await (await _debugger)
.addBreakpoint(_inspector.isolate.id, newRef.id, lineNumber);
}
await (await _debugger)
.reestablishBreakpoints(_previousBreakpoints, _disabledBreakpoints);
_disabledBreakpoints.clear();

unawaited(appConnection.onStart.then((_) async {
await (await _debugger).resumeFromStart();
Expand Down Expand Up @@ -257,12 +239,8 @@ class ChromeProxyService implements VmServiceInterface {
isolate: _inspector.isolateRef));
_vm.isolates.removeWhere((ref) => ref.id == isolate.id);
_inspector = null;
if (_restoreBreakpoints) {
_previousBreakpoints.clear();
_previousBreakpoints
..addAll(isolate.breakpoints)
..addAll(_disabledBreakpoints);
}
_previousBreakpoints.clear();
_previousBreakpoints.addAll(isolate.breakpoints);
_consoleSubscription.cancel();
_consoleSubscription = null;
}
Expand Down
2 changes: 0 additions & 2 deletions dwds/lib/src/services/debug_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ class DebugService {
MetadataProvider metadataProvider,
AppConnection appConnection,
LogWriter logWriter,
bool restoreBreakpoints,
{void Function(Map<String, dynamic>) onRequest,
void Function(Map<String, dynamic>) onResponse,
bool useSse,
Expand All @@ -148,7 +147,6 @@ class DebugService {
metadataProvider,
appConnection,
logWriter,
restoreBreakpoints,
executionContext,
expressionCompiler);
var authToken = _makeAuthToken();
Expand Down
1 change: 1 addition & 0 deletions dwds/test/debugger_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ void main() async {
var lineEntry = TargetLineEntry(92, [entry]);
var location = Location.from(
'foo.dart',
'foo.ddc.js',
lineEntry,
entry,
DartUri('package:foo/foo.dart'),
Expand Down
2 changes: 0 additions & 2 deletions dwds/test/fixtures/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ class TestServer {
hostname: hostname,
verbose: true,
urlEncoder: urlEncoder,
// ignore: deprecated_member_use_from_same_package
restoreBreakpoints: restoreBreakpoints,
expressionCompiler: expressionCompiler,
);

Expand Down
3 changes: 2 additions & 1 deletion dwds/test/reload_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ void main() {
isolateId = vm.isolates.first.id;
var isolate = await client.getIsolate(isolateId);
expect(isolate.pauseEvent.kind, EventKind.kResume);
expect(isolate.breakpoints.isEmpty, isTrue);
// Previous breakpoint should still exist.
expect(isolate.breakpoints.isNotEmpty, isTrue);
});
});

Expand Down