[PPAPI] It is now possible to pass filesystems from JavaScript to NaCl modules.

ppapi/proxy/raw_var_data.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ppapi/proxy/raw_var_data.h"
 
 #include <stack>
 
 #include "base/containers/hash_tables.h"
 #include "base/stl_util.h"
 #include "ipc/ipc_message.h"
+#include "ppapi/proxy/file_system_resource.h"
+#include "ppapi/proxy/plugin_dispatcher.h"
+#include "ppapi/proxy/plugin_globals.h"
+#include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/proxy/ppapi_param_traits.h"
+#include "ppapi/proxy/resource_creation_proxy.h"
 #include "ppapi/shared_impl/array_var.h"
 #include "ppapi/shared_impl/dictionary_var.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/resource_var.h"
 #include "ppapi/shared_impl/scoped_pp_var.h"
 #include "ppapi/shared_impl/var.h"
 #include "ppapi/shared_impl/var_tracker.h"
 
 using std::make_pair;
 
@@ skipping 34 matching lines @@
   } else {
     data->push_back(RawVarData::Create(var.type));
   }
   return data->size() - 1;
 }
 
 bool CanHaveChildren(PP_Var var) {
   return var.type == PP_VARTYPE_ARRAY || var.type == PP_VARTYPE_DICTIONARY;
 }
 
+Connection GetConnectionForInstance(PP_Instance instance) {
+  PluginDispatcher* dispatcher = PluginDispatcher::GetForInstance(instance);
+  return Connection(PluginGlobals::Get()->GetBrowserSender(), dispatcher);

[dmichael (off chromium), 2013/10/22 16:04:28]

I don't think this would do the right thing if it was called in-process. Maybe
it would be good to document that or add a DCHECK?

[Matt Giuca, 2013/10/23 08:05:54]

Done. (We reasoned that GetForInstance will always return NULL in the in-process
case.)

+}
+
 }  // namespace
 
 // RawVarDataGraph ------------------------------------------------------------
 RawVarDataGraph::RawVarDataGraph() {
 }
 
 RawVarDataGraph::~RawVarDataGraph() {
 }
 
 // This function uses a stack-based DFS search to traverse the var graph. Each
@@ skipping 611 matching lines @@
     creation_message_.reset(new IPC::Message(*message));
   else
     creation_message_.reset();
   pending_renderer_host_id_ = resource_var->GetPendingRendererHostId();
   pending_browser_host_id_ = resource_var->GetPendingBrowserHostId();
   initialized_ = true;
   return true;
 }
 
 PP_Var ResourceRawVarData::CreatePPVar(PP_Instance instance) {
-  // If pp_resource_ is NULL, it could be because we are on the plugin side and
-  // there is a pending resource host on the renderer.
-  // TODO(mgiuca): Create a plugin-side resource in this case.
-  // Currently, this should never occur. This will be needed when passing a
-  // resource from the renderer to the plugin (http://crbug.com/177017).
-  DCHECK(pp_resource_);
+  // If we are on the plugin side, and the var is a pending resource host,
+  // create a plugin-side resource. A var is a pending resource host if it has a
+  // pp_resource of 0, and a non-null creation message.
+  if (PpapiGlobals::Get()->IsPluginGlobals() && !pp_resource_ &&
+      creation_message_) {
+    DCHECK(pending_renderer_host_id_);
+    DCHECK(pending_browser_host_id_);
+    switch (creation_message_->type()) {
+      case PpapiPluginMsg_FileSystem_CreateFromPendingHost::ID: {

[dmichael (off chromium), 2013/10/22 16:04:28]

switch seems like an odd construct here. How about just an if/else?

[Matt Giuca, 2013/10/23 08:05:54]

Why? This is just the first type of resource we are going to allow passing -- in
the future there may be many others. This code is taking an enum value, and
performing a different action depending on which specific value it takes, with a
default case. It is perfect for a switch.

I realise that similar code in pepper_renderer_connection.cc also uses an
if/else -- I think that should be a switch also. I'm happy to change that over
to a switch for consistency. But if you really want these to be ifs, I can
change this one.

[dmichael (off chromium), 2013/10/23 17:33:21]

Oh, right, switch/case is fine. I wasn't thinking about future resource types.

+        PP_FileSystemType file_system_type;
+        if (!UnpackMessage<PpapiPluginMsg_FileSystem_CreateFromPendingHost>(
+                 *creation_message_, &file_system_type)) {

[dmichael (off chromium), 2013/10/22 16:04:28]

nit: indent is one space too far

[Matt Giuca, 2013/10/23 08:05:54]

ClangFormat did it. It's indenting four spaces from the start of the function
name (not from the start of the expression), which I believe is correct.

+          NOTREACHED() << "Invalid message of type "
+                          "PpapiPluginMsg_FileSystem_CreateFromPendingHost";
+          return PP_MakeNull();
+        }
+        // Create a plugin-side resource and attach it to the host resource.
+        pp_resource_ =
+            (new FileSystemResource(GetConnectionForInstance(instance),
+                                    instance,
+                                    pending_renderer_host_id_,
+                                    pending_browser_host_id_,
+                                    file_system_type))->GetReference();
+        DCHECK(pp_resource_);

[raymes, 2013/10/22 05:59:21]

I think this DCHECK might be a bit uneeded. This case shouldn't really happen.

[Matt Giuca, 2013/10/23 08:05:54]

Well, the point of a DCHECK is to check for things that shouldn't really happen
:). But in this case it is quite unlikely, so I removed it.

+        break;
+      }
+      default: {
+        NOTREACHED() << "Creation message has unexpected type "
+                     << creation_message_->type();
+        return PP_MakeNull();
+      }
+    }
+  }
 
   return PpapiGlobals::Get()->GetVarTracker()->MakeResourcePPVar(pp_resource_);
 }
 
 void ResourceRawVarData::PopulatePPVar(const PP_Var& var,
                                        const std::vector<PP_Var>& graph) {
 }
 
 void ResourceRawVarData::Write(IPC::Message* m,
                                const HandleWriter& handle_writer) {
@@ skipping 24 matching lines @@
     if (!IPC::ParamTraits<IPC::Message>::Read(m, iter, creation_message_.get()))
       return false;
   } else {
     creation_message_.reset();
   }
   return true;
 }
 
 }  // namespace proxy
 }  // namespace ppapi