Integrate registry_hash_store_contents with the rest of tracked prefs.

chrome/browser/prefs/profile_pref_store_manager.cc

Index: chrome/browser/prefs/profile_pref_store_manager.cc
diff --git a/chrome/browser/prefs/profile_pref_store_manager.cc b/chrome/browser/prefs/profile_pref_store_manager.cc
index faba02c8c1afb9a9bf0bc892bdd978d658fd4eb2..93fcc130c81d9b89d363a1e4fc318536eb6e2dbb 100644
--- a/chrome/browser/prefs/profile_pref_store_manager.cc
+++ b/chrome/browser/prefs/profile_pref_store_manager.cc
@@ -23,6 +23,11 @@
 #include "components/user_prefs/tracked/segregated_pref_store.h"
 #include "components/user_prefs/tracked/tracked_preferences_migration.h"
 
+#if defined(OS_WIN)
+#include "chrome/installer/util/browser_distribution.h"
+#include "components/user_prefs/tracked/registry_hash_store_contents_win.h"
+#endif
+
 namespace {
 
 void RemoveValueSilently(const base::WeakPtr<JsonPrefStore> pref_store,
@@ -33,8 +38,25 @@ void RemoveValueSilently(const base::WeakPtr<JsonPrefStore> pref_store,
   }
 }
 
+#if defined(OS_WIN)
+// Whether we are in testing mode; can be enabled via
+// UseTestingPreferenceValidationRegistryPath(). Forces a different registry key
+// to be used for storing preference validation MACs.
+base::string16 g_preference_validation_registry_path_for_testing;

[gab, 2016/08/08 04:37:44]

This will result in an undesired static initializer, global variables have to be
POD types, given this is always used from the same thread you can make it a raw
pointer initialized to |nullptr| (otherwise you would have had to use a
base::LazyInstance).

[proberge, 2016/08/31 17:30:15]

Done.

+#endif  // OS_WIN
+
 }  // namespace
 
+namespace chrome_prefs {
+
+void SetPreferenceValidationRegistryPathForTesting(base::string16 path) {
+#if defined(OS_WIN)

[gab, 2016/08/08 04:37:44]

Also DCHECK(!path.empty()) maybe?

[proberge, 2016/08/31 17:30:15]

Done.

+  g_preference_validation_registry_path_for_testing = path;
+#endif  // OS_WIN
+}
+
+}  // namespace chrome_prefs
+
 // Preference tracking and protection is not required on platforms where other
 // apps do not have access to chrome's persistent storage.
 const bool ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking =
@@ -108,12 +130,15 @@ PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
   }
 
   std::unique_ptr<PrefHashFilter> unprotected_pref_hash_filter(
-      new PrefHashFilter(GetPrefHashStore(false), unprotected_configuration,
-                         base::Closure(), validation_delegate,
-                         reporting_ids_count_, false));
+      new PrefHashFilter(GetPrefHashStore(false),
+                         GetExternalVerificationPrefHashStore(),
+                         GetExternalVerificationPrefHashStoreContents(),
+                         unprotected_configuration, base::Closure(),
+                         validation_delegate, reporting_ids_count_, false));
   std::unique_ptr<PrefHashFilter> protected_pref_hash_filter(new PrefHashFilter(
-      GetPrefHashStore(true), protected_configuration, on_reset_on_load,
-      validation_delegate, reporting_ids_count_, true));
+      GetPrefHashStore(true), GetExternalVerificationPrefHashStore(),
+      GetExternalVerificationPrefHashStoreContents(), protected_configuration,
+      on_reset_on_load, validation_delegate, reporting_ids_count_, true));
 
   PrefHashFilter* raw_unprotected_pref_hash_filter =
       unprotected_pref_hash_filter.get();
@@ -134,9 +159,9 @@ PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
       unprotected_pref_names, protected_pref_names,
       base::Bind(&RemoveValueSilently, unprotected_pref_store->AsWeakPtr()),
       base::Bind(&RemoveValueSilently, protected_pref_store->AsWeakPtr()),
