obsolete: 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 07bee0a8b8a960ecdde44b3e007a0e3b07fb7c1e..60f7bda9a826ff007b54d94c8507755a7317ef37 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -929,47 +929,81 @@ bool NaClProcessHost::StartNaClExecution() {
   } 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
+    std::vector<base::FilePath> file_paths(resource_files_info_len_ + 1);
+    file_paths[0] = file_path;

[teravest, 2014/11/19 15:59:30]

nit: This would be a bit easier to read now if file_path was renamed to
nexe_file_path, I think.

[Yusuke Sato, 2014/11/20 00:30:27]

Done.

+    for (size_t i = 0; i < resource_files_info_len_; ++i) {
+      if (!NaClBrowser::GetInstance()->GetFilePath(
+              resource_files_info_[i].file_token().lo,
+              resource_files_info_[i].file_token().hi,
+              &file_paths[i + 1])) {
+        file_paths.resize(1);  // We still need to open the main nexe file.
+        break;
+      }
+    }
+    // We have to reopen the files in the browser process; we don't want a
+    // compromised renderer to pass arbitrary fds that could get loaded
     // into the plugin process.
     if (base::PostTaskAndReplyWithResult(
            content::BrowserThread::GetBlockingPool(),
            FROM_HERE,
-           base::Bind(OpenNaClReadExecImpl,
-                      file_path,
+           base::Bind(OpenNaClFilesReadExecImpl,
+                      file_paths,
                       true /* is_executable */),
-           base::Bind(&NaClProcessHost::StartNaClFileResolved,
+           base::Bind(&NaClProcessHost::StartNaClFilesResolved,
                       weak_factory_.GetWeakPtr(),
                       params,
-                      file_path))) {
+                      file_paths.size()))) {
       return true;
     }
   }
   params.nexe_file = IPC::TakeFileHandleForProcess(nexe_file_.Pass(),
                                                    process_->GetData().handle);
-
   params.CheckNumOfDescriptors();
   process_->Send(new NaClProcessMsg_Start(params));
   return true;
 }
 
-void NaClProcessHost::StartNaClFileResolved(
+void NaClProcessHost::StartNaClFilesResolved(
     NaClStartParams params,
-    const base::FilePath& file_path,
-    base::File checked_nexe_file) {
-  if (checked_nexe_file.IsValid()) {
+    size_t checked_nexe_files_len,
+    scoped_ptr<OpenNaClFileReadExecImplResult[]> checked_nexe_files) {
+  const base::TimeDelta close_delay = base::TimeDelta::FromSeconds(5);
+
+  // The first element of the array is for the main nexe.
+  DCHECK(checked_nexe_files_len > 0);
+  if (checked_nexe_files[0].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(
+    content::BrowserThread::GetBlockingPool()->PostDelayedTask(
         FROM_HERE,
-        base::Bind(&CloseFile, base::Passed(nexe_file_.Pass())));
-    params.nexe_file_path_metadata = file_path;
+        base::Bind(&CloseFile, base::Passed(nexe_file_.Pass())),
+        close_delay);

[teravest, 2014/11/19 15:59:30]

Why is there a delay added here?

[Yusuke Sato, 2014/11/20 00:30:27]

Removed.

I thought posting dozens of CloseFile tasks during plugin startup negatively
affects the plugin startup time, but I was wrong. Doing this PostTask seems just
fine (confirmed with chrome://tracing).

+    params.nexe_file_path_metadata = checked_nexe_files[0].file_path();
     params.nexe_file = IPC::TakeFileHandleForProcess(
-        checked_nexe_file.Pass(), process_->GetData().handle);
+        checked_nexe_files[0].file().Pass(), process_->GetData().handle);
   } else {
     params.nexe_file = IPC::TakeFileHandleForProcess(
         nexe_file_.Pass(), process_->GetData().handle);
   }
+
+  // The rest is for resource files.
+  for (size_t i = 1; i < checked_nexe_files_len; ++i) {
+    // Release the resource files received from the renderer too.
+    content::BrowserThread::GetBlockingPool()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&CloseFile,
+                   base::Passed(resource_files_info_[i - 1].file().Pass())),
+        close_delay);
+    if (!checked_nexe_files[i].IsValid())
+      continue;
+    IPC::PlatformFileForTransit file = IPC::TakeFileHandleForProcess(
+        checked_nexe_files[i].file().Pass(), process_->GetData().handle);
+    std::string file_key = resource_files_info_[i - 1].file_key();
+    params.resource_files.push_back(NaClStartParams::ResourceFileInfo(
+        file, checked_nexe_files[i].file_path(), file_key));
+  }
+
+  params.CheckNumOfDescriptors();
   process_->Send(new NaClProcessMsg_Start(params));
 }