Non-SFI NaCl: Batch-open resource files

components/nacl/browser/nacl_file_host.cc

Index: components/nacl/browser/nacl_file_host.cc
diff --git a/components/nacl/browser/nacl_file_host.cc b/components/nacl/browser/nacl_file_host.cc
index 2410e11bd50099f10f6ef739d3f72c33852273a3..cbdd2234ce663af847064dc0c39fe430161a2ba7 100644
--- a/components/nacl/browser/nacl_file_host.cc
+++ b/components/nacl/browser/nacl_file_host.cc
@@ -14,6 +14,7 @@
 #include "components/nacl/browser/nacl_browser_delegate.h"
 #include "components/nacl/browser/nacl_host_message_filter.h"
 #include "components/nacl/common/nacl_host_messages.h"
+#include "components/nacl/common/nacl_types.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/site_instance.h"
@@ -37,33 +38,46 @@ void NotifyRendererOfError(
   nacl_host_message_filter->Send(reply_msg);
 }
 
-typedef void (*WriteFileInfoReply)(IPC::Message* reply_msg,
-                                   IPC::PlatformFileForTransit file_desc,
-                                   uint64 file_token_lo,
-                                   uint64 file_token_hi);
+

[Mark Seaborn, 2015/02/09 04:48:34]

Nit: remove empty line (only 1 between top-level decls)

[Yusuke Sato, 2015/02/11 05:54:20]

Done.

+typedef void (*WriteFileInfoReply)(
+    IPC::Message* reply_msg,
+    const std::vector<nacl::NaClFileInfo>& files_info);
 
 void DoRegisterOpenedNaClExecutableFile(
     scoped_refptr<nacl::NaClHostMessageFilter> nacl_host_message_filter,
-    base::File file,
-    base::FilePath file_path,
+    scoped_ptr<base::File[]> resource_files,
+    const std::vector<base::FilePath>& resource_file_paths,
     IPC::Message* reply_msg,
     WriteFileInfoReply write_reply_message) {
   // IO thread owns the NaClBrowser singleton.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   nacl::NaClBrowser* nacl_browser = nacl::NaClBrowser::GetInstance();
-  uint64 file_token_lo = 0;
-  uint64 file_token_hi = 0;
-  nacl_browser->PutFilePath(file_path, &file_token_lo, &file_token_hi);
-
-  IPC::PlatformFileForTransit file_desc = IPC::TakeFileHandleForProcess(
-      file.Pass(),
-      nacl_host_message_filter->PeerHandle());
+  std::vector<nacl::NaClFileInfo> reply_files_info;
+  for (size_t i = 0; i < resource_file_paths.size(); ++i) {
+    uint64 resource_file_token_lo = 0;
+    uint64 resource_file_token_hi = 0;
+    nacl_browser->PutFilePath(resource_file_paths[i],
+                              &resource_file_token_lo,
+                              &resource_file_token_hi);
+    IPC::PlatformFileForTransit resource_file = IPC::TakeFileHandleForProcess(
+        resource_files[i].Pass(), nacl_host_message_filter->PeerHandle());
+    nacl::NaClFileInfo resource_file_info(
+        resource_file, resource_file_token_lo, resource_file_token_hi);
+    reply_files_info.push_back(resource_file_info);
+  }
 
-  write_reply_message(reply_msg, file_desc, file_token_lo, file_token_hi);
+  write_reply_message(reply_msg, reply_files_info);
   nacl_host_message_filter->Send(reply_msg);
 }
 
