Pass the nacl start parameters as a struct.

chrome/browser/nacl_host/nacl_process_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/nacl_host/nacl_process_host.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 83 matching lines @@
 
 void SetCloseOnExec(nacl::Handle fd) {
 #if defined(OS_POSIX)
   int flags = fcntl(fd, F_GETFD);
   CHECK_NE(flags, -1);
   int rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
   CHECK_EQ(rc, 0);
 #endif
 }
 
-bool SendHandleToSelLdr(
+bool ShareHandleToSelLdr(
     base::ProcessHandle processh,
     nacl::Handle sourceh,
     bool close_source,
     std::vector<nacl::FileDescriptor> *handles_for_sel_ldr) {
 #if defined(OS_WIN)
   HANDLE channel;
   int flags = DUPLICATE_SAME_ACCESS;
   if (close_source)
     flags |= DUPLICATE_CLOSE_SOURCE;
   if (!DuplicateHandle(GetCurrentProcess(),
@@ skipping 758 matching lines @@
 
 // The asynchronous attempt to get the IRT file open has completed.
 void NaClProcessHost::IrtReady() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   if (!nacl_browser->IrtAvailable() || !SendStart()) {
     DLOG(ERROR) << "Cannot launch NaCl process";
     delete this;
   }
 }
 
-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);
 
+  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;
-  base::ProcessHandle nacl_process_handle;
-
   for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
 #if defined(OS_WIN)
     // Copy the handle into the renderer process.
     HANDLE handle_in_renderer;
     if (!DuplicateHandle(base::GetCurrentProcessHandle(),
                          reinterpret_cast<HANDLE>(
                              internal_->sockets_for_renderer[i]),
                          chrome_render_message_filter_->peer_handle(),
                          &handle_in_renderer,
                          0,  // Unused given DUPLICATE_SAME_ACCESS.
                          FALSE,
                          DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
       DLOG(ERROR) << "DuplicateHandle() failed";
       return false;
     }
     handles_for_renderer.push_back(
         reinterpret_cast<nacl::FileDescriptor>(handle_in_renderer));
 #else
     // No need to dup the imc_handle - we don't pass it anywhere else so
     // it cannot be closed.
     nacl::FileDescriptor imc_handle;
     imc_handle.fd = internal_->sockets_for_renderer[i];
     imc_handle.auto_close = true;
     handles_for_renderer.push_back(imc_handle);
 #endif
   }
 
   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(),
                        data.handle,
                        chrome_render_message_filter_->peer_handle(),
                        &nacl_process_handle,
                        PROCESS_DUP_HANDLE,
                        FALSE,
                        0)) {
     DLOG(ERROR) << "DuplicateHandle() failed";
     return false;
   }
 #else
   // We use pid as process handle on Posix
   nacl_process_handle = data.handle;
 #endif
 
   // Get the pid of the NaCl process
   base::ProcessId nacl_process_id = base::GetProcId(data.handle);
 
   ChromeViewHostMsg_LaunchNaCl::WriteReplyParams(
       reply_msg_, handles_for_renderer, nacl_process_handle, nacl_process_id);
   chrome_render_message_filter_->Send(reply_msg_);
   chrome_render_message_filter_ = NULL;
   reply_msg_ = NULL;
   internal_->sockets_for_renderer.clear();
+  return true;
+}
 
-  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_) {
     if (LaunchNaClGdb(base::GetProcId(process_->GetData().handle))) {
       // We will be called with wait_for_nacl_gdb_ = false once debugger is
       // attached to the program.
       return true;
     }
@@ skipping 12 matching lines @@
 }
 
 void NaClProcessHost::OnQueryKnownToValidate(const std::string& signature,
                                              bool* result) {
   *result = NaClBrowser::GetInstance()->QueryKnownToValidate(signature);
 }
 
 void NaClProcessHost::OnSetKnownToValidate(const std::string& signature) {
   NaClBrowser::GetInstance()->SetKnownToValidate(signature);
 }