Implement Pepper proxy for PPB_DirectoryReader

content/renderer/pepper/pepper_directory_reader_host.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 "content/renderer/pepper/pepper_directory_reader_host.h"
+
+#include "base/compiler_specific.h"
+#include "base/utf_string_conversions.h"
+#include "content/public/renderer/renderer_ppapi_host.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/shared_impl/file_type_conversion.h"
+#include "ppapi/shared_impl/ppb_file_ref_shared.h"
+#include "ppapi/thunk/enter.h"
+#include "webkit/fileapi/file_system_callback_dispatcher.h"
+#include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
+#include "webkit/plugins/ppapi/ppb_file_ref_impl.h"
+#include "webkit/plugins/ppapi/ppb_file_system_impl.h"
+#include "webkit/plugins/ppapi/resource_helper.h"
+
+using ppapi::thunk::EnterResource;
+using ppapi::thunk::PPB_FileRef_API;
+using webkit::ppapi::PPB_FileRef_Impl;
+
+namespace content {
+
+namespace {
+
+std::string FilePathStringToUTF8String(const FilePath::StringType& str) {
+#if defined(OS_WIN)
+  return WideToUTF8(str);
+#elif defined(OS_POSIX)
+  return str;
+#else
+#error "Unsupported platform."
+#endif
+}
+
+FilePath::StringType UTF8StringToFilePathString(const std::string& str) {
+#if defined(OS_WIN)
+  return UTF8ToWide(str);
+#elif defined(OS_POSIX)
+  return str;
+#else
+#error "Unsupported platform."
+#endif
+}
+
+class ReadDirectoryCallback : public fileapi::FileSystemCallbackDispatcher {
+ public:
+  typedef base::Callback<void (const PepperDirectoryReaderHost::Entries&,
+                               bool, int32_t)>
+      OnReadDirectoryCallback;
+
+  explicit ReadDirectoryCallback(OnReadDirectoryCallback callback)

[yzshen1, 2013/01/31 08:50:54]

Please use const ref.

[nhiroki, 2013/02/01 08:19:47]

Done.

+      : callback_(callback) {}
+  virtual ~ReadDirectoryCallback() {}
+
+  virtual void DidSucceed() OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidReadMetadata(const base::PlatformFileInfo& file_info,
+                               const FilePath& platform_path) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidReadDirectory(
+      const std::vector<base::FileUtilProxy::Entry>& entries,
+      bool has_more) OVERRIDE {
+    callback_.Run(entries, has_more, PP_OK);
+  }
+
+  virtual void DidOpenFileSystem(const std::string& name,
+                                 const GURL& root) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidFail(base::PlatformFileError error) OVERRIDE {
+    callback_.Run(PepperDirectoryReaderHost::Entries(),
+                  false,
+                  ppapi::PlatformFileErrorToPepperError(error));
+  }
+
+  virtual void DidWrite(int64 bytes, bool complete) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidOpenFile(base::PlatformFile file) OVERRIDE {
+    NOTREACHED();
+  }
+
+ private:
+  OnReadDirectoryCallback callback_;
+};
+
+}  // namespace
+
+PepperDirectoryReaderHost::PepperDirectoryReaderHost(
+    RendererPpapiHost* host,
+    PP_Instance instance,
+    PP_Resource resource)
+    : ResourceHost(host->GetPpapiHost(), instance, resource),
+      renderer_ppapi_host_(host),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+}
+
+PepperDirectoryReaderHost::~PepperDirectoryReaderHost() {
+}
+
+int32_t PepperDirectoryReaderHost::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    ppapi::host::HostMessageContext* context) {
+  IPC_BEGIN_MESSAGE_MAP(PepperDirectoryReaderHost, msg)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(
+        PpapiHostMsg_DirectoryReader_GetEntries, OnGetEntries)
+  IPC_END_MESSAGE_MAP()
+  return PP_ERROR_FAILED;
+}
+
+int32_t PepperDirectoryReaderHost::OnGetEntries(
+  ppapi::host::HostMessageContext* host_context,

[yzshen1, 2013/01/31 08:50:54]

wrong indent.

[nhiroki, 2013/02/01 08:19:47]

Done.

+  ppapi::HostResource resource) {
+  reply_context_ = host_context->MakeReplyMessageContext();
+
+  EnterResource<PPB_FileRef_API> enter(resource.host_resource(), true);
+  if (enter.failed())
+    return PP_ERROR_FAILED;
+  directory_ref_ = static_cast<PPB_FileRef_Impl*>(enter.object());
+
+  if (directory_ref_->GetFileSystemType() == PP_FILESYSTEMTYPE_EXTERNAL)
+    return PP_ERROR_FAILED;
+
+  webkit::ppapi::PluginInstance* plugin_instance =
+      renderer_ppapi_host_->GetPluginInstance(pp_instance());
+  if (!plugin_instance)
+    return PP_ERROR_FAILED;
+
+  if (!plugin_instance->delegate()->ReadDirectory(
+          directory_ref_->GetFileSystemURL(),
+          new ReadDirectoryCallback(
+              base::Bind(&PepperDirectoryReaderHost::OnReadDirectory,
+                         weak_factory_.GetWeakPtr()))))
+    return PP_ERROR_FAILED;
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PepperDirectoryReaderHost::OnReadDirectory(const Entries& entries,
+                                                bool has_more,
+                                                int32_t result) {
+  DCHECK(!entries.empty() || !has_more);
+

[yzshen1, 2013/01/31 08:50:54]

Is it better to simplify the code like this?

if (result == PP_OK && !AddNewEntries(entries, has_more))
  result = PP_ERROR_FAILED;
SendGetEntriesReply(result);

[nhiroki, 2013/02/01 08:19:47]

Done.

+  if (result != PP_OK) {
+    SendGetEntriesReply(result);
+    return;
+  }
+
+  if (!AddNewEntries(entries, has_more)) {
+    SendGetEntriesReply(PP_ERROR_FAILED);
+    return;
+  }
+
+  SendGetEntriesReply(result);
+}
+
+bool PepperDirectoryReaderHost::AddNewEntries(const Entries& entries,
+                                              bool has_more) {
+  std::string dir_path = directory_ref_->GetCreateInfo().path;
+  if (dir_path[dir_path.size() - 1] != '/')
+    dir_path += '/';
+  FilePath::StringType dir_file_path = UTF8StringToFilePathString(dir_path);
+  DCHECK(!entries.empty());

[yzshen1, 2013/01/31 08:50:54]

It is possible, according to the logic of the previous method, that
entries.empty() && !has_more. Right?

[nhiroki, 2013/02/01 08:19:47]

It's an intended case. Removed this check.

+
+  for (Entries::const_iterator it = entries.begin();

[yzshen1, 2013/01/31 08:50:54]

The ref count handling is wrong in the code below. I recommend you to read
resource_tracker and plugin_resource_tracker, and always consider ref counting
carefully when writing Pepper code.

- There are two kinds of ref here: ref count of the resource impl object; ref
count of PP_Resource.
- When we send a HostResource to the plugin side, we should add 1 ref for
PP_Resource on behalf of the plugin side. (And the plugin side is responsible to
notify the host side when it is done with the ref.)
- GetReference() on line 191 adds 1 PP_Resource ref. If we don't send it to the
plugin side (e.g., we hit line 188 or 196), it is leaked. (Btw, GetCreateInfo()
on line 199 doesn't add ref.)
- There is no need to EnterResource on line 191, PPB_FileRef_Impl is a subclass
of PPB_FileRef_API.

[nhiroki, 2013/02/01 08:19:47]

Thanks for the explanation. I read related sourcecodes and realized my mistakes.
I fixed here to increment just one refcount on behalf of the plugin side and to
take care of breaking out of the loop.

In my understanding, PluginResourceTracker will notify the host side when the
refcount of the plugin's PP_Resource becomes 0, so I don't have to decrement the
refcount of host's PP_Resource explicitly. Is this right?

[yzshen1, 2013/02/01 23:57:47]

Correct.

Thanks! :)

+       it != entries.end();
+       ++it) {
+    scoped_refptr<PPB_FileRef_Impl> file_ref = PPB_FileRef_Impl::CreateInternal(
+        directory_ref_->file_system()->pp_resource(),
+        FilePathStringToUTF8String(dir_file_path + it->name));
+    file_refs_.push_back(file_ref);
+
+    if (!file_ref) {
+      host_resources_.clear();
+      file_types_.clear();
+      file_refs_.clear();
+      return false;
+    }
+
+    EnterResource<PPB_FileRef_API> enter(file_ref->GetReference(), false);
+    if (enter.failed()) {
+      host_resources_.clear();
+      file_types_.clear();
+      file_refs_.clear();
+      return false;
+    }
+
+    host_resources_.push_back(enter.object()->GetCreateInfo());
+    file_types_.push_back(it->is_directory ?
+                          PP_FILETYPE_DIRECTORY : PP_FILETYPE_REGULAR);
+  }
+
+  return true;
+}
+
+void PepperDirectoryReaderHost::SendGetEntriesReply(int32_t result) {
+  reply_context_.params.set_result(result);
+  host()->SendReply(
+      reply_context_,
+      PpapiPluginMsg_DirectoryReader_GetEntriesReply(host_resources_,
+                                                     file_types_));
+  host_resources_.clear();
+  file_types_.clear();
+}
+
+}  // namespace content