+void WriteReplyParamsWrapper(
+    IPC::Message* reply_msg,
+    const std::vector<nacl::NaClFileInfo>& files_info) {
+  DCHECK(!files_info.empty());
+  NaClHostMsg_GetReadonlyPnaclFD::WriteReplyParams(reply_msg, files_info[0]);
+}
+
 void DoOpenPnaclFile(
     scoped_refptr<nacl::NaClHostMessageFilter> nacl_host_message_filter,
     const std::string& filename,
@@ -98,75 +112,93 @@ void DoOpenPnaclFile(
   // Not all PNaCl files are executable. Only register those that are
   // executable in the NaCl file_path cache.
   if (is_executable) {
+    scoped_ptr<base::File[]> resource_files(new base::File[1]);
+    resource_files[0] = file_to_open.Pass();
+    std::vector<base::FilePath> resource_file_paths(1);

[Mark Seaborn, 2015/02/09 04:48:34]

Are you sure that base::Bind() makes a copy of this argument?  Can you point me
to the code that does that, or at comments?

[Yusuke Sato, 2015/02/11 05:54:20]

Yes, base/callback.h has some comments:
https://code.google.com/p/chromium/codesearch#chromium/src/base/callback.h&q=...

+    resource_file_paths[0] = full_filepath;
+
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&DoRegisterOpenedNaClExecutableFile,
                    nacl_host_message_filter,
-                   Passed(file_to_open.Pass()), full_filepath, reply_msg,
-                   static_cast<WriteFileInfoReply>(
-                       NaClHostMsg_GetReadonlyPnaclFD::WriteReplyParams)));
+                   Passed(resource_files.Pass()),
+                   resource_file_paths,
+                   reply_msg,
+                   static_cast<WriteFileInfoReply>(WriteReplyParamsWrapper)));
   } else {
     IPC::PlatformFileForTransit target_desc =
         IPC::TakeFileHandleForProcess(file_to_open.Pass(),
                                       nacl_host_message_filter->PeerHandle());
     uint64_t dummy_file_token = 0;
     NaClHostMsg_GetReadonlyPnaclFD::WriteReplyParams(
-        reply_msg, target_desc, dummy_file_token, dummy_file_token);
+        reply_msg,
+        nacl::NaClFileInfo(target_desc, dummy_file_token, dummy_file_token));
     nacl_host_message_filter->Send(reply_msg);
   }
 }
 
-// Convert the file URL into a file descriptor.
+// Convert the resource URLs into file descriptors.
 // This function is security sensitive.  Be sure to check with a security
 // person before you modify it.
-void DoOpenNaClExecutableOnThreadPool(
+void DoOpenNaClResourcesOnThreadPool(
     scoped_refptr<nacl::NaClHostMessageFilter> nacl_host_message_filter,
-    const GURL& file_url,
+    const std::vector<GURL>& resource_urls,
     bool enable_validation_caching,
     IPC::Message* reply_msg) {
   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
 
-  base::FilePath file_path;
-  if (!nacl::NaClBrowser::GetDelegate()->MapUrlToLocalFilePath(
-          file_url,
-          true /* use_blocking_api */,
-          nacl_host_message_filter->profile_directory(),
-          &file_path)) {
-    NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);
-    return;
+  // Convert |resource_urls| into Files and FilePaths.
+  scoped_ptr<base::File[]> resource_files(new base::File[resource_urls.size()]);
+  std::vector<base::FilePath> resource_file_paths(resource_urls.size());
+
+  for (size_t i = 0; i < resource_urls.size(); ++i) {
+    if (!nacl::NaClBrowser::GetDelegate()->MapUrlToLocalFilePath(
+            resource_urls[i],
+            true /* use_blocking_api */,
+            nacl_host_message_filter->profile_directory(),
+            &resource_file_paths[i])) {
+      NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);
+      return;
+    }
+    resource_files[i] = nacl::OpenNaClReadExecImpl(resource_file_paths[i],
+                                                   true /* is_executable */);
+    if (!resource_files[i].IsValid()) {
+      NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);
+      return;
+    }
   }
 
