Integrate registry_hash_store_contents with the rest of tracked prefs.

components/user_prefs/tracked/pref_hash_filter.cc

Index: components/user_prefs/tracked/pref_hash_filter.cc
diff --git a/components/user_prefs/tracked/pref_hash_filter.cc b/components/user_prefs/tracked/pref_hash_filter.cc
index 2f0c6e71f5f38fb0f878f5d0c3c64fb2207efe83..a7211223591c7f0e4d6a7f774e2cf8d5f3751372 100644
--- a/components/user_prefs/tracked/pref_hash_filter.cc
+++ b/components/user_prefs/tracked/pref_hash_filter.cc
@@ -24,6 +24,14 @@
 #include "components/user_prefs/tracked/tracked_atomic_preference.h"
 #include "components/user_prefs/tracked/tracked_split_preference.h"
 
+#if defined(OS_WIN)
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/installer/util/browser_distribution.h"
+#include "components/user_prefs/tracked/registry_hash_store_contents_win.h"
+#endif
+
 namespace {
 
 void CleanupDeprecatedTrackedPreferences(
@@ -47,16 +55,30 @@ void CleanupDeprecatedTrackedPreferences(
   }
 }
 
+std::unique_ptr<HashStoreContents> MaybeGetRegistryHashStoreContents(
+    base::string16 profile_name) {
+#if defined(OS_WIN)
+  return base::WrapUnique(new RegistryHashStoreContentsWin(
+      BrowserDistribution::GetDistribution()->GetRegistryPath(), profile_name));
+#else
+  return nullptr;
+#endif
+}
+
 }  // namespace
 
 PrefHashFilter::PrefHashFilter(
     std::unique_ptr<PrefHashStore> pref_hash_store,
+    std::unique_ptr<PrefHashStore> registry_pref_hash_store,
     const std::vector<TrackedPreferenceMetadata>& tracked_preferences,
+    const base::FilePath& profile_path,
     const base::Closure& on_reset_on_load,
     TrackedPreferenceValidationDelegate* delegate,
     size_t reporting_ids_count,
     bool report_super_mac_validity)
     : pref_hash_store_(std::move(pref_hash_store)),
