Refactor Nacl to remove assumptions about SharedMemoryHandle.

Refactor Nacl to remove assumptions about SharedMemoryHandle.

No intended behavior change.

The existing code in Nacl assumes that SharedMemoryHandle is typedefed to
base::FileDescriptor. This will not be the case once I change
SharedMemoryHandle to support Mach primitives.

BUG=466437
Committed: https://crrev.com/02b844d3bc36f9e6ace93ffd39860d4041a87f59
Cr-Commit-Position: refs/heads/master@{#332970}

[erikchen, 2015-06-03 20:02:05]

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

[erikchen, 2015-06-03 20:02:06]

mseaborn: Please review.

[Mark Seaborn, 2015-06-04 20:17:55]

https://codereview.chromium.org/1143323010/diff/1/components/nacl/browser/nac...
File components/nacl/browser/nacl_process_host.cc (right):

https://codereview.chromium.org/1143323010/diff/1/components/nacl/browser/nac...
components/nacl/browser/nacl_process_host.cc:952:
base::SharedMemory::GetFdFromSharedMemoryHandle(duped_handle));
How will this help you switch to having SharedMemoryHandle be a Mach port on Mac
OS X?  This code is still assuming that you can convert a SharedMemoryHandle to
an FD.  Will it be possible to convert a Mach port to an FD?

Have you got follow-on changes prepared which do that?

[erikchen, 2015-06-04 20:25:42]

https://codereview.chromium.org/1143323010/diff/1/components/nacl/browser/nac...
File components/nacl/browser/nacl_process_host.cc (right):

https://codereview.chromium.org/1143323010/diff/1/components/nacl/browser/nac...
components/nacl/browser/nacl_process_host.cc:952:
base::SharedMemory::GetFdFromSharedMemoryHandle(duped_handle));
On 2015/06/04 20:17:55, Mark Seaborn wrote:
> How will this help you switch to having SharedMemoryHandle be a Mach port on
Mac
> OS X?  This code is still assuming that you can convert a SharedMemoryHandle
to
> an FD.  Will it be possible to convert a Mach port to an FD?
> 
> Have you got follow-on changes prepared which do that?

I have a CL that changes SharedMemoryHandle to be a class on OSX, which is a
necessary first step to making SharedMemoryHandle backed by Mach primitives. 

https://codereview.chromium.org/1163943004/

Before I can make SharedMemoryHandle a class, I need to update all existing
references to SharedMemoryHandle that assume that SharedMemoryHandle is
typedefed to FileDescriptor. This is one of those locations.

If your curious, my next high level steps are:
1) Make SharedMemoryHandle a class that wraps POSIX fd.
2) Make SharedMemoryHandle support both POSIX fd and Mach primitives.
3) Migrate consumers of base::SharedMemory to use Mach primitives.

[Mark Seaborn, 2015-06-04 20:45:01]

https://codereview.chromium.org/1143323010/diff/1/components/nacl/browser/nac...
File components/nacl/browser/nacl_process_host.cc (right):

https://codereview.chromium.org/1143323010/diff/1/components/nacl/browser/nac...
components/nacl/browser/nacl_process_host.cc:952:
base::SharedMemory::GetFdFromSharedMemoryHandle(duped_handle));
On 2015/06/04 20:25:42, erikchen wrote:
> On 2015/06/04 20:17:55, Mark Seaborn wrote:
> > How will this help you switch to having SharedMemoryHandle be a Mach port on
> Mac
> > OS X?  This code is still assuming that you can convert a SharedMemoryHandle
> to
> > an FD.  Will it be possible to convert a Mach port to an FD?
> > 
> > Have you got follow-on changes prepared which do that?
> 
> I have a CL that changes SharedMemoryHandle to be a class on OSX, which is a
> necessary first step to making SharedMemoryHandle backed by Mach primitives. 
> 
> https://codereview.chromium.org/1163943004/
> 
> Before I can make SharedMemoryHandle a class, I need to update all existing
> references to SharedMemoryHandle that assume that SharedMemoryHandle is
> typedefed to FileDescriptor. This is one of those locations.
> 
> If your curious, my next high level steps are:
> 1) Make SharedMemoryHandle a class that wraps POSIX fd.
> 2) Make SharedMemoryHandle support both POSIX fd and Mach primitives.
> 3) Migrate consumers of base::SharedMemory to use Mach primitives.

Which cases will require you to support storing an FD in SharedMemoryHandle on
Mac?  Do you intend to support that indefinitely?  Is it only NaCl that requires
this, or are there other cases?

Presumably you would change base::SharedMemory::Create() so that the caller can
specify whether to create an FD or a Mach port?

[Mark Seaborn, 2015-06-04 23:10:14]

LGTM (after discussion in person)

[erikchen, 2015-06-04 23:10:59]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-06-04 23:13:42]

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

[commit-bot: I haz the power, 2015-06-05 00:02:54]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-06-05 00:04:52]

Patchset 1 (id:??) landed as
https://crrev.com/02b844d3bc36f9e6ace93ffd39860d4041a87f59
Cr-Commit-Position: refs/heads/master@{#332970}