Pass the nacl start parameters as a struct.

Pass the nacl start parameters as a struct.

This convers the NaCl load message to use a struct since I plan on adding more stuff here that may vary depending on which proxy configuration you're using.

BUG=116317
TEST=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132685

[brettw, 2012-04-06 23:04:42]

Actually, hold off on reviewing this, it's a bit weird.

[Mark Seaborn, 2012-04-06 23:13:50]

Can you set BUG= to the NaCl/Chrome-IPC-PPAPI-proxy issue, please?

LGTM with the changes below.

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/browser/nacl_ho...
File chrome/browser/nacl_host/nacl_process_host.cc (left):

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/browser/nacl_ho...
chrome/browser/nacl_host/nacl_process_host.cc:982: // Sending the version string
over IPC avoids linkage issues in cases where
Can you keep this comment and move it to the point where we fill out 'version'?

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/browser/nacl_ho...
File chrome/browser/nacl_host/nacl_process_host.cc (right):

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/browser/nacl_ho...
chrome/browser/nacl_host/nacl_process_host.cc:983:
params->sockets.push_back(memory_fd);
The name "sockets" isn't accurate because this array also contains the IRT FD (a
file) and memory_fd.  Can you keep it named as "handles"?

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/common/nacl_typ...
File chrome/common/nacl_types.h (right):

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/common/nacl_typ...
chrome/common/nacl_types.h:35: // Parameters sent to the nacl process when we
start it.
'nacl' -> 'NaCl'

https://chromiumcodereview.appspot.com/10020002/diff/1/chrome/common/nacl_typ...
chrome/common/nacl_types.h:40: std::vector<FileDescriptor> sockets;
"sockets" -> "handles" (see other comment)

[Mark Seaborn, 2012-04-06 23:14:54]

On 2012/04/06 23:04:42, brettw wrote:
> Actually, hold off on reviewing this, it's a bit weird.

How's it weird?  The basic principle seemed OK.

I had started reviewing anyway.

Mark

[brettw, 2012-04-06 23:27:53]

New snap up. This addresses your comments.

I moved the "reply to renderer" stuff out into its own function (it was weird
before that MakeStartParams had the side effect of sending stuff to the
renderer). I also renamed SendHandleToSelLdr to ShareHandleToSelLdr since it
doesn't actually send anything.

[Mark Seaborn, 2012-04-07 00:23:34]

LGTM with ordering change

https://chromiumcodereview.appspot.com/10020002/diff/1018/chrome/browser/nacl...
File chrome/browser/nacl_host/nacl_process_host.cc (right):

https://chromiumcodereview.appspot.com/10020002/diff/1018/chrome/browser/nacl...
chrome/browser/nacl_host/nacl_process_host.cc:883: bool
NaClProcessHost::StartNaClExecution() {
The ordering of the file would be more logical if this came after
ReplyToRenderer(), and the diff would be smaller too.

https://chromiumcodereview.appspot.com/10020002/diff/1018/chrome/browser/nacl...
File chrome/browser/nacl_host/nacl_process_host.h (right):

https://chromiumcodereview.appspot.com/10020002/diff/1018/chrome/browser/nacl...
chrome/browser/nacl_host/nacl_process_host.h:95: // Called once all
initialization is complete and the nacl process is
'nacl' -> 'NaCl'