+      registry_pref_hash_store_(std::move(registry_pref_hash_store)),
+      profile_path_(profile_path),
       on_reset_on_load_(on_reset_on_load),
       report_super_mac_validity_(report_super_mac_validity) {
   DCHECK(pref_hash_store_);
@@ -153,17 +175,91 @@ void PrefHashFilter::FilterUpdate(const std::string& path) {
     changed_paths_.insert(std::make_pair(path, it->second));
 }
 
+// static
+void PrefHashFilter::FlushToRegistry(
+    base::string16 profile_name,
+    base::DictionaryValue* changed_paths_and_macs) {
+  DCHECK(!changed_paths_and_macs->empty());
+
+  std::unique_ptr<HashStoreContents> registry_hash_store_contents(
+      MaybeGetRegistryHashStoreContents(profile_name));
+
+  // We shouldn't be here if the Windows registry is unsupported.
+  DCHECK(registry_hash_store_contents);
+  if (!registry_hash_store_contents)
+    return;
+
+  for (base::DictionaryValue::Iterator it(*changed_paths_and_macs);
+       !it.IsAtEnd(); it.Advance()) {
+    const std::string& changed_path = it.key();
+
+    const base::DictionaryValue* split_values = NULL;
+    if (it.value().GetAsDictionary(&split_values)) {
+      for (base::DictionaryValue::Iterator inner_it(*split_values);
+           !inner_it.IsAtEnd(); inner_it.Advance()) {
+        std::string mac;
+        bool is_string = inner_it.value().GetAsString(&mac);
+        DCHECK(is_string);
+
+        registry_hash_store_contents->SetSplitMac(changed_path, inner_it.key(),
+                                                  mac);
+      }
+    } else {
+      std::string mac;
+      bool is_string = it.value().GetAsString(&mac);
+      DCHECK(is_string);
+
+      registry_hash_store_contents->SetMac(changed_path, mac);
+    }
+  }
+}
+
+base::Closure PrefHashFilter::GetPostSerializeCallback(

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

This will become private with PrefFilter API change suggestion.

Suggest renaming to "GetOnNextWriteSynchronousCallback"

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

Done.

+    base::DictionaryValue* pref_store_contents) {
+  if (changed_paths_.empty() || !registry_pref_hash_store_) {
+    return base::Closure();
+  }
+
+  base::DictionaryValue* changed_paths_macs = new base::DictionaryValue;

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

Use a std::unique_ptr here and use base::Passed instead of base::Owned in
Bind().

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

Done.

+
+  for (ChangedPathsMap::const_iterator it = changed_paths_.begin();
+       it != changed_paths_.end(); ++it) {
+    const std::string& changed_path = it->first;
+    const base::Value* value = NULL;

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

s/NULL/nullptr/ (here and elsewhere in this CL -- changed as of C++11)

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

Done.

+    pref_store_contents->Get(changed_path, &value);
+
+    const base::DictionaryValue* dict_value = NULL;
+    if (value && value->GetAsDictionary(&dict_value)) {

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

Dictionary doesn't mean it's a split mac (it's okay to store dictionaries as
atomic as well, split macs is only when someone wants to split the values in a
dictionary -- i.e. extensions.settings)

An alternative might be to use:

const TrackedPreference* changed_preference = it->second;
changed_preference->GetType()

(and introduce TrackedPreference:GetType() => ATOMIC/SPLIT)

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

Done.

+      // Don't break 'extensions.settings' into two DictionaryValues.

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

'extensions.settings' is too specific for a comment here.

And I don't think we need this comment anyway, it's logical to use
"WithoutPathExpansion" when the input is used as a "key" rather than a "path".

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

Done.

+      changed_paths_macs->SetWithoutPathExpansion(
+          changed_path, registry_pref_hash_store_->ComputeSplitMacs(
+                            changed_path, dict_value));
+    } else {
+      // Don't break 'session.restore_on_startup' into two DictionaryValues.

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

rm comment

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

Done.

+      changed_paths_macs->SetStringWithoutPathExpansion(
+          changed_path,
+          registry_pref_hash_store_->ComputeMac(changed_path, value));
+    }
+  }
+
+  return base::Bind(&FlushToRegistry,
+                    profile_path_.BaseName().LossyDisplayName(),
+                    base::Owned(changed_paths_macs));
+}
+
 // Updates the stored hashes for |changed_paths_| before serializing data to
 // disk. This is required as storing the hash everytime a pref's value changes
 // is too expensive (see perf regression @ http://crbug.com/331273).
 void PrefHashFilter::FilterSerializeData(
     base::DictionaryValue* pref_store_contents) {
   if (!changed_paths_.empty()) {
-    base::TimeTicks checkpoint = base::TimeTicks::Now();
-    {
       std::unique_ptr<PrefHashStoreTransaction> hash_store_transaction(
           pref_hash_store_->BeginTransaction(std::unique_ptr<HashStoreContents>(
               new DictionaryHashStoreContents(pref_store_contents))));
+
+      std::unique_ptr<PrefHashStoreTransaction> registry_hash_store_transaction(
+          MaybeGetRegistryHashStoreTransaction());

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

std::unique_ptr<PrefHashStoreTransaction> external_hash_store_transaction =
    external_validation_hash_store_contents
        ? WrapUnique(new
PrefHashStoreTransaction(external_validation_hash_store_contents))
        : nullptr;

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

Done.

+
       for (ChangedPathsMap::const_iterator it = changed_paths_.begin();
            it != changed_paths_.end(); ++it) {
         const std::string& changed_path = it->first;
@@ -171,14 +267,12 @@ void PrefHashFilter::FilterSerializeData(
         const base::Value* value = NULL;
         pref_store_contents->Get(changed_path, &value);
         changed_preference->OnNewValue(value, hash_store_transaction.get());
+
+        // Clear prefs in the registry, if available.
+        if (registry_hash_store_transaction)
+          registry_hash_store_transaction->ClearHash(changed_path);
       }
       changed_paths_.clear();
-    }
-    // TODO(gab): Remove this histogram by Feb 21 2014; after sufficient timing
-    // data has been gathered from the wild to be confident this doesn't
-    // significantly affect performance on the UI thread.

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

Remove comment but let's leave the stat in place (and monitor it to make sure it
doesn't regress now that we will compute GetOnNextWriteSynchronousCallback() in
FilterSerializeData).

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

Done.

-    UMA_HISTOGRAM_TIMES("Settings.FilterSerializeDataTime",
-                        base::TimeTicks::Now() - checkpoint);
   }
 }
 
@@ -187,7 +281,6 @@ void PrefHashFilter::FinalizeFilterOnLoad(
     std::unique_ptr<base::DictionaryValue> pref_store_contents,
     bool prefs_altered) {
   DCHECK(pref_store_contents);
-  base::TimeTicks checkpoint = base::TimeTicks::Now();
 
   bool did_reset = false;
   {
@@ -195,6 +288,9 @@ void PrefHashFilter::FinalizeFilterOnLoad(
         pref_hash_store_->BeginTransaction(std::unique_ptr<HashStoreContents>(
             new DictionaryHashStoreContents(pref_store_contents.get()))));
 
+    std::unique_ptr<PrefHashStoreTransaction> registry_hash_store_transaction(
+        MaybeGetRegistryHashStoreTransaction());
+
     CleanupDeprecatedTrackedPreferences(
         pref_store_contents.get(), hash_store_transaction.get());
 
@@ -206,7 +302,8 @@ void PrefHashFilter::FinalizeFilterOnLoad(
     for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin();
          it != tracked_paths_.end(); ++it) {
       if (it->second->EnforceAndReport(pref_store_contents.get(),
-                                       hash_store_transaction.get())) {
+                                       hash_store_transaction.get(),
+                                       registry_hash_store_transaction.get())) {
         did_reset = true;
         prefs_altered = true;
       }
@@ -225,12 +322,18 @@ void PrefHashFilter::FinalizeFilterOnLoad(
       on_reset_on_load_.Run();
   }
 
-  // TODO(gab): Remove this histogram by Feb 21 2014; after sufficient timing
-  // data has been gathered from the wild to be confident this doesn't
-  // significantly affect startup.

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

Same here, remove comment but leave stat.

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

Done.

-  UMA_HISTOGRAM_TIMES("Settings.FilterOnLoadTime",
-                      base::TimeTicks::Now() - checkpoint);
-
   post_filter_on_load_callback.Run(std::move(pref_store_contents),
                                    prefs_altered);
 }
+
+std::unique_ptr<PrefHashStoreTransaction>
+PrefHashFilter::MaybeGetRegistryHashStoreTransaction() {
+#if defined(OS_WIN)
+  if (registry_pref_hash_store_) {
+    return registry_pref_hash_store_->BeginTransaction(
+        MaybeGetRegistryHashStoreContents(
+            profile_path_.BaseName().LossyDisplayName()));
+  }
+#endif
+  return nullptr;
+}