Allow net::DirectoryLister to be used to recursively list the directory, and...

Allow net::DirectoryLister to be used to recursively list the directory, and add a FULL_PATH sort option.

BUG=41762
TEST=unit test

Review URL: http://codereview.chromium.org/3175023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57005 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/dom_ui/filebrowse_ui.cc

@@ -107,7 +107,8 @@ class FilebrowseHandler : public net::DirectoryLister::DirectoryListerDelegate,
   void Init();
 
   // DirectoryLister::DirectoryListerDelegate methods:
-  virtual void OnListFile(const file_util::FileEnumerator::FindInfo& data);
+  virtual void OnListFile(
+      const net::DirectoryLister::DirectoryListerData& data);
   virtual void OnListDone(int error);
 
   // DOMMessageHandler implementation.
@@ -766,6 +767,7 @@ void FilebrowseHandler::GetChildrenForPath(FilePath& path, bool is_refresh) {
   }
   if (currentpath_ == default_download_path) {
     lister_ = new net::DirectoryLister(currentpath_,
+                                       false,
                                        net::DirectoryLister::DATE,
                                        this);
   } else {
@@ -785,13 +787,13 @@ void FilebrowseHandler::HandleGetChildren(const ListValue* args) {
 }
 
 void FilebrowseHandler::OnListFile(
-    const file_util::FileEnumerator::FindInfo& data) {
+    const net::DirectoryLister::DirectoryListerData& data) {
 #if defined(OS_WIN)
-  if (data.dwFileAttributes & FILE_ATTRIBUTE_HIDDEN) {
+  if (data.info.dwFileAttributes & FILE_ATTRIBUTE_HIDDEN) {
     return;
   }
 #elif defined(OS_POSIX)
-  if (data.filename[0] == '.') {
+  if (data.info.filename[0] == '.') {
     return;
   }
 #endif
@@ -799,18 +801,18 @@ void FilebrowseHandler::OnListFile(
   DictionaryValue* file_value = new DictionaryValue();
 
 #if defined(OS_WIN)
-  int64 size = (static_cast<int64>(data.nFileSizeHigh) << 32) |
-      data.nFileSizeLow;
-  file_value->SetString(kPropertyTitle, data.cFileName);
+  int64 size = (static_cast<int64>(data.info.nFileSizeHigh) << 32) |
+      data.info.nFileSizeLow;
+  file_value->SetString(kPropertyTitle, data.info.cFileName);
   file_value->SetString(kPropertyPath,
-                        currentpath_.Append(data.cFileName).value());
+                        currentpath_.Append(data.info.cFileName).value());
   file_value->SetBoolean(kPropertyDirectory,
-      (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? true : false);
+      (data.info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? true : false);
 #elif defined(OS_POSIX)
-  file_value->SetString(kPropertyTitle, data.filename);
+  file_value->SetString(kPropertyTitle, data.info.filename);
   file_value->SetString(kPropertyPath,
-                        currentpath_.Append(data.filename).value());
-  file_value->SetBoolean(kPropertyDirectory, S_ISDIR(data.stat.st_mode));
+                        currentpath_.Append(data.info.filename).value());
+  file_value->SetBoolean(kPropertyDirectory, S_ISDIR(data.info.stat.st_mode));
 #endif
   filelist_value_->Append(file_value);
 }

chrome/browser/dom_ui/slideshow_ui.cc

@@ -69,7 +69,8 @@ class SlideshowHandler : public net::DirectoryLister::DirectoryListerDelegate,
   void Init();
 
   // DirectoryLister::DirectoryListerDelegate methods:
-  virtual void OnListFile(const file_util::FileEnumerator::FindInfo& data);
+  virtual void OnListFile(
+      const net::DirectoryLister::DirectoryListerData& data);
   virtual void OnListDone(int error);
 
   // DOMMessageHandler implementation.
@@ -228,21 +229,21 @@ bool SlideshowHandler::PathIsImageFile(const char* filename) {
 }
 
 void SlideshowHandler::OnListFile(
-    const file_util::FileEnumerator::FindInfo& data) {
+    const net::DirectoryLister::DirectoryListerData& data) {
 #if defined(OS_CHROMEOS)
-  if (data.filename[0] == '.') {
+  if (data.info.filename[0] == '.') {
     return;
   }
-  if (!PathIsImageFile(data.filename.c_str())) {
+  if (!PathIsImageFile(data.info.filename.c_str())) {
     return;
   }
 
   DictionaryValue* file_value = new DictionaryValue();
 
-  file_value->SetString(kPropertyTitle, data.filename);
+  file_value->SetString(kPropertyTitle, data.info.filename);
   file_value->SetString(kPropertyPath,
-                        currentpath_.Append(data.filename).value());
-  file_value->SetBoolean(kPropertyDirectory, S_ISDIR(data.stat.st_mode));
+                        currentpath_.Append(data.info.filename).value());
+  file_value->SetBoolean(kPropertyDirectory, S_ISDIR(data.info.stat.st_mode));
   filelist_value_->Append(file_value);
   std::string val;
   file_value->GetString(kPropertyTitle, &val);

net/base/directory_lister.cc

@@ -17,8 +17,9 @@ namespace net {
 
 static const int kFilesPerEvent = 8;
 
+// A task which is used to signal the delegate asynchronously.
 class DirectoryDataEvent : public Task {
- public:
+public:
   explicit DirectoryDataEvent(DirectoryLister* d) : lister(d), error(0) {
     // Allocations of the FindInfo aren't super cheap, so reserve space.
     data.reserve(64);
@@ -33,70 +34,85 @@ class DirectoryDataEvent : public Task {
   }
 
   scoped_refptr<DirectoryLister> lister;
-  std::vector<file_util::FileEnumerator::FindInfo> data;
+  std::vector<DirectoryLister::DirectoryListerData> data;
   int error;
 };
 
-// Comparator for sorting FindInfo's. This uses the locale aware filename
+// Comparator for sorting lister results. This uses the locale aware filename
 // comparison function on the filenames for sorting in the user's locale.
-static bool CompareFindInfoAlpha(const file_util::FileEnumerator::FindInfo& a,
-                                 const file_util::FileEnumerator::FindInfo& b) {
+// Static.
+bool DirectoryLister::CompareAlphaDirsFirst(const DirectoryListerData& a,
+                                            const DirectoryListerData& b) {
   // Parent directory before all else.
-  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a)))
+  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
     return true;
-  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b)))
+  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b.info)))
     return false;
 
   // Directories before regular files.
