Refactor params of NaClProcessMsg_Start.

components/nacl/browser/nacl_process_host.cc

Index: components/nacl/browser/nacl_process_host.cc
diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc
index 033502986c932dc6c41873e3eb18163df66670dd..c7e3f43ab16a1a5d422fb79fb4e0b83665fa1943 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
+#include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/path_service.h"
@@ -194,39 +195,91 @@ void SetCloseOnExec(NaClHandle fd) {
 #endif
 }
 
-bool ShareHandleToSelLdr(
-    base::ProcessHandle processh,
-    NaClHandle 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(),
-                       reinterpret_cast<HANDLE>(sourceh),
-                       processh,
-                       &channel,
-                       0,  // Unused given DUPLICATE_SAME_ACCESS.
-                       FALSE,
-                       flags)) {
-    LOG(ERROR) << "DuplicateHandle() failed";
-    return false;
-  }
-  handles_for_sel_ldr->push_back(
-      reinterpret_cast<nacl::FileDescriptor>(channel));
-#else
-  nacl::FileDescriptor channel;
-  channel.fd = sourceh;
-  channel.auto_close = close_source;
-  handles_for_sel_ldr->push_back(channel);
+// The maximum number of resource file handles NaClProcessMsg_Start message
+// can have. Currently IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage
+// is 128 and NaCl sends 5 handles for other purposes, hence 123.
+const size_t kMaxPreOpenResourceFiles = 123;
+
+#if defined(OS_POSIX)
+static_assert(kMaxPreOpenResourceFiles ==
+              IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage - 5,
+              "kMaxPreOpenResourceFiles is not up to date");
 #endif
-  return true;
+
+void BatchOpenResourceFiles(
+    const std::vector<NaClResourcePrefetchInfo>& resource_prefetch_info_list,
+    const base::FilePath& profile_directory,
+    base::ProcessHandle target_process,
+    std::vector<NaClResourceFileInfo>* prefetched_resource_files) {
+  // If output buffer is null, which means there is no resource info to be
+  // opened, do nothing.
+  if (!prefetched_resource_files) {
+    DCHECK(resource_prefetch_info_list.empty());
+    return;
+  }
+
+  NaClBrowserDelegate* browser_delegate = NaClBrowser::GetDelegate();
+  for (size_t i = 0;
+       i < resource_prefetch_info_list.size() &&
+           prefetched_resource_files->size() < kMaxPreOpenResourceFiles;
+       ++i) {
+    base::FilePath filepath;
+    if (!browser_delegate->MapUrlToLocalFilePath(
+            GURL(resource_prefetch_info_list[i].resource_url),
+            true,  // use blocking api.
+            profile_directory,
+            &filepath)) {
+      continue;
+    }
+
+    base::File file = nacl::OpenNaClReadExecImpl(
+        filepath, true /* executable */);
+    if (!file.IsValid())
+      continue;
+
+    // Note: this runs only in Non-SFI mode. So, do not pass the file path;
+    // there's no validation caching in that case, so it's unnecessary, moreover
+    // it would expose the file path to the plugin.
+    prefetched_resource_files->push_back(NaClResourceFileInfo(
+        IPC::TakeFileHandleForProcess(file.Pass(), target_process),
+        base::FilePath(),
+        resource_prefetch_info_list[i].manifest_key));
+  }
 }
 
-void CloseFile(base::File file) {
-  // The base::File destructor will close the file for us.
+base::File ReopenNexeFile(
+    base::File nexe_file, base::FilePath* nexe_file_path) {
+  // If no path is specified, use the original |nexe_file|.
+  if (!nexe_file_path || nexe_file_path->empty())
+    return nexe_file.Pass();
+
+  // Reopen the nexe file.
+  base::File reopened_file = nacl::OpenNaClReadExecImpl(
+      *nexe_file_path, true /* executable */);
+  if (!reopened_file.IsValid()) {
+    // On fail, clear the path, which will eventually passed to the loader.
+    nexe_file_path->clear();
+    return nexe_file.Pass();
+  }
+
+  // Note that, the original |nexe_file| will be closed automatically.
+  return reopened_file.Pass();
+}
+
+// StartNaClExecution needs file operations. This function takes it, and should
+// run on blocking pool.
+base::File ResolveNaClFile(
+    base::FilePath* nexe_file_path,
+    base::File nexe_file,
+    const std::vector<NaClResourcePrefetchInfo>& resource_prefetch_info_list,
+    const base::FilePath& profile_directory,
+    base::ProcessHandle target_process,
+    std::vector<NaClResourceFileInfo>* prefetched_resource_files) {
+  BatchOpenResourceFiles(resource_prefetch_info_list,
+                         profile_directory,
+                         target_process,
+                         prefetched_resource_files);
+  return ReopenNexeFile(nexe_file.Pass(), nexe_file_path);
 }
 
 }  // namespace
