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) {