Fix a race condition in preference metric reporting.

chrome/browser/prefs/pref_hash_store.h

+// 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.
+
+#ifndef CHROME_BROWSER_PREFS_PREF_HASH_STORE_H_
+#define CHROME_BROWSER_PREFS_PREF_HASH_STORE_H_
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+
+class PrefHashTracker;
+
+namespace base {
+class Value;
+}  // namespace base
+
+// Stores hashes of and verifies preference values. To use, first call
+// |InitializeTrackedValue| with each preference that should be tracked. Then
+// call |OnPrefValueChanged| to update the hash store when preference values
+// change.
+class PrefHashStore {
+ public:
+  virtual ~PrefHashStore() {}

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

I forget the C++ details, can you justify why it's better to explicitly specify
a public virtual destructor here rather than keeping it as a pure-virtual
interface?

Is this required for owners of a PrefHashStore to be able to delete it via
virtual dispatch to the impl's destructor?

[erikwright (departed), 2013/12/06 19:00:47]

All classes in the class hierarchy must have destructors. A subclass's
destructor will always implicitly invoke its parent class destructor. So if this
were pure-virtual (" = 0;") with no definition anywhere you would not be able to
link due to a missing symbol PrefHashStore::~PrefHashStore referenced by
PrefHashStoreImpl::~PrefHashStoreImpl

+
+  enum ValueState {
+    // The preference value corresponds to its stored hash.
+    UNCHANGED,
+    // The preference has been cleared since the last hash.
+    CLEARED,
+    // The preference value corresponds to its stored hash, which was calculated
+    // using a legacy hash algorithm.
+    MIGRATED,
+    // The preference value has been changed since the last hash.
+    CHANGED,
+    // No stored hash exists for the preference value.
+    UNKNOWN_VALUE

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

nit: add trailing comma

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

Done.

+  };
+
+  // Checks |initial_value| against the existing stored value hash.
+  virtual ValueState CheckValue(
+      const std::string& path, const base::Value* initial_value) = 0;
+
+  // Stores the current value of the preference at |path|.
+  virtual void StoreValue(const std::string& path,
+                          const base::Value* value) = 0;
+};
+
+#endif  // CHROME_BROWSER_PREFS_PREF_HASH_STORE_H_