A robust base::DeleteFile.

base/files/file_util_win.cc

Index: base/files/file_util_win.cc
diff --git a/base/files/file_util_win.cc b/base/files/file_util_win.cc
index d70454df3836a7067f71eecf5a0ae8673376f474..998134ba6a4ddfa9160ada319d7ad84f1580fd7f 100644
--- a/base/files/file_util_win.cc
+++ b/base/files/file_util_win.cc
@@ -11,6 +11,7 @@
 #include <shlobj.h>
 #include <stddef.h>
 #include <stdint.h>
+#include <string.h>
 #include <time.h>
 #include <winsock2.h>
 
@@ -18,6 +19,7 @@
 #include <limits>
 #include <string>
 
+#include "base/base32.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
@@ -40,36 +42,408 @@ 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);
+// The workhorse of base::DeleteFile. This class implements a robust file and

[Will Harris, 2016/12/07 19:47:43]

how much of this comment should be exposed via the main base::DeleteFile header
file (file_util.h)?

are partial successes exposed up via that interface?

[grt (UTC plus 2), 2016/12/07 20:56:06]

Updated!
I documented what I think is necessary for callers to know. Let me know if you
think more is needed. Thanks!

+// directory deletion strategy to work with Windows' pecurlarity around file

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

nit: pecurlarity?

[grt (UTC plus 2), 2016/12/07 20:56:06]

what, you don't have that word in Icelandic? :-)

+// 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.

[Sigurður Ásgeirsson, 2016/12/07 16:29:19]

This sounds like a single post-order traversal on the directory hierarchy.

As we discussed on chat, by doing two passes you can pre-flight by doing
OpenFile to test that all the files and dirs grant DELETE and/or not open
without FILE_SHARE_DELETE. This would allow you more of an all-or-nothing
success or failure.
I don't know whether that's desirable here, and it's another metric ****ton of
work.

[grt (UTC plus 2), 2016/12/07 20:56:06]

From the caller's point of view, I'm not sure what the difference is between
"bail on first error" (leaving things in a completely unknown state on failure)
vs. "best effort" (leaving things in a completely unknown state on failure)
aside from the latter potentially freeing up more disk space. I'd rather have
the disk space. :-)

+// - Rob the item of its read-only status, if applicable.
+// - In the case of a directory that does not have a reparse point (i.e., a
+//   plain old dirctory, not a mount point or a symlinks), recursively delete

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

nit: dirctory

[grt (UTC plus 2), 2016/12/07 20:56:06]

Done.

+//   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).

[Will Harris, 2016/12/07 19:47:43]

should the public header file describe that one of the constraints of using this
function is that the caller has write access to the parent directory?

Are checks made before starting any of these operations that this is the case?

[grt (UTC plus 2), 2016/12/07 20:56:06]

It's not a constraint. If the caller has write access to a temp dir or the
parent, the item(s) will be moved. If not, they will be deleted in place.
No. The moves will fail (in MoveToTemp) but the deletes will proceed.

It does makes sense, though, for the temp probing code to check for writability
so that if X:\Temp can't be written to it tries the parent dir and fallsback to
using the item's own dir as a last resort. Thanks.

+// - 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 temp 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 {

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

Maybe you'd want to expose this class for more nuanced users, like the
installer, if only for the ability to get detailed metrics on how it did?

Alternatively you could expose a DeleteFileWithCallback that'd allow the
installer to call in here and still have its metrics cake?

[grt (UTC plus 2), 2016/12/07 20:56:06]

I was mulling over metrics options. My inclination is to keep this class private
since it's all implementation-detail-goop. Adding a callback for metrics is a
nice idea. Do you think it's nonsensical to put some histograms in the impl
itself for now and read canary data before deciding if it's worth the effort to
expose fine-grained metrics to callers?

