Bring webdriver up to date with SDK changes, and a bit of refactoring.

Bring webdriver up to date with SDK changes, and a bit of refactoring.
Committed: https://code.google.com/p/dart/source/detail?r=20038

[Siggi Cherem (dart-lang), 2013-03-09 00:04:59]

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/lib/webdriver.dart
File pkg/webdriver/lib/webdriver.dart (right):

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/lib/webdriver.d...
pkg/webdriver/lib/webdriver.dart:275: if (successCodes.indexOf(rsp.statusCode) <
0) {
weird indentation here?

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/lib/webdriver.d...
pkg/webdriver/lib/webdriver.dart:297: } else {
empty else?

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/test/webdriver_...
File pkg/webdriver/test/webdriver_test.dart (right):

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/test/webdriver_...
pkg/webdriver/test/webdriver_test.dart:53: f.catchError(exceptionHandler);
fix here and below - this is branching the future, move the .catchError where
you want it in the chain (it could be before or after the then, but one should
be nested with the other)

(basically all of these should be like your fix in 'Get status')

[gram, 2013-03-09 00:12:36]

PTAL

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/lib/webdriver.dart
File pkg/webdriver/lib/webdriver.dart (right):

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/lib/webdriver.d...
pkg/webdriver/lib/webdriver.dart:275: if (successCodes.indexOf(rsp.statusCode) <
0) {
On 2013/03/09 00:04:59, Siggi Cherem (dart-lang) wrote:
> weird indentation here?

Done.

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/lib/webdriver.d...
pkg/webdriver/lib/webdriver.dart:297: } else {
On 2013/03/09 00:04:59, Siggi Cherem (dart-lang) wrote:
> empty else?

Done.

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/test/webdriver_...
File pkg/webdriver/test/webdriver_test.dart (right):

https://codereview.chromium.org/12413012/diff/1/pkg/webdriver/test/webdriver_...
pkg/webdriver/test/webdriver_test.dart:53: f.catchError(exceptionHandler);
On 2013/03/09 00:04:59, Siggi Cherem (dart-lang) wrote:
> fix here and below - this is branching the future, move the .catchError where
> you want it in the chain (it could be before or after the then, but one should
> be nested with the other)
> 
> (basically all of these should be like your fix in 'Get status')

Done.

[Siggi Cherem (dart-lang), 2013-03-09 00:14:22]

https://codereview.chromium.org/12413012/diff/5002/pkg/webdriver/test/webdriv...
File pkg/webdriver/test/webdriver_test.dart (right):

https://codereview.chromium.org/12413012/diff/5002/pkg/webdriver/test/webdriv...
pkg/webdriver/test/webdriver_test.dart:26: return true;
remove return

https://codereview.chromium.org/12413012/diff/5002/pkg/webdriver/test/webdriv...
pkg/webdriver/test/webdriver_test.dart:63: }).catchError(exceptionHandler);
mmm... just thinking more about it, would this trap exceptions thrown by the
test too? Maybe it does need to come first?

[gram, 2013-03-12 23:42:13]

PTAL. I have all tests passing now. Worked around storage problems using JS
execution. I added exit(0) to vm_config; without this after all the tests pass
the VM does not exit and so eventually timeout kicked in resulting in a
"failure".

https://codereview.chromium.org/12413012/diff/5002/pkg/webdriver/test/webdriv...
File pkg/webdriver/test/webdriver_test.dart (right):

https://codereview.chromium.org/12413012/diff/5002/pkg/webdriver/test/webdriv...
pkg/webdriver/test/webdriver_test.dart:26: return true;
On 2013/03/09 00:14:22, Siggi Cherem (dart-lang) wrote:
> remove return

Done.

[Siggi Cherem (dart-lang), 2013-03-14 16:41:24]

lgtm

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
File pkg/webdriver/lib/webdriver.dart (right):

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
pkg/webdriver/lib/webdriver.dart:247: 'The http method called for ${command} is
${http_method} but it has '
80 col

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
pkg/webdriver/lib/webdriver.dart:270: // strip them.
would results.trim() work here?

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
pkg/webdriver/lib/webdriver.dart:274: for (var i = results.length; --i >= 0;) {
style nit
for (var i = results.length - 1; i >= 0; i--)

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
pkg/webdriver/lib/webdriver.dart:281: if (successCodes.indexOf(rsp.statusCode) <
0) {
now that we have it, let's switch to use List.contains instead of indexOf)

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
pkg/webdriver/lib/webdriver.dart:1292: [ key, value ]);
style nit: remove extra spaces before/after []
 [key, value]

https://codereview.chromium.org/12413012/diff/13001/pkg/webdriver/lib/webdriv...
pkg/webdriver/lib/webdriver.dart:1319: [ key ]);
nit: spaces (here and below)

[gram, 2013-03-14 17:45:16]

Committed patchset #5 manually as r20038 (presubmit successful).