Skip to content

Commit

Permalink
Ensure vm service connection is established on page refresh. (#6356)
Browse files Browse the repository at this point in the history
  • Loading branch information
kenzieschmoll committed Sep 12, 2023
1 parent fd9fb14 commit d50d223
Show file tree
Hide file tree
Showing 39 changed files with 548 additions and 446 deletions.
4 changes: 4 additions & 0 deletions packages/devtools_app/lib/src/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
Map<String, String?> args,
DevToolsNavigationState? state,
) {
if (FrameworkCore.initializationInProgress) {
return const MaterialPage(child: CenteredCircularProgressIndicator());
}

// Provide the appropriate page route.
if (pages.containsKey(page)) {
Widget widget = pages[page!]!(
Expand Down
16 changes: 13 additions & 3 deletions packages/devtools_app/lib/src/extensions/embedded/_view_web.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:devtools_extensions/api.dart';
import 'package:flutter/material.dart';

import '../../shared/banner_messages.dart';
import '../../shared/common_widgets.dart';
import '../../shared/globals.dart';
import '_controller_web.dart';
import 'controller.dart';
Expand All @@ -25,7 +26,8 @@ class EmbeddedExtension extends StatefulWidget {
State<EmbeddedExtension> createState() => _EmbeddedExtensionState();
}

class _EmbeddedExtensionState extends State<EmbeddedExtension> {
class _EmbeddedExtensionState extends State<EmbeddedExtension>
with AutoDisposeMixin {
late final EmbeddedExtensionControllerImpl _embeddedExtensionController;
late final _ExtensionIFrameController iFrameController;

Expand All @@ -42,8 +44,16 @@ class _EmbeddedExtensionState extends State<EmbeddedExtension> {
Widget build(BuildContext context) {
return Container(
color: Theme.of(context).scaffoldBackgroundColor,
child: HtmlElementView(
viewType: _embeddedExtensionController.viewId,
child: ValueListenableBuilder<bool>(
valueListenable: extensionService.refreshInProgress,
builder: (context, refreshing, _) {
if (refreshing) {
return const CenteredCircularProgressIndicator();
}
return HtmlElementView(
viewType: _embeddedExtensionController.viewId,
);
},
),
);
}
Expand Down
10 changes: 9 additions & 1 deletion packages/devtools_app/lib/src/extensions/extension_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,20 @@ class ExtensionService extends DisposableController
);
}

/// Whether extensions are actively being refreshed by the DevTools server.
ValueListenable<bool> get refreshInProgress => _refreshInProgress;
final _refreshInProgress = ValueNotifier(false);

final _extensionEnabledStates =
<String, ValueNotifier<ExtensionEnabledState>>{};

Future<void> initialize() async {
await _maybeRefreshExtensions();
addAutoDisposeListener(
serviceConnection.serviceManager.connectedState,
_maybeRefreshExtensions,
() async {
await _maybeRefreshExtensions();
},
);

// TODO(https://github.com/flutter/flutter/issues/134470): refresh on
Expand All @@ -66,10 +72,12 @@ class ExtensionService extends DisposableController
final appRootPath = await _connectedAppRootPath();
if (appRootPath == null) return;

_refreshInProgress.value = true;
_availableExtensions.value =
await server.refreshAvailableExtensions(appRootPath)
..sort();
await _refreshExtensionEnabledStates();
_refreshInProgress.value = false;
}

Future<void> _refreshExtensionEnabledStates() async {
Expand Down
22 changes: 18 additions & 4 deletions packages/devtools_app/lib/src/framework/framework_core.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:async';

import 'package:devtools_app_shared/utils.dart';
import 'package:devtools_shared/devtools_shared.dart';
import 'package:devtools_shared/service.dart';
import 'package:logging/logging.dart';

Expand Down Expand Up @@ -50,11 +51,13 @@ class FrameworkCore {
_log.info('DevTools version ${devtools.version}.');
}

static bool initializationInProgress = false;

/// Returns true if we're able to connect to a device and false otherwise.
static Future<bool> initVmService(
String url, {
Uri? explicitUri,
required ErrorReporter errorReporter,
required String serviceUriAsString,
ErrorReporter? errorReporter = _defaultErrorReporter,
bool logException = true,
}) async {
if (serviceConnection.serviceManager.hasConnection) {
Expand All @@ -63,8 +66,10 @@ class FrameworkCore {
return true;
}

final Uri? uri = explicitUri ?? getServiceUriFromQueryString(url);
final normalizedUri = normalizeVmServiceUri(serviceUriAsString);
final Uri? uri = normalizedUri ?? getServiceUriFromQueryString(url);
if (uri != null) {
initializationInProgress = true;
final finishedCompleter = Completer<void>();

try {
Expand Down Expand Up @@ -95,12 +100,21 @@ class FrameworkCore {
if (logException) {
_log.shout(e, e, st);
}
errorReporter('Unable to connect to VM service at $uri: $e', e);
errorReporter!('Unable to connect to VM service at $uri: $e', e);
return false;
} finally {
initializationInProgress = false;
}
} else {
// Don't report an error here because we do not have a URI to connect to.
return false;
}
}

static void _defaultErrorReporter(String title, Object error) {
notificationService.pushError(
'$title, $error',
isReportable: false,
);
}
}
18 changes: 6 additions & 12 deletions packages/devtools_app/lib/src/framework/home_screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -279,20 +279,20 @@ class _ConnectDialogState extends State<ConnectDialog>
gac.home,
gac.HomeScreenEvents.connectToApp.name,
);
if (connectDialogController.text.isEmpty) {

final uri = connectDialogController.text;
if (uri.isEmpty) {
notificationService.push('Please enter a VM Service URL.');
return;
}

assert(() {
if (_debugSharedPreferences != null) {
_debugSharedPreferences!
.setString(_vmServiceUriKey, connectDialogController.text);
_debugSharedPreferences!.setString(_vmServiceUriKey, uri);
}
return true;
}());

final uri = normalizeVmServiceUri(connectDialogController.text);
// Cache the routerDelegate and notifications providers before the async
// gap as the landing screen may not be displayed by the time the async gap
// is complete but we still want to show notifications and change the route.
Expand All @@ -302,21 +302,15 @@ class _ConnectDialogState extends State<ConnectDialog>
final routerDelegate = DevToolsRouterDelegate.of(context);
final connected = await FrameworkCore.initVmService(
'',
explicitUri: uri,
errorReporter: (message, error) {
notificationService.pushError(
'$message $error',
isReportable: false,
);
},
serviceUriAsString: uri,
);
if (connected) {
final connectedUri =
serviceConnection.serviceManager.service!.connectedUri;
routerDelegate.updateArgsIfChanged({'uri': '$connectedUri'});
final shortUri = connectedUri.replace(path: '');
notificationService.push('Successfully connected to $shortUri.');
} else if (uri == null) {
} else if (normalizeVmServiceUri(uri) == null) {
notificationService.push(
'Failed to connect to the VM Service at "${connectDialogController.text}".\n'
'The link was not valid.',
Expand Down
11 changes: 1 addition & 10 deletions packages/devtools_app/lib/src/framework/initializer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'dart:async';

import 'package:devtools_app_shared/ui.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:devtools_shared/devtools_shared.dart';
import 'package:flutter/material.dart';
import 'package:logging/logging.dart';

Expand Down Expand Up @@ -130,17 +129,9 @@ class _InitializerState extends State<Initializer>
return;
}

errorReporter ??= (String message, Object error) {
notificationService.pushError(
'$message, $error',
isReportable: false,
);
};

final uri = normalizeVmServiceUri(widget.url!);
final connected = await FrameworkCore.initVmService(
'',
explicitUri: uri,
serviceUriAsString: widget.url!,
errorReporter: errorReporter,
logException: logException,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class DebuggerController extends DisposableController
addAutoDisposeListener(_selectedStackFrame, _updateCurrentFrame);
addAutoDisposeListener(_stackFramesWithLocation, _updateCurrentFrame);

if (serviceConnection.serviceManager.hasService) {
if (serviceConnection.serviceManager.connectedState.value.connected) {
_initialize();
}
}
Expand Down
Loading

0 comments on commit d50d223

Please sign in to comment.