gpu: Reuse existing code for shared memory allocation.

gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle(),
or GiveToProcess(), which is already taken care of by mojo, since these
shm handles go over a mojo message-pipe.

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

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

[sadrul, 2016-11-11 01:40:59]

Description was changed from

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle().

BUG=643746
==========

to

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle().

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

[sadrul, 2016-11-11 01:41:05]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 01:41:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-11-11 01:42:46]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 01:43:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-11-11 01:52:42]

sadrul@chromium.org changed reviewers:
+ thestig@chromium.org

[sadrul, 2016-11-11 01:52:43]

Hi! Would you mind looking at the //base/memory/ changes to see if that makes
sense? Thank you! (the rest of the changes in //gpu and //services/ui can be
cleaned up farther, but I wanted to get some feedback on the base change first).

[sadrul, 2016-11-11 02:00:30]

sadrul@chromium.org changed reviewers:
+ thakis@chromium.org

[sadrul, 2016-11-11 02:00:30]

+thakis@ for Q about mac

https://codereview.chromium.org/2496783002/diff/20001/base/memory/shared_memo...
File base/memory/shared_memory_mac.cc (right):

https://codereview.chromium.org/2496783002/diff/20001/base/memory/shared_memo...
base/memory/shared_memory_mac.cc:181: NOTREACHED();
Can I do return std::move(shm_);?

[commit-bot: I haz the power, 2016-11-11 03:06:16]

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

[commit-bot: I haz the power, 2016-11-11 03:06:17]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-11-14 16:41:23]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 16:41:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-11-14 16:44:14]

Description was changed from

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle().

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

to

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle(),
or GiveToProcess(), which is already taken care of by mojo, since these
shm handles go over a mojo message-pipe.

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

[sadrul, 2016-11-14 16:45:22]

sadrul@chromium.org changed reviewers:
+ piman@chromium.org, reveman@chromium.org

[sadrul, 2016-11-14 16:45:23]

+reveman@ for gpu changes.

+piman@ for content changes.

[commit-bot: I haz the power, 2016-11-14 17:43:36]

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

[commit-bot: I haz the power, 2016-11-14 17:43:38]

Dry run: This issue passed the CQ dry run.

[piman, 2016-11-14 18:49:02]

Nice, love it.
LGTM

[Nico, 2016-11-14 18:53:52]

https://codereview.chromium.org/2496783002/diff/20001/base/memory/shared_memo...
File base/memory/shared_memory_mac.cc (right):

https://codereview.chromium.org/2496783002/diff/20001/base/memory/shared_memo...
base/memory/shared_memory_mac.cc:181: NOTREACHED();
On 2016/11/11 02:00:30, sadrul wrote:
> Can I do return std::move(shm_);?

erikchen wrote this. Erik, can you answer?

https://codereview.chromium.org/2496783002/diff/40001/base/memory/shared_memo...
File base/memory/shared_memory.h (right):

https://codereview.chromium.org/2496783002/diff/40001/base/memory/shared_memo...
base/memory/shared_memory.h:200: SharedMemoryHandle TakeHandle();
make this WARN_UNUSED_RESULT

[Lei Zhang, 2016-11-14 19:00:29]

I'm going to defer to thakis, who is already looking at this.

[reveman, 2016-11-14 19:01:48]

lgtm

[erikchen, 2016-11-14 19:37:52]

erikchen@chromium.org changed reviewers:
+ erikchen@chromium.org

[erikchen, 2016-11-14 19:37:53]

https://codereview.chromium.org/2496783002/diff/20001/base/memory/shared_memo...
File base/memory/shared_memory_mac.cc (right):

https://codereview.chromium.org/2496783002/diff/20001/base/memory/shared_memo...
base/memory/shared_memory_mac.cc:181: NOTREACHED();
On 2016/11/14 18:53:51, Nico wrote:
> On 2016/11/11 02:00:30, sadrul wrote:
> > Can I do return std::move(shm_);?
> 
> erikchen wrote this. Erik, can you answer?

SharedMemoryHandle doesn't have ownership semantics, so std::move doesn't make
much sense.

Duplicate()/Close() increments and decrements a mach port reference count so you
could theoretically skip it, but Duplicate() also modifies
ownership_passes_to_ipc_. To be more similar to the other platforms, I recommend
just keeping the Duplicate()/Close().

[erikchen, 2016-11-14 19:39:04]

base/memory lgtm

[Nico, 2016-11-14 20:02:36]

lgtm

[sadrul, 2016-11-14 20:28:52]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2016-11-14 20:29:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-15 00:40:35]

Description was changed from

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle(),
or GiveToProcess(), which is already taken care of by mojo, since these
shm handles go over a mojo message-pipe.

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

to

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle(),
or GiveToProcess(), which is already taken care of by mojo, since these
shm handles go over a mojo message-pipe.

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

[commit-bot: I haz the power, 2016-11-15 00:40:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-15 00:45:01]

Description was changed from

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle(),
or GiveToProcess(), which is already taken care of by mojo, since these
shm handles go over a mojo message-pipe.

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

to

==========
gpu: Reuse existing code for shared memory allocation.

Introduce base::SharedMemory::TakeHandle() to take ownership of the handle
to shared memory. This is in order to avoid unnecessary DuplicateHandle(),
or GiveToProcess(), which is already taken care of by mojo, since these
shm handles go over a mojo message-pipe.

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

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

[commit-bot: I haz the power, 2016-11-15 00:45:02]

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