Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(320)

Unified Diff: components/nacl/renderer/ppb_nacl_private_impl.cc

Issue 649603004: Non-SFI NaCl: Batch-open resource files (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: address review comments Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/nacl/renderer/json_manifest.cc ('k') | ipc/file_descriptor_set_posix.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/nacl/renderer/ppb_nacl_private_impl.cc
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 2df50bbf293e23782a26875353538d74a4b584ac..6c406e1965c64f9c7912e029600711f896142247 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -15,6 +15,7 @@
#include "base/files/file.h"
#include "base/lazy_instance.h"
#include "base/logging.h"
+#include "base/macros.h"
#include "base/rand_util.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
@@ -63,6 +64,10 @@
#include "third_party/jsoncpp/source/include/json/reader.h"
#include "third_party/jsoncpp/source/include/json/value.h"
+#if defined(OS_POSIX)
+#include "ipc/file_descriptor_set_posix.h"
+#endif
+
namespace nacl {
namespace {
@@ -72,6 +77,20 @@ const char* const kPortableArch = "portable";
// The base URL for resources used by the PNaCl translator processes.
const char* kPNaClTranslatorBaseUrl = "chrome://pnacl-translator/";
+// The maximum number of resource files DownloadNexe() can open.
+const size_t kMaxPreOpenResourceFiles = 121;
+
+#if defined(OS_POSIX)
+// On POSIX platforms, the maximum number of files a process can pass to
+// another is limited to kMaxDescriptorsPerMessage which is 128. Since
+// the browser process may use up to 7 descriptors for passing non-resource
teravest 2014/11/12 20:33:01 Where is the browser process limited to 7 descript
Yusuke Sato 2014/11/12 23:09:11 Done. Added a CHECK to nacl_process_host.cc. Even
+// files when creating the NaCl plugin process, kMaxPreOpenResourceFiles
+// must be smaller than or equal to 121.
+COMPILE_ASSERT(kMaxPreOpenResourceFiles <=
+ FileDescriptorSet::kMaxDescriptorsPerMessage - 7,
+ k_max_pre_open_resource_files_too_large);
+#endif
+
base::LazyInstance<scoped_refptr<PnaclTranslationResourceHost> >
g_pnacl_resource_host = LAZY_INSTANCE_INITIALIZER;
@@ -301,6 +320,8 @@ void LaunchSelLdr(PP_Instance instance,
PP_Bool main_service_runtime,
const char* alleged_url,
const PP_NaClFileInfo* nexe_file_info,
+ const PP_NaClResourceFileHandle* resource_file_handles,
+ uint32_t resource_file_handles_len,
PP_Bool uses_nonsfi_mode,
PP_Bool enable_ppapi_dev,
PP_NaClAppProcessType pp_process_type,
@@ -354,14 +375,26 @@ void LaunchSelLdr(PP_Instance instance,
IPC::PlatformFileForTransit nexe_for_transit =
IPC::InvalidPlatformFileForTransit();
+ std::vector<NaClLaunchParams::ResourceFileInfo> resource_files_info;
+
#if defined(OS_POSIX)
if (nexe_file_info->handle != PP_kInvalidFileHandle)
nexe_for_transit = base::FileDescriptor(nexe_file_info->handle, true);
+ for (size_t i = 0; i < resource_file_handles_len; ++i) {
+ NaClLaunchParams::ResourceFileInfo resource_file_info(
+ base::FileDescriptor(resource_file_handles[i].handle, true),
+ resource_file_handles[i].token_lo,
+ resource_file_handles[i].token_hi,
+ resource_file_handles[i].key);
+ resource_files_info.push_back(resource_file_info);
+ }
#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;
+ // TODO(yusukes): Support pre-opening resource files.
+ CHECK(resource_file_handles_len == 0);
#else
#error Unsupported target platform.
#endif
@@ -371,6 +404,7 @@ void LaunchSelLdr(PP_Instance instance,
nexe_for_transit,
nexe_file_info->token_lo,
nexe_file_info->token_hi,
+ resource_files_info,
routing_id,
perm_bits,
PP_ToBool(uses_nonsfi_mode),
@@ -681,10 +715,14 @@ void ReportTranslationFinished(PP_Instance instance,
g_pnacl_resource_host.Get()->ReportTranslationFinished(instance, success);
}
-PP_FileHandle OpenNaClExecutable(PP_Instance instance,
- const char* file_url,
- uint64_t* nonce_lo,
- uint64_t* nonce_hi) {
+PP_FileHandle OpenNaClResources(
+ PP_Instance instance,
+ const char* file_url,
+ uint64_t* nonce_lo,
+ uint64_t* nonce_hi,
+ const std::vector<std::string>& resource_file_urls,
+ PP_NaClResourceFileHandle** out_resource_file_handles,
+ uint32_t* out_resource_file_handles_len) {
// Fast path only works for installed file URLs.
GURL gurl(file_url);
if (!gurl.SchemeIs("chrome-extension"))
@@ -709,18 +747,57 @@ PP_FileHandle OpenNaClExecutable(PP_Instance instance,
*nonce_lo = 0;
*nonce_hi = 0;
base::FilePath file_path;
- if (!sender->Send(
- new NaClHostMsg_OpenNaClExecutable(GetRoutingID(instance),
- GURL(file_url),
- &out_fd,
- nonce_lo,
- nonce_hi))) {
+
+ std::vector<GURL> resource_urls;
+ for (size_t i = 0; i < resource_file_urls.size(); ++i) {
+ GURL gurl(resource_file_urls[i]);
+ if (!gurl.SchemeIs("chrome-extension"))
+ return PP_kInvalidFileHandle;
+ if (!security_origin.canRequest(gurl))
+ return PP_kInvalidFileHandle;
+ resource_urls.push_back(gurl);
+ }
+
+ nacl::NaClOpenExecutableResult open_result;
+ if (!sender->Send(new NaClHostMsg_OpenNaClResources(GetRoutingID(instance),
+ GURL(file_url),
+ resource_urls,
+ &open_result))) {
return PP_kInvalidFileHandle;
}
- if (out_fd == IPC::InvalidPlatformFileForTransit())
+ if (open_result.file_info.file == IPC::InvalidPlatformFileForTransit())
return PP_kInvalidFileHandle;
+ for (size_t i = 0; i < open_result.resource_files_info.size(); ++i) {
+ if (open_result.resource_files_info[i].file ==
+ IPC::InvalidPlatformFileForTransit()) {
+ DLOG(ERROR) << "Could not open a resource file: "
+ << resource_file_urls[i];
+ return PP_kInvalidFileHandle;
+ }
+ }
+
+ out_fd = open_result.file_info.file;
+ *nonce_lo = open_result.file_info.file_token_lo;
+ *nonce_hi = open_result.file_info.file_token_hi;
+
+ if (out_resource_file_handles && !open_result.resource_files_info.empty()) {
+ DCHECK(out_resource_file_handles_len);
+ *out_resource_file_handles_len =
+ static_cast<uint32_t>(open_result.resource_files_info.size());
+ *out_resource_file_handles =
+ new PP_NaClResourceFileHandle[*out_resource_file_handles_len];
+ for (size_t i = 0; i < *out_resource_file_handles_len; ++i) {
+ PP_NaClResourceFileHandle& entry = (*out_resource_file_handles)[i];
+ entry.handle = IPC::PlatformFileForTransitToPlatformFile(
+ open_result.resource_files_info[i].file);
+ entry.token_lo = open_result.resource_files_info[i].file_token_lo;
+ entry.token_hi = open_result.resource_files_info[i].file_token_hi;
+ entry.key = NULL; // DownloadNexe fills this later.
+ }
+ }
+
return IPC::PlatformFileForTransitToPlatformFile(out_fd);
}
@@ -1214,22 +1291,52 @@ void DownloadNexeCompletion(const DownloadNexeRequest& request,
void DownloadNexe(PP_Instance instance,
const char* url,
+ PP_Bool download_resource_files,
PP_NaClFileInfo* out_file_info,
+ PP_NaClResourceFileHandle** out_resource_file_handles,
+ uint32_t* out_resource_file_handles_len,
PP_CompletionCallback callback) {
CHECK(url);
CHECK(out_file_info);
+ CHECK(!download_resource_files || out_resource_file_handles);
+
DownloadNexeRequest request;
request.instance = instance;
request.url = url;
request.callback = callback;
request.start_time = base::Time::Now();
+ std::vector<
+ std::pair<std::string /*url*/, std::string /*key*/> > resource_files;
+ std::vector<std::string> resource_file_urls;
+
+ if (download_resource_files) {
+ JsonManifest* manifest = GetJsonManifest(instance);
+ if (manifest)
+ manifest->GetFiles(&resource_files);
+ if (resource_files.size() > kMaxPreOpenResourceFiles)
+ resource_files.resize(kMaxPreOpenResourceFiles);
+ for (size_t i = 0; i < resource_files.size(); ++i) {
+ resource_file_urls.push_back(resource_files[i].first);
+ }
+ }
+
// Try the fast path for retrieving the file first.
- PP_FileHandle handle = OpenNaClExecutable(instance,
- url,
- &out_file_info->token_lo,
- &out_file_info->token_hi);
+ PP_FileHandle handle = OpenNaClResources(instance,
+ url,
+ &out_file_info->token_lo,
+ &out_file_info->token_hi,
+ resource_file_urls,
+ out_resource_file_handles,
+ out_resource_file_handles_len);
if (handle != PP_kInvalidFileHandle) {
+ if (!resource_files.empty()) {
+ for (size_t i = 0; i < resource_files.size(); ++i) {
+ PP_NaClResourceFileHandle* entry = &(*out_resource_file_handles)[i];
+ CHECK(entry->handle != PP_kInvalidFileHandle) << i;
+ entry->key = strdup(resource_files[i].second.c_str());
+ }
+ }
DownloadNexeCompletion(request,
out_file_info,
FileDownloader::SUCCESS,
@@ -1243,6 +1350,10 @@ void DownloadNexe(PP_Instance instance,
base::File target_file(CreateTemporaryFile(instance));
GURL gurl(url);
+ // On the slow path, do not retry to pre-open the resource files.
+ if (out_resource_file_handles)
+ *out_resource_file_handles = NULL;
+
content::PepperPluginInstance* plugin_instance =
content::PepperPluginInstance::Get(instance);
if (!plugin_instance) {
@@ -1380,10 +1491,13 @@ void DownloadFile(PP_Instance instance,
// Try the fast path for retrieving the file first.
uint64_t file_token_lo = 0;
uint64_t file_token_hi = 0;
- PP_FileHandle file_handle = OpenNaClExecutable(instance,
- url.c_str(),
- &file_token_lo,
- &file_token_hi);
+ PP_FileHandle file_handle = OpenNaClResources(instance,
+ url.c_str(),
+ &file_token_lo,
+ &file_token_hi,
+ std::vector<std::string>(),
+ NULL,
+ NULL);
if (file_handle != PP_kInvalidFileHandle) {
PP_NaClFileInfo file_info;
file_info.handle = file_handle;
« no previous file with comments | « components/nacl/renderer/json_manifest.cc ('k') | ipc/file_descriptor_set_posix.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698