From 12a3a8577060579a98df0fa711addf0545f2b404 Mon Sep 17 00:00:00 2001 From: "teravest@chromium.org" Date: Wed, 23 Jul 2014 20:22:49 +0000 Subject: 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 --- ppapi/native_client/src/trusted/plugin/plugin.cc | 50 +++++--------------- ppapi/native_client/src/trusted/plugin/plugin.h | 11 ++--- .../src/trusted/plugin/service_runtime.cc | 55 +++------------------- .../src/trusted/plugin/service_runtime.h | 8 ++-- 4 files changed, 27 insertions(+), 97 deletions(-) (limited to 'ppapi') 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(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(); -- cgit v1.1