Unverified Commit 65a79412 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] reduce globals in web validator and chrome launcher (#51443)

parent 64c5a434
...@@ -101,7 +101,13 @@ Future<T> runInContext<T>( ...@@ -101,7 +101,13 @@ Future<T> runInContext<T>(
logger: globals.logger, logger: globals.logger,
platform: globals.platform, platform: globals.platform,
), ),
ChromeLauncher: () => const ChromeLauncher(), ChromeLauncher: () => ChromeLauncher(
fileSystem: globals.fs,
processManager: globals.processManager,
logger: globals.logger,
operatingSystemUtils: globals.os,
platform: globals.platform,
),
CocoaPods: () => CocoaPods(), CocoaPods: () => CocoaPods(),
CocoaPodsValidator: () => const CocoaPodsValidator(), CocoaPodsValidator: () => const CocoaPodsValidator(),
Config: () => Config( Config: () => Config(
......
...@@ -73,7 +73,11 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { ...@@ -73,7 +73,11 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
if (iosWorkflow.appliesToHostPlatform || macOSWorkflow.appliesToHostPlatform) if (iosWorkflow.appliesToHostPlatform || macOSWorkflow.appliesToHostPlatform)
GroupedValidator(<DoctorValidator>[xcodeValidator, cocoapodsValidator]), GroupedValidator(<DoctorValidator>[xcodeValidator, cocoapodsValidator]),
if (webWorkflow.appliesToHostPlatform) if (webWorkflow.appliesToHostPlatform)
const WebValidator(), WebValidator(
chromeLauncher: globals.chromeLauncher,
platform: globals.platform,
fileSystem: globals.fs,
),
if (linuxWorkflow.appliesToHostPlatform) if (linuxWorkflow.appliesToHostPlatform)
LinuxDoctorValidator(), LinuxDoctorValidator(),
if (windowsWorkflow.appliesToHostPlatform) if (windowsWorkflow.appliesToHostPlatform)
......
...@@ -25,6 +25,7 @@ import 'ios/mac.dart'; ...@@ -25,6 +25,7 @@ import 'ios/mac.dart';
import 'macos/xcode.dart'; import 'macos/xcode.dart';
import 'persistent_tool_state.dart'; import 'persistent_tool_state.dart';
import 'version.dart'; import 'version.dart';
import 'web/chrome.dart';
Artifacts get artifacts => context.get<Artifacts>(); Artifacts get artifacts => context.get<Artifacts>();
Cache get cache => context.get<Cache>(); Cache get cache => context.get<Cache>();
...@@ -148,3 +149,6 @@ final AnsiTerminal _defaultAnsiTerminal = AnsiTerminal( ...@@ -148,3 +149,6 @@ final AnsiTerminal _defaultAnsiTerminal = AnsiTerminal(
/// The global Stdio wrapper. /// The global Stdio wrapper.
Stdio get stdio => context.get<Stdio>() ?? const Stdio(); Stdio get stdio => context.get<Stdio>() ?? const Stdio();
/// The [ChromeLauncher] instance.
ChromeLauncher get chromeLauncher => context.get<ChromeLauncher>();
...@@ -642,7 +642,7 @@ class BrowserManager { ...@@ -642,7 +642,7 @@ class BrowserManager {
bool headless = true, bool headless = true,
}) async { }) async {
final Chrome chrome = final Chrome chrome =
await chromeLauncher.launch(url.toString(), headless: headless); await globals.chromeLauncher.launch(url.toString(), headless: headless);
final Completer<BrowserManager> completer = Completer<BrowserManager>(); final Completer<BrowserManager> completer = Completer<BrowserManager>();
......
...@@ -5,17 +5,16 @@ ...@@ -5,17 +5,16 @@
import 'dart:async'; import 'dart:async';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:platform/platform.dart';
import 'package:process/process.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
import '../base/common.dart'; import '../base/common.dart';
import '../base/context.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/io.dart'; import '../base/io.dart';
import '../base/logger.dart';
import '../base/os.dart';
import '../convert.dart'; import '../convert.dart';
import '../globals.dart' as globals;
/// The [ChromeLauncher] instance.
ChromeLauncher get chromeLauncher => context.get<ChromeLauncher>();
/// An environment variable used to override the location of chrome. /// An environment variable used to override the location of chrome.
const String kChromeEnvironment = 'CHROME_EXECUTABLE'; const String kChromeEnvironment = 'CHROME_EXECUTABLE';
...@@ -30,37 +29,36 @@ const String kMacOSExecutable = ...@@ -30,37 +29,36 @@ const String kMacOSExecutable =
/// The expected executable name on Windows. /// The expected executable name on Windows.
const String kWindowsExecutable = r'Google\Chrome\Application\chrome.exe'; const String kWindowsExecutable = r'Google\Chrome\Application\chrome.exe';
/// The possible locations where the chrome executable can be located on windows.
final List<String> kWindowsPrefixes = <String>[
globals.platform.environment['LOCALAPPDATA'],
globals.platform.environment['PROGRAMFILES'],
globals.platform.environment['PROGRAMFILES(X86)'],
];
/// Find the chrome executable on the current platform. /// Find the chrome executable on the current platform.
/// ///
/// Does not verify whether the executable exists. /// Does not verify whether the executable exists.
String findChromeExecutable() { String findChromeExecutable(Platform platform, FileSystem fileSystem) {
if (globals.platform.environment.containsKey(kChromeEnvironment)) { if (platform.environment.containsKey(kChromeEnvironment)) {
return globals.platform.environment[kChromeEnvironment]; return platform.environment[kChromeEnvironment];
} }
if (globals.platform.isLinux) { if (platform.isLinux) {
return kLinuxExecutable; return kLinuxExecutable;
} }
if (globals.platform.isMacOS) { if (platform.isMacOS) {
return kMacOSExecutable; return kMacOSExecutable;
} }
if (globals.platform.isWindows) { if (platform.isWindows) {
/// The possible locations where the chrome executable can be located on windows.
final List<String> kWindowsPrefixes = <String>[
platform.environment['LOCALAPPDATA'],
platform.environment['PROGRAMFILES'],
platform.environment['PROGRAMFILES(X86)'],
];
final String windowsPrefix = kWindowsPrefixes.firstWhere((String prefix) { final String windowsPrefix = kWindowsPrefixes.firstWhere((String prefix) {
if (prefix == null) { if (prefix == null) {
return false; return false;
} }
final String path = globals.fs.path.join(prefix, kWindowsExecutable); final String path = fileSystem.path.join(prefix, kWindowsExecutable);
return globals.fs.file(path).existsSync(); return fileSystem.file(path).existsSync();
}, orElse: () => '.'); }, orElse: () => '.');
return globals.fs.path.join(windowsPrefix, kWindowsExecutable); return fileSystem.path.join(windowsPrefix, kWindowsExecutable);
} }
throwToolExit('Platform ${globals.platform.operatingSystem} is not supported.'); throwToolExit('Platform ${platform.operatingSystem} is not supported.');
return null; return null;
} }
...@@ -76,7 +74,23 @@ void launchChromeInstance(Chrome chrome) { ...@@ -76,7 +74,23 @@ void launchChromeInstance(Chrome chrome) {
/// Responsible for launching chrome with devtools configured. /// Responsible for launching chrome with devtools configured.
class ChromeLauncher { class ChromeLauncher {
const ChromeLauncher(); const ChromeLauncher({
@required FileSystem fileSystem,
@required Platform platform,
@required ProcessManager processManager,
@required OperatingSystemUtils operatingSystemUtils,
@required Logger logger,
}) : _fileSystem = fileSystem,
_platform = platform,
_processManager = processManager,
_operatingSystemUtils = operatingSystemUtils,
_logger = logger;
final FileSystem _fileSystem;
final Platform _platform;
final ProcessManager _processManager;
final OperatingSystemUtils _operatingSystemUtils;
final Logger _logger;
static bool get hasChromeInstance => _currentCompleter.isCompleted; static bool get hasChromeInstance => _currentCompleter.isCompleted;
...@@ -84,9 +98,9 @@ class ChromeLauncher { ...@@ -84,9 +98,9 @@ class ChromeLauncher {
/// Whether we can locate the chrome executable. /// Whether we can locate the chrome executable.
bool canFindChrome() { bool canFindChrome() {
final String chrome = findChromeExecutable(); final String chrome = findChromeExecutable(_platform, _fileSystem);
try { try {
return globals.processManager.canRun(chrome); return _processManager.canRun(chrome);
} on ArgumentError { } on ArgumentError {
return false; return false;
} }
...@@ -105,14 +119,14 @@ class ChromeLauncher { ...@@ -105,14 +119,14 @@ class ChromeLauncher {
// This is a JSON file which contains configuration from the // This is a JSON file which contains configuration from the
// browser session, such as window position. It is located // browser session, such as window position. It is located
// under the Chrome data-dir folder. // under the Chrome data-dir folder.
final String preferencesPath = globals.fs.path.join('Default', 'preferences'); final String preferencesPath = _fileSystem.path.join('Default', 'preferences');
final String chromeExecutable = findChromeExecutable(); final String chromeExecutable = findChromeExecutable(_platform, _fileSystem);
final Directory activeDataDir = globals.fs.systemTempDirectory.createTempSync('flutter_tool.'); final Directory activeDataDir = _fileSystem.systemTempDirectory.createTempSync('flutter_tool.');
// Seed data dir with previous state. // Seed data dir with previous state.
final File savedPreferencesFile = globals.fs.file(globals.fs.path.join(dataDir?.path ?? '', preferencesPath)); final File savedPreferencesFile = _fileSystem.file(_fileSystem.path.join(dataDir?.path ?? '', preferencesPath));
final File destinationFile = globals.fs.file(globals.fs.path.join(activeDataDir.path, preferencesPath)); final File destinationFile = _fileSystem.file(_fileSystem.path.join(activeDataDir.path, preferencesPath));
if (dataDir != null) { if (dataDir != null) {
if (savedPreferencesFile.existsSync()) { if (savedPreferencesFile.existsSync()) {
destinationFile.parent.createSync(recursive: true); destinationFile.parent.createSync(recursive: true);
...@@ -120,7 +134,7 @@ class ChromeLauncher { ...@@ -120,7 +134,7 @@ class ChromeLauncher {
} }
} }
final int port = debugPort ?? await globals.os.findFreePort(); final int port = debugPort ?? await _operatingSystemUtils.findFreePort();
final List<String> args = <String>[ final List<String> args = <String>[
chromeExecutable, chromeExecutable,
// Using a tmp directory ensures that a new instance of chrome launches // Using a tmp directory ensures that a new instance of chrome launches
...@@ -143,7 +157,7 @@ class ChromeLauncher { ...@@ -143,7 +157,7 @@ class ChromeLauncher {
url, url,
]; ];
final Process process = await globals.processManager.start(args); final Process process = await _processManager.start(args);
// When the process exits, copy the user settings back to the provided // When the process exits, copy the user settings back to the provided
// data-dir. // data-dir.
...@@ -164,7 +178,7 @@ class ChromeLauncher { ...@@ -164,7 +178,7 @@ class ChromeLauncher {
.transform(utf8.decoder) .transform(utf8.decoder)
.transform(const LineSplitter()) .transform(const LineSplitter())
.listen((String line) { .listen((String line) {
globals.printTrace('[CHROME]: $line'); _logger.printTrace('[CHROME]: $line');
}); });
// Wait until the DevTools are listening before trying to connect. // Wait until the DevTools are listening before trying to connect.
...@@ -172,7 +186,7 @@ class ChromeLauncher { ...@@ -172,7 +186,7 @@ class ChromeLauncher {
.transform(utf8.decoder) .transform(utf8.decoder)
.transform(const LineSplitter()) .transform(const LineSplitter())
.map((String line) { .map((String line) {
globals.printTrace('[CHROME]:$line'); _logger.printTrace('[CHROME]:$line');
return line; return line;
}) })
.firstWhere((String line) => line.startsWith('DevTools listening'), orElse: () { .firstWhere((String line) => line.startsWith('DevTools listening'), orElse: () {
......
...@@ -80,7 +80,7 @@ class ChromeDevice extends Device { ...@@ -80,7 +80,7 @@ class ChromeDevice extends Device {
Future<String> get emulatorId async => null; Future<String> get emulatorId async => null;
@override @override
bool isSupported() => featureFlags.isWebEnabled && chromeLauncher.canFindChrome(); bool isSupported() => featureFlags.isWebEnabled && globals.chromeLauncher.canFindChrome();
@override @override
String get name => 'Chrome'; String get name => 'Chrome';
...@@ -109,7 +109,7 @@ class ChromeDevice extends Device { ...@@ -109,7 +109,7 @@ class ChromeDevice extends Device {
} }
} }
} else { } else {
final String chrome = findChromeExecutable(); final String chrome = findChromeExecutable(globals.platform, globals.fs);
final ProcessResult result = await globals.processManager.run(<String>[ final ProcessResult result = await globals.processManager.run(<String>[
chrome, chrome,
'--version', '--version',
...@@ -136,7 +136,7 @@ class ChromeDevice extends Device { ...@@ -136,7 +136,7 @@ class ChromeDevice extends Device {
final String url = platformArgs['uri'] as String; final String url = platformArgs['uri'] as String;
final bool launchChrome = platformArgs['no-launch-chrome'] != true; final bool launchChrome = platformArgs['no-launch-chrome'] != true;
if (launchChrome) { if (launchChrome) {
_chrome = await chromeLauncher.launch( _chrome = await globals.chromeLauncher.launch(
url, url,
dataDir: globals.fs.currentDirectory dataDir: globals.fs.currentDirectory
.childDirectory('.dart_tool') .childDirectory('.dart_tool')
...@@ -177,7 +177,7 @@ class ChromeDevice extends Device { ...@@ -177,7 +177,7 @@ class ChromeDevice extends Device {
class WebDevices extends PollingDeviceDiscovery { class WebDevices extends PollingDeviceDiscovery {
WebDevices() : super('chrome'); WebDevices() : super('chrome');
final bool _chromeIsAvailable = chromeLauncher.canFindChrome(); final bool _chromeIsAvailable = globals.chromeLauncher.canFindChrome();
final ChromeDevice _webDevice = ChromeDevice(); final ChromeDevice _webDevice = ChromeDevice();
final WebServerDevice _webServerDevice = WebServerDevice(); final WebServerDevice _webServerDevice = WebServerDevice();
......
...@@ -2,20 +2,34 @@ ...@@ -2,20 +2,34 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:meta/meta.dart';
import 'package:platform/platform.dart';
import '../base/file_system.dart';
import '../doctor.dart'; import '../doctor.dart';
import '../globals.dart' as globals;
import 'chrome.dart'; import 'chrome.dart';
/// A validator that checks whether chrome is installed and can run. /// A validator that checks whether chrome is installed and can run.
class WebValidator extends DoctorValidator { class WebValidator extends DoctorValidator {
const WebValidator() : super('Chrome - develop for the web'); const WebValidator({
@required Platform platform,
@required ChromeLauncher chromeLauncher,
@required FileSystem fileSystem,
}) : _platform = platform,
_chromeLauncher = chromeLauncher,
_fileSystem = fileSystem,
super('Chrome - develop for the web');
final Platform _platform;
final ChromeLauncher _chromeLauncher;
final FileSystem _fileSystem;
@override @override
Future<ValidationResult> validate() async { Future<ValidationResult> validate() async {
final String chrome = findChromeExecutable(); final String chrome = findChromeExecutable(_platform, _fileSystem);
final bool canRunChrome = chromeLauncher.canFindChrome(); final bool canRunChrome = _chromeLauncher.canFindChrome();
final List<ValidationMessage> messages = <ValidationMessage>[ final List<ValidationMessage> messages = <ValidationMessage>[
if (globals.platform.environment.containsKey(kChromeEnvironment)) if (_platform.environment.containsKey(kChromeEnvironment))
if (!canRunChrome) if (!canRunChrome)
ValidationMessage.hint('$chrome is not executable.') ValidationMessage.hint('$chrome is not executable.')
else else
......
...@@ -71,7 +71,7 @@ void main() { ...@@ -71,7 +71,7 @@ void main() {
} }
test('can launch chrome and connect to the devtools', () => testbed.run(() async { test('can launch chrome and connect to the devtools', () => testbed.run(() async {
await chromeLauncher.launch('example_url', skipCheck: true); await globals.chromeLauncher.launch('example_url', skipCheck: true);
final VerificationResult result = verify(globals.processManager.start(captureAny)); final VerificationResult result = verify(globals.processManager.start(captureAny));
expect(result.captured.single, containsAll(expectChromeArgs())); expect(result.captured.single, containsAll(expectChromeArgs()));
...@@ -79,7 +79,7 @@ void main() { ...@@ -79,7 +79,7 @@ void main() {
})); }));
test('can launch chrome with a custom debug port', () => testbed.run(() async { test('can launch chrome with a custom debug port', () => testbed.run(() async {
await chromeLauncher.launch('example_url', skipCheck: true, debugPort: 10000); await globals.chromeLauncher.launch('example_url', skipCheck: true, debugPort: 10000);
final VerificationResult result = verify(globals.processManager.start(captureAny)); final VerificationResult result = verify(globals.processManager.start(captureAny));
expect(result.captured.single, containsAll(expectChromeArgs(debugPort: 10000))); expect(result.captured.single, containsAll(expectChromeArgs(debugPort: 10000)));
...@@ -87,7 +87,7 @@ void main() { ...@@ -87,7 +87,7 @@ void main() {
})); }));
test('can launch chrome headless', () => testbed.run(() async { test('can launch chrome headless', () => testbed.run(() async {
await chromeLauncher.launch('example_url', skipCheck: true, headless: true); await globals.chromeLauncher.launch('example_url', skipCheck: true, headless: true);
final VerificationResult result = verify(globals.processManager.start(captureAny)); final VerificationResult result = verify(globals.processManager.start(captureAny));
expect(result.captured.single, containsAll(expectChromeArgs())); expect(result.captured.single, containsAll(expectChromeArgs()));
...@@ -104,7 +104,7 @@ void main() { ...@@ -104,7 +104,7 @@ void main() {
..createSync(recursive: true) ..createSync(recursive: true)
..writeAsStringSync('example'); ..writeAsStringSync('example');
await chromeLauncher.launch('example_url', skipCheck: true, dataDir: dataDir); await globals.chromeLauncher.launch('example_url', skipCheck: true, dataDir: dataDir);
final VerificationResult result = verify(globals.processManager.start(captureAny)); final VerificationResult result = verify(globals.processManager.start(captureAny));
final String arg = (result.captured.single as List<String>) final String arg = (result.captured.single as List<String>)
.firstWhere((String arg) => arg.startsWith('--user-data-dir=')); .firstWhere((String arg) => arg.startsWith('--user-data-dir='));
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/doctor.dart'; import 'package:flutter_tools/src/doctor.dart';
import 'package:flutter_tools/src/web/chrome.dart'; import 'package:flutter_tools/src/web/chrome.dart';
import 'package:flutter_tools/src/web/web_validator.dart'; import 'package:flutter_tools/src/web/web_validator.dart';
...@@ -10,56 +12,63 @@ import 'package:process/process.dart'; ...@@ -10,56 +12,63 @@ import 'package:process/process.dart';
import 'package:platform/platform.dart'; import 'package:platform/platform.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/testbed.dart'; import '../../src/fake_process_manager.dart';
void main() { void main() {
group('WebValidator', () { Platform platform;
Testbed testbed; ProcessManager processManager;
WebValidator webValidator; ChromeLauncher chromeLauncher;
MockPlatform mockPlatform; FileSystem fileSystem;
MockProcessManager mockProcessManager; WebValidator webValidator;
setUp(() { setUp(() {
mockProcessManager = MockProcessManager(); fileSystem = MemoryFileSystem.test();
testbed = Testbed(setup: () { processManager = MockProcessManager();
when(mockProcessManager.canRun(kMacOSExecutable)).thenReturn(true); platform = FakePlatform(
return null; operatingSystem: 'macos',
}, overrides: <Type, Generator>{ environment: <String, String>{},
Platform: () => mockPlatform, );
ProcessManager: () => mockProcessManager, chromeLauncher = ChromeLauncher(
}); fileSystem: fileSystem,
webValidator = const WebValidator(); platform: platform,
mockPlatform = MockPlatform(); processManager: processManager,
when(mockPlatform.isMacOS).thenReturn(true); operatingSystemUtils: null,
when(mockPlatform.isWindows).thenReturn(false); logger: null,
when(mockPlatform.isLinux).thenReturn(false); );
}); webValidator = webValidator = WebValidator(
platform: platform,
chromeLauncher: chromeLauncher,
fileSystem: fileSystem,
);
});
test('Can find macOS executable ', () => testbed.run(() async { testWithoutContext('WebValidator can find executable on macOS', () async {
final ValidationResult result = await webValidator.validate(); when(processManager.canRun(kMacOSExecutable)).thenReturn(true);
expect(result.type, ValidationType.installed);
}));
test('Can notice missing macOS executable ', () => testbed.run(() async { final ValidationResult result = await webValidator.validate();
when(mockProcessManager.canRun(kMacOSExecutable)).thenReturn(false);
final ValidationResult result = await webValidator.validate();
expect(result.type, ValidationType.missing);
}));
test("Doesn't warn about CHROME_EXECUTABLE unless it cant find chrome ", () => testbed.run(() async { expect(result.type, ValidationType.installed);
when(mockProcessManager.canRun(kMacOSExecutable)).thenReturn(false);
final ValidationResult result = await webValidator.validate();
expect(result.messages, <ValidationMessage>[
ValidationMessage.hint('Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable.'),
]);
expect(result.type, ValidationType.missing);
}));
}); });
}
class MockPlatform extends Mock implements Platform { testWithoutContext('WebValidator Can notice missing macOS executable ', () async {
@override when(processManager.canRun(kMacOSExecutable)).thenReturn(false);
Map<String, String> get environment => const <String, String>{};
final ValidationResult result = await webValidator.validate();
expect(result.type, ValidationType.missing);
});
testWithoutContext('WebValidator does not warn about CHROME_EXECUTABLE unless it cant find chrome ', () async {
when(processManager.canRun(kMacOSExecutable)).thenReturn(false);
final ValidationResult result = await webValidator.validate();
expect(result.messages, <ValidationMessage>[
ValidationMessage.hint(
'Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable.'),
]);
expect(result.type, ValidationType.missing);
});
} }
class MockProcessManager extends Mock implements ProcessManager {} class MockProcessManager extends Mock implements ProcessManager {}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment