A robust base::DeleteFile.

base/files/file_util_win.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 "base/files/file_util.h"
 
 #include <windows.h>
 #include <io.h>
 #include <psapi.h>
 #include <shellapi.h>
 #include <shlobj.h>
 #include <stddef.h>
 #include <stdint.h>
+#include <string.h>
 #include <time.h>
+#include <winioctl.h>
 #include <winsock2.h>
 
 #include <algorithm>
 #include <limits>
 #include <string>
 
+#include "base/base32.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/metrics/histogram.h"
 #include "base/process/process_handle.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/windows_version.h"
 
 namespace base {
 
 namespace {
 
 const DWORD kFileShareAll =
     FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
 
-// Deletes all files and directories in a path.
-// Returns false on the first failure it encounters.
-bool DeleteFileRecursive(const FilePath& path,
-                         const FilePath::StringType& pattern,
-                         bool recursive) {
-  FileEnumerator traversal(path, false,
-                           FileEnumerator::FILES | FileEnumerator::DIRECTORIES,
-                           pattern);
-  for (FilePath current = traversal.Next(); !current.empty();
-       current = traversal.Next()) {
-    // Try to clear the read-only bit if we find it.
-    FileEnumerator::FileInfo info = traversal.GetInfo();
-    if ((info.find_data().dwFileAttributes & FILE_ATTRIBUTE_READONLY) &&
-        (recursive || !info.IsDirectory())) {
-      SetFileAttributes(
-          current.value().c_str(),
-          info.find_data().dwFileAttributes & ~FILE_ATTRIBUTE_READONLY);
-    }
-
-    if (info.IsDirectory()) {
-      if (recursive && (!DeleteFileRecursive(current, pattern, true) ||
-                        !RemoveDirectory(current.value().c_str())))
-        return false;
-    } else if (!::DeleteFile(current.value().c_str())) {
+// The workhorse of base::DeleteFile. This class implements a robust file and
+// directory deletion strategy to work with Windows' pecurlarity around file
+// deletion -- on Windows, one does not simply delete a file. The Win32
+// DeleteFile function is deceptively named (though properly documented) as it
+// marks a file for deletion at some unspecified time in the future when all
+// handles are closed. This here workhorse accomplishes two goals: recursive
+// deletes can succeed in the face of other software operating therein (so long
+// as they open files with SHARE_DELETE) and it's much more likely that callers
+// can delete a file or directory and then immediately create another with the
+// same name. This class implementes DeleteFile as follows:
+// - Open the item to be deleted.
+// - Rob the item of its read-only status, if applicable.
+// - In the case of a directory that is not a mount point or a symlink,
+//   recursively delete everything contained therein using the strategy
+//   described here.
+// - Paint the item with delete-on-close to make sure it is deletable.
+// - Hide the item.
+// - Clear the delete-on-close bit so that it can...
+// - Move the item up one directory (with a random name).
+// - Paint the item with delete-on-close for good.
+//
+// Failure possibilities are:
+// - The item could not be opened.
+// - The item's attributes could not be read.
+// - The item was read-only and this attribute could not be cleared.
+// - The item could not be deleted for various legitimate reasons such as it
+//   being open without SHARE_DELETE access, or the current user not having
+//   permission to delete it.
+// - The file was moved out to the parent directory, could not be deleted, and
+//   could not be moved back to its original location. This will DCHECK in a
+//   debug build, as it is highly undesirable and so far outside of expectation.
+//
+// There also exists a partial success case where the item could not be moved
+// prior to deletion but was deleted in situ. This is essentially the Win32
+// DeleteFile behavior, also known as "better than nothing".
+class TreeDeleter {
+ public:
+  // Deletes a single item (a file, directory, mount point, symlink, etc).
+  // Returns false if the item could not be deleted.
+  static bool DeleteItem(const FilePath& path);
+
+  // Recursively deletes |path| and its contents if |path| is a directory and
+  // is not a mount point or a symlink.
+  static bool DeleteRecursive(const FilePath& path);
+
+ private:
+  // Constructs an instance to delete |path| recursively or not.
+  explicit TreeDeleter(const FilePath& path);
+
+  // Main entrypoint to delete |path|. |recurse|, when true, indicates that an
+  // instance created for recursion should recurse into directories.
+  bool Delete(const FilePath& path, bool recurse);
+
+  // Deletes as much of the contents of |path| as possible, returning |true|
+  // only if everything within was deleted.
+  bool DeleteContents(const FilePath& path);
+
+  // Deletes |to_delete|, using |basic_info| to adjust its attributes in the
+  // process.
+  bool DoDelete(const FilePath& path,
+                const win::ScopedHandle& to_delete,
+                FILE_BASIC_INFO* basic_info);
+
+  // Moves |to_delete| out into the parent directory of the operation.
+  // |to_delete| is expected to already have its delete-on-close attribute set.
+  // This function will clear it, move the file, and re-establish
+  // delete-on-close. Returns false if the delete-on-close attribute is not set
+  // on exit.
+  bool MoveToParent(const FilePath& path, const win::ScopedHandle& to_delete);
+
+  // Returns a buffer used to determine if a directory is a mount point or a
+  // symlink.
+  REPARSE_GUID_DATA_BUFFER* GetReparseDataBuffer();
+
+  // Returns |true| if |to_delete| (whose attributes are represented in
+  // |basic_info|) is a mount point (a.k.a. a junction point) or a symlink.
+  bool IsMountPointOrSymlink(const win::ScopedHandle& to_delete,
+                             const FILE_BASIC_INFO& basic_info);
+
+  // Renames the item |to_delete| to |target|.
+  bool MoveTo(const win::ScopedHandle& to_delete, const FilePath& target);
+
+  // The parent directory into which items are to be moved immediately prior to
+  // deletion.
+  FilePath parent_directory_;
+
+  // The size, in bytes, of the FILE_RENAME_INFO block.
+  size_t rename_info_buffer_size_ = 0;
+
+  // Holds a FILE_RENAME_INFO when the instance will be used to move files out
+  // of their current directory prior to deletion.
+  std::unique_ptr<uint8_t> rename_info_buffer_;
+
+  // A buffer of size MAXIMUM_REPARSE_DATA_BUFFER_SIZE used to determine if a
+  // directory with a reparse point is either a mount point or a symlink. Only
+  // used for recursive deletes.
+  std::unique_ptr<uint8_t> reparse_data_buffer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TreeDeleter);
+};
+
+// static
+bool TreeDeleter::DeleteItem(const FilePath& path) {
+  return TreeDeleter(path).Delete(path, false /* !recurse */);
+}
+
+// static
+bool TreeDeleter::DeleteRecursive(const FilePath& path) {
+  return TreeDeleter(path).Delete(path, true /* recurse */);
+}
+
+TreeDeleter::TreeDeleter(const FilePath& path) {
+  const FilePath directory = path.DirName();
+  const FilePath parent_directory = directory.DirName();
+
+  // Prepare to move files up to the parent directory while deleting them if the
+  // path to be deleted is not at the root of a volume.
+  if (parent_directory != directory)
+    parent_directory_ = parent_directory;
+}
+
+bool TreeDeleter::Delete(const FilePath& path, bool recurse) {
+  // Open the item to be deleted with permission to hide and delete it with full
+  // sharing. Note that backup semantics are required to open a directory.
+  // Operate on a link or a mount point (implemented via reparse points) itself
+  // rather than on its target.
+  win::ScopedHandle to_delete(::CreateFile(
+      path.value().c_str(),
+      DELETE | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES,
+      FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr,
+      OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
+      nullptr));
+  if (!to_delete.IsValid()) {
+    DWORD error = ::GetLastError();
+    if (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND)
+      return true;
+    // Try a straight-up delete as a fallback.
+    if (::DeleteFile(path.value().c_str()))
+      return true;
+    ::SetLastError(error);
+    PLOG(ERROR) << "Failed to open " << path.value() << " for deletion";
+    return false;
+  }
+
+  // Remove the item's read-only bit if it is set. This is required for
+  // deletion.
+  bool read_only_set = false;
+  FILE_BASIC_INFO basic_info = {};
+  if (!::GetFileInformationByHandleEx(to_delete.Get(), FileBasicInfo,
+                                      &basic_info, sizeof(basic_info))) {
+    PLOG(ERROR) << "Failed to read attributes of " << path.value();
+    return false;
+  }
+  if ((basic_info.FileAttributes & FILE_ATTRIBUTE_READONLY) != 0) {
+    basic_info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY;
+    if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+                                      &basic_info, sizeof(basic_info))) {
+      PLOG(ERROR) << "Failed to strip read-only attribute of " << path.value();
       return false;
     }
-  }
+    read_only_set = true;
+  }
+
+  // Recurse into this item if desired, this item is a directory, and this item
+  // is neither a mount point nor a symlink.
+  if (recurse && (basic_info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 &&
+      !IsMountPointOrSymlink(to_delete, basic_info) && !DeleteContents(path)) {

[grt (UTC plus 2), 2016/12/04 08:52:06]

i wonder if this is overkill. a directory with a reparse point cannot contain
files, so perhaps it's correct enough to not recurse if |to_delete| is a
directory and has a reparse point. thoughts?

[jschuh, 2016/12/04 14:10:46]

For IsMountPointOrSymlink()? Doesn't seem like overkill to me.

[grt (UTC plus 2), 2016/12/04 21:31:48]

I'm not too familiar with the other uses for reparse points.
https://msdn.microsoft.com/en-us/library/dd541667.aspx lists the possibilities
as defined by MS, but 3rd parties can define their own. Since MSDN says that a
directory with a reparse point may not contain files, it seems to be reasonably
conservative to never recurse into them. Or are there cases where it really is
the right thing? Of note: Win32's DeleteFile operates on links rather than
targets and Win32's RemoveDirectory removes mounts/junctions even if the target
is not empty.

[jschuh, 2016/12/06 22:16:34]

Ah, I see. I think it would probably be safe enough to test just for
FILE_ATTRIBUTE_REPARSE_POINT, because recursively deleting where we shouldn't is
probably worse than not doing it.

[grt (UTC plus 2), 2016/12/07 08:34:39]

Done.

+    return false;
+  }
+
+  if (DoDelete(path, to_delete, &basic_info))
+    return true;
+
+  // Restore the item's read-only state on failure.
+  if (read_only_set) {

[Scott Hess - ex-Googler, 2016/12/06 08:15:56]

Maybe split this function into a helper, so that you don't lose failures (or
maybe I misunderstand why DoDelete failure warrants this reset, but the
return-false just above that doesn't).

[Hmm.  Maybe because DoDelete is taking over to_delete?]

[grt (UTC plus 2), 2016/12/07 08:34:39]

Nice catch! Indeed, the block above belongs in DoDelete so that the directory is
set back to read-only on failure.

+    basic_info.FileAttributes |= FILE_ATTRIBUTE_READONLY;
+    if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+                                      &basic_info, sizeof(basic_info))) {
+      PLOG(ERROR) << "Failed to restore read-only attribute of "
+                  << path.value();
+    }
+  }
+  return false;
+}
+
+bool TreeDeleter::DeleteContents(const FilePath& path) {
+  // Recurse through all subdirectories to clear them out. Ignore errors during
+  // this pass -- they will be reported if the directories themselves cannot be
+  // deleted below.
+  {
+    FileEnumerator enumer(path, false, FileEnumerator::DIRECTORIES);
+    for (FilePath item = enumer.Next(); !item.empty(); item = enumer.Next())
+      Delete(item, true /* recurse */);
+  }
+
+  // Now that the contents of all subdirectories have been cleared, repeatedly
+  // pass over the contents of this directory until there is nothing left that
+  // can be deleted. Bail out after five fruitless iterations. This is a
+  // mitigation for the failure mode where the item could not be moved out to
+  // the parent directory before deletion.
+  bool items_failed;
+  int retries = 0;
+  do {
+    items_failed = false;
+    FileEnumerator enumer(
+        path, false, (FileEnumerator::DIRECTORIES | FileEnumerator::FILES));
+    for (FilePath item = enumer.Next(); !item.empty(); item = enumer.Next()) {
+      if (Delete(item, false /* !recurse */))
+        retries = 0;  // Reset the retry count on any success.
+      else
+        items_failed = true;
+    }
+  } while (items_failed && ++retries < 5);

[grt (UTC plus 2), 2016/12/07 08:34:39]

wdyt of a short sleep here to give the system time to process whatever is in the
directory? maybe only a single retry with a short sleep?

+
+  // Report success if the last pass encountered no errors.
+  return !items_failed;
+}
+
+bool TreeDeleter::DoDelete(const FilePath& path,
+                           const win::ScopedHandle& to_delete,
+                           FILE_BASIC_INFO* basic_info) {
+  // Check to see if it's possible to delete this item. A non-empty directory,
+  // for example, cannot be deleted.
+  FILE_DISPOSITION_INFO disposition = {TRUE};  // DeleteFile
+  if (!::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+                                    &disposition, sizeof(disposition))) {
+    PLOG(ERROR) << "Unable to delete " << path.value();
+    return false;
+  }
+
+  // The item will now be deleted when all handles are closed. Hide it so that
+  // it appears to vanish right away and so that any other procs (e.g.,
+  // explorer.exe) observing the parent directory have a greater liklihood of
+  // not reacting when a new file suddenly appears and disappears.
+  bool hidden_set = false;
+  if ((basic_info->FileAttributes & FILE_ATTRIBUTE_HIDDEN) == 0) {
+    basic_info->FileAttributes |= FILE_ATTRIBUTE_HIDDEN;
+    if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+                                      basic_info, sizeof(*basic_info))) {
+      PLOG(WARNING) << "Failed to hide " << path.value();
+    }
+    hidden_set = true;
+  }
+
+  if (MoveToParent(path, to_delete))
+    return true;
+
+  // The item couldn't be deleted. Restore its visibility.
+  if (hidden_set) {
+    basic_info->FileAttributes &= ~FILE_ATTRIBUTE_HIDDEN;
+    if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+                                      basic_info, sizeof(*basic_info))) {
+      PLOG(WARNING) << "Failed to unhide " << path.value();
+    }
+  }
+  return false;
+}
+
+bool TreeDeleter::MoveToParent(const FilePath& path,

[jschuh, 2016/12/04 14:10:46]

Is there a better place to move to than the parent? Could we first check if one
of the temp resolutions is on the same drive, and if so use that as the target
first? My thought is to manually look for a temp by checking GetTempPath()'s
result, %SystemRoot%\Temp, %USERPROFILE%\AppData\Local\Temp, and ?:\Temp for the
target drive. Any of those just seems like a better location than the parent.

[grt (UTC plus 2), 2016/12/04 21:35:46]

I had similar misgivings about doing this, and I'm glad you brought it up. I
chose the parent because it's less work and because Niall mentions that this is
his strategy in his "Racing the Filesystem" talk. I do like that using some
temp-ish dir has the benefit that anything left behind should eventually be
reaped by another force. Do you think it's worth the added complexity?

[Scott Hess - ex-Googler, 2016/12/06 08:15:56]

In most cases in our code, there probably is going to be an obvious place where
you could dump deleted items temporarily.  At worst, most cases will have a
clear root which encloses all other operations.  Maybe rather than trying to
make this code smart enough to make intelligent guesses, it should instead
expose an API that allows the landing directory to be specified directly.

Alternately, since I see there is retry logic already, maybe recursive delete
could progress by renaming everything to the root, and only renaming slow items
out of the root.

[jschuh, 2016/12/06 22:16:34]

I just worry about accumulating a bunch of files in an odd location. But I trust
your judgement if you don't think it's worth the complexity.

[grt (UTC plus 2), 2016/12/07 08:34:39]

The next two things on my plate are:
- Survey current callers to see if most/all have a logical directory that should
be the destination. There are more than a few. :-(
- Try out a "search for TEMP". I'm convincing myself that TEMP is better if it's
on the same volume. If it's not (and X;\Temp doesn't exist), then moving up the
tree may be okay. Maybe most deletes outside of the installer are in the user
data dir somewhere.
- Another thought: for everything on the same volume as the user data dir, move
into a temp dir in UDD. We could have a low-priority purge pass after browser
launch to clean up stragglers.

I would love it if we could make base::DeleteFile "just work", but maybe we will
need either extra Win-specific APIs to set a temp location or make a cleanup
pass.

[grt (UTC plus 2), 2016/12/07 15:03:10]

I've added this. PTAL.

+                               const win::ScopedHandle& to_delete) {
+  // Do the shuffle to move the item out to the parent directory so that a new
+  // item with the same name can be created immediately in this directory, or
+  // so that the containing directory can be deleted immediately.
+
+  // There is nothing to do when deleting at the root of a volume.
+  if (parent_directory_.empty())
+    return true;
+
+  // Revoke deletion so that the item can be moved.
+  FILE_DISPOSITION_INFO disposition = {FALSE};  // DeleteFile
+  if (!::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+                                    &disposition, sizeof(disposition))) {
+    PLOG(WARNING) << "Unable to revoke deletion of " << path.value()
+                  << "; skipping move to parent";
+    return true;
+  }
+
+  // Move the file out to the parent directory. Failure to move is not a fatal
+  // failure, though it will lead to a race when deleting the containing
+  // directory. This is mitigated by retries in directory deletion.
+  bool file_moved = false;
+  int attempts = 0;
+  do {
+    char random[5];
+    base::RandBytes(&random[0], sizeof(random));
+    file_moved = MoveTo(
+        to_delete,
+        parent_directory_.AppendASCII(
+            base::Base32Encode(base::StringPiece(&random[0], sizeof(random)))
+                .append(".tmp")));

[Scott Hess - ex-Googler, 2016/12/06 08:15:56]

IMHO adding .tmp is just putting lipstick on the pig.

[jschuh, 2016/12/06 22:16:34]

I disagree. I think this is a good indicator if someone stumbles across this
file after it should have been deleted.

[grt (UTC plus 2), 2016/12/07 08:34:38]

This was my thinking exactly.

+  } while (!file_moved && ++attempts < 5);
+
+  // Mark the file to be deleted on close once again.
+  disposition.DeleteFile = TRUE;
+  if (!::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+                                    &disposition, sizeof(disposition))) {
+    // This is highly unexpected since the file was to be deleted moments ago.
+    PLOG(ERROR) << "Failed to mark file to be deleted on close";
+
+    if (file_moved) {
+      // Try to move the file back to where it was. If this fails, we're in the
+      // town of cats and leaving the file in the wrong place. Regardless, make
+      // a final attempt to mark the file for deletion.
+      bool file_moved_back = MoveTo(to_delete, path);
+      LONG error = ::GetLastError();
+      if (::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+                                       &disposition, sizeof(disposition))) {
+        // Whew! The file will eventually be deleted in situ.
+        return true;
+      }
+      if (!file_moved_back) {
+        ::SetLastError(error);
+        PLOG(DFATAL) << "Failed to move item back to original location";
+      }
+    }
+    return false;
+  }
+
   return true;
 }
