Skip to content

Hotfix: support Chrome 100 update #1566

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
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
4 changes: 4 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 11.5.2

- Hotfix: Fix missing `CallFrame.url` after update to Chrome 100.

## 11.5.1

- Update SDK contraint to `>=2.15.0 <3.0.0`.
Expand Down
2 changes: 0 additions & 2 deletions dwds/debug_extension/web/background.dart
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,6 @@ void _filterAndForward(Debuggee source, String method, Object params) {

if (debugSession == null) return;

if (method == 'Debugger.scriptParsed') return;

var event = _extensionEventFor(method, params);

debugSession.socketClient.sink.add(jsonEncode(serializers.serialize(event)));
Expand Down
28 changes: 24 additions & 4 deletions dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -307,15 +307,22 @@ class Debugger extends Domain {
}
}

/// Returns Chrome script uri for Chrome script ID.
String _urlForScriptId(String scriptId) =>
_remoteDebugger.scripts[scriptId]?.url;

/// Returns source [Location] for the paused event.
///
/// If we do not have [Location] data for the embedded JS location, null is
/// returned.
Future<Location> _sourceLocation(DebuggerPausedEvent e) {
var frame = e.params['callFrames'][0];
var location = frame['location'];
return _locations.locationForJs(
frame['url'] as String, (location['lineNumber'] as int) + 1);
var scriptId = location['scriptId'] as String;
var lineNumber = location['lineNumber'] as int;

var url = _urlForScriptId(scriptId);
return _locations.locationForJs(url, lineNumber + 1);
}

/// The variables visible in a frame in Dart protocol [BoundVariable] form.
Expand Down Expand Up @@ -427,10 +434,17 @@ class Debugger extends Domain {
// Chrome is 0 based. Account for this.
var line = location.lineNumber + 1;
var column = location.columnNumber + 1;

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}');
}

