Refactor params of NaClProcessMsg_Start.

components/nacl/browser/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 "components/nacl/browser/nacl_process_host.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 176 matching lines @@
 
 void SetCloseOnExec(NaClHandle 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 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);
-#endif
-  return true;
-}
-
 void CloseFile(base::File file) {
   // The base::File destructor will close the file for us.
 }
 
 }  // namespace
 
 unsigned NaClProcessHost::keepalive_throttle_interval_milliseconds_ =
     ppapi::kKeepaliveThrottleIntervalDefaultMilliseconds;
 
 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<NaClResourcePrefetchResult>& prefetched_resource_files,
     ppapi::PpapiPermissions permissions,
     int render_view_id,
     uint32 permission_bits,
     bool uses_nonsfi_mode,
     bool off_the_record,
     NaClAppProcessType process_type,
     const base::FilePath& profile_directory)
     : manifest_url_(manifest_url),
       nexe_file_(nexe_file.Pass()),
       nexe_token_(nexe_token),
-      prefetched_resource_files_info_(prefetched_resource_files_info),
+      prefetched_resource_files_(prefetched_resource_files),
       permissions_(permissions),
 #if defined(OS_WIN)
       process_launched_by_broker_(false),
 #endif
       reply_msg_(NULL),
 #if defined(OS_WIN)
       debug_exception_handler_requested_(false),
 #endif
       uses_nonsfi_mode_(uses_nonsfi_mode),
       enable_debug_stub_(false),
@@ skipping 27 matching lines @@
         base::StringPrintf("NaCl process exited with status %i (0x%x)",
                            exit_code, exit_code);
     if (exit_code == 0) {
       VLOG(1) << message;
     } else {
       LOG(ERROR) << message;
     }
     NaClBrowser::GetInstance()->OnProcessEnd(process_->GetData().id);
   }
 
-  for (size_t i = 0; i < prefetched_resource_files_info_.size(); ++i) {
+  // Note: this does not work on Windows, though we currently support this

[Mark Seaborn, 2015/04/24 00:53:24]

"this feature" -> "this prefetching feature"

[hidehiko, 2015/04/24 10:05:17]

Done.

+  // feature only on Non-SFI mode, which is supported only on Linux/ChromeOS,
+  // so it should be ok.

[Mark Seaborn, 2015/04/24 00:53:24]

I'm not sure what you mean by "it should be ok".  Are you saying this code path
would not be correct for Windows?

If so, you could do something like:
#ifdef OS_WINDOWS
DCHECK_EQ(prefetched_resource_files_.size(), 0);
#endif

[hidehiko, 2015/04/24 10:05:17]

Done.

+  for (size_t i = 0; i < prefetched_resource_files_.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));
+        prefetched_resource_files_[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.
     reply_msg_->set_reply_error();
     nacl_host_message_filter_->Send(reply_msg_);
@@ skipping 521 matching lines @@
     }
   } else {
     params.validation_cache_enabled = nacl_browser->ValidationCacheIsEnabled();
     params.validation_cache_key = nacl_browser->GetValidationCacheKey();
     params.version = NaClBrowser::GetDelegate()->GetVersionString();
     params.enable_debug_stub = enable_nacl_debug;
     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)) {
+    params.imc_bootstrap_handle =
+        IPC::TakeFileHandleForProcess(socket_for_sel_ldr_.Pass(), data.handle);
+    if (params.imc_bootstrap_handle == IPC::InvalidPlatformFileForTransit()) {
       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)) {
+    params.irt_handle = IPC::GetFileHandleForProcess(
+        irt_file.GetPlatformFile(), data.handle, false);
+    if (params.irt_handle == IPC::InvalidPlatformFileForTransit()) {
       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;
     }
-    FileDescriptor memory_fd;
-    memory_fd.fd = dup(memory_buffer.handle().fd);
-    if (memory_fd.fd < 0) {
+    base::ScopedFD memory_fd(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);
+    params.mac_shm_fd = IPC::GetFileHandleForProcess(
+        memory_fd.release(), data.handle, true);
 #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);
+        params.debug_stub_server_bound_socket = IPC::GetFileHandleForProcess(
+            server_bound_socket, data.handle, true);
       }
     }
 #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;
   }
 
   base::FilePath 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));
+    for (size_t i = 0; i < prefetched_resource_files_.size(); ++i) {
+      params.prefetched_resource_files.push_back(NaClResourcePrefetchResult(
+          prefetched_resource_files_[i].file,
+          base::FilePath(),
+          prefetched_resource_files_[i].file_key));
     }
-    prefetched_resource_files_info_.clear();
+    prefetched_resource_files_.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;
       }
     }
-    // TODO(yusukes): Handle |prefetched_resource_files_info_| for SFI-NaCl.
-    DCHECK(prefetched_resource_files_info_.empty());
+    // TODO(yusukes): Handle |prefetched_resource_files_| for SFI-NaCl.
+    DCHECK(prefetched_resource_files_.empty());
   }
 
   params.nexe_file = IPC::TakeFileHandleForProcess(nexe_file_.Pass(),
                                                    process_->GetData().handle);
   process_->Send(new NaClProcessMsg_Start(params));
   return true;
 }
 
 void NaClProcessHost::StartNaClFileResolved(
     NaClStartParams params,
@@ skipping 259 matching lines @@
         process.Pass(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif
 
 }  // namespace nacl