Integrate registry_hash_store_contents with the rest of tracked prefs.

components/user_prefs/tracked/pref_hash_filter.h

Index: components/user_prefs/tracked/pref_hash_filter.h
diff --git a/components/user_prefs/tracked/pref_hash_filter.h b/components/user_prefs/tracked/pref_hash_filter.h
index 8652a7f817c545a5b48e1154c60af1427eadbbbb..8602f0bd42ddf3abe3f87c49f42e436015c41227 100644
--- a/components/user_prefs/tracked/pref_hash_filter.h
+++ b/components/user_prefs/tracked/pref_hash_filter.h
@@ -15,6 +15,7 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/containers/scoped_ptr_hash_map.h"
+#include "base/files/file_path.h"
 #include "base/macros.h"
 #include "components/user_prefs/tracked/interceptable_pref_filter.h"
 #include "components/user_prefs/tracked/tracked_preference.h"
@@ -75,7 +76,9 @@ class PrefHashFilter : public InterceptablePrefFilter {
   // FinalizeFilterOnLoad.
   PrefHashFilter(
       std::unique_ptr<PrefHashStore> pref_hash_store,
+      std::unique_ptr<PrefHashStore> registry_pref_hash_store,

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

I think this can all be made generic, i.e.:

|external_validation_hash_store|

// |external_validation_hash_store| will be used (if non-null) to perform extra
validations without triggering resets.

(see other suggestions below)

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

Done.

       const std::vector<TrackedPreferenceMetadata>& tracked_preferences,
+      const base::FilePath& profile_path,

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

"profiles" are a foreign concept to components/.

It was a mistake of mine to not call this out on the interface of
RegistryHashStoreContentsWin. Please change its interface to call that string
something else (e.g. |store_key|), it just happens to users in chrome/ use the
|profile_name| as a |store_key| but the components/user_prefs/ layers shouldn't
be aware of that.

Also, why do we need the key again here? Shouldn't it already be embedded in
|external_validation_hash_store|? Oh I see it's because the former is the store,
whereas you're using it to get the HashStoreContents at run-time. In this case I
think the |external_validation_hash_store_contents| could also be passed in as
it doesn't need to be built with live data (like DictionaryHashStoreContents
does) and then the rest of the code can truly be generic (merely checking for
nullptr).

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

Done. Note that I chose to add a MakeCopy method to HashStoreContents. The
alternative, making it a RefCountedThreadSafe, was driving me crazy due to all
the existing uses of HashStoreContents which don't need to be managed by a
scoped_refptr.

       const base::Closure& on_reset_on_load,
       TrackedPreferenceValidationDelegate* delegate,
       size_t reporting_ids_count,
@@ -102,6 +105,8 @@ class PrefHashFilter : public InterceptablePrefFilter {
   // PrefFilter remaining implementation.
   void FilterUpdate(const std::string& path) override;
   void FilterSerializeData(base::DictionaryValue* pref_store_contents) override;
+  base::Closure GetPostSerializeCallback(
+      base::DictionaryValue* pref_store_contents) override;
 
  private:
   // InterceptablePrefFilter implementation.
@@ -110,6 +115,17 @@ class PrefHashFilter : public InterceptablePrefFilter {
       std::unique_ptr<base::DictionaryValue> pref_store_contents,
       bool prefs_altered) override;
 
+  // On Windows, returns a transaction acting on the registry. Returns nullptr
+  // for other OS.
+  std::unique_ptr<PrefHashStoreTransaction>
+  MaybeGetRegistryHashStoreTransaction();

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

Check |if (external_validation_hash_store)| instead of depending on Windows.

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

Done.

+
+  // Write to the registry the MACs contained in |changed_paths_and_macs|.
+  // Supports atomic and split preferences.
+  static void PrefHashFilter::FlushToRegistry(

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

FlushToExternalStore() -- no-ops (or simply not called) |if
(!external_validation_hash_store)|

Nothing else is Windows specific as it uses the HashStoreContents API :-)

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

Done.

+      base::string16 profile_name,
+      base::DictionaryValue* changed_paths_and_macs);
+
   // Callback to be invoked only once (and subsequently reset) on the next
   // FilterOnLoad event. It will be allowed to modify the |prefs| handed to
   // FilterOnLoad before handing them back to this PrefHashFilter.
@@ -120,12 +136,19 @@ class PrefHashFilter : public InterceptablePrefFilter {
   typedef base::ScopedPtrHashMap<std::string,
                                  std::unique_ptr<TrackedPreference>>
       TrackedPreferencesMap;
+
   // A map from changed paths to their corresponding TrackedPreferences (which
   // aren't owned by this map).
   typedef std::map<std::string, const TrackedPreference*> ChangedPathsMap;
 
   std::unique_ptr<PrefHashStore> pref_hash_store_;
 
+  // Expected to be nullptr for non-Windows.
+  std::unique_ptr<PrefHashStore> registry_pref_hash_store_;
+
+  // The path of the profile we're filtering prefs for.
+  const base::FilePath profile_path_;
+
   // Invoked if a reset occurs in a call to FilterOnLoad.
   const base::Closure on_reset_on_load_;