Non-SFI NaCl: Batch-open resource files

components/nacl/browser/nacl_process_host.h

 // 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.
 
 #ifndef COMPONENTS_NACL_BROWSER_NACL_PROCESS_HOST_H_
 #define COMPONENTS_NACL_BROWSER_NACL_PROCESS_HOST_H_
 
 #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"
 #include "base/memory/ref_counted.h"
 #include "base/memory/shared_memory.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/process/process.h"
 #include "components/nacl/common/nacl_types.h"
 #include "content/public/browser/browser_child_process_host_delegate.h"
@@ skipping 11 matching lines @@
 
 namespace IPC {
 class ChannelProxy;
 }
 
 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
 // when requested by the renderer.
 // After that, most of the communication is directly between NaCl plugin
 // running in the renderer and NaCl processes.
 class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
  public:
   // manifest_url: the URL of the manifest of the Native Client plugin being
   // executed.
   // nexe_file: A file that corresponds to the nexe module to be loaded.
   // nexe_token: A cache validation token for nexe_file.
   // permissions: PPAPI permissions, to control access to private APIs.
   // render_view_id: RenderView routing id, to control access to private APIs.
   // permission_bits: controls which interfaces the NaCl plugin can use.
   // uses_nonsfi_mode: whether the program should be loaded under non-SFI mode.
   // 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;
 
   // Do any minimal work that must be done at browser startup.
   static void EarlyStartup();
 
   // Specifies throttling time in milliseconds for PpapiHostMsg_Keepalive IPCs.
   static void SetPpapiKeepAliveThrottleForTesting(unsigned milliseconds);
 
@@ skipping 95 matching lines @@
   // created.
   void OnPpapiChannelsCreated(
       const IPC::ChannelHandle& browser_channel_handle,
       const IPC::ChannelHandle& ppapi_renderer_channel_handle,
       const IPC::ChannelHandle& trusted_renderer_channel_handle,
       const IPC::ChannelHandle& manifest_service_channel_handle);
 
   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_;
 
 #if defined(OS_WIN)
   // This field becomes true when the broker successfully launched
   // the NaCl loader.
   bool process_launched_by_broker_;
 #endif
   // The NaClHostMessageFilter that requested this NaCl process.  We use
   // this for sending the reply once the process has started.
@@ skipping 41 matching lines @@
   base::File socket_for_sel_ldr_;
 
   base::WeakPtrFactory<NaClProcessHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NaClProcessHost);
 };
 
 }  // namespace nacl
 
 #endif  // COMPONENTS_NACL_BROWSER_NACL_PROCESS_HOST_H_