Integrate registry_hash_store_contents with the rest of tracked prefs.

chrome/browser/prefs/tracked/pref_hash_browsertest.cc

 // Copyright 2014 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 <memory>
 #include <string>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 20 matching lines @@
 #include "chrome/test/base/testing_profile.h"
 #include "components/search_engines/default_search_manager.h"
 #include "components/user_prefs/tracked/tracked_preference_histogram_names.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/extension.h"
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/chromeos_switches.h"
 #endif
 
+#if defined(OS_WIN)
+#include "base/test/test_reg_util_win.h"
+#endif
+
 namespace {
 
 // Extension ID of chrome/test/data/extensions/good.crx
 const char kGoodCrxId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 
+// Prefix for the registry path.
+constexpr base::char16 kRegistryTestPathPrefix[] =
+    L"SOFTWARE\\Chromium\\PrefHashBrowserTest\\";
+
 // Explicit expectations from the caller of GetTrackedPrefHistogramCount(). This
 // enables detailed reporting of the culprit on failure.
 enum AllowedBuckets {
   // Allow no samples in any buckets.
   ALLOW_NONE = -1,
   // Any integer between BEGIN_ALLOW_SINGLE_BUCKET and END_ALLOW_SINGLE_BUCKET
   // indicates that only this specific bucket is allowed to have a sample.
   BEGIN_ALLOW_SINGLE_BUCKET = 0,
   END_ALLOW_SINGLE_BUCKET = 100,
   // Allow any buckets (no extra verifications performed).
   ALLOW_ANY
 };
 
+base::string16 GetRegistryPathForTestProfile() {
+  base::FilePath profile_dir;
+  EXPECT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &profile_dir));
+  return kRegistryTestPathPrefix + profile_dir.BaseName().value();
+}
+
 // Returns the number of times |histogram_name| was reported so far; adding the
 // results of the first 100 buckets (there are only ~19 reporting IDs as of this
 // writing; varies depending on the platform). |allowed_buckets| hints at extra
 // requirements verified in this method (see AllowedBuckets for details).
 int GetTrackedPrefHistogramCount(const char* histogram_name,
+                                 const char* histogram_suffix,
                                  int allowed_buckets) {
+  std::string full_histogram_name(histogram_name);
+  if (*histogram_suffix != 0)
+    full_histogram_name.append(".").append(histogram_suffix);
   const base::HistogramBase* histogram =
-      base::StatisticsRecorder::FindHistogram(histogram_name);
+      base::StatisticsRecorder::FindHistogram(full_histogram_name);
   if (!histogram)
     return 0;
 
   std::unique_ptr<base::HistogramSamples> samples(histogram->SnapshotSamples());
   int sum = 0;
   for (int i = 0; i < 100; ++i) {
     int count_for_id = samples->GetCount(i);
     EXPECT_GE(count_for_id, 0);
     sum += count_for_id;
 
     if (allowed_buckets == ALLOW_NONE ||
         (allowed_buckets != ALLOW_ANY && i != allowed_buckets)) {
       EXPECT_EQ(0, count_for_id) << "Unexpected reporting_id: " << i;
     }
   }
   return sum;
 }
 
+// Helper function to call GetTrackedPrefHistogramCount with no external
+// validation suffix.
+int GetTrackedPrefHistogramCount(const char* histogram_name,
+                                 int allowed_buckets) {
+  return GetTrackedPrefHistogramCount(histogram_name, "", allowed_buckets);
+}
+
 std::unique_ptr<base::DictionaryValue> ReadPrefsDictionary(
     const base::FilePath& pref_file) {
   JSONFileValueDeserializer deserializer(pref_file);
   int error_code = JSONFileValueDeserializer::JSON_NO_ERROR;
   std::string error_str;
   std::unique_ptr<base::Value> prefs =
       deserializer.Deserialize(&error_code, &error_str);
   if (!prefs || error_code != JSONFileValueDeserializer::JSON_NO_ERROR) {
     ADD_FAILURE() << "Error #" << error_code << ": " << error_str;
     return std::unique_ptr<base::DictionaryValue>();
   }
   if (!prefs->IsType(base::Value::TYPE_DICTIONARY)) {
     ADD_FAILURE();
     return std::unique_ptr<base::DictionaryValue>();
   }
   return std::unique_ptr<base::DictionaryValue>(
       static_cast<base::DictionaryValue*>(prefs.release()));
 }
 
