Skip to content

[MV3 Debug Extension] Support Bolt workflow #1983

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
Feb 23, 2023
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
Expand Up @@ -13,8 +13,11 @@
- Move shared test-only code to a new `test_common` package.

**Breaking changes**

- Require `sdkConfigurationProvider` in `ExpressionCompilerService`
constructor.
- Change DWDS parameter `isFlutterApp` from type `bool?` to type
`Future<bool> Function()?`.

## 17.0.0

Expand All @@ -40,6 +43,7 @@
- Fix expression compiler throwing when weak SDK summary is not found.

**Breaking changes**

- Include an optional param to `Dwds.start` to indicate whether it is running
internally or externally.
- Include an optional param to `Dwds.start` to indicate whether it a Flutter
Expand Down
53 changes: 53 additions & 0 deletions dwds/debug_extension_mv3/web/debug_session.dart
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ Future<void> _maybeConnectToDwds(int tabId, Object? params) async {
if (debugInfo == null) return;
if (contextOrigin != debugInfo.appOrigin) return;
final contextId = context['id'] as int;
// Find the correct frame to connect to (this is necessary if the Dart app is
// embedded in an IFRAME):
final isDartFrame = await _isDartFrame(tabId: tabId, contextId: contextId);
if (!isDartFrame) return;
final connected = await _connectToDwds(
dartAppContextId: contextId,
dartAppTabId: tabId,
Expand All @@ -257,6 +261,33 @@ Future<void> _maybeConnectToDwds(int tabId, Object? params) async {
}
}

Future<bool> _isDartFrame({required int tabId, required int contextId}) {
final completer = Completer<bool>();
chrome.debugger.sendCommand(
Debuggee(tabId: tabId),
'Runtime.evaluate',
_InjectedParams(
expression:
'[window.\$dartAppId, window.\$dartAppInstanceId, window.\$dwdsVersion]',
returnByValue: true,
contextId: contextId), allowInterop((dynamic response) {
final evalResponse = response as _EvalResponse;
final value = evalResponse.result.value;
final appId = value?[0];
final instanceId = value?[1];
final dwdsVersion = value?[2];
final frameIdentifier = 'Frame at tab $tabId with context $contextId';
if (appId == null || instanceId == null) {
debugWarn('$frameIdentifier is not a Dart frame.');
completer.complete(false);
} else {
debugLog('Dart $frameIdentifier is using DWDS $dwdsVersion.');
completer.complete(true);
}
}));
return completer.future;
}

Future<bool> _connectToDwds({
required int dartAppContextId,
required int dartAppTabId,
Expand Down Expand Up @@ -687,3 +718,25 @@ class _DebugSession {
}
}
}

@JS()
@anonymous
class _EvalResponse {
external _EvalResult get result;
}

@JS()
@anonymous
class _EvalResult {
external List<String?>? get value;
}

@JS()
@anonymous
class _InjectedParams {
external String get expresion;
external bool get returnByValue;
external int get contextId;
external factory _InjectedParams(
{String? expression, bool? returnByValue, int? contextId});
}
3 changes: 2 additions & 1 deletion dwds/lib/dart_web_debug_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ class Dwds {
bool launchDevToolsInNewWindow = true,
bool emitDebugEvents = true,
bool isInternalBuild = false,
bool isFlutterApp = false,
Future<bool> Function()? isFlutterApp,
}) async {
globalLoadStrategy = loadStrategy;
isFlutterApp ??= () => Future.value(true);

DevTools? devTools;
Future<String>? extensionUri;
Expand Down
14 changes: 7 additions & 7 deletions dwds/lib/src/handlers/injector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,16 @@ class DwdsInjector {
final bool _useSseForInjectedClient;
final bool _emitDebugEvents;
final bool _isInternalBuild;
final bool _isFlutterApp;
final Future<bool> Function() _isFlutterApp;

DwdsInjector(
this._loadStrategy, {
required bool enableDevtoolsLaunch,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DwdsInjector is only called from DWDS itself (here: https://github.com/dart-lang/webdev/pull/1983/files#diff-e56f8e114cfd5b74f48f729657275d651f1625eaf2ab0bf85463e6d602335047L122). All of the parameters passed in have default values in DWDS itself, therefore we use those values instead of setting the defaults in two places.

required bool useSseForInjectedClient,
required bool emitDebugEvents,
required bool isInternalBuild,
required Future<bool> Function() isFlutterApp,
Future<String>? extensionUri,
bool enableDevtoolsLaunch = false,
bool useSseForInjectedClient = true,
bool emitDebugEvents = true,
bool isInternalBuild = false,
bool isFlutterApp = false,
}) : _extensionUri = extensionUri,
_enableDevtoolsLaunch = enableDevtoolsLaunch,
_useSseForInjectedClient = useSseForInjectedClient,
Expand Down Expand Up @@ -117,7 +117,7 @@ class DwdsInjector {
_enableDevtoolsLaunch,
_emitDebugEvents,
_isInternalBuild,
_isFlutterApp,
await _isFlutterApp(),
);
body += await _loadStrategy.bootstrapFor(entrypoint);
_logger.info('Injected debugging metadata for '
Expand Down
2 changes: 1 addition & 1 deletion dwds/test/fixtures/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class TestServer {
urlEncoder: urlEncoder,
expressionCompiler: expressionCompiler,
isInternalBuild: isInternalBuild,
isFlutterApp: isFlutterApp,
isFlutterApp: () => Future.value(isFlutterApp),
devtoolsLauncher: serveDevTools
? (hostname) async {
final server = await DevToolsServer().serveDevTools(
Expand Down
30 changes: 25 additions & 5 deletions dwds/test/handlers/injector_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ void main() {
group('InjectedHandlerWithoutExtension', () {
late DwdsInjector injector;
setUp(() async {
injector = DwdsInjector(loadStrategy);
injector = DwdsInjector(
loadStrategy,
useSseForInjectedClient: true,
enableDevtoolsLaunch: true,
emitDebugEvents: true,
isInternalBuild: false,
isFlutterApp: () => Future.value(true),
);
final pipeline = const Pipeline().addMiddleware(injector.middleware);
server = await shelf_io.serve(pipeline.addHandler((request) {
if (request.url.path.endsWith(bootstrapJsExtension)) {
Expand Down Expand Up @@ -223,7 +230,14 @@ void main() {
group('InjectedHandlerWithoutExtension using WebSockets', () {
late DwdsInjector injector;
setUp(() async {
injector = DwdsInjector(loadStrategy, useSseForInjectedClient: false);
injector = DwdsInjector(
loadStrategy,
useSseForInjectedClient: false,
enableDevtoolsLaunch: true,
emitDebugEvents: true,
isInternalBuild: false,
isFlutterApp: () => Future.value(true),
);
final pipeline = const Pipeline().addMiddleware(injector.middleware);
server = await shelf_io.serve(pipeline.addHandler((request) {
if (request.url.path.endsWith(bootstrapJsExtension)) {
Expand Down Expand Up @@ -269,9 +283,15 @@ void main() {
group('InjectedHandlerWithExtension', () {
setUp(() async {
final extensionUri = 'http://localhost:4000';
final pipeline = const Pipeline().addMiddleware(
DwdsInjector(loadStrategy, extensionUri: Future.value(extensionUri))
.middleware);
final pipeline = const Pipeline().addMiddleware(DwdsInjector(
loadStrategy,
extensionUri: Future.value(extensionUri),
useSseForInjectedClient: true,
enableDevtoolsLaunch: true,
emitDebugEvents: true,
isInternalBuild: false,
isFlutterApp: () => Future.value(true),
).middleware);
server = await shelf_io.serve(pipeline.addHandler((request) {
return Response.ok(
'$entrypointExtensionMarker\n'
Expand Down
9 changes: 6 additions & 3 deletions dwds/test/puppeteer/extension_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -649,9 +649,12 @@ void main() async {
expect(iframeTarget, isNotNull);
});

// TODO(elliette): Pull TestServer out of TestContext, so we can add
// a test case for starting another test app, loading that app in
// the tab we were debugging, and be able to reconnect to that one.
// TODO(elliette): Pull TestServer out of TestContext, so we can add:
// 1. a test case for starting another test app, loading that app in
// the tab we were debugging, and being able to reconnect to that
// one.
// 2. a test case for embedding a Dart app in a tab with the same
// origin, and being able to connect to the embedded Dart app.
// See https://github.com/dart-lang/webdev/issues/1779

test('The Dart DevTools IFRAME has the correct query parameters',
Expand Down