Make cross-process CALayers work on Mac

Make cross-process CALayers work on Mac

Add a header file for the remote layer API and a helper function to
determine if it is supported at runtime.

Add a mechanism for disambiguating if a "surface handle" refers to an
IOSurfaceID or a CAContextID (the thing that is passed across processes
to do cross-process CALayer drawing).

Add support in RenderWidgetHostViewMac to put in a CALayerHost from
another process's CAContextID. In the process of this, make the root
CALayer hanging off of the RenderWidgetHostViewMac have flipped
geometry, and update LayoutLayers to take this into account.

This is working surprisingly well, but needs a bit more work before it
can be turned on. We still need to:
* exert GPU back-pressure from the GPU process'
  -[ImageTransportLayer drawInCGLContext] function (it's an open loop
  now).
* use the isAsynchronous property to make 60fps animation not jerky.
* ensure that these resources' lifetimes are being managed reasonably
  and aren't eating tons of GPU
* ensure that not rounding texture sizes to the nearest 64 pixels (as is
  done for the IOSurface scheme) isn't causing fragmentation.

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

[ccameron, 2014-06-19 00:02:44]

This has turned out to be surprisingly painless (so far).

[ccameron, 2014-06-19 21:13:13]

Pinging this again

[piman, 2014-06-19 21:51:02]

Seems pretty reasonable, but I'm not a mac / CA expert.

LGTM for my parts, modulo a few nits.

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
File content/common/gpu/image_transport_surface_calayer_mac.mm (right):

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
content/common/gpu/image_transport_surface_calayer_mac.mm:122: GLenum error =
glGetError();
note: there can be a stack of GL errors. If you want to be thorough and get rid
of all errors you need a while loop. See e.g. ErrorStateImpl::ClearRealGLErrors

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
content/common/gpu/image_transport_surface_calayer_mac.mm:145:
reinterpret_cast<GLint*>(&texture));
How about passing the texture id to the function? The caller has it.

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
File content/common/gpu/image_transport_surface_fbo_mac.cc (right):

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
content/common/gpu/image_transport_surface_fbo_mac.cc:273:
glGetIntegerv(GL_TEXTURE_BINDING_RECTANGLE_ARB, &previous_texture_id);
Note: you can use  gfx::ScopedTextureBinder

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/surfa...
File content/common/gpu/surface_handle_types_mac.cc (right):

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/surfa...
content/common/gpu/surface_handle_types_mac.cc:49: uint64
SurfaceHandleFromCAContextID(CAContextID ca_context_id) {
How about making a struct { type, id } instead of a uint64? Seems a little
cleaner than this xor magic...

[ccameron, 2014-06-19 23:56:41]

Thanks!

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
File content/common/gpu/image_transport_surface_calayer_mac.mm (right):

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
content/common/gpu/image_transport_surface_calayer_mac.mm:122: GLenum error =
glGetError();
On 2014/06/19 21:51:02, piman wrote:
> note: there can be a stack of GL errors. If you want to be thorough and get
rid
> of all errors you need a while loop. See e.g.
ErrorStateImpl::ClearRealGLErrors

Done.

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
content/common/gpu/image_transport_surface_calayer_mac.mm:145:
reinterpret_cast<GLint*>(&texture));
On 2014/06/19 21:51:02, piman wrote:
> How about passing the texture id to the function? The caller has it.

Umm, that's a good point. Somehow that eluded me. Done.

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
File content/common/gpu/image_transport_surface_fbo_mac.cc (right):

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/image...
content/common/gpu/image_transport_surface_fbo_mac.cc:273:
glGetIntegerv(GL_TEXTURE_BINDING_RECTANGLE_ARB, &previous_texture_id);
On 2014/06/19 21:51:02, piman wrote:
> Note: you can use  gfx::ScopedTextureBinder

That indicates that it expects GLES 2.0 textures, while this isn't in the
supported list. I'll leave this as it was for now (except insofar as I'm fixing
the bug where we didn't even check for _BINDING_ before).

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/surfa...
File content/common/gpu/surface_handle_types_mac.cc (right):

https://codereview.chromium.org/347653005/diff/40001/content/common/gpu/surfa...
content/common/gpu/surface_handle_types_mac.cc:49: uint64
SurfaceHandleFromCAContextID(CAContextID ca_context_id) {
On 2014/06/19 21:51:02, piman wrote:
> How about making a struct { type, id } instead of a uint64? Seems a little
> cleaner than this xor magic...

The thing that bugs we is that both id types are typedefs of uint32s, so we have
no safety between them -- I wanted the XOR so that people would be punished for
just reading the id field without checking the type (either via a cast or by
accessing as a member of a struct).

[ccameron, 2014-06-19 23:57:53]

Adding avi for base/cocoa stuff

[Avi (use Gerrit), 2014-06-20 00:15:11]

ui/base/cocoa LGTM with nits

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.h (right):

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:19: CGSConnectionID CGSMainConnectionID(void);
I'm a bit surprised we don't already have definitions for this already
somewhere...

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:35: }
Pretty sure you can omit the braces here...

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:36: +
(id)contextWithCGSConnection:(CAContextID)contextId
options:(NSDictionary*)optionsDict;
80 cols

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:48: }
...and the braces here.

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.mm (right):

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.mm:21: @selector(contextWithCGSConnection:
options:)] &&
no space