-  base::File file = nacl::OpenNaClReadExecImpl(file_path,
-                                               true /* is_executable */);
-  if (file.IsValid()) {
-    // Opening a NaCl executable works with or without validation caching.
-    // Validation caching requires that the file descriptor is registered now
-    // for later use, which will save time.
-    // When validation caching isn't used (e.g. Non-SFI mode), there is no
-    // reason to do that unnecessary registration.
-    if (enable_validation_caching) {
-      // This function is running on the blocking pool, but the path needs to be
-      // registered in a structure owned by the IO thread.
-      BrowserThread::PostTask(
-          BrowserThread::IO, FROM_HERE,
-          base::Bind(
-              &DoRegisterOpenedNaClExecutableFile,
-              nacl_host_message_filter,
-              Passed(file.Pass()), file_path, reply_msg,
-              static_cast<WriteFileInfoReply>(
-                  NaClHostMsg_OpenNaClExecutable::WriteReplyParams)));
-    } else {
+  // Opening NaCl executables works with or without validation caching.
+  // Validation caching requires that the file descriptor is registered now
+  // for later use, which will save time.
+  // When validation caching isn't used (e.g. Non-SFI mode), there is no
+  // reason to do that unnecessary registration.
+  if (enable_validation_caching) {
+    // This function is running on the blocking pool, but the path needs to be
+    // registered in a structure owned by the IO thread.
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(
+            &DoRegisterOpenedNaClExecutableFile,
+            nacl_host_message_filter,
+            Passed(resource_files.Pass()),
+            resource_file_paths,
+            reply_msg, static_cast<WriteFileInfoReply>(
+                NaClHostMsg_OpenNaClResources::WriteReplyParams)));
+  } else {
+    uint64_t dummy_file_token = 0;
+    std::vector<nacl::NaClFileInfo> reply_files_info;
+    for (size_t i = 0; i < resource_urls.size(); ++i) {
       IPC::PlatformFileForTransit file_desc =
-          IPC::TakeFileHandleForProcess(file.Pass(),
+          IPC::TakeFileHandleForProcess(resource_files[i].Pass(),
                                         nacl_host_message_filter->PeerHandle());
-      uint64_t dummy_file_token = 0;
-      NaClHostMsg_OpenNaClExecutable::WriteReplyParams(
-          reply_msg, file_desc, dummy_file_token, dummy_file_token);
-      nacl_host_message_filter->Send(reply_msg);
+      reply_files_info.push_back(nacl::NaClFileInfo(
+          file_desc, dummy_file_token, dummy_file_token));
     }
-  } else {
-    NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);
-    return;
+    NaClHostMsg_OpenNaClResources::WriteReplyParams(reply_msg,
+                                                    reply_files_info);
+    nacl_host_message_filter->Send(reply_msg);
   }
 }
 
@@ -223,20 +255,20 @@ bool PnaclCanOpenFile(const std::string& filename,
   return true;
 }
 
-void OpenNaClExecutable(
+void OpenNaClResources(
     scoped_refptr<nacl::NaClHostMessageFilter> nacl_host_message_filter,
     int render_view_id,
-    const GURL& file_url,
+    const std::vector<GURL>& resource_urls,
     bool enable_validation_caching,
     IPC::Message* reply_msg) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(
-            &OpenNaClExecutable,
+            &OpenNaClResources,
             nacl_host_message_filter,
             render_view_id,
-            file_url,
+            resource_urls,
             enable_validation_caching,
             reply_msg));
     return;
@@ -252,11 +284,14 @@ void OpenNaClExecutable(
     return;
   }
   content::SiteInstance* site_instance = rvh->GetSiteInstance();
-  if (!content::SiteInstance::IsSameWebSite(site_instance->GetBrowserContext(),
-                                            site_instance->GetSiteURL(),
-                                            file_url)) {
-    NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);
-    return;
+  for (size_t i = 0; i < resource_urls.size(); ++i) {
+    if (!content::SiteInstance::IsSameWebSite(
+            site_instance->GetBrowserContext(),
+            site_instance->GetSiteURL(),
+            resource_urls[i])) {
+      NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);
+      return;
+    }
   }
 
   // The URL is part of the current app. Now query the extension system for the
@@ -265,9 +300,9 @@ void OpenNaClExecutable(
   if (!BrowserThread::PostBlockingPoolTask(
       FROM_HERE,
       base::Bind(
-          &DoOpenNaClExecutableOnThreadPool,
+          &DoOpenNaClResourcesOnThreadPool,
           nacl_host_message_filter,
-          file_url,
+          resource_urls,
           enable_validation_caching,
           reply_msg))) {
     NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);