From 691b81f22042e1ab5a0ac5628857183910d2ef11 Mon Sep 17 00:00:00 2001 From: Nurhan Turgut Date: Mon, 14 Oct 2019 11:23:06 -0700 Subject: [PATCH 1/3] refactoring before implementing the firefox installer. This PR carries utilities to a common place. Renames the lock file with a generic name. --- lib/web_ui/dev/README.md | 2 +- lib/web_ui/dev/browser_lock.yaml | 7 ++ lib/web_ui/dev/chrome_installer.dart | 111 ++++++++------------------- lib/web_ui/dev/chrome_lock.yaml | 4 - lib/web_ui/dev/common.dart | 101 ++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 86 deletions(-) create mode 100644 lib/web_ui/dev/browser_lock.yaml delete mode 100644 lib/web_ui/dev/chrome_lock.yaml create mode 100644 lib/web_ui/dev/common.dart diff --git a/lib/web_ui/dev/README.md b/lib/web_ui/dev/README.md index 1c856a1e61b3a..5fcbe682c181d 100644 --- a/lib/web_ui/dev/README.md +++ b/lib/web_ui/dev/README.md @@ -41,7 +41,7 @@ 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 +`browser_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. Instead, we update this file manually once in a while. diff --git a/lib/web_ui/dev/browser_lock.yaml b/lib/web_ui/dev/browser_lock.yaml new file mode 100644 index 0000000000000..c2da9e63e9fa3 --- /dev/null +++ b/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 diff --git a/lib/web_ui/dev/chrome_installer.dart b/lib/web_ui/dev/chrome_installer.dart index 8b54e86b1f037..3d092a04bf8ee 100644 --- a/lib/web_ui/dev/chrome_installer.dart +++ b/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 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 getOrInstallChrome( } Future _findSystemChromeExecutable() async { - final io.ProcessResult which = await io.Process.run('which', ['google-chrome']); + final io.ProcessResult which = + await io.Process.run('which', ['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', [ @@ -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 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'); -} diff --git a/lib/web_ui/dev/chrome_lock.yaml b/lib/web_ui/dev/chrome_lock.yaml deleted file mode 100644 index 3023a96d1411a..0000000000000 --- a/lib/web_ui/dev/chrome_lock.yaml +++ /dev/null @@ -1,4 +0,0 @@ -# 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 diff --git a/lib/web_ui/dev/common.dart b/lib/web_ui/dev/common.dart new file mode 100644 index 0000000000000..9193627034908 --- /dev/null +++ b/lib/web_ui/dev/common.dart @@ -0,0 +1,101 @@ +// 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 getFirefoxDownloadUrl(String version); + String getChromeExecutablePath(io.Directory versionDir); + String getFirefoxExecutablePath(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'); + + @override + String getFirefoxDownloadUrl(String version) { + // TODO: implement getFirefoxDownloadUrl + return null; + } + + @override + String getFirefoxExecutablePath(io.Directory versionDir) { + // TODO: implement getFirefoxExecutablePath + return null; + } +} + +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'); + + @override + String getFirefoxDownloadUrl(String version) { + // TODO: implement getFirefoxDownloadUrl + return null; + } + + @override + String getFirefoxExecutablePath(io.Directory versionDir) { + // TODO: implement getFirefoxExecutablePath + return null; + } +} From 8c888ef236a0196b7a99393161805bfa649378b4 Mon Sep 17 00:00:00 2001 From: Nurhan Turgut Date: Mon, 14 Oct 2019 13:26:34 -0700 Subject: [PATCH 2/3] Fixed README file for browser_lock --- lib/web_ui/dev/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/dev/README.md b/lib/web_ui/dev/README.md index 5fcbe682c181d..87f89973e1676 100644 --- a/lib/web_ui/dev/README.md +++ b/lib/web_ui/dev/README.md @@ -41,8 +41,8 @@ felt test --debug test/golden_tests/engine/canvas_golden_test.dart ## Configuration files -`browser_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 From d0fe16421c2eb5668269e1928afedeadfb279496 Mon Sep 17 00:00:00 2001 From: Nurhan Turgut Date: Mon, 14 Oct 2019 13:43:06 -0700 Subject: [PATCH 3/3] addressing PR comments: removing unimplemented firefox methods. --- lib/web_ui/dev/common.dart | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/lib/web_ui/dev/common.dart b/lib/web_ui/dev/common.dart index 9193627034908..79f74a2342c2d 100644 --- a/lib/web_ui/dev/common.dart +++ b/lib/web_ui/dev/common.dart @@ -34,9 +34,7 @@ abstract class PlatformBinding { int getChromeBuild(YamlMap chromeLock); String getChromeDownloadUrl(String version); - String getFirefoxDownloadUrl(String version); String getChromeExecutablePath(io.Directory versionDir); - String getFirefoxExecutablePath(io.Directory versionDir); } const String _kBaseDownloadUrl = @@ -56,18 +54,6 @@ class _LinuxBinding implements PlatformBinding { @override String getChromeExecutablePath(io.Directory versionDir) => path.join(versionDir.path, 'chrome-linux', 'chrome'); - - @override - String getFirefoxDownloadUrl(String version) { - // TODO: implement getFirefoxDownloadUrl - return null; - } - - @override - String getFirefoxExecutablePath(io.Directory versionDir) { - // TODO: implement getFirefoxExecutablePath - return null; - } } class _MacBinding implements PlatformBinding { @@ -86,16 +72,4 @@ class _MacBinding implements PlatformBinding { 'Contents', 'MacOS', 'Chromium'); - - @override - String getFirefoxDownloadUrl(String version) { - // TODO: implement getFirefoxDownloadUrl - return null; - } - - @override - String getFirefoxExecutablePath(io.Directory versionDir) { - // TODO: implement getFirefoxExecutablePath - return null; - } }