Ported GpuCrashTest to Telemetry, renaming the test to ContextLost.

Ported GpuCrashTest to Telemetry, renaming the test to ContextLost.

Added command line argument for disabling the WebGL infobar, required
for automated recovery from context loss. Added browser-side handling
of debug URLs for navigations coming from Telemetry, covered under
existing --enable-gpu-benchmarking command line argument.

BUG=308675
R=dtu@chromium.org, jam@chromium.org, zmo@chromium.org

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

[Ken Russell (switch to Gerrit), 2013-10-17 19:17:07]

sky or jam or darin: browser_url_handler_impl.cc
zmo: gpu_data_manager_impl_private.cc
joi: OWNERS review of content_switches.{cc,h}
dtu: context_lost.py

[jam, 2013-10-17 20:04:22]

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
content/public/common/content_switches.cc:158: // Disable the per-domain infobar
which blocks 3D APIs after GPU reset.
nit: don't mention infobar in content, just say per domain blocking

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
content/public/common/content_switches.h:59: extern const char
kDisableDomainBlockingFor3DAPIsForTesting[];
nit: ForTesting is a method suffix which scripts use to ensure that these
methods are only called from test files. but since this is a switch, and it can
be set from anywhere, that check can't happen. so just drop the ForTesting in
the name. that's what we do for other testing-only switches

[jam, 2013-10-17 20:05:19]

lgtm with nits btw

[Zhenyao Mo, 2013-10-17 20:11:23]

LGTM

[Ken Russell (switch to Gerrit), 2013-10-17 20:50:11]

Thanks for the quick review.

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
content/public/common/content_switches.cc:158: // Disable the per-domain infobar
which blocks 3D APIs after GPU reset.
On 2013/10/17 20:04:23, jam wrote:
> nit: don't mention infobar in content, just say per domain blocking

Done.

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/27603004/diff/1/content/public/common/content...
content/public/common/content_switches.h:59: extern const char
kDisableDomainBlockingFor3DAPIsForTesting[];
On 2013/10/17 20:04:23, jam wrote:
> nit: ForTesting is a method suffix which scripts use to ensure that these
> methods are only called from test files. but since this is a switch, and it
can
> be set from anywhere, that check can't happen. so just drop the ForTesting in
> the name. that's what we do for other testing-only switches

Thanks. Done.

[dtu, 2013-10-17 21:07:19]

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
File content/test/gpu/gpu_tests/context_lost.py (right):

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:42: class
ContextLostValidator(page_test.PageTest):
nit: _ContextLostValidator

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:71: raise page_test.Failure('Test
didn\'t complete')
WaitFor throws a TimeoutException on timeout, you should catch that instead.

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:73: raise page_test.Failure('Test
failed')
More descriptive failure message?

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:77: enabled = True
Are the GPU bot scripts using this? If not, I'm going to remove them completely.

[dtu, 2013-10-17 21:07:36]

lgtm

[Ken Russell (switch to Gerrit), 2013-10-17 21:24:52]

+skyostil

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
File content/test/gpu/gpu_tests/context_lost.py (right):

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:42: class
ContextLostValidator(page_test.PageTest):
On 2013/10/17 21:07:19, dtu wrote:
> nit: _ContextLostValidator

Done.

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:71: raise page_test.Failure('Test
didn\'t complete')
On 2013/10/17 21:07:19, dtu wrote:
> WaitFor throws a TimeoutException on timeout, you should catch that instead.

Done.

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:73: raise page_test.Failure('Test
failed')
On 2013/10/17 21:07:19, dtu wrote:
> More descriptive failure message?

Attempted.

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:77: enabled = True
On 2013/10/17 21:07:19, dtu wrote:
> Are the GPU bot scripts using this? If not, I'm going to remove them
completely.

I believe the Android GPU bot is using it to avoid running the webgl_robustness
test by default.

[Ken Russell (switch to Gerrit), 2013-10-17 23:31:39]

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

[Jói, 2013-10-18 10:41:46]

LGTM

[Sami, 2013-10-18 13:05:38]

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
File content/test/gpu/gpu_tests/context_lost.py (right):

https://codereview.chromium.org/27603004/diff/9001/content/test/gpu/gpu_tests...
content/test/gpu/gpu_tests/context_lost.py:77: enabled = True
On 2013/10/17 21:24:53, Ken Russell wrote:
> On 2013/10/17 21:07:19, dtu wrote:
> > Are the GPU bot scripts using this? If not, I'm going to remove them
> completely.
> 
> I believe the Android GPU bot is using it to avoid running the
webgl_robustness
> test by default.

To be clear, that flag was only added there to avoid running the tests on
non-Android platforms -- it's fine to run that on all Android bots. I guess test
expectations could be used instead to skip the tests, or just not add that test
suite into the non-Android recipes.