-      base::Bind(&JsonPrefStore::RegisterOnNextSuccessfulWriteCallback,
+      base::Bind(&JsonPrefStore::RegisterOnNextSuccessfulWriteReply,

[gab, 2016/08/08 04:37:44]

I'd like us to break down this CL as much as possible, +1200 lines is a massive
CL. The changes to the ImportantFileWriter API and incurred side-effects are a
good candidate for the first CL.

[proberge, 2016/08/31 17:30:15]

Acknowledged.

                  unprotected_pref_store->AsWeakPtr()),
-      base::Bind(&JsonPrefStore::RegisterOnNextSuccessfulWriteCallback,
+      base::Bind(&JsonPrefStore::RegisterOnNextSuccessfulWriteReply,
                  protected_pref_store->AsWeakPtr()),
       GetPrefHashStore(false), GetPrefHashStore(true),
       raw_unprotected_pref_hash_filter, raw_protected_pref_hash_filter);
@@ -158,12 +183,11 @@ bool ProfilePrefStoreManager::InitializePrefsFromMasterPrefs(
   if (kPlatformSupportsPreferenceTracking) {
     copy.reset(master_prefs.DeepCopy());
     to_serialize = copy.get();
-    PrefHashFilter(GetPrefHashStore(false),
-                   tracking_configuration_,
-                   base::Closure(),
-                   NULL,
-                   reporting_ids_count_,
-                   false).Initialize(copy.get());
+    PrefHashFilter(
+        GetPrefHashStore(false), GetExternalVerificationPrefHashStore(),
+        GetExternalVerificationPrefHashStoreContents(), tracking_configuration_,
+        base::Closure(), NULL, reporting_ids_count_, false)
+        .Initialize(copy.get());
   }
 
   // This will write out to a single combined file which will be immediately
@@ -189,3 +213,33 @@ std::unique_ptr<PrefHashStore> ProfilePrefStoreManager::GetPrefHashStore(
   return std::unique_ptr<PrefHashStore>(
       new PrefHashStoreImpl(seed_, device_id_, use_super_mac));
 }
+
+std::unique_ptr<PrefHashStore>
+ProfilePrefStoreManager::GetExternalVerificationPrefHashStore() {
+  DCHECK(kPlatformSupportsPreferenceTracking);
+#if defined(OS_WIN)
+  return std::unique_ptr<PrefHashStore>(
+      new PrefHashStoreImpl("ChromiumRegistryHashStoreValidationSeed",

[gab, 2016/08/08 04:37:44]

Use Chrome instead of Chromium I'd say (this is the general recommendation in
comments too : Chromium is the project, Chrome is the product).

[proberge, 2016/08/31 17:30:15]

Done.

+                            device_id_, false /* use_super_mac */));
+#else
+  return nullptr;
+#endif
+}
+
+std::unique_ptr<HashStoreContents>
+ProfilePrefStoreManager::GetExternalVerificationPrefHashStoreContents() {
+  DCHECK(kPlatformSupportsPreferenceTracking);
+#if defined(OS_WIN)
+  if (g_preference_validation_registry_path_for_testing.size() > 0) {

[gab, 2016/08/08 04:37:44]

!empty() instead of size() > 0

(but actually now that it will be a pointer, simply test for nullptr)

[proberge, 2016/08/31 17:30:15]

Done.

+    return std::unique_ptr<HashStoreContents>(new RegistryHashStoreContentsWin(
+        g_preference_validation_registry_path_for_testing,
+        profile_path_.BaseName().LossyDisplayName()));
+  }
+
+  return std::unique_ptr<HashStoreContents>(new RegistryHashStoreContentsWin(
+      BrowserDistribution::GetDistribution()->GetRegistryPath(),
+      profile_path_.BaseName().LossyDisplayName()));
+#else
+  return nullptr;
+#endif
+}