Move DirectoryReader::ReadEntries to FileRef::ReadDirectoryEntries

webkit/plugins/ppapi/file_callbacks.cc

Index: webkit/plugins/ppapi/file_callbacks.cc
diff --git a/webkit/plugins/ppapi/file_callbacks.cc b/webkit/plugins/ppapi/file_callbacks.cc
index fedf583dc9b572e94f7afd95fba8fba6c3df0fe3..f9bc853f21145e2074e9a8e12a62d09f37571bad 100644
--- a/webkit/plugins/ppapi/file_callbacks.cc
+++ b/webkit/plugins/ppapi/file_callbacks.cc
@@ -5,14 +5,17 @@
 #include "webkit/plugins/ppapi/file_callbacks.h"
 
 #include "base/logging.h"
+#include "base/utf_string_conversions.h"
 #include "ppapi/c/pp_file_info.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_file_system.h"
 #include "ppapi/shared_impl/file_type_conversion.h"
+#include "ppapi/shared_impl/ppb_file_ref_shared.h"
 #include "ppapi/shared_impl/time_conversion.h"
 #include "ppapi/shared_impl/tracked_callback.h"
 #include "webkit/fileapi/file_system_types.h"
 #include "webkit/plugins/ppapi/plugin_module.h"
+#include "webkit/plugins/ppapi/ppb_file_ref_impl.h"
 
 using ppapi::Resource;
 using ppapi::TimeToPPTime;
@@ -21,13 +24,38 @@ using ppapi::TrackedCallback;
 namespace webkit {
 namespace ppapi {
 
+namespace {
+
+std::string FilePathStringToUTF8String(const base::FilePath::StringType& str) {

[teravest, 2013/05/02 17:32:10]

This is a weird place to have platform-specific treatment of file paths. Can you
add a comment about what's going on here?

[hamaji, 2013/05/02 18:49:40]

Removed, by using fileapi::FilePathToString.

+#if defined(OS_WIN)
+  return WideToUTF8(str);
+#elif defined(OS_POSIX)
+  return str;
+#else
+#error "Unsupported platform."
+#endif
+}
+
+base::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
+}
+
+}  // namespace
+
 FileCallbacks::FileCallbacks(
     Resource* resource,
     scoped_refptr<TrackedCallback> callback,
     PP_FileInfo* info)
     : callback_(callback),
       info_(info),
-      file_system_type_(PP_FILESYSTEMTYPE_INVALID) {
+      file_system_type_(PP_FILESYSTEMTYPE_INVALID),
+      read_entries_params_(NULL) {
 }
 
 FileCallbacks::FileCallbacks(
@@ -37,7 +65,18 @@ FileCallbacks::FileCallbacks(
     PP_FileSystemType file_system_type)
     : callback_(callback),
       info_(info),
-      file_system_type_(file_system_type) {
+      file_system_type_(file_system_type),
+      read_entries_params_(NULL) {
+}
+
+FileCallbacks::FileCallbacks(
+    ::ppapi::Resource* resource,
+    scoped_refptr< ::ppapi::TrackedCallback> callback,
+    base::internal::OwnedWrapper<ReadEntriesParams> params)
+    : callback_(callback),
+      info_(NULL),
+      file_system_type_(PP_FILESYSTEMTYPE_INVALID),
+      read_entries_params_(params) {
 }
 
 FileCallbacks::~FileCallbacks() {}
@@ -77,7 +116,43 @@ void FileCallbacks::DidCreateSnapshotFile(
 
 void FileCallbacks::DidReadDirectory(
     const std::vector<base::FileUtilProxy::Entry>& entries, bool has_more) {
-  NOTREACHED();
+  if (callback_->completed())
+    return;
+
+  // The current filesystem backend always returns false.
+  DCHECK(!has_more);
+
+  PPB_FileRef_Impl* dir_ref = read_entries_params_.get()->dir_ref;
+  std::vector< ::ppapi::PPB_FileRef_CreateInfo>* files

[teravest, 2013/05/02 17:32:10]

'=' should be on the first line here.

[hamaji, 2013/05/02 18:49:40]

Done.

+      = read_entries_params_.get()->files;
+  std::vector<PP_FileType>* file_types =
+      read_entries_params_.get()->file_types;
+  DCHECK(dir_ref);
+  DCHECK(files);
+  DCHECK(file_types);
+
+  std::string dir_path = dir_ref->GetCreateInfo().path;
+  if (dir_path[dir_path.size() - 1] != '/')
+    dir_path += '/';
+  base::FilePath::StringType dir_file_path =
+      UTF8StringToFilePathString(dir_path);

[teravest, 2013/05/02 17:32:10]

I feel like there has to be a better way to handle the string manipulation of
these paths. Why the converstion back and forth to FilePathString?

[hamaji, 2013/05/02 18:49:40]

I just moved this code from pepper_directory_reader_host.cc, but I agree. Now we
have only one conversion.

+
+  files->resize(entries.size());
+  file_types->resize(entries.size());
+  for (size_t i = 0; i < entries.size(); ++i) {
+    const base::FileUtilProxy::Entry& entry = entries[i];
+    scoped_refptr<PPB_FileRef_Impl> file_ref(PPB_FileRef_Impl::CreateInternal(
+        dir_ref->pp_instance(),
+        dir_ref->file_system_resource(),
+        FilePathStringToUTF8String(dir_file_path + entry.name)));
+    (*files)[i] = file_ref->GetCreateInfo();
+    (*file_types)[i] =
+        entry.is_directory ? PP_FILETYPE_DIRECTORY : PP_FILETYPE_REGULAR;
+    // Add a ref count on behalf of the plugin side.
+    file_ref->GetReference();
+  }
+
+  callback_->Run(PP_OK);
 }
 
 void FileCallbacks::DidOpenFileSystem(const std::string&,