+// Returns whether external validation is supported on the platform through
+// storing MACs in the registry.
+bool SupportsRegistryValidation() {
+#if defined(OS_WIN)
+  return true;
+#else
+  return false;
+#endif
+}
+
 #define PREF_HASH_BROWSER_TEST(fixture, test_name)                          \
   IN_PROC_BROWSER_TEST_P(fixture, PRE_##test_name) {                        \
     SetupPreferences();                                                     \
   }                                                                         \
   IN_PROC_BROWSER_TEST_P(fixture, test_name) {                              \
     VerifyReactionToPrefAttack();                                           \
   }                                                                         \
   INSTANTIATE_TEST_CASE_P(                                                  \
       fixture##Instance,                                                    \
       fixture,                                                              \
@@ skipping 112 matching lines @@
 
     return true;
   }
 
   void SetUpInProcessBrowserTestFixture() override {
     ExtensionBrowserTest::SetUpInProcessBrowserTestFixture();
 
     // Bots are on a domain, turn off the domain check for settings hardening in
     // order to be able to test all SettingsEnforcement groups.
     chrome_prefs::DisableDomainCheckForTesting();
+
+#if defined(OS_WIN)
+    // Avoid polluting prefs for the user and the bots by writing to a specific
+    // testing registry path.
+    base::string16 registry_key = GetRegistryPathForTestProfile();

[gab, 2016/09/16 19:47:32]

Override the entire registry hive for the test instead of doing manually (and
also note that your current approach would be flaky when tests are run in
parallel per using the same key for multiple tests).

i.e. have a registry_util::RegistryOverrideManager registry_override_manager_;
member and here do

registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER);

Then none of this custom registry overriding logic needs to be there :-)


The one tricky thing here is that you need to preserve the override across PRE_
tests which isn't currently supported. As such we have two options:

1) Add something like RegistryOverrideManager::SerializeOverride(HKEY override,
const FilePath& file) and RegistryOverrideManager::DeserializeOverride(HKEY
override, const FilePath& file) which takes the existing override and saves its
state to disk so that it can be regenerated in the next test (and unwinds the
override in serialize phase so that the destructor leaves it alone instead of
cleaning it up).

Note: PathService::Get(chrome::DIR_USER_DATA,...) remains the same between PRE_
and main test so we can serialize files in there if needed.

or

2) Since we are the only ones needing this, it's fine to do essentially that but
ourselves (i.e. self-managing our registry override instead of using
RegistryOverrideManager).

[proberge, 2016/09/20 21:35:44]

This was tricky to get working. It was a few weeks ago so I might not remember
all of it, but the current approach uses a different key for multiple tests.
GetRegistryPathForTestProfile() uses PathService::Get(chrome::DIR_USER_DATA ...)
to get a registry path which is different between tests, but the same for PRE_
and associated main test.

In other words, I'm doing option #2 but without the file serialization.

+    ProfilePrefStoreManager::SetPreferenceValidationRegistryPathForTesting(
+        registry_key);
+
+    // For PRE tests, delete the Registry key to avoid collisions.
+    if (IsPRETest()) {
+      base::win::RegKey key;
+      if (key.Open(HKEY_CURRENT_USER, registry_key.c_str(),
+                   KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
+        LONG result = key.DeleteKey(L"");
+        ASSERT_TRUE(result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND);
+      }
+    }
+#endif
+  }
+
+  void TearDown() override {
+#if defined(OS_WIN)
+    // For non-PRE tests, delete the Registry key to avoid polluting the
+    // registry.
+    // TODO(proberge): it would be nice to delete keys from interrupted tests
+    // as well.
+    base::string16 registry_key = GetRegistryPathForTestProfile();
+    if (!IsPRETest()) {
+      base::win::RegKey key;
+      if (key.Open(HKEY_CURRENT_USER, registry_key.c_str(),
+                   KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
+        LONG result = key.DeleteKey(L"");
+        ASSERT_TRUE(result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND);
+      }
+    }
+#endif
+    ExtensionBrowserTest::TearDown();
   }
 
   // In the PRE_ test, find the number of tracked preferences that were
   // initialized and save it to a file to be read back in the main test and used
   // as the total number of tracked preferences.
   void SetUpOnMainThread() override {
     ExtensionBrowserTest::SetUpOnMainThread();
 
     // File in which the PRE_ test will save the number of tracked preferences
     // on this platform.
@@ skipping 13 matching lines @@
 
       // Split tracked prefs are reported as Unchanged not as NullInitialized
       // when an empty dictionary is encountered on first run (this should only
       // hit for pref #5 in the current design).
       int num_split_tracked_prefs = GetTrackedPrefHistogramCount(
           user_prefs::tracked::kTrackedPrefHistogramUnchanged,
           BEGIN_ALLOW_SINGLE_BUCKET + 5);
       EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
                 num_split_tracked_prefs);
 
+      if (SupportsRegistryValidation()) {
+        // Same checks as above, but for the registry.
+        num_tracked_prefs_ = GetTrackedPrefHistogramCount(
+            user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
+            user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+            ALLOW_ANY);
+        EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM,
+                  num_tracked_prefs_ > 0);
+
+        int num_split_tracked_prefs = GetTrackedPrefHistogramCount(
+            user_prefs::tracked::kTrackedPrefHistogramUnchanged,
+            user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+            BEGIN_ALLOW_SINGLE_BUCKET + 5);
+        EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
+                  num_split_tracked_prefs);

