mac content shell drt: Hook up a dummy image transport

mac content shell drt: Hook up a dummy image transport

WebKit's layout tests require running with osmesa to get reliable pixel results.
Since content shell drt spins up a full gpu process, an image transport
implementation is needed for osmesa. content shell drt reads pixel baselines
off the renderer, so the implementation doesn't have to do much.

To make sure it doesn't get picked accidentally, put it behind a off-by-default
flag that's only toggled on in content shell drt.

BUG=111316
TEST=
`webkit/tools/layout_tests/run_webkit_tests.sh --driver-name 'Content Shell' --additional-drt-flag --dump-render-tree webgl/conformance/attribs/gl-disabled-vertex-attrib.html` passes
`webkit/tools/layout_tests/run_webkit_tests.sh --driver-name 'Content Shell' --additional-drt-flag --dump-render-tree animations/3d/change-transform-in-end-event.html` has only minor text antialiasing pixel differences
Note: Running `out/Release/Content\ Shell.app/Contents/MacOS/Content\ Shell third_party/WebKit/LayoutTests/animations/3d/change-transform-in-end-event.html --dump-render-tree` won't update the drt window correctly due to this being a dummy transport bitmap. Eventually --dump-render-tree will hide the window, so that should be ok. (And without --dump-render-tree, the test renders fine.)

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

[Nico, 2013-02-26 13:57:28]

A different take on https://codereview.chromium.org/12326013/

[jochen (gone - plz use gerrit), 2013-02-26 14:12:07]

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
File content/common/gpu/image_transport_surface.h (right):

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface.h:73: CONTENT_EXPORT static void
SetAllowOSMesa(bool allow);
I would call the method ...ForTesting so the presubmit checks make sure it's not
accidentally used for production code.

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
File content/common/gpu/image_transport_surface_mac.cc (right):

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:11: #include
"content/shell/shell_switches.h"
why do you need this?

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:420: explicit
DRTSurfaceOSMesa()
no explicit

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:427: ~DRTSurfaceOSMesa() {}
virtual

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:467: surface = new
DRTSurfaceOSMesa();
does it matter that it prints the error message? Can't we just use new
GLsurfaceOSMesa() here?

https://codereview.chromium.org/12315103/diff/1/content/shell/shell_main_dele...
File content/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/12315103/diff/1/content/shell/shell_main_dele...
content/shell/shell_main_delegate.cc:11: #include
"content/common/gpu/image_transport_surface.h"
content_shell is a content embedder and therefore can only include public
headers.

You can add a method to content/public/test/layouttest_support.h (and
content/test/layouttset_support.cc) to forward the call.

[Nico, 2013-02-26 14:26:52]

Thanks!

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
File content/common/gpu/image_transport_surface_mac.cc (right):

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:11: #include
"content/shell/shell_switches.h"
On 2013/02/26 14:12:07, jochen wrote:
> why do you need this?

Removed.

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:420: explicit
DRTSurfaceOSMesa()
On 2013/02/26 14:12:07, jochen wrote:
> no explicit

Done.

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:427: ~DRTSurfaceOSMesa() {}
On 2013/02/26 14:12:07, jochen wrote:
> virtual

Done.

https://codereview.chromium.org/12315103/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:467: surface = new
DRTSurfaceOSMesa();
On 2013/02/26 14:12:07, jochen wrote:
> does it matter that it prints the error message? Can't we just use new
> GLsurfaceOSMesa() here?

Sure it matters, random error log spew is confusing.

(Also, GLSurfaceOSMesa() returns false from its swap method, which causes
context lost events all the time.)

https://codereview.chromium.org/12315103/diff/1/content/shell/shell_main_dele...
File content/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/12315103/diff/1/content/shell/shell_main_dele...
content/shell/shell_main_delegate.cc:11: #include
"content/common/gpu/image_transport_surface.h"
On 2013/02/26 14:12:07, jochen wrote:
> content_shell is a content embedder and therefore can only include public
> headers.
> 
> You can add a method to content/public/test/layouttest_support.h (and
> content/test/layouttset_support.cc) to forward the call.

Done.

Is this automatically enforced somehow?

[jochen (gone - plz use gerrit), 2013-02-26 14:35:17]

lgtm

https://codereview.chromium.org/12315103/diff/1/content/shell/shell_main_dele...
File content/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/12315103/diff/1/content/shell/shell_main_dele...
content/shell/shell_main_delegate.cc:11: #include
"content/common/gpu/image_transport_surface.h"
On 2013/02/26 14:26:52, Nico wrote:
> On 2013/02/26 14:12:07, jochen wrote:
> > content_shell is a content embedder and therefore can only include public
> > headers.
> > 
> > You can add a method to content/public/test/layouttest_support.h (and
> > content/test/layouttset_support.cc) to forward the call.
> 
> Done.
> 
> Is this automatically enforced somehow?

sure, via include rules in the DEPS files.

https://codereview.chromium.org/12315103/diff/7001/content/shell/shell_main_d...
File content/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/12315103/diff/7001/content/shell/shell_main_d...
content/shell/shell_main_delegate.cc:104:
SetAllowOSMesaImageTransportForTesting();
this code only runs in the browser process. Is this enough for this purpose?

[Nico, 2013-02-26 14:37:25]

https://codereview.chromium.org/12315103/diff/7001/content/shell/shell_main_d...
File content/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/12315103/diff/7001/content/shell/shell_main_d...
content/shell/shell_main_delegate.cc:104:
SetAllowOSMesaImageTransportForTesting();
On 2013/02/26 14:35:18, jochen wrote:
> this code only runs in the browser process. Is this enough for this purpose?

Apparently :-P

I think the gpu <-> browser io surface is created in the browser process, early
during startup.

[Ken Russell (switch to Gerrit), 2013-02-26 20:38:19]

LGTM if this works for your purposes.

[commit-bot: I haz the power, 2013-02-26 20:41:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/12315103/7001

[Nico, 2013-02-26 21:43:49]

Committed patchset #2 manually as r184740 (presubmit successful).