Add member ownership_passes_to_ipc_ to SharedMemoryHandle on windows.

Add member ownership_passes_to_ipc_ to SharedMemoryHandle on windows.

Prior to attachment brokering, the semantics for SharedMemoryHandle differed
between Windows and other platforms. On other platforms, if a handle is passed
to the IPC stack, there is a flag that indicates whether ownership also passed.
On Windows, there was no flag, and whether ownership was passed, or leaked
(waiting for another process to close it), was entirely context dependent.
Adding this flag to attachment-brokered SharedMemoryHandles on Windows mean that
they now have the same ownership semantics as SharedMemoryHandles on other
platforms.

BUG=580636
Committed: https://crrev.com/71bc3b2b02bdf75f04e672c5e1e77999a124b536
Cr-Commit-Position: refs/heads/master@{#372783}

[erikchen, 2016-01-29 23:29:24]

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

[erikchen, 2016-01-29 23:29:39]

erikchen@chromium.org changed reviewers:
+ mark@chromium.org, tsepez@chromium.org

[erikchen, 2016-01-29 23:29:40]

mark: Please review base/
tsepez: Please review ipc/

[commit-bot: I haz the power, 2016-01-29 23:34:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643253003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643253003/20001

[Tom Sepez, 2016-01-29 23:40:47]

Tests?

[commit-bot: I haz the power, 2016-01-30 01:18:38]

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

[commit-bot: I haz the power, 2016-01-30 01:18:38]

Dry run: This issue passed the CQ dry run.

[erikchen, 2016-02-01 04:01:22]

Patchset #3 (id:40001) has been deleted

[erikchen, 2016-02-01 04:34:24]

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

[commit-bot: I haz the power, 2016-02-01 04:34:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643253003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643253003/60001

[erikchen, 2016-02-01 04:35:51]

On 2016/01/30 01:18:38, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

The real test will be removing the Dr.Memory suppressions and checking to see
that no more leaks are detected.

I added some checks to a shared memory unit test.

[commit-bot: I haz the power, 2016-02-01 06:41:55]

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

[commit-bot: I haz the power, 2016-02-01 06:41:56]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-02-01 17:13:32]

lgtm

[erikchen, 2016-02-01 21:46:44]

erikchen@chromium.org changed reviewers:
+ thakis@chromium.org
- mark@chromium.org

[erikchen, 2016-02-01 21:46:44]

-mark, who is sick. +thakis

[Nico, 2016-02-01 21:51:07]

hm, lgtm based on the corresponding _mac file having this already, but making
base "know" about IPC feels a bit icky. It's also uncommon:
https://code.google.com/p/chromium/codesearch#search/&q=ipc%20file:src/base&s...

Maybe the shm handle could have a TakeOwnership() function that hands out a
unique_ptr that ipc could call?

[erikchen, 2016-02-01 21:59:03]

On 2016/02/01 21:51:07, Nico wrote:
> hm, lgtm based on the corresponding _mac file having this already, but making
> base "know" about IPC feels a bit icky. It's also uncommon:
>
https://code.google.com/p/chromium/codesearch#search/&q=ipc%20file:src/base&s...
> 
> Maybe the shm handle could have a TakeOwnership() function that hands out a
> unique_ptr that ipc could call?

Note that the _mac and _win version of this member both stem from
base::FileDescriptor::auto_close, which has the exact same semantics, but a much
more confusing name.

The main problem stems from the fact that SharedMemoryHandle wraps an object
(HANDLE, mach_port, etc.) that has implicit ownership semantics, but
SharedMemoryHandle itself does not convey any of these semantics. I would love
to see SharedMemoryHandle become a scoped object (which is now possible), but
I'm not sure if that refactor is a worthwhile time investment.

[erikchen, 2016-02-01 21:59:12]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2016-02-01 22:04:12]

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

[commit-bot: I haz the power, 2016-02-01 22:14:35]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-02-01 22:15:58]

Description was changed from

==========
Add member ownership_passes_to_ipc_ to SharedMemoryHandle on windows.

Prior to attachment brokering, the semantics for SharedMemoryHandle differed
between Windows and other platforms. On other platforms, if a handle is passed
to the IPC stack, there is a flag that indicates whether ownership also passed.
On Windows, there was no flag, and whether ownership was passed, or leaked
(waiting for another process to close it), was entirely context dependent.
Adding this flag to attachment-brokered SharedMemoryHandles on Windows mean that
they now have the same ownership semantics as SharedMemoryHandles on other
platforms.

BUG=580636
==========

to

==========
Add member ownership_passes_to_ipc_ to SharedMemoryHandle on windows.

Prior to attachment brokering, the semantics for SharedMemoryHandle differed
between Windows and other platforms. On other platforms, if a handle is passed
to the IPC stack, there is a flag that indicates whether ownership also passed.
On Windows, there was no flag, and whether ownership was passed, or leaked
(waiting for another process to close it), was entirely context dependent.
Adding this flag to attachment-brokered SharedMemoryHandles on Windows mean that
they now have the same ownership semantics as SharedMemoryHandles on other
platforms.

BUG=580636

Committed: https://crrev.com/71bc3b2b02bdf75f04e672c5e1e77999a124b536
Cr-Commit-Position: refs/heads/master@{#372783}
==========

[commit-bot: I haz the power, 2016-02-01 22:15:58]

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