NaCl: Allow setting up multiple sockets for subprocess instead of just one

NaCl: Allow setting up multiple sockets for subprocess instead of just one

Remove the "channel number" parameter from messages, since this is now
fixed in the NaCl plugin code in sel_main_chrome.c.

Replace pair_ and descriptor_ with sockets_for_renderer_ and
sockets_for_sel_ldr_.

NaClProcessMsg_Start: Pass an array of FDs instead of one FD.

ViewHostMsg_LaunchNaCl:
 * Add socket count.
 * Return an array of FDs instead of one FD.

Expose this functionality to the NaCl plugin via a new function,
"launch_nacl_process_multi_fd".

BUG=50626
TEST=nacl_ui_tests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54113

[Mark Seaborn, 2010-07-28 22:52:11]

I've tested this on Linux and 64-bit Windows Vista.  Sending to the trybots now
too...

[Mark Schneckloth, 2010-07-28 23:09:23]

LGTM

http://codereview.chromium.org/2832093/diff/2001/3003
File chrome/common/nacl_messages_internal.h (right):

http://codereview.chromium.org/2832093/diff/2001/3003#newcode14
chrome/common/nacl_messages_internal.h:14: std::vector<nacl::FileDescriptor> /*
sockets */)
I'm not sure how IPC_MESSAGE_CONTROL1 is implemented, but should this be a
std::vector<nacl::FileDescriptor>&

The macro uses MakeRefTuple(arg1) so perhaps the "&" is implied?  I don't know
much about Chrome's IPC.

[Mark Seaborn, 2010-07-29 10:18:00]

On 2010/07/28 23:09:23, Mark Schneckloth wrote:
> LGTM
> 
> http://codereview.chromium.org/2832093/diff/2001/3003
> File chrome/common/nacl_messages_internal.h (right):
> 
> http://codereview.chromium.org/2832093/diff/2001/3003#newcode14
> chrome/common/nacl_messages_internal.h:14: std::vector<nacl::FileDescriptor>
/*
> sockets */)

> I'm not sure how IPC_MESSAGE_CONTROL1 is implemented, but should
> this be a std::vector<nacl::FileDescriptor>&

> The macro uses MakeRefTuple(arg1) so perhaps the "&" is implied?  I
> don't know much about Chrome's IPC.

There are no uses of C++ references in render_messages_internal.h,
even where message arguments are std::vectors, so I suspect references
are not appropriate here.  These types could be put into a
struct/class so using a reference type could break.  It's only a small
amount of data so passing by copy shouldn't hurt.

Cheers,
Mark

[gregoryd, 2010-07-29 23:45:50]

C++ references are not used in render_messages_internal.h that declares the
messages, but they are use in messages handlers: see for example
OnRendererHistograms(), OnDnsPrefetch() and OnCacheableMetadataAvailable() in
src\chrome\browser\renderer_host\resource_message_filter.h

On 2010/07/29 10:18:00, Mark Seaborn wrote:
> On 2010/07/28 23:09:23, Mark Schneckloth wrote:
> > LGTM
> > 
> > http://codereview.chromium.org/2832093/diff/2001/3003
> > File chrome/common/nacl_messages_internal.h (right):
> > 
> > http://codereview.chromium.org/2832093/diff/2001/3003#newcode14
> > chrome/common/nacl_messages_internal.h:14: std::vector<nacl::FileDescriptor>
> /*
> > sockets */)
> 
> > I'm not sure how IPC_MESSAGE_CONTROL1 is implemented, but should
> > this be a std::vector<nacl::FileDescriptor>&
> 
> > The macro uses MakeRefTuple(arg1) so perhaps the "&" is implied?  I
> > don't know much about Chrome's IPC.
> 
> There are no uses of C++ references in render_messages_internal.h,
> even where message arguments are std::vectors, so I suspect references
> are not appropriate here.  These types could be put into a
> struct/class so using a reference type could break.  It's only a small
> amount of data so passing by copy shouldn't hurt.
> 
> Cheers,
> Mark

[gregoryd, 2010-07-29 23:46:09]

One more comment

http://codereview.chromium.org/2832093/diff/2001/3002
File chrome/browser/nacl_host/nacl_process_host.h (right):

http://codereview.chromium.org/2832093/diff/2001/3002#newcode74
chrome/browser/nacl_host/nacl_process_host.h:74: std::vector<nacl::Handle>
sockets_for_sel_ldr_;
socket_for_loader would be better, Chrome people don't know about sel_ldr