Fix infinite spinner on app disconnect. (#7992)
diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index 2fc0a8f..c5a3720 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart
@@ -14,6 +14,7 @@ import 'example/conditional_screen.dart'; import 'extensions/extension_screen.dart'; +import 'framework/disconnect_observer.dart'; import 'framework/framework_core.dart'; import 'framework/home_screen.dart'; import 'framework/initializer.dart'; @@ -198,7 +199,7 @@ ) { // `page` will initially be null while the router is set up, then we will // be called again with an empty string for the root. - if (FrameworkCore.initializationInProgress || page == null) { + if (FrameworkCore.vmServiceInitializationInProgress || page == null) { return const MaterialPage(child: CenteredCircularProgressIndicator()); } @@ -256,7 +257,7 @@ page = queryParams.legacyPage; } - final connectedToVmService = + final paramsContainVmServiceUri = vmServiceUri != null && vmServiceUri.isNotEmpty; Widget scaffoldBuilder() { @@ -328,7 +329,7 @@ actions: isEmbedded() ? [] : [ - if (connectedToVmService) ...[ + if (paramsContainVmServiceUri) ...[ // Hide the hot reload button for Dart web apps, where the // hot reload service extension is not avilable and where the // [service.reloadServices] RPC is not implemented. @@ -354,12 +355,8 @@ ); } - return connectedToVmService - ? Initializer( - url: vmServiceUri, - allowConnectionScreenOnDisconnect: !embedMode.embedded, - builder: (_) => scaffoldBuilder(), - ) + return paramsContainVmServiceUri + ? Initializer(builder: (_) => scaffoldBuilder()) : scaffoldBuilder(); } @@ -433,6 +430,11 @@ theme: ThemeData(useMaterial3: true, colorScheme: darkColorScheme), ), builder: (context, child) { + if (child == null) { + return const CenteredMessage( + 'Uh-oh, something went wrong. Please refresh the page.', + ); + } return MultiProvider( providers: [ Provider<AnalyticsController>.value( @@ -448,7 +450,10 @@ child: NotificationsView( child: ReleaseNotesViewer( controller: releaseNotesController, - child: child, + child: DisconnectObserver( + routerDelegate: routerDelegate, + child: child, + ), ), ), );
diff --git a/packages/devtools_app/lib/src/framework/disconnect_observer.dart b/packages/devtools_app/lib/src/framework/disconnect_observer.dart new file mode 100644 index 0000000..64c212a --- /dev/null +++ b/packages/devtools_app/lib/src/framework/disconnect_observer.dart
@@ -0,0 +1,151 @@ +// 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 'package:devtools_app_shared/service.dart'; +import 'package:devtools_app_shared/ui.dart'; +import 'package:devtools_app_shared/utils.dart'; +import 'package:flutter/material.dart'; + +import '../shared/analytics/analytics.dart' as ga; +import '../shared/analytics/constants.dart' as gac; +import '../shared/config_specific/import_export/import_export.dart'; +import '../shared/connection_info.dart'; +import '../shared/globals.dart'; +import '../shared/query_parameters.dart'; +import '../shared/routing.dart'; +import '../shared/ui/colors.dart'; +import '../shared/utils.dart'; + +class DisconnectObserver extends StatefulWidget { + const DisconnectObserver({ + super.key, + required this.routerDelegate, + required this.child, + }); + + final Widget child; + final DevToolsRouterDelegate routerDelegate; + + @override + State<DisconnectObserver> createState() => DisconnectObserverState(); +} + +class DisconnectObserverState extends State<DisconnectObserver> + with AutoDisposeMixin { + OverlayEntry? currentDisconnectedOverlay; + + late ConnectedState currentConnectionState; + + @override + void initState() { + super.initState(); + + currentConnectionState = + serviceConnection.serviceManager.connectedState.value; + + addAutoDisposeListener(serviceConnection.serviceManager.connectedState, () { + final previousConnectionState = currentConnectionState; + currentConnectionState = + serviceConnection.serviceManager.connectedState.value; + + if (currentConnectionState.connected && + currentDisconnectedOverlay != null) { + setState(() { + hideDisconnectedOverlay(); + }); + } else if (!currentConnectionState.connected) { + if (previousConnectionState.connected && + !currentConnectionState.connected && + !currentConnectionState.userInitiatedConnectionState) { + // 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(); + showDisconnectedOverlay(); + } + } + }); + } + + @override + void dispose() { + hideDisconnectedOverlay(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return widget.child; + } + + void showDisconnectedOverlay() { + if (serviceConnection.serviceManager.connectedState.value.connected || + currentDisconnectedOverlay != null) { + return; + } + WidgetsBinding.instance.scheduleFrameCallback((_) { + ga.select( + gac.devToolsMain, + gac.appDisconnected, + ); + Overlay.of(context).insert(_createDisconnectedOverlay()); + }); + } + + void hideDisconnectedOverlay() { + currentDisconnectedOverlay?.remove(); + currentDisconnectedOverlay = null; + } + + Future<void> _reviewHistory() async { + assert(offlineDataController.offlineDataJson.isNotEmpty); + + offlineDataController.startShowingOfflineData( + offlineApp: offlineDataController.previousConnectedApp!, + ); + hideDisconnectedOverlay(); + final args = <String, String?>{ + DevToolsQueryParams.vmServiceUriKey: null, + DevToolsQueryParams.offlineScreenIdKey: offlineDataController + .offlineDataJson[DevToolsExportKeys.activeScreenId.name] as String, + }; + await widget.routerDelegate.popRoute(); + widget.routerDelegate.navigate(snapshotScreenId, args); + } + + OverlayEntry _createDisconnectedOverlay() { + final theme = Theme.of(context); + currentDisconnectedOverlay = OverlayEntry( + builder: (context) => Container( + color: theme.colorScheme.overlayShadowColor, + child: Center( + child: Column( + children: [ + const Spacer(), + Text('Disconnected', style: theme.textTheme.headlineMedium), + const SizedBox(height: defaultSpacing), + if (!isEmbedded()) + ConnectToNewAppButton( + routerDelegate: widget.routerDelegate, + onPressed: hideDisconnectedOverlay, + gaScreen: gac.devToolsMain, + ) + else + const Text('Run a new debug session to reconnect'), + const Spacer(), + if (offlineDataController.offlineDataJson.isNotEmpty) ...[ + ElevatedButton( + onPressed: _reviewHistory, + child: const Text('Review recent data (offline)'), + ), + const Spacer(), + ], + ], + ), + ), + ), + ); + return currentDisconnectedOverlay!; + } +}
diff --git a/packages/devtools_app/lib/src/framework/framework_core.dart b/packages/devtools_app/lib/src/framework/framework_core.dart index 4565a1f..1d7a420 100644 --- a/packages/devtools_app/lib/src/framework/framework_core.dart +++ b/packages/devtools_app/lib/src/framework/framework_core.dart
@@ -88,15 +88,16 @@ setGlobal(DTDManager, DTDManager()); } - static bool initializationInProgress = false; + static bool vmServiceInitializationInProgress = false; - /// Returns true if we're able to connect to a device and false otherwise. + /// Attempts to initialize a VM service connection and return whether the + /// connection attempt succeeded. static Future<bool> initVmService({ required String serviceUriAsString, ErrorReporter? errorReporter = _defaultErrorReporter, bool logException = true, }) async { - if (serviceConnection.serviceManager.hasConnection) { + if (serviceConnection.serviceManager.connectedState.value.connected) { // TODO(https://github.com/flutter/devtools/issues/1568): why do we call // this multiple times? return true; @@ -104,7 +105,7 @@ final uri = normalizeVmServiceUri(serviceUriAsString); if (uri != null) { - initializationInProgress = true; + vmServiceInitializationInProgress = true; final finishedCompleter = Completer<void>(); try { @@ -145,7 +146,7 @@ errorReporter!('Unable to connect to VM service at $uri: $e', e); return false; } finally { - initializationInProgress = false; + vmServiceInitializationInProgress = false; } } else { // Don't report an error here because we do not have a URI to connect to.
diff --git a/packages/devtools_app/lib/src/framework/home_screen.dart b/packages/devtools_app/lib/src/framework/home_screen.dart index 229a738..8a7a28a 100644 --- a/packages/devtools_app/lib/src/framework/home_screen.dart +++ b/packages/devtools_app/lib/src/framework/home_screen.dart
@@ -62,8 +62,9 @@ @override Widget build(BuildContext context) { - final connected = serviceConnection.serviceManager.hasConnection && - serviceConnection.serviceManager.connectedAppInitialized; + final connected = + serviceConnection.serviceManager.connectedState.value.connected && + serviceConnection.serviceManager.connectedAppInitialized; return Scrollbar( child: ListView( children: [ @@ -102,6 +103,9 @@ gaScreen: gac.home, minScreenWidthForTextBeforeScaling: _primaryMinScreenWidthForTextBeforeScaling, + routerDelegate: DevToolsRouterDelegate.of(context), + onPressed: () => + Navigator.of(context, rootNavigator: true).pop('dialog'), ), ], child: const ConnectedAppSummary(narrowView: false),
diff --git a/packages/devtools_app/lib/src/framework/initializer.dart b/packages/devtools_app/lib/src/framework/initializer.dart index ace1275..404b083 100644 --- a/packages/devtools_app/lib/src/framework/initializer.dart +++ b/packages/devtools_app/lib/src/framework/initializer.dart
@@ -7,21 +7,17 @@ import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; import 'package:flutter/material.dart'; -import 'package:logging/logging.dart'; -import '../shared/analytics/analytics.dart' as ga; import '../shared/analytics/constants.dart' as gac; import '../shared/common_widgets.dart'; -import '../shared/config_specific/import_export/import_export.dart'; +import '../shared/connection_info.dart'; import '../shared/framework_controller.dart'; import '../shared/globals.dart'; -import '../shared/primitives/utils.dart'; -import '../shared/query_parameters.dart'; import '../shared/routing.dart'; -import '../shared/ui/colors.dart'; -import 'framework_core.dart'; -final _log = Logger('initializer'); +// TODO(kenz): see if we can simplify this widget. Several things have changed +// with DevTools routing and service management code since this widget was +// originally written. /// Widget that requires business logic to be loaded before building its /// [builder]. @@ -33,87 +29,50 @@ /// connected. As we require additional services to be available, add them /// here. class Initializer extends StatefulWidget { - const Initializer({ - super.key, - required this.url, - required this.builder, - this.allowConnectionScreenOnDisconnect = true, - }); + const Initializer({super.key, required this.builder}); /// The builder for the widget's children. /// - /// Will only be built if [_InitializerState._checkLoaded] is true. + /// Will only be built when a connection to an app is established. final WidgetBuilder builder; - /// The url to attempt to load a vm service from. - /// - /// If null, the app will navigate to the [ConnectScreen]. - final String? url; - - /// Whether to allow navigating to the connection screen upon disconnect. - final bool allowConnectionScreenOnDisconnect; - @override State<Initializer> createState() => _InitializerState(); } class _InitializerState extends State<Initializer> with SingleTickerProviderStateMixin, AutoDisposeMixin { - /// Checks if the [service.serviceManager] is connected. - /// - /// This is a method and not a getter to communicate that its value may - /// change between successive calls. - bool _checkLoaded() => serviceConnection.serviceManager.hasConnection; + static const _waitForConnectionTimeout = Duration(seconds: 2); - OverlayEntry? currentDisconnectedOverlay; + Timer? _timer; + + bool _showConnectToNewAppButton = false; @override void initState() { super.initState(); - autoDisposeStreamSubscription( frameworkController.onConnectVmEvent.listen(_connectVm), ); - // If we become disconnected by means other than a manual disconnect action, - // attempt to reconnect. - addAutoDisposeListener(serviceConnection.serviceManager.connectedState, () { - final connectionState = - serviceConnection.serviceManager.connectedState.value; - if (connectionState.connected) { - setState(() {}); - } else if (!connectionState.userInitiatedConnectionState) { - // Try to reconnect (otherwise, will fall back to showing the - // disconnected overlay). - unawaited( - _attemptUrlConnection( - logException: false, - errorReporter: (_, __) { - _log.warning( - 'Attempted to reconnect to the application, but failed.', - ); - }, - ), - ); - } + _timer = Timer(_waitForConnectionTimeout, () { + setState(() { + _showConnectToNewAppButton = true; + }); }); - - unawaited(_attemptUrlConnection()); } @override - void didUpdateWidget(Initializer oldWidget) { - super.didUpdateWidget(oldWidget); - - // Handle widget rebuild when the URL has changed. - if (widget.url != null && widget.url != oldWidget.url) { - unawaited(_attemptUrlConnection()); - } + void dispose() { + _timer?.cancel(); + 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). + /// 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(), @@ -121,126 +80,36 @@ }); } - Future<void> _attemptUrlConnection({ - ErrorReporter? errorReporter, - bool logException = true, - }) async { - if (widget.url == null) { - _handleNoConnection(); - return; - } - - final connected = await FrameworkCore.initVmService( - serviceUriAsString: widget.url!, - errorReporter: errorReporter, - logException: logException, - ); - - if (!connected) { - _handleNoConnection(); - } - } - - /// Shows a "disconnected" overlay if the [service.serviceManager] is not currently connected. - void _handleNoConnection() { - WidgetsBinding.instance.scheduleFrameCallback((_) { - if (!_checkLoaded() && - ModalRoute.of(context)!.isCurrent && - currentDisconnectedOverlay == null) { - ga.select( - gac.devToolsMain, - gac.appDisconnected, - ); - Overlay.of(context).insert(_createDisconnectedOverlay()); - - addAutoDisposeListener( - serviceConnection.serviceManager.connectedState, - () { - final connectedState = - serviceConnection.serviceManager.connectedState.value; - if (connectedState.connected) { - // Hide the overlay if we become reconnected. - hideDisconnectedOverlay(); - } - }, - ); - } - }); - } - - void hideDisconnectedOverlay() { - setState(() { - currentDisconnectedOverlay?.remove(); - currentDisconnectedOverlay = null; - }); - } - - void _reviewHistory() { - assert(offlineDataController.offlineDataJson.isNotEmpty); - - offlineDataController.startShowingOfflineData( - offlineApp: offlineDataController.previousConnectedApp!, - ); - hideDisconnectedOverlay(); - final args = <String, String?>{ - DevToolsQueryParams.vmServiceUriKey: null, - DevToolsQueryParams.offlineScreenIdKey: offlineDataController - .offlineDataJson[DevToolsExportKeys.activeScreenId.name] as String, - }; - final routerDelegate = DevToolsRouterDelegate.of(context); - Router.neglect( - context, - () => routerDelegate.navigate(snapshotScreenId, args), - ); - } - - OverlayEntry _createDisconnectedOverlay() { - final theme = Theme.of(context); - currentDisconnectedOverlay = OverlayEntry( - builder: (context) => Container( - color: theme.colorScheme.overlayShadowColor, - child: Center( - child: Column( - children: [ - const Spacer(), - Text('Disconnected', style: theme.textTheme.headlineMedium), - const SizedBox(height: defaultSpacing), - if (widget.allowConnectionScreenOnDisconnect) - ElevatedButton( - onPressed: () { - hideDisconnectedOverlay(); - DevToolsRouterDelegate.of(context).navigateHome( - clearUriParam: true, - clearScreenParam: true, - ); - }, - child: const Text(connectToNewAppText), - ) - else - const Text('Run a new debug session to reconnect'), - const Spacer(), - if (offlineDataController.offlineDataJson.isNotEmpty) - ElevatedButton( - onPressed: _reviewHistory, - child: const Text('Review recent data (offline)'), - ), - const Spacer(), - ], - ), - ), - ), - ); - return currentDisconnectedOverlay!; - } - @override Widget build(BuildContext context) { - return _checkLoaded() || offlineDataController.showingOfflineData.value - ? widget.builder(context) - : Scaffold( - body: currentDisconnectedOverlay != null - ? const SizedBox() - : const CenteredCircularProgressIndicator(), - ); + return ValueListenableBuilder( + valueListenable: serviceConnection.serviceManager.connectedState, + builder: (context, connectedState, _) { + if (connectedState.connected || + offlineDataController.showingOfflineData.value) { + return widget.builder(context); + } + // TODO(kenz): this should be more sophisticated logic. + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + const Spacer(), + CenteredMessage( + _showConnectToNewAppButton + ? 'Cannot connect to VM service.' + : 'Waiting for VM service connection...', + ), + if (_showConnectToNewAppButton) ...[ + const SizedBox(height: defaultSpacing), + ConnectToNewAppButton( + routerDelegate: DevToolsRouterDelegate.of(context), + gaScreen: gac.devToolsMain, + ), + ], + const Spacer(), + ], + ); + }, + ); } }
diff --git a/packages/devtools_app/lib/src/framework/scaffold.dart b/packages/devtools_app/lib/src/framework/scaffold.dart index 3db8174..cd866e4 100644 --- a/packages/devtools_app/lib/src/framework/scaffold.dart +++ b/packages/devtools_app/lib/src/framework/scaffold.dart
@@ -370,7 +370,8 @@ bottomNavigationBar: StatusLine( currentScreen: _currentScreen, isEmbedded: widget.embedMode.embedded, - isConnected: serviceConnection.serviceManager.hasConnection && + isConnected: serviceConnection + .serviceManager.connectedState.value.connected && serviceConnection.serviceManager.connectedAppInitialized, ), ),
diff --git a/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_controller.dart b/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_controller.dart index 10aa74f..2af7d1d 100644 --- a/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_controller.dart +++ b/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_controller.dart
@@ -308,7 +308,7 @@ } Future<String?> packageDirectoryForMainIsolate() async { - if (!serviceConnection.serviceManager.hasConnection) { + if (!serviceConnection.serviceManager.connectedState.value.connected) { return null; } final packageUriString = await serviceConnection.serviceManager
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_screen.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_screen.dart index b72e4ac..f496e32 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_screen.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_screen.dart
@@ -129,7 +129,7 @@ } }); addAutoDisposeListener(preferences.inspector.pubRootDirectories, () { - if (serviceConnection.serviceManager.hasConnection && + if (serviceConnection.serviceManager.connectedState.value.connected && controller.firstInspectorTreeLoadCompleted) { _refreshInspector(); }
diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen.dart index ccb54af..b84574c 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen.dart
@@ -129,7 +129,7 @@ } }); addAutoDisposeListener(preferences.inspectorV2.pubRootDirectories, () { - if (serviceConnection.serviceManager.hasConnection && + if (serviceConnection.serviceManager.connectedState.value.connected && controller.firstInspectorTreeLoadCompleted) { _refreshInspector(); }
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/chart/controller/memory_tracker.dart b/packages/devtools_app/lib/src/screens/memory/panes/chart/controller/memory_tracker.dart index cbbf4c0..aad2b2b 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/chart/controller/memory_tracker.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/chart/controller/memory_tracker.dart
@@ -102,7 +102,8 @@ // Polls for current Android meminfo using: // > adb shell dumpsys meminfo -d <package_name> - _adbMemoryInfo = serviceConnection.serviceManager.hasConnection && + _adbMemoryInfo = serviceConnection + .serviceManager.connectedState.value.connected && serviceConnection.serviceManager.vm!.operatingSystem == 'android' && isAndroidChartVisible.value ? await _fetchAdbInfo()
diff --git a/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart b/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart index 89df4e9..0b21a73 100644 --- a/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart +++ b/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart
@@ -66,7 +66,7 @@ // because we can't use targetOrigin in postMessage as only the scheme is fixed // for VS Code (vscode-webview://[some guid]). if (globals.containsKey(ServiceConnectionManager) && - !serviceConnection.serviceManager.hasConnection) { + !serviceConnection.serviceManager.connectedState.value.connected) { return; } if (!window.navigator.userAgent.contains('Electron')) return;
diff --git a/packages/devtools_app/lib/src/shared/connection_info.dart b/packages/devtools_app/lib/src/shared/connection_info.dart index 3480292..bd1a348 100644 --- a/packages/devtools_app/lib/src/shared/connection_info.dart +++ b/packages/devtools_app/lib/src/shared/connection_info.dart
@@ -95,8 +95,10 @@ const ConnectToNewAppButton({ super.key, required this.gaScreen, + required this.routerDelegate, this.elevated = false, this.minScreenWidthForTextBeforeScaling, + this.onPressed, }); final String gaScreen; @@ -105,6 +107,10 @@ final double? minScreenWidthForTextBeforeScaling; + final DevToolsRouterDelegate routerDelegate; + + final VoidCallback? onPressed; + @override Widget build(BuildContext context) { return GaDevToolsButton( @@ -115,11 +121,11 @@ gaSelection: gac.HomeScreenEvents.connectToNewApp.name, minScreenWidthForTextBeforeScaling: minScreenWidthForTextBeforeScaling, onPressed: () { - DevToolsRouterDelegate.of(context).navigateHome( + routerDelegate.navigateHome( clearUriParam: true, clearScreenParam: true, ); - Navigator.of(context, rootNavigator: true).pop('dialog'); + onPressed?.call(); }, ); }
diff --git a/packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart b/packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart index b51ca15..d1d036d 100644 --- a/packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart +++ b/packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart
@@ -304,7 +304,9 @@ (element) => RegExp('^[/\\s]*\$').firstMatch(element) != null, ); - if (!serviceConnection.serviceManager.hasConnection) return; + if (!serviceConnection.serviceManager.connectedState.value.connected) { + return; + } await _pubRootDirectoryBusyTracker(() async { final localInspectorService = _inspectorService; if (localInspectorService is! InspectorService) return; @@ -320,7 +322,9 @@ Future<void> removePubRootDirectories( List<String> pubRootDirectories, ) async { - if (!serviceConnection.serviceManager.hasConnection) return; + if (!serviceConnection.serviceManager.connectedState.value.connected) { + return; + } await _pubRootDirectoryBusyTracker(() async { final localInspectorService = _inspectorService; if (localInspectorService is! InspectorService) return;
diff --git a/packages/devtools_app/lib/src/shared/preferences/_inspector_v2_preferences.dart b/packages/devtools_app/lib/src/shared/preferences/_inspector_v2_preferences.dart index ccb0b1f..0229a86 100644 --- a/packages/devtools_app/lib/src/shared/preferences/_inspector_v2_preferences.dart +++ b/packages/devtools_app/lib/src/shared/preferences/_inspector_v2_preferences.dart
@@ -304,7 +304,9 @@ (element) => RegExp('^[/\\s]*\$').firstMatch(element) != null, ); - if (!serviceConnection.serviceManager.hasConnection) return; + if (!serviceConnection.serviceManager.connectedState.value.connected) { + return; + } await _pubRootDirectoryBusyTracker(() async { final localInspectorService = _inspectorService; if (localInspectorService is! InspectorService) return; @@ -320,7 +322,9 @@ Future<void> removePubRootDirectories( List<String> pubRootDirectories, ) async { - if (!serviceConnection.serviceManager.hasConnection) return; + if (!serviceConnection.serviceManager.connectedState.value.connected) { + return; + } await _pubRootDirectoryBusyTracker(() async { final localInspectorService = _inspectorService; if (localInspectorService is! InspectorService) return;
diff --git a/packages/devtools_app/lib/src/shared/routing.dart b/packages/devtools_app/lib/src/shared/routing.dart index c04f60e..86d1b37 100644 --- a/packages/devtools_app/lib/src/shared/routing.dart +++ b/packages/devtools_app/lib/src/shared/routing.dart
@@ -244,7 +244,7 @@ /// /// Existing arguments (for example &uri=) will be preserved unless /// overwritten by [argUpdates]. - void updateArgsIfChanged(Map<String, String> argUpdates) { + void updateArgsIfChanged(Map<String, String?> argUpdates) { final argsChanged = _changesArgs(argUpdates); if (!argsChanged) { return; @@ -263,6 +263,10 @@ notifyListeners(); } + void clearUriParameter() { + updateArgsIfChanged({'uri': null}); + } + Future<void> replaceState(DevToolsNavigationState state) async { final currentConfig = currentConfiguration!; _replaceStack(
diff --git a/packages/devtools_app/lib/src/shared/screen.dart b/packages/devtools_app/lib/src/shared/screen.dart index 2a31902..3e17e38 100644 --- a/packages/devtools_app/lib/src/shared/screen.dart +++ b/packages/devtools_app/lib/src/shared/screen.dart
@@ -415,8 +415,9 @@ /// [Screen] subclass. Widget build(BuildContext context) { if (!requiresConnection) { - final connected = serviceConnection.serviceManager.hasConnection && - serviceConnection.serviceManager.connectedAppInitialized; + final connected = + serviceConnection.serviceManager.connectedState.value.connected && + serviceConnection.serviceManager.connectedAppInitialized; // Do not use the disconnected body in offline mode, because the default // [buildScreenBody] should be used for offline states. if (!connected && !offlineDataController.showingOfflineData.value) {
diff --git a/packages/devtools_app/lib/src/shared/utils.dart b/packages/devtools_app/lib/src/shared/utils.dart index d10d99c..f87f315 100644 --- a/packages/devtools_app/lib/src/shared/utils.dart +++ b/packages/devtools_app/lib/src/shared/utils.dart
@@ -290,7 +290,7 @@ ControllerCreationMode get devToolsMode { return offlineDataController.showingOfflineData.value ? ControllerCreationMode.offlineData - : serviceConnection.serviceManager.hasConnection + : serviceConnection.serviceManager.connectedState.value.connected ? ControllerCreationMode.connected : ControllerCreationMode.disconnected; }
diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 522fec9..972d97e 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -12,6 +12,8 @@ * Improve messaging when a screen is unavailable for the platform of 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) ## Inspector updates
diff --git a/packages/devtools_app/test/shared/disconnect_observer_test.dart b/packages/devtools_app/test/shared/disconnect_observer_test.dart new file mode 100644 index 0000000..c58869b --- /dev/null +++ b/packages/devtools_app/test/shared/disconnect_observer_test.dart
@@ -0,0 +1,147 @@ +// Copyright 2019 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 'package:devtools_app/devtools_app.dart'; +import 'package:devtools_app/src/framework/disconnect_observer.dart'; +import 'package:devtools_app/src/shared/framework_controller.dart'; +import 'package:devtools_app_shared/shared.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/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('DisconnectObserver', () { + late FakeServiceConnectionManager fakeServiceConnectionManager; + + setUp(() { + fakeServiceConnectionManager = FakeServiceConnectionManager(); + setGlobal(ServiceConnectionManager, fakeServiceConnectionManager); + setGlobal(FrameworkController, FrameworkController()); + setGlobal(OfflineDataController, OfflineDataController()); + setGlobal(IdeTheme, IdeTheme()); + }); + + Future<void> pumpDisconnectObserver(WidgetTester tester) async { + await tester.pumpWidget( + wrap( + Builder( + builder: (context) { + return DisconnectObserver( + routerDelegate: DevToolsRouterDelegate.of(context), + child: const Placeholder(), + ); + }, + ), + ), + ); + await tester.pumpAndSettle(); + } + + void verifyObserverState( + WidgetTester tester, { + required bool connected, + required bool showingOverlay, + }) { + final DisconnectObserverState state = + tester.state(find.byType(DisconnectObserver)); + expect(state.currentConnectionState.connected, connected); + expect( + state.currentDisconnectedOverlay, + showingOverlay ? isNotNull : isNull, + ); + expect( + find.text('Disconnected'), + showingOverlay ? findsOneWidget : findsNothing, + ); + expect( + find.byType(ConnectToNewAppButton), + showingOverlay && !isEmbedded() ? findsOneWidget : findsNothing, + ); + expect( + find.text('Run a new debug session to reconnect'), + showingOverlay && isEmbedded() ? findsOneWidget : findsNothing, + ); + expect( + find.text('Review recent data (offline)'), + showingOverlay && offlineDataController.offlineDataJson.isNotEmpty + ? findsOneWidget + : findsNothing, + ); + } + + testWidgets( + 'initiailized in a disconnected state', + (WidgetTester tester) async { + fakeServiceConnectionManager.serviceManager.setConnectedState(false); + await pumpDisconnectObserver(tester); + verifyObserverState(tester, connected: false, showingOverlay: false); + }, + ); + + testWidgets( + 'initiailized in a connected state', + (WidgetTester tester) async { + await pumpDisconnectObserver(tester); + verifyObserverState(tester, connected: true, showingOverlay: false); + }, + ); + + testWidgets( + 'handles connection changes', + (WidgetTester tester) async { + fakeServiceConnectionManager.serviceManager.setConnectedState(false); + await pumpDisconnectObserver(tester); + verifyObserverState(tester, connected: false, showingOverlay: false); + + // Establish a connection. + fakeServiceConnectionManager.serviceManager.setConnectedState(true); + await tester.pumpAndSettle(); + verifyObserverState(tester, connected: true, showingOverlay: false); + + // Trigger a disconnect. + fakeServiceConnectionManager.serviceManager.setConnectedState(false); + await tester.pumpAndSettle(); + verifyObserverState(tester, connected: false, showingOverlay: true); + + // Trigger a reconnect. + fakeServiceConnectionManager.serviceManager.setConnectedState(true); + await tester.pumpAndSettle(); + verifyObserverState(tester, connected: true, showingOverlay: false); + }, + ); + + group('disconnected overlay', () { + Future<void> showOverlayAndVerifyContents(WidgetTester tester) async { + await pumpDisconnectObserver(tester); + verifyObserverState(tester, connected: true, showingOverlay: false); + fakeServiceConnectionManager.serviceManager.setConnectedState(false); + await tester.pumpAndSettle(); + verifyObserverState(tester, connected: false, showingOverlay: true); + } + + testWidgets( + 'builds for embedded mode', + (WidgetTester tester) async { + setGlobal(IdeTheme, IdeTheme(embedMode: EmbedMode.embedOne)); + await showOverlayAndVerifyContents(tester); + }, + ); + + testWidgets( + 'builds for reviewing history', + (WidgetTester tester) async { + offlineDataController.offlineDataJson = {'foo': 'bar'}; + await showOverlayAndVerifyContents(tester); + }, + ); + }); + + // TODO(kenz): test navigation that occurs by clicking on buttons. This will + // require either modifying the test wrappers to take a set of routes or + // writing an integration test for this user journey. + }); +}
diff --git a/packages/devtools_app/test/shared/home_screen_test.dart b/packages/devtools_app/test/shared/home_screen_test.dart index 880b0c6..0ca97f3 100644 --- a/packages/devtools_app/test/shared/home_screen_test.dart +++ b/packages/devtools_app/test/shared/home_screen_test.dart
@@ -22,7 +22,7 @@ fakeServiceConnection = FakeServiceConnectionManager(), ); setGlobal(IdeTheme, IdeTheme()); - fakeServiceConnection.serviceManager.hasConnection = false; + fakeServiceConnection.serviceManager.setConnectedState(false); }); testWidgetsWithWindowSize(
diff --git a/packages/devtools_app/test/shared/initializer_test.dart b/packages/devtools_app/test/shared/initializer_test.dart index 1bbcb81..8b2c3d2 100644 --- a/packages/devtools_app/test/shared/initializer_test.dart +++ b/packages/devtools_app/test/shared/initializer_test.dart
@@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -@TestOn('vm') -library; - import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/framework/initializer.dart'; import 'package:devtools_app/src/shared/framework_controller.dart'; @@ -19,11 +16,17 @@ void main() { group('Initializer', () { const initializedKey = Key('initialized'); + const waitingText = 'Waiting for VM service connection...'; + const cannotConnectText = 'Cannot connect to VM service.'; + + late FakeServiceConnectionManager fakeServiceConnectionManager; + setUp(() { - final serviceManager = FakeServiceConnectionManager(); - when(serviceManager.serviceManager.connectedApp!.isDartWebApp) - .thenAnswer((_) => Future.value(false)); - setGlobal(ServiceConnectionManager, serviceManager); + fakeServiceConnectionManager = FakeServiceConnectionManager(); + when( + fakeServiceConnectionManager.serviceManager.connectedApp!.isDartWebApp, + ).thenAnswer((_) => Future.value(false)); + setGlobal(ServiceConnectionManager, fakeServiceConnectionManager); setGlobal(FrameworkController, FrameworkController()); setGlobal(OfflineDataController, OfflineDataController()); setGlobal(IdeTheme, IdeTheme()); @@ -33,7 +36,6 @@ await tester.pumpWidget( wrap( Initializer( - url: null, builder: (_) => const SizedBox(key: initializedKey), ), ), @@ -41,64 +43,73 @@ await tester.pump(); } + Future<void> advanceTimer(WidgetTester tester) async { + // Wait a short delay to let the initializer timer advance. + await tester.pumpAndSettle(const Duration(seconds: 3)); + } + testWidgets( - 'shows disconnected overlay if not connected', + 'immediately calls builder when connection exists', (WidgetTester tester) async { - setGlobal( - ServiceConnectionManager, - FakeServiceConnectionManager( - hasConnection: false, - ), - ); await pumpInitializer(tester); - expect(find.text('Disconnected'), findsOneWidget); + expect(find.text(waitingText), findsNothing); + expect(find.text(cannotConnectText), findsNothing); + expect(find.byType(ConnectToNewAppButton), findsNothing); + expect(find.byKey(initializedKey), findsOneWidget); + + // Verify expectations are still true after the timer advances. + await advanceTimer(tester); + expect(find.text(waitingText), findsNothing); + expect(find.text(cannotConnectText), findsNothing); + expect(find.byType(ConnectToNewAppButton), findsNothing); + expect(find.byKey(initializedKey), findsOneWidget); }, ); testWidgets( - 'shows disconnected overlay upon disconnect', + 'calls builder late if connection is established late', (WidgetTester tester) async { - final serviceConnection = FakeServiceConnectionManager(); - setGlobal(ServiceConnectionManager, serviceConnection); + fakeServiceConnectionManager.serviceManager.setConnectedState(false); - // Expect standard connected state. - serviceConnection.serviceManager.changeState(true); await pumpInitializer(tester); + expect(find.text(waitingText), findsOneWidget); + expect(find.text(cannotConnectText), findsNothing); + expect(find.byType(ConnectToNewAppButton), findsNothing); + expect(find.byKey(initializedKey), findsNothing); + + fakeServiceConnectionManager.serviceManager.setConnectedState(true); + await tester.pump(); + + expect(find.text(waitingText), findsNothing); + expect(find.text(cannotConnectText), findsNothing); + expect(find.byType(ConnectToNewAppButton), findsNothing); expect(find.byKey(initializedKey), findsOneWidget); - expect(find.text('Disconnected'), findsNothing); - // Trigger a disconnect. - serviceConnection.serviceManager.changeState(false); - await tester.pumpAndSettle(const Duration(microseconds: 1000)); - - // Expect Disconnected overlay. - expect(find.text('Disconnected'), findsOneWidget); + // Verify expectations are still true after the timer advances. + await advanceTimer(tester); + expect(find.text(waitingText), findsNothing); + expect(find.text(cannotConnectText), findsNothing); + expect(find.byType(ConnectToNewAppButton), findsNothing); + expect(find.byKey(initializedKey), findsOneWidget); }, ); testWidgets( - 'closes disconnected overlay upon reconnect', + 'shows cannot connect message if connection is not established', (WidgetTester tester) async { - final serviceConnection = FakeServiceConnectionManager(); - setGlobal(ServiceConnectionManager, serviceConnection); + fakeServiceConnectionManager.serviceManager.setConnectedState(false); - // Expect standard connected state. - serviceConnection.serviceManager.changeState(true); await pumpInitializer(tester); - expect(find.byKey(initializedKey), findsOneWidget); - expect(find.text('Disconnected'), findsNothing); + expect(find.text(waitingText), findsOneWidget); + expect(find.text(cannotConnectText), findsNothing); + expect(find.byType(ConnectToNewAppButton), findsNothing); + expect(find.byKey(initializedKey), findsNothing); - // Trigger a disconnect and ensure the overlay appears. - serviceConnection.serviceManager.changeState(false); - await tester.pumpAndSettle(); - expect(find.text('Disconnected'), findsOneWidget); - - // Trigger a reconnect - serviceConnection.serviceManager.changeState(true); - await tester.pumpAndSettle(); - - // Expect no overlay. - expect(find.text('Disconnected'), findsNothing); + await advanceTimer(tester); + expect(find.text(waitingText), findsNothing); + expect(find.text(cannotConnectText), findsOneWidget); + expect(find.byType(ConnectToNewAppButton), findsOneWidget); + expect(find.byKey(initializedKey), findsNothing); }, ); });
diff --git a/packages/devtools_app/test/shared/scaffold_debugger_test.dart b/packages/devtools_app/test/shared/scaffold_debugger_test.dart index d03baeb..cdf5fd5 100644 --- a/packages/devtools_app/test/shared/scaffold_debugger_test.dart +++ b/packages/devtools_app/test/shared/scaffold_debugger_test.dart
@@ -27,7 +27,6 @@ when(mockServiceManager.connectedState).thenReturn( ValueNotifier<ConnectedState>(const ConnectedState(false)), ); - when(mockServiceManager.hasConnection).thenReturn(false); when(mockServiceManager.isolateManager).thenReturn(FakeIsolateManager()); when(mockServiceConnection.appState).thenReturn( AppState(
diff --git a/packages/devtools_app/test/shared/scaffold_debugging_controls_test.dart b/packages/devtools_app/test/shared/scaffold_debugging_controls_test.dart index d19d476..bdb9843 100644 --- a/packages/devtools_app/test/shared/scaffold_debugging_controls_test.dart +++ b/packages/devtools_app/test/shared/scaffold_debugging_controls_test.dart
@@ -28,7 +28,6 @@ when(mockServiceManager.connectedState).thenReturn( ValueNotifier<ConnectedState>(const ConnectedState(false)), ); - when(mockServiceManager.hasConnection).thenReturn(false); when(mockServiceManager.isolateManager).thenReturn(FakeIsolateManager()); when(mockServiceConnection.appState).thenReturn( AppState(
diff --git a/packages/devtools_app/test/shared/scaffold_no_app_test.dart b/packages/devtools_app/test/shared/scaffold_no_app_test.dart index 3b455af..0731c20 100644 --- a/packages/devtools_app/test/shared/scaffold_no_app_test.dart +++ b/packages/devtools_app/test/shared/scaffold_no_app_test.dart
@@ -28,7 +28,6 @@ when(mockServiceManager.connectedState).thenReturn( ValueNotifier<ConnectedState>(const ConnectedState(false)), ); - when(mockServiceManager.hasConnection).thenReturn(false); final mockErrorBadgeManager = MockErrorBadgeManager(); when(mockServiceConnection.errorBadgeManager)
diff --git a/packages/devtools_app/test/shared/scaffold_profile_test.dart b/packages/devtools_app/test/shared/scaffold_profile_test.dart index a17eaec..eaeb71e 100644 --- a/packages/devtools_app/test/shared/scaffold_profile_test.dart +++ b/packages/devtools_app/test/shared/scaffold_profile_test.dart
@@ -27,7 +27,6 @@ when(mockServiceManager.connectedState).thenReturn( ValueNotifier<ConnectedState>(const ConnectedState(false)), ); - when(mockServiceManager.hasConnection).thenReturn(false); when(mockServiceManager.isolateManager).thenReturn(FakeIsolateManager()); when(mockServiceConnection.appState).thenReturn( AppState(
diff --git a/packages/devtools_app/test/shared/scaffold_test.dart b/packages/devtools_app/test/shared/scaffold_test.dart index 6ba563a..28d1158 100644 --- a/packages/devtools_app/test/shared/scaffold_test.dart +++ b/packages/devtools_app/test/shared/scaffold_test.dart
@@ -29,7 +29,6 @@ when(mockServiceManager.connectedState).thenReturn( ValueNotifier<ConnectedState>(const ConnectedState(false)), ); - when(mockServiceManager.hasConnection).thenReturn(false); final mockErrorBadgeManager = MockErrorBadgeManager(); when(mockServiceConnection.errorBadgeManager)
diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md index cc1c775..586bbb2 100644 --- a/packages/devtools_app_shared/CHANGELOG.md +++ b/packages/devtools_app_shared/CHANGELOG.md
@@ -2,6 +2,8 @@ * Add `caseInsensitiveFuzzyMatch` extension method on `String`. * Add common widgets `DevToolsClearableTextField`, `InputDecorationSuffixButton`, and `RoundedDropDownButton`. +* Deprecate `ServiceManager.hasConnection` in favor of +`ServiceManager.connectedState.value.connected`. ## 0.2.1 * Add `navigateToCode` utility method for jumping to code in IDEs.
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 daeebf2..24e9580 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart
@@ -138,10 +138,11 @@ VM? vm; String? sdkVersion; - bool get hasConnection => service != null && connectedApp != null; + @Deprecated('Check connectedState.value.connected instead.') + bool get hasConnection => connectedState.value.connected; bool get connectedAppInitialized => - hasConnection && connectedApp!.connectedAppInitialized; + connectedApp?.connectedAppInitialized ?? false; ValueListenable<ConnectedState> get connectedState => _connectedState; @@ -428,7 +429,7 @@ } Future<void> manuallyDisconnect() async { - if (hasConnection) { + if (connectedState.value.connected) { await vmServiceClosed( connectionState: const ConnectedState(false, userInitiatedConnectionState: true),
diff --git a/packages/devtools_extensions/lib/src/template/extension_manager.dart b/packages/devtools_extensions/lib/src/template/extension_manager.dart index 32c0207..c4554b6 100644 --- a/packages/devtools_extensions/lib/src/template/extension_manager.dart +++ b/packages/devtools_extensions/lib/src/template/extension_manager.dart
@@ -157,7 +157,7 @@ // TODO(kenz): investigate. this is weird but `vmServiceUri` != null even // when the `toString()` representation is 'null'. if (vmServiceUri == null || vmServiceUri == 'null') { - if (serviceManager.hasConnection) { + if (serviceManager.connectedState.value.connected) { await serviceManager.manuallyDisconnect(); } if (loadQueryParams().containsKey(_vmServiceQueryParameter)) {
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 2609246..69e4271 100644 --- a/packages/devtools_test/lib/src/mocks/fake_service_manager.dart +++ b/packages/devtools_test/lib/src/mocks/fake_service_manager.dart
@@ -33,8 +33,8 @@ }) { _serviceManager = FakeServiceManager( service: service, - hasConnection: hasConnection, connectedAppInitialized: connectedAppInitialized, + hasConnection: hasConnection, availableLibraries: availableLibraries, availableServices: availableServices, rootLibrary: rootLibrary, @@ -96,13 +96,13 @@ implements ServiceManager<VmServiceWrapper> { FakeServiceManager({ VmServiceWrapper? service, - this.hasConnection = true, this.connectedAppInitialized = true, this.availableServices = const [], this.availableLibraries = const [], this.onVmServiceOpened, Map<String, Response>? serviceExtensionResponses, String? rootLibrary, + bool hasConnection = true, }) : serviceExtensionResponses = serviceExtensionResponses ?? _defaultServiceExtensionResponses, _isolateManager = FakeIsolateManager(rootLibrary: rootLibrary) { @@ -114,6 +114,7 @@ isProfileBuild: false, isWebApp: false, ); + setConnectedState(hasConnection); when(vm.operatingSystem).thenReturn('macos'); unawaited(vmServiceOpened(this.service!, onClosed: Future.value())); @@ -173,10 +174,7 @@ Future<VmService> onServiceAvailable = Future.value(MockVmService()); @override - bool get isServiceAvailable => hasConnection; - - @override - bool hasConnection; + bool get isServiceAvailable => connectedState.value.connected; @override bool connectedAppInitialized; @@ -241,7 +239,7 @@ @override Future<void> manuallyDisconnect() async { - changeState(false, manual: true); + setConnectedState(false, manual: true); } @override @@ -250,8 +248,7 @@ final _connectedState = ValueNotifier<ConnectedState>(const ConnectedState(false)); - void changeState(bool value, {bool manual = false}) { - hasConnection = value; + void setConnectedState(bool value, {bool manual = false}) { _connectedState.value = ConnectedState(value, userInitiatedConnectionState: manual); }