diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index fe53aa6c2..11b6df61f 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -7,6 +7,7 @@ the scope. - Remove showing uncaptured values from the stack during evaluation. - Refactor code to break most circular dependencies between files. +- Migrate `package:dwds` to null safety. **Breaking changes** - Remove no longer used `ExpressionCompilerService.handler`. diff --git a/dwds/lib/dart_web_debug_service.dart b/dwds/lib/dart_web_debug_service.dart index 1429aa676..b65004169 100644 --- a/dwds/lib/dart_web_debug_service.dart +++ b/dwds/lib/dart_web_debug_service.dart @@ -2,12 +2,9 @@ // 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. -// @dart = 2.9 - import 'dart:async'; import 'package:logging/logging.dart'; -import 'package:meta/meta.dart'; import 'package:shelf/shelf.dart'; import 'package:sse/server/sse_handler.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; @@ -33,7 +30,7 @@ class Dwds { static final _logger = Logger('DWDS'); final Middleware middleware; final Handler handler; - final DevTools _devTools; + final DevTools? _devTools; final DevHandler _devHandler; final AssetReader _assetReader; final bool _enableDebugging; @@ -67,53 +64,42 @@ class Dwds { } static Future start({ - @required AssetReader assetReader, - @required Stream buildResults, - @required ConnectionProvider chromeConnection, - @required LoadStrategy loadStrategy, - @required bool enableDebugging, + required AssetReader assetReader, + required Stream buildResults, + required ConnectionProvider chromeConnection, + required LoadStrategy loadStrategy, + required bool enableDebugging, // TODO(annagrin): make expressionCompiler argument required // [issue 881](https://github.com/dart-lang/webdev/issues/881) - ExpressionCompiler expressionCompiler, - bool enableDebugExtension, - String hostname, - bool useSseForDebugProxy, - bool useSseForDebugBackend, - bool useSseForInjectedClient, - UrlEncoder urlEncoder, - bool spawnDds, + ExpressionCompiler? expressionCompiler, + bool enableDebugExtension = false, + String hostname = 'localhost', + bool useSseForDebugProxy = true, + bool useSseForDebugBackend = true, + bool useSseForInjectedClient = true, + UrlEncoder? urlEncoder, + bool spawnDds = true, // TODO(elliette): DevTools is inconsistently capitalized throughout this - // file. Change all occurances of devtools/Devtools to devTools/DevTools. - bool enableDevtoolsLaunch, - DevtoolsLauncher devtoolsLauncher, - bool launchDevToolsInNewWindow, - SdkConfigurationProvider sdkConfigurationProvider, - bool emitDebugEvents, + // file. Change all occurrences of devtools/Devtools to devTools/DevTools. + bool enableDevtoolsLaunch = true, + DevtoolsLauncher? devtoolsLauncher, + bool launchDevToolsInNewWindow = true, + SdkConfigurationProvider? sdkConfigurationProvider, + bool emitDebugEvents = true, }) async { - hostname ??= 'localhost'; - enableDebugging ??= true; - enableDebugExtension ??= false; - useSseForDebugProxy ??= true; - useSseForDebugBackend ??= true; - useSseForInjectedClient ??= true; - enableDevtoolsLaunch ??= true; - launchDevToolsInNewWindow ??= true; - spawnDds ??= true; globalLoadStrategy = loadStrategy; - emitDebugEvents ??= true; - sdkConfigurationProvider ??= DefaultSdkConfigurationProvider(); - DevTools devTools; - Future extensionUri; - ExtensionBackend extensionBackend; + DevTools? devTools; + Future? extensionUri; + ExtensionBackend? extensionBackend; if (enableDebugExtension) { final handler = useSseForDebugBackend ? SseSocketHandler(SseHandler(Uri.parse('/\$debug'), // Proxy servers may actively kill long standing connections. // Allow for clients to reconnect in a short window. Making the // window too long may cause issues if the user closes a debug - // session and initites a new one during the keepAlive window. + // session and initiates a new one during the keepAlive window. keepAlive: const Duration(seconds: 5))) : WebSocketSocketHandler(); @@ -154,7 +140,6 @@ class Dwds { urlEncoder, useSseForDebugProxy, useSseForInjectedClient, - serveDevTools, expressionCompiler, injected, spawnDds, diff --git a/dwds/lib/dwds.dart b/dwds/lib/dwds.dart index fbf6198e9..40f103467 100644 --- a/dwds/lib/dwds.dart +++ b/dwds/lib/dwds.dart @@ -2,8 +2,6 @@ // 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. -// @dart = 2.9 - export 'dart_web_debug_service.dart' show Dwds, ConnectionProvider; export 'src/connections/app_connection.dart' show AppConnection; export 'src/connections/debug_connection.dart' show DebugConnection; diff --git a/dwds/lib/src/debugging/inspector.dart b/dwds/lib/src/debugging/inspector.dart index b147fb9c0..3b1e22bbc 100644 --- a/dwds/lib/src/debugging/inspector.dart +++ b/dwds/lib/src/debugging/inspector.dart @@ -374,7 +374,7 @@ class AppInspector implements AppInspectorInterface { } final scriptRef = _scriptRefsById[objectId]; if (scriptRef != null) { - return await _getScript(scriptRef); + return _getScript(scriptRef); } final instance = await _instanceHelper .instanceFor(remoteObjectFor(objectId), offset: offset, count: count); diff --git a/dwds/lib/src/handlers/dev_handler.dart b/dwds/lib/src/handlers/dev_handler.dart index a013abd34..d51e2a21b 100644 --- a/dwds/lib/src/handlers/dev_handler.dart +++ b/dwds/lib/src/handlers/dev_handler.dart @@ -2,8 +2,6 @@ // 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. -// @dart = 2.9 - import 'dart:async'; import 'dart:convert'; import 'dart:io'; @@ -55,7 +53,7 @@ class DevHandler { final _subs = []; final _sseHandlers = {}; final _injectedConnections = {}; - final DevTools _devTools; + final DevTools? _devTools; final AssetReader _assetReader; final LoadStrategy _loadStrategy; final String _hostname; @@ -64,23 +62,22 @@ class DevHandler { final _appConnectionByAppId = {}; final Stream buildResults; final Future Function() _chromeConnection; - final ExtensionBackend _extensionBackend; + final ExtensionBackend? _extensionBackend; final StreamController extensionDebugConnections = StreamController(); - final UrlEncoder _urlEncoder; + final UrlEncoder? _urlEncoder; final bool _useSseForDebugProxy; final bool _useSseForInjectedClient; - final bool _serveDevTools; final bool _spawnDds; final bool _launchDevToolsInNewWindow; - final ExpressionCompiler _expressionCompiler; + final ExpressionCompiler? _expressionCompiler; final DwdsInjector _injected; final SdkConfigurationProvider _sdkConfigurationProvider; /// Null until [close] is called. /// /// All subsequent calls to [close] will return this future. - Future _closed; + Future? _closed; Stream get connectedApps => _connectedApps.stream; @@ -95,14 +92,12 @@ class DevHandler { this._urlEncoder, this._useSseForDebugProxy, this._useSseForInjectedClient, - this._serveDevTools, this._expressionCompiler, this._injected, this._spawnDds, this._launchDevToolsInNewWindow, this._sdkConfigurationProvider, ) { - _validateDevToolsOptions(); _subs.add(buildResults.listen(_emitBuildResults)); _listen(); if (_extensionBackend != null) { @@ -112,8 +107,9 @@ class DevHandler { Handler get handler => (request) async { final path = request.requestedUri.path; - if (_sseHandlers.containsKey(path)) { - return _sseHandlers[path].handler(request); + final sseHandler = _sseHandlers[path]; + if (sseHandler != null) { + return sseHandler.handler(request); } return Response.notFound(''); }; @@ -141,26 +137,26 @@ class DevHandler { /// Starts a [DebugService] for local debugging. Future _startLocalDebugService( ChromeConnection chromeConnection, AppConnection appConnection) async { - ChromeTab appTab; - ExecutionContext executionContext; - WipConnection tabConnection; + ChromeTab? appTab; + ExecutionContext? executionContext; + WipConnection? tabConnection; final appInstanceId = appConnection.request.instanceId; for (var tab in await chromeConnection.getTabs()) { if (tab.isChromeExtension || tab.isBackgroundPage) continue; - tabConnection = await tab.connect(); + final connection = tabConnection = await tab.connect(); if (_enableLogging) { - tabConnection.onSend.listen((message) { + connection.onSend.listen((message) { _log(' wip', '==> $message'); }); - tabConnection.onReceive.listen((message) { + connection.onReceive.listen((message) { _log(' wip', '<== $message'); }); - tabConnection.onNotification.listen((message) { + connection.onNotification.listen((message) { _log(' wip', '<== $message'); }); } - final contextIds = tabConnection.runtime.onExecutionContextCreated + final contextIds = connection.runtime.onExecutionContextCreated .map((context) => context.id) // There is no way to calculate the number of existing execution // contexts so keep receiving them until there is a 50ms gap after @@ -168,25 +164,25 @@ class DevHandler { .takeUntilGap(const Duration(milliseconds: 50)); // We enqueue this work as we need to begin listening (`.hasNext`) // before events are received. - unawaited(Future.microtask(() => tabConnection.runtime.enable())); + unawaited(Future.microtask(() => connection.runtime.enable())); await for (var contextId in contextIds) { - final result = await tabConnection.sendCommand('Runtime.evaluate', { + final result = await connection.sendCommand('Runtime.evaluate', { 'expression': r'window["$dartAppInstanceId"];', 'contextId': contextId, }); - final evaluatedAppId = result.result['result']['value']; + final evaluatedAppId = result.result?['result']?['value']; if (evaluatedAppId == appInstanceId) { appTab = tab; executionContext = RemoteDebuggerExecutionContext( - contextId, WebkitDebugger(WipDebugger(tabConnection))); + contextId, WebkitDebugger(WipDebugger(connection))); break; } } if (appTab != null) break; - unawaited(tabConnection.close()); + unawaited(connection.close()); } - if (appTab == null) { + if (appTab == null || tabConnection == null || executionContext == null) { throw AppConnectionException( 'Could not connect to application with appInstanceId: ' '$appInstanceId'); @@ -238,26 +234,27 @@ class DevHandler { Future loadAppServices(AppConnection appConnection) async { final appId = appConnection.request.appId; - if (_servicesByAppId[appId] == null) { + var appServices = _servicesByAppId[appId]; + if (appServices == null) { final debugService = await _startLocalDebugService( await _chromeConnection(), appConnection); - final appServices = await _createAppDebugServices( + appServices = await _createAppDebugServices( appConnection.request.appId, debugService); unawaited(appServices.chromeProxyService.remoteDebugger.onClose.first .whenComplete(() async { - await appServices.close(); + await appServices?.close(); _servicesByAppId.remove(appConnection.request.appId); _logger.info('Stopped debug service on ' 'ws://${debugService.hostname}:${debugService.port}\n'); })); _servicesByAppId[appId] = appServices; } - return _servicesByAppId[appId]; + return appServices; } void _handleConnection(SocketConnection injectedConnection) { _injectedConnections.add(injectedConnection); - AppConnection appConnection; + AppConnection? appConnection; injectedConnection.stream.listen((data) async { try { final message = serializers.deserialize(jsonDecode(data)); @@ -270,27 +267,28 @@ class DevHandler { appConnection = await _handleConnectRequest(message, injectedConnection); } else { - if (appConnection == null) { + final connection = appConnection; + if (connection == null) { throw StateError('Not connected to an application.'); } if (message is DevToolsRequest) { - await _handleDebugRequest(appConnection, injectedConnection); + await _handleDebugRequest(connection, injectedConnection); } else if (message is IsolateExit) { - await _handleIsolateExit(appConnection); + await _handleIsolateExit(connection); } else if (message is IsolateStart) { - await _handleIsolateStart(appConnection, injectedConnection); + await _handleIsolateStart(connection, injectedConnection); } else if (message is BatchedDebugEvents) { - await _servicesByAppId[appConnection.request.appId] + await _servicesByAppId[connection.request.appId] ?.chromeProxyService - ?.parseBatchedDebugEvents(message); + .parseBatchedDebugEvents(message); } else if (message is DebugEvent) { - await _servicesByAppId[appConnection.request.appId] + await _servicesByAppId[connection.request.appId] ?.chromeProxyService - ?.parseDebugEvent(message); + .parseDebugEvent(message); } else if (message is RegisterEvent) { - await _servicesByAppId[appConnection.request.appId] + await _servicesByAppId[connection.request.appId] ?.chromeProxyService - ?.parseRegisterEvent(message); + .parseRegisterEvent(message); } } } catch (e, s) { @@ -311,15 +309,15 @@ class DevHandler { unawaited(injectedConnection.sink.done.then((_) async { _injectedConnections.remove(injectedConnection); - if (appConnection != null) { - _appConnectionByAppId.remove(appConnection.request.appId); - final services = _servicesByAppId[appConnection.request.appId]; + final connection = appConnection; + if (connection != null) { + _appConnectionByAppId.remove(connection.request.appId); + final services = _servicesByAppId[connection.request.appId]; if (services != null) { if (services.connectedInstanceId == null || - services.connectedInstanceId == - appConnection.request.instanceId) { + services.connectedInstanceId == connection.request.instanceId) { services.connectedInstanceId = null; - services.chromeProxyService?.destroyIsolate(); + services.chromeProxyService.destroyIsolate(); } } } @@ -386,7 +384,7 @@ class DevHandler { debuggerStart: debuggerStart, devToolsStart: DateTime.now(), ); - if (_serveDevTools) { + if (_devTools != null) { await _launchDevTools( appServices.chromeProxyService.remoteDebugger, _constructDevToolsUri(appServices.debugService.uri, @@ -399,7 +397,7 @@ class DevHandler { // After a page refresh, reconnect to the same app services if they // were previously launched and create the new isolate. final services = _servicesByAppId[message.appId]; - final existingAppConection = _appConnectionByAppId[message.appId]; + final existingConnection = _appConnectionByAppId[message.appId]; final connection = AppConnection(message, sseConnection); // We can take over a connection if there is no connectedInstanceId (this @@ -409,13 +407,13 @@ class DevHandler { // reload). final canReuseConnection = services != null && (services.connectedInstanceId == null || - existingAppConection?.isInKeepAlivePeriod == true); + existingConnection?.isInKeepAlivePeriod == true); if (canReuseConnection) { // Disconnect any old connection (eg. those in the keep-alive waiting // state when reloading the page). - existingAppConection?.shutDown(); - services.chromeProxyService?.destroyIsolate(); + existingConnection?.shutDown(); + services.chromeProxyService.destroyIsolate(); // Reconnect to existing service. services.connectedInstanceId = message.instanceId; @@ -429,14 +427,14 @@ class DevHandler { Future _handleIsolateExit(AppConnection appConnection) async { _servicesByAppId[appConnection.request.appId] ?.chromeProxyService - ?.destroyIsolate(); + .destroyIsolate(); } Future _handleIsolateStart( AppConnection appConnection, SocketConnection sseConnection) async { await _servicesByAppId[appConnection.request.appId] ?.chromeProxyService - ?.createIsolate(appConnection); + .createIsolate(appConnection); } void _listen() async { @@ -481,33 +479,47 @@ class DevHandler { } void _listenForDebugExtension() async { - while (await _extensionBackend.connections.hasNext) { + while (await _extensionBackend!.connections.hasNext) { _startExtensionDebugService(); } } /// Starts a [DebugService] for Dart Debug Extension. void _startExtensionDebugService() async { - final extensionDebugger = await _extensionBackend.extensionDebugger; + final extensionDebugger = await _extensionBackend!.extensionDebugger; // Waits for a `DevToolsRequest` to be sent from the extension background // when the extension is clicked. extensionDebugger.devToolsRequestStream.listen((devToolsRequest) async { - final connection = _appConnectionByAppId[devToolsRequest.appId]; + // TODO(grouma) - Ideally we surface those warnings to the extension so + // that it can be displayed to the user through an alert. + final tabUrl = devToolsRequest.tabUrl; + final appId = devToolsRequest.appId; + if (tabUrl == null) { + _logger.warning('Failed to start extension debug service. ' + 'Missing tab url in DevTools request for app with id: $appId'); + return; + } + final connection = _appConnectionByAppId[appId]; if (connection == null) { - // TODO(grouma) - Ideally we surface this warning to the extension so - // that it can be displayed to the user through an alert. - _logger.warning( - 'Not connected to an app with id: ${devToolsRequest.appId}'); + _logger.warning('Failed to start extension debug service. ' + 'Not connected to an app with id: $appId'); + return; + } + final executionContext = extensionDebugger.executionContext; + if (executionContext == null) { + _logger.warning('Failed to start extension debug service. ' + 'No execution context for app with id: $appId'); return; } + final debuggerStart = DateTime.now(); - final appId = devToolsRequest.appId; - if (_servicesByAppId[appId] == null) { + var appServices = _servicesByAppId[appId]; + if (appServices == null) { final debugService = await DebugService.start( _hostname, extensionDebugger, - extensionDebugger.executionContext, - basePathForServerUri(devToolsRequest.tabUrl), + executionContext, + basePathForServerUri(tabUrl), _assetReader, _loadStrategy, connection, @@ -522,7 +534,7 @@ class DevHandler { spawnDds: _spawnDds, sdkConfigurationProvider: _sdkConfigurationProvider, ); - final appServices = await _createAppDebugServices( + appServices = await _createAppDebugServices( devToolsRequest.appId, debugService, ); @@ -530,25 +542,24 @@ class DevHandler { extensionDebugger.sendEvent('dwds.encodedUri', encodedUri); unawaited(appServices.chromeProxyService.remoteDebugger.onClose.first .whenComplete(() async { - appServices.chromeProxyService.destroyIsolate(); - await appServices.close(); + appServices?.chromeProxyService.destroyIsolate(); + await appServices?.close(); _servicesByAppId.remove(devToolsRequest.appId); _logger.info('Stopped debug service on ' - '${await appServices.debugService.encodedUri}\n'); + '${await appServices?.debugService.encodedUri}\n'); })); extensionDebugConnections.add(DebugConnection(appServices)); _servicesByAppId[appId] = appServices; } - final appServices = _servicesByAppId[appId]; final encodedUri = await appServices.debugService.encodedUri; appServices.dwdsStats.updateLoadTime( debuggerStart: debuggerStart, devToolsStart: DateTime.now()); - if (_serveDevTools) { + if (_devTools != null) { // If we only want the URI, this means we are embedding Dart DevTools in // Chrome DevTools. Therefore return early. - if (devToolsRequest.uriOnly != null && devToolsRequest.uriOnly) { + if (devToolsRequest.uriOnly ?? false) { final devToolsUri = _constructDevToolsUri( encodedUri, ideQueryParam: 'ChromeDevTools', @@ -565,23 +576,19 @@ class DevHandler { }); } - void _ensureServeDevtools() { - if (!_serveDevTools) { - _logger.severe('Expected _serveDevTools'); - throw StateError('Expected _serveDevTools'); - } - } - - void _validateDevToolsOptions() { - if (_serveDevTools && _devTools == null) { - _logger.severe('DevHandler: invalid DevTools options'); - throw StateError('DevHandler: invalid DevTools options'); + DevTools _ensureDevTools() { + final devTools = _devTools; + if (devTools == null) { + throw StateError('DevHandler: DevTools is not available'); } + return devTools; } Future _launchDevTools( RemoteDebugger remoteDebugger, String devToolsUri) async { - _ensureServeDevtools(); + // TODO(annagrin): move checking whether devtools should be started + // and the creation of the uri logic here so it is easier to follow. + _ensureDevTools(); // TODO(grouma) - We may want to log the debugServiceUri if we don't launch // DevTools so that users can manually connect. emitEvent(DwdsEvent.devtoolsLaunch()); @@ -595,11 +602,11 @@ class DevHandler { String debugServiceUri, { String ideQueryParam = '', }) { - _ensureServeDevtools(); + final devTools = _ensureDevTools(); return Uri( scheme: 'http', - host: _devTools.hostname, - port: _devTools.port, + host: devTools.hostname, + port: devTools.port, queryParameters: { 'uri': debugServiceUri, if (ideQueryParam.isNotEmpty) 'ide': ideQueryParam, @@ -621,8 +628,8 @@ extension on Stream { ? StreamController.broadcast(sync: true) : StreamController(sync: true); - StreamSubscription subscription; - Timer gapTimer; + late StreamSubscription subscription; + Timer? gapTimer; controller.onListen = () { subscription = listen((e) { controller.add(e); diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index d6a138cc2..3d40515a8 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -135,35 +135,23 @@ class TestContext { } Future setUp({ - ReloadConfiguration reloadConfiguration, - bool serveDevTools, - bool enableDebugExtension, - bool autoRun, - bool enableDebugging, - bool useSse, - bool spawnDds, - String hostname, - bool waitToDebug, + ReloadConfiguration reloadConfiguration = ReloadConfiguration.none, + bool serveDevTools = false, + bool enableDebugExtension = false, + bool autoRun = true, + bool enableDebugging = true, + bool useSse = true, + bool spawnDds = true, + String hostname = 'localhost', + bool waitToDebug = false, UrlEncoder urlEncoder, - bool restoreBreakpoints, - CompilationMode compilationMode, - NullSafety nullSafety, - bool enableExpressionEvaluation, - bool verboseCompiler, + CompilationMode compilationMode = CompilationMode.buildDaemon, + NullSafety nullSafety = NullSafety.weak, + bool enableExpressionEvaluation = false, + bool verboseCompiler = false, SdkConfigurationProvider sdkConfigurationProvider, }) async { - reloadConfiguration ??= ReloadConfiguration.none; - serveDevTools ??= false; - enableDebugExtension ??= false; - autoRun ??= true; - enableDebugging ??= true; - waitToDebug ??= false; - compilationMode ??= CompilationMode.buildDaemon; - enableExpressionEvaluation ??= false; - spawnDds ??= true; - verboseCompiler ??= false; sdkConfigurationProvider ??= DefaultSdkConfigurationProvider(); - nullSafety ??= NullSafety.weak; try { configureLogWriter(); @@ -349,7 +337,6 @@ class TestContext { enableDebugging, useSse, urlEncoder, - restoreBreakpoints, expressionCompiler, spawnDds, ddcService, diff --git a/dwds/test/fixtures/server.dart b/dwds/test/fixtures/server.dart index bf6bff7d3..9c1464151 100644 --- a/dwds/test/fixtures/server.dart +++ b/dwds/test/fixtures/server.dart @@ -79,7 +79,6 @@ class TestServer { bool enableDebugging, bool useSse, UrlEncoder urlEncoder, - bool restoreBreakpoints, ExpressionCompiler expressionCompiler, bool spawnDds, ExpressionCompilerService ddcService, diff --git a/dwds/test/restore_breakpoints_test.dart b/dwds/test/restore_breakpoints_test.dart index af7b1ea86..4691c8592 100644 --- a/dwds/test/restore_breakpoints_test.dart +++ b/dwds/test/restore_breakpoints_test.dart @@ -24,9 +24,7 @@ WipConnection get tabConnection => context.tabConnection; void main() { setUpAll(() async { setCurrentLogWriter(); - await context.setUp( - restoreBreakpoints: true, - ); + await context.setUp(); }); tearDownAll(() async {