diff --git a/packages/devtools_app/lib/devtools.dart b/packages/devtools_app/lib/devtools.dart index f1829d80be9..4f1907d793e 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 c5a372038e6..6b9b1898ee2 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart @@ -55,6 +55,7 @@ import 'shared/analytics/metrics.dart'; 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 @@ class DevToolsAppState extends State with AutoDisposeMixin { super.initState(); setGlobal(GlobalKey, 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 @@ class DevToolsAppState extends State with AutoDisposeMixin { _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 _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 64c212ac8e9..cb017cdd77e 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 @@ class DisconnectObserverState extends State // 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 8a7a28a9a01..136a65331dd 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 @@ class _ConnectInputState extends State with BlockingActionMixin { 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 404b083238b..e37d503eae3 100644 --- a/packages/devtools_app/lib/src/framework/initializer.dart +++ b/packages/devtools_app/lib/src/framework/initializer.dart @@ -11,7 +11,6 @@ import 'package:flutter/material.dart'; 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 @@ class _InitializerState extends State @override void initState() { super.initState(); - autoDisposeStreamSubscription( - frameworkController.onConnectVmEvent.listen(_connectVm), - ); _timer = Timer(_waitForConnectionTimeout, () { setState(() { @@ -68,18 +64,6 @@ class _InitializerState extends State 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 86d1b37c17b..9f471be572c 100644 --- a/packages/devtools_app/lib/src/shared/routing.dart +++ b/packages/devtools_app/lib/src/shared/routing.dart @@ -52,18 +52,6 @@ class DevToolsRouteInformationParser 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 @@ class DevToolsRouterDelegate extends RouterDelegate ChangeNotifier, PopNavigatorRouterDelegateMixin { DevToolsRouterDelegate(this._getPage, [GlobalKey? key]) - : navigatorKey = key ?? GlobalKey(); + : navigatorKey = key ?? GlobalKey(), + _isTestMode = false; + + @visibleForTesting + DevToolsRouterDelegate.test(this._getPage, [GlobalKey? key]) + : navigatorKey = key ?? GlobalKey(), + _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 navigatorKey; @@ -199,15 +199,10 @@ class DevToolsRouterDelegate extends RouterDelegate 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 @@ class DevToolsRouterDelegate extends RouterDelegate } /// Replaces the navigation stack with a new route. - void _replaceStack(DevToolsRouteConfiguration configuration) { + Future _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 setNewRoutePath(DevToolsRouteConfiguration configuration) { - _replaceStack(configuration); + Future setNewRoutePath(DevToolsRouteConfiguration configuration) async { + await _replaceStack(configuration); notifyListeners(); return SynchronousFuture(null); } @@ -244,7 +253,7 @@ class DevToolsRouterDelegate extends RouterDelegate /// /// Existing arguments (for example &uri=) will be preserved unless /// overwritten by [argUpdates]. - void updateArgsIfChanged(Map argUpdates) { + Future updateArgsIfChanged(Map argUpdates) async { final argsChanged = _changesArgs(argUpdates); if (!argsChanged) { return; @@ -253,7 +262,8 @@ class DevToolsRouterDelegate extends RouterDelegate final currentConfig = currentConfiguration!; final currentPage = currentConfig.page; final newArgs = currentConfig.params.withUpdates(argUpdates); - _replaceStack( + + await _replaceStack( DevToolsRouteConfiguration( currentPage, newArgs, @@ -263,13 +273,13 @@ class DevToolsRouterDelegate extends RouterDelegate notifyListeners(); } - void clearUriParameter() { - updateArgsIfChanged({'uri': null}); + Future clearUriParameter() async { + await updateArgsIfChanged({'uri': null}); } Future replaceState(DevToolsNavigationState state) async { final currentConfig = currentConfiguration!; - _replaceStack( + await _replaceStack( DevToolsRouteConfiguration( currentConfig.page, currentConfig.params, @@ -298,11 +308,13 @@ class DevToolsRouterDelegate extends RouterDelegate } 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 @@ class DevToolsRouterDelegate extends RouterDelegate } return currentState.hasChanges(changes); } + + /// Connects to the VM Service if it is not already connected. + Future _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 1700095c701..ecfa7dab979 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -4,7 +4,7 @@ publish_to: none # 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 972d97eabd0..dd099d84c77 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 @@ To learn more about DevTools, check out the 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 08e08612616..2737b85c5a8 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 'dart:async'; 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 @@ void main() { setGlobal(PreferencesController, PreferencesController()); setGlobal(IdeTheme, getIdeTheme()); setGlobal(GlobalKey, GlobalKey()); + 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 d697da14930..8b497088b91 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 @@ void main() { 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 74b7511b1a0..25f62aa8d78 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 @@ void main() { 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 e38721ea7f5..59630a4c772 100644 --- a/packages/devtools_test/lib/src/helpers/wrappers.dart +++ b/packages/devtools_test/lib/src/helpers/wrappers.dart @@ -39,7 +39,7 @@ Widget wrap(Widget widget, {DevToolsQueryParams? queryParams}) { 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 9fa9f191f20..133ce29b5af 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 @@ MockServiceManager _createMockServiceManagerWithDefaults() { when(mockServiceManager.isolateManager).thenReturn(fakeIsolateManager); when(mockServiceManager.serviceExtensionManager) .thenReturn(fakeServiceExtensionManager); + when(mockServiceManager.connectedState) + .thenReturn(ValueNotifier(const ConnectedState(true))); return mockServiceManager; }