Add GpuMemoryBuffer CreateFromClientId interface

Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

R=sievers,fsamuel
TBR=khushalsagar
BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/f75160bf37852d01f1f467237435c4d89802b8ed
Cr-Commit-Position: refs/heads/master@{#394958}

[ccameron, 2016-05-12 23:29:41]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
==========

to

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[ccameron, 2016-05-12 23:29:42]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ccameron, 2016-05-12 23:30:54]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

This does not implement the interface yet.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ccameron, 2016-05-12 23:30:54]

ccameron@chromium.org changed reviewers:
+ reveman@chromium.org

[ccameron, 2016-05-12 23:41:38]

reveman@, ptal -- does this interface look good?

https://codereview.chromium.org/1975023002/diff/1/content/browser/gpu/browser...
File content/browser/gpu/browser_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/1975023002/diff/1/content/browser/gpu/browser...
content/browser/gpu/browser_gpu_memory_buffer_manager.cc:579: // TODO(ccameron):
Implement this.
I've left this blank in this patch -- the plan is to have the next patch
implement this.

AFAICT, we'll need to implement this in two ways
Scheme 1: Store the information in BufferInfo that is necessary to open the
buffer
Scheme 2: Send an IPC to the GPU process to create a GpuMemoryBufferHandle for
us to open.

Shared memory will need to do Scheme 1. IOSurfaces can do either -- Scheme 1
will be more efficient (just 1 hop), but will involve a fair bit of #if-MACOSX
code. I suspect other types will need to do Scheme 2.

[reveman, 2016-05-13 12:28:37]

lg with some nits

I think it's a good idea to land the interface change and impl in different
patches but lets have a patch that implements it before we land the interface
change to be sure we're not going down the wrong path with this approach.

https://codereview.chromium.org/1975023002/diff/1/content/browser/gpu/browser...
File content/browser/gpu/browser_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/1975023002/diff/1/content/browser/gpu/browser...
content/browser/gpu/browser_gpu_memory_buffer_manager.cc:579: // TODO(ccameron):
Implement this.
On 2016/05/12 at 23:41:38, ccameron wrote:
> I've left this blank in this patch -- the plan is to have the next patch
implement this.
> 
> AFAICT, we'll need to implement this in two ways
> Scheme 1: Store the information in BufferInfo that is necessary to open the
buffer
> Scheme 2: Send an IPC to the GPU process to create a GpuMemoryBufferHandle for
us to open.
> 
> Shared memory will need to do Scheme 1. IOSurfaces can do either -- Scheme 1
will be more efficient (just 1 hop), but will involve a fair bit of #if-MACOSX
code. I suspect other types will need to do Scheme 2.

