Fix a race condition in preference metric reporting.

Introduces:
- PrefFilter: An interface to intercept preference values as they are loaded from disk, before any changes are possible.
- PrefHashFilter: An implementation that verifies preference values against hashes in a PrefHashStore.
- PrefHashStore(Impl): An interface and implementation for storing and verifying hashes of preferences.
- PrefHashCalculator: A utility for calculating preference value hashes.

TBR=brettw (base/base.gyp), scottbyer (chrome/service/service_process_prefs.cc)
BUG=321680
NOTRY=True

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242382

Review URL: https://codereview.chromium.org/90563003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@242407 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 83 insertions, 13 deletions

diff --git a/base/prefs/json_pref_store.cc b/base/prefs/json_pref_store.cc
index ad97b84..f417b8b 100644
--- a/base/prefs/json_pref_store.cc
+++ b/base/prefs/json_pref_store.cc
@@ -13,6 +13,7 @@
 #include "base/json/json_string_value_serializer.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/prefs/pref_filter.h"
 #include "base/sequenced_task_runner.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
@@ -151,12 +152,14 @@ scoped_refptr<base::SequencedTaskRunner> JsonPrefStore::GetTaskRunnerForFile(
 }
 
 JsonPrefStore::JsonPrefStore(const base::FilePath& filename,
-                             base::SequencedTaskRunner* sequenced_task_runner)
+                             base::SequencedTaskRunner* sequenced_task_runner,
+                             scoped_ptr<PrefFilter> pref_filter)
     : path_(filename),
       sequenced_task_runner_(sequenced_task_runner),
       prefs_(new base::DictionaryValue()),
       read_only_(false),
       writer_(filename, sequenced_task_runner),
+      pref_filter_(pref_filter.Pass()),
       initialized_(false),
       read_error_(PREF_READ_ERROR_OTHER) {}
 
@@ -264,7 +267,14 @@ void JsonPrefStore::CommitPendingWrite() {
 }
 
 void JsonPrefStore::ReportValueChanged(const std::string& key) {
+  if (pref_filter_) {
+    const base::Value* tmp = NULL;
+    prefs_->Get(key, &tmp);
+    pref_filter_->FilterUpdate(key, tmp);
+  }
+
   FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key));
+
   if (!read_only_)
     writer_.ScheduleWrite(this);
 }
@@ -307,6 +317,9 @@ void JsonPrefStore::OnFileRead(base::Value* value_owned,
       NOTREACHED() << "Unknown error: " << error;
   }
 
+  if (pref_filter_)
+    pref_filter_->FilterOnLoad(prefs_.get());
+
   if (error_delegate_.get() && error != PREF_READ_ERROR_NONE)
     error_delegate_->OnError(error);
 
diff --git a/base/prefs/json_pref_store.h b/base/prefs/json_pref_store.h
index 21fc8f9..ad13feb 100644
--- a/base/prefs/json_pref_store.h
+++ b/base/prefs/json_pref_store.h
@@ -18,6 +18,8 @@
 #include "base/prefs/base_prefs_export.h"
 #include "base/prefs/persistent_pref_store.h"
 
+class PrefFilter;
+
 namespace base {
 class DictionaryValue;
 class FilePath;
@@ -41,7 +43,8 @@ class BASE_PREFS_EXPORT JsonPrefStore
   // |sequenced_task_runner| is must be a shutdown-blocking task runner, ideally
   // created by GetTaskRunnerForFile() method above.
   JsonPrefStore(const base::FilePath& pref_filename,
-                base::SequencedTaskRunner* sequenced_task_runner);
+                base::SequencedTaskRunner* sequenced_task_runner,
+                scoped_ptr<PrefFilter> pref_filter);
 
   // PrefStore overrides:
   virtual bool GetValue(const std::string& key,
@@ -87,6 +90,7 @@ class BASE_PREFS_EXPORT JsonPrefStore
   // Helper for safely writing pref data.
   base::ImportantFileWriter writer_;
 
+  scoped_ptr<PrefFilter> pref_filter_;
   ObserverList<PrefStore::Observer, true> observers_;
 
   scoped_ptr<ReadErrorDelegate> error_delegate_;
diff --git a/base/prefs/json_pref_store_unittest.cc b/base/prefs/json_pref_store_unittest.cc
index a26afd7..119a29c 100644
--- a/base/prefs/json_pref_store_unittest.cc
+++ b/base/prefs/json_pref_store_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
+#include "base/prefs/pref_filter.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -60,7 +61,9 @@ TEST_F(JsonPrefStoreTest, NonExistentFile) {
   base::FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
   ASSERT_FALSE(PathExists(bogus_input_file));
   scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
-      bogus_input_file, message_loop_.message_loop_proxy().get());
+      bogus_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
   EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE,
             pref_store->ReadPrefs());
   EXPECT_FALSE(pref_store->ReadOnly());