@@ -238,8 +291,7 @@ NaClProcessHost::NaClProcessHost(
     const GURL& manifest_url,
     base::File nexe_file,
     const NaClFileToken& nexe_token,
-    const std::vector<
-      nacl::NaClResourceFileInfo>& prefetched_resource_files_info,
+    const std::vector<NaClResourcePrefetchInfo>& resource_prefetch_info_list,
     ppapi::PpapiPermissions permissions,
     int render_view_id,
     uint32 permission_bits,
@@ -250,7 +302,7 @@ NaClProcessHost::NaClProcessHost(
     : manifest_url_(manifest_url),
       nexe_file_(nexe_file.Pass()),
       nexe_token_(nexe_token),
-      prefetched_resource_files_info_(prefetched_resource_files_info),
+      resource_prefetch_info_list_(resource_prefetch_info_list),
       permissions_(permissions),
 #if defined(OS_WIN)
       process_launched_by_broker_(false),
@@ -283,6 +335,12 @@ NaClProcessHost::NaClProcessHost(
 }
 
 NaClProcessHost::~NaClProcessHost() {
+  // Release the file received from the renderer. This has to be done on a
+  // thread where IO is permitted, though.
+  content::BrowserThread::GetBlockingPool()->PostTask(
+      FROM_HERE,
+      base::Bind(&ignore_result<base::File>, base::Passed(nexe_file_.Pass())));
+
   // Report exit status only if the process was successfully started.
   if (process_->GetData().handle != base::kNullProcessHandle) {
     int exit_code = 0;
@@ -298,16 +356,6 @@ NaClProcessHost::~NaClProcessHost() {
     NaClBrowser::GetInstance()->OnProcessEnd(process_->GetData().id);
   }
 
-  for (size_t i = 0; i < prefetched_resource_files_info_.size(); ++i) {
-    // The process failed to launch for some reason. Close resource file
-    // handles.
-    base::File file(IPC::PlatformFileForTransitToFile(
-        prefetched_resource_files_info_[i].file));
-    content::BrowserThread::GetBlockingPool()->PostTask(
-        FROM_HERE,
-        base::Bind(&CloseFile, base::Passed(file.Pass())));
-  }
-
   if (reply_msg_) {
     // The process failed to launch for some reason.
     // Don't keep the renderer hanging.
@@ -824,6 +872,14 @@ bool NaClProcessHost::StartNaClExecution() {
   params.process_type = process_type_;
   bool enable_nacl_debug = enable_debug_stub_ &&
       NaClBrowser::GetDelegate()->URLMatchesDebugPatterns(manifest_url_);
+
+#if defined(OS_MACOSX)
+  base::ScopedFD memory_fd;
+#endif  // defined(OS_MACOSX)
+#if defined(OS_POSIX)
+  base::ScopedFD debug_stub_server_bound_socket;
+#endif  // defined(OS_POSIX)
+
   if (uses_nonsfi_mode_) {
     // Currently, non-SFI mode is supported only on Linux.
 #if defined(OS_LINUX)
@@ -843,22 +899,6 @@ bool NaClProcessHost::StartNaClExecution() {
     params.enable_mojo = base::CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kEnableNaClMojo);
 
-    const ChildProcessData& data = process_->GetData();
-    if (!ShareHandleToSelLdr(data.handle,
-                             socket_for_sel_ldr_.TakePlatformFile(),
-                             true,
-                             &params.handles)) {
-      return false;
-    }
-
-    const base::File& irt_file = nacl_browser->IrtFile();
-    CHECK(irt_file.IsValid());
-    // Send over the IRT file handle.  We don't close our own copy!
-    if (!ShareHandleToSelLdr(data.handle, irt_file.GetPlatformFile(), 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
@@ -872,95 +912,125 @@ bool NaClProcessHost::StartNaClExecution() {
       DLOG(ERROR) << "Failed to allocate memory buffer";
       return false;
     }
-    FileDescriptor memory_fd;
-    memory_fd.fd = dup(memory_buffer.handle().fd);
-    if (memory_fd.fd < 0) {
+    memory_fd.reset(dup(memory_buffer.handle().fd));
+    if (!memory_fd.is_valid()) {
       DLOG(ERROR) << "Failed to dup() a file descriptor";
       return false;
     }
-    memory_fd.auto_close = true;
-    params.handles.push_back(memory_fd);
 #endif
 
 #if defined(OS_POSIX)
     if (params.enable_debug_stub) {
-      net::SocketDescriptor server_bound_socket = GetDebugStubSocketHandle();
-      if (server_bound_socket != net::kInvalidSocket) {
-        params.debug_stub_server_bound_socket =
-            FileDescriptor(server_bound_socket, true);
-      }
+      debug_stub_server_bound_socket.reset(GetDebugStubSocketHandle());
     }
 #endif
   }
 
-  if (!crash_info_shmem_.ShareToProcess(process_->GetData().handle,
-                                        &params.crash_info_shmem_handle)) {
-    DLOG(ERROR) << "Failed to ShareToProcess() a shared memory buffer";
-    return false;
+  // Transfer resources to |params|.
+  // Hereafter we should never return false, and should always send an IPC
+  // to NaCl plugin process. Otherwise, the resources passed to |params|
+  // may be leaked.
+  {
+    const ChildProcessData& data = process_->GetData();
+    if (!uses_nonsfi_mode_) {
+      params.imc_bootstrap_handle = IPC::TakeFileHandleForProcess(
+          socket_for_sel_ldr_.Pass(), data.handle);
+
+      const base::File& irt_file = nacl_browser->IrtFile();
+      CHECK(irt_file.IsValid());
+      // Send over the IRT file handle.  We don't close our own copy!
+      params.irt_handle = IPC::GetFileHandleForProcess(
+          irt_file.GetPlatformFile(), data.handle, false);
+
+#if defined(OS_MACOSX)
+      params.mac_shm_fd = IPC::GetFileHandleForProcess(
+          memory_fd.release(), data.handle, true);
+#endif
+
+#if defined(OS_POSIX)
+      params.debug_stub_server_bound_socket = IPC::GetFileHandleForProcess(
+          debug_stub_server_bound_socket.release(), data.handle, true);
+#endif
+    }
+
+    if (!crash_info_shmem_.ShareToProcess(data.handle,
+                                          &params.crash_info_shmem_handle)) {
+      DLOG(ERROR) << "Failed to ShareToProcess() a shared memory buffer";
+      // Do not return.
+    }
   }
 
-  base::FilePath file_path;
-  if (uses_nonsfi_mode_) {
+  // We have to reopen the file in the browser process; we don't want a
+  // compromised renderer to pass an arbitrary fd that could get loaded
+  // into the plugin process.
+  scoped_ptr<base::FilePath> nexe_file_path;
+  if (!uses_nonsfi_mode_) {
     // Don't retrieve the file path when using nonsfi mode; there's no
     // validation caching in that case, so it's unnecessary work, and would
     // expose the file path to the plugin.
-
-    // Pass the pre-opened resource files to the loader. For the same reason
-    // as above, use an empty base::FilePath.
-    for (size_t i = 0; i < prefetched_resource_files_info_.size(); ++i) {
-      params.prefetched_resource_files.push_back(
-          NaClResourceFileInfo(prefetched_resource_files_info_[i].file,
-                               base::FilePath(),
-                               prefetched_resource_files_info_[i].file_key));
-    }
-    prefetched_resource_files_info_.clear();
-  } else {
-    if (NaClBrowser::GetInstance()->GetFilePath(nexe_token_.lo,
-                                                nexe_token_.hi,
-                                                &file_path)) {
-      // We have to reopen the file in the browser process; we don't want a
-      // compromised renderer to pass an arbitrary fd that could get loaded
-      // into the plugin process.
-      if (base::PostTaskAndReplyWithResult(
-              content::BrowserThread::GetBlockingPool(),
-              FROM_HERE,
-              base::Bind(OpenNaClReadExecImpl,
-                         file_path,
-                         true /* is_executable */),
-              base::Bind(&NaClProcessHost::StartNaClFileResolved,
-                         weak_factory_.GetWeakPtr(),
-                         params,
-                         file_path))) {
-        return true;
-      }
+    nexe_file_path.reset(new base::FilePath);
+    if (!NaClBrowser::GetInstance()->GetFilePath(
+            nexe_token_.lo, nexe_token_.hi, nexe_file_path.get())) {
+      // Failed. Reset the pointer.
+      nexe_file_path.reset();
     }
+  }
+
+  // Pass the pre-opened resource files to the loader.
+  scoped_ptr<std::vector<NaClResourceFileInfo> > resource_info_list;
+  if (!resource_prefetch_info_list_.empty()) {
     // TODO(yusukes): Handle |prefetched_resource_files_info_| for SFI-NaCl.
-    DCHECK(prefetched_resource_files_info_.empty());
+    DCHECK(uses_nonsfi_mode_);
+    resource_info_list.reset(new std::vector<NaClResourceFileInfo>);
   }
 
-  params.nexe_file = IPC::TakeFileHandleForProcess(nexe_file_.Pass(),
-                                                   process_->GetData().handle);
-  process_->Send(new NaClProcessMsg_Start(params));
+  // If file operation is necessary, run it on a blocking pool, where file
+  // operations are allowed.
+  if (nexe_file_path.get() || resource_info_list.get()) {
+    if (base::PostTaskAndReplyWithResult(
+            content::BrowserThread::GetBlockingPool(),
+            FROM_HERE,
+            base::Bind(&ResolveNaClFile,
+                       nexe_file_path.get(),
+                       base::Passed(&nexe_file_),
+                       resource_prefetch_info_list_,
+                       profile_directory_,
+                       process_->GetData().handle,
+                       resource_info_list.get()),
+            base::Bind(&NaClProcessHost::StartNaClExecutionAfterFileResolved,
+                       weak_factory_.GetWeakPtr(),
+                       params,
+                       base::Passed(&nexe_file_path),
+                       base::Passed(&resource_info_list)))) {
+      return true;
+    }
+  }
+
+  StartNaClExecutionAfterFileResolved(
+      params,
+      scoped_ptr<base::FilePath>(),
+      scoped_ptr<std::vector<NaClResourceFileInfo> >(),
+      nexe_file_.Pass());
   return true;
 }
 
-void NaClProcessHost::StartNaClFileResolved(
+void NaClProcessHost::StartNaClExecutionAfterFileResolved(
     NaClStartParams params,
-    const base::FilePath& file_path,
-    base::File checked_nexe_file) {
-  if (checked_nexe_file.IsValid()) {
-    // Release the file received from the renderer. This has to be done on a
-    // thread where IO is permitted, though.
-    content::BrowserThread::GetBlockingPool()->PostTask(
-        FROM_HERE,
-        base::Bind(&CloseFile, base::Passed(nexe_file_.Pass())));
-    params.nexe_file_path_metadata = file_path;
-    params.nexe_file = IPC::TakeFileHandleForProcess(
-        checked_nexe_file.Pass(), process_->GetData().handle);
-  } else {
-    params.nexe_file = IPC::TakeFileHandleForProcess(
-        nexe_file_.Pass(), process_->GetData().handle);
+    scoped_ptr<base::FilePath> nexe_file_path,
+    scoped_ptr<std::vector<NaClResourceFileInfo> > prefetched_resource_files,
+    base::File nexe_file) {
+  // Pass the nexe file and its path to params.
+  params.nexe_file = IPC::TakeFileHandleForProcess(
+      nexe_file.Pass(), process_->GetData().handle);
+  if (nexe_file_path.get()) {
+    params.nexe_file_path_metadata = *nexe_file_path;
+  }
+
+  // Pass prefetched resources to params.
+  if (prefetched_resource_files.get()) {
+    prefetched_resource_files->swap(params.prefetched_resource_files);
   }
+
   process_->Send(new NaClProcessMsg_Start(params));
 }