Add test fixture for GPU browser tests which do image comparisons....

Add test fixture for GPU browser tests which do image comparisons.
BUG=59131
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67877

[kkania, 2010-11-11 03:32:46]

This test just loads the webgl teapot example and takes a snapshot using the
ThumbnailGenerator.

I just created another test exe for these tests. It seemed the only good way to
be able to run just these tests on the GPU bots. It is still possible to use
OSMesa or the gpu depending on the flag you give it.

[Paweł Hajdan Jr., 2010-11-11 10:33:36]

Drive-by with minor test comments.

http://codereview.chromium.org/4723006/diff/42002/chrome/test/gpu/gpu_pixel_b...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/4723006/diff/42002/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:112:
PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
Check return value of this.

http://codereview.chromium.org/4723006/diff/42002/chrome/test/ui_test_utils.h
File chrome/test/ui_test_utils.h (right):

http://codereview.chromium.org/4723006/diff/42002/chrome/test/ui_test_utils.h...
chrome/test/ui_test_utils.h:460: bool TakeSnapshot(RenderWidgetHost* rwh,
nit: The name is too generic. Should it be TakeRenderWidgetSnapshot?

[kkania, 2010-11-15 20:07:03]

Addressed Pawel's comments.

http://codereview.chromium.org/4723006/diff/42002/chrome/test/gpu/gpu_pixel_b...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/4723006/diff/42002/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:112:
PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
On 2010/11/11 10:33:36, Paweł Hajdan Jr. wrote:
> Check return value of this.

Done.

http://codereview.chromium.org/4723006/diff/42002/chrome/test/ui_test_utils.h
File chrome/test/ui_test_utils.h (right):

http://codereview.chromium.org/4723006/diff/42002/chrome/test/ui_test_utils.h...
chrome/test/ui_test_utils.h:460: bool TakeSnapshot(RenderWidgetHost* rwh,
On 2010/11/11 10:33:36, Paweł Hajdan Jr. wrote:
> nit: The name is too generic. Should it be TakeRenderWidgetSnapshot?

Done.

[Paweł Hajdan Jr., 2010-11-15 20:20:11]

Code I commented in the drive-by LGTM. Thanks!

[apatrick_chromium, 2010-11-15 22:10:23]

A few things...

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:87: // This enables DOM automation for
tab contentses.
contentses -> contents

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:93: if
(command_line->HasSwitch(switches::kUseGL)) {
This used to be the case but I just changed it. EGL is now default on Windows
and desktop is default on other platforms. There is also now
test_launcher_utils::OverrideGLImplementation.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:160: img_name =
StringPrintf("%s_%s_%d-%d.png",
nit: device and vendor ids are conventionally rendered in hex.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/test_launcher_u...
File chrome/test/test_launcher_utils.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/test_launcher_u...
chrome/test/test_launcher_utils.cc:38: if
(!command_line->HasSwitch(switches::kUseGL)) {
This code is now deleted.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/ui_test_utils.cc
File chrome/test/ui_test_utils.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/ui_test_utils.c...
chrome/test/ui_test_utils.cc:844: SnapshotTaker() {}
Initialize bitmap_ to null.

[kkania, 2010-11-15 23:09:16]

Addressed apatrick's comments

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:87: // This enables DOM automation for
tab contentses.
On 2010/11/15 22:10:23, apatrick_chromium wrote:
> contentses -> contents

Done.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:93: if
(command_line->HasSwitch(switches::kUseGL)) {
On 2010/11/15 22:10:23, apatrick_chromium wrote:
> This used to be the case but I just changed it. EGL is now default on Windows
> and desktop is default on other platforms. There is also now
> test_launcher_utils::OverrideGLImplementation.

Ok. Changed this to use OSMesa by default.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:160: img_name =
StringPrintf("%s_%s_%d-%d.png",
On 2010/11/15 22:10:23, apatrick_chromium wrote:
> nit: device and vendor ids are conventionally rendered in hex.

Done.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/test_launcher_u...
File chrome/test/test_launcher_utils.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/test_launcher_u...
chrome/test/test_launcher_utils.cc:38: if
(!command_line->HasSwitch(switches::kUseGL)) {
On 2010/11/15 22:10:23, apatrick_chromium wrote:
> This code is now deleted.

Done.

http://codereview.chromium.org/4723006/diff/56001/chrome/test/ui_test_utils.cc
File chrome/test/ui_test_utils.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/ui_test_utils.c...
chrome/test/ui_test_utils.cc:844: SnapshotTaker() {}
On 2010/11/15 22:10:23, apatrick_chromium wrote:
> Initialize bitmap_ to null.

Done.

[kkania, 2010-11-16 23:13:57]

[+ nduca]

[Vangelis Kokkevis, 2010-11-19 18:49:05]

Looks good.  No real comments other than the spelling error..

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/4723006/diff/56001/chrome/test/gpu/gpu_pixel_b...
chrome/test/gpu/gpu_pixel_browsertest.cc:87: // This enables DOM automation for
tab contentses.
typo: contentses

[kkania, 2010-11-24 01:24:07]

apatrick, I am still waiting for your LGTM; can you review again?

[apatrick_chromium, 2010-11-24 01:51:40]

lgtm