[PPAPI] Fixed FileSystems from JavaScript not having a context.

content/browser/renderer_host/pepper/pepper_renderer_connection.cc

Index: content/browser/renderer_host/pepper/pepper_renderer_connection.cc
diff --git a/content/browser/renderer_host/pepper/pepper_renderer_connection.cc b/content/browser/renderer_host/pepper/pepper_renderer_connection.cc
index 8c76c697e5474da2f5b75d1f889748fa7639851b..26394d76eb7cf062bf95b8a51c459b29c5832f80 100644
--- a/content/browser/renderer_host/pepper/pepper_renderer_connection.cc
+++ b/content/browser/renderer_host/pepper/pepper_renderer_connection.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/renderer_host/pepper/pepper_renderer_connection.h"
 
+#include "base/bind.h"
+#include "base/memory/ref_counted.h"
 #include "content/browser/browser_child_process_host_impl.h"
 #include "content/browser/ppapi_plugin_process_host.h"
 #include "content/browser/renderer_host/pepper/browser_ppapi_host_impl.h"
@@ -22,6 +24,124 @@
 
 namespace content {
 
+namespace {
+
+// Responsible for creating the pending resource hosts, holding their IDs until
+// all of them have been created for a single message, and sending the reply to
+// say that the hosts have been created.
+class PendingHostCreator
+    : public base::RefCounted<PendingHostCreator> {
+ public:
+  PendingHostCreator(BrowserPpapiHostImpl* host,
+                     BrowserMessageFilter* connection,
+                     int routing_id,
+                     int sequence_id,
+                     size_t nested_msgs_size);
+
+  // Adds the given resource host as a pending one. The host is remembered as
+  // host number |index|, and will ultimately be sent to the plugin to be
+  // attached to a real resource.
+  void AddPendingResourceHost(
+      size_t index,
+      scoped_ptr<ppapi::host::ResourceHost> resource_host);
+
+ private:
+  friend class base::RefCounted<PendingHostCreator>;
+
+  // When the last reference to this class is released, all of the resource
+  // hosts would have been added. This destructor sends the message to the
+  // plugin to tell it to attach real hosts to all of the pending hosts that
+  // have been added by this object.
+  ~PendingHostCreator();
+
+  BrowserPpapiHostImpl* host_;
+  BrowserMessageFilter* connection_;
+  int routing_id_;
+  int sequence_id_;
+  std::vector<int> pending_resource_host_ids_;
+};
+
+PendingHostCreator::PendingHostCreator(BrowserPpapiHostImpl* host,
+                                       BrowserMessageFilter* connection,
+                                       int routing_id,
+                                       int sequence_id,
+                                       size_t nested_msgs_size)
+    : host_(host),
+      connection_(connection),
+      routing_id_(routing_id),
+      sequence_id_(sequence_id),
+      pending_resource_host_ids_(nested_msgs_size, 0) {}
+
+void PendingHostCreator::AddPendingResourceHost(
+    size_t index,
+    scoped_ptr<ppapi::host::ResourceHost> resource_host) {
+  pending_resource_host_ids_[index] =
+      host_->GetPpapiHost()->AddPendingResourceHost(resource_host.Pass());
+}
+
+PendingHostCreator::~PendingHostCreator() {
+  connection_->Send(new PpapiHostMsg_CreateResourceHostsFromHostReply(
+      routing_id_, sequence_id_, pending_resource_host_ids_));
+}
+
+void HandleNestedCreateMessage(
+    BrowserPpapiHostImpl* host,
+    const scoped_refptr<PendingHostCreator>& creator,
+    const ppapi::proxy::ResourceMessageCallParams& params,
+    PP_Instance instance,
+    size_t index,
+    const IPC::Message& nested_msg) {
+  scoped_ptr<ppapi::host::ResourceHost> resource_host;
+  if (host->IsValidInstance(instance)) {
+    if (nested_msg.type() == PpapiHostMsg_FileRef_CreateExternal::ID) {
+      // FileRef_CreateExternal is only permitted from the renderer. Because
+      // of this, we handle this message here and not in
+      // content_browser_pepper_host_factory.cc.
+      base::FilePath external_path;
+      if (ppapi::UnpackMessage<PpapiHostMsg_FileRef_CreateExternal>(
+              nested_msg, &external_path)) {
+        resource_host.reset(new PepperFileRefHost(
+            host, instance, params.pp_resource(), external_path));
+      }
+    } else if (nested_msg.type() ==
+               PpapiHostMsg_FileSystem_CreateFromRenderer::ID) {
+      // Similarly, FileSystem_CreateFromRenderer is only permitted from the
+      // renderer.
+      std::string root_url;
+      PP_FileSystemType file_system_type;
+      if (ppapi::UnpackMessage<PpapiHostMsg_FileSystem_CreateFromRenderer>(
+              nested_msg, &root_url, &file_system_type)) {
+        PepperFileSystemBrowserHost* browser_host =
+            new PepperFileSystemBrowserHost(
+                host, instance, params.pp_resource(), file_system_type);
+        resource_host.reset(browser_host);
+        // Open the file system resource host. This is an asynchronous
+        // operation, and we must only add the pending resource host and
+        // send the message once it completes.
+        browser_host->OpenExisting(
+            GURL(root_url),
+            base::Bind(&PendingHostCreator::AddPendingResourceHost,
+                       creator,
+                       index,
+                       base::Passed(&resource_host)));
+        // Do not fall through; the fall-through case adds the pending
+        // resource host to the list. We must do this asynchronously.
+        return;
+      }
+    }
+  }
+
+  if (!resource_host.get()) {
+    resource_host = host->GetPpapiHost()->CreateResourceHost(
+        params, instance, nested_msg);
+  }
+
+  if (resource_host.get())
+    creator->AddPendingResourceHost(index, resource_host.Pass());
+}
+
+}  // namespace
+
 PepperRendererConnection::PepperRendererConnection(int render_process_id)
     : render_process_id_(render_process_id) {
   // Only give the renderer permission for stable APIs.
@@ -93,57 +213,23 @@ void PepperRendererConnection::OnMsgCreateResourceHostsFromHost(
     PP_Instance instance,
     const std::vector<IPC::Message>& nested_msgs) {
   BrowserPpapiHostImpl* host = GetHostForChildProcess(child_process_id);

[yzshen1, 2013/11/02 00:25:07]

You could move line 220 - 221 here and return directly if host == NULL, so that
you won't pass a NULL pointer into |creator|.

[Matt Giuca, 2013/11/02 00:51:45]

Done.

+  scoped_refptr<PendingHostCreator> creator =
+      new PendingHostCreator(
+          host, this, routing_id, params.sequence(), nested_msgs.size());
 
-  std::vector<int> pending_resource_host_ids(nested_msgs.size(), 0);
   if (!host) {
     DLOG(ERROR) << "Invalid plugin process ID.";
   } else {
     for (size_t i = 0; i < nested_msgs.size(); ++i) {
-      const IPC::Message& nested_msg = nested_msgs[i];
-      scoped_ptr<ppapi::host::ResourceHost> resource_host;
-      if (host->IsValidInstance(instance)) {
-        if (nested_msg.type() == PpapiHostMsg_FileRef_CreateExternal::ID) {
-          // FileRef_CreateExternal is only permitted from the renderer. Because
-          // of this, we handle this message here and not in
-          // content_browser_pepper_host_factory.cc.
-          base::FilePath external_path;
-          if (ppapi::UnpackMessage<PpapiHostMsg_FileRef_CreateExternal>(
-                  nested_msg, &external_path)) {
-            resource_host.reset(new PepperFileRefHost(
-                host, instance, params.pp_resource(), external_path));
-          }
-        } else if (nested_msg.type() ==
-                   PpapiHostMsg_FileSystem_CreateFromRenderer::ID) {
-          // Similarly, FileSystem_CreateFromRenderer is only permitted from the
-          // renderer.
-          std::string root_url;
-          PP_FileSystemType file_system_type;
-          if (ppapi::UnpackMessage<PpapiHostMsg_FileSystem_CreateFromRenderer>(
-                  nested_msg, &root_url, &file_system_type)) {
-            resource_host.reset(
-                new PepperFileSystemBrowserHost(host,
-                                                instance,
-                                                params.pp_resource(),
-                                                GURL(root_url),
-                                                file_system_type));
-          }
-        }
-      }
-
-      if (!resource_host.get()) {
-        resource_host = host->GetPpapiHost()->CreateResourceHost(
-            params, instance, nested_msg);
-      }
-
-      if (resource_host.get()) {
-        pending_resource_host_ids[i] =
-            host->GetPpapiHost()->AddPendingResourceHost(resource_host.Pass());
-      }
+      HandleNestedCreateMessage(
+          host, creator, params, instance, i, nested_msgs[i]);
     }
   }
 
-  Send(new PpapiHostMsg_CreateResourceHostsFromHostReply(
-       routing_id, params.sequence(), pending_resource_host_ids));
+  // Note: All of the pending host IDs that were added as part of this
+  // operation will automatically be sent to the plugin when |creator| is
+  // released. This may happen immediately, or (if there are asynchronous
+  // requests to create resource hosts), once all of them complete.
 }
 
 void PepperRendererConnection::OnMsgDidCreateInProcessInstance(