+ public:
+  // Deletes a single item (a file, empty 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
+  // does not have a reparse point.
+  static bool DeleteRecursive(const FilePath& path);
+
+ private:
+  TreeDeleter();
+
+  // Sets |temp_dir_path_| to the path of a directory on the same volume as
+  // |path| (opened as |to_delete|) that will be used to temporarily hold files
+  // and directories as they are being deleted.
+  void ChooseTempDirIfNotChosen(const FilePath& path,
+                                const win::ScopedHandle& to_delete);
+
+  // 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 |to_delete|, using |basic_info| to adjust its attributes in the
+  // process.
+  bool DoDelete(const FilePath& path,
+                bool recurse,
+                const win::ScopedHandle& to_delete,
+                FILE_BASIC_INFO* basic_info);
+
+  // Deletes as much of the contents of |path| as possible, returning |true|
+  // only if everything within was deleted.
+  bool DeleteContents(const FilePath& path);
+
+  // Moves |to_delete| out into the temp 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 MoveToTemp(const FilePath& path, const win::ScopedHandle& to_delete);
+
+  // Renames the item |to_delete| to |target|.
+  bool MoveTo(const win::ScopedHandle& to_delete, const FilePath& target);
+
+  // Returns true if the serial number of the volume on which |path| resides
+  // can be obtained and equals |volume_serial_number|.
+  static bool VolumeSerialNumberIs(const FilePath& path,
+                                   ULONGLONG volume_serial_number);
+
+  // The directory into which items are to be moved immediately prior to
+  // deletion.
+  FilePath temp_dir_path_;
+
+  // 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_;
+
+  DISALLOW_COPY_AND_ASSIGN(TreeDeleter);
+};
+
+// static
+bool TreeDeleter::DeleteItem(const FilePath& path) {
+  return TreeDeleter().Delete(MakeAbsoluteFilePath(path), false /* !recurse */);
+}
+
+// static
+bool TreeDeleter::DeleteRecursive(const FilePath& path) {
+  return TreeDeleter().Delete(MakeAbsoluteFilePath(path), true /* recurse */);
+}
+
+TreeDeleter::TreeDeleter() {}
+
+void TreeDeleter::ChooseTempDirIfNotChosen(const FilePath& path,
+                                           const win::ScopedHandle& to_delete) {
+  if (!temp_dir_path_.empty())
+    return;
+
+  // Use the dir of |path| if all else fails.
+  temp_dir_path_ = path.DirName();
+
+  // Get the volume serial number of the path to be deleted.
+  FILE_ID_INFO item_id_info = {};

[grt (UTC plus 2), 2016/12/07 20:56:06]

turns out this wasn't supported on Win7.

+  if (!::GetFileInformationByHandleEx(to_delete.Get(), FileIdInfo,
+                                      &item_id_info, sizeof(item_id_info))) {
+    PLOG(ERROR) << "Failed to get file ID info of " << path.value();
+    return;
+  }
+
+  FilePath temp_path;
+
+  // Try %TEMP%.
+  if (GetTempDir(&temp_path) &&
+      VolumeSerialNumberIs(temp_path, item_id_info.VolumeSerialNumber)) {
+    temp_dir_path_ = temp_path;
+    return;
+  }
+
+  // Try X:\Temp and X:\Tmp.
+  wchar_t volume_path_name[MAX_PATH];
+  if (::GetVolumePathName(path.NormalizePathSeparators().value().c_str(),
+                          volume_path_name, arraysize(volume_path_name))) {
+    static constexpr const wchar_t* kTempNames[] = {L"Temp", L"Tmp"};
+    for (const wchar_t* temp_name : kTempNames) {
+      temp_path = FilePath(volume_path_name).Append(temp_name);
+      if (VolumeSerialNumberIs(temp_path, item_id_info.VolumeSerialNumber)) {
+        temp_dir_path_ = temp_path;

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

Do you also need to test whether you can create or write to the temp directory?

[grt (UTC plus 2), 2016/12/07 20:56:06]

Hmm. Yeah, that makes sense. If X:\Temp can't be written to, it's better to try
the parent dir and fallback to using the item's own dir as a last resort.
Thanks.

+        return;
+      }
     }
+  }
+
+  // Try the parent dir of |path|.
+  temp_path = path.DirName().DirName();
+  if (VolumeSerialNumberIs(temp_path, item_id_info.VolumeSerialNumber))
+    temp_dir_path_ = temp_path;
+}
+
+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;
+  }
+
+  ChooseTempDirIfNotChosen(path, to_delete);

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

nit: EnsureTempDirChosen?