-  bool a_is_directory = file_util::FileEnumerator::IsDirectory(a);
-  bool b_is_directory = file_util::FileEnumerator::IsDirectory(b);
+  bool a_is_directory = file_util::FileEnumerator::IsDirectory(a.info);
+  bool b_is_directory = file_util::FileEnumerator::IsDirectory(b.info);
   if (a_is_directory != b_is_directory)
     return a_is_directory;
 
   return file_util::LocaleAwareCompareFilenames(
-      file_util::FileEnumerator::GetFilename(a),
-      file_util::FileEnumerator::GetFilename(b));
+      file_util::FileEnumerator::GetFilename(a.info),
+      file_util::FileEnumerator::GetFilename(b.info));
 }
 
-static bool CompareFindInfoDate(const file_util::FileEnumerator::FindInfo& a,
-                                const file_util::FileEnumerator::FindInfo& b) {
+// Static.
+bool DirectoryLister::CompareDate(const DirectoryListerData& a,
+                                  const DirectoryListerData& b) {
   // Parent directory before all else.
-  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a)))
+  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
     return true;
-  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b)))
+  if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b.info)))
     return false;
 
   // Directories before regular files.
-  bool a_is_directory = file_util::FileEnumerator::IsDirectory(a);
-  bool b_is_directory = file_util::FileEnumerator::IsDirectory(b);
+  bool a_is_directory = file_util::FileEnumerator::IsDirectory(a.info);
+  bool b_is_directory = file_util::FileEnumerator::IsDirectory(b.info);
   if (a_is_directory != b_is_directory)
     return a_is_directory;
 #if defined(OS_POSIX)
-  return a.stat.st_mtime > b.stat.st_mtime;
+  return a.info.stat.st_mtime > b.info.stat.st_mtime;
 #elif defined(OS_WIN)
