Revert 46633 - Windows: Make file_util::Delete("c:\\foo_dir", false) work correctly. Add more unit tests for Delete.

BUG=42374
TEST=included
Review URL: http://codereview.chromium.org/1763008

TBR=thestig@chromium.org
Review URL: http://codereview.chromium.org/2038004

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

2 files changed, 22 insertions, 143 deletions

diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index c496290..25b93e1 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -31,8 +31,6 @@
 
 namespace {
 
-const wchar_t bogus_content[] = L"I'm cannon fodder.";
-
 const file_util::FileEnumerator::FILE_TYPE FILES_AND_DIRECTORIES =
     static_cast<file_util::FileEnumerator::FILE_TYPE>(
         file_util::FileEnumerator::FILES |
@@ -387,146 +385,36 @@ TEST_F(FileUtilTest, FileAndDirectorySize) {
   EXPECT_EQ(size_f1 + size_f2 + 3, computed_size);
 }
 
-TEST_F(FileUtilTest, DeleteNonExistent) {
-  FilePath non_existent = test_dir_.AppendASCII("bogus_file_dne.foobar");
-  ASSERT_FALSE(file_util::PathExists(non_existent));
-
-  EXPECT_TRUE(file_util::Delete(non_existent, false));
-  ASSERT_FALSE(file_util::PathExists(non_existent));
-  EXPECT_TRUE(file_util::Delete(non_existent, true));
-  ASSERT_FALSE(file_util::PathExists(non_existent));
-}
-
-TEST_F(FileUtilTest, DeleteFile) {
+// Tests that the Delete function works as expected, especially
+// the recursion flag.  Also coincidentally tests PathExists.
+TEST_F(FileUtilTest, Delete) {
   // Create a file
-  FilePath file_name = test_dir_.Append(FPL("Test DeleteFile 1.txt"));
-  CreateTextFile(file_name, bogus_content);
-  ASSERT_TRUE(file_util::PathExists(file_name));
-
-  // Make sure it's deleted
-  EXPECT_TRUE(file_util::Delete(file_name, false));
-  EXPECT_FALSE(file_util::PathExists(file_name));
-
-  // Test recursive case, create a new file
-  file_name = test_dir_.Append(FPL("Test DeleteFile 2.txt"));
-  CreateTextFile(file_name, bogus_content);
-  ASSERT_TRUE(file_util::PathExists(file_name));
-
-  // Make sure it's deleted
-  EXPECT_TRUE(file_util::Delete(file_name, true));
-  EXPECT_FALSE(file_util::PathExists(file_name));
-}
+  FilePath file_name = test_dir_.Append(FILE_PATH_LITERAL("Test File.txt"));
+  CreateTextFile(file_name, L"I'm cannon fodder.");
 
-#if defined(OS_WIN)
-// Tests that the Delete function works for wild cards, especially
-// with the recursion flag.  Also coincidentally tests PathExists.
-// TODO(erikkay): see if anyone's actually using this feature of the API
-TEST_F(FileUtilTest, DeleteWildCard) {
-  // Create a file and a directory
-  FilePath file_name = test_dir_.Append(FPL("Test DeleteWildCard.txt"));
-  CreateTextFile(file_name, bogus_content);
   ASSERT_TRUE(file_util::PathExists(file_name));
 
-  FilePath subdir_path = test_dir_.Append(FPL("DeleteWildCardDir"));
+  FilePath subdir_path = test_dir_.Append(FILE_PATH_LITERAL("Subdirectory"));
   file_util::CreateDirectory(subdir_path);
+
   ASSERT_TRUE(file_util::PathExists(subdir_path));
 
-  // Create the wildcard path
   FilePath directory_contents = test_dir_;
-  directory_contents = directory_contents.Append(FPL("*"));
-
+#if defined(OS_WIN)
+  // TODO(erikkay): see if anyone's actually using this feature of the API
+  directory_contents = directory_contents.Append(FILE_PATH_LITERAL("*"));
   // Delete non-recursively and check that only the file is deleted
-  EXPECT_TRUE(file_util::Delete(directory_contents, false));
+  ASSERT_TRUE(file_util::Delete(directory_contents, false));
   EXPECT_FALSE(file_util::PathExists(file_name));
   EXPECT_TRUE(file_util::PathExists(subdir_path));
+#endif
 
   // Delete recursively and make sure all contents are deleted
-  EXPECT_TRUE(file_util::Delete(directory_contents, true));
+  ASSERT_TRUE(file_util::Delete(directory_contents, true));
   EXPECT_FALSE(file_util::PathExists(file_name));
   EXPECT_FALSE(file_util::PathExists(subdir_path));
 }
 
-// TODO(erikkay): see if anyone's actually using this feature of the API
-TEST_F(FileUtilTest, DeleteNonExistantWildCard) {
-  // Create a file and a directory
-  FilePath subdir_path = test_dir_.Append(FPL("DeleteNonExistantWildCard"));
-  file_util::CreateDirectory(subdir_path);
-  ASSERT_TRUE(file_util::PathExists(subdir_path));
-
-  // Create the wildcard path
-  FilePath directory_contents = subdir_path;
-  directory_contents = directory_contents.Append(FPL("*"));
-
-  // Delete non-recursively and check nothing got deleted
-  EXPECT_TRUE(file_util::Delete(directory_contents, false));
-  EXPECT_TRUE(file_util::PathExists(subdir_path));
-
-  // Delete recursively and check nothing got deleted
-  EXPECT_TRUE(file_util::Delete(directory_contents, true));
-  EXPECT_TRUE(file_util::PathExists(subdir_path));
-}
-#endif
-
-// Tests non-recursive Delete() for a directory.
-TEST_F(FileUtilTest, DeleteDirNonRecursive) {
-  // Create a subdirectory and put a file and two directories inside.
-  FilePath test_subdir = test_dir_.Append(FPL("DeleteDirNonRecursive"));
-  file_util::CreateDirectory(test_subdir);
-  ASSERT_TRUE(file_util::PathExists(test_subdir));
-
-  FilePath file_name = test_subdir.Append(FPL("Test DeleteDir.txt"));
-  CreateTextFile(file_name, bogus_content);
-  ASSERT_TRUE(file_util::PathExists(file_name));
-
-  FilePath subdir_path1 = test_subdir.Append(FPL("TestSubDir1"));
-  file_util::CreateDirectory(subdir_path1);
-  ASSERT_TRUE(file_util::PathExists(subdir_path1));
-
-  FilePath subdir_path2 = test_subdir.Append(FPL("TestSubDir2"));
-  file_util::CreateDirectory(subdir_path2);
-  ASSERT_TRUE(file_util::PathExists(subdir_path2));
-
-  // Delete non-recursively and check that the empty dir got deleted
-  EXPECT_TRUE(file_util::Delete(subdir_path2, false));
-  EXPECT_FALSE(file_util::PathExists(subdir_path2));
-
-  // Delete non-recursively and check that nothing got deleted
-  EXPECT_FALSE(file_util::Delete(test_subdir, false));
-  EXPECT_TRUE(file_util::PathExists(test_subdir));
-  EXPECT_TRUE(file_util::PathExists(file_name));
-  EXPECT_TRUE(file_util::PathExists(subdir_path1));
-}
-
-// Tests recursive Delete() for a directory.
-TEST_F(FileUtilTest, DeleteDirRecursive) {
-  // Create a subdirectory and put a file and two directories inside.
-  FilePath test_subdir = test_dir_.Append(FPL("DeleteDirRecursive"));
-  file_util::CreateDirectory(test_subdir);
-  ASSERT_TRUE(file_util::PathExists(test_subdir));
-
-  FilePath file_name = test_subdir.Append(FPL("Test DeleteDirRecursive.txt"));
-  CreateTextFile(file_name, bogus_content);
-  ASSERT_TRUE(file_util::PathExists(file_name));
-
-  FilePath subdir_path1 = test_subdir.Append(FPL("TestSubDir1"));
-  file_util::CreateDirectory(subdir_path1);
-  ASSERT_TRUE(file_util::PathExists(subdir_path1));
-
-  FilePath subdir_path2 = test_subdir.Append(FPL("TestSubDir2"));
-  file_util::CreateDirectory(subdir_path2);
-  ASSERT_TRUE(file_util::PathExists(subdir_path2));
-
-  // Delete recursively and check that the empty dir got deleted
-  EXPECT_TRUE(file_util::Delete(subdir_path2, true));
-  EXPECT_FALSE(file_util::PathExists(subdir_path2));
-
-  // Delete recursively and check that everything got deleted
-  EXPECT_TRUE(file_util::Delete(test_subdir, true));
-  EXPECT_FALSE(file_util::PathExists(file_name));
-  EXPECT_FALSE(file_util::PathExists(subdir_path1));
-  EXPECT_FALSE(file_util::PathExists(test_subdir));
-}
-
 TEST_F(FileUtilTest, MoveFileNew) {
   // Create a file
   FilePath file_name_from =
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 4f96ef3..8d502e7 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -73,19 +73,11 @@ bool Delete(const FilePath& path, bool recursive) {
   if (path.value().length() >= MAX_PATH)
     return false;
 
-  if (!recursive) {
-    // If not recursing, then first check to see if |path| is a directory.
-    // If it is, then remove it with RemoveDirectory.
-    FileInfo file_info;
-    if (GetFileInfo(path, &file_info) && file_info.is_directory)
-      return RemoveDirectory(path.value().c_str()) != 0;
-
-    // Otherwise, it's a file, wildcard or non-existant. Try DeleteFile first
-    // because it should be faster. If DeleteFile fails, then we fall through
-    // to SHFileOperation, which will do the right thing.
-    if (DeleteFile(path.value().c_str()) != 0)
-      return true;
-  }
+  // If we're not recursing use DeleteFile; it should be faster. DeleteFile
+  // fails if passed a directory though, which is why we fall through on
+  // failure to the SHFileOperation.
+  if (!recursive && DeleteFile(path.value().c_str()) != 0)
+    return true;
 
   // SHFILEOPSTRUCT wants the path to be terminated with two NULLs,
   // so we have to use wcscpy because wcscpy_s writes non-NULLs
@@ -101,10 +93,9 @@ bool Delete(const FilePath& path, bool recursive) {
   if (!recursive)
     file_operation.fFlags |= FOF_NORECURSION | FOF_FILESONLY;
   int err = SHFileOperation(&file_operation);
-  // Some versions of Windows return ERROR_FILE_NOT_FOUND (0x2) when deleting
-  // an empty directory and some return 0x402 when they should be returning
-  // ERROR_FILE_NOT_FOUND. MSDN says Vista and up won't return 0x402.
-  return (err == 0 || err == ERROR_FILE_NOT_FOUND || err == 0x402);
+  // Some versions of Windows return ERROR_FILE_NOT_FOUND when
+  // deleting an empty directory.
+  return (err == 0 || err == ERROR_FILE_NOT_FOUND);
 }
 
 bool DeleteAfterReboot(const FilePath& path) {
@@ -609,7 +600,7 @@ bool CreateDirectory(const FilePath& full_path) {
 
 bool GetFileInfo(const FilePath& file_path, FileInfo* results) {
   WIN32_FILE_ATTRIBUTE_DATA attr;
-  if (!GetFileAttributesEx(file_path.value().c_str(),
+  if (!GetFileAttributesEx(file_path.ToWStringHack().c_str(),
                            GetFileExInfoStandard, &attr)) {
     return false;
   }