[gab, 2016/09/16 19:47:32]

Instead of duplicating every check, would it be possible to have a method that
can be called that does the old check + the new check when it applies. Then
redirect all existing checks to it?

[proberge, 2016/09/20 21:35:44]

It would be hard to read the test. The method would need to take in a the
current GetTrackedPrefHistogramCount parameters and the two EXPECT_EQ arguments.

Consider this case:
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM,
num_tracked_prefs_ > 0);

The second argument relies on the result of GetTrackedPrefHistogramCount, so we
would want the last parameter to be a function "X > 0" which we then bind the
result of GetTrackedPrefHistogramCount to. Afaik this would be pretty verbose in
C++. 

+      }
+
       num_tracked_prefs_ += num_split_tracked_prefs;
 
       std::string num_tracked_prefs_str = base::IntToString(num_tracked_prefs_);
       EXPECT_EQ(static_cast<int>(num_tracked_prefs_str.size()),
                 base::WriteFile(num_tracked_prefs_file,
                                 num_tracked_prefs_str.c_str(),
                                 num_tracked_prefs_str.size()));
     } else {
       std::string num_tracked_prefs_str;
       EXPECT_TRUE(base::ReadFileToString(num_tracked_prefs_file,
@@ skipping 121 matching lines @@
               GetTrackedPrefHistogramCount(
                   user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
                   ALLOW_NONE));
     EXPECT_EQ(0, GetTrackedPrefHistogramCount(
                      user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
                      ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      // Expect all prefs to be reported as Unchanged with no resets.
+      EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM
+                    ? num_tracked_prefs()
+                    : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramUnchanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    ALLOW_ANY));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestUnchangedDefault, UnchangedDefault);
 
 // Augments PrefHashBrowserTestUnchangedDefault to confirm that nothing is reset
 // when nothing is tampered with, even if Chrome itself wrote custom prefs in
 // its last run.
 class PrefHashBrowserTestUnchangedCustom
     : public PrefHashBrowserTestUnchangedDefault {
@@ skipping 70 matching lines @@
               GetTrackedPrefHistogramCount(
                   user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
                   ALLOW_NONE));
     EXPECT_EQ(0, GetTrackedPrefHistogramCount(
                      user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
                      ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      // Expect homepage to have been cleared.
+      EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramCleared,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 2));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestClearedAtomic, ClearedAtomic);
 
 // Verifies that clearing the MACs results in untrusted Initialized pings for
 // non-null protected prefs.
 class PrefHashBrowserTestUntrustedInitialized : public PrefHashBrowserTestBase {
  public:
   void SetupPreferences() override {
@@ skipping 102 matching lines @@
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramChanged, ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramCleared, ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      // The MACs have been cleared but the preferences have not been tampered.
+      // The registry should report all prefs as unchanged.
+      EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM
+                    ? num_tracked_prefs()
+                    : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramUnchanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    ALLOW_ANY));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestUntrustedInitialized,
                        UntrustedInitialized);
 
 // Verifies that changing an atomic pref results in it being reported (and reset
 // if the protection level allows it).
 class PrefHashBrowserTestChangedAtomic : public PrefHashBrowserTestBase {
  public:
@@ skipping 70 matching lines @@
               GetTrackedPrefHistogramCount(
                   user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
                   ALLOW_NONE));
     EXPECT_EQ(0, GetTrackedPrefHistogramCount(
                      user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
                      ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      // Expect a single Changed event for tracked pref #4 (startup URLs).
+      EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramChanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 4));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestChangedAtomic, ChangedAtomic);
 
 // Verifies that changing or adding an entry in a split pref results in both
 // items being reported (and remove if the protection level allows it).
 class PrefHashBrowserTestChangedSplitPref : public PrefHashBrowserTestBase {
  public:
   void SetupPreferences() override {
@@ skipping 77 matching lines @@
               GetTrackedPrefHistogramCount(
                   user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
                   ALLOW_NONE));
     EXPECT_EQ(0, GetTrackedPrefHistogramCount(
                      user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
                      ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      // Expect a single split pref changed report with a count of 2 for tracked
+      // pref #5 (extensions).
+      EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramChanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 5));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestChangedSplitPref, ChangedSplitPref);
 
 // Verifies that adding a value to unprotected preferences for a key which is
 // still using the default (i.e. has no value stored in protected preferences)
 // doesn't allow that value to slip in with no valid MAC (regression test for
 // http://crbug.com/414554)
 class PrefHashBrowserTestUntrustedAdditionToPrefs
