Integrate registry_hash_store_contents with the rest of tracked prefs.

base/files/important_file_writer.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/files/important_file_writer.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string>
@@ skipping 40 matching lines @@
 void LogFailure(const FilePath& path, TempFileFailure failure_code,
                 StringPiece message) {
   UMA_HISTOGRAM_ENUMERATION("ImportantFile.TempFileFailures", failure_code,
                             TEMP_FILE_FAILURE_MAX);
   DPLOG(WARNING) << "temp file failure: " << path.value() << " : " << message;
 }
 
 // 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);
+                                       std::unique_ptr<std::string> data,
+                                       base::Closure callback) {
+  bool result = ImportantFileWriter::WriteFileAtomically(path, *data);
+
+  if (result && !callback.is_null())
+    callback.Run();
+
+  return result;
 }
 
 }  // namespace
 
 // static
 bool ImportantFileWriter::WriteFileAtomically(const FilePath& path,
                                               StringPiece data) {
 #if defined(OS_CHROMEOS)
   // On Chrome OS, chrome gets killed when it cannot finish shutdown quickly,
   // and this function seems to be one of the slowest shutdown steps.
@@ skipping 88 matching lines @@
 void ImportantFileWriter::WriteNow(std::unique_ptr<std::string> data) {
   DCHECK(CalledOnValidThread());
   if (!IsValueInRangeForNumericType<int32_t>(data->length())) {
     NOTREACHED();
     return;
   }
 
   if (HasPendingWrite())
     timer_.Stop();
 
-  auto task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data));
+  auto task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data),
+                   blocking_on_next_successful_write_);
+  blocking_on_next_successful_write_.Reset();

[gab, 2016/08/03 18:19:34]

This won't respect the contract (it won't stay around if the first write is a
failure -- to do so you'd have to only Reset() it in the
ForwardSuccessfulWrite() on success).

But maybe that's not the contract you want?

Maybe what you want is more:

  // Registers |on_next_write_callback| to be synchronously invoked from
  // WriteFileAtomically() on its next write (i.e. from |task_runner_|), with
  // |success| indicating whether it succeeded or not.
  void RegisterOnWriteSynchronousCallback(
      const Callback<void(bool success)>& on_next_write_callback);

Then you can put the old registry entries back on failure? Or you may decide to
not do anything on failure (if it's too complicated to implement a rollback for
a use case that will pretty much never happen) -- i.e. just report failure with
UMA at first to see if it even matters to handle it.
But I'm pretty sure you don't want the API to let the Callback stick around
until the next successful write (as that wouldn't match the MACs you saved
anyways so dumping them to the registry doesn't make sense -- it does make sense
for the existing use case which is merely waiting for "a" successful write to
complete the migration).

[proberge, 2016/08/04 00:13:45]

Right, unlike the migration case we care about a specific write, not any
successful write. I updated the contract to take in a bool callback.

+
   if (!PostWriteTask(task)) {
     // 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.
     NOTREACHED();
 
     task.Run();
   }
 }
 
@@ skipping 20 matching lines @@
   }
   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;
 }
 
+void ImportantFileWriter::RegisterOnNextSuccessfulWriteBlockingCallback(
+    const Closure& on_next_successful_write) {
+  DCHECK(blocking_on_next_successful_write_.is_null());
+  blocking_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();
   }
 }
 
 }  // namespace base