From f2761b6b041fc78a0de2fd8831b14847e65477eb Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 22 Apr 2020 01:28:51 -0700 Subject: [PATCH] [flutter_tools] refactor Chrome launch logic to remove globals/statics (#55160) --- .../lib/src/build_runner/devfs_web.dart | 8 +- .../src/build_runner/resident_web_runner.dart | 14 +- .../lib/src/commands/devices.dart | 9 +- .../flutter_tools/lib/src/context_runner.dart | 8 - packages/flutter_tools/lib/src/device.dart | 8 +- packages/flutter_tools/lib/src/doctor.dart | 13 +- packages/flutter_tools/lib/src/globals.dart | 4 - .../lib/src/test/flutter_web_platform.dart | 14 +- .../flutter_tools/lib/src/web/chrome.dart | 125 +++++++--- .../flutter_tools/lib/src/web/web_device.dart | 232 +++++++++++++----- .../lib/src/web/web_validator.dart | 81 ++++-- .../permeable/devices_test.dart | 128 +++++----- .../test/general.shard/doctor.dart | 4 +- .../resident_web_runner_cold_test.dart | 11 +- .../resident_web_runner_test.dart | 79 ++++-- .../test/general.shard/web/chrome_test.dart | 108 +++++--- .../general.shard/web/devfs_web_test.dart | 3 + .../test/general.shard/web/devices_test.dart | 231 ++++++++++------- .../general.shard/web/web_validator_test.dart | 15 +- 19 files changed, 715 insertions(+), 380 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_runner/devfs_web.dart b/packages/flutter_tools/lib/src/build_runner/devfs_web.dart index b0089255af5..4e7ccf343c3 100644 --- a/packages/flutter_tools/lib/src/build_runner/devfs_web.dart +++ b/packages/flutter_tools/lib/src/build_runner/devfs_web.dart @@ -125,6 +125,7 @@ class WebAssetServer implements AssetReader { /// Unhandled exceptions will throw a [ToolExit] with the error and stack /// trace. static Future start( + ChromiumLauncher chromiumLauncher, String hostname, int port, UrlTunneller urlTunneller, @@ -208,8 +209,8 @@ class WebAssetServer implements AssetReader { enableDebugExtension: true, buildResults: const Stream.empty(), chromeConnection: () async { - final Chrome chrome = await ChromeLauncher.connectedInstance; - return chrome.chromeConnection; + final Chromium chromium = await chromiumLauncher.connectedInstance; + return chromium.chromeConnection; }, hostname: hostname, urlEncoder: urlTunneller, @@ -562,6 +563,7 @@ class WebDevFS implements DevFS { @required this.enableDwds, @required this.entrypoint, @required this.expressionCompiler, + @required this.chromiumLauncher, this.testMode = false, }); @@ -575,6 +577,7 @@ class WebDevFS implements DevFS { final bool enableDwds; final bool testMode; final ExpressionCompiler expressionCompiler; + final ChromiumLauncher chromiumLauncher; WebAssetServer webAssetServer; @@ -632,6 +635,7 @@ class WebDevFS implements DevFS { @override Future create() async { webAssetServer = await WebAssetServer.start( + chromiumLauncher, hostname, port, urlTunneller, diff --git a/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart b/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart index b7dd68c83d8..1805ef21616 100644 --- a/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart +++ b/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart @@ -114,6 +114,7 @@ abstract class ResidentWebRunner extends ResidentRunner { StreamSubscription _stdErrSub; bool _exited = false; WipConnection _wipConnection; + ChromiumLauncher _chromiumLauncher; vmservice.VmService get _vmService => _connectionResult?.debugConnection?.vmService; @@ -158,10 +159,6 @@ abstract class ResidentWebRunner extends ResidentRunner { 'Failed to clean up temp directory: ${_generatedEntrypointDirectory.path}', ); } - if (ChromeLauncher.hasChromeInstance) { - final Chrome chrome = await ChromeLauncher.connectedInstance; - await chrome.close(); - } _exited = true; } @@ -394,6 +391,10 @@ class _ResidentWebRunner extends ResidentWebRunner { ? await globals.os.findFreePort() : int.tryParse(debuggingOptions.port); + if (device.device is ChromiumDevice) { + _chromiumLauncher = (device.device as ChromiumDevice).chromeLauncher; + } + try { return await asyncGuard(() async { // Ensure dwds resources are cached. If the .packages file is missing then @@ -419,6 +420,7 @@ class _ResidentWebRunner extends ResidentWebRunner { enableDwds: _enableDwds, entrypoint: globals.fs.file(target).uri, expressionCompiler: expressionCompiler, + chromiumLauncher: _chromiumLauncher, ); final Uri url = await device.devFS.create(); if (debuggingOptions.buildInfo.isDebug) { @@ -645,8 +647,8 @@ class _ResidentWebRunner extends ResidentWebRunner { Completer connectionInfoCompleter, Completer appStartedCompleter, }) async { - if (device.device is ChromeDevice) { - final Chrome chrome = await ChromeLauncher.connectedInstance; + if (_chromiumLauncher != null) { + final Chromium chrome = await _chromiumLauncher.connectedInstance; final ChromeTab chromeTab = await chrome.chromeConnection.getTab((ChromeTab chromeTab) { return !chromeTab.url.startsWith('chrome-extension'); }); diff --git a/packages/flutter_tools/lib/src/commands/devices.dart b/packages/flutter_tools/lib/src/commands/devices.dart index 0a239e422ff..312e82ac11c 100644 --- a/packages/flutter_tools/lib/src/commands/devices.dart +++ b/packages/flutter_tools/lib/src/commands/devices.dart @@ -14,12 +14,10 @@ import '../runner/flutter_command.dart'; class DevicesCommand extends FlutterCommand { DevicesCommand() { - argParser.addFlag('machine', negatable: false, help: 'Output device information in machine readable structured JSON format', ); - argParser.addOption( 'timeout', abbr: 't', @@ -60,7 +58,9 @@ class DevicesCommand extends FlutterCommand { final List devices = await deviceManager.refreshAllConnectedDevices(timeout: timeout); - if (devices.isEmpty) { + if (boolArg('machine')) { + await printDevicesAsJson(devices); + } else if (devices.isEmpty) { final StringBuffer status = StringBuffer('No devices detected.'); status.writeln(); status.writeln(); @@ -80,8 +80,6 @@ class DevicesCommand extends FlutterCommand { globals.printStatus('• $diagnostic', hangingIndent: 2); } } - } else if (boolArg('machine')) { - await printDevicesAsJson(devices); } else { globals.printStatus('${devices.length} connected ${pluralize('device', devices.length)}:\n'); await Device.printDevices(devices); @@ -97,5 +95,4 @@ class DevicesCommand extends FlutterCommand { ) ); } - } diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index 73bf3d49332..bfcee3e231f 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -45,7 +45,6 @@ import 'persistent_tool_state.dart'; import 'reporting/reporting.dart'; import 'run_hot.dart'; import 'version.dart'; -import 'web/chrome.dart'; import 'web/workflow.dart'; import 'windows/visual_studio.dart'; import 'windows/visual_studio_validator.dart'; @@ -99,13 +98,6 @@ Future runInContext( logger: globals.logger, platform: globals.platform, ), - ChromeLauncher: () => ChromeLauncher( - fileSystem: globals.fs, - processManager: globals.processManager, - logger: globals.logger, - operatingSystemUtils: globals.os, - platform: globals.platform, - ), CocoaPods: () => CocoaPods( fileSystem: globals.fs, processManager: globals.processManager, diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 976e94186e3..a85ae7f9097 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -90,7 +90,13 @@ class DeviceManager { featureFlags: featureFlags, ), WindowsDevices(), - WebDevices(), + WebDevices( + featureFlags: featureFlags, + fileSystem: globals.fs, + logger: globals.logger, + platform: globals.platform, + processManager: globals.processManager, + ), ]); String _specifiedDeviceId; diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index 6fa37d479d8..bad0a9dd540 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart @@ -35,6 +35,7 @@ import 'reporting/reporting.dart'; import 'tester/flutter_tester.dart'; import 'version.dart'; import 'vscode/vscode_validator.dart'; +import 'web/chrome.dart'; import 'web/web_validator.dart'; import 'web/workflow.dart'; import 'windows/visual_studio_validator.dart'; @@ -82,10 +83,16 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { if (globals.iosWorkflow.appliesToHostPlatform || macOSWorkflow.appliesToHostPlatform) GroupedValidator([XcodeValidator(xcode: globals.xcode, userMessages: userMessages), cocoapodsValidator]), if (webWorkflow.appliesToHostPlatform) - WebValidator( - chromeLauncher: globals.chromeLauncher, + ChromeValidator( + chromiumLauncher: ChromiumLauncher( + browserFinder: findChromeExecutable, + fileSystem: globals.fs, + logger: globals.logger, + operatingSystemUtils: globals.os, + platform: globals.platform, + processManager: globals.processManager, + ), platform: globals.platform, - fileSystem: globals.fs, ), if (linuxWorkflow.appliesToHostPlatform) LinuxDoctorValidator( diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index b91cbf145bb..22bee70e360 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -34,7 +34,6 @@ import 'persistent_tool_state.dart'; import 'project.dart'; import 'reporting/reporting.dart'; import 'version.dart'; -import 'web/chrome.dart'; Artifacts get artifacts => context.get(); BuildSystem get buildSystem => context.get(); @@ -183,8 +182,5 @@ PlistParser get plistParser => context.get() ?? ( )); PlistParser _plistInstance; -/// The [ChromeLauncher] instance. -ChromeLauncher get chromeLauncher => context.get(); - /// The global template renderer TemplateRenderer get templateRenderer => context.get(); diff --git a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart index 258f731ec09..9ba5dad731a 100644 --- a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart @@ -555,7 +555,7 @@ class BrowserManager { } /// The browser instance that this is connected to via [_channel]. - final Chrome _browser; + final Chromium _browser; // TODO(nweiz): Consider removing the duplication between this and // [_browser.name]. @@ -623,8 +623,16 @@ class BrowserManager { bool debug = false, bool headless = true, }) async { - final Chrome chrome = - await globals.chromeLauncher.launch(url.toString(), headless: headless); + final ChromiumLauncher chromiumLauncher = ChromiumLauncher( + browserFinder: findChromeExecutable, + fileSystem: globals.fs, + operatingSystemUtils: globals.os, + logger: globals.logger, + platform: globals.platform, + processManager: globals.processManager, + ); + final Chromium chrome = + await chromiumLauncher.launch(url.toString(), headless: headless); final Completer completer = Completer(); diff --git a/packages/flutter_tools/lib/src/web/chrome.dart b/packages/flutter_tools/lib/src/web/chrome.dart index e1e882ff9ff..214c10ff94d 100644 --- a/packages/flutter_tools/lib/src/web/chrome.dart +++ b/packages/flutter_tools/lib/src/web/chrome.dart @@ -15,11 +15,13 @@ import '../base/io.dart'; import '../base/logger.dart'; import '../base/os.dart'; import '../convert.dart'; -import '../globals.dart' as globals; -/// An environment variable used to override the location of chrome. +/// An environment variable used to override the location of Google Chrome. const String kChromeEnvironment = 'CHROME_EXECUTABLE'; +/// An environment variable used to override the location of Microsoft Edge. +const String kEdgeEnvironment = 'EDGE_ENVIRONMENT'; + /// The expected executable name on linux. const String kLinuxExecutable = 'google-chrome'; @@ -27,9 +29,14 @@ const String kLinuxExecutable = 'google-chrome'; const String kMacOSExecutable = '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome'; -/// The expected executable name on Windows. +/// The expected Chrome executable name on Windows. const String kWindowsExecutable = r'Google\Chrome\Application\chrome.exe'; +/// The expected Edge executable name on Windows. +const String kWindowsEdgeExecutable = r'Microsoft\Edge\Application\msedge.exe'; + +typedef BrowserFinder = String Function(Platform, FileSystem); + /// Find the chrome executable on the current platform. /// /// Does not verify whether the executable exists. @@ -63,43 +70,73 @@ String findChromeExecutable(Platform platform, FileSystem fileSystem) { return null; } -@visibleForTesting -void resetChromeForTesting() { - ChromeLauncher._currentCompleter = Completer(); +/// Find the Microsoft Edge executable on the current platform. +/// +/// Does not verify whether the executable exists. +String findEdgeExecutable(Platform platform, FileSystem fileSystem) { + if (platform.environment.containsKey(kEdgeEnvironment)) { + return platform.environment[kEdgeEnvironment]; + } + if (platform.isWindows) { + /// The possible locations where the Edge executable can be located on windows. + final List kWindowsPrefixes = [ + platform.environment['LOCALAPPDATA'], + platform.environment['PROGRAMFILES'], + platform.environment['PROGRAMFILES(X86)'], + ]; + final String windowsPrefix = kWindowsPrefixes.firstWhere((String prefix) { + if (prefix == null) { + return false; + } + final String path = fileSystem.path.join(prefix, kWindowsEdgeExecutable); + return fileSystem.file(path).existsSync(); + }, orElse: () => '.'); + return fileSystem.path.join(windowsPrefix, kWindowsEdgeExecutable); + } + // Not yet supported for macOS and Linux. + return ''; } -@visibleForTesting -void launchChromeInstance(Chrome chrome) { - ChromeLauncher._currentCompleter.complete(chrome); -} - -/// Responsible for launching chrome with devtools configured. -class ChromeLauncher { - const ChromeLauncher({ +/// A launcher for Chromium browsers with devtools configured. +class ChromiumLauncher { + ChromiumLauncher({ @required FileSystem fileSystem, @required Platform platform, @required ProcessManager processManager, @required OperatingSystemUtils operatingSystemUtils, @required Logger logger, + @required BrowserFinder browserFinder, }) : _fileSystem = fileSystem, _platform = platform, _processManager = processManager, _operatingSystemUtils = operatingSystemUtils, - _logger = logger; + _logger = logger, + _browserFinder = browserFinder, + _fileSystemUtils = FileSystemUtils( + fileSystem: fileSystem, + platform: platform, + ); final FileSystem _fileSystem; final Platform _platform; final ProcessManager _processManager; final OperatingSystemUtils _operatingSystemUtils; final Logger _logger; + final BrowserFinder _browserFinder; + final FileSystemUtils _fileSystemUtils; - static bool get hasChromeInstance => _currentCompleter.isCompleted; + bool get hasChromeInstance => _currentCompleter.isCompleted; - static Completer _currentCompleter = Completer(); + Completer _currentCompleter = Completer(); + + @visibleForTesting + void testLaunchChromium(Chromium chromium) { + _currentCompleter.complete(chromium); + } /// Whether we can locate the chrome executable. - bool canFindChrome() { - final String chrome = findChromeExecutable(_platform, _fileSystem); + bool canFindExecutable() { + final String chrome = _browserFinder(_platform, _fileSystem); try { return _processManager.canRun(chrome); } on ArgumentError { @@ -107,7 +144,10 @@ class ChromeLauncher { } } - /// Launch the chrome browser to a particular `host` page. + /// The executable this launcher will use. + String findExecutable() => _browserFinder(_platform, _fileSystem); + + /// Launch a Chromium browser to a particular `host` page. /// /// `headless` defaults to false, and controls whether we open a headless or /// a `headfull` browser. @@ -116,13 +156,19 @@ class ChromeLauncher { /// port is picked automatically. /// /// `skipCheck` does not attempt to make a devtools connection before returning. - Future launch(String url, { bool headless = false, int debugPort, bool skipCheck = false, Directory cacheDir }) async { + Future launch(String url, { + bool headless = false, + int debugPort, + bool skipCheck = false, + Directory cacheDir, + }) async { if (_currentCompleter.isCompleted) { throwToolExit('Only one instance of chrome can be started.'); } - final String chromeExecutable = findChromeExecutable(_platform, _fileSystem); - final Directory userDataDir = _fileSystem.systemTempDirectory.createTempSync('flutter_tools_chrome_device.'); + final String chromeExecutable = _browserFinder(_platform, _fileSystem); + final Directory userDataDir = _fileSystem.systemTempDirectory + .createTempSync('flutter_tools_chrome_device.'); if (cacheDir != null) { // Seed data dir with previous state. @@ -148,7 +194,12 @@ class ChromeLauncher { '--disable-default-apps', '--disable-translate', if (headless) - ...['--headless', '--disable-gpu', '--no-sandbox', '--window-size=2400,1800'], + ...[ + '--headless', + '--disable-gpu', + '--no-sandbox', + '--window-size=2400,1800', + ], url, ]; @@ -180,12 +231,13 @@ class ChromeLauncher { return 'Failed to spawn stderr'; }); final Uri remoteDebuggerUri = await _getRemoteDebuggerUrl(Uri.parse('http://localhost:$port')); - return _connect(Chrome._( + return _connect(Chromium._( port, ChromeConnection('localhost', port), url: url, process: process, remoteDebuggerUri: remoteDebuggerUri, + chromiumLauncher: this, ), skipCheck); } @@ -217,7 +269,7 @@ class ChromeLauncher { if (sourceLocalStorageDir.existsSync()) { targetLocalStorageDir.createSync(recursive: true); - globals.fsUtils.copyDirectorySync(sourceLocalStorageDir, targetLocalStorageDir); + _fileSystemUtils.copyDirectorySync(sourceLocalStorageDir, targetLocalStorageDir); } } @@ -236,11 +288,11 @@ class ChromeLauncher { if (sourceLocalStorageDir.existsSync()) { targetLocalStorageDir.createSync(recursive: true); - globals.fsUtils.copyDirectorySync(sourceLocalStorageDir, targetLocalStorageDir); + _fileSystemUtils.copyDirectorySync(sourceLocalStorageDir, targetLocalStorageDir); } } - static Future _connect(Chrome chrome, bool skipCheck) async { + Future _connect(Chromium chrome, bool skipCheck) async { // The connection is lazy. Try a simple call to make sure the provided // connection is valid. if (!skipCheck) { @@ -256,7 +308,7 @@ class ChromeLauncher { return chrome; } - static Future get connectedInstance => _currentCompleter.future; + Future get connectedInstance => _currentCompleter.future; /// Returns the full URL of the Chrome remote debugger for the main page. /// @@ -281,27 +333,30 @@ class ChromeLauncher { } } -/// A class for managing an instance of Chrome. -class Chrome { - Chrome._( +/// A class for managing an instance of a Chromium browser. +class Chromium { + Chromium._( this.debugPort, this.chromeConnection, { this.url, Process process, this.remoteDebuggerUri, - }) : _process = process; + @required ChromiumLauncher chromiumLauncher, + }) : _process = process, + _chromiumLauncher = chromiumLauncher; final String url; final int debugPort; final Process _process; final ChromeConnection chromeConnection; final Uri remoteDebuggerUri; + final ChromiumLauncher _chromiumLauncher; Future get onExit => _process.exitCode; Future close() async { - if (ChromeLauncher.hasChromeInstance) { - ChromeLauncher._currentCompleter = Completer(); + if (_chromiumLauncher.hasChromeInstance) { + _chromiumLauncher._currentCompleter = Completer(); } chromeConnection.close(); _process?.kill(); diff --git a/packages/flutter_tools/lib/src/web/web_device.dart b/packages/flutter_tools/lib/src/web/web_device.dart index 649afa70b88..d2bdc8ae9f4 100644 --- a/packages/flutter_tools/lib/src/web/web_device.dart +++ b/packages/flutter_tools/lib/src/web/web_device.dart @@ -3,14 +3,17 @@ // found in the LICENSE file. import 'package:meta/meta.dart'; +import 'package:platform/platform.dart'; +import 'package:process/process.dart'; import '../application_package.dart'; import '../base/file_system.dart'; import '../base/io.dart'; +import '../base/logger.dart'; +import '../base/os.dart'; import '../build_info.dart'; import '../device.dart'; import '../features.dart'; -import '../globals.dart' as globals; import '../project.dart'; import 'chrome.dart'; @@ -26,16 +29,29 @@ class WebApplicationPackage extends ApplicationPackage { Directory get webSourcePath => flutterProject.directory.childDirectory('web'); } -class ChromeDevice extends Device { - ChromeDevice() : super( - 'chrome', - category: Category.web, - platformType: PlatformType.web, - ephemeral: false, - ); +/// 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, + }) : _fileSystem = fileSystem, + _logger = logger, + super( + name, + category: Category.web, + platformType: PlatformType.web, + ephemeral: false, + ); + + final ChromiumLauncher chromeLauncher; + + final FileSystem _fileSystem; + final Logger _logger; /// The active chrome instance. - Chrome _chrome; + Chromium _chrome; // TODO(jonahwilliams): this is technically false, but requires some refactoring // to allow hot mode restart only devices. @@ -83,47 +99,11 @@ class ChromeDevice extends Device { Future get emulatorId async => null; @override - bool isSupported() => featureFlags.isWebEnabled && globals.chromeLauncher.canFindChrome(); - - @override - String get name => 'Chrome'; + bool isSupported() => chromeLauncher.canFindExecutable(); @override DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); - @override - Future get sdkNameAndVersion async => _sdkNameAndVersion ??= await _computeSdkNameAndVersion(); - - String _sdkNameAndVersion; - Future _computeSdkNameAndVersion() async { - if (!isSupported()) { - return 'unknown'; - } - // See https://bugs.chromium.org/p/chromium/issues/detail?id=158372 - String version = 'unknown'; - if (globals.platform.isWindows) { - final ProcessResult result = await globals.processManager.run([ - r'reg', 'query', r'HKEY_CURRENT_USER\Software\Google\Chrome\BLBeacon', '/v', 'version', - ]); - if (result.exitCode == 0) { - final List parts = (result.stdout as String).split(RegExp(r'\s+')); - if (parts.length > 2) { - version = 'Google Chrome ' + parts[parts.length - 2]; - } - } - } else { - final String chrome = findChromeExecutable(globals.platform, globals.fs); - final ProcessResult result = await globals.processManager.run([ - chrome, - '--version', - ]); - if (result.exitCode == 0) { - version = result.stdout as String; - } - } - return version.trim(); - } - @override Future startApp( covariant WebApplicationPackage package, { @@ -139,9 +119,9 @@ class ChromeDevice extends Device { final String url = platformArgs['uri'] as String; final bool launchChrome = platformArgs['no-launch-chrome'] != true; if (launchChrome) { - _chrome = await globals.chromeLauncher.launch( + _chrome = await chromeLauncher.launch( url, - cacheDir: globals.fs.currentDirectory + cacheDir: _fileSystem.currentDirectory .childDirectory('.dart_tool') .childDirectory('chrome-device'), headless: debuggingOptions.webRunHeadless, @@ -149,7 +129,7 @@ class ChromeDevice extends Device { ); } - globals.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); } @@ -177,27 +157,140 @@ class ChromeDevice extends Device { } } -class WebDevices extends PollingDeviceDiscovery { - WebDevices() : super('chrome'); +/// 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, + }) : _platform = platform, + _processManager = processManager, + super( + name: 'chrome', + chromeLauncher: chromiumLauncher, + logger: logger, + fileSystem: fileSystem, + ); - final bool _chromeIsAvailable = globals.chromeLauncher.canFindChrome(); - final ChromeDevice _webDevice = ChromeDevice(); - final WebServerDevice _webServerDevice = WebServerDevice(); + final Platform _platform; + final ProcessManager _processManager; + + @override + String get name => 'Chrome'; + + @override + Future get sdkNameAndVersion async => _sdkNameAndVersion ??= await _computeSdkNameAndVersion(); + + String _sdkNameAndVersion; + Future _computeSdkNameAndVersion() async { + if (!isSupported()) { + return 'unknown'; + } + // See https://bugs.chromium.org/p/chromium/issues/detail?id=158372 + String version = 'unknown'; + if (_platform.isWindows) { + final ProcessResult result = await _processManager.run([ + r'reg', 'query', r'HKEY_CURRENT_USER\Software\Google\Chrome\BLBeacon', '/v', 'version', + ]); + if (result.exitCode == 0) { + final List parts = (result.stdout as String).split(RegExp(r'\s+')); + if (parts.length > 2) { + version = 'Google Chrome ' + parts[parts.length - 2]; + } + } + } else { + final String chrome = chromeLauncher.findExecutable(); + final ProcessResult result = await _processManager.run([ + chrome, + '--version', + ]); + if (result.exitCode == 0) { + version = result.stdout as String; + } + } + return version.trim(); + } + +} + +/// The Microsoft Edge browser based on Chromium. +// This is not currently used, see https://github.com/flutter/flutter/issues/55322 +class MicrosoftEdgeDevice extends ChromiumDevice { + MicrosoftEdgeDevice({ + @required ChromiumLauncher chromiumLauncher, + @required Logger logger, + @required FileSystem fileSystem, + }) : super( + name: 'edge', + chromeLauncher: chromiumLauncher, + logger: logger, + fileSystem: fileSystem, + ); + + @override + String get name => 'Edge'; + + @override + Future get sdkNameAndVersion async => ''; +} + +class WebDevices extends PollingDeviceDiscovery { + WebDevices({ + @required FileSystem fileSystem, + @required Logger logger, + @required Platform platform, + @required ProcessManager processManager, + @required FeatureFlags featureFlags, + }) : _featureFlags = featureFlags, + super('Chrome') { + final OperatingSystemUtils operatingSystemUtils = OperatingSystemUtils( + fileSystem: fileSystem, + platform: platform, + logger: logger, + processManager: processManager, + ); + _chromeDevice = GoogleChromeDevice( + fileSystem: fileSystem, + logger: logger, + platform: platform, + processManager: processManager, + chromiumLauncher: ChromiumLauncher( + browserFinder: findChromeExecutable, + fileSystem: fileSystem, + logger: logger, + platform: platform, + processManager: processManager, + operatingSystemUtils: operatingSystemUtils, + ), + ); + _webServerDevice = WebServerDevice( + logger: logger, + ); + } + + GoogleChromeDevice _chromeDevice; + WebServerDevice _webServerDevice; + final FeatureFlags _featureFlags; @override bool get canListAnything => featureFlags.isWebEnabled; @override Future> pollingGetDevices({ Duration timeout }) async { + if (!_featureFlags.isWebEnabled) { + return []; + } return [ - if (_chromeIsAvailable) - _webDevice, _webServerDevice, + if (_chromeDevice.isSupported()) + _chromeDevice, ]; } @override - bool get supportsPlatform => featureFlags.isWebEnabled; + bool get supportsPlatform => _featureFlags.isWebEnabled; } @visibleForTesting @@ -208,12 +301,17 @@ String parseVersionForWindows(String input) { /// A special device type to allow serving for arbitrary browsers. class WebServerDevice extends Device { - WebServerDevice() : super( - 'web-server', - platformType: PlatformType.web, - category: Category.web, - ephemeral: false, - ); + WebServerDevice({ + @required Logger logger, + }) : _logger = logger, + super( + 'web-server', + platformType: PlatformType.web, + category: Category.web, + ephemeral: false, + ); + + final Logger _logger; @override void clearLogs() { } @@ -244,7 +342,7 @@ class WebServerDevice extends Device { Future get isLocalEmulator async => false; @override - bool isSupported() => featureFlags.isWebEnabled; + bool isSupported() => true; @override bool isSupportedForProject(FlutterProject flutterProject) { @@ -271,11 +369,11 @@ class WebServerDevice extends Device { }) async { final String url = platformArgs['uri'] as String; if (debuggingOptions.startPaused) { - globals.printStatus('Waiting for connection from Dart debug extension at $url', emphasis: true); + _logger.printStatus('Waiting for connection from Dart debug extension at $url', emphasis: true); } else { - globals.printStatus('$mainPath is being served at $url', emphasis: true); + _logger.printStatus('$mainPath is being served at $url', emphasis: true); } - globals.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); } diff --git a/packages/flutter_tools/lib/src/web/web_validator.dart b/packages/flutter_tools/lib/src/web/web_validator.dart index 70cf224b666..43cb00d692f 100644 --- a/packages/flutter_tools/lib/src/web/web_validator.dart +++ b/packages/flutter_tools/lib/src/web/web_validator.dart @@ -5,47 +5,39 @@ import 'package:meta/meta.dart'; import 'package:platform/platform.dart'; -import '../base/file_system.dart'; import '../doctor.dart'; import 'chrome.dart'; -/// A validator that checks whether chrome is installed and can run. -class WebValidator extends DoctorValidator { - const WebValidator({ - @required Platform platform, - @required ChromeLauncher chromeLauncher, - @required FileSystem fileSystem, - }) : _platform = platform, - _chromeLauncher = chromeLauncher, - _fileSystem = fileSystem, - super('Chrome - develop for the web'); +/// A validator for Chromium-based brosers. +abstract class ChromiumValidator extends DoctorValidator { + const ChromiumValidator(String title) : super(title); - final Platform _platform; - final ChromeLauncher _chromeLauncher; - final FileSystem _fileSystem; + Platform get _platform; + ChromiumLauncher get _chromiumLauncher; + String get _name; @override Future validate() async { - final String chrome = findChromeExecutable(_platform, _fileSystem); - final bool canRunChrome = _chromeLauncher.canFindChrome(); + final bool canRunChromium = _chromiumLauncher.canFindExecutable(); + final String chromimSearchLocation = _chromiumLauncher.findExecutable(); final List messages = [ if (_platform.environment.containsKey(kChromeEnvironment)) - if (!canRunChrome) - ValidationMessage.hint('$chrome is not executable.') + if (!canRunChromium) + ValidationMessage.hint('$chromimSearchLocation is not executable.') else - ValidationMessage('$kChromeEnvironment = $chrome') + ValidationMessage('$kChromeEnvironment = $chromimSearchLocation') else - if (!canRunChrome) - const ValidationMessage.hint('Cannot find Chrome. Try setting ' - '$kChromeEnvironment to a Chrome executable.') + if (!canRunChromium) + ValidationMessage.hint('Cannot find $_name. Try setting ' + '$kChromeEnvironment to a $_name executable.') else - ValidationMessage('Chrome at $chrome'), + ValidationMessage('$_name at $chromimSearchLocation'), ]; - if (!canRunChrome) { + if (!canRunChromium) { return ValidationResult( ValidationType.missing, messages, - statusInfo: 'Cannot find chrome executable at $chrome', + statusInfo: 'Cannot find $_name executable at $chromimSearchLocation', ); } return ValidationResult( @@ -54,3 +46,42 @@ class WebValidator extends DoctorValidator { ); } } + +/// A validator that checks whether Chrome is installed and can run. +class ChromeValidator extends ChromiumValidator { + const ChromeValidator({ + @required Platform platform, + @required ChromiumLauncher chromiumLauncher, + }) : _platform = platform, + _chromiumLauncher = chromiumLauncher, + super('Chrome - develop for the web'); + + @override + final Platform _platform; + + @override + final ChromiumLauncher _chromiumLauncher; + + @override + String get _name => 'Chrome'; +} + +/// A validator that checks whethere Edge is installed and can run. +// This is not currently used, see https://github.com/flutter/flutter/issues/55322 +class EdgeValidator extends ChromiumValidator { + const EdgeValidator({ + @required Platform platform, + @required ChromiumLauncher chromiumLauncher, + }) : _platform = platform, + _chromiumLauncher = chromiumLauncher, + super('Edge - develop for the web'); + + @override + final Platform _platform; + + @override + final ChromiumLauncher _chromiumLauncher; + + @override + String get _name => 'Edge'; +} diff --git a/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart b/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart index a39d3feacfe..4be5edab051 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/devices_test.dart @@ -4,92 +4,80 @@ import 'dart:convert'; -import 'package:file/file.dart'; import 'package:args/command_runner.dart'; + import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/device.dart'; -import 'package:flutter_tools/src/web/chrome.dart'; import 'package:flutter_tools/src/commands/devices.dart'; import 'package:flutter_tools/src/base/logger.dart'; -import 'package:flutter_tools/src/base/config.dart'; -import 'package:flutter_tools/src/globals.dart' as globals; +import 'package:flutter_tools/src/device.dart'; +import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/base/context.dart'; +import 'package:flutter_tools/src/web/web_device.dart'; +import 'package:mockito/mockito.dart'; import '../../src/common.dart'; import '../../src/context.dart'; +import '../../src/testbed.dart'; void main() { - group('devices', () { - Directory configDir; - Config config; + setUpAll(() { + Cache.disableLocking(); + }); - tearDown(() { - if (configDir != null) { - tryToDelete(configDir); - configDir = null; - } + testUsingContext('devices can display no connected devices with the --machine flag', () async { + final BufferLogger logger = context.get() as BufferLogger; + final DevicesCommand command = DevicesCommand(); + final CommandRunner runner = createTestCommandRunner(command); + await runner.run(['devices', '--machine']); + + expect( + json.decode(logger.statusText), + isEmpty, + ); + }, overrides: { + FeatureFlags: () => TestFeatureFlags(isWebEnabled: false), + }); + + testUsingContext('devices can display via the --machine flag', () async { + when(deviceManager.refreshAllConnectedDevices()).thenAnswer((Invocation invocation) async { + return [ + WebServerDevice(logger: BufferLogger.test()), + ]; }); - setUpAll(() { - Cache.disableLocking(); - configDir ??= globals.fs.systemTempDirectory.createTempSync( - 'flutter_config_dir_test.', - ); - config = Config.test( - Config.kFlutterSettings, - directory: configDir, - logger: globals.logger, - )..setValue('enable-web', true); - }); + final BufferLogger logger = context.get() as BufferLogger; + final DevicesCommand command = DevicesCommand(); + final CommandRunner runner = createTestCommandRunner(command); + await runner.run(['devices', '--machine']); - // Test assumes no devices connected. - // Should return only `web-server` device - testUsingContext('Test the --machine flag', () async { - final BufferLogger logger = context.get() as BufferLogger; - final DevicesCommand command = DevicesCommand(); - final CommandRunner runner = createTestCommandRunner(command); - await runner.run(['devices', '--machine']); - expect( - json.decode(logger.statusText), - >[ - { - 'name': 'Web Server', - 'id': 'web-server', - 'isSupported': true, - 'targetPlatform': 'web-javascript', - 'emulator': false, - 'sdk': 'Flutter Tools', - 'capabilities': { - 'hotReload': true, - 'hotRestart': true, - 'screenshot': false, - 'fastStart': false, - 'flutterExit': true, - 'hardwareRendering': false, - 'startPaused': true - } + expect( + json.decode(logger.statusText), + contains(equals( + { + 'name': 'Web Server', + 'id': 'web-server', + 'isSupported': true, + 'targetPlatform': 'web-javascript', + 'emulator': false, + 'sdk': 'Flutter Tools', + 'capabilities': { + 'hotReload': true, + 'hotRestart': true, + 'screenshot': false, + 'fastStart': false, + 'flutterExit': true, + 'hardwareRendering': false, + 'startPaused': true } - ] - ); - }, - overrides: { - DeviceManager: () => DeviceManager(), - Config: () => config, - ChromeLauncher: () => _DisabledChromeLauncher(), - }); + } + )), + ); + }, overrides: { + FeatureFlags: () => TestFeatureFlags(isWebEnabled: true), + DeviceManager: () => MockDeviceManager(), }); } -// Without ChromeLauncher DeviceManager constructor fails with noSuchMethodError -// trying to call canFindChrome on null -// Also, Chrome may have different versions on different machines and -// JSON will not match, because the `sdk` field of the Device contains version number -// Mock the launcher to make it appear that we don't have Chrome. -class _DisabledChromeLauncher implements ChromeLauncher { - @override - bool canFindChrome() => false; +class MockDeviceManager extends Mock implements DeviceManager { - @override - Future launch(String url, {bool headless = false, int debugPort, bool skipCheck = false, Directory cacheDir}) - => Future.error('Chrome disabled'); -} +} \ No newline at end of file diff --git a/packages/flutter_tools/test/general.shard/doctor.dart b/packages/flutter_tools/test/general.shard/doctor.dart index 3839adf3441..cc2ae8c75b8 100644 --- a/packages/flutter_tools/test/general.shard/doctor.dart +++ b/packages/flutter_tools/test/general.shard/doctor.dart @@ -44,7 +44,7 @@ void main() { test('doctor validators includes web when feature is enabled', () => testbed.run(() { expect(DoctorValidatorsProvider.defaultInstance.validators, - contains(isA())); + contains(isA())); }, overrides: { FeatureFlags: () => TestFeatureFlags( isWebEnabled: true, @@ -53,7 +53,7 @@ void main() { test('doctor validators does not include web when feature is disabled', () => testbed.run(() { expect(DoctorValidatorsProvider.defaultInstance.validators, - isNot(contains(isA()))); + isNot(contains(isA()))); }, overrides: { FeatureFlags: () => TestFeatureFlags( isWebEnabled: false, diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart index 9444d43fa2c..9b9027ad080 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart @@ -131,14 +131,18 @@ void main() { final MockChromeConnection mockChromeConnection = MockChromeConnection(); final MockChromeTab mockChromeTab = MockChromeTab(); final MockWipConnection mockWipConnection = MockWipConnection(); + final MockChromiumLauncher chromiumLauncher = MockChromiumLauncher(); when(mockChromeConnection.getTab(any)).thenAnswer((Invocation invocation) async { return mockChromeTab; }); when(mockChromeTab.connect()).thenAnswer((Invocation invocation) async { return mockWipConnection; }); + when(chromiumLauncher.connectedInstance).thenAnswer((Invocation invocation) async { + return chrome; + }); when(chrome.chromeConnection).thenReturn(mockChromeConnection); - launchChromeInstance(chrome); + when(chromeDevice.chromeLauncher).thenReturn(chromiumLauncher); when(mockFlutterDevice.device).thenReturn(chromeDevice); final Completer connectionInfoCompleter = Completer(); unawaited(residentWebRunner.run( @@ -163,10 +167,11 @@ class MockDebugConnection extends Mock implements DebugConnection {} class MockVmService extends Mock implements VmService {} class MockStatus extends Mock implements Status {} class MockFlutterDevice extends Mock implements FlutterDevice {} -class MockChromeDevice extends Mock implements ChromeDevice {} -class MockChrome extends Mock implements Chrome {} +class MockChromeDevice extends Mock implements ChromiumDevice {} +class MockChrome extends Mock implements Chromium {} class MockChromeConnection extends Mock implements ChromeConnection {} class MockChromeTab extends Mock implements ChromeTab {} class MockWipConnection extends Mock implements WipConnection {} class MockBuildSystem extends Mock implements BuildSystem {} class MockPub extends Mock implements Pub {} +class MockChromiumLauncher extends Mock implements ChromiumLauncher {} diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart index 30362a7434b..6eee507ddfd 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart @@ -93,7 +93,6 @@ void main() { FakeVmServiceHost fakeVmServiceHost; setUp(() { - resetChromeForTesting(); mockDebugConnection = MockDebugConnection(); mockDevice = MockDevice(); mockAppConnection = MockAppConnection(); @@ -173,8 +172,10 @@ void main() { } test('runner with web server device does not support debugging without --start-paused', () => testbed.run(() { + when(mockFlutterDevice.device).thenReturn(WebServerDevice( + logger: BufferLogger.test(), + )); fakeVmServiceHost = FakeVmServiceHost(requests: []); - when(mockFlutterDevice.device).thenReturn(WebServerDevice()); final ResidentRunner profileResidentWebRunner = DwdsWebRunnerFactory().createWebRunner( mockFlutterDevice, flutterProject: FlutterProject.current(), @@ -194,7 +195,9 @@ void main() { test('runner with web server device supports debugging with --start-paused', () => testbed.run(() { fakeVmServiceHost = FakeVmServiceHost(requests: []); _setupMocks(); - when(mockFlutterDevice.device).thenReturn(WebServerDevice()); + when(mockFlutterDevice.device).thenReturn(WebServerDevice( + logger: BufferLogger.test(), + )); final ResidentRunner profileResidentWebRunner = DwdsWebRunnerFactory().createWebRunner( mockFlutterDevice, flutterProject: FlutterProject.current(), @@ -355,7 +358,23 @@ void main() { ), ]); _setupMocks(); - launchChromeInstance(mockChrome); + final ChromiumLauncher chromiumLauncher = MockChromeLauncher(); + when(chromiumLauncher.launch(any, cacheDir: anyNamed('cacheDir'))) + .thenAnswer((Invocation invocation) async { + return mockChrome; + }); + when(chromiumLauncher.connectedInstance).thenAnswer((Invocation invocation) async { + return mockChrome; + }); + when(mockFlutterDevice.device).thenReturn(GoogleChromeDevice( + fileSystem: globals.fs, + chromiumLauncher: chromiumLauncher, + logger: globals.logger, + platform: FakePlatform(operatingSystem: 'linux'), + processManager: FakeProcessManager.any(), + )); + when(chromiumLauncher.canFindExecutable()).thenReturn(true); + chromiumLauncher.testLaunchChromium(mockChrome); when(mockWebDevFS.update( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -395,7 +414,7 @@ void main() { expect(config, allOf([ containsPair('cd27', 'web-javascript'), - containsPair('cd28', null), + containsPair('cd28', ''), containsPair('cd29', 'false'), containsPair('cd30', 'true'), ])); @@ -417,7 +436,23 @@ void main() { ), ]); _setupMocks(); - launchChromeInstance(mockChrome); + final ChromiumLauncher chromiumLauncher = MockChromeLauncher(); + when(chromiumLauncher.launch(any, cacheDir: anyNamed('cacheDir'))) + .thenAnswer((Invocation invocation) async { + return mockChrome; + }); + when(chromiumLauncher.connectedInstance).thenAnswer((Invocation invocation) async { + return mockChrome; + }); + when(chromiumLauncher.canFindExecutable()).thenReturn(true); + when(mockFlutterDevice.device).thenReturn(GoogleChromeDevice( + fileSystem: globals.fs, + chromiumLauncher: chromiumLauncher, + logger: globals.logger, + platform: FakePlatform(operatingSystem: 'linux'), + processManager: FakeProcessManager.any(), + )); + chromiumLauncher.testLaunchChromium(mockChrome); Uri entrypointFileUri; when(mockWebDevFS.update( mainUri: anyNamed('mainUri'), @@ -461,7 +496,7 @@ void main() { expect(config, allOf([ containsPair('cd27', 'web-javascript'), - containsPair('cd28', null), + containsPair('cd28', ''), containsPair('cd29', 'false'), containsPair('cd30', 'true'), ])); @@ -1062,7 +1097,22 @@ void main() { ) ]); _setupMocks(); - when(mockFlutterDevice.device).thenReturn(ChromeDevice()); + final ChromiumLauncher chromiumLauncher = MockChromeLauncher(); + when(chromiumLauncher.launch(any, cacheDir: anyNamed('cacheDir'))) + .thenAnswer((Invocation invocation) async { + return mockChrome; + }); + when(chromiumLauncher.connectedInstance).thenAnswer((Invocation invocation) async { + return mockChrome; + }); + when(mockFlutterDevice.device).thenReturn(GoogleChromeDevice( + fileSystem: globals.fs, + chromiumLauncher: chromiumLauncher, + logger: globals.logger, + platform: FakePlatform(operatingSystem: 'linux'), + processManager: FakeProcessManager.any(), + )); + when(chromiumLauncher.canFindExecutable()).thenReturn(true); when(mockWebDevFS.create()).thenAnswer((Invocation invocation) async { return Uri.parse('http://localhost:8765/app/'); }); @@ -1077,7 +1127,7 @@ void main() { return mockWipConnection; }); when(chrome.chromeConnection).thenReturn(mockChromeConnection); - launchChromeInstance(chrome); + chromiumLauncher.testLaunchChromium(chrome); final DelegateLogger delegateLogger = globals.logger as DelegateLogger; final MockStatus mockStatus = MockStatus(); @@ -1110,13 +1160,14 @@ void main() { expect(fakeVmServiceHost.hasRemainingExpectations, false); }, overrides: { Logger: () => DelegateLogger(BufferLogger.test()), - ChromeLauncher: () => MockChromeLauncher(), })); test('Sends unlaunched app.webLaunchUrl event for Web Server device', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: []); _setupMocks(); - when(mockFlutterDevice.device).thenReturn(WebServerDevice()); + when(mockFlutterDevice.device).thenReturn(WebServerDevice( + logger: globals.logger, + )); when(mockWebDevFS.create()).thenAnswer((Invocation invocation) async { return Uri.parse('http://localhost:8765/app/'); }); @@ -1219,9 +1270,9 @@ void main() { })); } -class MockChromeLauncher extends Mock implements ChromeLauncher {} +class MockChromeLauncher extends Mock implements ChromiumLauncher {} class MockFlutterUsage extends Mock implements Usage {} -class MockChromeDevice extends Mock implements ChromeDevice {} +class MockChromeDevice extends Mock implements ChromiumDevice {} class MockDebugConnection extends Mock implements DebugConnection {} class MockAppConnection extends Mock implements AppConnection {} class MockVmService extends Mock implements VmService {} @@ -1229,7 +1280,7 @@ class MockStatus extends Mock implements Status {} class MockFlutterDevice extends Mock implements FlutterDevice {} class MockWebDevFS extends Mock implements WebDevFS {} class MockResidentCompiler extends Mock implements ResidentCompiler {} -class MockChrome extends Mock implements Chrome {} +class MockChrome extends Mock implements Chromium {} class MockChromeConnection extends Mock implements ChromeConnection {} class MockChromeTab extends Mock implements ChromeTab {} class MockWipConnection extends Mock implements WipConnection {} diff --git a/packages/flutter_tools/test/general.shard/web/chrome_test.dart b/packages/flutter_tools/test/general.shard/web/chrome_test.dart index 2ee9d2c7f90..fef3b822aa2 100644 --- a/packages/flutter_tools/test/general.shard/web/chrome_test.dart +++ b/packages/flutter_tools/test/general.shard/web/chrome_test.dart @@ -5,7 +5,7 @@ import 'dart:async'; import 'package:file/memory.dart'; -import 'package:flutter_tools/src/base/common.dart'; +import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/os.dart'; @@ -16,7 +16,7 @@ import 'package:platform/platform.dart'; import '../../src/common.dart'; import '../../src/context.dart'; -const List _kChromeArgs = [ +const List kChromeArgs = [ '--disable-background-timer-throttling', '--disable-extensions', '--disable-popup-blocking', @@ -30,7 +30,7 @@ const List _kChromeArgs = [ const String kDevtoolsStderr = '\n\nDevTools listening\n\n'; void main() { - ChromeLauncher chromeLauncher; + ChromiumLauncher chromeLauncher; FileSystem fileSystem; Platform platform; FakeProcessManager processManager; @@ -49,66 +49,91 @@ void main() { }); fileSystem = MemoryFileSystem.test(); processManager = FakeProcessManager.list([]); - chromeLauncher = ChromeLauncher( + chromeLauncher = ChromiumLauncher( fileSystem: fileSystem, platform: platform, processManager: processManager, operatingSystemUtils: operatingSystemUtils, logger: logger, + browserFinder: findChromeExecutable, ); }); - tearDown(() { - resetChromeForTesting(); + testWithoutContext('can launch chrome and connect to the devtools', () async { + expect( + () async => await testLaunchChrome( + '/.tmp_rand0/flutter_tools_chrome_device.rand0', + processManager, + chromeLauncher, + ), + returnsNormally, + ); }); - test('can launch chrome and connect to the devtools', () async { - await testLaunchChrome('/.tmp_rand0/flutter_tools_chrome_device.rand0', processManager, chromeLauncher); + testWithoutContext('cannot have two concurrent instances of chrome', () async { + await testLaunchChrome( + '/.tmp_rand0/flutter_tools_chrome_device.rand0', + processManager, + chromeLauncher, + ); + + expect( + () async => await testLaunchChrome( + '/.tmp_rand0/flutter_tools_chrome_device.rand1', + processManager, + chromeLauncher, + ), + throwsToolExit(), + ); }); - test('cannot have two concurrent instances of chrome', () async { - await testLaunchChrome('/.tmp_rand0/flutter_tools_chrome_device.rand0', processManager, chromeLauncher); - bool pass = false; - try { - await testLaunchChrome('/.tmp_rand0/flutter_tools_chrome_device.rand1', processManager, chromeLauncher); - } on ToolExit catch (_) { - pass = true; - } - expect(pass, isTrue); - }); - - test('can launch new chrome after stopping a previous chrome', () async { - final Chrome chrome = await testLaunchChrome('/.tmp_rand0/flutter_tools_chrome_device.rand0', processManager, chromeLauncher); + testWithoutContext('can launch new chrome after stopping a previous chrome', () async { + final Chromium chrome = await testLaunchChrome( + '/.tmp_rand0/flutter_tools_chrome_device.rand0', + processManager, + chromeLauncher, + ); await chrome.close(); - await testLaunchChrome('/.tmp_rand0/flutter_tools_chrome_device.rand1', processManager, chromeLauncher); + + expect( + () async => await testLaunchChrome( + '/.tmp_rand0/flutter_tools_chrome_device.rand1', + processManager, + chromeLauncher, + ), + returnsNormally, + ); }); - test('can launch chrome with a custom debug port', () async { + testWithoutContext('can launch chrome with a custom debug port', () async { processManager.addCommand(const FakeCommand( command: [ 'example_chrome', '--user-data-dir=/.tmp_rand1/flutter_tools_chrome_device.rand1', '--remote-debugging-port=10000', - ..._kChromeArgs, + ...kChromeArgs, 'example_url', ], stderr: kDevtoolsStderr, )); - await chromeLauncher.launch( - 'example_url', - skipCheck: true, - debugPort: 10000, + expect( + () async => await chromeLauncher.launch( + 'example_url', + skipCheck: true, + debugPort: 10000, + ), + returnsNormally, ); }); - test('can launch chrome headless', () async { + testWithoutContext('can launch chrome headless', () async { processManager.addCommand(const FakeCommand( command: [ 'example_chrome', '--user-data-dir=/.tmp_rand1/flutter_tools_chrome_device.rand1', '--remote-debugging-port=1234', - ..._kChromeArgs, + ...kChromeArgs, '--headless', '--disable-gpu', '--no-sandbox', @@ -118,14 +143,17 @@ void main() { stderr: kDevtoolsStderr, )); - await chromeLauncher.launch( - 'example_url', - skipCheck: true, - headless: true, + expect( + () async => await chromeLauncher.launch( + 'example_url', + skipCheck: true, + headless: true, + ), + returnsNormally, ); }); - test('can seed chrome temp directory with existing session data', () async { + testWithoutContext('can seed chrome temp directory with existing session data', () async { final Completer exitCompleter = Completer.sync(); final Directory dataDir = fileSystem.directory('chrome-stuff'); @@ -148,7 +176,7 @@ void main() { 'example_chrome', '--user-data-dir=/.tmp_rand1/flutter_tools_chrome_device.rand1', '--remote-debugging-port=1234', - ..._kChromeArgs, + ...kChromeArgs, 'example_url', ], completer: exitCompleter)); @@ -183,23 +211,23 @@ void main() { expect(storageDir.existsSync(), true); - expect(storageDir.childFile('LOCK').existsSync(), true); + expect(storageDir.childFile('LOCK'), exists); expect(storageDir.childFile('LOCK').readAsBytesSync(), hasLength(0)); - expect(storageDir.childFile('LOG').existsSync(), true); + expect(storageDir.childFile('LOG'), exists); expect(storageDir.childFile('LOG').readAsStringSync(), 'contents'); }); } class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {} -Future testLaunchChrome(String userDataDir, FakeProcessManager processManager, ChromeLauncher chromeLauncher) { +Future testLaunchChrome(String userDataDir, FakeProcessManager processManager, ChromiumLauncher chromeLauncher) { processManager.addCommand(FakeCommand( command: [ 'example_chrome', '--user-data-dir=$userDataDir', '--remote-debugging-port=1234', - ..._kChromeArgs, + ...kChromeArgs, 'example_url', ], stderr: kDevtoolsStderr, diff --git a/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart b/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart index db565fe851c..5b6ceb2beda 100644 --- a/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart @@ -376,6 +376,7 @@ void main() { entrypoint: Uri.base, testMode: true, expressionCompiler: null, + chromiumLauncher: null, ); webDevFS.requireJS.createSync(recursive: true); webDevFS.stackTraceMapper.createSync(recursive: true); @@ -469,6 +470,7 @@ void main() { entrypoint: Uri.base, testMode: true, expressionCompiler: null, + chromiumLauncher: null, ); webDevFS.requireJS.createSync(recursive: true); webDevFS.stackTraceMapper.createSync(recursive: true); @@ -482,6 +484,7 @@ void main() { test('Launches DWDS with the correct arguments', () => testbed.run(() async { globals.fs.file('.packages').writeAsStringSync('\n'); final WebAssetServer server = await WebAssetServer.start( + null, 'any', 8123, (String url) => null, 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 e29c62dce97..7422d62dfc8 100644 --- a/packages/flutter_tools/test/general.shard/web/devices_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devices_test.dart @@ -2,36 +2,42 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:flutter_tools/src/base/io.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/web/chrome.dart'; import 'package:flutter_tools/src/web/web_device.dart'; import 'package:mockito/mockito.dart'; -import 'package:process/process.dart'; import 'package:platform/platform.dart'; import '../../src/common.dart'; import '../../src/context.dart'; +import '../../src/testbed.dart'; void main() { - MockChromeLauncher mockChromeLauncher; - MockPlatform mockPlatform; - MockProcessManager mockProcessManager; - MockWebApplicationPackage mockWebApplicationPackage; + testWithoutContext('No web devices listed if feature is disabled', () async { + final WebDevices webDevices = WebDevices( + featureFlags: TestFeatureFlags(isWebEnabled: false), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform( + operatingSystem: 'linux', + environment: {} + ), + processManager: FakeProcessManager.any(), + ); - setUp(() async { - mockWebApplicationPackage = MockWebApplicationPackage(); - mockProcessManager = MockProcessManager(); - mockChromeLauncher = MockChromeLauncher(); - mockPlatform = MockPlatform(); - when(mockChromeLauncher.launch(any)).thenAnswer((Invocation invocation) async { - return null; - }); - when(mockWebApplicationPackage.name).thenReturn('test'); + expect(await webDevices.pollingGetDevices(), isEmpty); }); - test('Chrome defaults', () async { - final ChromeDevice chromeDevice = ChromeDevice(); + testWithoutContext('GoogleChromeDevice defaults', () async { + final GoogleChromeDevice chromeDevice = GoogleChromeDevice( + chromiumLauncher: null, + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform(operatingSystem: 'linux'), + processManager: FakeProcessManager.any(), + ); expect(chromeDevice.name, 'Chrome'); expect(chromeDevice.id, 'chrome'); @@ -41,13 +47,35 @@ void main() { expect(chromeDevice.supportsFlutterExit, true); expect(chromeDevice.supportsScreenshot, false); expect(await chromeDevice.isLocalEmulator, false); - expect(chromeDevice.getLogReader(app: mockWebApplicationPackage), isA()); + expect(chromeDevice.getLogReader(), isA()); expect(chromeDevice.getLogReader(), isA()); expect(await chromeDevice.portForwarder.forward(1), 1); }); - test('Server defaults', () async { - final WebServerDevice device = WebServerDevice(); + testWithoutContext('MicrosoftEdge defaults', () async { + final MicrosoftEdgeDevice chromeDevice = MicrosoftEdgeDevice( + chromiumLauncher: null, + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + ); + + expect(chromeDevice.name, 'Edge'); + expect(chromeDevice.id, 'edge'); + expect(chromeDevice.supportsHotReload, true); + expect(chromeDevice.supportsHotRestart, true); + expect(chromeDevice.supportsStartPaused, true); + expect(chromeDevice.supportsFlutterExit, true); + expect(chromeDevice.supportsScreenshot, false); + expect(await chromeDevice.isLocalEmulator, false); + expect(chromeDevice.getLogReader(), isA()); + expect(chromeDevice.getLogReader(), isA()); + expect(await chromeDevice.portForwarder.forward(1), 1); + }); + + testWithoutContext('Server defaults', () async { + final WebServerDevice device = WebServerDevice( + logger: BufferLogger.test(), + ); expect(device.name, 'Web Server'); expect(device.id, 'web-server'); @@ -57,95 +85,130 @@ void main() { expect(device.supportsFlutterExit, true); expect(device.supportsScreenshot, false); expect(await device.isLocalEmulator, false); - expect(device.getLogReader(app: mockWebApplicationPackage), isA()); + expect(device.getLogReader(), isA()); expect(device.getLogReader(), isA()); expect(await device.portForwarder.forward(1), 1); }); - testUsingContext('Chrome device is listed when Chrome is available', () async { - when(mockChromeLauncher.canFindChrome()).thenReturn(true); + testWithoutContext('Chrome device is listed when Chrome can be run', () async { + final WebDevices webDevices = WebDevices( + featureFlags: TestFeatureFlags(isWebEnabled: true), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform( + operatingSystem: 'linux', + environment: {} + ), + processManager: FakeProcessManager.any(), + ); - final WebDevices deviceDiscoverer = WebDevices(); - final List devices = await deviceDiscoverer.pollingGetDevices(); - expect(devices, contains(isA())); - }, overrides: { - ChromeLauncher: () => mockChromeLauncher, + expect(await webDevices.pollingGetDevices(), + contains(isA())); }); - testUsingContext('Chrome device is not listed when Chrome is not available', () async { - when(mockChromeLauncher.canFindChrome()).thenReturn(false); + testWithoutContext('Chrome device is not listed when Chrome cannot be run', () async { + final MockProcessManager processManager = MockProcessManager(); + when(processManager.canRun(any)).thenReturn(false); + final WebDevices webDevices = WebDevices( + featureFlags: TestFeatureFlags(isWebEnabled: true), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform( + operatingSystem: 'linux', + environment: {} + ), + processManager: processManager, + ); - final WebDevices deviceDiscoverer = WebDevices(); - final List devices = await deviceDiscoverer.pollingGetDevices(); - expect(devices, isNot(contains(isA()))); - }, overrides: { - ChromeLauncher: () => mockChromeLauncher, + expect(await webDevices.pollingGetDevices(), + isNot(contains(isA()))); }); - testUsingContext('Web Server device is listed even when Chrome is not available', () async { - when(mockChromeLauncher.canFindChrome()).thenReturn(false); + testWithoutContext('Web Server device is listed by default', () async { + final WebDevices webDevices = WebDevices( + featureFlags: TestFeatureFlags(isWebEnabled: true), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform( + operatingSystem: 'linux', + environment: {} + ), + processManager: FakeProcessManager.any(), + ); - final WebDevices deviceDiscoverer = WebDevices(); - final List devices = await deviceDiscoverer.pollingGetDevices(); - expect(devices, contains(isA())); - }, overrides: { - ChromeLauncher: () => mockChromeLauncher, + expect(await webDevices.pollingGetDevices(), + contains(isA())); }); - testUsingContext('Chrome invokes version command on non-Windows platforms', () async{ - when(mockPlatform.isWindows).thenReturn(false); - when(mockProcessManager.canRun('chrome.foo')).thenReturn(true); - when(mockProcessManager.run(['chrome.foo', '--version'])).thenAnswer((Invocation invocation) async { - return MockProcessResult(0, 'ABC'); - }); - final ChromeDevice chromeDevice = ChromeDevice(); + testWithoutContext('Chrome invokes version command on non-Windows platforms', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand( + command: [ + kLinuxExecutable, + '--version', + ], + stdout: 'ABC' + ) + ]); + final WebDevices webDevices = WebDevices( + featureFlags: TestFeatureFlags(isWebEnabled: true), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform( + operatingSystem: 'linux', + environment: {} + ), + processManager: processManager, + ); + + + final GoogleChromeDevice chromeDevice = (await webDevices.pollingGetDevices()) + .whereType().first; expect(chromeDevice.isSupported(), true); expect(await chromeDevice.sdkNameAndVersion, 'ABC'); // Verify caching works correctly. expect(await chromeDevice.sdkNameAndVersion, 'ABC'); - verify(mockProcessManager.run(['chrome.foo', '--version'])).called(1); - }, overrides: { - Platform: () => mockPlatform, - ProcessManager: () => mockProcessManager, + expect(processManager.hasRemainingExpectations, false); }); - testUsingContext('Chrome invokes different version command on windows.', () async { - when(mockPlatform.isWindows).thenReturn(true); - when(mockProcessManager.canRun('chrome.foo')).thenReturn(true); - when(mockProcessManager.run([ - 'reg', - 'query', - r'HKEY_CURRENT_USER\Software\Google\Chrome\BLBeacon', - '/v', - 'version', - ])).thenAnswer((Invocation invocation) async { - return MockProcessResult(0, r'HKEY_CURRENT_USER\Software\Google\Chrome\BLBeacon\ version REG_SZ 74.0.0 A'); - }); - final ChromeDevice chromeDevice = ChromeDevice(); + testWithoutContext('Chrome version check invokes registry query on windows.', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand( + command: [ + 'reg', + 'query', + r'HKEY_CURRENT_USER\Software\Google\Chrome\BLBeacon', + '/v', + 'version', + ], + stdout: r'HKEY_CURRENT_USER\Software\Google\Chrome\BLBeacon\ version REG_SZ 74.0.0 A', + ) + ]); + final WebDevices webDevices = WebDevices( + featureFlags: TestFeatureFlags(isWebEnabled: true), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + platform: FakePlatform( + operatingSystem: 'windows', + environment: {} + ), + processManager: processManager, + ); + + + final GoogleChromeDevice chromeDevice = (await webDevices.pollingGetDevices()) + .whereType().first; expect(chromeDevice.isSupported(), true); expect(await chromeDevice.sdkNameAndVersion, 'Google Chrome 74.0.0'); - }, overrides: { - Platform: () => mockPlatform, - ProcessManager: () => mockProcessManager, + + // Verify caching works correctly. + expect(await chromeDevice.sdkNameAndVersion, 'Google Chrome 74.0.0'); + expect(processManager.hasRemainingExpectations, false); }); } -class MockChromeLauncher extends Mock implements ChromeLauncher {} -class MockPlatform extends Mock implements Platform { - @override - Map environment = {'FLUTTER_WEB': 'true', kChromeEnvironment: 'chrome.foo'}; -} +// This is used to set `canRun` to false in a test. class MockProcessManager extends Mock implements ProcessManager {} -class MockProcessResult extends Mock implements ProcessResult { - MockProcessResult(this.exitCode, this.stdout); - - @override - final int exitCode; - - @override - final String stdout; -} -class MockWebApplicationPackage extends Mock implements WebApplicationPackage {} diff --git a/packages/flutter_tools/test/general.shard/web/web_validator_test.dart b/packages/flutter_tools/test/general.shard/web/web_validator_test.dart index 2adcbd8e912..a6a6ca8ba67 100644 --- a/packages/flutter_tools/test/general.shard/web/web_validator_test.dart +++ b/packages/flutter_tools/test/general.shard/web/web_validator_test.dart @@ -4,6 +4,7 @@ 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/doctor.dart'; import 'package:flutter_tools/src/web/chrome.dart'; import 'package:flutter_tools/src/web/web_validator.dart'; @@ -17,9 +18,9 @@ import '../../src/fake_process_manager.dart'; void main() { Platform platform; ProcessManager processManager; - ChromeLauncher chromeLauncher; + ChromiumLauncher chromeLauncher; FileSystem fileSystem; - WebValidator webValidator; + ChromiumValidator webValidator; setUp(() { fileSystem = MemoryFileSystem.test(); @@ -28,17 +29,17 @@ void main() { operatingSystem: 'macos', environment: {}, ); - chromeLauncher = ChromeLauncher( + chromeLauncher = ChromiumLauncher( fileSystem: fileSystem, platform: platform, processManager: processManager, operatingSystemUtils: null, - logger: null, + logger: BufferLogger.test(), + browserFinder: findChromeExecutable, ); - webValidator = webValidator = WebValidator( + webValidator = webValidator = ChromeValidator( platform: platform, - chromeLauncher: chromeLauncher, - fileSystem: fileSystem, + chromiumLauncher: chromeLauncher, ); });