Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@ 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 'package:logging/logging.dart';

import '../../framework/framework_core.dart';
import '../../service/connected_app/connection_info.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/framework/routing.dart';
import '../../shared/globals.dart';
import '../../shared/primitives/query_parameters.dart';
import '../../shared/ui/common_widgets.dart';
import '../../shared/utils/utils.dart';

final _log = Logger('disconnect_observer');

class DisconnectObserver extends StatefulWidget {
const DisconnectObserver({
Expand All @@ -37,6 +43,14 @@ class DisconnectObserverState extends State<DisconnectObserver>

late ConnectedState currentConnectionState;

/// Stores the last known VM service URI so we can attempt to reconnect
/// after the connection is lost (e.g. when the machine sleeps).
String? _lastVmServiceUri;

final _isReconnecting = ValueNotifier<bool>(false);

final _reconnectErrorText = ValueNotifier<String?>(null);

@override
void initState() {
super.initState();
Expand All @@ -59,8 +73,14 @@ class DisconnectObserverState extends State<DisconnectObserver>
!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
// action, so show the overlay and ensure the 'uri' query parameter
// has been cleared.
//
// Store the VM service URI before clearing so we can attempt
// reconnection later (e.g. after machine sleep/wake).
_lastVmServiceUri =
widget.routerDelegate.currentConfiguration?.params.vmServiceUri ??
serviceConnection.serviceManager.serviceUri;
unawaited(widget.routerDelegate.clearUriParameter());
showDisconnectedOverlay();
}
Expand All @@ -71,6 +91,8 @@ class DisconnectObserverState extends State<DisconnectObserver>
@override
void dispose() {
hideDisconnectedOverlay();
_isReconnecting.dispose();
_reconnectErrorText.dispose();
super.dispose();
}

Expand Down Expand Up @@ -120,34 +142,144 @@ class DisconnectObserverState extends State<DisconnectObserver>
builder: (context) => Material(
child: Container(
color: theme.colorScheme.surface,
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(),
],
],
),
child: MultiValueListenableBuilder(
listenables: [_isReconnecting, _reconnectErrorText],
builder: (context, values, _) {
final isReconnecting = values.first as bool;
final reconnectErrorText = values[1] as String?;
return Center(
child: Column(
children: [
const Spacer(),
Text('Disconnected', style: theme.textTheme.headlineMedium),
const SizedBox(height: defaultSpacing),
if (isReconnecting)
const CircularProgressIndicator()
else
_ReconnectActions(
theme: theme,
onReconnect: _attemptReconnect,
routerDelegate: widget.routerDelegate,
onConnectToNewApp: hideDisconnectedOverlay,
),
if (reconnectErrorText case final error?) ...[
const SizedBox(height: denseSpacing),
Text(
error,
style: theme.regularTextStyle.copyWith(
color: theme.colorScheme.error,
),
textAlign: TextAlign.center,
),
],
const Spacer(),
if (offlineDataController.offlineDataJson.isNotEmpty) ...[
ElevatedButton(
onPressed: _reviewHistory,
child: const Text('Review recent data (offline)'),
),
const Spacer(),
],
],
),
);
},
),
),
),
);
return currentDisconnectedOverlay!;
}

Future<void> _attemptReconnect() async {
Comment thread
srawlins marked this conversation as resolved.
_isReconnecting.value = true;
_reconnectErrorText.value = null;

try {
await dtdManager.reconnect();
} catch (error, stackTrace) {
_log.warning('Failed to reconnect DTD.', error, stackTrace);
}

var reconnectionSuccess =
serviceConnection.serviceManager.connectedState.value.connected;

final uri = _lastVmServiceUri;
if (!reconnectionSuccess && uri != null) {
// Call initVmService directly — do NOT use routerDelegate.navigate()
// because that goes through _replaceStack which calls manuallyDisconnect
// when clearing the URI, causing the disconnect observer to suppress
// the overlay (userInitiatedConnectionState = true).
reconnectionSuccess = await FrameworkCore.initVmService(
serviceUriAsString: uri,
logException: false,
errorReporter: (title, error) {
_reconnectErrorText.value = '$title, $error';
},
);
}

_isReconnecting.value = false;

if (reconnectionSuccess) {
safeUnawaited(
widget.routerDelegate.updateArgsIfChanged({
DevToolsQueryParams.vmServiceUriKey: _lastVmServiceUri,
}),
);
_reconnectErrorText.value = null;
hideDisconnectedOverlay();
} else {
showDisconnectedOverlay();
}
}
}