+
+REPARSE_GUID_DATA_BUFFER* TreeDeleter::GetReparseDataBuffer() {
+  if (!reparse_data_buffer_)
+    reparse_data_buffer_.reset(new uint8_t[MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
+  return reinterpret_cast<REPARSE_GUID_DATA_BUFFER*>(
+      reparse_data_buffer_.get());
+}
+
+bool TreeDeleter::IsMountPointOrSymlink(const win::ScopedHandle& to_delete,
+                                        const FILE_BASIC_INFO& basic_info) {
+  if ((basic_info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0)
+    return false;
+
+  REPARSE_GUID_DATA_BUFFER* buffer = GetReparseDataBuffer();
+  DWORD bytes_used = 0;
+  if (!::DeviceIoControl(to_delete.Get(), FSCTL_GET_REPARSE_POINT, nullptr, 0,
+                         buffer, MAXIMUM_REPARSE_DATA_BUFFER_SIZE, &bytes_used,
+                         nullptr) ||
+      bytes_used < sizeof(DWORD)) {
+    PLOG(ERROR) << "Failed getting reparse data";
+    return true;  // Err on the safe side.
+  }
+
+  return buffer->ReparseTag == IO_REPARSE_TAG_MOUNT_POINT ||
+         buffer->ReparseTag == IO_REPARSE_TAG_SYMLINK;
+}
+
+bool TreeDeleter::MoveTo(const win::ScopedHandle& to_delete,
+                         const FilePath& target) {
+  // Compute the proper size of the FILE_RENAME_INFO including the target path.
+  size_t path_bytes = (target.value().length() + 1) * sizeof(wchar_t);
+  size_t struct_size = offsetof(FILE_RENAME_INFO, FileName) + path_bytes;
+
+  // Allocate a new buffer if needed.
+  if (rename_info_buffer_size_ < struct_size) {
+    rename_info_buffer_.reset();
+    rename_info_buffer_.reset(new uint8_t[struct_size]);
+    rename_info_buffer_size_ = struct_size;
+  }
+
+  // Formulate a FILE_RENAME_INFO struct in the buffer.
+  FILE_RENAME_INFO* file_rename_info =
+      reinterpret_cast<FILE_RENAME_INFO*>(rename_info_buffer_.get());
+  file_rename_info->ReplaceIfExists = FALSE;
+  file_rename_info->RootDirectory = nullptr;
+  file_rename_info->FileNameLength = target.value().length();
+  ::memcpy(&file_rename_info->FileName[0], target.value().c_str(), path_bytes);
+
+  if (!::SetFileInformationByHandle(to_delete.Get(), FileRenameInfo,
+                                    file_rename_info, struct_size)) {
+    PLOG(ERROR) << "Failed to move item to " << target.value();
+    return false;
+  }
+  return true;
+}
 
 }  // namespace
 
 FilePath MakeAbsoluteFilePath(const FilePath& input) {
   ThreadRestrictions::AssertIOAllowed();
   wchar_t file_path[MAX_PATH];
   if (!_wfullpath(file_path, input.value().c_str(), MAX_PATH))
     return FilePath();
   return FilePath(file_path);
 }
 
 bool DeleteFile(const FilePath& path, bool recursive) {
   ThreadRestrictions::AssertIOAllowed();
 
+  // Wildcards are not supported.
+  DCHECK_EQ(path.BaseName().value().find_first_of(L"*?"),
+            FilePath::StringType::npos);
+
   if (path.empty())
     return true;
 
   if (path.value().length() >= MAX_PATH)
     return false;
 
-  // Handle any path with wildcards.
-  if (path.BaseName().value().find_first_of(L"*?") !=
-      FilePath::StringType::npos) {
-    return DeleteFileRecursive(path.DirName(), path.BaseName().value(),
-                               recursive);
-  }
-  DWORD attr = GetFileAttributes(path.value().c_str());
-  // We're done if we can't find the path.
-  if (attr == INVALID_FILE_ATTRIBUTES)
-    return true;
-  // We may need to clear the read-only bit.
-  if ((attr & FILE_ATTRIBUTE_READONLY) &&
-      !SetFileAttributes(path.value().c_str(),
-                          attr & ~FILE_ATTRIBUTE_READONLY)) {
-    return false;
-  }
-  // Directories are handled differently if they're recursive.
-  if (!(attr & FILE_ATTRIBUTE_DIRECTORY))
-    return !!::DeleteFile(path.value().c_str());
-  // Handle a simple, single file delete.
-  if (!recursive || DeleteFileRecursive(path, L"*", true))
-    return !!RemoveDirectory(path.value().c_str());
-
-  return false;
+  return recursive ? TreeDeleter::DeleteRecursive(path)
+                   : TreeDeleter::DeleteItem(path);
 }
 
 bool DeleteFileAfterReboot(const FilePath& path) {
   ThreadRestrictions::AssertIOAllowed();
 
   if (path.value().length() >= MAX_PATH)
     return false;
 
   return MoveFileEx(path.value().c_str(), NULL,
                     MOVEFILE_DELAY_UNTIL_REBOOT |
@@ skipping 697 matching lines @@
     // Like Move, this function is not transactional, so we just
     // leave the copied bits behind if deleting from_path fails.
     // If to_path exists previously then we have already overwritten
     // it by now, we don't get better off by deleting the new bits.
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace base