content: Add DeletedGpuMemoryBuffer IPC.

content: Add DeletedGpuMemoryBuffer IPC.

This IPC is used by the renderer to notify the browser when
a GPU memory buffer instance is deleted. This allows the browser
to maintain proper lifetime management of GPU memory buffers for
which ownership can't be passed with IPC.

This also renames the GpuCommandBufferMsg_DestroyGpuMemoryBuffer
IPC to GpuCommandBufferMsg_UnregisterGpuMemoryBuffer to not
be confused with real GPU memory buffer destruction, which
doesn't necessarily happen at when the GLImage is destroyed.

BUG=

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

[reveman, 2014-07-22 01:27:42]

Please take a look.

[piman, 2014-07-22 03:20:46]

LGTM, but what's the use case? There's no non-empty implementation of
GpuMemoryBufferImpl::DeletedByChildProcess.

[reveman, 2014-07-22 13:41:14]

On 2014/07/22 03:20:46, piman (slow to review) wrote:
> LGTM, but what's the use case? There's no non-empty implementation of
> GpuMemoryBufferImpl::DeletedByChildProcess.

Sorry, this probably needs a bit more explaining.

The use cases are buffers that are allocated on the GPU process side. The
implementation is empty right now as we have not yet added any buffer types that
are allocated on the GPU process side or converted SurfaceTexture, IOSurface
types over to using this system.

This is mostly a cleanup that makes the lifetime of buffers more consistent
between types. Here's what's inconsistent prior to this patch:

(ignore the fact that our current GLES2 interface doesn't provide an API that
allow you to do this)

1. Buffer allocated by browser and passed to renderer
2. Renderer registers buffer with command buffer 1
(GpuCommandBufferMsg_RegisterGpuMemoryBuffer)
3. Renderer registers buffer with command buffer 2
(GpuCommandBufferMsg_RegisterGpuMemoryBuffer)
4. Unregister buffer from command buffer 1,
(GpuCommandBufferMsg_UnregisterGpuMemoryBuffer)
5. Use buffer with command buffer 2.
6. Unregister buffer from command buffer 2,
(GpuCommandBufferMsg_UnregisterGpuMemoryBuffer)

Step 3 would fail with GPU process side allocated buffer types but it would work
with buffer types such as shared-memory for which ownership is passed using IPC.
The current framework makes it impossible to implement this for GPU process side
allocated buffer types as UnregisterGpuMemoryBuffer is what the GPU process
currently has to use as an indication that a buffer is no longer going to be
used and can be destroyed. This happens to work OK with current usage but it
adds confusion about the lifetime of buffers and might be a problem in the
future if we expose new API or like to use these buffers in a different way
internally.

[alexst (slow to review), 2014-07-22 17:50:30]

On 2014/07/22 13:41:14, reveman wrote:
> On 2014/07/22 03:20:46, piman (slow to review) wrote:
> > LGTM, but what's the use case? There's no non-empty implementation of
> > GpuMemoryBufferImpl::DeletedByChildProcess.
> 
> Sorry, this probably needs a bit more explaining.
> 
> The use cases are buffers that are allocated on the GPU process side. The
> implementation is empty right now as we have not yet added any buffer types
that
> are allocated on the GPU process side or converted SurfaceTexture, IOSurface
> types over to using this system.
> 
> This is mostly a cleanup that makes the lifetime of buffers more consistent
> between types. Here's what's inconsistent prior to this patch:
> 
> (ignore the fact that our current GLES2 interface doesn't provide an API that
> allow you to do this)
> 
> 1. Buffer allocated by browser and passed to renderer
> 2. Renderer registers buffer with command buffer 1
> (GpuCommandBufferMsg_RegisterGpuMemoryBuffer)
> 3. Renderer registers buffer with command buffer 2
> (GpuCommandBufferMsg_RegisterGpuMemoryBuffer)
> 4. Unregister buffer from command buffer 1,
> (GpuCommandBufferMsg_UnregisterGpuMemoryBuffer)
> 5. Use buffer with command buffer 2.
> 6. Unregister buffer from command buffer 2,
> (GpuCommandBufferMsg_UnregisterGpuMemoryBuffer)
> 
> Step 3 would fail with GPU process side allocated buffer types but it would
work
> with buffer types such as shared-memory for which ownership is passed using
IPC.
> The current framework makes it impossible to implement this for GPU process
side
> allocated buffer types as UnregisterGpuMemoryBuffer is what the GPU process
> currently has to use as an indication that a buffer is no longer going to be
> used and can be destroyed. This happens to work OK with current usage but it
> adds confusion about the lifetime of buffers and might be a problem in the
> future if we expose new API or like to use these buffers in a different way
> internally.

Thank you for the explination, lgtm.

[reveman, 2014-07-22 18:11:41]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-07-22 18:12:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reveman@chromium.org/409723003/20001

[commit-bot: I haz the power, 2014-07-22 21:05:31]

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...)

[commit-bot: I haz the power, 2014-07-22 21:08:48]

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

[commit-bot: I haz the power, 2014-07-22 21:08:50]

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

[reveman, 2014-07-22 22:06:44]

+jschuh for ipc

[jschuh, 2014-07-22 22:45:40]

IPC security lgtm. Security-wise this seems to be a no-op, but take care that
other implementations of DeletedByChildProcess aren't a vector for stale object
re-use or double frees.

[reveman, 2014-07-23 16:09:18]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-07-23 16:09:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reveman@chromium.org/409723003/20001

[commit-bot: I haz the power, 2014-07-23 16:14:14]

Change committed as 284941