[go: nahoru, domu]

base: Support retry for GetDeletePathRecursivelyCallback()

Currently GetDeleteFileCallback() supports retry on Windows. However,
GetDeletePathRecursivelyCallback() doesn't support retry. To make the
behavior consistent, this CL updates GetDeletePathRecursivelyCallback()
to also support retry.

A new UMA is added to record the actual retry count.

Bug: 1321695
Change-Id: I34e413b369acbcd58de7662d4077152ae595e7e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3621141
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1005784}
diff --git a/base/files/file_util.cc b/base/files/file_util.cc
index 0528ad26..a186a6c 100644
--- a/base/files/file_util.cc
+++ b/base/files/file_util.cc
@@ -37,6 +37,8 @@
 
 namespace base {
 
+#if !BUILDFLAG(IS_WIN)
+
 namespace {
 
 void RunAndReply(OnceCallback<bool()> action_callback,
@@ -48,7 +50,6 @@
 
 }  // namespace
 
-#if !BUILDFLAG(IS_WIN)
 OnceClosure GetDeleteFileCallback(const FilePath& path,
                                   OnceCallback<void(bool)> reply_callback) {
   return BindOnce(&RunAndReply, BindOnce(&DeleteFile, path),
@@ -57,7 +58,6 @@
                       : BindPostTask(SequencedTaskRunnerHandle::Get(),
                                      std::move(reply_callback)));
 }
-#endif  // !BUILDFLAG(IS_WIN)
 
 OnceClosure GetDeletePathRecursivelyCallback(
     const FilePath& path,
@@ -69,6 +69,8 @@
                                      std::move(reply_callback)));
 }
 
+#endif  // !BUILDFLAG(IS_WIN)
+
 int64_t ComputeDirectorySize(const FilePath& root_path) {
   int64_t running_size = 0;
   FileEnumerator file_iter(root_path, true, FileEnumerator::FILES);
diff --git a/base/files/file_util_win.cc b/base/files/file_util_win.cc
index 9798384..6fdd30a 100644
--- a/base/files/file_util_win.cc
+++ b/base/files/file_util_win.cc
@@ -313,22 +313,24 @@
 
 constexpr int kMaxDeleteAttempts = 9;
 
-void LogFileDeleteRetryCount(int attempt) {
-  UmaHistogramExactLinear("Windows.FileDeleteRetryCount", attempt,
-                          kMaxDeleteAttempts);
+void LogFileDeleteRetryCount(bool recursive, int attempt) {
+  UmaHistogramExactLinear(recursive ? "Windows.PathRecursivelyDeleteRetryCount"
+                                    : "Windows.FileDeleteRetryCount",
+                          attempt, kMaxDeleteAttempts);
 }
 
-void DeleteFileWithRetry(int attempt,
-                         const FilePath& file_path,
+void DeleteFileWithRetry(const FilePath& path,
+                         bool recursive,
+                         int attempt,
                          OnceCallback<void(bool)> reply_callback) {
   // Retry every 250ms for up to two seconds. These values were pulled out of
   // thin air, and may be adjusted in the future based on the metrics collected.
   static constexpr TimeDelta kDeleteFileRetryDelay = Milliseconds(250);
 
-  if (DeleteFile(file_path)) {
+  if (DeleteFileOrSetLastError(path, recursive)) {
     // Log how many times we had to retry the RetryDeleteFile operation before
     // it succeeded. This will be from 0 to kMaxDeleteAttempts - 1.
-    LogFileDeleteRetryCount(attempt);
+    LogFileDeleteRetryCount(recursive, attempt);
     // Consider introducing further retries until the item has been removed from
     // the filesystem and its name is ready for reuse; see the comments in
     // chrome/installer/mini_installer/delete_with_retry.cc for details.
@@ -341,7 +343,7 @@
   DCHECK_LE(attempt, kMaxDeleteAttempts);
   if (attempt == kMaxDeleteAttempts) {
     // Log kMaxDeleteAttempts to indicate failure after exhausting all attempts.
-    LogFileDeleteRetryCount(attempt);
+    LogFileDeleteRetryCount(recursive, attempt);
     if (!reply_callback.is_null())
       std::move(reply_callback).Run(false);
     return;
@@ -349,21 +351,37 @@
 
   ThreadPool::PostDelayedTask(FROM_HERE,
                               {TaskPriority::BEST_EFFORT, MayBlock()},
-                              BindOnce(&DeleteFileWithRetry, attempt, file_path,
-                                       std::move(reply_callback)),
+                              BindOnce(&DeleteFileWithRetry, path, recursive,
+                                       attempt, std::move(reply_callback)),
                               kDeleteFileRetryDelay);
 }
 
+OnceClosure GetDeleteFileCallbackInternal(
+    const FilePath& path,
+    bool recursive,
+    OnceCallback<void(bool)> reply_callback) {
+  OnceCallback<void(bool)> bound_callback;
+  if (!reply_callback.is_null()) {
+    bound_callback = BindPostTask(SequencedTaskRunnerHandle::Get(),
+                                  std::move(reply_callback));
+  }
+  return BindOnce(&DeleteFileWithRetry, path, recursive, /*attempt=*/0,
+                  std::move(bound_callback));
+}
+
 }  // namespace
 
 OnceClosure GetDeleteFileCallback(const FilePath& path,
                                   OnceCallback<void(bool)> reply_callback) {
-  OnceCallback<void(bool)> bound_callback;
-  if (!reply_callback.is_null()) {
-    bound_callback = BindPostTask(SequencedTaskRunnerHandle::Get(),
-                                  std::move(reply_callback));
-  }
-  return BindOnce(&DeleteFileWithRetry, 0, path, std::move(bound_callback));
+  return GetDeleteFileCallbackInternal(path, /*recursive=*/false,
+                                       std::move(reply_callback));
+}
+
+OnceClosure GetDeletePathRecursivelyCallback(
+    const FilePath& path,
+    OnceCallback<void(bool)> reply_callback) {
+  return GetDeleteFileCallbackInternal(path, /*recursive=*/true,
+                                       std::move(reply_callback));
 }
 
 FilePath MakeAbsoluteFilePath(const FilePath& input) {