Non-SFI NaCl: Batch-open resource files

components/nacl/browser/nacl_process_host.h

Index: components/nacl/browser/nacl_process_host.h
diff --git a/components/nacl/browser/nacl_process_host.h b/components/nacl/browser/nacl_process_host.h
index e35367564195b2c0e4dbfebaac5e092af8f24faf..d3a87bfcdf610402dd9de3fe2d49cedb16a9b96b 100644
--- a/components/nacl/browser/nacl_process_host.h
+++ b/components/nacl/browser/nacl_process_host.h
@@ -7,6 +7,10 @@
 
 #include "build/build_config.h"
 
+#include <string>
+#include <utility>
+#include <vector>
+
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util_proxy.h"
@@ -38,6 +42,23 @@ namespace nacl {
 class NaClHostMessageFilter;
 void* AllocateAddressSpaceASLR(base::ProcessHandle process, size_t size);
 
+class ResourceFileInfo {

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

It's rather confusing that you have three ResourceFileInfo types -- this class
plus 2 structs in nacl_types.h.

This one appears to contain the same information as
NaClLaunchParams::ResourceFileInfo, which contains:

    NaClFileInfo file_info; // file descriptor + a NaClFileToken
    std::string file_key;

Were there some obstacles to using the same type in both cases?  Is the
difference in whether these types use an auto-freeing FD (i.e. base::File)?

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

I defined these structs to use exactly the same types as the main nexe whenever
possible, but it seems like this was just confusing. I've removed most of them.
New patch set uses nacl::NaClResourceFileInfo in most places.

+  MOVE_ONLY_TYPE_FOR_CPP_03(ResourceFileInfo, RValue);
+ public:
+  ResourceFileInfo();
+  ResourceFileInfo(base::File file,
+                   const NaClFileToken& file_token,
+                   const std::string& file_key);
+  ~ResourceFileInfo();
+
+  ResourceFileInfo(RValue other);
+  ResourceFileInfo& operator=(RValue other);
+
+  base::File file_;

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

Nit: If these are public, they shouldn't have a "_" suffix

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

Removed the class. Done.

+  NaClFileToken file_token_;
+  std::string file_key_;
+};
+
 // Represents the browser side of the browser <--> NaCl communication
 // channel. There will be one NaClProcessHost per NaCl process
 // The browser is responsible for starting the NaCl process
@@ -57,16 +78,19 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   // off_the_record: was the process launched from an incognito renderer?
   // process_type: the type of NaCl process.
   // profile_directory: is the path of current profile directory.
-  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(
+      const GURL& manifest_url,
+      base::File nexe_file,
+      const NaClFileToken& nexe_token,
+      scoped_ptr<ResourceFileInfo[]> resource_files_info,
+      size_t resource_files_info_len,

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

How about making resource_files_info a vector instead of passing the length
separately?

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

It was difficult to do so for the move-only class, but now it's possible as I've
removed the class. Done.

+      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() override;
 
   void OnProcessCrashed(int exit_status) override;
@@ -182,6 +206,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   GURL manifest_url_;
   base::File nexe_file_;
   NaClFileToken nexe_token_;
+  scoped_ptr<ResourceFileInfo[]> resource_files_info_;
+  size_t resource_files_info_len_;
 
   ppapi::PpapiPermissions permissions_;