Don't try to get chrome version on windows.

Don't try to get chrome version on windows.

We still make sure that the binary is valid.

Enable the browser controller on windows again

R=kustermann@google.com

Committed: https://code.google.com/p/dart/source/detail?r=27333

[kustermann, 2013-09-10 09:27:00]

lgtm but I'd prefer not having an additional instance variable.

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
File tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:321: String _version = "Version not
found yet";
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.

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:337: Future<String> _getVersion() {
Make it Future<bool>

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:348: _logEvent("Chrome binary not
available.");
How about printing the path (i.e. _binary) here as well?

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

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
File tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:321: String _version = "Version not
found yet";
On 2013/09/10 09:27:01, kustermann wrote:
> 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.

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

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:337: Future<String> _getVersion() {
On 2013/09/10 09:27:01, kustermann wrote:
> Make it Future<bool>

Done.

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:348: _logEvent("Chrome binary not
available.");
On 2013/09/10 09:27:01, kustermann wrote:
> How about printing the path (i.e. _binary) here as well?

I do, in the next line!

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

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
File tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/23815007/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:321: String _version = "Version not
found yet";
On 2013/09/10 10:07:04, ricow1 wrote:
> On 2013/09/10 09:27:01, kustermann wrote:
> > 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.
> 
> Seems like a valid instance variable to me. Could easily be usefullin other
> future functions well
> 

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.

[ricow1, 2013-09-10 12:29:47]

Committed patchset #2 manually as r27333 (presubmit successful).