// TODO(sdk/issues/37240) - ideally we look for an exact location instead
// of the closest location on a given line.
Location bestLocation;
for (var location in await _locations.locationsForUrl(frame.url)) {
for (var location in await _locations.locationsForUrl(url)) {
if (location.jsLocation.line == line) {
bestLocation ??= location;
if ((location.jsLocation.column - column).abs() <
Expand Down Expand Up @@ -530,8 +544,14 @@ class Debugger extends Domain {
// If we don't have source location continue stepping.
if (_isStepping && (await _sourceLocation(e)) == null) {
var frame = e.params['callFrames'][0];
var url = '${frame["url"]}';
var scriptId = '${frame["location"]["scriptId"]}';

var url = _urlForScriptId(scriptId);
if (url == null) {
logger.severe('Stepping failed: '
'cannot find location for script $scriptId');
}

// TODO(grouma) - In the future we should send all previously computed
// skipLists.
await _remoteDebugger.stepInto(params: {
Expand Down
6 changes: 0 additions & 6 deletions dwds/lib/src/debugging/modules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ class Modules {
final _sourceToLibrary = <String, Uri>{};
var _moduleMemoizer = AsyncMemoizer<void>();

// The Chrome script ID to corresponding module.
final _scriptIdToModule = <String, String>{};

final Map<String, String> _libraryToModule = {};

String _entrypoint;
Expand All @@ -43,9 +40,6 @@ class Modules {
_entrypoint = entrypoint;
}

/// Returns the module for the Chrome script ID.
String moduleForScriptId(String scriptId) => _scriptIdToModule[scriptId];

/// Returns the containing module for the provided Dart server path.
Future<String> moduleForSource(String serverPath) async {
await _moduleMemoizer.runOnce(_initializeMapping);
Expand Down
737 changes: 307 additions & 430 deletions dwds/lib/src/injected/client.js

Large diffs are not rendered by default.

10 changes: 8 additions & 2 deletions dwds/lib/src/services/expression_evaluator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ class ExpressionEvaluator {

var functionName = jsFrame.functionName;
var jsLine = jsFrame.location.lineNumber + 1;
var jsScriptId = jsFrame.location.scriptId;
var jsScope = await _collectLocalJsScope(jsFrame);

// Find corresponding dart location and scope.
Expand All @@ -158,12 +159,13 @@ class ExpressionEvaluator {
// so this will result in expressions not evaluated in some
// cases. Invent location matching strategy for those cases.
// [issue 890](https://github.com/dart-lang/webdev/issues/890)
var locationMap = await _locations.locationForJs(jsFrame.url, jsLine);
var url = _urlForScriptId(jsScriptId);
var locationMap = await _locations.locationForJs(url, jsLine);
if (locationMap == null) {
return _createError(
ErrorKind.internal,
'Cannot find Dart location for JS location: '
'url: ${jsFrame.url}'
'url: $url, '
'function: $functionName, '
'line: $jsLine');
}
Expand Down Expand Up @@ -300,4 +302,8 @@ class ExpressionEvaluator {

return jsScope;
}

/// Returns Chrome script uri for Chrome script ID.
String _urlForScriptId(String scriptId) =>
_inspector.remoteDebugger.scripts[scriptId]?.url;
}
2 changes: 1 addition & 1 deletion dwds/lib/src/version.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dwds/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: dwds
# Every time this changes you need to run `dart run build_runner build`.
version: 11.5.1
version: 11.5.2
homepage: https://github.com/dart-lang/webdev/tree/master/dwds
description: >-
A service that proxies between the Chrome debug protocol and the Dart VM
Expand Down
17 changes: 13 additions & 4 deletions dwds/test/debugger_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import 'package:dwds/src/loaders/strategy.dart';
import 'package:test/test.dart';
import 'package:vm_service/vm_service.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
show CallFrame, DebuggerPausedEvent, StackTrace, WipCallFrame;
show CallFrame, DebuggerPausedEvent, StackTrace, WipCallFrame, WipScript;

import 'fixtures/context.dart';
import 'fixtures/debugger_data.dart';
Expand Down Expand Up @@ -72,22 +72,31 @@ final sampleSyncFrame = WipCallFrame({
'columnNumber': 72,
},
'location': {'scriptId': '69', 'lineNumber': 37, 'columnNumber': 0},
'url': 'http://127.0.0.1:8081/foo.ddc.js',
'url': '',
'scopeChain': [],
'this': {'type': 'undefined'},
});

final sampleAsyncFrame = CallFrame({
'functionName': 'myFunc',
'url': 'http://127.0.0.1:8081/bar.ddc.js',
'url': '',
'scriptId': '71',
'lineNumber': 40,
'columnNumber': 1,
});

final Map<String, WipScript> scripts = {
'69': WipScript(<String, dynamic>{
'url': 'http://127.0.0.1:8081/foo.ddc.js',
}),
'71': WipScript(<String, dynamic>{
'url': 'http://127.0.0.1:8081/bar.ddc.js',
}),
};

void main() async {
setUpAll(() async {
webkitDebugger = FakeWebkitDebugger();
webkitDebugger = FakeWebkitDebugger(scripts: scripts);
pausedController = StreamController<DebuggerPausedEvent>();
webkitDebugger.onPaused = pausedController.stream;
globalLoadStrategy = TestStrategy();
Expand Down
8 changes: 3 additions & 5 deletions dwds/test/fixtures/fakes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,6 @@ class FakeModules implements Modules {
throw UnimplementedError();
}

@override
String moduleForScriptId(String serverId) => '';

@override
Future<String> moduleForSource(String serverPath) {
throw UnimplementedError();
Expand All @@ -135,13 +132,14 @@ class FakeModules implements Modules {
}

class FakeWebkitDebugger implements WebkitDebugger {
final Map<String, WipScript> _scripts;
@override
Future disable() => null;

@override
Future enable() => null;

FakeWebkitDebugger() {
FakeWebkitDebugger({Map<String, WipScript> scripts}) : _scripts = scripts {
globalLoadStrategy = RequireStrategy(
ReloadConfiguration.none,
(_) async => {},
Expand Down Expand Up @@ -185,7 +183,7 @@ class FakeWebkitDebugger implements WebkitDebugger {
Future<WipResponse> resume() => null;

@override
Map<String, WipScript> get scripts => null;
Map<String, WipScript> get scripts => _scripts;

List<WipResponse> results = variables1;
int resultsReturned = 0;
Expand Down