Pepper: Nexe downloading out of the trusted plugin.

components/nacl/renderer/ppb_nacl_private_impl.cc

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 3926b440403cc1760c424b709db4a791dfa4fdfb..04a75e96a6be42bb5c4d9bde40417203a3043df6 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -20,6 +20,7 @@
 #include "components/nacl/common/nacl_messages.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "components/nacl/common/nacl_types.h"
+#include "components/nacl/renderer/file_downloader.h"
 #include "components/nacl/renderer/histogram.h"
 #include "components/nacl/renderer/json_manifest.h"
 #include "components/nacl/renderer/manifest_downloader.h"
@@ -251,6 +252,43 @@ class ManifestServiceProxy : public ManifestServiceChannel::Delegate {
   DISALLOW_COPY_AND_ASSIGN(ManifestServiceProxy);
 };
 
+blink::WebURLLoader* CreateWebURLLoader(PP_Instance instance,
+                                        const GURL& gurl) {
+  content::PepperPluginInstance* plugin_instance =
+      content::PepperPluginInstance::Get(instance);
+  blink::WebURLLoaderOptions options;
+  options.untrustedHTTP = true;
+
+  blink::WebSecurityOrigin security_origin =
+      plugin_instance->GetContainer()->element().document().securityOrigin();

[bbudge, 2014/05/12 22:32:06]

This function and the following one could be simplified significantly if you
pass in the WebDocument as the first parameter. i.e. caller does
plugin_instance->GetContainer()->element().document().

[teravest, 2014/05/13 17:05:32]

Done.

+  // Options settings here follow the original behavior in the trusted
+  // plugin and PepperURLLoaderHost.
+  if (security_origin.canRequest(gurl)) {
+    options.allowCredentials = true;
+  } else {
+    // Allow CORS.
+    options.crossOriginRequestPolicy =
+        blink::WebURLLoaderOptions::CrossOriginRequestPolicyUseAccessControl;
+  }
+
+  blink::WebFrame* frame =
+      plugin_instance->GetContainer()->element().document().frame();
+  return frame->createAssociatedURLLoader(options);
+}
+
+blink::WebURLRequest CreateWebURLRequest(PP_Instance instance,
+                                         const GURL& gurl) {
+  content::PepperPluginInstance* plugin_instance =
+      content::PepperPluginInstance::Get(instance);
+  blink::WebFrame* frame =
+      plugin_instance->GetContainer()->element().document().frame();
+  blink::WebURLRequest request;
+  request.initialize();
+  request.setURL(gurl);
+  request.setFirstPartyForCookies(frame->document().firstPartyForCookies());
+  return request;
+}
+
 // Launch NaCl's sel_ldr process.
 void LaunchSelLdr(PP_Instance instance,
                   const char* alleged_url,
@@ -669,24 +707,6 @@ void DispatchEvent(PP_Instance instance,
   DispatchProgressEvent(instance, event);
 }
 
-void NexeFileDidOpen(PP_Instance instance,
-                     int32_t pp_error,
-                     int32_t fd,
-                     int32_t http_status,
-                     int64_t nexe_bytes_read,
-                     const char* url,
-                     int64_t time_since_open) {
-  NexeLoadManager* load_manager = GetNexeLoadManager(instance);
-  if (load_manager) {
-    load_manager->NexeFileDidOpen(pp_error,
-                                  fd,
-                                  http_status,
-                                  nexe_bytes_read,
-                                  url,
-                                  time_since_open);
-  }
-}
-
 void ReportLoadSuccess(PP_Instance instance,
                        const char* url,
                        uint64_t loaded_bytes,
@@ -906,31 +926,8 @@ void DownloadManifestToBuffer(PP_Instance instance,
   }
 
   const GURL& gurl = load_manager->manifest_base_url();
-
-  content::PepperPluginInstance* plugin_instance =
-      content::PepperPluginInstance::Get(instance);
-  blink::WebURLLoaderOptions options;
-  options.untrustedHTTP = true;
-
-  blink::WebSecurityOrigin security_origin =
-      plugin_instance->GetContainer()->element().document().securityOrigin();
-  // Options settings here follow the original behavior in the trusted
-  // plugin and PepperURLLoaderHost.
-  if (security_origin.canRequest(gurl)) {
-    options.allowCredentials = true;
-  } else {
-    // Allow CORS.
-    options.crossOriginRequestPolicy =
-        blink::WebURLLoaderOptions::CrossOriginRequestPolicyUseAccessControl;
-  }
-
-  blink::WebFrame* frame =
-      plugin_instance->GetContainer()->element().document().frame();
-  blink::WebURLLoader* url_loader = frame->createAssociatedURLLoader(options);
-  blink::WebURLRequest request;
-  request.initialize();
-  request.setURL(gurl);
-  request.setFirstPartyForCookies(frame->document().firstPartyForCookies());
+  blink::WebURLLoader* url_loader = CreateWebURLLoader(instance, gurl);
+  blink::WebURLRequest request = CreateWebURLRequest(instance, gurl);
 
   // ManifestDownloader deletes itself after invoking the callback.
   ManifestDownloader* client = new ManifestDownloader(
@@ -1251,6 +1248,148 @@ void PostMessageToJavaScript(PP_Instance instance, const char* message) {
                  std::string(message)));
 }
 
+// Encapsulates some of the state for a call to DownloadNexe to prevent
+// argument lists from getting too long.
+struct DownloadNexeRequest {
+  PP_Instance instance;
+  const char* url;
+  PP_CompletionCallback callback;
+  base::Time start_time;
+};
+
+class DownloadNexeProgressTracker {

[bbudge, 2014/05/12 22:32:06]

I don't see where this is used. Also, it doesn't seem to do anything.

[teravest, 2014/05/13 17:05:32]

Thanks for pointing that out. I'd completely forgotten about this.
I've changed the progress event code to use this now; this object is necessary
to keep from dispatching progress events more often than once every 10ms.

+ public:
+  void ReportProgress(const std::string& url,
+                      int64_t total_bytes_received,
+                      int64_t total_bytes_to_be_received) {
+    base::Time now = base::Time::Now();
+    if (now - last_event_ > base::TimeDelta::FromMilliseconds(10)) {
+      // Actually dispatch the progress event.
+      last_event_ = now;
+    }
+  }
+ private:
+  base::Time last_event_;
+};
+
+void DownloadNexeProgress(PP_Instance instance,
+                          const std::string& url,
+                          int64_t total_bytes_received,
+                          int64_t total_bytes_to_be_received);
+
+void DownloadNexeCompletion(const DownloadNexeRequest& request,
+                            blink::WebURLLoader* loader,
+                            base::PlatformFile target_file,
+                            PP_FileHandle* out_handle,
+                            FileDownloader::Status status,
+                            int http_status);
+
+void DownloadNexe(PP_Instance instance,
+                  const char* url,
+                  PP_FileHandle* out_handle,
+                  PP_CompletionCallback callback) {
+  CHECK(url);
+  CHECK(out_handle);
+  DownloadNexeRequest request;
+  request.instance = instance;
+  request.url = url;
+  request.callback = callback;
+  request.start_time = base::Time::Now();
+
+  // 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,
+                                                 &file_token_lo,
+                                                 &file_token_hi);
+
+  // We shouldn't hit this if the file URL is in an installed app.

[bbudge, 2014/05/12 22:32:06]

comment seems wrong.

[teravest, 2014/05/13 17:05:32]

Removed.

+  if (file_handle != PP_kInvalidFileHandle) {
+    DownloadNexeCompletion(request, NULL, file_handle, out_handle,
+                           FileDownloader::SUCCESS, 200);
+    return;
+  }
+
+  // The fast path didn't work, we'll fetch the file using URLLoader and write
+  // it to local storage.
+  base::PlatformFile target_file = CreateTemporaryFile(instance);
+  GURL gurl(url);
+  blink::WebURLLoader* url_loader = CreateWebURLLoader(instance, gurl);

[bbudge, 2014/05/12 22:32:06]

Now that we're in Chrome-land, can we use scoped_ptr to protect against leaks?

[teravest, 2014/05/13 17:05:32]

Done.

+  blink::WebURLRequest url_request = CreateWebURLRequest(instance, gurl);
+
+  // FileDownloader deletes itself after invoking the callback.
+  FileDownloader* client = new FileDownloader(

[bbudge, 2014/05/12 22:32:06]

'client' is a slightly confusing name.

[teravest, 2014/05/13 17:05:32]

Changed to file_downloader

+      target_file,
+      base::Bind(&DownloadNexeCompletion, request, url_loader, target_file,
+                 out_handle),
+      base::Bind(&DownloadNexeProgress, instance, url));
+  url_loader->loadAsynchronously(url_request, client);
+}
+
+void DownloadNexeProgress(PP_Instance instance,
+                          const std::string& url,
+                          int64_t total_bytes_received,
+                          int64_t total_bytes_to_be_received) {
+  ProgressEvent event(PP_NACL_EVENT_PROGRESS,
+                      url,
+                      total_bytes_to_be_received >= 0,
+                      total_bytes_received,
+                      total_bytes_to_be_received);
+  DispatchProgressEvent(instance, event);

[bbudge, 2014/05/12 22:32:06]

Might be a little more readable if you create the event as a temporary in the
function invocation.

[teravest, 2014/05/13 17:05:32]

Done.

+}
+
+void DownloadNexeCompletion(const DownloadNexeRequest& request,
+                            blink::WebURLLoader* loader,

[bbudge, 2014/05/12 22:32:06]

scoped_ptr<blink::WebURLLoader>?

[teravest, 2014/05/13 17:05:32]

Done.

+                            base::PlatformFile target_file,
+                            PP_FileHandle* out_handle,
+                            FileDownloader::Status status,
+                            int http_status) {
+  int32_t pp_error;
+  switch (status) {
+    case FileDownloader::SUCCESS:
+      *out_handle = target_file;
+      pp_error = PP_OK;
+      break;
+    case FileDownloader::ACCESS_DENIED:
+      pp_error = PP_ERROR_NOACCESS;
+      break;
+    case FileDownloader::FAILED:
+      pp_error = PP_ERROR_FAILED;
+      break;
+    default:
+      NOTREACHED();
+      return;
+  }
+  delete loader;
+
+  int64_t bytes_read = -1;
+  if (pp_error == PP_OK && target_file != base::kInvalidPlatformFileValue) {
+    base::PlatformFileInfo info;
+    if (GetPlatformFileInfo(target_file, &info))
+      bytes_read = info.size;
+  }
+
+  if (bytes_read == -1) {
+    base::ClosePlatformFile(target_file);
+    pp_error = PP_ERROR_FAILED;
+  }
+
+  base::TimeDelta download_time = base::Time::Now() - request.start_time;
+
+  NexeLoadManager* load_manager = GetNexeLoadManager(request.instance);
+  if (load_manager) {
+    load_manager->NexeFileDidOpen(pp_error,
+                                  target_file,
+                                  http_status,
+                                  bytes_read,
+                                  request.url,
+                                  download_time);
+  }
+
+  request.callback.func(request.callback.user_data, pp_error);
+}
+
 const PPB_NaCl_Private nacl_interface = {
   &LaunchSelLdr,
   &StartPpapiProxy,
@@ -1265,7 +1404,6 @@ const PPB_NaCl_Private nacl_interface = {
   &ReportTranslationFinished,
   &OpenNaClExecutable,
   &DispatchEvent,
-  &NexeFileDidOpen,
   &ReportLoadSuccess,
   &ReportLoadError,
   &ReportLoadAbort,
@@ -1298,7 +1436,8 @@ const PPB_NaCl_Private nacl_interface = {
   &ManifestResolveKey,
   &GetPNaClResourceInfo,
   &GetCpuFeatureAttrs,
-  &PostMessageToJavaScript
+  &PostMessageToJavaScript,
+  &DownloadNexe
 };
 
 }  // namespace