[grt (UTC plus 2), 2016/12/07 20:56:06]

Resolved the odd name otherwise.

 
-    if (info.IsDirectory()) {
-      if (recursive && (!DeleteFileRecursive(current, pattern, true) ||
-                        !RemoveDirectory(current.value().c_str())))
-        return false;
-    } else if (!::DeleteFile(current.value().c_str())) {
+  // 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;
   }
+
+  if (DoDelete(path, recurse, to_delete, &basic_info))
+    return true;
+
+  // Restore the item's read-only state on failure.
+  if (read_only_set) {
+    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::DoDelete(const FilePath& path,
+                           bool recurse,
+                           const win::ScopedHandle& to_delete,
+                           FILE_BASIC_INFO* basic_info) {
+  // Recurse into this item if desired, this item is a directory, and this item
+  // does not have a reparse point (i.e., is not a mount point, a symlink, or
+  // some other directory-like thing implemented by a filesystem filter).
+  if (recurse && (basic_info->FileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 &&
+      (basic_info->FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0 &&
+      !DeleteContents(path)) {
+    return false;
+  }
+
+  // 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 temp directory have a greater liklihood of

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

nit: likelihood?

[grt (UTC plus 2), 2016/12/07 20:56:06]

Done.

(BTW: Damn, you're good. :-) )

+  // 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 (MoveToTemp(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::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 temp directory before deletion.

[Sigurður Ásgeirsson, 2016/12/07 16:29:18]

ah - oki - so failure to move to the temp dir is not fatal, or is it?

[grt (UTC plus 2), 2016/12/07 20:56:06]

Not fatal. I've improved things a bit so that the rename is still likely to
happen if nothing other than the root directory being deleted could be written
to.

+  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);
+
+  // Report success if the last pass encountered no errors.
+  return !items_failed;
+}
+
+bool TreeDeleter::MoveToTemp(const FilePath& path,
+                             const win::ScopedHandle& to_delete) {
+  DCHECK(!temp_dir_path_.empty());
+
+  // Do the shuffle to move the item out to the temp 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.
+
+  // 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 temp";
+    return true;
+  }
+
+  // Move the file out to the temp 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,
+        temp_dir_path_.AppendASCII(
+            base::Base32Encode(base::StringPiece(&random[0], sizeof(random)))
+                .append(".tmp")));
+  } 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.

[Sigurður Ásgeirsson, 2016/12/07 16:29:19]

I could see this happen if there are things on the system watching all moves to
scan all the things for your "security". I wonder if AVs back off on scanning
.tmp files in %TMP% dirs, as a rule.

[grt (UTC plus 2), 2016/12/07 20:56:06]

We're doing this on a handle that we opened with DELETE access. A scanner that
opened the same file would have to have used SHARE_DELETE, so I'm not sure how
they could interfere unless they were doing crazycrazy in the kernel. Or am I
forgetting about something again?

[Sigurður Ásgeirsson, 2016/12/08 14:29:26]

Use the source, (young) Padawan
<https://github.com/Microsoft/Windows-driver-samples/blob/master/filesys/fastf...>.
Looks like the file can become non-deletable if someone maps it in the interrim,
there's a smattering of other failure conditions.

+    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;
 }
 
+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;
+}
+
+// static
+bool TreeDeleter::VolumeSerialNumberIs(const FilePath& path,
+                                       ULONGLONG volume_serial_number) {
+  FILE_ID_INFO id_info = {};
+  File file(path, (File::FLAG_OPEN | File::FLAG_SHARE_DELETE |
+                   File::FLAG_BACKUP_SEMANTICS));
+  if (!file.IsValid()) {
+    PLOG(ERROR) << "Unable to get volume serial number of " << path.value();
+    return false;
+  }
+  return (::GetFileInformationByHandleEx(file.GetPlatformFile(), FileIdInfo,
+                                         &id_info, sizeof(id_info)) &&
+          id_info.VolumeSerialNumber == volume_serial_number);
+}
+
 }  // namespace
 
 FilePath MakeAbsoluteFilePath(const FilePath& input) {
@@ -83,36 +457,18 @@ FilePath MakeAbsoluteFilePath(const FilePath& input) {
 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) {