Pepper: Move FileRef to the "new" resource proxy.

ppapi/proxy/file_chooser_resource_unittest.cc

Index: ppapi/proxy/file_chooser_resource_unittest.cc
diff --git a/ppapi/proxy/file_chooser_resource_unittest.cc b/ppapi/proxy/file_chooser_resource_unittest.cc
index a5801eed161c00a9055c220832fc529aee08d9b3..2c44336b5dbad6f65192331191c8b46ddf3b3fbf 100644
--- a/ppapi/proxy/file_chooser_resource_unittest.cc
+++ b/ppapi/proxy/file_chooser_resource_unittest.cc
@@ -5,6 +5,7 @@
 #include "base/message_loop/message_loop.h"
 #include "ppapi/c/dev/ppb_file_chooser_dev.h"
 #include "ppapi/c/pp_errors.h"
+#include "ppapi/c/ppb_file_ref.h"
 #include "ppapi/proxy/file_chooser_resource.h"
 #include "ppapi/proxy/locking_resource_releaser.h"
 #include "ppapi/proxy/ppapi_messages.h"
@@ -93,11 +94,13 @@ TEST_F(FileChooserResourceTest, Show) {
   // Synthesize a response with one file ref in it. Note that it must have a
   // host resource value set or deserialization will fail. Since there isn't
   // actually a host, this can be whatever we want.
-  std::vector<PPB_FileRef_CreateInfo> create_info_array;
-  PPB_FileRef_CreateInfo create_info;
-  create_info.resource.SetHostResource(pp_instance(), 123);
-  create_info.path = "foo/bar";
-  create_info.name = "baz";
+  std::vector<FileRef_CreateInfo> create_info_array;
+  FileRef_CreateInfo create_info;
+  // In practice, we would be dealing with files on an external filesystem, but
+  // plugins aren't allowed to create resources there, so we use LOCALTEMPORARY
+  // here instead.
+  create_info.file_system_type = PP_FILESYSTEMTYPE_LOCALTEMPORARY;

[dmichael (off chromium), 2013/08/07 22:19:13]

I'm not sure I understand why you need to do this...  you're synthesizing the
message, so it seems like you're totally bypassing any checks about creating
external FileRefs from the plugin...

[teravest, 2013/08/08 00:50:06]

The problem is that an external FileRef always needs a pending_resource_id to
attach to, which we don't have in this case. We could try to make that work, but
I figured it would be more complicated than using a LOCALTEMPORARY file in the
test instead.

[dmichael (off chromium), 2013/08/08 21:24:03]

Hmm. It feels quite weird to unit test a file type that FileChooser will never
actually get. Like...  I could imagine us having a DCHECK somewhere in
FileChooserResource that the file system is external.

I don't know that I would want to hold your CL up on this detail. But maybe you
can stick in a TODO or file a bug to do it with the right kind of file. Maybe
even just leave the test mostly alone (aside from changing CreateInfo types) and
disable it with a filed bug.

It seems this is likely to come up elsewhere, and we might want to figure out
how to test it. Maybe just stick a fake pending resource id in, and perhaps look
to see if we send an appropriate AttachToPendingResource message (assuming the
Send goes in to a TestSink).

+  create_info.internal_path = "/foo/bar";
   create_info_array.push_back(create_info);
   ASSERT_TRUE(plugin_dispatcher()->OnMessageReceived(
       PpapiPluginMsg_ResourceReply(reply_params,
@@ -108,21 +111,20 @@ TEST_F(FileChooserResourceTest, Show) {
   LockingResourceReleaser dest_deletor(dest[0]);  // Ensure it's cleaned up.
 
   const PPB_FileRef_1_0* file_ref_iface = thunk::GetPPB_FileRef_1_0_Thunk();
-  EXPECT_EQ(PP_FILESYSTEMTYPE_EXTERNAL,
+  EXPECT_EQ(PP_FILESYSTEMTYPE_LOCALTEMPORARY,
             file_ref_iface->GetFileSystemType(dest[0]));
 
   PP_Var name_var(file_ref_iface->GetName(dest[0]));
   {
     ProxyAutoLock lock;
     ScopedPPVar release_name_var(ScopedPPVar::PassRef(), name_var);
-    EXPECT_VAR_IS_STRING(create_info.name, name_var);
+    EXPECT_VAR_IS_STRING("bar", name_var);
   }
-  // Path should be undefined since it's external filesystem.
   PP_Var path_var(file_ref_iface->GetPath(dest[0]));
   {
     ProxyAutoLock lock;
     ScopedPPVar release_path_var(ScopedPPVar::PassRef(), path_var);
-    EXPECT_EQ(PP_VARTYPE_UNDEFINED, path_var.type);
+    EXPECT_VAR_IS_STRING(create_info.internal_path, path_var);
   }
 }