Integrate registry_hash_store_contents with the rest of tracked prefs.

components/prefs/json_pref_store.cc

 // Copyright (c) 2012 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 "components/prefs/json_pref_store.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 154 matching lines @@
       alternate_path_(pref_alternate_filename),
       sequenced_task_runner_(sequenced_task_runner),
       prefs_(new base::DictionaryValue()),
       read_only_(false),
       writer_(pref_filename, sequenced_task_runner),
       pref_filter_(std::move(pref_filter)),
       initialized_(false),
       filtering_in_progress_(false),
       pending_lossy_write_(false),
       read_error_(PREF_READ_ERROR_NONE),
-      has_pending_successful_write_reply_(false),
-      has_pending_write_callbacks_(false),
+      has_pending_write_reply_(false),
       write_count_histogram_(writer_.commit_interval(), path_) {
   DCHECK(!path_.empty());
 }
 
 bool JsonPrefStore::GetValue(const std::string& key,
                              const base::Value** result) const {
   DCHECK(CalledOnValidThread());
 
   base::Value* tmp = nullptr;
   if (!prefs_->Get(key, &tmp))
@@ skipping 136 matching lines @@
 
   FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key));
 
   ScheduleWrite(flags);
 }
 
 void JsonPrefStore::RunOrScheduleNextSuccessfulWriteCallback(
     bool write_success) {
   DCHECK(CalledOnValidThread());
 
-  has_pending_write_callbacks_ = false;
-  if (has_pending_successful_write_reply_) {
-    has_pending_successful_write_reply_ = false;
+  has_pending_write_reply_ = false;
+  if (!on_next_successful_write_reply_.is_null()) {
+    base::Closure on_successful_write = on_next_successful_write_reply_;

[proberge, 2016/10/04 20:17:58]

is this fine? I need to clear on_next_successful_reply_ before re-registering
the callback with RegisterOnNextSuccessfulWriteReply

[gab, 2016/10/04 20:25:13]

Yes except do:

base::Closure on_successful_write = std::move(on_next_successful_write_reply_);

instead.

[proberge, 2016/10/04 20:40:49]

Done.

+    on_next_successful_write_reply_ = base::Closure();

[gab, 2016/10/04 20:25:40]

And of course the move makes this line not be required, just to be clear.

[proberge, 2016/10/04 20:40:49]

Done.

     if (write_success) {
-      on_next_successful_write_reply_.Run();
+      on_successful_write.Run();
     } else {
-      RegisterOnNextSuccessfulWriteReply(on_next_successful_write_reply_);
+      RegisterOnNextSuccessfulWriteReply(on_successful_write);
     }
   }
 }
 
 // static
 void JsonPrefStore::PostWriteCallback(
     const base::Callback<void(bool success)>& on_next_write_reply,
     const base::Callback<void(bool success)>& on_next_write_callback,
     scoped_refptr<base::SequencedTaskRunner> reply_task_runner,
     bool write_success) {
   if (!on_next_write_callback.is_null())
     on_next_write_callback.Run(write_success);
 
   // We can't run |on_next_write_reply| on the current thread. Bounce back to
   // the |reply_task_runner| which is the correct sequenced thread.
   reply_task_runner->PostTask(FROM_HERE,
                               base::Bind(on_next_write_reply, write_success));
 }
 
 void JsonPrefStore::RegisterOnNextSuccessfulWriteReply(
     const base::Closure& on_next_successful_write_reply) {
   DCHECK(CalledOnValidThread());
-  DCHECK(!has_pending_successful_write_reply_);
+  DCHECK(on_next_successful_write_reply_.is_null());
 
-  has_pending_successful_write_reply_ = true;
   on_next_successful_write_reply_ = on_next_successful_write_reply;
 
   // If there are pending callbacks, avoid erasing them; the reply will be used
   // as we set |on_next_successful_write_reply_|. Otherwise, setup a reply with
   // an empty callback.
-  if (!has_pending_write_callbacks_) {
+  if (!has_pending_write_reply_) {
+    has_pending_write_reply_ = true;
     writer_.RegisterOnNextWriteCallbacks(
         base::Closure(),
         base::Bind(
             &PostWriteCallback,
             base::Bind(&JsonPrefStore::RunOrScheduleNextSuccessfulWriteCallback,
                        AsWeakPtr()),
             base::Callback<void(bool success)>(),
             base::SequencedTaskRunnerHandle::Get()));
   }
 }
 
 void JsonPrefStore::RegisterOnNextWriteSynchronousCallbacks(
     OnWriteCallbackPair callbacks) {
   DCHECK(CalledOnValidThread());
-  DCHECK(!has_pending_write_callbacks_);
 
-  has_pending_write_callbacks_ = true;
+  has_pending_write_reply_ = true;
 
   writer_.RegisterOnNextWriteCallbacks(
       callbacks.first,
       base::Bind(
           &PostWriteCallback,
           base::Bind(&JsonPrefStore::RunOrScheduleNextSuccessfulWriteCallback,
                      AsWeakPtr()),
           callbacks.second, base::SequencedTaskRunnerHandle::Get()));

[gab, 2016/10/04 20:25:13]

nit: Just noticed while reading this closely that it's a bit weird that it reads
in order: callbacks.first, RunOrScheduleNextSuccessfulWriteCallback,
callbacks.second.

WDYT of flipping the args of PostWriteCallback() so that
|on_next_write_callback| is first?

[proberge, 2016/10/04 20:40:49]

Done.

 }
 
 void JsonPrefStore::ClearMutableValues() {
   NOTIMPLEMENTED();
 }
 
 void JsonPrefStore::OnFileRead(std::unique_ptr<ReadResult> read_result) {
   DCHECK(CalledOnValidThread());
 
   DCHECK(read_result);
@@ skipping 196 matching lines @@
   DCHECK_EQ(31, num_buckets);
 
   // The histogram below is an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS
   // macro adapted to allow for a dynamically suffixed histogram name.
   // Note: The factory creates and owns the histogram.
   base::HistogramBase* histogram = base::Histogram::FactoryGet(
       histogram_name, min_value, max_value, num_buckets,
       base::HistogramBase::kUmaTargetedHistogramFlag);
   return histogram;
 }