@@ skipping 53 matching lines @@
               GetTrackedPrefHistogramCount(
                   user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
                   ALLOW_NONE));
     EXPECT_EQ(0, GetTrackedPrefHistogramCount(
                      user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
                      ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      EXPECT_EQ((protection_level_ > PROTECTION_DISABLED_ON_PLATFORM &&
+                 protection_level_ < PROTECTION_ENABLED_BASIC)
+                    ? changed_expected
+                    : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramChanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 3));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestUntrustedAdditionToPrefs,
                        UntrustedAdditionToPrefs);
 
 // Verifies that adding a value to unprotected preferences while wiping a
 // user-selected value from protected preferences doesn't allow that value to
 // slip in with no valid MAC (regression test for http://crbug.com/414554).
 class PrefHashBrowserTestUntrustedAdditionToPrefsAfterWipe
@@ skipping 54 matching lines @@
               GetTrackedPrefHistogramCount(
                   user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
                   ALLOW_NONE));
     EXPECT_EQ(0, GetTrackedPrefHistogramCount(
                      user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
                      ALLOW_NONE));
     EXPECT_EQ(
         0, GetTrackedPrefHistogramCount(
                user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
                ALLOW_NONE));
+
+    if (SupportsRegistryValidation()) {
+      EXPECT_EQ(changed_expected,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramChanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 2));
+      EXPECT_EQ(cleared_expected,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramCleared,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 2));
+    }
   }
 };
 
 PREF_HASH_BROWSER_TEST(PrefHashBrowserTestUntrustedAdditionToPrefsAfterWipe,
                        UntrustedAdditionToPrefsAfterWipe);
+class PrefHashBrowserTestRegistryValidationFailure
+    : public PrefHashBrowserTestBase {
+ public:
+  void SetupPreferences() override {
+    profile()->GetPrefs()->SetString(prefs::kHomePage, "http://example.com");
+  }
+
+  void AttackPreferencesOnDisk(
+      base::DictionaryValue* unprotected_preferences,
+      base::DictionaryValue* protected_preferences) override {
+    // See TearDown
+  }
+
+  void TearDown() override {
+// Modifying prefs in the registry while in AttackPreferencesOnDisk does not
+// seem to work (despite returning ERROR_SUCCESS). Do it earlier, during the
+// PRE test TearDown instead.

[gab, 2016/09/16 19:47:32]

I think it should, this probably has to do with the custom override/cleanup used
in this patch set, try again after setting up registry redirection as suggested
above.

[proberge, 2016/09/20 21:35:44]

Hmm. I tried moving it back to AttackPreferencesOnDisk and it seems to work.
Maybe I had a transient issue that has since then been fixed.

+#if defined(OS_WIN)
+    base::string16 registry_key =
+        GetRegistryPathForTestProfile() + L"\\PreferenceMACs\\Default";
+    base::win::RegKey key;
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, registry_key.c_str(),
+                                      KEY_SET_VALUE | KEY_WOW64_32KEY));
+    // An incorrect hash should still have the correct size.
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.WriteValue(L"homepage", base::string16(64, 'A').c_str()));
+#endif
+    PrefHashBrowserTestBase::TearDown();
+  }
+
+  void VerifyReactionToPrefAttack() override {
+    EXPECT_EQ(
+        protection_level_ > PROTECTION_DISABLED_ON_PLATFORM
+            ? num_tracked_prefs()
+            : 0,
+        GetTrackedPrefHistogramCount(
+            user_prefs::tracked::kTrackedPrefHistogramUnchanged, ALLOW_ANY));
+
+    if (SupportsRegistryValidation()) {
+      EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
+                GetTrackedPrefHistogramCount(
+                    user_prefs::tracked::kTrackedPrefHistogramChanged,
+                    user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
+                    BEGIN_ALLOW_SINGLE_BUCKET + 2));
+    }

[gab, 2016/09/16 19:47:32]

Note: for this test I'm fine not using the aforementioned helper as it actually
does verify that they don't have the same result.

[proberge, 2016/09/20 21:35:44]

Acknowledged.

+  }

[gab, 2016/09/16 19:47:32]

I think we're missing a test for the very use case this feature is intended to
address : pref + matching MAC injected in prefs but registry unmodified (and
thus catching the change).

[proberge, 2016/09/20 21:35:44]

PrefHashBrowserTestRegistryValidationFailure is meant to test the case you're
talking about. Implementing the pref + matching mac injection was daunting. In
theory these two cases have identical expected behavior:
1. pref + matching mac injected, registry mac unmodified
2. pref + matching mac unmodified, registry mac tampered

+};
+
+PREF_HASH_BROWSER_TEST(PrefHashBrowserTestRegistryValidationFailure,
+                       RegistryValidationFailure);