Don't try to get chrome version on windows.

tools/testing/dart/browser_controller.dart

 // Copyright (c) 2013, 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.
 library browser;
 
 import "dart:async";
 import "dart:convert" show LineSplitter, UTF8;
 import "dart:core";
 import "dart:io";
 
@@ skipping 22 matching lines @@
   Process process;
 
   Function logger;
 
   /**
    * Id of the browser
    */
   String id;
 
   /** Print everything (stdout, stderr, usageLog) whenever we add to it */
-  bool debugPrint = false;
+  bool debugPrint = true;
 
   // This future returns when the process exits. It is also the return value
   // of close()
   Future done;
 
   Browser();
 
   factory Browser.byName(String name) {
     if (name == 'ff' || name == 'firefox') {
       return new Firefox();
@@ skipping 257 matching lines @@
   // Delete the user specific browser cache and profile data.
   // Safari only have one per user, and you can't specify one by command line.
   static bool deleteCache = false;
 
 }
 
 
 class Chrome extends Browser {
   static String _binary = _getBinary();
 
+  String _version = "Version not found yet";

[kustermann, 2013/09/10 09:27:01]

I'm not totally happy about this. You add new instance variables to communicate
the version string between a caller and a callee.

Why not using other mechanisms, _getVersion() could return "null" or complete
with an error if unsuccessfull.

[ricow1, 2013/09/10 10:07:04]

Seems like a valid instance variable to me. Could easily be usefullin other
future functions well

[kustermann, 2013/09/10 10:32:43]

The issue that _version is not available right from the start, it will be set at
some point in the future, that's why I don't like it.

+
   // This is extracted to a function since we may need to support several
   // locations.
   static String _getWindowsBinary() {
     return "C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe";
   }
 
   static String _getBinary() {
     if (Platform.isWindows) return _getWindowsBinary();
     if (Platform.isMacOS) {
       return "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome";
     }
     if (Platform.isLinux) return 'google-chrome';
   }
 
+  Future<String> _getVersion() {

[kustermann, 2013/09/10 09:27:01]

Make it Future<bool>

[ricow1, 2013/09/10 10:07:04]

Done.

+    if (Platform.isWindows) {
+      // The version flag does not work on windows.
+      // See issue:
+      // https://code.google.com/p/chromium/issues/detail?id=158372
+      // The registry hack does not seem to work.
+      _version = "Can't get version on windows";
+      // We still validate that the binary exists so that we can give good
+      // feedback.
+      return new File(_binary).exists().then((exists) {
+        if (!exists) {
+          _logEvent("Chrome binary not available.");

[kustermann, 2013/09/10 09:27:01]

How about printing the path (i.e. _binary) here as well?

[ricow1, 2013/09/10 10:07:04]

I do, in the next line!

+          _logEvent("Make sure $_binary is a valid program for running chrome");
+        }
+        return exists;
+      });
+    }
+    return Process.run(_binary, ["--version"]).then((var versionResult) {
+      if (versionResult.exitCode != 0) {
+        _logEvent("Failed to chrome get version");
+        _logEvent("Make sure $_binary is a valid program for running chrome");
+        return false;
+      }
+      _version = versionResult.stdout;
+      return true;
+    });
+  }
+
+
   Future<bool> start(String url) {
     _logEvent("Starting chrome browser on: $url");
     // Get the version and log that.
-    return Process.run(_binary, ["--version"]).then((var versionResult) {
-      if (versionResult.exitCode != 0) {
-        _logEvent("Failed to chrome get version");
-        _logEvent("Make sure $binary is a valid program for running chrome");
-        return new Future.value(false);
-      }
-      version = versionResult.stdout;
-      _logEvent("Got version: $version");
+    return _getVersion().then((success) {
+      if (!success) return false;
+      _logEvent("Got version: $_version");
 
       return new Directory('').createTemp().then((userDir) {
         _cleanup = () { userDir.deleteSync(recursive: true); };
         var args = ["--user-data-dir=${userDir.path}", url,
                     "--disable-extensions", "--disable-popup-blocking",
                     "--bwsi", "--no-first-run"];
         return startBrowser(_binary, args);
 
       });
     }).catchError((e) {
@@ skipping 796 matching lines @@
     Dart test driver, number of tests: <div id="number"></div><br>
     Currently executing: <div id="currently_executing"></div><br>
     Unhandled error: <div id="unhandled_error"></div>
     <iframe id="embedded_iframe"></iframe>
   </body>
 </html>
 """;
     return driverContent;
   }
 }