Allow closing SharedMemory without unmapping it.

Allow closing SharedMemory without unmapping it.

This allows us to reduce the number of fds that are in use if the SharedMemory is mapped once and never shared to another process.

Committed: https://crrev.com/569918beae87b992d951e105f94d15eb5939bdcb
Cr-Commit-Position: refs/heads/master@{#307776}

[jbauman, 2014-12-03 01:07:48]

jbauman@chromium.org changed reviewers:
+ danakj@chromium.org

[jbauman, 2014-12-04 23:44:25]

ping

[danakj, 2014-12-10 20:51:51]

Have you looked at other callsites of close?

Should this be unmapping too?

https://code.google.com/p/chromium/codesearch#chromium/src/components/nacl/br...

Maybe we should have

SharedMemory::CloseAndUnmap and
SharedMemory::CloseWithoutUnmap

to make things more obvious?

[jbauman, 2014-12-10 20:55:16]

On 2014/12/10 20:51:51, danakj wrote:
> Have you looked at other callsites of close?

Yeah, they should all be correct now.
> 
> Should this be unmapping too?
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/nacl/br...

That shared memory 
> 
> Maybe we should have
> 
> SharedMemory::CloseAndUnmap and
> SharedMemory::CloseWithoutUnmap
> 
> to make things more obvious?

Possibly, though doing Close and keeping the SharedMemory around is really rare.

[jbauman, 2014-12-10 20:56:16]

On 2014/12/10 20:55:16, jbauman wrote:
> On 2014/12/10 20:51:51, danakj wrote:
> > Have you looked at other callsites of close?
> 
> Yeah, they should all be correct now.
> > 
> > Should this be unmapping too?
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/nacl/br...
> 
That shared memory is never mapped into the browser process.
> > 
> > Maybe we should have
> > 
> > SharedMemory::CloseAndUnmap and
> > SharedMemory::CloseWithoutUnmap
> > 
> > to make things more obvious?
> 
> Possibly, though doing Close and keeping the SharedMemory around is really
rare.

[danakj, 2014-12-10 20:57:25]

Ok this seems fine then. Thanks, LGTM

[jbauman, 2014-12-10 21:03:16]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2014-12-10 21:03:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/762273002/40001

[commit-bot: I haz the power, 2014-12-10 22:07:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-10 22:08:05]

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