@@ -72,7 +75,9 @@ TEST_F(JsonPrefStoreTest, InvalidFile) {
   base::FilePath invalid_file = temp_dir_.path().AppendASCII("invalid.json");
   ASSERT_TRUE(base::CopyFile(invalid_file_original, invalid_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(invalid_file, message_loop_.message_loop_proxy().get());
+      new JsonPrefStore(invalid_file,
+                        message_loop_.message_loop_proxy().get(),
+                        scoped_ptr<PrefFilter>());
   EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_JSON_PARSE,
             pref_store->ReadPrefs());
   EXPECT_FALSE(pref_store->ReadOnly());
@@ -157,8 +162,10 @@ TEST_F(JsonPrefStoreTest, Basic) {
   // Test that the persistent value can be loaded.
   base::FilePath input_file = temp_dir_.path().AppendASCII("write.json");
   ASSERT_TRUE(PathExists(input_file));
-  scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(input_file, message_loop_.message_loop_proxy().get());
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
   ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
   ASSERT_FALSE(pref_store->ReadOnly());
 
@@ -183,8 +190,10 @@ TEST_F(JsonPrefStoreTest, BasicAsync) {
   // Test that the persistent value can be loaded.
   base::FilePath input_file = temp_dir_.path().AppendASCII("write.json");
   ASSERT_TRUE(PathExists(input_file));
-  scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(input_file, message_loop_.message_loop_proxy().get());
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
 
   {
     MockPrefStoreObserver mock_observer;
@@ -219,8 +228,10 @@ TEST_F(JsonPrefStoreTest, BasicAsync) {
 TEST_F(JsonPrefStoreTest, PreserveEmptyValues) {
   FilePath pref_file = temp_dir_.path().AppendASCII("empty_values.json");
 
-  scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(pref_file, message_loop_.message_loop_proxy());
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      pref_file,
+      message_loop_.message_loop_proxy(),
+      scoped_ptr<PrefFilter>());
 
   // Set some keys with empty values.
   pref_store->SetValue("list", new base::ListValue);
@@ -231,7 +242,10 @@ TEST_F(JsonPrefStoreTest, PreserveEmptyValues) {
   MessageLoop::current()->RunUntilIdle();
 
   // Reload.
-  pref_store = new JsonPrefStore(pref_file, message_loop_.message_loop_proxy());
+  pref_store = new JsonPrefStore(
+      pref_file,
+      message_loop_.message_loop_proxy(),
+      scoped_ptr<PrefFilter>());
   ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
   ASSERT_FALSE(pref_store->ReadOnly());
 
@@ -248,7 +262,9 @@ TEST_F(JsonPrefStoreTest, AsyncNonExistingFile) {
   base::FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
   ASSERT_FALSE(PathExists(bogus_input_file));
   scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
-      bogus_input_file, message_loop_.message_loop_proxy().get());
+      bogus_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
   MockPrefStoreObserver mock_observer;
   pref_store->AddObserver(&mock_observer);
 
diff --git a/base/prefs/pref_filter.h b/base/prefs/pref_filter.h
new file mode 100644
index 0000000..3d136f7
--- /dev/null
+++ b/base/prefs/pref_filter.h
@@ -0,0 +1,35 @@
+// 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 BASE_PREFS_PREF_FILTER_H_
+#define BASE_PREFS_PREF_FILTER_H_
+
+#include <string>
+
+#include "base/prefs/base_prefs_export.h"
+
+namespace base {
+class DictionaryValue;
+class Value;
+}  // namespace base
+
+// Filters preferences as they are loaded from disk or updated at runtime.
+// Currently supported only by JsonPrefStore.
+class BASE_PREFS_EXPORT PrefFilter {
+ public:
+  virtual ~PrefFilter() {}
+
+  // Receives notification when the pref store data has been loaded but before
+  // Observers are notified.
+  // Changes made by a PrefFilter during FilterOnLoad do not result in
+  // notifications to |PrefStore::Observer|s.
+  virtual void FilterOnLoad(base::DictionaryValue* pref_store_contents) = 0;
+
+  // Receives notification when a pref store value is changed, before Observers
+  // are notified.
+  virtual void FilterUpdate(const std::string& path,
+                            const base::Value* value) = 0;
+};
+
+#endif  // BASE_PREFS_PREF_FILTER_H_
diff --git a/base/prefs/pref_service_factory.cc b/base/prefs/pref_service_factory.cc
index 9c59853..d644cb1 100644
--- a/base/prefs/pref_service_factory.cc
+++ b/base/prefs/pref_service_factory.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/prefs/default_pref_store.h"
 #include "base/prefs/json_pref_store.h"
+#include "base/prefs/pref_filter.h"
 #include "base/prefs/pref_notifier_impl.h"
 #include "base/prefs/pref_service.h"
 
@@ -37,7 +38,8 @@ PrefServiceFactory::~PrefServiceFactory() {}
 void PrefServiceFactory::SetUserPrefsFile(
     const base::FilePath& prefs_file,
     base::SequencedTaskRunner* task_runner) {
-  user_prefs_ = new JsonPrefStore(prefs_file, task_runner);
+  user_prefs_ = new JsonPrefStore(
+      prefs_file, task_runner, scoped_ptr<PrefFilter>());
 }
 
 scoped_ptr<PrefService> PrefServiceFactory::Create(