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

content/renderer/pepper/pepper_file_chooser_host.cc

 // Copyright (c) 2012 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 "content/renderer/pepper/pepper_file_chooser_host.h"
 
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/renderer/renderer_ppapi_host.h"
-#include "content/renderer/pepper/ppb_file_ref_impl.h"
 #include "content/renderer/render_view_impl.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/host/dispatch_host_message.h"
 #include "ppapi/host/ppapi_host.h"
 #include "ppapi/proxy/ppapi_messages.h"
-#include "ppapi/proxy/ppb_file_ref_proxy.h"
 #include "third_party/WebKit/public/platform/WebCString.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebFileChooserCompletion.h"
 #include "third_party/WebKit/public/web/WebFileChooserParams.h"
 
 namespace content {
 
 class PepperFileChooserHost::CompletionHandler
     : public WebKit::WebFileChooserCompletion {
@@ skipping 47 matching lines @@
       display_name(display_name) {
 }
 
 
 PepperFileChooserHost::PepperFileChooserHost(
     RendererPpapiHost* host,
     PP_Instance instance,
     PP_Resource resource)
     : ResourceHost(host->GetPpapiHost(), instance, resource),
       renderer_ppapi_host_(host),
-      handler_(NULL) {
+      handler_(NULL),
+      weak_factory_(this) {
 }
 
 PepperFileChooserHost::~PepperFileChooserHost() {
 }
 
 int32_t PepperFileChooserHost::OnResourceMessageReceived(
     const IPC::Message& msg,
     ppapi::host::HostMessageContext* context) {
   IPC_BEGIN_MESSAGE_MAP(PepperFileChooserHost, msg)
     PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileChooser_Show, OnShow)
   IPC_END_MESSAGE_MAP()
   return PP_ERROR_FAILED;
 }
 
 void PepperFileChooserHost::StoreChosenFiles(
     const std::vector<ChosenFileInfo>& files) {
-  std::vector<ppapi::PPB_FileRef_CreateInfo> chosen_files;
+  num_pending_file_resources_ = files.size();
   for (size_t i = 0; i < files.size(); i++) {
 #if defined(OS_WIN)
     base::FilePath file_path(UTF8ToWide(files[i].path));
 #else
     base::FilePath file_path(files[i].path);
 #endif
-
-    PPB_FileRef_Impl* ref = PPB_FileRef_Impl::CreateExternal(
-        pp_instance(), file_path, files[i].display_name);
-    ppapi::PPB_FileRef_CreateInfo create_info;
-    ppapi::proxy::PPB_FileRef_Proxy::SerializeFileRef(ref->GetReference(),
-                                                      &create_info);
-    chosen_files.push_back(create_info);
+    IPC::Message create_msg =
+        PpapiHostMsg_FileRef_CreateExternal(file_path);

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

Is there any slicing problem with doing this? It seems like you could just
create it in the function parameter list below.

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

What do you mean? I don't understand.

[dmichael (off chromium), 2013/08/08 01:58:41]

"Slicing" is when you assign an object to a type that's its base class. It's
possible in those cases to lose data, and you certainly lose the dynamic type.
I.e., create_msg really *should* be a PpapiHostMsg_FileRef_CreateExternal, not
its base, IPC::Message. I think it probably works okay in practice here, since
all the data is really in the Pickle base class. But it still looks funny to me,
and *may* have some subtle problems.

I was suggesting just putting:
 PpapiHostMsg_FileRef_CreateExternal(file_path),
right in the parameter list on line 112 instead of using create_msg.

+    RendererPpapiHost::GetForPPInstance(pp_instance())->
+        CreateBrowserResourceHost(
+            pp_instance(),
+            create_msg,
+            base::Bind(&PepperFileChooserHost::DidCreateResourceHost,
+                       weak_factory_.GetWeakPtr(),
+                       file_path,
+                       files[i].display_name));
   }
 
-  reply_context_.params.set_result(
-      (chosen_files.size() > 0) ? PP_OK : PP_ERROR_USERCANCEL);
-  host()->SendReply(reply_context_,
-                    PpapiPluginMsg_FileChooser_ShowReply(chosen_files));
-
-  reply_context_ = ppapi::host::ReplyMessageContext();
-  handler_ = NULL;  // Handler deletes itself.
+  if (files.size() == 0) {
+    std::vector<ppapi::FileRef_CreateInfo> chosen_files;
+    reply_context_.params.set_result(PP_ERROR_USERCANCEL);
+    host()->SendReply(reply_context_,
+                      PpapiPluginMsg_FileChooser_ShowReply(chosen_files));
+    reply_context_ = ppapi::host::ReplyMessageContext();
+    handler_ = NULL;  // Handler deletes itself.
+  }
 }
 
 int32_t PepperFileChooserHost::OnShow(
     ppapi::host::HostMessageContext* context,
     bool save_as,
     bool open_multiple,
     const std::string& suggested_file_name,
     const std::vector<std::string>& accept_mime_types) {
   if (handler_)
     return PP_ERROR_INPROGRESS;  // Already pending.
@@ skipping 26 matching lines @@
   if (!render_view || !render_view->runFileChooser(params, handler_)) {
     delete handler_;
     handler_ = NULL;
     return PP_ERROR_NOACCESS;
   }
 
   reply_context_ = context->MakeReplyMessageContext();
   return PP_OK_COMPLETIONPENDING;
 }
 
+void PepperFileChooserHost::DidCreateResourceHost(
+    const base::FilePath& file_path,
+    const std::string& display_name,
+    int32_t id) {
+  num_pending_file_resources_--;
+  ppapi::FileRef_CreateInfo info;
+  info.file_system_type = PP_FILESYSTEMTYPE_EXTERNAL;
+  info.internal_path = std::string();
+  info.display_name = display_name;
+  info.pending_host_resource_id = id;

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

Suggestion: What do you think of adding a couple of constructors or static
"named" constructors, so for example you could do:
FileRef_CreateInfo::CreateExternal(display_name, id);

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

Done. (I had a function that was pretty close already. I just modified it to
take a display_name parameter also)

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

Any reason you aren't using it here?

+  chosen_files_.push_back(info);
+  if (num_pending_file_resources_ == 0) {
+    reply_context_.params.set_result(PP_OK);
+    host()->SendReply(reply_context_,
+                      PpapiPluginMsg_FileChooser_ShowReply(chosen_files_));
+    reply_context_ = ppapi::host::ReplyMessageContext();
+    handler_ = NULL;  // Handler deletes itself.
+  }
+}
+
 }  // namespace content