-  if (a.ftLastWriteTime.dwHighDateTime == b.ftLastWriteTime.dwHighDateTime) {
-    return a.ftLastWriteTime.dwLowDateTime > b.ftLastWriteTime.dwLowDateTime;
+  if (a.info.ftLastWriteTime.dwHighDateTime ==
+      b.info.ftLastWriteTime.dwHighDateTime) {
+    return a.info.ftLastWriteTime.dwLowDateTime >
+           b.info.ftLastWriteTime.dwLowDateTime;
   } else {
-    return a.ftLastWriteTime.dwHighDateTime > b.ftLastWriteTime.dwHighDateTime;
+    return a.info.ftLastWriteTime.dwHighDateTime >
+           b.info.ftLastWriteTime.dwHighDateTime;
   }
 #endif
 }
 
+// Comparator for sorting find result by paths. This uses the locale-aware
+// comparison function on the filenames for sorting in the user's locale.
+// Static.
+bool DirectoryLister::CompareFullPath(const DirectoryListerData& a,
+                                      const DirectoryListerData& b) {
+  return file_util::LocaleAwareCompareFilenames(a.path, b.path);
+}
 
 DirectoryLister::DirectoryLister(const FilePath& dir,
                                  DirectoryListerDelegate* delegate)
     : dir_(dir),
+      recursive_(false),
       delegate_(delegate),
-      sort_(DEFAULT),
+      sort_(ALPHA_DIRS_FIRST),
       message_loop_(NULL),
       thread_(kNullThreadHandle) {
   DCHECK(!dir.value().empty());
 }
 
 DirectoryLister::DirectoryLister(const FilePath& dir,
+                                 bool recursive,
                                  SORT_TYPE sort,
                                  DirectoryListerDelegate* delegate)
     : dir_(dir),
+      recursive_(false),
       delegate_(delegate),
       sort_(sort),
       message_loop_(NULL),
@@ -145,15 +161,21 @@ void DirectoryLister::ThreadMain() {
     Release();
     return;
   }
-  file_util::FileEnumerator file_enum(dir_, false,
-      static_cast<file_util::FileEnumerator::FILE_TYPE>(
-      file_util::FileEnumerator::FILES |
-      file_util::FileEnumerator::DIRECTORIES |
-      file_util::FileEnumerator::INCLUDE_DOT_DOT));
 
