Implement a periodic timer which sends the current state of a browser test back to the browser cont…

Implement a periodic timer which sends the current state of a browser test back to the browser controller

R=ricow@google.com

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

[ricow1, 2013-11-20 12:20:40]

LGMT

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
File dart/tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:633: // TODO(ricow): Add
prefetching to the browsers. We spend a lot of time waiting
while we are here, could you delete this

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:638: String lastKnownMessage =
'';
I actually think this should be on the BrowserTest class instead

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:1319: function
reportMessage(msg, isFirstMessage, isStatusUpdate, isDone) {
yoiu don't use the isDone parameter, so why pass it in?

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:1332: test_completed = true;
you should probably do something if isDone is not true here

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:1374: reportMessage(msg, msg ==
'STARTING', false);
if you want to use it, pass in isDOne here as well

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
File dart/tools/testing/dart/test_controller.js (right):

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:6: * The new communication protocol
between test_controller.js and the driving
remove new (that will get old pretty fast :-) )

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:133: value: '' +
window.document.documentElement.innerHTML,
is this to guarantee a string?

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:170: }
add blank line

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:179: }
add blank line

[ricow1, 2013-11-20 12:20:56]

actually, lgtm++

[kustermann, 2013-11-20 13:32:24]

Committed patchset #3 manually as r30468 (presubmit successful).

[kustermann, 2013-11-20 13:32:39]

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
File dart/tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:638: String lastKnownMessage =
'';
On 2013/11/20 12:20:41, ricow1 wrote:
> I actually think this should be on the BrowserTest class instead

Good point. Done

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:1319: function
reportMessage(msg, isFirstMessage, isStatusUpdate, isDone) {
On 2013/11/20 12:20:41, ricow1 wrote:
> yoiu don't use the isDone parameter, so why pass it in?

Removed it.

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:1332: test_completed = true;
On 2013/11/20 12:20:41, ricow1 wrote:
> you should probably do something if isDone is not true here

I removed it.

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/b...
dart/tools/testing/dart/browser_controller.dart:1374: reportMessage(msg, msg ==
'STARTING', false);
On 2013/11/20 12:20:41, ricow1 wrote:
> if you want to use it, pass in isDOne here as well

Removed it.

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
File dart/tools/testing/dart/test_controller.js (right):

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:6: * The new communication protocol
between test_controller.js and the driving
On 2013/11/20 12:20:41, ricow1 wrote:
> remove new (that will get old pretty fast :-) )

Done.

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:133: value: '' +
window.document.documentElement.innerHTML,
On 2013/11/20 12:20:41, ricow1 wrote:
> is this to guarantee a string?
Yes.

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:170: }
On 2013/11/20 12:20:41, ricow1 wrote:
> add blank line

Done.

https://codereview.chromium.org/77763004/diff/50001/dart/tools/testing/dart/t...
dart/tools/testing/dart/test_controller.js:179: }
On 2013/11/20 12:20:41, ricow1 wrote:
> add blank line

Done.