 Chromium Code Reviews
 Chromium Code Reviews Issue 649603004:
  Non-SFI NaCl: Batch-open resource files  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 649603004:
  Non-SFI NaCl: Batch-open resource files  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| 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..0844a84ea68a9b533bc8a4a2f1d5d6877fae79de 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 | 
| +// 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]; | 
| 
teravest
2014/11/10 20:36:12
Please use a pointer instead of a reference here.
 
Yusuke Sato
2014/11/11 00:58:35
Done.
 | 
| + 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; |