Add synchronous callback support to important_file_writer.cc

base/files/important_file_writer.cc

Index: base/files/important_file_writer.cc
diff --git a/base/files/important_file_writer.cc b/base/files/important_file_writer.cc
index edd400ce183f13fada7f89ad861c95e5510f7d55..f5fd4330b780e9b28d9b7b7391b8cd70c53ffee4 100644
--- a/base/files/important_file_writer.cc
+++ b/base/files/important_file_writer.cc
@@ -57,9 +57,13 @@ void LogFailure(const FilePath& path, TempFileFailure failure_code,
 
 // Helper function to call WriteFileAtomically() with a
 // std::unique_ptr<std::string>.
-bool WriteScopedStringToFileAtomically(const FilePath& path,
-                                       std::unique_ptr<std::string> data) {
-  return ImportantFileWriter::WriteFileAtomically(path, *data);
+void WriteScopedStringToFileAtomically(const FilePath& path,
+                                       std::unique_ptr<std::string> data,
+                                       Callback<void(bool success)> callback) {
+  bool result = ImportantFileWriter::WriteFileAtomically(path, *data);
+
+  if (!callback.is_null())
+    callback.Run(result);
 }
 
 }  // namespace
@@ -168,8 +172,10 @@ void ImportantFileWriter::WriteNow(std::unique_ptr<std::string> data) {
   if (HasPendingWrite())
     timer_.Stop();
 
-  auto task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data));
-  if (!PostWriteTask(task)) {
+  auto task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data),

[gab, 2016/09/19 18:01:30]

In the mindset of the comment below, I think s/auto/Closure/ makes this code
easier to read without introducing many more characters.

[proberge, 2016/09/19 21:24:25]

Done.

+                   Passed(&on_next_write_callback_));
+
+  if (!task_runner_->PostTask(FROM_HERE, MakeCriticalClosure(Bind(task)))) {

[gab, 2016/09/19 18:01:30]

|task| is already a Closure, why do we need the extra Bind() wrapping it?

[proberge, 2016/09/19 21:24:25]

Done.

     // Posting the task to background message loop is not expected
     // to fail, but if it does, avoid losing data and just hit the disk
     // on the current thread.
@@ -203,37 +209,9 @@ void ImportantFileWriter::DoScheduledWrite() {
   serializer_ = nullptr;
 }
 
-void ImportantFileWriter::RegisterOnNextSuccessfulWriteCallback(
-    const Closure& on_next_successful_write) {
-  DCHECK(on_next_successful_write_.is_null());
-  on_next_successful_write_ = on_next_successful_write;
-}
-
-bool ImportantFileWriter::PostWriteTask(const Callback<bool()>& task) {
-  // TODO(gab): This code could always use PostTaskAndReplyWithResult and let
-  // ForwardSuccessfulWrite() no-op if |on_next_successful_write_| is null, but
-  // PostTaskAndReply causes memory leaks in tests (crbug.com/371974) and
-  // suppressing all of those is unrealistic hence we avoid most of them by
-  // using PostTask() in the typical scenario below.
-  if (!on_next_successful_write_.is_null()) {
-    return PostTaskAndReplyWithResult(
-        task_runner_.get(),
-        FROM_HERE,
-        MakeCriticalClosure(task),
-        Bind(&ImportantFileWriter::ForwardSuccessfulWrite,
-             weak_factory_.GetWeakPtr()));
-  }
-  return task_runner_->PostTask(
-      FROM_HERE,
-      MakeCriticalClosure(Bind(IgnoreResult(task))));
-}
-
-void ImportantFileWriter::ForwardSuccessfulWrite(bool result) {
-  DCHECK(CalledOnValidThread());
-  if (result && !on_next_successful_write_.is_null()) {
-    on_next_successful_write_.Run();
-    on_next_successful_write_.Reset();
-  }
+void ImportantFileWriter::RegisterOnNextWriteCallback(
+    const Callback<void(bool success)>& on_next_write_callback) {
+  on_next_write_callback_ = on_next_write_callback;
 }
 
 }  // namespace base