From 34ec94a16d21b1dd2169ca43121c68898a052ace Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Tue, 19 Oct 2021 01:33:08 -0700 Subject: [PATCH] [flutter_tools] migrate web_device.dart to null-safety (#91632) --- .../lib/src/android/android_device.dart | 2 +- packages/flutter_tools/lib/src/device.dart | 22 +-- .../flutter_tools/lib/src/web/web_device.dart | 130 +++++++++--------- .../test/general.shard/web/devices_test.dart | 52 ++++++- 4 files changed, 122 insertions(+), 84 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 7bdd848316..25ccf546e4 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -541,7 +541,7 @@ class AndroidDevice extends Device { String mainPath, String route, DebuggingOptions debuggingOptions, - Map platformArgs, + Map platformArgs = const {}, bool prebuiltApplication = false, bool ipv6 = false, String userIdentifier, diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index f3d0b21eed..b1da252358 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -468,7 +468,7 @@ abstract class Device { /// The ID returned matches that in the output of `flutter emulators`. Fetching /// this name may require connecting to the device and if an error occurs null /// will be returned. - Future get emulatorId; + Future get emulatorId; /// Whether this device can run the provided [buildMode]. /// @@ -501,7 +501,7 @@ abstract class Device { /// Specify [userIdentifier] to install for a particular user (Android only). Future installApp( covariant ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }); /// Uninstall an app package from the current device. @@ -510,7 +510,7 @@ abstract class Device { /// defaults to all users (Android only). Future uninstallApp( covariant ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }); /// Check if the device is supported by Flutter. @@ -550,12 +550,12 @@ abstract class Device { /// reader will also include log messages from before the invocation time. /// Defaults to false. FutureOr getLogReader({ - covariant ApplicationPackage app, + covariant ApplicationPackage? app, bool includePastLogs = false, }); /// Get the port forwarder for this device. - DevicePortForwarder get portForwarder; + DevicePortForwarder? get portForwarder; /// Get the DDS instance for this device. final DartDevelopmentService dds = DartDevelopmentService(); @@ -572,13 +572,13 @@ abstract class Device { /// start call. The build mode is not used by all platforms. Future startApp( covariant ApplicationPackage package, { - String mainPath, - String route, - DebuggingOptions debuggingOptions, + String? mainPath, + String? route, + required DebuggingOptions debuggingOptions, Map platformArgs, bool prebuiltApplication = false, bool ipv6 = false, - String userIdentifier, + String? userIdentifier, }); /// Whether this device implements support for hot reload. @@ -603,7 +603,7 @@ abstract class Device { /// Specify [userIdentifier] to stop app installed to a profile (Android only). Future stopApp( covariant ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }); /// Query the current application memory usage.. @@ -917,7 +917,7 @@ class DiscoveredApp { // An empty device log reader class NoOpDeviceLogReader implements DeviceLogReader { - NoOpDeviceLogReader(this.name); + NoOpDeviceLogReader(String? nameOrNull) : name = nameOrNull ?? ''; @override final String name; diff --git a/packages/flutter_tools/lib/src/web/web_device.dart b/packages/flutter_tools/lib/src/web/web_device.dart index 2a56825a4d..73ef855029 100644 --- a/packages/flutter_tools/lib/src/web/web_device.dart +++ b/packages/flutter_tools/lib/src/web/web_device.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.8 - import 'package:meta/meta.dart'; import 'package:process/process.dart'; @@ -36,10 +34,10 @@ class WebApplicationPackage extends ApplicationPackage { /// A web device that supports a chromium browser. abstract class ChromiumDevice extends Device { ChromiumDevice({ - @required String name, - @required this.chromeLauncher, - @required FileSystem fileSystem, - @required Logger logger, + required String name, + required this.chromeLauncher, + required FileSystem fileSystem, + required Logger logger, }) : _fileSystem = fileSystem, _logger = logger, super( @@ -55,7 +53,7 @@ abstract class ChromiumDevice extends Device { final Logger _logger; /// The active chrome instance. - Chromium _chrome; + Chromium? _chrome; // This device does not actually support hot reload, but the current implementation of the resident runner // requires both supportsHotReload and supportsHotRestart to be true in order to allow hot restart. @@ -80,11 +78,11 @@ abstract class ChromiumDevice extends Device { @override void clearLogs() { } - DeviceLogReader _logReader; + DeviceLogReader? _logReader; @override DeviceLogReader getLogReader({ - ApplicationPackage app, + ApplicationPackage? app, bool includePastLogs = false, }) { return _logReader ??= NoOpDeviceLogReader(app?.name); @@ -93,13 +91,13 @@ abstract class ChromiumDevice extends Device { @override Future installApp( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async => true; @override Future isAppInstalled( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async => true; @override @@ -109,28 +107,28 @@ abstract class ChromiumDevice extends Device { Future get isLocalEmulator async => false; @override - Future get emulatorId async => null; + Future get emulatorId async => null; @override bool isSupported() => chromeLauncher.canFindExecutable(); @override - DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); + DevicePortForwarder? get portForwarder => const NoOpDevicePortForwarder(); @override Future startApp( covariant WebApplicationPackage package, { - String mainPath, - String route, - DebuggingOptions debuggingOptions, - Map platformArgs, + String? mainPath, + String? route, + required DebuggingOptions debuggingOptions, + Map platformArgs = const {}, bool prebuiltApplication = false, bool ipv6 = false, - String userIdentifier, + String? userIdentifier, }) async { // See [ResidentWebRunner.run] in flutter_tools/lib/src/resident_web_runner.dart // for the web initialization and server logic. - final String url = platformArgs['uri'] as String; + final String url = platformArgs['uri']! as String; final bool launchChrome = platformArgs['no-launch-chrome'] != true; if (launchChrome) { _chrome = await chromeLauncher.launch( @@ -142,14 +140,14 @@ abstract class ChromiumDevice extends Device { debugPort: debuggingOptions.webBrowserDebugPort, ); } - _logger.sendEvent('app.webLaunchUrl', {'url': url, 'launched': launchChrome}); + _logger.sendEvent('app.webLaunchUrl', {'url': url, 'launched': launchChrome}); return LaunchResult.succeeded(observatoryUri: url != null ? Uri.parse(url): null); } @override Future stopApp( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async { await _chrome?.close(); return true; @@ -161,7 +159,7 @@ abstract class ChromiumDevice extends Device { @override Future uninstallApp( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async => true; @override @@ -179,11 +177,11 @@ abstract class ChromiumDevice extends Device { /// The Google Chrome browser based on Chromium. class GoogleChromeDevice extends ChromiumDevice { GoogleChromeDevice({ - @required Platform platform, - @required ProcessManager processManager, - @required ChromiumLauncher chromiumLauncher, - @required Logger logger, - @required FileSystem fileSystem, + required Platform platform, + required ProcessManager processManager, + required ChromiumLauncher chromiumLauncher, + required Logger logger, + required FileSystem fileSystem, }) : _platform = platform, _processManager = processManager, super( @@ -200,9 +198,8 @@ class GoogleChromeDevice extends ChromiumDevice { String get name => 'Chrome'; @override - Future get sdkNameAndVersion async => _sdkNameAndVersion ??= await _computeSdkNameAndVersion(); + late final Future sdkNameAndVersion = _computeSdkNameAndVersion(); - String _sdkNameAndVersion; Future _computeSdkNameAndVersion() async { if (!isSupported()) { return 'unknown'; @@ -238,10 +235,10 @@ class GoogleChromeDevice extends ChromiumDevice { /// The Microsoft Edge browser based on Chromium. class MicrosoftEdgeDevice extends ChromiumDevice { MicrosoftEdgeDevice({ - @required ChromiumLauncher chromiumLauncher, - @required Logger logger, - @required FileSystem fileSystem, - @required ProcessManager processManager, + required ChromiumLauncher chromiumLauncher, + required Logger logger, + required FileSystem fileSystem, + required ProcessManager processManager, }) : _processManager = processManager, super( name: 'edge', @@ -260,7 +257,7 @@ class MicrosoftEdgeDevice extends ChromiumDevice { Future _meetsVersionConstraint() async { final String rawVersion = (await sdkNameAndVersion).replaceFirst('Microsoft Edge ', ''); - final Version version = Version.parse(rawVersion); + final Version? version = Version.parse(rawVersion); if (version == null) { return false; } @@ -268,8 +265,8 @@ class MicrosoftEdgeDevice extends ChromiumDevice { } @override - Future get sdkNameAndVersion async => _sdkNameAndVersion ??= await _getSdkNameAndVersion(); - String _sdkNameAndVersion; + late final Future sdkNameAndVersion = _getSdkNameAndVersion(); + Future _getSdkNameAndVersion() async { if (_processManager.canRun('reg')) { final ProcessResult result = await _processManager.run([ @@ -290,12 +287,15 @@ class MicrosoftEdgeDevice extends ChromiumDevice { class WebDevices extends PollingDeviceDiscovery { WebDevices({ - @required FileSystem fileSystem, - @required Logger logger, - @required Platform platform, - @required ProcessManager processManager, - @required FeatureFlags featureFlags, + required FileSystem fileSystem, + required Logger logger, + required Platform platform, + required ProcessManager processManager, + required FeatureFlags featureFlags, }) : _featureFlags = featureFlags, + _webServerDevice = WebServerDevice( + logger: logger, + ), super('Chrome') { final OperatingSystemUtils operatingSystemUtils = OperatingSystemUtils( fileSystem: fileSystem, @@ -332,31 +332,29 @@ class WebDevices extends PollingDeviceDiscovery { fileSystem: fileSystem, ); } - _webServerDevice = WebServerDevice( - logger: logger, - ); } - GoogleChromeDevice _chromeDevice; - WebServerDevice _webServerDevice; - MicrosoftEdgeDevice _edgeDevice; + late final GoogleChromeDevice _chromeDevice; + final WebServerDevice _webServerDevice; + MicrosoftEdgeDevice? _edgeDevice; final FeatureFlags _featureFlags; @override bool get canListAnything => featureFlags.isWebEnabled; @override - Future> pollingGetDevices({ Duration timeout }) async { + Future> pollingGetDevices({ Duration? timeout }) async { if (!_featureFlags.isWebEnabled) { return []; } + final MicrosoftEdgeDevice? edgeDevice = _edgeDevice; return [ if (WebServerDevice.showWebServerDevice) _webServerDevice, if (_chromeDevice.isSupported()) _chromeDevice, - if (await _edgeDevice?._meetsVersionConstraint() ?? false) - _edgeDevice, + if (edgeDevice != null && await edgeDevice._meetsVersionConstraint()) + edgeDevice, ]; } @@ -376,7 +374,7 @@ String parseVersionForWindows(String input) { /// A special device type to allow serving for arbitrary browsers. class WebServerDevice extends Device { WebServerDevice({ - @required Logger logger, + required Logger logger, }) : _logger = logger, super( 'web-server', @@ -394,13 +392,13 @@ class WebServerDevice extends Device { void clearLogs() { } @override - Future get emulatorId async => null; + Future get emulatorId async => null; - DeviceLogReader _logReader; + DeviceLogReader? _logReader; @override DeviceLogReader getLogReader({ - ApplicationPackage app, + ApplicationPackage? app, bool includePastLogs = false, }) { return _logReader ??= NoOpDeviceLogReader(app?.name); @@ -409,13 +407,13 @@ class WebServerDevice extends Device { @override Future installApp( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async => true; @override Future isAppInstalled( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async => true; @override @@ -442,22 +440,22 @@ class WebServerDevice extends Device { String get name => 'Web Server'; @override - DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); + DevicePortForwarder? get portForwarder => const NoOpDevicePortForwarder(); @override Future get sdkNameAndVersion async => 'Flutter Tools'; @override Future startApp(ApplicationPackage package, { - String mainPath, - String route, - DebuggingOptions debuggingOptions, - Map platformArgs, + String? mainPath, + String? route, + required DebuggingOptions debuggingOptions, + Map platformArgs = const {}, bool prebuiltApplication = false, bool ipv6 = false, - String userIdentifier, + String? userIdentifier, }) async { - final String url = platformArgs['uri'] as String; + final String? url = platformArgs['uri'] as String?; if (debuggingOptions.startPaused) { _logger.printStatus('Waiting for connection from Dart debug extension at $url', emphasis: true); } else { @@ -467,14 +465,14 @@ class WebServerDevice extends Device { 'The web-server device requires the Dart Debug Chrome extension for debugging. ' 'Consider using the Chrome or Edge devices for an improved development workflow.' ); - _logger.sendEvent('app.webLaunchUrl', {'url': url, 'launched': false}); + _logger.sendEvent('app.webLaunchUrl', {'url': url, 'launched': false}); return LaunchResult.succeeded(observatoryUri: url != null ? Uri.parse(url): null); } @override Future stopApp( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async { return true; } @@ -485,7 +483,7 @@ class WebServerDevice extends Device { @override Future uninstallApp( ApplicationPackage app, { - String userIdentifier, + String? userIdentifier, }) async { return true; } diff --git a/packages/flutter_tools/test/general.shard/web/devices_test.dart b/packages/flutter_tools/test/general.shard/web/devices_test.dart index 617d83b8fb..6f618b07c1 100644 --- a/packages/flutter_tools/test/general.shard/web/devices_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devices_test.dart @@ -2,9 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.8 +import 'dart:async'; import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/build_info.dart'; @@ -32,8 +33,10 @@ void main() { }); testWithoutContext('GoogleChromeDevice defaults', () async { + final TestChromiumLauncher launcher = TestChromiumLauncher(); + final GoogleChromeDevice chromeDevice = GoogleChromeDevice( - chromiumLauncher: null, + chromiumLauncher: launcher, fileSystem: MemoryFileSystem.test(), logger: BufferLogger.test(), platform: FakePlatform(), @@ -50,7 +53,7 @@ void main() { expect(await chromeDevice.isLocalEmulator, false); expect(chromeDevice.getLogReader(), isA()); expect(chromeDevice.getLogReader(), isA()); - expect(await chromeDevice.portForwarder.forward(1), 1); + expect(await chromeDevice.portForwarder!.forward(1), 1); expect(chromeDevice.supportsRuntimeMode(BuildMode.debug), true); expect(chromeDevice.supportsRuntimeMode(BuildMode.profile), true); @@ -59,8 +62,10 @@ void main() { }); testWithoutContext('MicrosoftEdge defaults', () async { + final TestChromiumLauncher launcher = TestChromiumLauncher(); + final MicrosoftEdgeDevice chromeDevice = MicrosoftEdgeDevice( - chromiumLauncher: null, + chromiumLauncher: launcher, fileSystem: MemoryFileSystem.test(), logger: BufferLogger.test(), processManager: FakeProcessManager.any(), @@ -76,7 +81,7 @@ void main() { expect(await chromeDevice.isLocalEmulator, false); expect(chromeDevice.getLogReader(), isA()); expect(chromeDevice.getLogReader(), isA()); - expect(await chromeDevice.portForwarder.forward(1), 1); + expect(await chromeDevice.portForwarder!.forward(1), 1); expect(chromeDevice.supportsRuntimeMode(BuildMode.debug), true); expect(chromeDevice.supportsRuntimeMode(BuildMode.profile), true); @@ -99,7 +104,7 @@ void main() { expect(await device.isLocalEmulator, false); expect(device.getLogReader(), isA()); expect(device.getLogReader(), isA()); - expect(await device.portForwarder.forward(1), 1); + expect(await device.portForwarder!.forward(1), 1); expect(device.supportsRuntimeMode(BuildMode.debug), true); expect(device.supportsRuntimeMode(BuildMode.profile), true); @@ -353,3 +358,38 @@ void main() { expect((await macosWebDevices.pollingGetDevices()).whereType(), isEmpty); }); } + +/// A test implementation of the [ChromiumLauncher] that launches a fixed instance. +class TestChromiumLauncher implements ChromiumLauncher { + TestChromiumLauncher(); + + bool _hasInstance = false; + void setInstance(Chromium chromium) { + _hasInstance = true; + currentCompleter.complete(chromium); + } + + @override + Completer currentCompleter = Completer(); + + @override + bool canFindExecutable() { + return true; + } + + @override + Future get connectedInstance => currentCompleter.future; + + @override + String findExecutable() { + return 'chrome'; + } + + @override + bool get hasChromeInstance => _hasInstance; + + @override + Future launch(String url, {bool headless = false, int? debugPort, bool skipCheck = false, Directory? cacheDir}) async { + return currentCompleter.future; + } +}