Allow devtools_server to re-use a disconnected DevTools instance (#8009)
diff --git a/packages/devtools_app/lib/devtools.dart b/packages/devtools_app/lib/devtools.dart index f1829d8..4f1907d 100644 --- a/packages/devtools_app/lib/devtools.dart +++ b/packages/devtools_app/lib/devtools.dart
@@ -9,4 +9,4 @@ // the constant declaration `const version =`. // If you change the declaration you must also modify the regex in // tools/update_version.dart. -const version = '2.37.0'; +const version = '2.37.1';
diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index c5a3720..6b9b189 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart
@@ -55,6 +55,7 @@ import 'shared/common_widgets.dart'; import 'shared/console/primitives/simple_items.dart'; import 'shared/feature_flags.dart'; +import 'shared/framework_controller.dart'; import 'shared/globals.dart'; import 'shared/offline_data.dart'; import 'shared/offline_screen.dart'; @@ -140,6 +141,10 @@ super.initState(); setGlobal(GlobalKey<NavigatorState>, routerDelegate.navigatorKey); + autoDisposeStreamSubscription( + frameworkController.onConnectVmEvent.listen(_connectVm), + ); + // TODO(https://github.com/flutter/devtools/issues/6018): Once // https://github.com/flutter/flutter/issues/129692 is fixed, disable the // browser's native context menu on secondary-click, and instead use the @@ -190,6 +195,18 @@ _clearCachedRoutes(); } + /// Connects to the VM with the given URI. + /// + /// This request usually comes from the IDE via the server API to reuse the + /// DevTools window after being disconnected (for example if the user stops + /// a debug session then launches a new one). + Future<void> _connectVm(ConnectVmEvent event) async { + await routerDelegate.updateArgsIfChanged({ + 'uri': event.serviceProtocolUri.toString(), + if (event.notify) 'notify': 'true', + }); + } + /// Gets the page for a given page/path and args. Page _getPage( BuildContext context,
diff --git a/packages/devtools_app/lib/src/framework/disconnect_observer.dart b/packages/devtools_app/lib/src/framework/disconnect_observer.dart index 64c212a..cb017cd 100644 --- a/packages/devtools_app/lib/src/framework/disconnect_observer.dart +++ b/packages/devtools_app/lib/src/framework/disconnect_observer.dart
@@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + import 'package:devtools_app_shared/service.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; @@ -61,7 +63,7 @@ // We became disconnected by means other than a manual disconnect // action, so show the overlay and ensure the 'uri' query paraemter // has been cleared. - widget.routerDelegate.clearUriParameter(); + unawaited(widget.routerDelegate.clearUriParameter()); showDisconnectedOverlay(); } }
diff --git a/packages/devtools_app/lib/src/framework/home_screen.dart b/packages/devtools_app/lib/src/framework/home_screen.dart index 8a7a28a..136a653 100644 --- a/packages/devtools_app/lib/src/framework/home_screen.dart +++ b/packages/devtools_app/lib/src/framework/home_screen.dart
@@ -291,7 +291,7 @@ if (connected) { final connectedUri = Uri.parse(serviceConnection.serviceManager.serviceUri!); - routerDelegate.updateArgsIfChanged({'uri': '$connectedUri'}); + await routerDelegate.updateArgsIfChanged({'uri': '$connectedUri'}); final shortUri = connectedUri.replace(path: ''); notificationService.push('Successfully connected to $shortUri.'); } else if (normalizeVmServiceUri(uri) == null) {
diff --git a/packages/devtools_app/lib/src/framework/initializer.dart b/packages/devtools_app/lib/src/framework/initializer.dart index 404b083..e37d503 100644 --- a/packages/devtools_app/lib/src/framework/initializer.dart +++ b/packages/devtools_app/lib/src/framework/initializer.dart
@@ -11,7 +11,6 @@ import '../shared/analytics/constants.dart' as gac; import '../shared/common_widgets.dart'; import '../shared/connection_info.dart'; -import '../shared/framework_controller.dart'; import '../shared/globals.dart'; import '../shared/routing.dart'; @@ -51,9 +50,6 @@ @override void initState() { super.initState(); - autoDisposeStreamSubscription( - frameworkController.onConnectVmEvent.listen(_connectVm), - ); _timer = Timer(_waitForConnectionTimeout, () { setState(() { @@ -68,18 +64,6 @@ super.dispose(); } - /// Connects to the VM with the given URI. - /// - /// This request usually comes from the IDE via the server API to reuse the - /// DevTools window after being disconnected (for example if the user stops - /// a debug session then launches a new one). - void _connectVm(ConnectVmEvent event) { - DevToolsRouterDelegate.of(context).updateArgsIfChanged({ - 'uri': event.serviceProtocolUri.toString(), - if (event.notify) 'notify': 'true', - }); - } - @override Widget build(BuildContext context) { return ValueListenableBuilder(
diff --git a/packages/devtools_app/lib/src/shared/routing.dart b/packages/devtools_app/lib/src/shared/routing.dart index 86d1b37..9f471be 100644 --- a/packages/devtools_app/lib/src/shared/routing.dart +++ b/packages/devtools_app/lib/src/shared/routing.dart
@@ -52,18 +52,6 @@ uri = uri.copyWith(queryParameters: _testQueryParams!.params); } - final uriFromParams = uri.queryParameters['uri']; - if (uriFromParams == null) { - // If the uri has been modified and we do not have a vm service uri as a - // query parameter, ensure we manually disconnect from any previously - // connected applications. - await serviceConnection.serviceManager.manuallyDisconnect(); - } else if (_testQueryParams == null) { - // Otherwise, connect to the vm service from the query parameter before - // loading the route (but do not do this in a testing environment). - await FrameworkCore.initVmService(serviceUriAsString: uriFromParams); - } - // routeInformation.path comes from the address bar and (when not empty) is // prefixed with a leading slash. Internally we use "page IDs" that do not // start with slashes but match the screenId for each screen. @@ -112,11 +100,23 @@ ChangeNotifier, PopNavigatorRouterDelegateMixin<DevToolsRouteConfiguration> { DevToolsRouterDelegate(this._getPage, [GlobalKey<NavigatorState>? key]) - : navigatorKey = key ?? GlobalKey<NavigatorState>(); + : navigatorKey = key ?? GlobalKey<NavigatorState>(), + _isTestMode = false; + + @visibleForTesting + DevToolsRouterDelegate.test(this._getPage, [GlobalKey<NavigatorState>? key]) + : navigatorKey = key ?? GlobalKey<NavigatorState>(), + _isTestMode = true; static DevToolsRouterDelegate of(BuildContext context) => Router.of(context).routerDelegate as DevToolsRouterDelegate; + /// Whether or not the router is being used for testing purposes. + /// + /// This is to be used in a testing environment only and can be set via the + /// [DevToolsRouterDelegate.test] constructor. + final bool _isTestMode; + @override final GlobalKey<NavigatorState> navigatorKey; @@ -199,15 +199,10 @@ final newParams = currentConfiguration?.params.withUpdates(argUpdates) ?? DevToolsQueryParams.empty(); - // Ensure we disconnect from any previously connected applications if we do - // not have a vm service uri as a query parameter, unless we are loading an - // offline file. - if (page != snapshotScreenId && newParams.vmServiceUri == null) { - unawaited(serviceConnection.serviceManager.manuallyDisconnect()); - } - - _replaceStack( - DevToolsRouteConfiguration(page, newParams, state), + unawaited( + _replaceStack( + DevToolsRouteConfiguration(page, newParams, state), + ), ); notifyListeners(); } @@ -226,16 +221,30 @@ } /// Replaces the navigation stack with a new route. - void _replaceStack(DevToolsRouteConfiguration configuration) { + Future<void> _replaceStack(DevToolsRouteConfiguration configuration) async { _currentPage = configuration.page; _routes ..clear() ..add(configuration); + + if (configuration.page != snapshotScreenId) { + // Handle changing the VM service connection (ignored if we are loading an + // offline file): + final vmServiceUri = configuration.params.vmServiceUri; + + if (vmServiceUri == null) { + // Disconnect from any previously connected applications if we do not + // have a vm service uri as a query parameter. + await serviceConnection.serviceManager.manuallyDisconnect(); + } else { + await _maybeConnectToVmService(vmServiceUri); + } + } } @override - Future<void> setNewRoutePath(DevToolsRouteConfiguration configuration) { - _replaceStack(configuration); + Future<void> setNewRoutePath(DevToolsRouteConfiguration configuration) async { + await _replaceStack(configuration); notifyListeners(); return SynchronousFuture<void>(null); } @@ -244,7 +253,7 @@ /// /// Existing arguments (for example &uri=) will be preserved unless /// overwritten by [argUpdates]. - void updateArgsIfChanged(Map<String, String?> argUpdates) { + Future<void> updateArgsIfChanged(Map<String, String?> argUpdates) async { final argsChanged = _changesArgs(argUpdates); if (!argsChanged) { return; @@ -253,7 +262,8 @@ final currentConfig = currentConfiguration!; final currentPage = currentConfig.page; final newArgs = currentConfig.params.withUpdates(argUpdates); - _replaceStack( + + await _replaceStack( DevToolsRouteConfiguration( currentPage, newArgs, @@ -263,13 +273,13 @@ notifyListeners(); } - void clearUriParameter() { - updateArgsIfChanged({'uri': null}); + Future<void> clearUriParameter() async { + await updateArgsIfChanged({'uri': null}); } Future<void> replaceState(DevToolsNavigationState state) async { final currentConfig = currentConfiguration!; - _replaceStack( + await _replaceStack( DevToolsRouteConfiguration( currentConfig.page, currentConfig.params, @@ -298,11 +308,13 @@ } final currentConfig = currentConfiguration!; - _replaceStack( - DevToolsRouteConfiguration( - currentConfig.page, - currentConfig.params, - currentConfig.state?.merge(stateUpdate) ?? stateUpdate, + unawaited( + _replaceStack( + DevToolsRouteConfiguration( + currentConfig.page, + currentConfig.params, + currentConfig.state?.merge(stateUpdate) ?? stateUpdate, + ), ), ); // Add the new state to the browser history. @@ -328,6 +340,15 @@ } return currentState.hasChanges(changes); } + + /// Connects to the VM Service if it is not already connected. + Future<void> _maybeConnectToVmService(String vmServiceUri) async { + final alreadyConnected = + serviceConnection.serviceManager.connectedState.value.connected; + // Skip connecting if we are already connected or in a test environment. + if (alreadyConnected || _isTestMode) return; + await FrameworkCore.initVmService(serviceUriAsString: vmServiceUri); + } } /// Encapsulates state associated with a [Router] navigation event.
diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 1700095..ecfa7da 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml
@@ -4,7 +4,7 @@ # Note: this version should only be updated by running tools/update_version.dart # that updates all versions of devtools packages (devtools_app, devtools_test). -version: 2.37.0 +version: 2.37.1 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_app
diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 972d97e..dd099d8 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -1,9 +1,9 @@ This is draft for future release notes, that are going to land on [the Flutter website](https://docs.flutter.dev/tools/devtools/release-notes). -# DevTools 2.37.0 release notes +# DevTools 2.37.1 release notes -The 2.37.0 release of the Dart and Flutter DevTools +The 2.37.1 release of the Dart and Flutter DevTools includes the following changes among other general improvements. To learn more about DevTools, check out the [DevTools overview](/tools/devtools/overview). @@ -14,6 +14,8 @@ connected app. - [#7958](https://github.com/flutter/devtools/pull/7958) * Fix a bug where an infinite spinner was shown upon app disconnect. - [#7992](https://github.com/flutter/devtools/pull/7992) +* Fix a bug where trying to reuse a disconnected DevTools instance would +fail. - [#8009](https://github.com/flutter/devtools/pull/8009) ## Inspector updates
diff --git a/packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart b/packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart index 08e0861..2737b85 100644 --- a/packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart +++ b/packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart
@@ -7,10 +7,12 @@ import 'package:devtools_app/src/screens/logging/logging_screen_v2/logging_controller_v2.dart'; import 'package:devtools_app/src/screens/logging/logging_screen_v2/logging_model.dart'; import 'package:devtools_app/src/screens/logging/logging_screen_v2/logging_table_row.dart'; +import 'package:devtools_app/src/service/service_manager.dart'; import 'package:devtools_app/src/shared/globals.dart'; import 'package:devtools_app/src/shared/preferences/preferences.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; +import 'package:devtools_test/devtools_test.dart'; import 'package:devtools_test/helpers.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -26,6 +28,7 @@ setGlobal(PreferencesController, PreferencesController()); setGlobal(IdeTheme, getIdeTheme()); setGlobal(GlobalKey<NavigatorState>, GlobalKey<NavigatorState>()); + setGlobal(ServiceConnectionManager, FakeServiceConnectionManager()); TestWidgetsFlutterBinding.ensureInitialized(); loggingTableModel = LoggingTableModel(); });
diff --git a/packages/devtools_app/test/standalone_ui/vs_code/debug_sessions_test.dart b/packages/devtools_app/test/standalone_ui/vs_code/debug_sessions_test.dart index d697da1..8b49708 100644 --- a/packages/devtools_app/test/standalone_ui/vs_code/debug_sessions_test.dart +++ b/packages/devtools_app/test/standalone_ui/vs_code/debug_sessions_test.dart
@@ -26,6 +26,7 @@ setUpAll(() { // Set test mode so that the debug list of extensions will be used. setTestMode(); + setGlobal(ServiceConnectionManager, FakeServiceConnectionManager()); final devices = stubbedDevices.map((d) => MapEntry(d.id, d)); deviceMap = {for (final d in devices) d.key: d.value};
diff --git a/packages/devtools_app/test/standalone_ui/vs_code/devtools_test.dart b/packages/devtools_app/test/standalone_ui/vs_code/devtools_test.dart index 74b7511..25f62aa 100644 --- a/packages/devtools_app/test/standalone_ui/vs_code/devtools_test.dart +++ b/packages/devtools_app/test/standalone_ui/vs_code/devtools_test.dart
@@ -41,6 +41,7 @@ when(mockEditorClient.supportsHotRestart).thenReturn(true); setGlobal(IdeTheme, IdeTheme()); setGlobal(PreferencesController, PreferencesController()); + setGlobal(ServiceConnectionManager, FakeServiceConnectionManager()); final mockDtdManager = MockDTDManager(); when(
diff --git a/packages/devtools_test/lib/src/helpers/wrappers.dart b/packages/devtools_test/lib/src/helpers/wrappers.dart index e38721e..59630a4 100644 --- a/packages/devtools_test/lib/src/helpers/wrappers.dart +++ b/packages/devtools_test/lib/src/helpers/wrappers.dart
@@ -39,7 +39,7 @@ colorScheme: lightColorScheme, ), ), - routerDelegate: DevToolsRouterDelegate( + routerDelegate: DevToolsRouterDelegate.test( (context, page, args, state) => MaterialPage( child: Material( child: Directionality(
diff --git a/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart b/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart index 9fa9f19..133ce29 100644 --- a/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart +++ b/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart
@@ -174,6 +174,8 @@ when(mockServiceManager.isolateManager).thenReturn(fakeIsolateManager); when(mockServiceManager.serviceExtensionManager) .thenReturn(fakeServiceExtensionManager); + when(mockServiceManager.connectedState) + .thenReturn(ValueNotifier(const ConnectedState(true))); return mockServiceManager; }