We should do Scheme 1 as we need to store something in BufferInfo that allows us
to create the buffer without a GPU trip anyhow. It's a bug that we don't. See
crbug.com/597932. We should be able to do this without much ifdef-code, I think.

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
File gpu/command_buffer/client/gpu_memory_buffer_manager.h (right):

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gpu_memory_buffer_manager.h:36: // Creates a new
GpuMemoryBuffer instance of a child process' buffer.
I don't think we should limit this to child processes. It can be "client" buffer
and the client can potentially be the browser process. The interface is
generally not aware of processes so it's a bit confusing to even mention child
process here.

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gpu_memory_buffer_manager.h:38:
CreateGpuMemoryBufferFromChildId(
CreateGpuMemoryBufferFromClientId or just CreateGpuMemoryBufferFromId

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gpu_memory_buffer_manager.h:39: int child_client_id,
s/child_client_id/client_id/

[ccameron, 2016-05-16 18:41:16]

Thanks!

I've updated this to include the implementation of the interface for IOSurfaces,
since it's not much additional code.

https://codereview.chromium.org/1975023002/diff/1/content/browser/gpu/browser...
File content/browser/gpu/browser_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/1975023002/diff/1/content/browser/gpu/browser...
content/browser/gpu/browser_gpu_memory_buffer_manager.cc:579: // TODO(ccameron):
Implement this.
On 2016/05/13 12:28:37, reveman wrote:
> On 2016/05/12 at 23:41:38, ccameron wrote:
> > I've left this blank in this patch -- the plan is to have the next patch
> implement this.
> > 
> > AFAICT, we'll need to implement this in two ways
> > Scheme 1: Store the information in BufferInfo that is necessary to open the
> buffer
> > Scheme 2: Send an IPC to the GPU process to create a GpuMemoryBufferHandle
for
> us to open.
> > 
> > Shared memory will need to do Scheme 1. IOSurfaces can do either -- Scheme 1
> will be more efficient (just 1 hop), but will involve a fair bit of #if-MACOSX
> code. I suspect other types will need to do Scheme 2.
> 
> We should do Scheme 1 as we need to store something in BufferInfo that allows
us
> to create the buffer without a GPU trip anyhow. It's a bug that we don't. See
> crbug.com/597932. We should be able to do this without much ifdef-code, I
think.

I've added the implementation for IOSurfaces here. It has more ifdef code than
I'd like because we can't just hold on to the handle (because that affects the
refcounting we need from the window server).

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
File gpu/command_buffer/client/gpu_memory_buffer_manager.h (right):

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gpu_memory_buffer_manager.h:36: // Creates a new
GpuMemoryBuffer instance of a child process' buffer.
On 2016/05/13 12:28:37, reveman wrote:
> I don't think we should limit this to child processes. It can be "client"
buffer
> and the client can potentially be the browser process. The interface is
> generally not aware of processes so it's a bit confusing to even mention child
> process here.

Ah, good call. Renamed it to just ClientId.

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gpu_memory_buffer_manager.h:38:
CreateGpuMemoryBufferFromChildId(
On 2016/05/13 12:28:37, reveman wrote:
> CreateGpuMemoryBufferFromClientId or just CreateGpuMemoryBufferFromId

Done.

https://codereview.chromium.org/1975023002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gpu_memory_buffer_manager.h:39: int child_client_id,
On 2016/05/13 12:28:37, reveman wrote:
> s/child_client_id/client_id/

Done.

[ccameron, 2016-05-16 18:42:46]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromChildId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

This does not implement the interface yet.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

Add support for opening IOSurface-backed GpuMemoryBuffers
using this interface.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ccameron, 2016-05-17 22:25:03]

ping on this.

[ccameron, 2016-05-17 22:25:29]

(lmk if I should go to a different reviewer if you're backed up atm).

[ccameron, 2016-05-19 02:17:09]

ccameron@chromium.org changed reviewers:
+ kmarshall@chromium.org, sievers@chromium.org

[ccameron, 2016-05-19 02:17:10]

ptal -- I've left the implemntation as a NOTIMPLEMENTED, like we discussed
earlier.

Adding sievers@ for gpu/ and content/
Adding kmarshall@ for blimp/

[ccameron, 2016-05-19 04:55:19]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

Add support for opening IOSurface-backed GpuMemoryBuffers
using this interface.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ccameron, 2016-05-19 19:47:50]

ccameron@chromium.org changed reviewers:
+ fsamuel@chromium.org

[ccameron, 2016-05-19 19:47:51]

Adding fsamuel for components/

[Fady Samuel, 2016-05-19 19:50:24]

mus lgtm

[Kevin M, 2016-05-19 19:51:14]

kmarshall@chromium.org changed reviewers:
+ khushalsagar@chromium.org

[Kevin M, 2016-05-19 19:51:15]

For blimp/:
R -= me 
R += +khushalsagar@chromium.org , he's more familiar with this than I am.

[no sievers, 2016-05-19 20:07:56]

lgtm

[ccameron, 2016-05-19 21:02:23]

On 2016/05/19 19:51:15, Kevin M wrote:
> For blimp/:
> R -= me 
> R += mailto:+khushalsagar@chromium.org , he's more familiar with this than I
am.

sg -- the components/ part just needs a rubber-stamp.

[ccameron, 2016-05-19 22:07:25]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

R=sievers,fsamuel
TBR=khushalsagar
BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ccameron, 2016-05-19 23:56:17]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2016-05-19 23:56:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1975023002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1975023002/120001

[commit-bot: I haz the power, 2016-05-20 02:16:25]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

R=sievers,fsamuel
TBR=khushalsagar
BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

R=sievers,fsamuel
TBR=khushalsagar
BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-05-20 02:16:27]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-05-20 02:18:08]

Description was changed from

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

R=sievers,fsamuel
TBR=khushalsagar
BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Add GpuMemoryBuffer CreateFromClientId interface

This will be used by the cc::ResourceProvider in the browser process
to get access to IOSurfaces.

R=sievers,fsamuel
TBR=khushalsagar
BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/f75160bf37852d01f1f467237435c4d89802b8ed
Cr-Commit-Position: refs/heads/master@{#394958}
==========

[commit-bot: I haz the power, 2016-05-20 02:18:10]

Patchset 7 (id:??) landed as
https://crrev.com/f75160bf37852d01f1f467237435c4d89802b8ed
Cr-Commit-Position: refs/heads/master@{#394958}