[Telemetry] Fix a possible source of flakiness on Android.

[Telemetry] Fix a possible source of flakiness on Android.

With this, we survive killing the adb process which produces the same stack as:
http://build.chromium.org/p/chromium.perf/builders/Android%20GN/builds/764/steps/jsgamebench/logs/stdio

BUG=163661
TEST='killall -9 adb' while running a benchmark

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=171088

[tonyg, 2012-12-04 03:41:10]

ptal

[nduca, 2012-12-04 03:46:18]

I'm having a hard time grokking "connection gone"... what does it mean exactly?

[nduca, 2012-12-04 04:07:47]

https://codereview.chromium.org/11348370/diff/1/tools/telemetry/telemetry/mul...
File tools/telemetry/telemetry/multi_page_benchmark_runner.py (right):

https://codereview.chromium.org/11348370/diff/1/tools/telemetry/telemetry/mul...
tools/telemetry/telemetry/multi_page_benchmark_runner.py:71: while retries:
This shouldn't be done. We really need to robustly handle this somehow inside
telemetry. The point of page_runner is to do the retry logic again. Perhaps you
should raise BrowserGone. That'll cause us to recreate the browser. Then,
page_runner could track the number of tries and fail catastrophically if that
happens?

[tonyg, 2012-12-04 05:21:56]

On 2012/12/04 04:07:47, nduca wrote:
>
https://codereview.chromium.org/11348370/diff/1/tools/telemetry/telemetry/mul...
> File tools/telemetry/telemetry/multi_page_benchmark_runner.py (right):
> 
>
https://codereview.chromium.org/11348370/diff/1/tools/telemetry/telemetry/mul...
> tools/telemetry/telemetry/multi_page_benchmark_runner.py:71: while retries:
> This shouldn't be done. We really need to robustly handle this somehow inside
> telemetry. The point of page_runner is to do the retry logic again. Perhaps
you
> should raise BrowserGone. That'll cause us to recreate the browser. Then,
> page_runner could track the number of tries and fail catastrophically if that
> happens?

You are right, I'll rework this. But first a little cleanup is needed:
https://codereview.chromium.org/11415280/

[tonyg, 2012-12-04 17:47:41]

On 2012/12/04 03:46:18, nduca wrote:
> I'm having a hard time grokking "connection gone"... what does it mean
exactly?

We only raise this exception when the browser process is still running but there
is an error communicating with devtools. So it is about the connection to the
browser being gone, not the browser itself being gone.

[tonyg, 2012-12-04 17:48:17]

On 2012/12/04 05:21:56, tonyg wrote:
> On 2012/12/04 04:07:47, nduca wrote:
> >
>
https://codereview.chromium.org/11348370/diff/1/tools/telemetry/telemetry/mul...
> > File tools/telemetry/telemetry/multi_page_benchmark_runner.py (right):
> > 
> >
>
https://codereview.chromium.org/11348370/diff/1/tools/telemetry/telemetry/mul...
> > tools/telemetry/telemetry/multi_page_benchmark_runner.py:71: while retries:
> > This shouldn't be done. We really need to robustly handle this somehow
inside
> > telemetry. The point of page_runner is to do the retry logic again. Perhaps
> you
> > should raise BrowserGone. That'll cause us to recreate the browser. Then,
> > page_runner could track the number of tries and fail catastrophically if
that
> > happens?
> 
> You are right, I'll rework this. But first a little cleanup is needed:
> https://codereview.chromium.org/11415280/

OK, I moved the retry logic into page_runner. This is ready for another look.

[nduca, 2012-12-04 22:56:44]

lgtm but lets see if we can make browser_gone inherit from connection_gone or
vice versa so that the common case ("just recreate the browser") doesn't have to
think about connection_gone. Either inheretence of a why field...

[tonyg, 2012-12-04 23:19:11]

On 2012/12/04 22:56:44, nduca wrote:
> lgtm but lets see if we can make browser_gone inherit from connection_gone or
> vice versa so that the common case ("just recreate the browser") doesn't have
to
> think about connection_gone. Either inheretence of a why field...

Done. Want to take another look?

[nduca, 2012-12-04 23:35:44]

lgtm

https://codereview.chromium.org/11348370/diff/8004/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/page_runner.py (right):

https://codereview.chromium.org/11348370/diff/8004/tools/telemetry/telemetry/...
tools/telemetry/telemetry/page_runner.py:166: raise
browser_gone_exception.BrowserGoneException()
should this mark the page as failing?

https://codereview.chromium.org/11348370/diff/8004/tools/telemetry/telemetry/...
tools/telemetry/telemetry/page_runner.py:192: raise
browser_gone_exception.BrowserGoneException()
addfailure/

https://codereview.chromium.org/11348370/diff/8004/tools/telemetry/telemetry/...
tools/telemetry/telemetry/page_runner.py:313: try:
mental note --- whatever @bulach comes up with to containerize the power
throttling hooks for perf tests should be where this goes too, since this is
also perf-test specific shizzle.

[tonyg, 2012-12-05 00:01:27]

https://codereview.chromium.org/11348370/diff/8004/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/page_runner.py (right):

https://codereview.chromium.org/11348370/diff/8004/tools/telemetry/telemetry/...
tools/telemetry/telemetry/page_runner.py:166: raise
browser_gone_exception.BrowserGoneException()
On 2012/12/04 23:35:44, nduca wrote:
> should this mark the page as failing?

No, we are retrying it with hopes it will succeed. If we add a failure here, the
exit code would be 1 and the perf step would fail.

[nduca, 2012-12-05 00:04:35]

Oh, I get it. Sorry about that.