ozone: gpu: Add plumbing for platform-specific gpu messaging

ozone: gpu: Add plumbing for platform-specific gpu messaging

ChromeOS without X11 will do display configuration and cursor
movement from the GPU process. This is the conduit for the messages
needed to make this work.

This plumbing also supports other platforms with similar requirements
(e.g. wayland's port). X11 does not need any specific messaging here,
since we just call the X server from the browser.

BUG=377497
TEST=content_shell --ozone-platform=egltest

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

[spang, 2014-06-16 19:58:08]

This is a bit ugly since we now have to mint route ids for the GpuProcessHost
channel and because we need to bounce messages between threads.

[spang, 2014-06-16 21:19:36]

+piman

Is this a reasonable approach, or should we perhaps have a new IPC::Channel
altogether?

[alexst (slow to review), 2014-06-16 21:34:23]

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/browser_...
File content/browser/gpu/browser_gpu_channel_host_factory.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/browser_...
content/browser/gpu/browser_gpu_channel_host_factory.cc:205:
ui::GpuPlatformSupportHost::Initialize();
Do we need to explicitly init this here? Presumably the ozone platform was
initialized already and this could happen inside that code?

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:530: int32 host_route_id =
GenerateRouteID();
This is a public function that seems to only be used here and only on the IO
thread, is there a reason you chose to use AtomicSequenceNumber?

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
File ui/ozone/gpu/gpu_platform_support.h (right):

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
ui/ozone/gpu/gpu_platform_support.h:22: static GpuPlatformSupport* Initialize();
Init returns the object here, so does GetInstance but GetInstance isn't called
anywhere that I can see. 
We also call ChannelEstablished right after Initialize. Same as my other
comment, should the platform init the object and OnChannelEstablished act as an
activation trigger?

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
File ui/ozone/gpu/gpu_platform_support_host.h (right):

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
ui/ozone/gpu/gpu_platform_support_host.h:23: class OZONE_GPU_EXPORT
GpuPlatformSupportHost : public IPC::Listener {
Threading notes would be useful here.

[rjkroege, 2014-06-16 21:38:49]

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:530: int32 host_route_id =
GenerateRouteID();
you are presuming that we never call this code more than once / gpu process
instance. Is this valid?

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.h (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.h:155: int32 GenerateRouteID();
does this need to be public? I missed seeing it being called anywhere outside of
gpu_process_host.cc

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host_ui_shim.h (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host_ui_shim.h:81: // Called to add a listener
for a particular message routing ID.
This comment probably should say why this is here.

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host_ui_shim.h:111: void
OnPlatformSupportInitialized(int32 host_route_id, int32 gpu_route_id);
More comments about this message?

https://codereview.chromium.org/338193003/diff/1/content/common/gpu/gpu_messa...
File content/common/gpu/gpu_messages.h (right):

https://codereview.chromium.org/338193003/diff/1/content/common/gpu/gpu_messa...
content/common/gpu/gpu_messages.h:326: // Tells the browser to initialize
GpuPlatformSupport.
isn't it telling the GPU process?

https://codereview.chromium.org/338193003/diff/1/content/gpu/gpu_child_thread.cc
File content/gpu/gpu_child_thread.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/gpu/gpu_child_thread...
content/gpu/gpu_child_thread.cc:163: void
GpuChildThread::OnInitializePlatformSupport(int32 host_route_id) {
MessageFilter might make this cleaner. Might let you remove the #if

[piman, 2014-06-16 23:53:15]

I'd rather reuse the existing channel - we already have 2 channels for better or
worse, I'd rather not add a 3rd one. It seems we can piggy back on it.

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:530: int32 host_route_id =
GenerateRouteID();
What is the purpose of the host route id?
1- GenerateRouteID is only called once, so this will always be 1
2- you're sending back a gpu route id, can't you use that same id?

More generally, it seems that you have only one GpuPlatformSupport{Host,}, do
you need routing IDs at all?

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:531: if (!Send(new
GpuMsg_InitializePlatformSupport(host_route_id)))
Can you combine this with the GpuMsg_Initialize message?

Can you also combine GpuHostMsg_PlatformSupportInitialized with
GpuHostMsg_Initialized, and use RouteOnUIThread to forward to the UI shim?

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
File ui/ozone/gpu/gpu_platform_support.cc (right):

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
ui/ozone/gpu/gpu_platform_support.cc:31: CHECK(!instance_);
DCHECK instead

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
File ui/ozone/gpu/gpu_platform_support_host.cc (right):

https://codereview.chromium.org/338193003/diff/1/ui/ozone/gpu/gpu_platform_su...
ui/ozone/gpu/gpu_platform_support_host.cc:34: CHECK(!instance_);
DCHECK instead.

[spang, 2014-06-17 14:47:34]

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/browser_...
File content/browser/gpu/browser_gpu_channel_host_factory.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/browser_...
content/browser/gpu/browser_gpu_channel_host_factory.cc:205:
ui::GpuPlatformSupportHost::Initialize();
On 2014/06/16 21:34:23, alexst wrote:
> Do we need to explicitly init this here? Presumably the ozone platform was
> initialized already and this could happen inside that code?

Actually, our other initializer can't do this because it is a lower level
component. It would be a layering violation.

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:530: int32 host_route_id =
GenerateRouteID();
On 2014/06/16 23:53:14, piman wrote:
> What is the purpose of the host route id?
> 1- GenerateRouteID is only called once, so this will always be 1

Right. I thought of making it a constant, and we can definitely still do that -
I was just aiming to be consistent with GpuChannelHost.

> 2- you're sending back a gpu route id, can't you use that same id?

We could. If we say that the ids for both ends are in the same namespace, then
that would work, but the browser could not set up a route for itself without a
round trip to the GPU process. That's not a problem for this so we can certainly
do it.

> 
> More generally, it seems that you have only one GpuPlatformSupport{Host,}, do
> you need routing IDs at all?

We simply need a way to ensure the common GPU code does not attempt to handle
our messages, and instead hands them off to the platform code. That's all I'm
using the routing ids for.

Should we attach our own filter instead, and handle just the platform messages?
We should be able to avoid the routing ids that way.

https://codereview.chromium.org/338193003/diff/1/content/browser/gpu/gpu_proc...
content/browser/gpu/gpu_process_host.cc:531: if (!Send(new
GpuMsg_InitializePlatformSupport(host_route_id)))
On 2014/06/16 23:53:14, piman wrote:
> Can you combine this with the GpuMsg_Initialize message?
> 
> Can you also combine GpuHostMsg_PlatformSupportInitialized with
> GpuHostMsg_Initialized, and use RouteOnUIThread to forward to the UI shim?

Will merge them.

https://codereview.chromium.org/338193003/diff/1/content/common/gpu/gpu_messa...
File content/common/gpu/gpu_messages.h (right):

https://codereview.chromium.org/338193003/diff/1/content/common/gpu/gpu_messa...
content/common/gpu/gpu_messages.h:326: // Tells the browser to initialize
GpuPlatformSupport.
On 2014/06/16 21:38:49, rjkroege wrote:
> isn't it telling the GPU process? 

Yes - oops.

[piman, 2014-06-17 21:17:13]

On Tue, Jun 17, 2014 at 7:47 AM, <spang@chromium.org> wrote:

>
> https://codereview.chromium.org/338193003/diff/1/content/
> browser/gpu/browser_gpu_channel_host_factory.cc
> File content/browser/gpu/browser_gpu_channel_host_factory.cc (right):
>
> https://codereview.chromium.org/338193003/diff/1/content/
> browser/gpu/browser_gpu_channel_host_factory.cc#newcode205
> content/browser/gpu/browser_gpu_channel_host_factory.cc:205:
> ui::GpuPlatformSupportHost::Initialize();
> On 2014/06/16 21:34:23, alexst wrote:
>
>> Do we need to explicitly init this here? Presumably the ozone platform
>>
> was
>
>> initialized already and this could happen inside that code?
>>
>
> Actually, our other initializer can't do this because it is a lower
> level component. It would be a layering violation.
>
>
> https://codereview.chromium.org/338193003/diff/1/content/
> browser/gpu/gpu_process_host.cc
> File content/browser/gpu/gpu_process_host.cc (right):
>
> https://codereview.chromium.org/338193003/diff/1/content/
> browser/gpu/gpu_process_host.cc#newcode530
> content/browser/gpu/gpu_process_host.cc:530: int32 host_route_id =
> GenerateRouteID();
> On 2014/06/16 23:53:14, piman wrote:
>
>> What is the purpose of the host route id?
>> 1- GenerateRouteID is only called once, so this will always be 1
>>
>
> Right. I thought of making it a constant, and we can definitely still do
> that - I was just aiming to be consistent with GpuChannelHost.
>
>
>  2- you're sending back a gpu route id, can't you use that same id?
>>
>
> We could. If we say that the ids for both ends are in the same
> namespace, then that would work, but the browser could not set up a
> route for itself without a round trip to the GPU process. That's not a
> problem for this so we can certainly do it.


Generally, we use the same routing id on both sides - see e.g.
CommandBufferProxyImpl vs GpuCommandBufferStub.
But I don't think you need routing ids at all (see below).


>
>  More generally, it seems that you have only one
>>
> GpuPlatformSupport{Host,}, do
>
>> you need routing IDs at all?
>>
>
> We simply need a way to ensure the common GPU code does not attempt to
> handle our messages, and instead hands them off to the platform code.
> That's all I'm using the routing ids for.
>
> Should we attach our own filter instead, and handle just the platform
> messages? We should be able to avoid the routing ids that way.


You could use a filter, but I don't think it's needed, at least as of
currently.
The messages are already filtered explicitly in the OnMessageReceived
/ OnControlMessageReceived, so it's just a matter of doing the right thing
there for each of the messages that you need for this. Either way you'll
have to explicitly handle messages either in the GpuProcessHost&UIStub
(browser side) / GpuChildThread&GpuChannelManager (gpu process side) or in
the filter(s).

If you want to use a filter, keep in mind that it runs on the IO thread -
that's really the main use case, to avoid round trips to a UI thread or
other. It may be desirable for some of the things you want to do in the GPU
process (e.g. low-latency cursor updates), but it's better to think first
along this axis (which thread we need these to run) rather than groupings
around features.


>
> https://codereview.chromium.org/338193003/diff/1/content/
> browser/gpu/gpu_process_host.cc#newcode531
> content/browser/gpu/gpu_process_host.cc:531: if (!Send(new
> GpuMsg_InitializePlatformSupport(host_route_id)))
> On 2014/06/16 23:53:14, piman wrote:
>
>> Can you combine this with the GpuMsg_Initialize message?
>>
>
>  Can you also combine GpuHostMsg_PlatformSupportInitialized with
>> GpuHostMsg_Initialized, and use RouteOnUIThread to forward to the UI
>>
> shim?
>
> Will merge them.
>
>
> https://codereview.chromium.org/338193003/diff/1/content/
> common/gpu/gpu_messages.h
> File content/common/gpu/gpu_messages.h (right):
>
> https://codereview.chromium.org/338193003/diff/1/content/
> common/gpu/gpu_messages.h#newcode326
> content/common/gpu/gpu_messages.h:326: // Tells the browser to
> initialize GpuPlatformSupport.
> On 2014/06/16 21:38:49, rjkroege wrote:
>
>> isn't it telling the GPU process?
>>
>
> Yes - oops.
>
> https://codereview.chromium.org/338193003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[spang, 2014-06-18 15:44:26]

PTAL. I've removed everything related to routing.

All unhandled messages now get routed to platform support to see if it can be
handled there. 

On 2014/06/17 21:17:13, piman wrote:
> On Tue, Jun 17, 2014 at 7:47 AM, <mailto:spang@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > browser/gpu/browser_gpu_channel_host_factory.cc
> > File content/browser/gpu/browser_gpu_channel_host_factory.cc (right):
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > browser/gpu/browser_gpu_channel_host_factory.cc#newcode205
> > content/browser/gpu/browser_gpu_channel_host_factory.cc:205:
> > ui::GpuPlatformSupportHost::Initialize();
> > On 2014/06/16 21:34:23, alexst wrote:
> >
> >> Do we need to explicitly init this here? Presumably the ozone platform
> >>
> > was
> >
> >> initialized already and this could happen inside that code?
> >>
> >
> > Actually, our other initializer can't do this because it is a lower
> > level component. It would be a layering violation.
> >
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > browser/gpu/gpu_process_host.cc
> > File content/browser/gpu/gpu_process_host.cc (right):
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > browser/gpu/gpu_process_host.cc#newcode530
> > content/browser/gpu/gpu_process_host.cc:530: int32 host_route_id =
> > GenerateRouteID();
> > On 2014/06/16 23:53:14, piman wrote:
> >
> >> What is the purpose of the host route id?
> >> 1- GenerateRouteID is only called once, so this will always be 1
> >>
> >
> > Right. I thought of making it a constant, and we can definitely still do
> > that - I was just aiming to be consistent with GpuChannelHost.
> >
> >
> >  2- you're sending back a gpu route id, can't you use that same id?
> >>
> >
> > We could. If we say that the ids for both ends are in the same
> > namespace, then that would work, but the browser could not set up a
> > route for itself without a round trip to the GPU process. That's not a
> > problem for this so we can certainly do it.
> 
> 
> Generally, we use the same routing id on both sides - see e.g.
> CommandBufferProxyImpl vs GpuCommandBufferStub.
> But I don't think you need routing ids at all (see below).
> 
> 
> >
> >  More generally, it seems that you have only one
> >>
> > GpuPlatformSupport{Host,}, do
> >
> >> you need routing IDs at all?
> >>
> >
> > We simply need a way to ensure the common GPU code does not attempt to
> > handle our messages, and instead hands them off to the platform code.
> > That's all I'm using the routing ids for.
> >
> > Should we attach our own filter instead, and handle just the platform
> > messages? We should be able to avoid the routing ids that way.
> 
> 
> You could use a filter, but I don't think it's needed, at least as of
> currently.
> The messages are already filtered explicitly in the OnMessageReceived
> / OnControlMessageReceived, so it's just a matter of doing the right thing
> there for each of the messages that you need for this. Either way you'll
> have to explicitly handle messages either in the GpuProcessHost&UIStub
> (browser side) / GpuChildThread&GpuChannelManager (gpu process side) or in
> the filter(s).
> 
> If you want to use a filter, keep in mind that it runs on the IO thread -
> that's really the main use case, to avoid round trips to a UI thread or
> other. It may be desirable for some of the things you want to do in the GPU
> process (e.g. low-latency cursor updates), but it's better to think first
> along this axis (which thread we need these to run) rather than groupings
> around features.
> 
> 
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > browser/gpu/gpu_process_host.cc#newcode531
> > content/browser/gpu/gpu_process_host.cc:531: if (!Send(new
> > GpuMsg_InitializePlatformSupport(host_route_id)))
> > On 2014/06/16 23:53:14, piman wrote:
> >
> >> Can you combine this with the GpuMsg_Initialize message?
> >>
> >
> >  Can you also combine GpuHostMsg_PlatformSupportInitialized with
> >> GpuHostMsg_Initialized, and use RouteOnUIThread to forward to the UI
> >>
> > shim?
> >
> > Will merge them.
> >
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > common/gpu/gpu_messages.h
> > File content/common/gpu/gpu_messages.h (right):
> >
> > https://codereview.chromium.org/338193003/diff/1/content/
> > common/gpu/gpu_messages.h#newcode326
> > content/common/gpu/gpu_messages.h:326: // Tells the browser to
> > initialize GpuPlatformSupport.
> > On 2014/06/16 21:38:49, rjkroege wrote:
> >
> >> isn't it telling the GPU process?
> >>
> >
> > Yes - oops.
> >
> > https://codereview.chromium.org/338193003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[rjkroege, 2014-06-18 17:02:26]

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support.h (right):

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support.h:27: // Called when the GPU process is spun
up & channel established.
called by who?

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support_host.h (right):

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support_host.h:22: // to support additional messages
needed by specific platforms.
I think it would be really nice to add a little example of how this should be
used. In particular, the calling sequence that we would expect and what are the
expectations on a platform that needs this functionality.

[spang, 2014-06-18 20:11:14]

I moved the files into to the monolithic ozone component. Makine a new component
seems like an unnecessary distraction at the moment.

Doing so makes the separate initialization path go away.


On 2014/06/18 17:02:26, rjkroege wrote:
>
https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
> File ui/ozone/gpu/gpu_platform_support.h (right):
> 
>
https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
> ui/ozone/gpu/gpu_platform_support.h:27: // Called when the GPU process is spun
> up & channel established.
> called by who?
> 
>
https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
> File ui/ozone/gpu/gpu_platform_support_host.h (right):
> 
>
https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
> ui/ozone/gpu/gpu_platform_support_host.h:22: // to support additional messages
> needed by specific platforms.
> I think it would be really nice to add a little example of how this should be
> used. In particular, the calling sequence that we would expect and what are
the
> expectations on a platform that needs this functionality.

[piman, 2014-06-18 21:51:39]

https://codereview.chromium.org/338193003/diff/40001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support.h (right):

https://codereview.chromium.org/338193003/diff/40001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support.h:29: static GpuPlatformSupport* instance_;
Rather than adding more statics, could this be accessed via the OzonePlatform,
since it's a member of it?

https://codereview.chromium.org/338193003/diff/40001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support_host.h (right):

https://codereview.chromium.org/338193003/diff/40001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support_host.h:38: static GpuPlatformSupportHost*
instance_;
Same concern re: statics.

[spang, 2014-06-18 23:21:43]

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support.h (right):

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support.h:27: // Called when the GPU process is spun
up & channel established.
On 2014/06/18 17:02:25, rjkroege wrote:
> called by who? 

By content. But the implementation shouldn't really care either way.

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support_host.h (right):

https://codereview.chromium.org/338193003/diff/20001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support_host.h:22: // to support additional messages
needed by specific platforms.
On 2014/06/18 17:02:25, rjkroege wrote:
> I think it would be really nice to add a little example of how this should be
> used. In particular, the calling sequence that we would expect and what are
the
> expectations on a platform that needs this functionality.

The display configuration bits will provide an example. I think that probably
fills this need.

https://codereview.chromium.org/338193003/diff/40001/ui/ozone/gpu/gpu_platfor...
File ui/ozone/gpu/gpu_platform_support.h (right):

https://codereview.chromium.org/338193003/diff/40001/ui/ozone/gpu/gpu_platfor...
ui/ozone/gpu/gpu_platform_support.h:29: static GpuPlatformSupport* instance_;
On 2014/06/18 21:51:38, piman wrote:
> Rather than adding more statics, could this be accessed via the OzonePlatform,
> since it's a member of it?

Yes, either way works. I've changed it.

[piman, 2014-06-19 01:17:40]

lgtm

[rjkroege, 2014-06-19 14:26:28]

lgtm

[spang, 2014-06-19 15:00:09]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2014-06-19 15:01:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/spang@chromium.org/338193003/60001

[commit-bot: I haz the power, 2014-06-19 20:01:04]

Change committed as 278465

[chrishtr, 2014-06-19 21:32:08]

A revert of this CL has been created in
https://codereview.chromium.org/339793007/ by chrishtr@chromium.org.

The reason for reverting is: Suspect it is causing a failure of
webgl_conformance_tests for this bot:

http://build.chromium.org/p/chromium.gpu/builders/Win7%20Release%20(Intel)/bu....