refactoring chrome_installer

lib/web_ui/dev/README.md

@@ -41,8 +41,8 @@ felt test --debug test/golden_tests/engine/canvas_golden_test.dart
 
 ## Configuration files
 
-`chrome_lock.yaml` contains the version of Chrome we use to test Flutter for
-web. Chrome is not automatically updated whenever a new release is available.
+`browser_lock.yaml` contains the version of browsers we use to test Flutter for
+web. Versions are not automatically updated whenever a new release is available.
 Instead, we update this file manually once in a while.
 
 `goldens_lock.yaml` refers to a revision in the https://github.com/flutter/goldens

lib/web_ui/dev/browser_lock.yaml

@@ -0,0 +1,7 @@
+chrome:
+  # It seems Chrome can't always release from the same build for all operating
+  # systems, so we specify per-OS build number.
+  Linux: 695653
+  Mac: 695656
+firefox:
+  version: 69.0.3

lib/web_ui/dev/chrome_installer.dart

@@ -10,12 +10,14 @@ import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 import 'package:yaml/yaml.dart';
 
+import 'common.dart';
 import 'environment.dart';
 
 void addChromeVersionOption(ArgParser argParser) {
-  final io.File lockFile = io.File(path.join(environment.webUiRootDir.path, 'dev', 'chrome_lock.yaml'));
+  final io.File lockFile = io.File(
+      path.join(environment.webUiRootDir.path, 'dev', 'browser_lock.yaml'));
   final YamlMap lock = loadYaml(lockFile.readAsStringSync());
-  final int pinnedChromeVersion = _PlatformBinding.instance.getChromeBuild(lock);
+  final int pinnedChromeVersion = PlatformBinding.instance.getChromeBuild(lock);
 
   argParser
     ..addOption(
@@ -34,7 +36,7 @@ void addChromeVersionOption(ArgParser argParser) {
 ///
 /// If [requestedVersion] is null, uses the version specified on the
 /// command-line. If not specified on the command-line, uses the version
-/// specified in the "chrome_lock.yaml" file.
+/// specified in the "browser_lock.yaml" file.
 ///
 /// If [requestedVersion] is not null, installs that version. The value
 /// may be "latest" (the latest available build of Chrome), "system"
@@ -58,16 +60,18 @@ Future<ChromeInstallation> getOrInstallChrome(
   ChromeInstaller installer;
   try {
     installer = requestedVersion == 'latest'
-      ? await ChromeInstaller.latest()
-      : ChromeInstaller(version: requestedVersion);
+        ? await ChromeInstaller.latest()
+        : ChromeInstaller(version: requestedVersion);
 
     if (installer.isInstalled) {
-      infoLog.writeln('Installation was skipped because Chrome version ${installer.version} is already installed.');
+      infoLog.writeln(
+          'Installation was skipped because Chrome version ${installer.version} is already installed.');
     } else {
       infoLog.writeln('Installing Chrome version: ${installer.version}');
       await installer.install();
       final ChromeInstallation installation = installer.getInstallation();
-      infoLog.writeln('Installations complete. To launch it run ${installation.executable}');
+      infoLog.writeln(
+          'Installations complete. To launch it run ${installation.executable}');
     }
     return installer.getInstallation();
   } finally {
@@ -76,12 +80,12 @@ Future<ChromeInstallation> getOrInstallChrome(
 }
 
 Future<String> _findSystemChromeExecutable() async {
-  final io.ProcessResult which = await io.Process.run('which', <String>['google-chrome']);
+  final io.ProcessResult which =
+      await io.Process.run('which', <String>['google-chrome']);
 
   if (which.exitCode != 0) {
-    throw ChromeInstallerException(
-      'Failed to locate system Chrome installation.'
-    );
+    throw BrowserInstallerException(
+        'Failed to locate system Chrome installation.');
   }
 
   return which.stdout;
@@ -106,14 +110,12 @@ class ChromeInstaller {
     @required String version,
   }) {
     if (version == 'system') {
-      throw ChromeInstallerException(
-        'Cannot install system version of Chrome. System Chrome must be installed manually.'
-      );
+      throw BrowserInstallerException(
+          'Cannot install system version of Chrome. System Chrome must be installed manually.');
     }
     if (version == 'latest') {
-      throw ChromeInstallerException(
-        'Expected a concrete Chromer version, but got $version. Maybe use ChromeInstaller.latest()?'
-      );
+      throw BrowserInstallerException(
+          'Expected a concrete Chromer version, but got $version. Maybe use ChromeInstaller.latest()?');
     }
     final io.Directory chromeInstallationDir = io.Directory(
       path.join(environment.webUiDartToolDir.path, 'chrome'),
@@ -162,7 +164,7 @@ class ChromeInstaller {
 
     return ChromeInstallation(
       version: version,
-      executable: _PlatformBinding.instance.getExecutablePath(versionDir),
+      executable: PlatformBinding.instance.getChromeExecutablePath(versionDir),
     );
   }
 
@@ -172,13 +174,14 @@ class ChromeInstaller {
     }
 
     versionDir.createSync(recursive: true);
-    final String url = _PlatformBinding.instance.getDownloadUrl(version);
+    final String url = PlatformBinding.instance.getChromeDownloadUrl(version);
     final StreamedResponse download = await client.send(Request(
       'GET',
       Uri.parse(url),
     ));
 
-    final io.File downloadedFile = io.File(path.join(versionDir.path, 'chrome.zip'));
+    final io.File downloadedFile =
+        io.File(path.join(versionDir.path, 'chrome.zip'));
     await download.stream.pipe(downloadedFile.openWrite());
 
     final io.ProcessResult unzipResult = await io.Process.run('unzip', <String>[
@@ -188,10 +191,9 @@ class ChromeInstaller {
     ]);
 
     if (unzipResult.exitCode != 0) {
-      throw ChromeInstallerException(
-        'Failed to unzip the downloaded Chrome archive ${downloadedFile.path}.\n'
-        'The unzip process exited with code ${unzipResult.exitCode}.'
-      );
+      throw BrowserInstallerException(
+          'Failed to unzip the downloaded Chrome archive ${downloadedFile.path}.\n'
+          'The unzip process exited with code ${unzipResult.exitCode}.');
     }
 
     downloadedFile.deleteSync();
@@ -202,71 +204,18 @@ class ChromeInstaller {
   }
 }
 
-class ChromeInstallerException implements Exception {
-  ChromeInstallerException(this.message);
-
-  final String message;
-
-  @override
-  String toString() => message;
-}
-
 /// Fetches the latest available Chrome build version.
 Future<String> fetchLatestChromeVersion() async {
   final Client client = Client();
   try {
-    final Response response = await client.get('https://www.googleapis.com/download/storage/v1/b/chromium-browser-snapshots/o/Linux_x64%2FLAST_CHANGE?alt=media');
+    final Response response = await client.get(
+        'https://www.googleapis.com/download/storage/v1/b/chromium-browser-snapshots/o/Linux_x64%2FLAST_CHANGE?alt=media');
     if (response.statusCode != 200) {
-      throw ChromeInstallerException(
-        'Failed to fetch latest Chrome version. Server returned status code ${response.statusCode}'
-      );
+      throw BrowserInstallerException(
+          'Failed to fetch latest Chrome version. Server returned status code ${response.statusCode}');
     }
     return response.body;
   } finally {
     client.close();
   }
 }
-
-abstract class _PlatformBinding {
-  static _PlatformBinding get instance {
-    if (_instance == null) {
-      if (io.Platform.isLinux) {
-        _instance = _LinuxBinding();
-      } else if (io.Platform.isMacOS) {
-        _instance = _MacBinding();
-      } else {
-        throw '${io.Platform.operatingSystem} is not supported';
-      }
-    }
-    return _instance;
-  }
-  static _PlatformBinding _instance;
-
-  int getChromeBuild(YamlMap chromeLock);
-  String getDownloadUrl(String version);
-  String getExecutablePath(io.Directory versionDir);
-}
-
-const String _kBaseDownloadUrl = 'https://www.googleapis.com/download/storage/v1/b/chromium-browser-snapshots/o';
-
-class _LinuxBinding implements _PlatformBinding {
-  @override
-  int getChromeBuild(YamlMap chromeLock) => chromeLock['Linux'];
-
-  @override
-  String getDownloadUrl(String version) => '$_kBaseDownloadUrl/Linux_x64%2F$version%2Fchrome-linux.zip?alt=media';
-
-  @override
-  String getExecutablePath(io.Directory versionDir) => path.join(versionDir.path, 'chrome-linux', 'chrome');
-}
-
-class _MacBinding implements _PlatformBinding {
-  @override
-  int getChromeBuild(YamlMap chromeLock) => chromeLock['Mac'];
-
-  @override
-  String getDownloadUrl(String version) => '$_kBaseDownloadUrl/Mac%2F$version%2Fchrome-mac.zip?alt=media';
-
-  @override
-  String getExecutablePath(io.Directory versionDir) => path.join(versionDir.path, 'chrome-mac', 'Chromium.app', 'Contents', 'MacOS', 'Chromium');
-}

lib/web_ui/dev/common.dart

@@ -0,0 +1,75 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:io' as io;
+
+import 'package:path/path.dart' as path;
+import 'package:yaml/yaml.dart';
+
+class BrowserInstallerException implements Exception {
+  BrowserInstallerException(this.message);
+
+  final String message;
+
+  @override
+  String toString() => message;
+}
+
+abstract class PlatformBinding {
+  static PlatformBinding get instance {
+    if (_instance == null) {
+      if (io.Platform.isLinux) {
+        _instance = _LinuxBinding();
+      } else if (io.Platform.isMacOS) {
+        _instance = _MacBinding();
+      } else {
+        throw '${io.Platform.operatingSystem} is not supported';
+      }
+    }
+    return _instance;
+  }
+
+  static PlatformBinding _instance;
+
+  int getChromeBuild(YamlMap chromeLock);
+  String getChromeDownloadUrl(String version);
+  String getChromeExecutablePath(io.Directory versionDir);
+}
+
+const String _kBaseDownloadUrl =
+    'https://www.googleapis.com/download/storage/v1/b/chromium-browser-snapshots/o';
+
+class _LinuxBinding implements PlatformBinding {
+  @override
+  int getChromeBuild(YamlMap browserLock) {
+    final YamlMap chromeMap = browserLock['chrome'];
+    return chromeMap['Linux'];
+  }
+
+  @override
+  String getChromeDownloadUrl(String version) =>
+      '$_kBaseDownloadUrl/Linux_x64%2F$version%2Fchrome-linux.zip?alt=media';
+
+  @override
+  String getChromeExecutablePath(io.Directory versionDir) =>
+      path.join(versionDir.path, 'chrome-linux', 'chrome');
+}
+
+class _MacBinding implements PlatformBinding {
+  @override
+  int getChromeBuild(YamlMap browserLock) => browserLock['Mac'];
+
+  @override
+  String getChromeDownloadUrl(String version) =>
+      '$_kBaseDownloadUrl/Mac%2F$version%2Fchrome-mac.zip?alt=media';
+
+  @override
+  String getChromeExecutablePath(io.Directory versionDir) => path.join(
+      versionDir.path,
+      'chrome-mac',
+      'Chromium.app',
+      'Contents',
+      'MacOS',
+      'Chromium');
+}