Non-SFI NaCl: Batch-open resource files

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 eabe29e44d85b5d7219a79d2b31977c5c2e94c67..c93660cb18b06b0438486eef7e8a4e73bf839fd8 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -235,19 +235,26 @@ void CloseFile(base::File file) {
 unsigned NaClProcessHost::keepalive_throttle_interval_milliseconds_ =
     ppapi::kKeepaliveThrottleIntervalDefaultMilliseconds;
 
-NaClProcessHost::NaClProcessHost(const GURL& manifest_url,
-                                 base::File nexe_file,
-                                 const NaClFileToken& nexe_token,
-                                 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)
+NaClProcessHost::NaClProcessHost(
+    const GURL& manifest_url,
+    base::File nexe_file,
+    const NaClFileToken& nexe_token,
+    scoped_ptr<base::File[]> resource_files,

[teravest, 2014/10/24 16:18:06]

Maybe resource_files/resource_file_tokens/resource_keys could all go into a
struct or class so they're kept together?

Then that class can enforce the lengths are the same, and NaClProcessHost
doesn't have to have 3 more parameters in its constructor.

Why is resource_file_tokens getting passed in here anyway? I don't see it
getting used...

[Yusuke Sato, 2014/11/04 22:50:21]

Done.
I'll use this in the next CL for SFI-NaCl (and I don't want to tweak the IPC
definitions every time...) There is a TODO(yusukes) at line 898.

+    const std::vector<std::pair<uint64_t, uint64_t> >& resource_file_tokens,
+    const std::vector<std::string>& resource_keys,
+    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),
+      resource_files_(resource_files.Pass()),
+      resource_file_tokens_(resource_file_tokens),
+      resource_keys_(resource_keys),
       permissions_(permissions),
 #if defined(OS_WIN)
       process_launched_by_broker_(false),
@@ -264,6 +271,7 @@ NaClProcessHost::NaClProcessHost(const GURL& manifest_url,
       profile_directory_(profile_directory),
       render_view_id_(render_view_id),
       weak_factory_(this) {
+  CHECK(resource_file_tokens_.size() == resource_keys_.size());
   process_.reset(content::BrowserChildProcessHost::Create(
       PROCESS_TYPE_NACL_LOADER, this));
 
@@ -880,24 +888,34 @@ bool NaClProcessHost::StartNaClExecution() {
     return false;
   }
 
-  base::FilePath file_path;
-  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;
+  if (uses_nonsfi_mode_) {
+    params.resource_keys = resource_keys_;
+    for (size_t i = 0; i < resource_keys_.size(); ++i) {
+      params.resource_files.push_back(IPC::TakeFileHandleForProcess(
+          resource_files_[i].Pass(), process_->GetData().handle));
+    }
+  } else {
+    // TODO(yusukes): 1) Support pre-opening resource files. 2) Validate file
+    // tokens for resource files.
+    base::FilePath file_path;
+    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;
+      }
     }
   }