Enable Canvas2D hardware acceleration for Android WebView

Enable Canvas2D hardware acceleration for Android WebView

This patch is verified under stock browser which based on the recent Chrome M30 WebView, while verification under the latest trunk build seems unavailable from my side currently.

[boliu, 2014-01-06 18:16:13]

https://codereview.chromium.org/110583008/diff/50001/cc/resources/resource_pr...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/110583008/diff/50001/cc/resources/resource_pr...
cc/resources/resource_provider.cc:548: if (resource->mailbox.IsTexture() &&
ContextGL() != NULL) {
I know I did the same thing in my hack patch, but it is just a hack. We are
leaking resource handles if the context is destroyed but there are still gl
resources left.

https://codereview.chromium.org/110583008/diff/50001/webkit/renderer/composit...
File webkit/renderer/compositor_bindings/web_external_texture_layer_impl.cc
(right):

https://codereview.chromium.org/110583008/diff/50001/webkit/renderer/composit...
webkit/renderer/compositor_bindings/web_external_texture_layer_impl.cc:85: if
(bitmap && name.IsZero())
This is patching over another issue which I hacked around here:
https://codereview.chromium.org/124063003/diff/60001/cc/trees/layer_tree_host.cc

The right fix is that if we are in hardware canvas2d, then bitmap should be
false.

[changjun.yang, 2014-01-08 09:09:49]

On 2014/01/06 18:16:13, boliu wrote:
>
https://codereview.chromium.org/110583008/diff/50001/cc/resources/resource_pr...
> File cc/resources/resource_provider.cc (right):
> 
>
https://codereview.chromium.org/110583008/diff/50001/cc/resources/resource_pr...
> cc/resources/resource_provider.cc:548: if (resource->mailbox.IsTexture() &&
> ContextGL() != NULL) {
> I know I did the same thing in my hack patch, but it is just a hack. We are
> leaking resource handles if the context is destroyed but there are still gl
> resources left.

Looks https://code.google.com/p/chromium/issues/detail?id=331967 is related to
this issue, have you guys planned further solutions?

> 
>
https://codereview.chromium.org/110583008/diff/50001/webkit/renderer/composit...
> File webkit/renderer/compositor_bindings/web_external_texture_layer_impl.cc
> (right):
> 
>
https://codereview.chromium.org/110583008/diff/50001/webkit/renderer/composit...
> webkit/renderer/compositor_bindings/web_external_texture_layer_impl.cc:85: if
> (bitmap && name.IsZero())
> This is patching over another issue which I hacked around here:
>
https://codereview.chromium.org/124063003/diff/60001/cc/trees/layer_tree_host.cc
> 
> The right fix is that if we are in hardware canvas2d, then bitmap should be
> false.

A new proposal is raised for this issue, please help to review the updated patch
set. To be clarified, the prototype is based on M30 WebView and then rebased to
trunk directly without verification.

[boliu, 2014-01-08 17:55:04]

On 2014/01/08 09:09:49, changjun.yang wrote:
> On 2014/01/06 18:16:13, boliu wrote:
> >
>
https://codereview.chromium.org/110583008/diff/50001/cc/resources/resource_pr...
> > File cc/resources/resource_provider.cc (right):
> > 
> >
>
https://codereview.chromium.org/110583008/diff/50001/cc/resources/resource_pr...
> > cc/resources/resource_provider.cc:548: if (resource->mailbox.IsTexture() &&
> > ContextGL() != NULL) {
> > I know I did the same thing in my hack patch, but it is just a hack. We are
> > leaking resource handles if the context is destroyed but there are still gl
> > resources left.
> 
> Looks https://code.google.com/p/chromium/issues/detail?id=331967 is related to
> this issue, have you guys planned further solutions?

They are not related. crbug.com/331967 is about the command buffer service side
of things. This issue is in the compositor which only deals with the command
buffer client. Command buffer docs:
http://dev.chromium.org/developers/design-documents/gpu-command-buffer

I think the fix here should probably be in
TextureLayerImpl::DidLoseOutputSurface (which should really be renamed to
TextureLayerImpl::ReleaseResources), but I haven't looked closely at how
TextureLayerImpl and mailboxes interact though.

https://codereview.chromium.org/110583008/diff/230001/cc/trees/thread_proxy.cc
File cc/trees/thread_proxy.cc (right):

https://codereview.chromium.org/110583008/diff/230001/cc/trees/thread_proxy.c...
cc/trees/thread_proxy.cc:1443: layer_tree_host_impl_->GetRendererCapabilities();
Much less hacky, still has some problems though.

renderer_capabilities_main_thread_copy_ lives on the main thread (ie the blink
thread). This function is called by LayerTreeHostImpl on the "impl" thread (also
known as the compositor thread). Documentation for this is
http://dev.chromium.org/developers/design-documents/compositor-thread-archite...

So this assignment is not thread safe. The right way would be to post a task to
main thread.

I think this is the right way to go, but this opens up a larger issue that that
main thread copy of the capabilities can get out of sync with the impl thread
copy. So we should do an audit of every renderer capability.

If a cap is not used on main thread at all, then we should not make it
accessible on main thread.
Maybe it's used on the main thread, but never used by webview, in which case we
need to add DCHECKs.
If it is used by webview, then need to make sure that either the inconsistency
is harmless or can be handled in some fashion.

Sounds like something you want to take? I can file a bug and cc you and
compositor owner (who might have different opinion than me)