Pepper: Remove LOAD_MODULE SRPC call in SFI mode.

This change puts nexe information in the parameters for starting a NaCl
plugin instead of sending that information over SRPC. This may remove the
need for that IPC round trip entirely; perhaps we could report load failure
as part of the RPCs for starting sel_ldr or performing StartModule().

nacl_defines.gypi as added as a dependency in components/components_tests.gyp
because nacl_process_host.h now includes
"native_client/src/public/nacl_file_info.h", which requires nacl_defines.

BUG=333950
R=bbudge@chromium.org, brettw@chromium.org, hidehiko@chromium.org, jschuh@chromium.org, mseaborn@chromium.org, sky@chromium.org

Review URL: https://codereview.chromium.org/332463003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285028 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 143 insertions, 129 deletions

diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 6da60fb..42c994b 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -5,6 +5,7 @@
 import("//build/config/crypto.gni")
 import("//build/config/features.gni")
 import("//build/config/ui.gni")
+import("//native_client/build/config/nacl_defines.gni")
 import("//tools/grit/grit_rule.gni")
 
 about_credits_file = "$target_gen_dir/about_credits.html"
@@ -128,8 +129,7 @@ static_library("browser") {
     #        # chrome/browser/metrics/variations/generate_resources_map.py
     #        '<(SHARED_INTERMEDIATE_DIR)/chrome/browser/metrics/variations/generated_resources_map.cc',
 
-    # TODO(GYP) Also add these nacl_defines to direct dependents.
-    #defines = nacl_defines
+    defines = nacl_defines
 
     # TODO(GYP) remove this when the real webrtc target is used below.
     configs += [ "//content:webrtc_stub_config" ]
diff --git a/components/components_tests.gyp b/components/components_tests.gyp
index e655cb8..e11d730 100644
--- a/components/components_tests.gyp
+++ b/components/components_tests.gyp
@@ -497,6 +497,12 @@
               ],
             }],
             ['disable_nacl==0', {
+              'includes': [
+                'nacl/nacl_defines.gypi',
+              ],
+              'defines': [
+                '<@(nacl_defines)',
+              ],
               'sources': [
                 'nacl/browser/nacl_file_host_unittest.cc',
                 'nacl/browser/nacl_process_host_unittest.cc',
diff --git a/components/nacl/browser/DEPS b/components/nacl/browser/DEPS
index e1c944e..1381695 100644
--- a/components/nacl/browser/DEPS
+++ b/components/nacl/browser/DEPS
@@ -1,6 +1,7 @@
 include_rules = [
   "+content/public/browser",
   "+content/public/test",
+  "+native_client/src/public",
   "+native_client/src/shared/imc/nacl_imc_c.h",
   "+net",
   "+ppapi/host",
diff --git a/components/nacl/browser/nacl_host_message_filter.cc b/components/nacl/browser/nacl_host_message_filter.cc
index 0b35afc..d6505ed 100644
--- a/components/nacl/browser/nacl_host_message_filter.cc
+++ b/components/nacl/browser/nacl_host_message_filter.cc
@@ -145,10 +145,40 @@ void NaClHostMessageFilter::LaunchNaClContinuation(
     const nacl::NaClLaunchParams& launch_params,
     IPC::Message* reply_msg,
     ppapi::PpapiPermissions permissions) {
+  NaClFileToken nexe_token = {
+      launch_params.nexe_token_lo,  // lo
+      launch_params.nexe_token_hi   // hi
+  };
+
+  base::PlatformFile nexe_file;
+#if defined(OS_WIN)
+  // Duplicate the nexe file handle from the renderer process into the browser
+  // process.
+  if (!::DuplicateHandle(PeerHandle(),
+                         launch_params.nexe_file,
+                         base::GetCurrentProcessHandle(),
+                         &nexe_file,
+                         0,  // Unused, given DUPLICATE_SAME_ACCESS.
+                         FALSE,
+                         DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
+    NaClHostMsg_LaunchNaCl::WriteReplyParams(
+        reply_msg,
+        NaClLaunchResult(),
+        std::string("Failed to duplicate nexe file handle"));
+    Send(reply_msg);
+    return;
+  }
+#elif defined(OS_POSIX)
+  nexe_file =
+      IPC::PlatformFileForTransitToPlatformFile(launch_params.nexe_file);
+#else
+#error Unsupported platform.
+#endif
+
   NaClProcessHost* host = new NaClProcessHost(
       GURL(launch_params.manifest_url),
-      base::File(
-          IPC::PlatformFileForTransitToPlatformFile(launch_params.nexe_file)),
+      base::File(nexe_file),
+      nexe_token,
       permissions,
       launch_params.render_view_id,
       launch_params.permission_bits,
diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc
index 0a3a0c9..a922954 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -250,6 +250,7 @@ unsigned NaClProcessHost::keepalive_throttle_interval_milliseconds_ =
 
 NaClProcessHost::NaClProcessHost(const GURL& manifest_url,
                                  base::File nexe_file,
+                                 NaClFileToken nexe_token,
                                  ppapi::PpapiPermissions permissions,
                                  int render_view_id,
                                  uint32 permission_bits,
@@ -262,6 +263,7 @@ NaClProcessHost::NaClProcessHost(const GURL& manifest_url,
                                  const base::FilePath& profile_directory)
     : manifest_url_(manifest_url),
       nexe_file_(nexe_file.Pass()),
+      nexe_token_(nexe_token),
       permissions_(permissions),
 #if defined(OS_WIN)
       process_launched_by_broker_(false),
@@ -820,12 +822,6 @@ bool NaClProcessHost::StartNaClExecution() {
   if (uses_nonsfi_mode_) {
     // Currently, non-SFI mode is supported only on Linux.
 #if defined(OS_LINUX)
-    // nexe_file_ still keeps the ownership at this moment, because |params|
-    // may just be destroyed before sending IPC is properly processed.
-    // Note that although we set auto_close=true for FileDescriptor's
-    // constructor, it is not automatically handled in its destructor as RAII.
-    params.nexe_file =
-        base::FileDescriptor(nexe_file_.GetPlatformFile(), true);
     // In non-SFI mode, we do not use SRPC. Make sure that the socketpair is
     // not created.
     DCHECK_EQ(internal_->socket_for_sel_ldr, NACL_INVALID_HANDLE);
@@ -840,6 +836,11 @@ bool NaClProcessHost::StartNaClExecution() {
     params.uses_irt = uses_irt_;
     params.enable_dyncode_syscalls = enable_dyncode_syscalls_;
 
+    // TODO(teravest): Resolve the file tokens right now instead of making the
+    // loader send IPC to resolve them later.
+    params.nexe_token_lo = nexe_token_.lo;
+    params.nexe_token_hi = nexe_token_.hi;
+
     const ChildProcessData& data = process_->GetData();
     if (!ShareHandleToSelLdr(data.handle,
                              internal_->socket_for_sel_ldr, true,
@@ -891,14 +892,13 @@ bool NaClProcessHost::StartNaClExecution() {
 #endif
   }
 
-  // Here we are about to send the IPC, so release file descriptors to delegate
-  // the ownership to the message.
-  if (uses_nonsfi_mode_) {
-    nexe_file_.TakePlatformFile();
-  } else {
+  if (!uses_nonsfi_mode_) {
     internal_->socket_for_sel_ldr = NACL_INVALID_HANDLE;
   }
 
+  params.nexe_file = IPC::TakeFileHandleForProcess(nexe_file_.Pass(),
+                                                   process_->GetData().handle);
+
   process_->Send(new NaClProcessMsg_Start(params));
   return true;
 }
diff --git a/components/nacl/browser/nacl_process_host.h b/components/nacl/browser/nacl_process_host.h
index a5e3e52..53348de 100644
--- a/components/nacl/browser/nacl_process_host.h
+++ b/components/nacl/browser/nacl_process_host.h
@@ -18,6 +18,7 @@
 #include "content/public/browser/browser_child_process_host_delegate.h"
 #include "content/public/browser/browser_child_process_host_iterator.h"
 #include "ipc/ipc_channel_handle.h"
+#include "native_client/src/public/nacl_file_info.h"
 #include "net/socket/socket_descriptor.h"
 #include "ppapi/shared_impl/ppapi_permissions.h"
 #include "url/gurl.h"
@@ -46,6 +47,8 @@ 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.
@@ -63,6 +66,7 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   // profile_directory: is the path of current profile directory.
   NaClProcessHost(const GURL& manifest_url,
                   base::File nexe_file,
+                  NaClFileToken nexe_token,
                   ppapi::PpapiPermissions permissions,
                   int render_view_id,
                   uint32 permission_bits,
@@ -189,6 +193,7 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
 
   GURL manifest_url_;
   base::File nexe_file_;
+  NaClFileToken nexe_token_;
 
   ppapi::PpapiPermissions permissions_;
 
diff --git a/components/nacl/common/nacl_host_messages.h b/components/nacl/common/nacl_host_messages.h
index a998f37..0722d9d 100644
--- a/components/nacl/common/nacl_host_messages.h
+++ b/components/nacl/common/nacl_host_messages.h
@@ -22,6 +22,8 @@
 IPC_STRUCT_TRAITS_BEGIN(nacl::NaClLaunchParams)
   IPC_STRUCT_TRAITS_MEMBER(manifest_url)
   IPC_STRUCT_TRAITS_MEMBER(nexe_file)
+  IPC_STRUCT_TRAITS_MEMBER(nexe_token_lo)
+  IPC_STRUCT_TRAITS_MEMBER(nexe_token_hi)
   IPC_STRUCT_TRAITS_MEMBER(render_view_id)
   IPC_STRUCT_TRAITS_MEMBER(permission_bits)
   IPC_STRUCT_TRAITS_MEMBER(uses_irt)
diff --git a/components/nacl/common/nacl_messages.h b/components/nacl/common/nacl_messages.h
index a3b89e2..6d613b0 100644
--- a/components/nacl/common/nacl_messages.h
+++ b/components/nacl/common/nacl_messages.h
@@ -15,6 +15,8 @@
 
 IPC_STRUCT_TRAITS_BEGIN(nacl::NaClStartParams)
   IPC_STRUCT_TRAITS_MEMBER(nexe_file)
+  IPC_STRUCT_TRAITS_MEMBER(nexe_token_lo)
+  IPC_STRUCT_TRAITS_MEMBER(nexe_token_hi)
   IPC_STRUCT_TRAITS_MEMBER(handles)
   IPC_STRUCT_TRAITS_MEMBER(debug_stub_server_bound_socket)
   IPC_STRUCT_TRAITS_MEMBER(validation_cache_enabled)
diff --git a/components/nacl/common/nacl_types.cc b/components/nacl/common/nacl_types.cc
index 0b096a65..f2c5951 100644
--- a/components/nacl/common/nacl_types.cc
+++ b/components/nacl/common/nacl_types.cc
@@ -9,6 +9,8 @@ namespace nacl {
 
 NaClStartParams::NaClStartParams()
     : nexe_file(IPC::InvalidPlatformFileForTransit()),
+      nexe_token_lo(0),
+      nexe_token_hi(0),
       validation_cache_enabled(false),
       enable_exception_handling(false),
       enable_debug_stub(false),
@@ -22,6 +24,8 @@ NaClStartParams::~NaClStartParams() {
 
 NaClLaunchParams::NaClLaunchParams()
     : nexe_file(IPC::InvalidPlatformFileForTransit()),
+      nexe_token_lo(0),
+      nexe_token_hi(0),
       render_view_id(0),
       permission_bits(0),
       uses_irt(false),
@@ -33,6 +37,8 @@ NaClLaunchParams::NaClLaunchParams()
 NaClLaunchParams::NaClLaunchParams(
     const std::string& manifest_url,
     const IPC::PlatformFileForTransit& nexe_file,
+    uint64_t nexe_token_lo,
+    uint64_t nexe_token_hi,
     int render_view_id,
     uint32 permission_bits,
     bool uses_irt,
@@ -42,6 +48,8 @@ NaClLaunchParams::NaClLaunchParams(
     bool enable_crash_throttling)
     : manifest_url(manifest_url),
       nexe_file(nexe_file),
+      nexe_token_lo(nexe_token_lo),
+      nexe_token_hi(nexe_token_hi),
       render_view_id(render_view_id),
       permission_bits(permission_bits),
       uses_irt(uses_irt),
@@ -51,18 +59,6 @@ NaClLaunchParams::NaClLaunchParams(
       enable_crash_throttling(enable_crash_throttling) {
 }
 
-NaClLaunchParams::NaClLaunchParams(const NaClLaunchParams& l)
-    : manifest_url(l.manifest_url),
-      nexe_file(l.nexe_file),
-      render_view_id(l.render_view_id),
-      permission_bits(l.permission_bits),
-      uses_irt(l.uses_irt),
-      uses_nonsfi_mode(l.uses_nonsfi_mode),
-      enable_dyncode_syscalls(l.enable_dyncode_syscalls),
-      enable_exception_handling(l.enable_exception_handling),
-      enable_crash_throttling(l.enable_crash_throttling) {
-}
-
 NaClLaunchParams::~NaClLaunchParams() {
 }
 
diff --git a/components/nacl/common/nacl_types.h b/components/nacl/common/nacl_types.h
index e2c7793..6aea090 100644
--- a/components/nacl/common/nacl_types.h
+++ b/components/nacl/common/nacl_types.h
@@ -44,6 +44,8 @@ struct NaClStartParams {
   ~NaClStartParams();
 
   IPC::PlatformFileForTransit nexe_file;
+  uint64_t nexe_token_lo;
+  uint64_t nexe_token_hi;
 
   std::vector<FileDescriptor> handles;
   FileDescriptor debug_stub_server_bound_socket;
@@ -60,6 +62,7 @@ struct NaClStartParams {
   bool enable_ipc_proxy;
   bool uses_irt;
   bool enable_dyncode_syscalls;
+
   // NOTE: Any new fields added here must also be added to the IPC
   // serialization in nacl_messages.h and (for POD fields) the constructor
   // in nacl_types.cc.
@@ -73,6 +76,8 @@ struct NaClLaunchParams {
   NaClLaunchParams();
   NaClLaunchParams(const std::string& manifest_url,
                    const IPC::PlatformFileForTransit& nexe_file,
+                   uint64_t nexe_token_lo,
+                   uint64_t nexe_token_hi,
                    int render_view_id,
                    uint32 permission_bits,
                    bool uses_irt,
@@ -80,11 +85,16 @@ struct NaClLaunchParams {
                    bool enable_dyncode_syscalls,
                    bool enable_exception_handling,
                    bool enable_crash_throttling);
-  NaClLaunchParams(const NaClLaunchParams& l);
   ~NaClLaunchParams();
 
   std::string manifest_url;
+  // On Windows, the HANDLE passed here is valid in the renderer's context.
+  // It's the responsibility of the browser to duplicate this handle properly
+  // for passing it to the plugin.
   IPC::PlatformFileForTransit nexe_file;
+  uint64_t nexe_token_lo;
+  uint64_t nexe_token_hi;
+
   int render_view_id;
   uint32 permission_bits;
   bool uses_irt;
diff --git a/components/nacl/loader/nacl_listener.cc b/components/nacl/loader/nacl_listener.cc
index 18588bd..516395d 100644
--- a/components/nacl/loader/nacl_listener.cc
+++ b/components/nacl/loader/nacl_listener.cc
@@ -5,6 +5,7 @@
 #include "components/nacl/loader/nacl_listener.h"
 
 #include <errno.h>
+#include <fcntl.h>
 #include <stdlib.h>
 
 #if defined(OS_POSIX)
@@ -27,6 +28,7 @@
 #include "native_client/src/public/chrome_main.h"
 #include "native_client/src/public/nacl_app.h"
 #include "native_client/src/public/nacl_file_info.h"
+#include "native_client/src/trusted/service_runtime/include/sys/fcntl.h"
 
 #if defined(OS_POSIX)
 #include "base/file_descriptor_posix.h"
@@ -37,7 +39,6 @@
 #include "components/nacl/loader/nonsfi/nonsfi_main.h"
 #include "content/public/common/child_process_sandbox_support_linux.h"
 #include "native_client/src/trusted/desc/nacl_desc_io.h"
-#include "native_client/src/trusted/service_runtime/include/sys/fcntl.h"
 #include "ppapi/nacl_irt/plugin_startup.h"
 #endif
 
@@ -390,6 +391,22 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
   args->prereserved_sandbox_size = prereserved_sandbox_size_;
 #endif
 
+  NaClFileInfo nexe_file_info;
+  base::PlatformFile nexe_file = IPC::PlatformFileForTransitToPlatformFile(
+      params.nexe_file);
+#if defined(OS_WIN)
+  nexe_file_info.desc =
+      _open_osfhandle(reinterpret_cast<intptr_t>(nexe_file),
+                      _O_RDONLY | _O_BINARY);
+#elif defined(OS_POSIX)
+  nexe_file_info.desc = nexe_file;
+#else
+#error Unsupported target platform.
+#endif
+  nexe_file_info.file_token.lo = params.nexe_token_lo;
+  nexe_file_info.file_token.hi = params.nexe_token_hi;
+  args->nexe_desc = NaClDescIoFromFileInfo(nexe_file_info, NACL_ABI_O_RDONLY);
+
   NaClChromeMainStartApp(nap, args);
 }
 
@@ -472,6 +489,8 @@ void NaClListener::StartNonSfi(const nacl::NaClStartParams& params) {
   CHECK(params.handles.empty());
 
   CHECK(params.nexe_file != IPC::InvalidPlatformFileForTransit());
+  CHECK(params.nexe_token_lo == 0);
+  CHECK(params.nexe_token_hi == 0);
   nacl::nonsfi::MainStart(
       NaClDescIoDescFromDescAllocCtor(
           IPC::PlatformFileForTransitToPlatformFile(params.nexe_file),
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 9ada43d..894f773 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -346,12 +346,25 @@ void LaunchSelLdr(PP_Instance instance,
   std::string error_message_string;
   NaClLaunchResult launch_result;
 
-  content::RendererPpapiHost* host =
-      content::RendererPpapiHost::GetForPPInstance(instance);
+  IPC::PlatformFileForTransit nexe_for_transit =
+      IPC::InvalidPlatformFileForTransit();
+#if defined(OS_POSIX)
+  if (nexe_file_info->handle != PP_kInvalidFileHandle)
+    nexe_for_transit = base::FileDescriptor(nexe_file_info->handle, true);
+#elif defined(OS_WIN)
+  // Duplicate the handle on the browser side instead of the renderer.
+  // This is because BrokerGetFileForProcess isn't part of content/public, and
+  // it's simpler to do the duplication in the browser anyway.
+  nexe_for_transit = nexe_file_info->handle;
+#else
+#error Unsupported target platform.
+#endif
   if (!sender->Send(new NaClHostMsg_LaunchNaCl(
           NaClLaunchParams(
               instance_info.url.spec(),
-              host->ShareHandleWithRemote(nexe_file_info->handle, true),
+              nexe_for_transit,
+              nexe_file_info->token_lo,
+              nexe_file_info->token_hi,
               routing_id,
               perm_bits,
               PP_ToBool(uses_irt),
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 88d6ee0..d960471 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -66,16 +66,16 @@ void Plugin::HistogramTimeSmall(const std::string& name,
                                       kTimeSmallBuckets);
 }
 
-bool Plugin::LoadHelperNaClModule(PP_NaClFileInfo file_info,
-                                  NaClSubprocess* subprocess,
-                                  const SelLdrStartParams& params) {
+bool Plugin::LoadHelperNaClModuleInternal(NaClSubprocess* subprocess,
+                                          const SelLdrStartParams& params) {
   CHECK(!pp::Module::Get()->core()->IsMainThread());
   ServiceRuntime* service_runtime =
       new ServiceRuntime(this,
                          pp_instance(),
                          false,  // No main_service_runtime.
                          false,  // No non-SFI mode (i.e. in SFI-mode).
-                         pp::BlockUntilComplete(), pp::BlockUntilComplete());
+                         pp::BlockUntilComplete(),
+                         pp::BlockUntilComplete());
   subprocess->set_service_runtime(service_runtime);
   PLUGIN_PRINTF(("Plugin::LoadHelperNaClModule "
                  "(service_runtime=%p)\n",
@@ -102,17 +102,14 @@ bool Plugin::LoadHelperNaClModule(PP_NaClFileInfo file_info,
   if (!service_runtime_started)
     return false;
 
-  // Now actually load the nexe, which can happen on a background thread.
+  // Now actually start the nexe.
   //
   // We can't use pp::BlockUntilComplete() inside an in-process plugin, so we
   // have to roll our own blocking logic, similar to WaitForSelLdrStart()
   // above, except without timeout logic.
   pp::Module::Get()->core()->CallOnMainThread(
       0,
-      callback_factory_.NewCallback(
-          &Plugin::LoadNexeAndStart,
-          service_runtime,
-          file_info));
+      callback_factory_.NewCallback(&Plugin::StartNexe, service_runtime));
   return service_runtime->WaitForNexeStart();
 }
 
@@ -153,19 +150,8 @@ void Plugin::LoadNaClModule(PP_NaClFileInfo file_info,
       pp::Var(pp::PASS_REF, nacl_interface_->GetManifestBaseURL(pp_instance()));
   std::string manifest_base_url_str = manifest_base_url.AsString();
 
-  PP_NaClFileInfo file_info_for_srpc = kInvalidNaClFileInfo;
-  PP_NaClFileInfo file_info_for_ipc = kInvalidNaClFileInfo;
-  if (uses_nonsfi_mode) {
-    // In non-SFI mode, LaunchSelLdr is used to pass the nexe file's descriptor
-    // to the NaCl loader process.
-    file_info_for_ipc = file_info;
-  } else {
-    // Otherwise (i.e. in SFI-mode), LoadModule SRPC is still being used.
-    file_info_for_srpc = file_info;
-  }
-
   SelLdrStartParams params(manifest_base_url_str,
-                           file_info_for_ipc,
+                           file_info,
                            true /* uses_irt */,
                            true /* uses_ppapi */,
                            enable_dyncode_syscalls,
@@ -188,18 +174,16 @@ void Plugin::LoadNaClModule(PP_NaClFileInfo file_info,
   // We don't take any action once nexe loading has completed, so pass an empty
   // callback here for |callback|.
   pp::CompletionCallback callback = callback_factory_.NewCallback(
-      &Plugin::LoadNexeAndStart, service_runtime, file_info_for_srpc);
+      &Plugin::StartNexe, service_runtime);
   StartSelLdrOnMainThread(
       static_cast<int32_t>(PP_OK), service_runtime, params, callback);
 }
 
-void Plugin::LoadNexeAndStart(int32_t pp_error,
-                              ServiceRuntime* service_runtime,
-                              PP_NaClFileInfo file_info) {
+void Plugin::StartNexe(int32_t pp_error, ServiceRuntime* service_runtime) {
   CHECK(pp::Module::Get()->core()->IsMainThread());
   if (pp_error != PP_OK)
     return;
-  service_runtime->LoadNexeAndStart(file_info);
+  service_runtime->StartNexe();
 }
 
 bool Plugin::LoadNaClModuleContinuationIntern() {
@@ -242,14 +226,8 @@ NaClSubprocess* Plugin::LoadHelperNaClModule(const nacl::string& helper_url,
   // TODO(sehr): define new UMA stats for translator related nexe events.
   // NOTE: The PNaCl translator nexes are not built to use the IRT.  This is
   // done to save on address space and swap space.
-  //
-  // Currently, this works only in SFI-mode. So, LoadModule SRPC is still used.
-  // So, pass kInvalidNaClFileInfo here, and instead |file_info| is passed
-  // to LoadNaClModuleFromBackgroundThread() below.
-  // TODO(teravest, hidehiko): Pass file_info to params, so that LaunchSelLdr
-  // will look at the info.
   SelLdrStartParams params(helper_url,
-                           kInvalidNaClFileInfo,
+                           file_info,
                            false /* uses_irt */,
                            false /* uses_ppapi */,
                            false /* enable_dyncode_syscalls */,
@@ -258,11 +236,9 @@ NaClSubprocess* Plugin::LoadHelperNaClModule(const nacl::string& helper_url,
 
   // Helper NaCl modules always use the PNaCl manifest, as there is no
   // corresponding NMF.
-  if (!LoadHelperNaClModule(file_info,
-                            nacl_subprocess.get(),
-                            params)) {
+  if (!LoadHelperNaClModuleInternal(nacl_subprocess.get(), params))
     return NULL;
-  }
+
   // We need not wait for the init_done callback.  We can block
   // here in StartSrpcServices, since helper NaCl modules
   // are spawned from a private thread.
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h
index 721018c..ce3273d 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin.h
@@ -147,13 +147,12 @@ class Plugin : public pp::Instance {
   // uma_interface_ normally.
   void HistogramTimeSmall(const std::string& name, int64_t ms);
 
-  // Load a nacl module from the file specified in file_info.
+  // Loads and starts a helper (e.g. llc, ld) NaCl module.
   // Only to be used from a background (non-main) thread for the PNaCl
   // translator. This will fully initialize the |subprocess| if the load was
   // successful.
-  bool LoadHelperNaClModule(PP_NaClFileInfo file_info,
-                            NaClSubprocess* subprocess,
-                            const SelLdrStartParams& params);
+  bool LoadHelperNaClModuleInternal(NaClSubprocess* subprocess,
+                                    const SelLdrStartParams& params);
 
   // Start sel_ldr from the main thread, given the start params.
   // |pp_error| is set by CallOnMainThread (should be PP_OK).
@@ -169,9 +168,7 @@ class Plugin : public pp::Instance {
                              ServiceRuntime* service_runtime);
 
   // This is invoked on the main thread.
-  void LoadNexeAndStart(int32_t pp_error,
-                        ServiceRuntime* service_runtime,
-                        PP_NaClFileInfo file_info);
+  void StartNexe(int32_t pp_error, ServiceRuntime* service_runtime);
 
   // Callback used when getting the URL for the .nexe file.  If the URL loading
   // is successful, the file descriptor is opened and can be passed to sel_ldr
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index fbbd781..7802af30 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -466,16 +466,10 @@ void ServiceRuntime::SignalNexeStarted(bool ok) {
   NaClXCondVarSignal(&cond_);
 }
 
-void ServiceRuntime::LoadNexeAndStart(PP_NaClFileInfo file_info) {
-  NaClLog(4, "ServiceRuntime::LoadNexeAndStart (handle_valid=%d "
-             "token_lo=%" NACL_PRIu64 " token_hi=%" NACL_PRIu64 ")\n",
-      file_info.handle != PP_kInvalidFileHandle,
-      file_info.token_lo,
-      file_info.token_hi);
-
-  bool ok = LoadNexeAndStartInternal(file_info);
+void ServiceRuntime::StartNexe() {
+  bool ok = StartNexeInternal();
   if (ok) {
-    NaClLog(4, "ServiceRuntime::LoadNexeAndStart (success)\n");
+    NaClLog(4, "ServiceRuntime::StartNexe (success)\n");
   } else {
     ReapLogs();
   }
@@ -484,47 +478,12 @@ void ServiceRuntime::LoadNexeAndStart(PP_NaClFileInfo file_info) {
   SignalNexeStarted(ok);
 }
 
-bool ServiceRuntime::LoadNexeAndStartInternal(
-    const PP_NaClFileInfo& file_info) {
-  if(!SetupCommandChannel()) {
+bool ServiceRuntime::StartNexeInternal() {
+  if (!SetupCommandChannel())
     return false;
-  }
-  if (!InitReverseService()) {
-    return false;
-  }
-  if (!LoadModule(file_info)) {
-    ErrorInfo error_info;
-    error_info.SetReport(PP_NACL_ERROR_SEL_LDR_COMMUNICATION_CMD_CHANNEL,
-                         "ServiceRuntime: load module failed");
-    ReportLoadError(error_info);
-    return false;
-  }
-  if (!StartModule()) {
-    return false;
-  }
-  return true;
-}
-
-bool ServiceRuntime::LoadModule(const PP_NaClFileInfo& file_info) {
-  if (uses_nonsfi_mode_) {
-    // In non-SFI mode, loading is done a part of LaunchSelLdr.
-    return true;
-  }
-
-  NaClFileInfo nacl_file_info;
-  nacl_file_info.desc = ConvertFileDescriptor(file_info.handle, true);
-  nacl_file_info.file_token.lo = file_info.token_lo;
-  nacl_file_info.file_token.hi = file_info.token_hi;
-  NaClDesc* desc = NaClDescIoFromFileInfo(nacl_file_info, O_RDONLY);
-  if (desc == NULL) {
+  if (!InitReverseService())
     return false;
-  }
-  // We don't use a scoped_ptr here since we would immediately release the
-  // DescWrapper to LoadModule().
-  nacl::DescWrapper* wrapper =
-    plugin_->wrapper_factory()->MakeGenericCleanup(desc);
-  // TODO(teravest, hidehiko): Replace this by Chrome IPC.
-  return subprocess_->LoadModule(&command_channel_, wrapper);
+  return StartModule();
 }
 
 void ServiceRuntime::ReapLogs() {
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index e3facc6..1b24ff5 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -178,11 +178,10 @@ class ServiceRuntime {
   // successfully or unsuccessfully).
   void SignalNexeStarted(bool ok);
 
-  // Establish an SrpcClient to the sel_ldr instance and load the nexe.
-  // The nexe to be started is passed through |file_info|.
+  // Establish an SrpcClient to the sel_ldr instance and start the nexe.
   // This function must be called on the main thread.
   // This function must only be called once.
-  void LoadNexeAndStart(PP_NaClFileInfo file_info);
+  void StartNexe();
 
   // Starts the application channel to the nexe.
   SrpcClient* SetupAppChannel();
@@ -204,11 +203,10 @@ class ServiceRuntime {
 
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(ServiceRuntime);
-  bool LoadNexeAndStartInternal(const PP_NaClFileInfo& file_info);
+  bool StartNexeInternal();
 
   bool SetupCommandChannel();
   bool InitReverseService();
-  bool LoadModule(const PP_NaClFileInfo& file_info);
   bool StartModule();
   void ReapLogs();