GPU: 'Pass' SharedMemory when possible.

GPU:'Pass' SharedMemory when possible.

This avoids some duplication of shared memory handles just
for the purposes of passing them around. Instead we Map
immediately when the memory arrives in the process, and then
'Pass' the shared memory from there.

BUG=177063, 353822
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=259772

[epenner, 2014-03-25 22:14:52]

https://codereview.chromium.org/211703003/diff/1/content/common/gpu/gpu_comma...
File content/common/gpu/gpu_command_buffer_stub.cc (left):

https://codereview.chromium.org/211703003/diff/1/content/common/gpu/gpu_comma...
content/common/gpu/gpu_command_buffer_stub.cc:686: base::SharedMemoryHandle
transfer_buffer,
Since this is just a handle, the app will crash if we simply wrap it in a
SharedMemory and 'Pass' it. Duplicating here replaces the dupe the we used to do
in RegisterXferBuffer (with the exception that we at least don't need to dupe
handles in CreateXferBuffer anymore - we can just Create/Map/'Pass').

If we want to remove this duplication (and the one in mojo), I think I need to
understand these handles better. Given this function just gives us a handle, it
would make sense to me that we don't own it. For example since we duplicated it
before, this handle must be getting destroyed somewhere?

However if we do own it, perhaps we just need to 'Map' the memory here, like I
did in CreateTransferBuffer.

https://codereview.chromium.org/211703003/diff/1/gpu/command_buffer/service/c...
File gpu/command_buffer/service/command_buffer_service.cc (right):

https://codereview.chromium.org/211703003/diff/1/gpu/command_buffer/service/c...
gpu/command_buffer/service/command_buffer_service.cc:127: if
(!shared_memory->CreateAndMapAnonymous(size))
This was causing the crashing tests. I wasn't mapping the memory here.

[epenner, 2014-03-25 22:16:01]

Ptal. I've got some comments on what was missing and some last clarifications.

The only thing now is whether we can also avoid the 'dupe' when handed a raw
handle (AKA, can we just map it and we are good?)

[epenner, 2014-03-25 22:25:37]

https://codereview.chromium.org/211703003/diff/1/content/common/gpu/gpu_comma...
File content/common/gpu/gpu_command_buffer_stub.cc (left):

https://codereview.chromium.org/211703003/diff/1/content/common/gpu/gpu_comma...
content/common/gpu/gpu_command_buffer_stub.cc:686: base::SharedMemoryHandle
transfer_buffer,
Hmm, is there even a possibility we were leaking file-descriptors here? If this
is legit file descriptor, we would need to close it if we don't use it. But I
don't see us doing that anywhere.

[epenner, 2014-03-25 23:02:12]

https://codereview.chromium.org/211703003/diff/1/content/common/gpu/gpu_comma...
File content/common/gpu/gpu_command_buffer_stub.cc (left):

https://codereview.chromium.org/211703003/diff/1/content/common/gpu/gpu_comma...
content/common/gpu/gpu_command_buffer_stub.cc:686: base::SharedMemoryHandle
transfer_buffer,
On 2014/03/25 22:25:37, epenner wrote:
> Hmm, is there even a possibility we were leaking file-descriptors here? If
this
> is legit file descriptor, we would need to close it if we don't use it. But I
> don't see us doing that anywhere.

Actually, forget this I'm stupid :P. We were closing it before by passing the
handle to the SharedMemory, which then closes it in it's destructor. I'll update
this to simply Map/Pass, and we should be good.

[piman, 2014-03-25 23:42:49]

lgtm

[epenner, 2014-03-26 05:57:03]

The CQ bit was checked by epenner@chromium.org

[commit-bot: I haz the power, 2014-03-26 05:57:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/211703003/190001

[commit-bot: I haz the power, 2014-03-26 06:36:49]

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

[commit-bot: I haz the power, 2014-03-26 06:36:50]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[epenner, 2014-03-26 06:37:53]

On 2014/03/26 06:36:50, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

+viettrungluu@ for mojo OWNERS.

[viettrungluu, 2014-03-26 15:43:47]

mojo OWNERS lgtm

[viettrungluu, 2014-03-26 15:44:04]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-03-26 15:44:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/211703003/190001

[commit-bot: I haz the power, 2014-03-26 16:13:54]

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

[commit-bot: I haz the power, 2014-03-26 16:13:54]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[viettrungluu, 2014-03-26 19:14:05]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-03-26 19:14:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/211703003/190001

[commit-bot: I haz the power, 2014-03-26 21:01:53]

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

[commit-bot: I haz the power, 2014-03-26 21:01:54]

Retried try job too often on win_x64_rel for step(s) base_unittests,
chrome_elf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[epenner, 2014-03-26 21:43:05]

The CQ bit was checked by epenner@chromium.org

[commit-bot: I haz the power, 2014-03-26 21:44:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/211703003/300001

[commit-bot: I haz the power, 2014-03-27 02:25:44]

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

[commit-bot: I haz the power, 2014-03-27 02:25:44]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[epenner, 2014-03-27 03:00:43]

The CQ bit was checked by epenner@chromium.org

[commit-bot: I haz the power, 2014-03-27 03:02:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/211703003/300001

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

Change committed as 259772