[PPAPI] It is now possible to pass a FileSystem to JS in PostMessage.

content/renderer/pepper/resource_converter.cc

Index: content/renderer/pepper/resource_converter.cc
diff --git a/content/renderer/pepper/resource_converter.cc b/content/renderer/pepper/resource_converter.cc
index c4590b84b99cf1c835fc1c10200335286d020ffd..3543405b60d8852c07d12adf7d53f45a47740661 100644
--- a/content/renderer/pepper/resource_converter.cc
+++ b/content/renderer/pepper/resource_converter.cc
@@ -20,6 +20,8 @@
 #include "third_party/WebKit/public/platform/WebMediaStreamSource.h"
 #include "third_party/WebKit/public/web/WebDOMFileSystem.h"
 #include "third_party/WebKit/public/web/WebDOMMediaStreamTrack.h"
+#include "third_party/WebKit/public/web/WebFrame.h"
+#include "webkit/common/fileapi/file_system_util.h"
 
 using ppapi::ResourceVar;
 
@@ -54,6 +56,30 @@ PP_FileSystemType WebFileSystemTypeToPPAPI(blink::WebFileSystem::Type type) {
   }
 }
 
+// Converts a fileapi::FileSystemType to a blink::WebFileSystemType.
+// Returns true on success, false if |type| does not correspond to a
+// WebFileSystemType.
+bool FileApiFileSystemTypeToWebFileSystemType(

[Matt Giuca, 2014/02/24 04:45:37]

See Kinuko's comment on Patch Set 4 (not yet resolved).

[kinuko, 2014/02/25 05:07:05]

I'm fine with this code.

+    fileapi::FileSystemType type,
+    blink::WebFileSystemType* result_type) {
+  switch (type) {
+    case fileapi::kFileSystemTypeTemporary:
+      *result_type = blink::WebFileSystemTypeTemporary;
+      return true;
+    case fileapi::kFileSystemTypePersistent:
+      *result_type = blink::WebFileSystemTypePersistent;
+      return true;
+    case fileapi::kFileSystemTypeIsolated:
+      *result_type = blink::WebFileSystemTypeIsolated;
+      return true;
+    case fileapi::kFileSystemTypeExternal:
+      *result_type = blink::WebFileSystemTypeExternal;
+      return true;
+    default:

[kinuko, 2014/02/25 05:07:05]

nit: do we want to put NOTREACHED() here?

[Matt Giuca, 2014/03/06 01:06:26]

I don't think we want to. It isn't a programming error if the type is not
supported, it could mean bad data from the plugin.

This code is the first place where the file system type of the value passed from
the user is checked. If the file type is some crazy type (which can happen if,
for example, the URL ends with "/test"), then this is a user error, not a Chrome
error, and it should fail properly, not crash the renderer.

[kinuko, 2014/03/06 05:20:40]

Ok (btw NOTREACHED won't crash renderer in production build)

[Matt Giuca, 2014/03/06 06:53:54]

Yeah, I know, but the debug build still shouldn't crash in response to a user
error (if it's possible for a user to trigger a debug crash then that should be
considered a bug).

+      return false;
+  }
+}
+
 // Given a V8 value containing a DOMFileSystem, creates a resource host and
 // returns the resource information for serialization.
 // On error, false.
@@ -92,6 +118,29 @@ bool DOMFileSystemToResource(
   return true;
 }
 
+bool ResourceHostToDOMFileSystem(
+    content::PepperFileSystemHost* file_system_host,
+    v8::Handle<v8::Context> context,
+    v8::Handle<v8::Value>* dom_file_system) {
+  GURL root_url = file_system_host->GetRootUrl();
+  GURL origin;
+  fileapi::FileSystemType mount_type;
+  base::FilePath virtual_path;
+  fileapi::ParseFileSystemSchemeURL(
+      root_url, &origin, &mount_type, &virtual_path);
+
+  std::string name = GetFileSystemName(origin, mount_type);

[raymes, 2014/02/25 04:55:44]

missing namespace

[kinuko, 2014/02/25 05:07:05]

Hmm.. does it mean we may not get the same .name values if we pass DOMFileSystem
to pepper and then do the other way around, if the original DOMFileSystem.name
field was not created by fileapi::GetFileSystemName()?

In either way, fileapi::GetFileSystemName(origin, type) is more common way to
create a name. (s/mount_type/type/)

[Matt Giuca, 2014/03/06 01:06:26]

I don't understand this concern. Can you elaborate?
Do you just mean I should rename my mount_type variable to type? (Done). Or am I
using GetFileSystemName incorrectly?

[kinuko, 2014/03/06 05:20:40]

I could be mistaken something. If we pass a DOMFileSystem [A] (whose name must
have been initialized somewhere in chrome code when it's created) to plugin and
then do the other way around it will go through this code, is my understanding
correct?

And then we create 'name' field here for the newly created DOMFileSystem, which
may lead to have a different name from the original one ([A]), that's what I
guessed. (I'm not telling that it's wrong or it should be avoided, just trying
to understand what will happen)
Yes. Thanks.

[kinuko, 2014/03/06 05:30:44]

Probably this code's main target is to convert a pepper filesystem (that is
created by PPAPI) into a corresponding DOMFileSystem? If so it'll likely be
Temporary or Persistent filesystem, and in the case creating a name by
GetFileSystemName() will give what's expected. So I assume this code is fine
basically

[Matt Giuca, 2014/03/06 06:53:54]

OK thanks for explaining. You can also get other types of file system, basically
from round-tripping. For example, you can pass an isolated file system from
renderer to plugin and back.

I have manually tested a round-trip of an isolated FS (my automated tests only
test temporary FS roundtrip) and it seems to work OK. Why do you think the name
would be different? I thought there was a standard pattern for naming file
systems?

[kinuko, 2014/03/06 07:05:51]

Most (or possibly all) filesystems use the standard pattern, but technically
each FileSystem backend can assign arbitrary name if it wants, since fileapi
infra just delegates the task to each fs implementation, so we can't really
guarantee it. I don't think it'll be a big problem in most cases, but wanted to
note.

+  blink::WebFileSystemType blink_type;
+  if (!FileApiFileSystemTypeToWebFileSystemType(mount_type, &blink_type))
+    return false;
+  blink::WebFrame* frame = blink::WebFrame::frameForContext(context);
+  *dom_file_system = frame->createSerializableFileSystem(

[kinuko, 2014/02/25 05:07:05]

Let me make sure once more (recalling past discussion)... we won't go through
here across different origins/pages, right?

[kinuko, 2014/02/26 03:53:56]

Btw can you instead use new WebDOMFileSystem::create() (and toV8Value()) instead
of WebFrame::createSerializableFileSystem?

We're deprecating the WebFrame methods.

[Matt Giuca, 2014/03/06 01:06:26]

I don't know what you mean exactly. Can you explain the question in more detail?
Do you mean "will the plugin have the same origin to the page it is embedded
in?" (I believe the answer is yes.)
Done.

[kinuko, 2014/03/06 05:20:40]

Yes, that's what I was asking. Serializable FileSystem isn't supposed to be
passed across different origins, and I wanted to make sure it won't happen in
your code.

[Matt Giuca, 2014/03/06 06:53:54]

Yes, I think I remember this conversation now. I believe the plugin will always
be in the same domain as the page.

Raymes: Can you confirm this?

[kinuko, 2014/03/06 07:05:51]

Thanks. lgtm then (as far as Raymes says yes to this question)

+      blink_type,
+      blink::WebString::fromUTF8(name),
+      blink::WebString::fromUTF8(root_url.spec()));

[raymes, 2014/02/25 04:55:44]

I defer to kinuko for the creation of the dom filesystem (everything between
lines 126-140).

+  return true;
+}
+
 bool DOMMediaStreamTrackToResource(
     PP_Instance instance,
     RendererPpapiHost* host,
@@ -239,11 +288,16 @@ bool ResourceConverterImpl::ToV8Value(const PP_Var& var,
   }
 
   // Convert to the appropriate type of resource host.
-  // TODO(mgiuca): Convert FileSystemHost resources into DOMFileSystem V8
-  // objects. (http://crbug.com/345158)
-  LOG(ERROR) << "The type of resource #" << resource_id
-             << " cannot be converted to a JavaScript object.";
-  return false;
+  if (resource_host->IsFileSystemHost()) {
+    return ResourceHostToDOMFileSystem(
+        static_cast<content::PepperFileSystemHost*>(resource_host),
+        context,
+        result);
+  } else {
+    LOG(ERROR) << "The type of resource #" << resource_id
+               << " cannot be converted to a JavaScript object.";
+    return false;
+  }
 }
 
 scoped_refptr<HostResourceVar> ResourceConverterImpl::CreateResourceVar(