-  while (!canceled_.IsSet() && !(file_enum.Next().value().empty())) {
-    e->data.push_back(file_util::FileEnumerator::FindInfo());
-    file_enum.GetFindInfo(&e->data[e->data.size() - 1]);
+  int types = file_util::FileEnumerator::FILES |
+              file_util::FileEnumerator::DIRECTORIES;
+  if (!recursive_)
+    types |= file_util::FileEnumerator::INCLUDE_DOT_DOT;
+
+  file_util::FileEnumerator file_enum(dir_, recursive_,
+      static_cast<file_util::FileEnumerator::FILE_TYPE>(types));
+
+  FilePath path;
+  while (!canceled_.IsSet() && !(path = file_enum.Next()).empty()) {
+    DirectoryListerData data;
+    file_enum.GetFindInfo(&data.info);
+    data.path = path;
+    e->data.push_back(data);
 
     /* TODO(brettw) bug 24107: It would be nice to send incremental updates.
        We gather them all so they can be sorted, but eventually the sorting
@@ -169,11 +191,14 @@ void DirectoryLister::ThreadMain() {
   if (!e->data.empty()) {
     // Sort the results. See the TODO above (this sort should be removed and we
     // should do it from JS).
-    if (sort_ == DATE) {
-      std::sort(e->data.begin(), e->data.end(), CompareFindInfoDate);
-    } else {
-      std::sort(e->data.begin(), e->data.end(), CompareFindInfoAlpha);
-    }
+    if (sort_ == DATE)
+      std::sort(e->data.begin(), e->data.end(), CompareDate);
+    else if (sort_ == FULL_PATH)
+      std::sort(e->data.begin(), e->data.end(), CompareFullPath);
+    else if (sort_ == ALPHA_DIRS_FIRST)
+      std::sort(e->data.begin(), e->data.end(), CompareAlphaDirsFirst);
+    else
+      DCHECK_EQ(NO_SORT, sort_);
 
     message_loop_->PostTask(FROM_HERE, e);
     e = new DirectoryDataEvent(this);
@@ -184,8 +209,8 @@ void DirectoryLister::ThreadMain() {
   message_loop_->PostTask(FROM_HERE, e);
 }
 
-void DirectoryLister::OnReceivedData(
-    const file_util::FileEnumerator::FindInfo* data, int count) {
+void DirectoryLister::OnReceivedData(const DirectoryListerData* data,
+                                     int count) {
   // Since the delegate can clear itself during the OnListFile callback, we
   // need to null check it during each iteration of the loop.  Similarly, it is
   // necessary to check the canceled_ flag to avoid sending data to a delegate

net/base/directory_lister.h

@@ -6,11 +6,14 @@
 #define NET_BASE_DIRECTORY_LISTER_H_
 #pragma once
 
+#include <vector>
+
 #include "base/cancellation_flag.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/platform_thread.h"
 #include "base/ref_counted.h"
+#include "base/task.h"
 
 class MessageLoop;
 
@@ -26,27 +29,42 @@ namespace net {
 class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
                         public PlatformThread::Delegate {
  public:
+  // Represents one file found.
+  struct DirectoryListerData {
+    file_util::FileEnumerator::FindInfo info;
+    FilePath path;
+  };
+
   // Implement this class to receive directory entries.
   class DirectoryListerDelegate {
    public:
-    virtual void OnListFile(
-        const file_util::FileEnumerator::FindInfo& data) = 0;
+    // Called for each file found by the lister.
+    virtual void OnListFile(const DirectoryListerData& data) = 0;
+
+    // Called when the listing is complete.
     virtual void OnListDone(int error) = 0;
 
    protected:
     virtual ~DirectoryListerDelegate() {}
   };
 
+  // Sort options
+  // ALPHA_DIRS_FIRST is the default sort :
+  //   directories first in name order, then files by name order
+  // FULL_PATH sorts by paths as strings, ignoring files v. directories
+  // DATE sorts by last modified date
   enum SORT_TYPE {
-    DEFAULT,
+    NO_SORT,
     DATE,
-    ALPHA
+    ALPHA_DIRS_FIRST,
+    FULL_PATH
   };
 
   DirectoryLister(const FilePath& dir,
                   DirectoryListerDelegate* delegate);
 
   DirectoryLister(const FilePath& dir,
+                  bool recursive,
                   SORT_TYPE sort,
                   DirectoryListerDelegate* delegate);
 
@@ -70,13 +88,21 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
   friend class base::RefCountedThreadSafe<DirectoryLister>;
   friend class DirectoryDataEvent;
 
+  // Comparison methods for sorting, chosen based on |sort_|.
+  static bool CompareAlphaDirsFirst(const DirectoryListerData& a,
+                                    const DirectoryListerData& b);
+  static bool CompareDate(const DirectoryListerData& a,
+                          const DirectoryListerData& b);
+  static bool CompareFullPath(const DirectoryListerData& a,
+                              const DirectoryListerData& b);
+
   ~DirectoryLister();
 
-  void OnReceivedData(const file_util::FileEnumerator::FindInfo* data,
-                      int count);
+  void OnReceivedData(const DirectoryListerData* data, int count);
   void OnDone(int error);
 
   FilePath dir_;
+  bool recursive_;
   DirectoryListerDelegate* delegate_;
   SORT_TYPE sort_;
   MessageLoop* message_loop_;

net/base/directory_lister_unittest.cc

@@ -5,6 +5,7 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/i18n/file_util_icu.h"
+#include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "net/base/directory_lister.h"
@@ -17,14 +18,32 @@ class DirectoryListerTest : public testing::Test {};
 
 class ListerDelegate : public net::DirectoryLister::DirectoryListerDelegate {
  public:
-  ListerDelegate() : error_(-1) {
+  explicit ListerDelegate(bool recursive) : error_(-1), recursive_(recursive) {
   }
-  void OnListFile(const file_util::FileEnumerator::FindInfo& data) {
-    file_list_.push_back(data);
+  void OnListFile(const net::DirectoryLister::DirectoryListerData& data) {
+    file_list_.push_back(data.info);
+    paths_.push_back(data.path);
   }
   void OnListDone(int error) {
     error_ = error;
     MessageLoop::current()->Quit();
+    if (recursive_)
+      CheckRecursiveSort();
+    else
+      CheckSort();
+  }
+  void CheckRecursiveSort() {
+    // Check that we got files in the right order.
+    if (!file_list_.empty()) {
+      for (size_t previous = 0, current = 1;
+           current < file_list_.size();
+           previous++, current++) {
+        EXPECT_TRUE(file_util::LocaleAwareCompareFilenames(
+            paths_[previous], paths_[current]));
+      }
+    }
+  }
+  void CheckSort() {
     // Check that we got files in the right order.
     if (!file_list_.empty()) {
       for (size_t previous = 0, current = 1;
@@ -48,14 +67,16 @@ class ListerDelegate : public net::DirectoryLister::DirectoryListerDelegate {
   int error() const { return error_; }
  private:
   int error_;
+  bool recursive_;
   std::vector<file_util::FileEnumerator::FindInfo> file_list_;
+  std::vector<FilePath> paths_;
 };
 
 TEST(DirectoryListerTest, BigDirTest) {
   FilePath path;
   ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &path));
 
-  ListerDelegate delegate;
+  ListerDelegate delegate(false);
   scoped_refptr<net::DirectoryLister> lister =
       new net::DirectoryLister(path, &delegate);
 
@@ -66,11 +87,29 @@ TEST(DirectoryListerTest, BigDirTest) {
   EXPECT_EQ(delegate.error(), net::OK);
 }
 
+TEST(DirectoryListerTest, BigDirRecursiveTest) {
+  FilePath path;
+  ASSERT_TRUE(PathService::Get(base::DIR_EXE, &path));
+
+  ListerDelegate delegate(true);
+  scoped_refptr<net::DirectoryLister> lister =
+      new net::DirectoryLister(path,
+                               true,
+                               net::DirectoryLister::FULL_PATH,
+                               &delegate);
+
+  lister->Start();
+
+  MessageLoop::current()->Run();
+
+  EXPECT_EQ(delegate.error(), net::OK);
+}
+
 TEST(DirectoryListerTest, CancelTest) {
   FilePath path;
   ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &path));
 
-  ListerDelegate delegate;
+  ListerDelegate delegate(false);
   scoped_refptr<net::DirectoryLister> lister =
       new net::DirectoryLister(path, &delegate);
 

net/url_request/url_request_file_dir_job.cc

@@ -102,7 +102,7 @@ bool URLRequestFileDirJob::GetCharset(string* charset) {
 }
 
 void URLRequestFileDirJob::OnListFile(
-    const file_util::FileEnumerator::FindInfo& data) {
+    const net::DirectoryLister::DirectoryListerData& data) {
   // We wait to write out the header until we get the first file, so that we
   // can catch errors from DirectoryLister and show an error page.
   if (!wrote_header_) {
@@ -122,26 +122,26 @@ void URLRequestFileDirJob::OnListFile(
   }
 
 #if defined(OS_WIN)
-  int64 size = (static_cast<unsigned __int64>(data.nFileSizeHigh) << 32) |
-      data.nFileSizeLow;
+  int64 size = (static_cast<unsigned __int64>(data.info.nFileSizeHigh) << 32) |
+      data.info.nFileSizeLow;
 
   // Note that we should not convert ftLastWriteTime to the local time because
   // ICU's datetime formatting APIs expect time in UTC and take into account
   // the timezone before formatting.
   data_.append(net::GetDirectoryListingEntry(
-      data.cFileName, std::string(),
-      (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? true : false,
+      data.info.cFileName, std::string(),
+      (data.info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? true : false,
       size,
-      base::Time::FromFileTime(data.ftLastWriteTime)));
+      base::Time::FromFileTime(data.info.ftLastWriteTime)));
 
 #elif defined(OS_POSIX)
   // TOOD(jungshik): The same issue as for the directory name.
   data_.append(net::GetDirectoryListingEntry(
-      WideToUTF16(base::SysNativeMBToWide(data.filename)),
-      data.filename,
-      S_ISDIR(data.stat.st_mode),
-      data.stat.st_size,
-      base::Time::FromTimeT(data.stat.st_mtime)));
+      WideToUTF16(base::SysNativeMBToWide(data.info.filename)),
+      data.info.filename,
+      S_ISDIR(data.info.stat.st_mode),
+      data.info.stat.st_size,
+      base::Time::FromTimeT(data.info.stat.st_mtime)));
 #endif
 
   // TODO(darin): coalesce more?

net/url_request/url_request_file_dir_job.h

@@ -28,7 +28,8 @@ class URLRequestFileDirJob
   virtual bool GetCharset(std::string* charset);
 
   // DirectoryLister::DirectoryListerDelegate methods:
-  virtual void OnListFile(const file_util::FileEnumerator::FindInfo& data);
+  virtual void OnListFile(
+      const net::DirectoryLister::DirectoryListerData& data);
   virtual void OnListDone(int error);
 
   bool list_complete() const { return list_complete_; }