[ccameron, 2014-06-20 00:55:06]

Thanks!!

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.h (right):

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:19: CGSConnectionID CGSMainConnectionID(void);
On 2014/06/20 00:15:11, Avi wrote:
> I'm a bit surprised we don't already have definitions for this already
> somewhere...

The definition seems pretty obscure -- a search on site:apple.com only gets 3
hits (none of them documentation).

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:35: }
On 2014/06/20 00:15:11, Avi wrote:
> Pretty sure you can omit the braces here...

Done.

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:36: +
(id)contextWithCGSConnection:(CAContextID)contextId
options:(NSDictionary*)optionsDict;
On 2014/06/20 00:15:11, Avi wrote:
> 80 cols

How did this pass presubmit?? Done.

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:48: }
On 2014/06/20 00:15:11, Avi wrote:
> ...and the braces here.

Done.

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.mm (right):

https://codereview.chromium.org/347653005/diff/60001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.mm:21: @selector(contextWithCGSConnection:
options:)] &&
On 2014/06/20 00:15:11, Avi wrote:
> no space

Done.

[ccameron, 2014-06-20 01:09:31]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-06-20 01:10:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/347653005/80001

[commit-bot: I haz the power, 2014-06-20 07:42:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[ccameron, 2014-06-20 07:51:09]

Adding ben: Need a stamp from a ui/base OWNER

[commit-bot: I haz the power, 2014-06-20 07:51:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-20 07:51:42]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[Ken Russell (switch to Gerrit), 2014-06-20 18:37:40]

LGTM overall. Very nice work. One question.

https://codereview.chromium.org/347653005/diff/80001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.h (right):

https://codereview.chromium.org/347653005/diff/80001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:19: CGSConnectionID CGSMainConnectionID(void);
Does it really work on systems which don't support this API (e.g. 10.8) to call
this API directly? I would think it would be necessary to look up this function
pointer dynamically.

[Avi (use Gerrit), 2014-06-20 18:42:18]

https://codereview.chromium.org/347653005/diff/80001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.h (right):

https://codereview.chromium.org/347653005/diff/80001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:19: CGSConnectionID CGSMainConnectionID(void);
On 2014/06/20 18:37:40, Ken Russell wrote:
> Does it really work on systems which don't support this API (e.g. 10.8) to
call
> this API directly? I would think it would be necessary to look up this
function
> pointer dynamically.

CGSMainConnectionID() has been around forever. You don't need to do a dynamic
lookup.

[ccameron, 2014-06-20 18:44:15]

Thanks!

https://codereview.chromium.org/347653005/diff/80001/ui/base/cocoa/remote_lay...
File ui/base/cocoa/remote_layer_api.h (right):

https://codereview.chromium.org/347653005/diff/80001/ui/base/cocoa/remote_lay...
ui/base/cocoa/remote_layer_api.h:19: CGSConnectionID CGSMainConnectionID(void);
On 2014/06/20 18:37:40, Ken Russell wrote:
> Does it really work on systems which don't support this API (e.g. 10.8) to
call
> this API directly? I would think it would be necessary to look up this
function
> pointer dynamically.

Cause it's not documented, I'm not sure when it was introduced. I found a forum
posting (https://discussions.apple.com/message/8329852) where someone is
complaining in 2008 that something isn't working on 10.3 because it can't find
this symbol, so I suspect it has been around for a long time.

The try jobs didn't have any issues, and I think they're 10.6. Of note is that
the function won't actually get called unless the rest of the API is present
(which is only 10.9+).

[ccameron, 2014-06-20 20:42:11]

Adding sky for ui/OWNERS stamp

[sky, 2014-06-20 23:09:02]

sky->thakis

(thakis is better for mac stuff)

[ccameron, 2014-06-23 20:15:19]

sky, thakis is away, can you do the ui/base review? I've had avi and kbr get
some mac-specific eyes on this.

[sky, 2014-06-23 21:52:07]

I'm going to rubber stamp. Wait for avi to approve though. LGTM

[Avi (use Gerrit), 2014-06-23 22:04:34]

I thought my approval was clear: I LGed earlier and defended kbr's not wrapping
the CGS call in a try.

In any case, this CL is approved by Americans For Faster CALayers.

[ccameron, 2014-06-23 22:15:57]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-06-23 22:18:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/347653005/80001

[commit-bot: I haz the power, 2014-06-23 22:26:59]

Change committed as 279200