[go: nahoru, domu]

base: Add `reply_callback` for GetDeleteFileCallback()

Currently since GetDeleteFileCallback() has retry logic on Windows,
there's a risk of accessing files in the path while it's still being
deleted (or to be deleted in the next retry). This could cause access
issues, or file/data corruption and is unsafe.

This CL updates GetDeleteFileCallback() to take an additional reply
callback, so that when the callback is run, the caller is sure that the
operation has finished, and even better, to know whether it succeeded
or failed.

Same change for GetDeletePathRecursivelyCallback(), even though it does
not support retry yet, which will be done in the next CL.

Bug: 1321695
Change-Id: I75feb5df153eb6512d8655f897737cff9617dc9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3632646
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1005255}
diff --git a/base/files/file_util.cc b/base/files/file_util.cc
index c3ffff8..0528ad26 100644
--- a/base/files/file_util.cc
+++ b/base/files/file_util.cc
@@ -14,6 +14,7 @@
 #include <fstream>
 #include <limits>
 #include <memory>
+#include <utility>
 #include <vector>
 
 #include "base/check_op.h"
@@ -25,7 +26,9 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/bind_post_task.h"
 #include "base/threading/scoped_blocking_call.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
 
 #if BUILDFLAG(IS_WIN)
@@ -34,14 +37,36 @@
 
 namespace base {
 
+namespace {
+
+void RunAndReply(OnceCallback<bool()> action_callback,
+                 OnceCallback<void(bool)> reply_callback) {
+  bool result = std::move(action_callback).Run();
+  if (!reply_callback.is_null())
+    std::move(reply_callback).Run(result);
+}
+
+}  // namespace
+
 #if !BUILDFLAG(IS_WIN)
-OnceClosure GetDeleteFileCallback(const FilePath& path) {
-  return BindOnce(IgnoreResult(&DeleteFile), path);
+OnceClosure GetDeleteFileCallback(const FilePath& path,
+                                  OnceCallback<void(bool)> reply_callback) {
+  return BindOnce(&RunAndReply, BindOnce(&DeleteFile, path),
+                  reply_callback.is_null()
+                      ? std::move(reply_callback)
+                      : BindPostTask(SequencedTaskRunnerHandle::Get(),
+                                     std::move(reply_callback)));
 }
 #endif  // !BUILDFLAG(IS_WIN)
 
-OnceClosure GetDeletePathRecursivelyCallback(const FilePath& path) {
-  return BindOnce(IgnoreResult(&DeletePathRecursively), path);
+OnceClosure GetDeletePathRecursivelyCallback(
+    const FilePath& path,
+    OnceCallback<void(bool)> reply_callback) {
+  return BindOnce(&RunAndReply, BindOnce(&DeletePathRecursively, path),
+                  reply_callback.is_null()
+                      ? std::move(reply_callback)
+                      : BindPostTask(SequencedTaskRunnerHandle::Get(),
+                                     std::move(reply_callback)));
 }
 
 int64_t ComputeDirectorySize(const FilePath& root_path) {
@@ -393,7 +418,7 @@
   const int uniquifier = GetUniquePathNumber(path);
   if (uniquifier > 0)
     return path.InsertBeforeExtensionASCII(StringPrintf(" (%d)", uniquifier));
-  return uniquifier == 0 ? path : base::FilePath();
+  return uniquifier == 0 ? path : FilePath();
 }
 
 namespace internal {