Send pref change notifications when changing the default value for a pref.

TBR=ben@chromium.org
BUG=none


Review URL: https://chromiumcodereview.appspot.com/12541008

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

5 files changed, 111 insertions, 17 deletions

diff --git a/base/base.gyp b/base/base.gyp
index f75c7c2..8118d87a 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -521,6 +521,7 @@
         'posix/file_descriptor_shuffle_unittest.cc',
         'posix/unix_domain_socket_linux_unittest.cc',
         'pr_time_unittest.cc',
+        'prefs/default_pref_store_unittest.cc',
         'prefs/json_pref_store_unittest.cc',
         'prefs/mock_pref_change_callback.h',
         'prefs/overlay_user_pref_store_unittest.cc',
diff --git a/base/prefs/default_pref_store.cc b/base/prefs/default_pref_store.cc
index 3df9ad9..babb4d5 100644
--- a/base/prefs/default_pref_store.cc
+++ b/base/prefs/default_pref_store.cc
@@ -9,20 +9,37 @@ using base::Value;
 
 DefaultPrefStore::DefaultPrefStore() {}
 
-bool DefaultPrefStore::GetValue(
-    const std::string& key,
-    const base::Value** result) const {
+bool DefaultPrefStore::GetValue(const std::string& key,
+                                const Value** result) const {
   return prefs_.GetValue(key, result);
 }
 
-void DefaultPrefStore::SetDefaultValue(const std::string& key, Value* value) {
+void DefaultPrefStore::AddObserver(PrefStore::Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void DefaultPrefStore::RemoveObserver(PrefStore::Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+size_t DefaultPrefStore::NumberOfObservers() const {
+  return observers_.size();
+}
+
+void DefaultPrefStore::SetDefaultValue(const std::string& key,
+                                       scoped_ptr<Value> value) {
   DCHECK(!GetValue(key, NULL));
-  prefs_.SetValue(key, value);
+  prefs_.SetValue(key, value.release());
 }
 
-void DefaultPrefStore::RemoveDefaultValue(const std::string& key) {
-  DCHECK(GetValue(key, NULL));
-  prefs_.RemoveValue(key);
+void DefaultPrefStore::ReplaceDefaultValue(const std::string& key,
+                                           scoped_ptr<Value> value) {
+  const Value* old_value = NULL;
+  GetValue(key, &old_value);
+  bool notify = !old_value->Equals(value.get());
+  prefs_.SetValue(key, value.release());
+  if (notify)
+    FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key));
 }
 
 DefaultPrefStore::const_iterator DefaultPrefStore::begin() const {
diff --git a/base/prefs/default_pref_store.h b/base/prefs/default_pref_store.h
index 89a0602..d44e866 100644
--- a/base/prefs/default_pref_store.h
+++ b/base/prefs/default_pref_store.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/observer_list.h"
 #include "base/prefs/base_prefs_export.h"
 #include "base/prefs/pref_store.h"
 #include "base/prefs/pref_value_map.h"
@@ -19,24 +20,31 @@ class BASE_PREFS_EXPORT DefaultPrefStore : public PrefStore {
 
   DefaultPrefStore();
 
+  // PrefStore implementation:
   virtual bool GetValue(const std::string& key,
                         const base::Value** result) const OVERRIDE;
+  virtual void AddObserver(PrefStore::Observer* observer) OVERRIDE;
+  virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE;
+  virtual size_t NumberOfObservers() const OVERRIDE;
 
-  // Stores a new |value| for |key|. Assumes ownership of |value|.
-  void SetDefaultValue(const std::string& key, Value* value);
+  // Sets a |value| for |key|. Should only be called if a value has not been
+  // set yet; otherwise call ReplaceDefaultValue().
+  void SetDefaultValue(const std::string& key, scoped_ptr<Value> value);
 
-  // Removes the value for |key|.
-  void RemoveDefaultValue(const std::string& key);
+  // Replaces the the value for |key| with a new value. Should only be called
+  // if a value has alreday been set; otherwise call SetDefaultValue().
+  void ReplaceDefaultValue(const std::string& key, scoped_ptr<Value> value);
 
   const_iterator begin() const;
   const_iterator end() const;
 
- protected:
+ private:
   virtual ~DefaultPrefStore();
 
- private:
   PrefValueMap prefs_;
 
+  ObserverList<PrefStore::Observer, true> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(DefaultPrefStore);
 };
 
diff --git a/base/prefs/default_pref_store_unittest.cc b/base/prefs/default_pref_store_unittest.cc
new file mode 100644
index 0000000..7181989
--- /dev/null
+++ b/base/prefs/default_pref_store_unittest.cc
@@ -0,0 +1,69 @@
+// Copyright (c) 2012 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 "base/prefs/default_pref_store.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using base::StringValue;
+using base::Value;
+
+namespace {
+
+class MockPrefStoreObserver : public PrefStore::Observer {
+ public:
+  explicit MockPrefStoreObserver(DefaultPrefStore* pref_store);
+  virtual ~MockPrefStoreObserver();
+
+  int change_count() {
+    return change_count_;
+  }
+
+  // PrefStore::Observer implementation:
+  virtual void OnPrefValueChanged(const std::string& key) OVERRIDE;
+  virtual void OnInitializationCompleted(bool succeeded) OVERRIDE {}
+
+ private:
+  DefaultPrefStore* pref_store_;
+
+  int change_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockPrefStoreObserver);
+};
+
+MockPrefStoreObserver::MockPrefStoreObserver(DefaultPrefStore* pref_store)
+    : pref_store_(pref_store), change_count_(0) {
+  pref_store_->AddObserver(this);
+}
+
+MockPrefStoreObserver::~MockPrefStoreObserver() {
+  pref_store_->RemoveObserver(this);
+}
+
+void MockPrefStoreObserver::OnPrefValueChanged(const std::string& key) {
+  change_count_++;
+}
+
+}  // namespace
+
+TEST(DefaultPrefStoreTest, NotifyPrefValueChanged) {
+  scoped_refptr<DefaultPrefStore> pref_store(new DefaultPrefStore);
+  MockPrefStoreObserver observer(pref_store.get());
+  std::string kPrefKey("pref_key");
+
+  // Setting a default value shouldn't send a change notification.
+  pref_store->SetDefaultValue(kPrefKey,
+                              scoped_ptr<Value>(new StringValue("foo")));
+  EXPECT_EQ(0, observer.change_count());
+
+  // Replacing the default value should send a change notification...
+  pref_store->ReplaceDefaultValue(kPrefKey,
+                                  scoped_ptr<Value>(new StringValue("bar")));
+  EXPECT_EQ(1, observer.change_count());
+
+  // But only if the value actually changed.
+  pref_store->ReplaceDefaultValue(kPrefKey,
+                                  scoped_ptr<Value>(new StringValue("bar")));
+  EXPECT_EQ(1, observer.change_count());
+}
+
diff --git a/base/prefs/pref_registry.cc b/base/prefs/pref_registry.cc
index 9ca1102..31d788d 100644
--- a/base/prefs/pref_registry.cc
+++ b/base/prefs/pref_registry.cc
@@ -39,8 +39,7 @@ void PrefRegistry::SetDefaultPrefValue(const char* pref_name,
         << "Wrong type for new default: " << pref_name;
   }
 
-  defaults_->RemoveDefaultValue(pref_name);
-  defaults_->SetDefaultValue(pref_name, value);
+  defaults_->ReplaceDefaultValue(pref_name, make_scoped_ptr(value));
 }
 
 void PrefRegistry::SetRegistrationCallback(
@@ -57,7 +56,7 @@ void PrefRegistry::RegisterPreference(const char* path,
   DCHECK(!defaults_->GetValue(path, NULL)) <<
       "Trying to register a previously registered pref: " << path;
 
-  defaults_->SetDefaultValue(path, default_value);
+  defaults_->SetDefaultValue(path, make_scoped_ptr(default_value));
 
   if (!registration_callback_.is_null())
     registration_callback_.Run(path, default_value);