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

4 files changed, 27 insertions, 97 deletions

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();