class _ReconnectActions extends StatelessWidget {
const _ReconnectActions({
required this.theme,
required this.onReconnect,
required this.routerDelegate,
required this.onConnectToNewApp,
});

final ThemeData theme;
final VoidCallback onReconnect;
final DevToolsRouterDelegate routerDelegate;
final VoidCallback onConnectToNewApp;

@override
Widget build(BuildContext context) {
final reconnectButton = ElevatedButton(
onPressed: onReconnect,
child: const Text('Reconnect'),
);

if (!isEmbedded()) {
return Row(
mainAxisAlignment: MainAxisAlignment.center,
children: [
reconnectButton,
const SizedBox(width: defaultSpacing),
ConnectToNewAppButton(
routerDelegate: routerDelegate,
onPressed: onConnectToNewApp,
gaScreen: gac.devToolsMain,
),
],
);
}

return Column(
mainAxisSize: MainAxisSize.min,
children: [
reconnectButton,
const SizedBox(height: defaultSpacing),
Text(
'Or run a new debug session to connect to it.',
style: theme.textTheme.bodyMedium,
),
],
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class _NotConnectedOverlayState extends State<NotConnectedOverlay> {
if (showReconnectButton)
ElevatedButton(
onPressed: () => dtdManager.reconnect(),
child: const Text('Retry'),
child: const Text('Reconnect'),
),
],
),
Expand Down
4 changes: 3 additions & 1 deletion packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ TODO: Remove this section if there are not any updates.

## Inspector updates

TODO: Remove this section if there are not any updates.
- Added a "Reconnect" button to the disconnected overlay in embedded/IDE mode,
and fixed reconnection to restore the VM service connection after machine
sleep/wake (#9683).

## Performance updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,30 @@
import 'package:devtools_app/devtools_app.dart';
import 'package:devtools_app/src/framework/observer/disconnect_observer.dart';
import 'package:devtools_app/src/shared/framework/framework_controller.dart';
import 'package:devtools_app/src/shared/primitives/query_parameters.dart';
import 'package:devtools_app_shared/service.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';
import 'package:mockito/mockito.dart';

import '../../test_infra/matchers/matchers.dart';

void main() {
group('DisconnectObserver', () {
late FakeServiceConnectionManager fakeServiceConnectionManager;
late MockDTDManager mockDtdManager;

setUp(() {
fakeServiceConnectionManager = FakeServiceConnectionManager();
mockDtdManager = MockDTDManager();
when(mockDtdManager.reconnect()).thenAnswer((_) async {});
setGlobal(ServiceConnectionManager, fakeServiceConnectionManager);
setGlobal(DTDManager, mockDtdManager);
setGlobal(FrameworkController, FrameworkController());
setGlobal(OfflineDataController, OfflineDataController());
setGlobal(IdeTheme, IdeTheme());
Expand All @@ -30,6 +37,7 @@ void main() {
Future<void> pumpDisconnectObserver(
WidgetTester tester, {
Widget child = const Placeholder(),
DevToolsQueryParams? queryParams,
}) async {
await tester.pumpWidget(
wrap(
Expand All @@ -41,6 +49,7 @@ void main() {
);
},
),
queryParams: queryParams,
),
);
await tester.pumpAndSettle();
Expand All @@ -67,8 +76,14 @@ void main() {
find.byType(ConnectToNewAppButton),
showingOverlay && !isEmbedded() ? findsOneWidget : findsNothing,
);
// The Reconnect button should be present in both embedded and
// non-embedded modes when the overlay is showing.
expect(
find.text('Run a new debug session to reconnect.'),
find.text('Reconnect'),
Comment thread
srawlins marked this conversation as resolved.
showingOverlay ? findsOneWidget : findsNothing,
);
expect(
find.text('Or run a new debug session to connect to it.'),
showingOverlay && isEmbedded() ? findsOneWidget : findsNothing,
);
expect(
Expand Down Expand Up @@ -134,6 +149,40 @@ void main() {
await showOverlayAndVerifyContents(tester);
});

testWidgets(
'reconnect button restores previous VM service URI on success',
(WidgetTester tester) async {
const previousVmServiceUri = 'http://127.0.0.1:8181/';
when(mockDtdManager.reconnect()).thenAnswer((_) async {
fakeServiceConnectionManager.serviceManager.setConnectedState(true);
});

await pumpDisconnectObserver(
tester,
queryParams: DevToolsQueryParams({
DevToolsQueryParams.vmServiceUriKey: previousVmServiceUri,
}),
);
verifyObserverState(tester, connected: true, showingOverlay: false);

fakeServiceConnectionManager.serviceManager.setConnectedState(false);
await tester.pumpAndSettle();
verifyObserverState(tester, connected: false, showingOverlay: true);

await tester.tap(find.text('Reconnect'));
await tester.pumpAndSettle();

verify(mockDtdManager.reconnect()).called(1);
verifyObserverState(tester, connected: true, showingOverlay: false);
final context = tester.element(find.byType(DisconnectObserver));
final routerDelegate = DevToolsRouterDelegate.of(context);
expect(
routerDelegate.currentConfiguration!.params.vmServiceUri,
previousVmServiceUri,
);
},
);

// Regression test for https://github.com/flutter/devtools/issues/8050.
testWidgets('hides widgets at lower z-index', (
WidgetTester tester,
Expand Down
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disconnect_observer_disconnect change is expected, but I'm trying to figure out what caused the memory_diff changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the overlay stacking fix I changed the wrapper structure (Positioned.fill + Material) and moved the background color from Container to Material. The behavior should be equivalent, but that kind of wrapper change can still cause screenshot/golden diffs like memory_diff.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm why the change to Position.filled? Can we revert back to Material + Container or is using Position.filled necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Positioned.fill is the part that fixes the overlay stacking issue, so I kept that. I reverted the visual wrapper shape back to Material + Container to minimize unintended screenshot churn, so now it is Positioned.fill -> Material -> Container(color: surface).
That should preserve the stacking fix while reducing the memory_diff side effects.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm maybe I missed something, but what stacking issue are you referring to? (Screenshots would help). I thought this PR was just for adding a reconnect button

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Positioned.fill change was intended to make the disconnect overlay explicitly full-screen in the overlay stack (covering all bounds and blocking underlying interactions). It wasn’t for visuals. That said, since this PR is reconnect-focused and we don’t have a concrete repro in this thread, I reverted it to keep scope tight. I can follow up separately with a focused stacking fix plus screenshots/repro if needed.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.