A robust base::DeleteFile.

base/files/file_util_unittest.cc

 // Copyright (c) 2012 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 <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <fstream>
 #include <set>
@@ skipping 174 matching lines @@
 
 // file_util winds up using autoreleased objects on the Mac, so this needs
 // to be a PlatformTest
 class FileUtilTest : public PlatformTest {
  protected:
   void SetUp() override {
     PlatformTest::SetUp();
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   }
 
+  // Sets the file to read-only.
+  static void SetReadOnly(const FilePath& path, bool read_only) {
+#if defined(OS_WIN)
+    // On Windows, it involves setting/removing the 'readonly' bit.
+    DWORD attrs = GetFileAttributes(path.value().c_str());
+    ASSERT_NE(INVALID_FILE_ATTRIBUTES, attrs);

[Nico, 2017/01/10 18:31:55]

I thought ASSERT doesn't do the right thing in helper functions (it returns from
the current frame but the calling frame will just continue)

[grt (UTC plus 2), 2017/01/12 15:07:03]

Indeed! I've added ASSERT_NO_FATAL_FAILURE where relevant. Thanks.

+    ASSERT_TRUE(SetFileAttributes(
+        path.value().c_str(), read_only ? (attrs | FILE_ATTRIBUTE_READONLY)
+                                        : (attrs & ~FILE_ATTRIBUTE_READONLY)));
+
+    DWORD expected =
+        read_only
+            ? ((attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)) |
+               FILE_ATTRIBUTE_READONLY)
+            : (attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY));
+
+    // Ignore FILE_ATTRIBUTE_NOT_CONTENT_INDEXED if present.
+    attrs = GetFileAttributes(path.value().c_str()) &
+            ~FILE_ATTRIBUTE_NOT_CONTENT_INDEXED;
+    ASSERT_EQ(expected, attrs);
+#else
+    // On all other platforms, it involves removing/setting the write bit.
+    mode_t mode = read_only ? S_IRUSR : (S_IRUSR | S_IWUSR);
+    EXPECT_TRUE(SetPosixFilePermissions(
+        path, DirectoryExists(path) ? (mode | S_IXUSR) : mode));
+#endif
+  }
+
+  static bool IsReadOnly(const FilePath& path) {
+#if defined(OS_WIN)
+    DWORD attrs = GetFileAttributes(path.value().c_str());
+    EXPECT_NE(INVALID_FILE_ATTRIBUTES, attrs);
+    return attrs & FILE_ATTRIBUTE_READONLY;
+#else
+    int mode = 0;
+    EXPECT_TRUE(GetPosixFilePermissions(path, &mode));
+    return !(mode & S_IWUSR);
+#endif
+  }
+
   ScopedTempDir temp_dir_;
 };
 
 // Collects all the results from the given file enumerator, and provides an
 // interface to query whether a given file is present.
 class FindResultCollector {
  public:
   explicit FindResultCollector(FileEnumerator* enumerator) {
     FilePath cur_file;
     while (!(cur_file = enumerator->Next()).value().empty()) {
@@ skipping 455 matching lines @@
   // Test recursive case, create a new file
   file_name = temp_dir_.GetPath().Append(FPL("Test DeleteFile 2.txt"));
   CreateTextFile(file_name, bogus_content);
   ASSERT_TRUE(PathExists(file_name));
 
   // Make sure it's deleted
   EXPECT_TRUE(DeleteFile(file_name, true));
   EXPECT_FALSE(PathExists(file_name));
 }
 
+TEST_F(FileUtilTest, DeleteReadOnlyFile) {
+  // Create a read-only file

[Nico, 2017/01/10 18:31:55]

nit: style guide says comments should be sentences ending with punctuation (also
elsewhere)

[grt (UTC plus 2), 2017/01/12 15:07:03]

Done.

+  FilePath file_name = temp_dir_.GetPath().Append(FPL("Test DeleteFile 1.txt"));
+  CreateTextFile(file_name, bogus_content);
+  ASSERT_TRUE(PathExists(file_name));
+  SetReadOnly(file_name, true);
+  ASSERT_TRUE(IsReadOnly(file_name));
+
+  // Make sure it's deleted
+  EXPECT_TRUE(DeleteFile(file_name, false));
+  EXPECT_FALSE(PathExists(file_name));
+
+  // Test recursive case, create a new file
+  file_name = temp_dir_.GetPath().Append(FPL("Test DeleteFile 2.txt"));
+  CreateTextFile(file_name, bogus_content);
+  ASSERT_TRUE(PathExists(file_name));
+  SetReadOnly(file_name, true);
+  ASSERT_TRUE(IsReadOnly(file_name));
+
+  // Make sure it's deleted
+  EXPECT_TRUE(DeleteFile(file_name, true));
+  EXPECT_FALSE(PathExists(file_name));
+}
+
 #if defined(OS_POSIX)
 TEST_F(FileUtilTest, DeleteSymlinkToExistentFile) {
   // Create a file.
   FilePath file_name = temp_dir_.GetPath().Append(FPL("Test DeleteFile 2.txt"));
   CreateTextFile(file_name, bogus_content);
   ASSERT_TRUE(PathExists(file_name));
 
   // Create a symlink to the file.
   FilePath file_link = temp_dir_.GetPath().Append("file_link_2");
   ASSERT_TRUE(CreateSymbolicLink(file_name, file_link))
@@ skipping 195 matching lines @@
   ASSERT_TRUE(SetPosixFilePermissions(dir1.Append(kExeFileName),
                                       FILE_PERMISSION_EXECUTE_BY_USER));
 
   EXPECT_TRUE(ExecutableExistsInPath(env.get(), kExeFileName));
   EXPECT_FALSE(ExecutableExistsInPath(env.get(), kRegularFileName));
   EXPECT_FALSE(ExecutableExistsInPath(env.get(), kDneFileName));
 }
 
 #endif  // defined(OS_POSIX)
 
-#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 =
-      temp_dir_.GetPath().Append(FPL("Test DeleteWildCard.txt"));
-  CreateTextFile(file_name, bogus_content);
-  ASSERT_TRUE(PathExists(file_name));
-
-  FilePath subdir_path = temp_dir_.GetPath().Append(FPL("DeleteWildCardDir"));
-  CreateDirectory(subdir_path);
-  ASSERT_TRUE(PathExists(subdir_path));
-
-  // Create the wildcard path
-  FilePath directory_contents = temp_dir_.GetPath();
-  directory_contents = directory_contents.Append(FPL("*"));
-
-  // Delete non-recursively and check that only the file is deleted
-  EXPECT_TRUE(DeleteFile(directory_contents, false));
-  EXPECT_FALSE(PathExists(file_name));
-  EXPECT_TRUE(PathExists(subdir_path));
-
-  // Delete recursively and make sure all contents are deleted
-  EXPECT_TRUE(DeleteFile(directory_contents, true));
-  EXPECT_FALSE(PathExists(file_name));
-  EXPECT_FALSE(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 =
-      temp_dir_.GetPath().Append(FPL("DeleteNonExistantWildCard"));
-  CreateDirectory(subdir_path);
-  ASSERT_TRUE(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(DeleteFile(directory_contents, false));
-  EXPECT_TRUE(PathExists(subdir_path));
-
-  // Delete recursively and check nothing got deleted
-  EXPECT_TRUE(DeleteFile(directory_contents, true));
-  EXPECT_TRUE(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 =
       temp_dir_.GetPath().Append(FPL("DeleteDirNonRecursive"));
   CreateDirectory(test_subdir);
   ASSERT_TRUE(PathExists(test_subdir));
 
   FilePath file_name = test_subdir.Append(FPL("Test DeleteDir.txt"));
   CreateTextFile(file_name, bogus_content);
@@ skipping 471 matching lines @@
 
   EXPECT_TRUE(CopyDirectory(from_path, dir_name_to, true));
 
   // Check everything has been copied.
   EXPECT_TRUE(PathExists(dir_name_from));
   EXPECT_TRUE(PathExists(file_name_from));
   EXPECT_TRUE(PathExists(dir_name_to));
   EXPECT_TRUE(PathExists(file_name_to));
 }
 
-// Sets the source file to read-only.
-void SetReadOnly(const FilePath& path, bool read_only) {
-#if defined(OS_WIN)
-  // On Windows, it involves setting/removing the 'readonly' bit.
-  DWORD attrs = GetFileAttributes(path.value().c_str());
-  ASSERT_NE(INVALID_FILE_ATTRIBUTES, attrs);
-  ASSERT_TRUE(SetFileAttributes(
-      path.value().c_str(),
-      read_only ? (attrs | FILE_ATTRIBUTE_READONLY) :
-          (attrs & ~FILE_ATTRIBUTE_READONLY)));
-
-  DWORD expected = read_only ?
-      ((attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)) |
-          FILE_ATTRIBUTE_READONLY) :
-      (attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY));
-
-  // Ignore FILE_ATTRIBUTE_NOT_CONTENT_INDEXED if present.
-  attrs = GetFileAttributes(path.value().c_str()) &
-          ~FILE_ATTRIBUTE_NOT_CONTENT_INDEXED;
-  ASSERT_EQ(expected, attrs);
-#else
-  // On all other platforms, it involves removing/setting the write bit.
-  mode_t mode = read_only ? S_IRUSR : (S_IRUSR | S_IWUSR);
-  EXPECT_TRUE(SetPosixFilePermissions(
-      path, DirectoryExists(path) ? (mode | S_IXUSR) : mode));
-#endif
-}
-
-bool IsReadOnly(const FilePath& path) {
-#if defined(OS_WIN)
-  DWORD attrs = GetFileAttributes(path.value().c_str());
-  EXPECT_NE(INVALID_FILE_ATTRIBUTES, attrs);
-  return attrs & FILE_ATTRIBUTE_READONLY;
-#else
-  int mode = 0;
-  EXPECT_TRUE(GetPosixFilePermissions(path, &mode));
-  return !(mode & S_IWUSR);
-#endif
-}
-
 TEST_F(FileUtilTest, CopyDirectoryACL) {
   // Create source directories.
   FilePath src = temp_dir_.GetPath().Append(FILE_PATH_LITERAL("src"));
   FilePath src_subdir = src.Append(FILE_PATH_LITERAL("subdir"));
   CreateDirectory(src_subdir);
   ASSERT_TRUE(PathExists(src_subdir));
 
   // Create a file under the directory.
   FilePath src_file = src.Append(FILE_PATH_LITERAL("src.txt"));
   CreateTextFile(src_file, L"Gooooooooooooooooooooogle");
@@ skipping 1090 matching lines @@
   // Trying to close it should crash. This is important for security.
   EXPECT_DEATH(CloseWithScopedFD(fds[1]), "");
 #endif
 }
 
 #endif  // defined(OS_POSIX)
 
 }  // namespace
 
 }  // namespace base