From b42346438602b6ef52891e49159958b6e2230ed0 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 18 Aug 2023 13:10:24 -0700 Subject: [PATCH 1/2] Extensions cleanup and TODOs --- .../extensions/embedded/_controller_web.dart | 13 +++---- .../lib/src/shared/feature_flags.dart | 2 +- .../provider_screen_integration_test.dart | 2 + .../test/provider/provider_screen_test.dart | 2 + .../src/template/connected_app_manager.dart | 38 ------------------- .../lib/src/template/devtools_extension.dart | 3 +- .../lib/src/template/extension_manager.dart | 26 +++++++++++-- packages/devtools_test/lib/src/utils.dart | 1 + 8 files changed, 36 insertions(+), 51 deletions(-) delete mode 100644 packages/devtools_extensions/lib/src/template/connected_app_manager.dart diff --git a/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart b/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart index 24b2743a3f4..9a9bc79c882 100644 --- a/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart +++ b/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart @@ -5,7 +5,7 @@ // ignore_for_file: avoid_web_libraries_in_flutter, as designed import 'dart:async'; import 'dart:html' as html; -import 'dart:ui' as ui; +import 'dart:ui_web' as ui_web; import 'package:devtools_extensions/api.dart'; import 'package:path/path.dart' as path; @@ -18,9 +18,9 @@ import 'controller.dart'; /// Each time [EmbeddedExtensionControllerImpl.init] is called, we create a new /// [html.IFrameElement] and register it to /// [EmbeddedExtensionControllerImpl.viewId] via -/// [ui.platformViewRegistry.registerViewFactory]. Each new [html.IFrameElement] -/// must have a unique id in the [PlatformViewRegistry], which -/// [_viewIdIncrementer] is used to create. +/// [ui_web.platformViewRegistry.registerViewFactory]. Each new +/// [html.IFrameElement] must have a unique id in the [PlatformViewRegistry], +/// which [_viewIdIncrementer] is used to create. var _viewIdIncrementer = 0; class EmbeddedExtensionControllerImpl extends EmbeddedExtensionController { @@ -70,10 +70,7 @@ class EmbeddedExtensionControllerImpl extends EmbeddedExtensionController { ..height = '100%' ..width = '100%'; - // This ignore is required due to - // https://github.com/flutter/flutter/issues/41563 - // ignore: undefined_prefixed_name - final registered = ui.platformViewRegistry.registerViewFactory( + final registered = ui_web.platformViewRegistry.registerViewFactory( viewId, (int viewId) => _extensionIFrame, ); diff --git a/packages/devtools_app/lib/src/shared/feature_flags.dart b/packages/devtools_app/lib/src/shared/feature_flags.dart index 0399bc08292..9ee5bf1b8ee 100644 --- a/packages/devtools_app/lib/src/shared/feature_flags.dart +++ b/packages/devtools_app/lib/src/shared/feature_flags.dart @@ -72,7 +72,7 @@ abstract class FeatureFlags { /// Flag to enable DevTools extensions. /// /// https://github.com/flutter/devtools/issues/1632 - static bool devToolsExtensions = true; // enableExperiments; + static bool devToolsExtensions = enableExperiments; /// Flag to enable debugging via DAP. /// diff --git a/packages/devtools_app/test/provider/provider_screen_integration_test.dart b/packages/devtools_app/test/provider/provider_screen_integration_test.dart index 5b16b91d48a..f88c07e07a1 100644 --- a/packages/devtools_app/test/provider/provider_screen_integration_test.dart +++ b/packages/devtools_app/test/provider/provider_screen_integration_test.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. +// TODO(https://github.com/flutter/devtools/issues/6215): remove this test. + @TestOn('vm') import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/screens/provider/instance_viewer/instance_details.dart'; diff --git a/packages/devtools_app/test/provider/provider_screen_test.dart b/packages/devtools_app/test/provider/provider_screen_test.dart index 5d52e13bcd6..865ecd39735 100644 --- a/packages/devtools_app/test/provider/provider_screen_test.dart +++ b/packages/devtools_app/test/provider/provider_screen_test.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. +// TODO(https://github.com/flutter/devtools/issues/6215): remove this test. + @TestOn('vm') import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/screens/provider/instance_viewer/instance_details.dart'; diff --git a/packages/devtools_extensions/lib/src/template/connected_app_manager.dart b/packages/devtools_extensions/lib/src/template/connected_app_manager.dart deleted file mode 100644 index f9a7c606be2..00000000000 --- a/packages/devtools_extensions/lib/src/template/connected_app_manager.dart +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2023 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 'dart:async'; - -import 'package:devtools_shared/service.dart'; -import 'package:vm_service/vm_service.dart'; - -class ConnectedAppManager { - bool get hasConnectedApp => _vmServiceUri != null; - - VmService? vmService; - Uri? _vmServiceUri; - - Future connectToVmService(String? vmServiceUri) async { - if (vmServiceUri == null) { - vmService = null; - _vmServiceUri = null; - return; - } - - final finishedCompleter = Completer(); - vmService = await connect( - uri: Uri.parse(vmServiceUri), - finishedCompleter: finishedCompleter, - createService: ({ - // ignore: avoid-dynamic, code needs to match API from VmService. - required Stream /*String|List*/ inStream, - required void Function(String message) writeMessage, - required Uri connectedUri, - }) { - _vmServiceUri = connectedUri; - return VmService(inStream, writeMessage); - }, - ); - } -} diff --git a/packages/devtools_extensions/lib/src/template/devtools_extension.dart b/packages/devtools_extensions/lib/src/template/devtools_extension.dart index 4fbda145fae..b8a6254da06 100644 --- a/packages/devtools_extensions/lib/src/template/devtools_extension.dart +++ b/packages/devtools_extensions/lib/src/template/devtools_extension.dart @@ -9,11 +9,12 @@ import 'dart:html' as html; import 'package:devtools_app_shared/service.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; +import 'package:devtools_shared/service.dart'; import 'package:flutter/widgets.dart'; import 'package:logging/logging.dart'; +import 'package:vm_service/vm_service.dart'; import '../../api.dart'; -import 'connected_app_manager.dart'; part 'extension_manager.dart'; diff --git a/packages/devtools_extensions/lib/src/template/extension_manager.dart b/packages/devtools_extensions/lib/src/template/extension_manager.dart index e49d680fee9..ff7d017e093 100644 --- a/packages/devtools_extensions/lib/src/template/extension_manager.dart +++ b/packages/devtools_extensions/lib/src/template/extension_manager.dart @@ -7,8 +7,6 @@ part of 'devtools_extension.dart'; final _log = Logger('devtools_extensions/extension_manager'); class ExtensionManager { - final appManager = ConnectedAppManager(); - final _registeredEventHandlers = {}; @@ -58,7 +56,7 @@ class ExtensionManager { break; case DevToolsExtensionEventType.vmServiceConnection: final vmServiceUri = extensionEvent.data?['uri'] as String?; - unawaited(appManager.connectToVmService(vmServiceUri)); + unawaited(connectToVmService(vmServiceUri)); break; case DevToolsExtensionEventType.unknown: default: @@ -75,4 +73,26 @@ class ExtensionManager { void postMessageToDevTools(DevToolsExtensionEvent event) { html.window.parent?.postMessage(event.toJson(), html.window.origin!); } + + Future connectToVmService(String? vmServiceUri) async { + if (vmServiceUri == null) return; + + final finishedCompleter = Completer(); + final vmService = await connect( + uri: Uri.parse(vmServiceUri), + finishedCompleter: finishedCompleter, + createService: ({ + // ignore: avoid-dynamic, code needs to match API from VmService. + required Stream /*String|List*/ inStream, + required void Function(String message) writeMessage, + required Uri connectedUri, + }) { + return VmService(inStream, writeMessage); + }, + ); + await serviceManager.vmServiceOpened( + vmService, + onClosed: finishedCompleter.future, + ); + } } diff --git a/packages/devtools_test/lib/src/utils.dart b/packages/devtools_test/lib/src/utils.dart index ed8ad74ea9f..775f875c039 100644 --- a/packages/devtools_test/lib/src/utils.dart +++ b/packages/devtools_test/lib/src/utils.dart @@ -213,6 +213,7 @@ void _mockFlutterAssets() { ); } +// TODO(https://github.com/flutter/devtools/issues/6215): remove this helper. /// Load fonts used by the devtool for golden-tests to use them Future loadFonts() async { // source: https://medium.com/swlh/test-your-flutter-widgets-using-golden-files-b533ac0de469 From bcb7f3618af9f91db75238331a7e56901a4a6534 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 18 Aug 2023 14:21:42 -0700 Subject: [PATCH 2/2] handle error --- .../lib/src/template/extension_manager.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/devtools_extensions/lib/src/template/extension_manager.dart b/packages/devtools_extensions/lib/src/template/extension_manager.dart index ff7d017e093..dee1c4ad5cf 100644 --- a/packages/devtools_extensions/lib/src/template/extension_manager.dart +++ b/packages/devtools_extensions/lib/src/template/extension_manager.dart @@ -56,7 +56,13 @@ class ExtensionManager { break; case DevToolsExtensionEventType.vmServiceConnection: final vmServiceUri = extensionEvent.data?['uri'] as String?; - unawaited(connectToVmService(vmServiceUri)); + unawaited( + connectToVmService(vmServiceUri).catchError((e) { + // TODO(kenz): post a notification to DevTools for errors + // or create an error panel for the extensions screens. + print('Error connecting to VM service: $e'); + }), + ); break; case DevToolsExtensionEventType.unknown: default: