From 5310e085b49e3fead1d212c19761bdad4e5a4b65 Mon Sep 17 00:00:00 2001 From: Kenzie Davisson <43759233+kenzieschmoll@users.noreply.github.com> Date: Tue, 12 Mar 2024 12:17:56 -0700 Subject: [PATCH] Add a DevTools server API to notify when a vm service connection changes (#7291) --- .../src/extensions/embedded/_view_web.dart | 5 +- .../lib/src/framework/home_screen.dart | 2 +- .../lib/src/service/service_manager.dart | 17 ++ .../lib/src/shared/framework_controller.dart | 4 +- .../lib/src/shared/server/server.dart | 22 ++ .../devtools_app/lib/src/shared/utils.dart | 2 +- packages/devtools_app_shared/CHANGELOG.md | 1 + .../lib/src/service/service_manager.dart | 20 +- .../connection_ui/_vm_service_connect.dart | 2 +- .../lib/src/template/devtools_extension.dart | 6 +- .../lib/src/template/extension_manager.dart | 14 +- packages/devtools_shared/CHANGELOG.md | 5 + .../devtools_shared/lib/src/devtools_api.dart | 9 + .../lib/src/server/handlers/_dtd.dart | 4 +- .../lib/src/server/handlers/_general.dart | 230 +++++++++++++++++ .../lib/src/server/server_api.dart | 26 +- packages/devtools_shared/pubspec.yaml | 1 + packages/devtools_shared/test/helpers.dart | 131 ++++++++++ .../test/server/dtd_api_test.dart | 2 +- .../test/server/general_api_test.dart | 238 ++++++++++++++++++ .../lib/src/mocks/fake_service_manager.dart | 4 + tool/ci/package_tests.sh | 2 +- 22 files changed, 720 insertions(+), 27 deletions(-) create mode 100644 packages/devtools_shared/lib/src/server/handlers/_general.dart create mode 100644 packages/devtools_shared/test/helpers.dart create mode 100644 packages/devtools_shared/test/server/general_api_test.dart diff --git a/packages/devtools_app/lib/src/extensions/embedded/_view_web.dart b/packages/devtools_app/lib/src/extensions/embedded/_view_web.dart index ce2d4bc2755..4a5d4bca0e3 100644 --- a/packages/devtools_app/lib/src/extensions/embedded/_view_web.dart +++ b/packages/devtools_app/lib/src/extensions/embedded/_view_web.dart @@ -273,8 +273,9 @@ class _ExtensionIFrameController extends DisposableController } break; case DevToolsExtensionEventType.vmServiceConnection: - final service = serviceConnection.serviceManager.service; - updateVmServiceConnection(uri: service?.wsUri); + updateVmServiceConnection( + uri: serviceConnection.serviceManager.serviceUri, + ); break; case DevToolsExtensionEventType.showNotification: _handleShowNotification(event); diff --git a/packages/devtools_app/lib/src/framework/home_screen.dart b/packages/devtools_app/lib/src/framework/home_screen.dart index 052dc5db413..aa49cd1f642 100644 --- a/packages/devtools_app/lib/src/framework/home_screen.dart +++ b/packages/devtools_app/lib/src/framework/home_screen.dart @@ -295,7 +295,7 @@ class _ConnectInputState extends State with BlockingActionMixin { await FrameworkCore.initVmService(serviceUriAsString: uri); if (connected) { final connectedUri = - Uri.parse(serviceConnection.serviceManager.service!.wsUri!); + Uri.parse(serviceConnection.serviceManager.serviceUri!); routerDelegate.updateArgsIfChanged({'uri': '$connectedUri'}); final shortUri = connectedUri.replace(path: ''); notificationService.push('Successfully connected to $shortUri.'); diff --git a/packages/devtools_app/lib/src/service/service_manager.dart b/packages/devtools_app/lib/src/service/service_manager.dart index ae5c068237b..44b8cac1a3d 100644 --- a/packages/devtools_app/lib/src/service/service_manager.dart +++ b/packages/devtools_app/lib/src/service/service_manager.dart @@ -18,6 +18,7 @@ import '../shared/diagnostics/inspector_service.dart'; import '../shared/error_badge_manager.dart'; import '../shared/feature_flags.dart'; import '../shared/globals.dart'; +import '../shared/server/server.dart' as server; import '../shared/title.dart'; import '../shared/utils.dart'; import 'service_registrations.dart' as registrations; @@ -130,6 +131,13 @@ class ServiceConnectionManager { if (debugLogServiceProtocolEvents) { serviceTrafficLogger = VmServiceTrafficLogger(service!); } + + unawaited( + server.notifyForVmServiceConnection( + vmServiceUri: serviceManager.serviceUri!, + connected: true, + ), + ); } void _beforeCloseVmService(VmServiceWrapper? service) { @@ -140,6 +148,15 @@ class ServiceConnectionManager { ? OfflineConnectedApp.parse(serviceManager.connectedApp!.toJson()) : null; offlineController.previousConnectedApp = previousConnectedApp; + + // This must be called before we close the VM service so that + // [serviceManager.serviceUri] is not null. + unawaited( + server.notifyForVmServiceConnection( + vmServiceUri: serviceManager.serviceUri!, + connected: false, + ), + ); } void _afterCloseVmService(VmServiceWrapper? service) { diff --git a/packages/devtools_app/lib/src/shared/framework_controller.dart b/packages/devtools_app/lib/src/shared/framework_controller.dart index f066c77e981..24b4e6cb576 100644 --- a/packages/devtools_app/lib/src/shared/framework_controller.dart +++ b/packages/devtools_app/lib/src/shared/framework_controller.dart @@ -67,9 +67,7 @@ class FrameworkController { final connectionState = serviceConnection.serviceManager.connectedState.value; if (connectionState.connected) { - _connectedController.add( - serviceConnection.serviceManager.service!.wsUri!, - ); + _connectedController.add(serviceConnection.serviceManager.serviceUri!); } else { _disconnectedController.add(null); } diff --git a/packages/devtools_app/lib/src/shared/server/server.dart b/packages/devtools_app/lib/src/shared/server/server.dart index 96010bab653..be6b23f2ca1 100644 --- a/packages/devtools_app/lib/src/shared/server/server.dart +++ b/packages/devtools_app/lib/src/shared/server/server.dart @@ -75,6 +75,26 @@ Future requestFile({ return null; } +Future notifyForVmServiceConnection({ + required String vmServiceUri, + required bool connected, +}) async { + if (isDevToolsServerAvailable) { + final uri = Uri( + path: apiNotifyForVmServiceConnection, + queryParameters: { + apiParameterValueKey: vmServiceUri, + apiParameterVmServiceConnected: connected.toString(), + }, + ); + final resp = await request(uri.toString()); + final statusOk = resp?.statusOk ?? false; + if (!statusOk) { + logWarning(resp, apiNotifyForVmServiceConnection); + } + } +} + DevToolsJsonFile _devToolsJsonFileFromResponse( Response resp, String filePath, @@ -100,4 +120,6 @@ void logWarning(Response? response, String apiType) { extension ResponseExtension on Response { bool get statusOk => statusCode == 200; + bool get statusForbidden => statusCode == 403; + bool get statusError => statusCode == 500; } diff --git a/packages/devtools_app/lib/src/shared/utils.dart b/packages/devtools_app/lib/src/shared/utils.dart index 037c1461726..51c64a2f463 100644 --- a/packages/devtools_app/lib/src/shared/utils.dart +++ b/packages/devtools_app/lib/src/shared/utils.dart @@ -75,7 +75,7 @@ List generateDeviceDescription( ConnectionDescription? vmServiceConnection; if (includeVmServiceConnection && serviceConnection.serviceManager.service != null) { - final description = serviceConnection.serviceManager.service!.wsUri!; + final description = serviceConnection.serviceManager.serviceUri!; vmServiceConnection = ConnectionDescription( title: 'VM Service Connection', description: description, diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md index fd5c32efdd3..396849f8706 100644 --- a/packages/devtools_app_shared/CHANGELOG.md +++ b/packages/devtools_app_shared/CHANGELOG.md @@ -2,6 +2,7 @@ * Remove deprecated `background` and `onBackground` values for `lightColorScheme` and `darkColorScheme`. * Rename `Split` to `SplitPane`. +* Add `ServiceManager.serviceUri` field to store the connected VM service URI. ## 0.0.10 * Add `DTDManager` class and export from `service.dart`. diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index 19a92329030..df81ba36503 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -105,6 +105,13 @@ class ServiceManager { ConnectedApp? connectedApp; T? service; + + /// The URI of the most recent VM service connection [service]. + /// + /// We store this in a local variable so that we still have access to it when + /// the VM service closes. + String? serviceUri; + VM? vm; String? sdkVersion; @@ -216,6 +223,8 @@ class ServiceManager { return; } this.service = service; + serviceUri = service.wsUri!; + if (_serviceAvailable.isCompleted) { _serviceAvailable = Completer(); } @@ -366,16 +375,19 @@ class ServiceManager { void _closeVmServiceConnection() { _serviceAvailable = Completer(); service = null; + serviceUri = null; vm = null; sdkVersion = null; connectedApp = null; } Future manuallyDisconnect() async { - await vmServiceClosed( - connectionState: - const ConnectedState(false, userInitiatedConnectionState: true), - ); + if (hasConnection) { + await vmServiceClosed( + connectionState: + const ConnectedState(false, userInitiatedConnectionState: true), + ); + } } Future callServiceOnMainIsolate(String name) async { diff --git a/packages/devtools_extensions/lib/src/template/_simulated_devtools_environment/connection_ui/_vm_service_connect.dart b/packages/devtools_extensions/lib/src/template/_simulated_devtools_environment/connection_ui/_vm_service_connect.dart index c4720dc6e6c..ed4669fd8f5 100644 --- a/packages/devtools_extensions/lib/src/template/_simulated_devtools_environment/connection_ui/_vm_service_connect.dart +++ b/packages/devtools_extensions/lib/src/template/_simulated_devtools_environment/connection_ui/_vm_service_connect.dart @@ -30,7 +30,7 @@ class VmServiceConnectionDisplay extends StatelessWidget { disconnectedHint: '(e.g., http://127.0.0.1:60851/fH-kAEXc7MQ=/)', onConnect: (value) => simController.updateVmServiceConnection(uri: value), onDisconnect: () => simController.updateVmServiceConnection(uri: null), - currentConnection: () => serviceManager.service!.wsUri ?? '--', + currentConnection: () => serviceManager.serviceUri ?? '--', help: const VmServiceHelp(), ); } diff --git a/packages/devtools_extensions/lib/src/template/devtools_extension.dart b/packages/devtools_extensions/lib/src/template/devtools_extension.dart index 6ab6762847e..84f2ca5e6a4 100644 --- a/packages/devtools_extensions/lib/src/template/devtools_extension.dart +++ b/packages/devtools_extensions/lib/src/template/devtools_extension.dart @@ -138,8 +138,10 @@ class _DevToolsExtensionState extends State void initState() { super.initState(); _initGlobals(); - extensionManager._init( - connectToVmService: widget.requiresRunningApplication, + unawaited( + extensionManager._init( + connectToVmService: widget.requiresRunningApplication, + ), ); for (final handler in widget.eventHandlers.entries) { extensionManager.registerEventHandler(handler.key, handler.value); diff --git a/packages/devtools_extensions/lib/src/template/extension_manager.dart b/packages/devtools_extensions/lib/src/template/extension_manager.dart index 6d391f2162a..ba98f53e485 100644 --- a/packages/devtools_extensions/lib/src/template/extension_manager.dart +++ b/packages/devtools_extensions/lib/src/template/extension_manager.dart @@ -45,7 +45,7 @@ class ExtensionManager { EventListener? _handleMessageListener; // ignore: unused_element, false positive due to part files - void _init({required bool connectToVmService}) { + Future _init({required bool connectToVmService}) async { window.addEventListener( 'message', _handleMessageListener = _handleMessage.toJS, @@ -56,6 +56,11 @@ class ExtensionManager { final themeValue = queryParams[ExtensionEventParameters.theme]; _setThemeForValue(themeValue); + final dtdUri = queryParams[_dtdQueryParameter]; + if (dtdUri != null) { + await _connectToDtd(dtdUri); + } + final vmServiceUri = queryParams[_vmServiceQueryParameter]; if (connectToVmService) { if (vmServiceUri == null) { @@ -71,11 +76,6 @@ class ExtensionManager { unawaited(_connectToVmService(vmServiceUri)); } } - - final dtdUri = queryParams[_dtdQueryParameter]; - if (dtdUri != null) { - unawaited(_connectToDtd(dtdUri)); - } } // ignore: unused_element, false positive due to part files @@ -186,7 +186,7 @@ class ExtensionManager { ); _updateQueryParameter( _vmServiceQueryParameter, - serviceManager.service!.wsUri!, + serviceManager.serviceUri!, ); } catch (e) { final errorMessage = diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md index 877fb46e461..183047bc5f8 100644 --- a/packages/devtools_shared/CHANGELOG.md +++ b/packages/devtools_shared/CHANGELOG.md @@ -1,6 +1,11 @@ # 8.0.0 * **Breaking change:** rename `ServerApi.getCompleted` to `ServerApi.success` and make the `value` parameter optional. +* **Breaking change:** remove the `String? dtdUri` parameter from `ServerApi.handle` and replace +it with a parameter `DTDConnectionInfo? dtd`. +* Introduce a new typedef `DTDConnectionInfo`. +* Add a new API `apiNotifyForVmServiceConnection` that DevTools will call when a +VM service connection is connected or disconnected from the client. * Add a helper method `packageRootFromFileUriString`. * Refactor yaml extension methods. diff --git a/packages/devtools_shared/lib/src/devtools_api.dart b/packages/devtools_shared/lib/src/devtools_api.dart index 0753b211231..9ef6d3357b1 100644 --- a/packages/devtools_shared/lib/src/devtools_api.dart +++ b/packages/devtools_shared/lib/src/devtools_api.dart @@ -5,6 +5,15 @@ /// All server APIs prefix: const apiPrefix = 'api/'; +/// Key used for any request or response to specify a value argument. +const apiParameterValueKey = 'value'; + +/// Notifies the DevTools server when a DevTools app client connects to a new +/// VM service. +const apiNotifyForVmServiceConnection = + '${apiPrefix}notifyForVmServiceConnection'; +const apiParameterVmServiceConnected = 'connected'; + /// Flutter GA properties APIs: const apiGetFlutterGAEnabled = '${apiPrefix}getFlutterGAEnabled'; const apiGetFlutterGAClientId = '${apiPrefix}getFlutterGAClientId'; diff --git a/packages/devtools_shared/lib/src/server/handlers/_dtd.dart b/packages/devtools_shared/lib/src/server/handlers/_dtd.dart index 822d1514462..cfe5c9cbf82 100644 --- a/packages/devtools_shared/lib/src/server/handlers/_dtd.dart +++ b/packages/devtools_shared/lib/src/server/handlers/_dtd.dart @@ -9,10 +9,10 @@ part of '../server_api.dart'; abstract class _DtdApiHandler { static shelf.Response handleGetDtdUri( ServerApi api, - String? dtdUri, + DTDConnectionInfo? dtd, ) { return ServerApi._encodeResponse( - {DtdApi.uriPropertyName: dtdUri}, + {DtdApi.uriPropertyName: dtd?.uri}, api: api, ); } diff --git a/packages/devtools_shared/lib/src/server/handlers/_general.dart b/packages/devtools_shared/lib/src/server/handlers/_general.dart new file mode 100644 index 00000000000..3e9a9dd371a --- /dev/null +++ b/packages/devtools_shared/lib/src/server/handlers/_general.dart @@ -0,0 +1,230 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// ignore_for_file: avoid_classes_with_only_static_members + +part of '../server_api.dart'; + +@visibleForTesting +abstract class Handler { + /// Stores the calculated package roots for VM service connections that are + /// initiated in [handleNotifyForVmServiceConnection]. + /// + /// This map is used to lookup package roots for a particular VM service when + /// [handleNotifyForVmServiceConnection] is called for a VM service disconnect + /// in DevTools app. + /// + /// If the Dart Tooling Daemon was not started by DevTools, this map will + /// never be used. + static final _packageRootsForVmServiceConnections = {}; + + static Future handleNotifyForVmServiceConnection( + ServerApi api, + Map queryParams, + DTDConnectionInfo? dtd, + ) async { + final missingRequiredParams = ServerApi._checkRequiredParameters( + const [apiParameterValueKey, apiParameterVmServiceConnected], + queryParams: queryParams, + api: api, + requestName: apiNotifyForVmServiceConnection, + ); + if (missingRequiredParams != null) return missingRequiredParams; + + final dtdUri = dtd?.uri; + final dtdSecret = dtd?.secret; + if (dtdUri == null || dtdSecret == null) { + // If DevTools server did not start DTD, there is nothing for us to do. + // This assertion may change in the future if the DevTools server has + // other functionality that interacts with VM service connections from + // DevTools app clients. + return api.success(); + } + + final connectedAsString = queryParams[apiParameterVmServiceConnected]!; + late bool connected; + try { + connected = bool.parse(connectedAsString); + } catch (e) { + return api.badRequest( + 'Cannot parse $apiParameterVmServiceConnected parameter:\n$e', + ); + } + + final vmServiceUriAsString = queryParams[apiParameterValueKey]!; + final vmServiceUri = normalizeVmServiceUri(vmServiceUriAsString); + if (vmServiceUri == null) { + return api.badRequest( + 'Cannot normalize VM service URI: $vmServiceUriAsString', + ); + } + + final detectRootResponse = await detectRootPackageForVmService( + vmServiceUriAsString: vmServiceUriAsString, + vmServiceUri: vmServiceUri, + connected: connected, + api: api, + ); + if (detectRootResponse.success) { + final rootUri = detectRootResponse.uri; + if (rootUri == null) { + return api.success(); + } + return updateDtdWorkspaceRoots( + dtd!, + rootFromVmService: rootUri, + connected: connected, + api: api, + ); + } else { + return api.serverError(detectRootResponse.message); + } + } + + @visibleForTesting + static Future detectRootPackageForVmService({ + required String vmServiceUriAsString, + required Uri vmServiceUri, + required bool connected, + required ServerApi api, + }) async { + late Uri rootPackageUri; + if (connected) { + // TODO(kenz): should we first try to lookup the root from + // [_packageRootsForVmServiceConnections]? Could the root library of the + // main isolate change during the lifetime of a VM service instance? + + VmService? vmService; + try { + vmService = await connect( + uri: vmServiceUri, + finishedCompleter: Completer(), + serviceFactory: VmService.defaultFactory, + ); + + final root = await vmService.rootPackageDirectoryForMainIsolate; + if (root == null) { + return ( + success: false, + message: 'No root library found for main isolate ' + '($vmServiceUriAsString).', + uri: null, + ); + } + rootPackageUri = Uri.parse(root); + _packageRootsForVmServiceConnections[vmServiceUriAsString] = + rootPackageUri; + } catch (e) { + return ( + success: false, + message: 'Error detecting project roots ($vmServiceUriAsString)\n$e', + uri: null, + ); + } finally { + await vmService?.dispose(); + vmService = null; + } + } else { + final cachedRootForVmService = + _packageRootsForVmServiceConnections[vmServiceUriAsString]; + if (cachedRootForVmService == null) { + // If there is no root to remove, there is nothing for us to do. + return (success: true, message: null, uri: null); + } + rootPackageUri = cachedRootForVmService; + } + return (success: true, message: null, uri: rootPackageUri); + } + + @visibleForTesting + static Future updateDtdWorkspaceRoots( + DTDConnectionInfo dtd, { + required Uri rootFromVmService, + required bool connected, + required ServerApi api, + }) async { + DTDConnection? dtdConnection; + try { + dtdConnection = await DartToolingDaemon.connect(Uri.parse(dtd.uri!)); + final currentRoots = (await dtdConnection.getIDEWorkspaceRoots()) + .ideWorkspaceRoots + .toSet(); + // Add or remove [rootFromVmService] depending on whether this was a + // connect or disconnect notification. + final newRoots = connected + ? (currentRoots..add(rootFromVmService)).toList() + : (currentRoots..remove(rootFromVmService)).toList(); + await dtdConnection.setIDEWorkspaceRoots(dtd.secret!, newRoots); + return api.success(); + } catch (e) { + return api.serverError('$e'); + } finally { + await dtdConnection?.close(); + } + } +} + +extension on VmService { + Future get rootPackageDirectoryForMainIsolate async { + final fileUriString = await _rootLibraryForMainIsolate; + return fileUriString != null + ? packageRootFromFileUriString(fileUriString) + : null; + } + + Future get _rootLibraryForMainIsolate async { + final mainIsolate = await _detectMainIsolate; + final rootLib = mainIsolate.rootLib?.uri; + if (rootLib == null) return null; + + final fileUriAsString = + (await lookupResolvedPackageUris(mainIsolate.id!, [rootLib])) + .uris + ?.first; + return fileUriAsString; + } + + /// Uses heuristics to detect the main isolate. + /// + /// Assumes an isolate is the main isolate if it meets any of the criteria in + /// the following order: + /// + /// 1. The isolate is the main Flutter isolate. + /// 2. The isolate has ':main(' in its name. + /// 3. The isolate is the first in the list of isolates on the VM. + Future get _detectMainIsolate async { + final isolateRefs = (await getVM()).isolates!; + final isolateCandidates = + await Future.wait<({IsolateRef ref, Isolate isolate})>( + isolateRefs.map( + (ref) async => (ref: ref, isolate: await getIsolate(ref.id!)), + ), + ); + + Isolate? mainIsolate; + for (final isolate in isolateCandidates) { + final isFlutterIsolate = (isolate.isolate.extensionRPCs ?? []) + .any((ext) => ext.startsWith('ext.flutter')); + if (isFlutterIsolate) { + mainIsolate = isolate.isolate; + break; + } + } + mainIsolate ??= isolateCandidates + .firstWhereOrNull((isolate) => isolate.ref.name!.contains(':main(')) + ?.isolate; + + // Fallback to selecting the first isolate in the list. + mainIsolate ??= isolateCandidates.first.isolate; + + return mainIsolate; + } +} + +@visibleForTesting +typedef DetectRootPackageResponse = ({ + bool success, + String? message, + Uri? uri, +}); diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index 150f4814427..33366ca7b46 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -8,13 +8,20 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:collection/collection.dart'; +import 'package:dtd/dtd.dart'; +import 'package:meta/meta.dart'; import 'package:shelf/shelf.dart' as shelf; import 'package:unified_analytics/unified_analytics.dart'; +import 'package:vm_service/vm_service.dart'; import '../deeplink/deeplink_manager.dart'; import '../devtools_api.dart'; import '../extensions/extension_enablement.dart'; import '../extensions/extension_manager.dart'; +import '../service/service.dart'; +import '../service_utils.dart'; +import '../utils/file_utils.dart'; import 'file_system.dart'; import 'usage.dart'; @@ -23,6 +30,10 @@ import 'usage.dart'; part 'handlers/_deeplink.dart'; part 'handlers/_devtools_extensions.dart'; part 'handlers/_dtd.dart'; +part 'handlers/_general.dart'; + +/// Describes an instance of the Dart Tooling Daemon. +typedef DTDConnectionInfo = ({String? uri, String? secret}); /// The DevTools server API. /// @@ -46,13 +57,19 @@ class ServerApi { required DeeplinkManager deeplinkManager, required Analytics analytics, ServerApi? api, - String? dtdUri, + DTDConnectionInfo? dtd, }) { api ??= ServerApi(); final queryParams = request.requestedUri.queryParameters; // TODO(kenz): break this switch statement up so that it uses helper methods // for each case. Also use [_checkRequiredParameters] helper. switch (request.url.path) { + case apiNotifyForVmServiceConnection: + return Handler.handleNotifyForVmServiceConnection( + api, + queryParams, + dtd, + ); // ----- Flutter Tool GA store. ----- case apiGetFlutterGAEnabled: // Is Analytics collection enabled? @@ -266,7 +283,7 @@ class ServerApi { deeplinkManager, ); case DtdApi.apiGetDtdUri: - return _DtdApiHandler.handleGetDtdUri(api, dtdUri); + return _DtdApiHandler.handleGetDtdUri(api, dtd); default: return api.notImplemented(); } @@ -330,6 +347,11 @@ class ServerApi { /// The response optionally contains a single String [value]. shelf.Response success([String? value]) => shelf.Response.ok(value); + /// A [shelf.Response] for API calls that are forbidden for the current state + /// of the server. + shelf.Response forbidden([String? reason]) => + shelf.Response.forbidden(reason); + /// A [shelf.Response] for API calls that encountered a request problem e.g., /// setActiveSurvey not called. /// diff --git a/packages/devtools_shared/pubspec.yaml b/packages/devtools_shared/pubspec.yaml index bd1631a3c29..4ef52407ad6 100644 --- a/packages/devtools_shared/pubspec.yaml +++ b/packages/devtools_shared/pubspec.yaml @@ -11,6 +11,7 @@ environment: dependencies: args: ^2.4.2 collection: ^1.15.0 + dtd: ^1.0.0 extension_discovery: ^2.0.0 meta: ^1.9.1 path: ^1.8.0 diff --git a/packages/devtools_shared/test/helpers.dart b/packages/devtools_shared/test/helpers.dart new file mode 100644 index 00000000000..054d430c108 --- /dev/null +++ b/packages/devtools_shared/test/helpers.dart @@ -0,0 +1,131 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as path; + +typedef TestDtdConnectionInfo = ({ + String? uri, + String? secret, + Process? dtdProcess, +}); + +/// Helper method to start DTD for the purpose of testing. +Future startDtd() async { + const dtdConnectTimeout = Duration(seconds: 10); + + final completer = Completer(); + Process? dtdProcess; + StreamSubscription? dtdStoutSubscription; + + TestDtdConnectionInfo onFailure() => + (uri: null, secret: null, dtdProcess: dtdProcess); + + try { + dtdProcess = await Process.start( + Platform.resolvedExecutable, + ['tooling-daemon', '--machine'], + ); + + dtdStoutSubscription = dtdProcess.stdout.listen((List data) { + try { + final decoded = utf8.decode(data); + final json = jsonDecode(decoded) as Map; + if (json + case { + 'tooling_daemon_details': { + 'uri': final String uri, + 'trusted_client_secret': final String secret, + } + }) { + completer.complete( + (uri: uri, secret: secret, dtdProcess: dtdProcess), + ); + } else { + completer.complete(onFailure()); + } + } catch (e) { + completer.complete(onFailure()); + } + }); + + return completer.future + .timeout(dtdConnectTimeout, onTimeout: onFailure) + .then((value) async { + await dtdStoutSubscription?.cancel(); + return value; + }); + } catch (e) { + await dtdStoutSubscription?.cancel(); + return onFailure(); + } +} + +class TestDartApp { + static final dartVMServiceRegExp = RegExp( + r'The Dart VM service is listening on (http://127.0.0.1:.*)', + ); + + final directory = Directory('tmp/test_app'); + + Process? process; + + Future start() async { + _initTestApp(); + process = await Process.start( + Platform.resolvedExecutable, + ['--observe=0', 'run', 'bin/main.dart'], + workingDirectory: directory.path, + ); + + final serviceUriCompleter = Completer(); + late StreamSubscription sub; + sub = process!.stdout + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen((line) async { + if (line.contains(dartVMServiceRegExp)) { + await sub.cancel(); + serviceUriCompleter.complete( + dartVMServiceRegExp.firstMatch(line)!.group(1), + ); + } + }); + return await serviceUriCompleter.future.timeout( + const Duration(seconds: 5), + onTimeout: () async { + await sub.cancel(); + return ''; + }, + ); + } + + Future kill() async { + process?.kill(); + await process?.exitCode; + process = null; + if (directory.existsSync()) directory.deleteSync(recursive: true); + } + + void _initTestApp() { + if (directory.existsSync()) { + directory.deleteSync(recursive: true); + } + directory.createSync(recursive: true); + + final mainFile = File(path.join(directory.path, 'bin', 'main.dart')) + ..createSync(recursive: true); + mainFile.writeAsStringSync(''' +import 'dart:async'; +void main() async { + for (int i = 0; i < 10000; i++) { + await Future.delayed(const Duration(seconds: 2)); + } +} +'''); + } +} diff --git a/packages/devtools_shared/test/server/dtd_api_test.dart b/packages/devtools_shared/test/server/dtd_api_test.dart index e5b933e0fe6..6928bf27387 100644 --- a/packages/devtools_shared/test/server/dtd_api_test.dart +++ b/packages/devtools_shared/test/server/dtd_api_test.dart @@ -30,7 +30,7 @@ void main() { request, extensionsManager: ExtensionsManager(buildDir: '/'), deeplinkManager: FakeDeeplinkManager(), - dtdUri: dtdUri, + dtd: (uri: dtdUri, secret: null), analytics: const NoOpAnalytics(), ); expect(response.statusCode, HttpStatus.ok); diff --git a/packages/devtools_shared/test/server/general_api_test.dart b/packages/devtools_shared/test/server/general_api_test.dart new file mode 100644 index 00000000000..51159295e87 --- /dev/null +++ b/packages/devtools_shared/test/server/general_api_test.dart @@ -0,0 +1,238 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:devtools_shared/devtools_server.dart'; +import 'package:devtools_shared/devtools_shared.dart'; +import 'package:devtools_shared/devtools_test_utils.dart'; +import 'package:devtools_shared/src/extensions/extension_manager.dart'; +import 'package:devtools_shared/src/server/server_api.dart' as server; +import 'package:dtd/dtd.dart'; +import 'package:shelf/shelf.dart'; +import 'package:test/test.dart'; +import 'package:unified_analytics/unified_analytics.dart'; + +import '../fakes.dart'; +import '../helpers.dart'; + +void main() { + group('General DevTools server API', () { + group(apiNotifyForVmServiceConnection, () { + Future sendNotifyRequest({ + required DTDConnectionInfo dtd, + Map? queryParameters, + }) async { + final request = Request( + 'get', + Uri( + scheme: 'https', + host: 'localhost', + path: apiNotifyForVmServiceConnection, + queryParameters: queryParameters, + ), + ); + return server.ServerApi.handle( + request, + extensionsManager: ExtensionsManager(buildDir: '/'), + deeplinkManager: FakeDeeplinkManager(), + dtd: dtd, + analytics: const NoOpAnalytics(), + ); + } + + test( + 'succeeds when DTD is not available', + () async { + final response = await sendNotifyRequest( + dtd: (uri: null, secret: null), + queryParameters: { + apiParameterValueKey: 'fake_uri', + apiParameterVmServiceConnected: 'true', + }, + ); + expect(response.statusCode, HttpStatus.ok); + expect(await response.readAsString(), isEmpty); + }, + ); + + test( + 'returns badRequest for invalid VM service argument', + () async { + final response = await sendNotifyRequest( + dtd: (uri: 'ws://dtd:uri', secret: 'fake_secret'), + queryParameters: { + apiParameterValueKey: 'fake_uri', + apiParameterVmServiceConnected: 'true', + }, + ); + expect(response.statusCode, HttpStatus.badRequest); + expect( + await response.readAsString(), + contains('Cannot normalize VM service URI'), + ); + }, + ); + test( + 'returns badRequest for invalid $apiParameterVmServiceConnected argument', + () async { + final response = await sendNotifyRequest( + dtd: (uri: 'ws://dtd:uri', secret: 'fake_secret'), + queryParameters: { + apiParameterValueKey: 'ws://127.0.0.1:8181/LEpVqqD7E_Y=/ws', + apiParameterVmServiceConnected: 'bad_arg', + }, + ); + expect(response.statusCode, HttpStatus.badRequest); + expect( + await response.readAsString(), + contains('Cannot parse $apiParameterVmServiceConnected parameter'), + ); + }, + ); + }); + + group('updateDtdWorkspaceRoots', () { + TestDtdConnectionInfo? dtd; + DTDConnection? testDtdConnection; + + setUp(() async { + dtd = await startDtd(); + expect(dtd!.uri, isNotNull, reason: 'Error starting DTD for test'); + testDtdConnection = + await DartToolingDaemon.connect(Uri.parse(dtd!.uri!)); + }); + + tearDown(() async { + await testDtdConnection?.close(); + dtd?.dtdProcess?.kill(); + await dtd?.dtdProcess?.exitCode; + dtd = null; + }); + + Future updateWorkspaceRoots({ + required Uri root, + required bool connected, + }) async { + await server.Handler.updateDtdWorkspaceRoots( + (uri: dtd!.uri, secret: dtd!.secret), + rootFromVmService: root, + connected: connected, + api: ServerApi(), + ); + } + + Future verifyWorkspaceRoots(Set roots) async { + final currentRoots = + (await testDtdConnection!.getIDEWorkspaceRoots()).ideWorkspaceRoots; + expect(currentRoots, hasLength(roots.length)); + expect(currentRoots, containsAll(roots)); + } + + test( + 'adds and removes workspace roots', + () async { + await verifyWorkspaceRoots({}); + final rootUri1 = Uri.parse('file:///Users/me/package_root_1'); + final rootUri2 = Uri.parse('file:///Users/me/package_root_2'); + + await updateWorkspaceRoots(root: rootUri1, connected: true); + await verifyWorkspaceRoots({rootUri1}); + + // Add a second root and verify the roots are unioned. + await updateWorkspaceRoots(root: rootUri2, connected: true); + await verifyWorkspaceRoots({rootUri1, rootUri2}); + + // Verify duplicates cannot be added. + await updateWorkspaceRoots(root: rootUri2, connected: true); + await verifyWorkspaceRoots({rootUri1, rootUri2}); + + // Verify roots are removed for disconnect events. + await updateWorkspaceRoots(root: rootUri2, connected: false); + await verifyWorkspaceRoots({rootUri1}); + await updateWorkspaceRoots(root: rootUri1, connected: false); + await verifyWorkspaceRoots({}); + }, + timeout: const Timeout.factor(4), + ); + }); + + group('detectRootPackageForVmService', () { + TestDartApp? app; + String? vmServiceUriString; + + setUp(() async { + app = TestDartApp(); + vmServiceUriString = await app!.start(); + // Await a short delay to give the VM a chance to initialize. + await delay(duration: const Duration(seconds: 1)); + expect(vmServiceUriString, isNotEmpty); + }); + + tearDown(() async { + await app?.kill(); + app = null; + vmServiceUriString = null; + }); + + test('succeeds for a connect event', () async { + final vmServiceUri = normalizeVmServiceUri(vmServiceUriString!); + expect(vmServiceUri, isNotNull); + final response = await server.Handler.detectRootPackageForVmService( + vmServiceUriAsString: vmServiceUriString!, + vmServiceUri: vmServiceUri!, + connected: true, + api: ServerApi(), + ); + expect(response.success, true); + expect(response.message, isNull); + expect(response.uri, isNotNull); + expect(response.uri!.toString(), endsWith(app!.directory.path)); + }); + + test('succeeds for a disconnect event when cache is empty', () async { + final response = await server.Handler.detectRootPackageForVmService( + vmServiceUriAsString: vmServiceUriString!, + vmServiceUri: Uri.parse('ws://127.0.0.1:63555/fake-uri=/ws'), + connected: false, + api: ServerApi(), + ); + expect(response, (success: true, message: null, uri: null)); + }); + + test( + 'succeeds for a disconnect event when cache contains entry for VM service', + () async { + final vmServiceUri = normalizeVmServiceUri(vmServiceUriString!); + expect(vmServiceUri, isNotNull); + final response = await server.Handler.detectRootPackageForVmService( + vmServiceUriAsString: vmServiceUriString!, + vmServiceUri: vmServiceUri!, + connected: true, + api: ServerApi(), + ); + expect(response.success, true); + expect(response.message, isNull); + expect(response.uri, isNotNull); + expect(response.uri!.toString(), endsWith(app!.directory.path)); + + final disconnectResponse = + await server.Handler.detectRootPackageForVmService( + vmServiceUriAsString: vmServiceUriString!, + vmServiceUri: vmServiceUri, + connected: false, + api: ServerApi(), + ); + expect(disconnectResponse.success, true); + expect(disconnectResponse.message, isNull); + expect(disconnectResponse.uri, isNotNull); + expect( + disconnectResponse.uri!.toString(), + endsWith(app!.directory.path), + ); + }, + ); + }); + }); +} diff --git a/packages/devtools_test/lib/src/mocks/fake_service_manager.dart b/packages/devtools_test/lib/src/mocks/fake_service_manager.dart index 4f3e1bdb005..08939603020 100644 --- a/packages/devtools_test/lib/src/mocks/fake_service_manager.dart +++ b/packages/devtools_test/lib/src/mocks/fake_service_manager.dart @@ -115,6 +115,7 @@ class FakeServiceManager extends Fake serviceExtensionResponses ?? _defaultServiceExtensionResponses, _isolateManager = FakeIsolateManager(rootLibrary: rootLibrary) { this.service = service ?? createFakeService(); + serviceUri = this.service!.wsUri; mockConnectedApp( connectedApp!, isFlutterApp: true, @@ -169,6 +170,9 @@ class FakeServiceManager extends Fake @override VmServiceWrapper? service; + @override + String? serviceUri; + @override VM get vm => _mockVM; final _mockVM = MockVM(); diff --git a/tool/ci/package_tests.sh b/tool/ci/package_tests.sh index 0f0c9b56fd4..214ef30fd84 100755 --- a/tool/ci/package_tests.sh +++ b/tool/ci/package_tests.sh @@ -28,7 +28,7 @@ elif [ "$PACKAGE" = "devtools_shared" ]; then pushd $DEVTOOLS_DIR/packages/devtools_shared echo `pwd` - flutter test test/ + dart test test/ popd fi