Make browser platforms customizable

See #391

Author: nex3

lib/src/backend/platform_selector.dart

@@ -76,6 +76,7 @@ class PlatformSelector {
 
     return _inner.evaluate((variable) {
       if (variable == platform.identifier) return true;
+      if (variable == platform.parent?.identifier) return true;
       if (variable == os.identifier) return true;
       switch (variable) {
         case "dart-vm":

lib/src/runner/browser/browser.dart

@@ -105,6 +105,7 @@ abstract class Browser {
       _process.then((process) => process.kill());
 
       if (stackTrace == null) stackTrace = new Trace.current();
+      if (_onExitCompleter.isCompleted) return;
       _onExitCompleter.completeError(
           new ApplicationException(
               "Failed to run $name: ${getErrorMessage(error)}."),

lib/src/runner/browser/browser_manager.dart

@@ -15,6 +15,7 @@ import '../../util/stack_trace_mapper.dart';
 import '../application_exception.dart';
 import '../configuration/suite.dart';
 import '../environment.dart';
+import '../executable_settings.dart';
 import '../plugin/platform_helpers.dart';
 import '../runner_suite.dart';
 import 'browser.dart';
@@ -93,12 +94,14 @@ class BrowserManager {
   /// [future]. If [debug] is true, starts the browser in debug mode, with its
   /// debugger interfaces on and detected.
   ///
+  /// The [settings] indicate how to invoke this browser's executable.
+  ///
   /// Returns the browser manager, or throws an [ApplicationException] if a
   /// connection fails to be established.
-  static Future<BrowserManager> start(
-      TestPlatform platform, Uri url, Future<WebSocketChannel> future,
+  static Future<BrowserManager> start(TestPlatform platform, Uri url,
+      Future<WebSocketChannel> future, ExecutableSettings settings,
       {bool debug: false}) {
-    var browser = _newBrowser(url, platform, debug: debug);
+    var browser = _newBrowser(url, platform, settings, debug: debug);
 
     var completer = new Completer<BrowserManager>();
 
@@ -128,26 +131,27 @@ class BrowserManager {
     });
   }
 
-  /// Starts the browser identified by [browser] and has it load [url].
+  /// Starts the browser identified by [browser] using [settings] and has it load [url].
   ///
   /// If [debug] is true, starts the browser in debug mode.
-  static Browser _newBrowser(Uri url, TestPlatform browser,
+  static Browser _newBrowser(
+      Uri url, TestPlatform browser, ExecutableSettings settings,
       {bool debug: false}) {
-    switch (browser) {
+    switch (browser.root) {
       case TestPlatform.dartium:
-        return new Dartium(url, debug: debug);
+        return new Dartium(url, settings: settings, debug: debug);
       case TestPlatform.contentShell:
-        return new ContentShell(url, debug: debug);
+        return new ContentShell(url, settings: settings, debug: debug);
       case TestPlatform.chrome:
-        return new Chrome(url, debug: debug);
+        return new Chrome(url, settings: settings, debug: debug);
       case TestPlatform.phantomJS:
-        return new PhantomJS(url, debug: debug);
+        return new PhantomJS(url, settings: settings, debug: debug);
       case TestPlatform.firefox:
-        return new Firefox(url);
+        return new Firefox(url, settings: settings);
       case TestPlatform.safari:
-        return new Safari(url);
+        return new Safari(url, settings: settings);
       case TestPlatform.internetExplorer:
-        return new InternetExplorer(url);
+        return new InternetExplorer(url, settings: settings);
       default:
         throw new ArgumentError("$browser is not a browser.");
     }

lib/src/runner/browser/chrome.dart

@@ -5,10 +5,11 @@
 import 'dart:async';
 import 'dart:io';
 
-import 'package:path/path.dart' as p;
-
+import '../../backend/test_platform.dart';
 import '../../util/io.dart';
+import '../executable_settings.dart';
 import 'browser.dart';
+import 'default_settings.dart';
 
 // TODO(nweiz): move this into its own package?
 /// A class for running an instance of Chrome.
@@ -25,14 +26,10 @@ class Chrome extends Browser {
 
   /// Starts a new instance of Chrome open to the given [url], which may be a
   /// [Uri] or a [String].
-  ///
-  /// If [executable] is passed, it's used as the Chrome executable. Otherwise
-  /// the default executable name for the current OS will be used.
-  factory Chrome(url, {String executable, bool debug: false}) {
+  factory Chrome(url, {ExecutableSettings settings, bool debug: false}) {
+    settings ??= defaultSettings[TestPlatform.chrome];
     var remoteDebuggerCompleter = new Completer<Uri>.sync();
     return new Chrome._(() async {
-      if (executable == null) executable = _defaultExecutable();
-
       var tryPort = ([int port]) async {
         var dir = createTempDir();
         var args = [
@@ -45,7 +42,7 @@ class Chrome extends Browser {
           "--no-default-browser-check",
           "--disable-default-apps",
           "--disable-translate",
-        ];
+        ]..addAll(settings.arguments);
 
         // Currently, Chrome doesn't provide any way of ensuring that this port
         // was successfully bound. It produces an error if the binding fails,
@@ -54,7 +51,7 @@ class Chrome extends Browser {
         // though.
         if (port != null) args.add("--remote-debugging-port=$port");
 
-        var process = await Process.start(executable, args);
+        var process = await Process.start(settings.executable, args);
 
         if (port != null) {
           remoteDebuggerCompleter.complete(
@@ -76,32 +73,4 @@ class Chrome extends Browser {
 
   Chrome._(Future<Process> startBrowser(), this.remoteDebuggerUrl)
       : super(startBrowser);
-
-  /// Return the default executable for the current operating system.
-  static String _defaultExecutable() {
-    if (Platform.isMacOS) {
-      return '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome';
-    }
-    if (!Platform.isWindows) return 'google-chrome';
-
-    // Chrome could be installed in several places on Windows. The only way to
-    // find it is to check.
-    var prefixes = [
-      Platform.environment['LOCALAPPDATA'],
-      Platform.environment['PROGRAMFILES'],
-      Platform.environment['PROGRAMFILES(X86)']
-    ];
-    var suffix = r'Google\Chrome\Application\chrome.exe';
-
-    for (var prefix in prefixes) {
-      if (prefix == null) continue;
-
-      var path = p.join(prefix, suffix);
-      if (new File(p.join(prefix, suffix)).existsSync()) return path;
-    }
-
-    // Fall back on looking it up on the path. This probably won't work, but at
-    // least it will fail with a useful error message.
-    return "chrome.exe";
-  }
 }

lib/src/runner/browser/content_shell.dart

@@ -5,10 +5,13 @@
 import 'dart:async';
 import 'dart:io';
 
+import '../../backend/test_platform.dart';
 import '../../util/io.dart';
 import '../../utils.dart';
 import '../application_exception.dart';
+import '../executable_settings.dart';
 import 'browser.dart';
+import 'default_settings.dart';
 
 final _observatoryRegExp = new RegExp(r"^Observatory listening on ([^ ]+)");
 final _errorTimeout = const Duration(seconds: 10);
@@ -27,18 +30,18 @@ class ContentShell extends Browser {
 
   final Future<Uri> remoteDebuggerUrl;
 
-  factory ContentShell(url, {String executable, bool debug: false}) {
+  factory ContentShell(url, {ExecutableSettings settings, bool debug: false}) {
+    settings ??= defaultSettings[TestPlatform.contentShell];
     var observatoryCompleter = new Completer<Uri>.sync();
     var remoteDebuggerCompleter = new Completer<Uri>.sync();
     return new ContentShell._(() {
-      if (executable == null) executable = _defaultExecutable();
-
       var tryPort = ([int port]) async {
-        var args = ["--dump-render-tree", url.toString()];
+        var args = ["--dump-render-tree", url.toString()]
+          ..addAll(settings.arguments);
         if (port != null) args.add("--remote-debugging-port=$port");
 
-        var process = await Process
-            .start(executable, args, environment: {"DART_FLAGS": "--checked"});
+        var process = await Process.start(settings.executable, args,
+            environment: {"DART_FLAGS": "--checked"});
 
         if (debug) {
           observatoryCompleter.complete(lineSplitter
@@ -111,8 +114,4 @@ class ContentShell extends Browser {
   ContentShell._(Future<Process> startBrowser(), this.observatoryUrl,
       this.remoteDebuggerUrl)
       : super(startBrowser);
-
-  /// Return the default executable for the current operating system.
-  static String _defaultExecutable() =>
-      Platform.isWindows ? "content_shell.exe" : "content_shell";
 }

lib/src/runner/browser/dartium.dart

@@ -7,11 +7,13 @@ import 'dart:convert';
 import 'dart:io';
 
 import 'package:async/async.dart';
-import 'package:path/path.dart' as p;
 
+import '../../backend/test_platform.dart';
 import '../../util/io.dart';
 import '../../utils.dart';
+import '../executable_settings.dart';
 import 'browser.dart';
+import 'default_settings.dart';
 
 final _observatoryRegExp =
     new RegExp(r'Observatory listening (?:on|at) ([^ "]+)');
@@ -30,12 +32,11 @@ class Dartium extends Browser {
 
   final Future<Uri> remoteDebuggerUrl;
 
-  factory Dartium(url, {String executable, bool debug: false}) {
+  factory Dartium(url, {ExecutableSettings settings, bool debug: false}) {
+    settings ??= defaultSettings[TestPlatform.dartium];
     var observatoryCompleter = new Completer<Uri>.sync();
     var remoteDebuggerCompleter = new Completer<Uri>.sync();
     return new Dartium._(() async {
-      if (executable == null) executable = _defaultExecutable();
-
       var tryPort = ([int port]) async {
         var dir = createTempDir();
         var args = [
@@ -48,7 +49,7 @@ class Dartium extends Browser {
           "--no-default-browser-check",
           "--disable-default-apps",
           "--disable-translate"
-        ];
+        ]..addAll(settings.arguments);
 
         if (port != null) {
           args.add("--remote-debugging-port=$port");
@@ -62,8 +63,8 @@ class Dartium extends Browser {
           args.add("--vmodule=startup_browser_creator_impl=1");
         }
 
-        var process = await Process
-            .start(executable, args, environment: {"DART_FLAGS": "--checked"});
+        var process = await Process.start(settings.executable, args,
+            environment: {"DART_FLAGS": "--checked"});
 
         if (port != null) {
           // Dartium on Windows prints all standard IO to stderr, so we need to
@@ -124,47 +125,6 @@ class Dartium extends Browser {
       this.remoteDebuggerUrl)
       : super(startBrowser);
 
-  /// Starts a new instance of Dartium open to the given [url], which may be a
-  /// [Uri] or a [String].
-  ///
-  /// If [executable] is passed, it's used as the Dartium executable. Otherwise
-  /// the default executable name for the current OS will be used.
-
-  /// Return the default executable for the current operating system.
-  static String _defaultExecutable() {
-    var dartium = _executableInEditor();
-    if (dartium != null) return dartium;
-    return Platform.isWindows ? "dartium.exe" : "dartium";
-  }
-
-  static String _executableInEditor() {
-    var dir = p.dirname(sdkDir);
-
-    if (Platform.isWindows) {
-      if (!new File(p.join(dir, "DartEditor.exe")).existsSync()) return null;
-
-      var dartium = p.join(dir, "chromium\\chrome.exe");
-      return new File(dartium).existsSync() ? dartium : null;
-    }
-
-    if (Platform.isMacOS) {
-      if (!new File(p.join(dir, "DartEditor.app/Contents/MacOS/DartEditor"))
-          .existsSync()) {
-        return null;
-      }
-
-      var dartium =
-          p.join(dir, "chromium/Chromium.app/Contents/MacOS/Chromium");
-      return new File(dartium).existsSync() ? dartium : null;
-    }
-
-    assert(Platform.isLinux);
-    if (!new File(p.join(dir, "DartEditor")).existsSync()) return null;
-
-    var dartium = p.join(dir, "chromium", "chrome");
-    return new File(dartium).existsSync() ? dartium : null;
-  }
-
   // TODO(nweiz): simplify this when sdk#23923 is fixed.
   /// Returns the Observatory URL for the Dartium executable with the given
   /// [stdout] stream, or `null` if the correct one couldn't be found.

lib/src/runner/browser/default_settings.dart

@@ -0,0 +1,37 @@
+// Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
+// for details. 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:collection';
+
+import '../../backend/test_platform.dart';
+import '../executable_settings.dart';
+
+/// Default settings for starting browser executables.
+final defaultSettings = new UnmodifiableMapView({
+  TestPlatform.chrome: new ExecutableSettings(
+      linuxExecutable: 'google-chrome',
+      macOSExecutable:
+          '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome',
+      windowsExecutable: r'Google\Chrome\Application\chrome.exe'),
+  TestPlatform.contentShell: new ExecutableSettings(
+      linuxExecutable: 'content_shell',
+      macOSExecutable: 'content_shell',
+      windowsExecutable: 'content_shell.exe'),
+  TestPlatform.dartium: new ExecutableSettings(
+      linuxExecutable: 'dartium',
+      macOSExecutable: 'dartium',
+      windowsExecutable: 'dartium.exe'),
+  TestPlatform.firefox: new ExecutableSettings(
+      linuxExecutable: 'firefox',
+      macOSExecutable: '/Applications/Firefox.app/Contents/MacOS/firefox-bin',
+      windowsExecutable: r'Mozilla Firefox\firefox.exe'),
+  TestPlatform.internetExplorer: new ExecutableSettings(
+      windowsExecutable: r'Internet Explorer\iexplore.exe'),
+  TestPlatform.phantomJS: new ExecutableSettings(
+      linuxExecutable: 'phantomjs',
+      macOSExecutable: 'phantomjs',
+      windowsExecutable: 'phantomjs.exe'),
+  TestPlatform.safari: new ExecutableSettings(
+      macOSExecutable: '/Applications/Safari.app/Contents/MacOS/Safari')
+});