Improve unit tests to verify that directories are listed before files in file:/// urls.

And fix current regression.

BUG=28420
TEST=improved unit tests

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

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

5 files changed, 42 insertions, 0 deletions

diff --git a/base/file_util.h b/base/file_util.h
index 7ec1dce..9eb1729 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -431,6 +431,9 @@ class FileEnumerator {
   // Write the file info into |info|.
   void GetFindInfo(FindInfo* info);
 
+  // Looks inside a FindInfo and determines if it's a directory.
+  static bool IsDirectory(const FindInfo& info);
+
  private:
   // Returns true if the given path should be skipped in enumeration.
   bool ShouldSkip(const FilePath& path);
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index ed5abcc..cdf7be9 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -583,6 +583,10 @@ void FileEnumerator::GetFindInfo(FindInfo* info) {
   info->filename.assign(cur_entry->filename.value());
 }
 
+bool FileEnumerator::IsDirectory(const FindInfo& info) {
+  return S_ISDIR(info.stat.st_mode);
+}
+
 FilePath FileEnumerator::Next() {
   ++current_directory_entry_;
 
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 9d29d80..6dfc049 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -727,6 +727,10 @@ void FileEnumerator::GetFindInfo(FindInfo* info) {
   memcpy(info, &find_data_, sizeof(*info));
 }
 
+bool FileEnumerator::IsDirectory(const FindInfo& info) {
+  return (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0;
+}
+
 FilePath FileEnumerator::Next() {
   if (!is_in_find_op_) {
     if (pending_paths_.empty())
diff --git a/net/base/directory_lister.cc b/net/base/directory_lister.cc
index 35b30cd..ef3130b 100644
--- a/net/base/directory_lister.cc
+++ b/net/base/directory_lister.cc
@@ -41,6 +41,11 @@ class DirectoryDataEvent : public Task {
 // comparison function on the filenames for sorting in the user's locale.
 static bool CompareFindInfo(const file_util::FileEnumerator::FindInfo& a,
                             const file_util::FileEnumerator::FindInfo& b) {
+  bool a_is_directory = file_util::FileEnumerator::IsDirectory(a);
+  bool b_is_directory = file_util::FileEnumerator::IsDirectory(b);
+  if (a_is_directory != b_is_directory)
+    return a_is_directory;
+
 #if defined(OS_WIN)
   return file_util::LocaleAwareCompareFilenames(FilePath(a.cFileName),
                                                 FilePath(b.cFileName));
diff --git a/net/base/directory_lister_unittest.cc b/net/base/directory_lister_unittest.cc
index a0c0c5b..85e2b647 100644
--- a/net/base/directory_lister_unittest.cc
+++ b/net/base/directory_lister_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "base/file_path.h"
 #include "base/file_util.h"
+#include "base/i18n/file_util_icu.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "net/base/directory_lister.h"
@@ -19,14 +20,39 @@ class ListerDelegate : public net::DirectoryLister::DirectoryListerDelegate {
   ListerDelegate() : error_(-1) {
   }
   void OnListFile(const file_util::FileEnumerator::FindInfo& data) {
+    file_list_.push_back(data);
   }
   void OnListDone(int error) {
     error_ = error;
     MessageLoop::current()->Quit();
+    // 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++) {
+        // Directories should come before files.
+        if (file_util::FileEnumerator::IsDirectory(file_list_[previous]) &&
+            !file_util::FileEnumerator::IsDirectory(file_list_[current])) {
+          continue;
+        }
+        EXPECT_EQ(file_util::FileEnumerator::IsDirectory(file_list_[previous]),
+                  file_util::FileEnumerator::IsDirectory(file_list_[current]));
+#if defined(OS_WIN)
+        EXPECT_TRUE(file_util::LocaleAwareCompareFilenames(
+                        FilePath(file_list_[previous].cFileName),
+                        FilePath(file_list_[current].cFileName)));
+#elif defined(OS_POSIX)
+        EXPECT_TRUE(file_util::LocaleAwareCompareFilenames(
+                        FilePath(file_list_[previous].filename),
+                        FilePath(file_list_[current].filename)));
+#endif
+      }
+    }
   }
   int error() const { return error_; }
  private:
   int error_;
+  std::vector<file_util::FileEnumerator::FindInfo> file_list_;
 };
 
 TEST(DirectoryListerTest, BigDirTest) {