Fix a race condition in preference metric reporting.

chrome/browser/prefs/pref_hash_store_impl_unittest.cc

Index: chrome/browser/prefs/pref_hash_store_impl_unittest.cc
diff --git a/chrome/browser/prefs/pref_hash_store_impl_unittest.cc b/chrome/browser/prefs/pref_hash_store_impl_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a49b12f25c092f68b353ba1fab99a79e1b55b993
--- /dev/null
+++ b/chrome/browser/prefs/pref_hash_store_impl_unittest.cc
@@ -0,0 +1,64 @@
+// Copyright 2013 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 "chrome/browser/prefs/pref_hash_store_impl.h"
+
+#include <string>
+
+#include "base/prefs/pref_service.h"
+#include "base/prefs/scoped_user_pref_update.h"
+#include "base/prefs/testing_pref_service.h"
+#include "base/values.h"
+#include "chrome/browser/prefs/pref_hash_calculator.h"
+#include "chrome/browser/prefs/pref_hash_store.h"
+#include "chrome/common/pref_names.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(PrefHashStoreImplTest, TestCase) {
+  base::StringValue string_1("string1");
+  base::StringValue string_2("string2");
+
+  TestingPrefServiceSimple local_state;
+  PrefHashStoreImpl::RegisterPrefs(local_state.registry());
+
+  // 32 NULL bytes is the seed that was used to generate the legacy hash.
+  PrefHashStoreImpl pref_hash_store(
+      PrefHashCalculator(std::string(32,0), "device_id"),
+      &local_state,
+      "store_id");
+
+  ASSERT_EQ(PrefHashStore::UNKNOWN_VALUE,
+            pref_hash_store.CheckValue("path1", &string_1));
+  ASSERT_EQ(PrefHashStore::UNKNOWN_VALUE,
+            pref_hash_store.CheckValue("path1", &string_1));

[gab, 2013/12/06 17:23:38]

Same check twice above, typo?

[erikwright (departed), 2013/12/09 17:59:40]

Previously it was a mutating operation. But with our API changes it isn't
anymore.

I've made CheckValue 'const' to make this explicit.

+  pref_hash_store.StoreValue("path1", &string_1);
+  ASSERT_EQ(PrefHashStore::UNCHANGED,
+            pref_hash_store.CheckValue("path1", &string_1));
+  ASSERT_EQ(PrefHashStore::CLEARED, pref_hash_store.CheckValue("path1", NULL));
+  pref_hash_store.StoreValue("path1", NULL);
+  ASSERT_EQ(PrefHashStore::UNCHANGED,
+            pref_hash_store.CheckValue("path1", NULL));
+  ASSERT_EQ(PrefHashStore::CHANGED,
+            pref_hash_store.CheckValue("path1", &string_2));
+
+  DictionaryValue dict;
+  dict.Set("a", new StringValue("foo"));
+  dict.Set("d", new StringValue("bad"));
+  dict.Set("b", new StringValue("bar"));
+  dict.Set("c", new StringValue("baz"));
+
+  // Manually shove in a legacy hash.
+  DictionaryPrefUpdate update(&local_state, prefs::kProfilePreferenceHashes);
+  DictionaryValue* child_dictionary = NULL;
+  ASSERT_TRUE(update->GetDictionary("store_id", &child_dictionary));
+  child_dictionary->SetString(
+      "path1",
+      "C503FB7C65EEFD5C07185F616A0AA67923C069909933F362022B1F187E73E9A2");
+
+  ASSERT_EQ(PrefHashStore::MIGRATED,
+            pref_hash_store.CheckValue("path1", &dict));
+  pref_hash_store.StoreValue("path1", &dict);
+  ASSERT_EQ(PrefHashStore::UNCHANGED,
+            pref_hash_store.CheckValue("path1", &dict));
+}