Pass the nacl start parameters as a struct.

chrome/browser/nacl_host/nacl_process_host.cc

Index: chrome/browser/nacl_host/nacl_process_host.cc
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 1d79727a415bfcb1dfe98c710e4bc40930842f32..76b1dfd6dc8e11a99524caa96383c6039a51e159 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -101,7 +101,7 @@ void SetCloseOnExec(nacl::Handle fd) {
 #endif
 }
 
-bool SendHandleToSelLdr(
+bool ShareHandleToSelLdr(
     base::ProcessHandle processh,
     nacl::Handle sourceh,
     bool close_source,
@@ -880,14 +880,71 @@ void NaClProcessHost::IrtReady() {
   }
 }
 
-bool NaClProcessHost::SendStart() {
+bool NaClProcessHost::StartNaClExecution() {

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

The ordering of the file would be more logical if this came after
ReplyToRenderer(), and the diff would be smaller too.

   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
+
+  nacl::NaClStartParams params;
+  params.validation_cache_key = nacl_browser->GetValidatorCacheKey();
+  params.version = chrome::VersionInfo().CreateVersionString();
+  params.enable_exception_handling = enable_exception_handling_;
+
   base::PlatformFile irt_file = nacl_browser->IrtFile();
   CHECK_NE(irt_file, base::kInvalidPlatformFileValue);
 
-  std::vector<nacl::FileDescriptor> handles_for_renderer;
-  base::ProcessHandle nacl_process_handle;
+  const ChildProcessData& data = process_->GetData();
+  for (size_t i = 0; i < internal_->sockets_for_sel_ldr.size(); i++) {
+    if (!ShareHandleToSelLdr(data.handle,
+                             internal_->sockets_for_sel_ldr[i], true,
+                             &params.handles)) {
+      return false;
+    }
+  }
+
+  // Send over the IRT file handle.  We don't close our own copy!
+  if (!ShareHandleToSelLdr(data.handle, irt_file, false, &params.handles))
+    return false;
+
+#if defined(OS_MACOSX)
+  // For dynamic loading support, NaCl requires a file descriptor that
+  // was created in /tmp, since those created with shm_open() are not
+  // mappable with PROT_EXEC.  Rather than requiring an extra IPC
+  // round trip out of the sandbox, we create an FD here.
+  base::SharedMemory memory_buffer;
+  base::SharedMemoryCreateOptions options;
+  options.size = 1;
+  options.executable = true;
+  if (!memory_buffer.Create(options)) {
+    DLOG(ERROR) << "Failed to allocate memory buffer";
+    return false;
+  }
+  nacl::FileDescriptor memory_fd;
+  memory_fd.fd = dup(memory_buffer.handle().fd);
+  if (memory_fd.fd < 0) {
+    DLOG(ERROR) << "Failed to dup() a file descriptor";
+    return false;
+  }
+  memory_fd.auto_close = true;
+  params.handles.push_back(memory_fd);
+#endif
+
+  IPC::Message* start_message = new NaClProcessMsg_Start(params);
+#if defined(OS_WIN)
+  if (debug_context_ != NULL) {
+    debug_context_->SetStartMessage(start_message);
+    debug_context_->SendStartMessage();
+  } else {
+    process_->Send(start_message);
+  }
+#else
+  process_->Send(start_message);
+#endif
+
+  internal_->sockets_for_sel_ldr.clear();
+  return true;
+}
 
+bool NaClProcessHost::ReplyToRenderer() {
+  std::vector<nacl::FileDescriptor> handles_for_renderer;
   for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
 #if defined(OS_WIN)
     // Copy the handle into the renderer process.
@@ -916,6 +973,7 @@ bool NaClProcessHost::SendStart() {
   }
 
   const ChildProcessData& data = process_->GetData();
+  base::ProcessHandle nacl_process_handle;
 #if defined(OS_WIN)
   // Copy the process handle into the renderer process.
   if (!DuplicateHandle(base::GetCurrentProcessHandle(),
@@ -942,66 +1000,13 @@ bool NaClProcessHost::SendStart() {
   chrome_render_message_filter_ = NULL;
   reply_msg_ = NULL;
   internal_->sockets_for_renderer.clear();
-
-  std::vector<nacl::FileDescriptor> handles_for_sel_ldr;
-  for (size_t i = 0; i < internal_->sockets_for_sel_ldr.size(); i++) {
-    if (!SendHandleToSelLdr(data.handle,
-                            internal_->sockets_for_sel_ldr[i], true,
-                            &handles_for_sel_ldr)) {
-      return false;
-    }
-  }
-
-  // Send over the IRT file handle.  We don't close our own copy!
-  if (!SendHandleToSelLdr(data.handle, irt_file, false, &handles_for_sel_ldr))
-    return false;
-
-#if defined(OS_MACOSX)
-  // For dynamic loading support, NaCl requires a file descriptor that
-  // was created in /tmp, since those created with shm_open() are not
-  // mappable with PROT_EXEC.  Rather than requiring an extra IPC
-  // round trip out of the sandbox, we create an FD here.
-  base::SharedMemory memory_buffer;
-  base::SharedMemoryCreateOptions options;
-  options.size = 1;
-  options.executable = true;
-  if (!memory_buffer.Create(options)) {
-    DLOG(ERROR) << "Failed to allocate memory buffer";
-    return false;
-  }
-  nacl::FileDescriptor memory_fd;
-  memory_fd.fd = dup(memory_buffer.handle().fd);
-  if (memory_fd.fd < 0) {
-    DLOG(ERROR) << "Failed to dup() a file descriptor";
-    return false;
-  }
-  memory_fd.auto_close = true;
-  handles_for_sel_ldr.push_back(memory_fd);
-#endif
-
-  // Sending the version string over IPC avoids linkage issues in cases where
-  // NaCl is not compiled into the main Chromium executable or DLL.
-  chrome::VersionInfo version_info;
-  IPC::Message* start_message =
-      new NaClProcessMsg_Start(handles_for_sel_ldr,
-                               nacl_browser->GetValidatorCacheKey(),
-                               version_info.CreateVersionString(),
-                               enable_exception_handling_);
-#if defined(OS_WIN)
-  if (debug_context_ != NULL) {
-    debug_context_->SetStartMessage(start_message);
-    debug_context_->SendStartMessage();
-  } else {
-    process_->Send(start_message);
-  }
-#else
-  process_->Send(start_message);
-#endif
-
-  internal_->sockets_for_sel_ldr.clear();
   return true;
 }
 
+bool NaClProcessHost::SendStart() {
+  return ReplyToRenderer() && StartNaClExecution();
+}
+
 bool NaClProcessHost::StartWithLaunchedProcess() {
 #if defined(OS_LINUX)
   if (wait_for_nacl_gdb_) {