diff options
author | teravest@chromium.org <teravest@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-23 20:22:49 +0000 |
---|---|---|
committer | teravest@chromium.org <teravest@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-23 20:22:49 +0000 |
commit | 12a3a8577060579a98df0fa711addf0545f2b404 (patch) | |
tree | e45a054d9840bd94fd4fef464d2288f14f104ceb | |
parent | df82b4f579d6e22c187922d1547f400549c836d6 (diff) | |
download | chromium_src-12a3a8577060579a98df0fa711addf0545f2b404.zip chromium_src-12a3a8577060579a98df0fa711addf0545f2b404.tar.gz chromium_src-12a3a8577060579a98df0fa711addf0545f2b404.tar.bz2 |
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
-rw-r--r-- | chrome/browser/BUILD.gn | 4 | ||||
-rw-r--r-- | components/components_tests.gyp | 6 | ||||
-rw-r--r-- | components/nacl/browser/DEPS | 1 | ||||
-rw-r--r-- | components/nacl/browser/nacl_host_message_filter.cc | 34 | ||||
-rw-r--r-- | components/nacl/browser/nacl_process_host.cc | 22 | ||||
-rw-r--r-- | components/nacl/browser/nacl_process_host.h | 5 | ||||
-rw-r--r-- | components/nacl/common/nacl_host_messages.h | 2 | ||||
-rw-r--r-- | components/nacl/common/nacl_messages.h | 2 | ||||
-rw-r--r-- | components/nacl/common/nacl_types.cc | 20 | ||||
-rw-r--r-- | components/nacl/common/nacl_types.h | 12 | ||||
-rw-r--r-- | components/nacl/loader/nacl_listener.cc | 21 | ||||
-rw-r--r-- | components/nacl/renderer/ppb_nacl_private_impl.cc | 19 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/plugin.cc | 50 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/plugin.h | 11 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/service_runtime.cc | 55 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/service_runtime.h | 8 |
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(); |