Make HostZoomMap play nicely with preference sync.

Review URL: http://codereview.chromium.org/1560023

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

3 files changed, 211 insertions, 10 deletions

diff --git a/chrome/browser/host_zoom_map.cc b/chrome/browser/host_zoom_map.cc
index f912ac4..f83d6f6 100644
--- a/chrome/browser/host_zoom_map.cc
+++ b/chrome/browser/host_zoom_map.cc
@@ -8,9 +8,27 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/scoped_pref_update.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_source.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
 
-HostZoomMap::HostZoomMap(Profile* profile) : profile_(profile) {
+HostZoomMap::HostZoomMap(Profile* profile)
+    : profile_(profile),
+      updating_preferences_(false) {
+  Load();
+  registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
+                 Source<Profile>(profile));
+  profile_->GetPrefs()->AddPrefObserver(prefs::kPerHostZoomLevels, this);
+}
+
+void HostZoomMap::Load() {
+  if (!profile_)
+    return;
+
+  AutoLock auto_lock(lock_);
+  host_zoom_levels_.clear();
   const DictionaryValue* host_zoom_dictionary =
       profile_->GetPrefs()->GetDictionary(prefs::kPerHostZoomLevels);
   // Careful: The returned value could be NULL if the pref has never been set.
@@ -40,6 +58,9 @@ int HostZoomMap::GetZoomLevel(const std::string& host) const {
 
 void HostZoomMap::SetZoomLevel(const std::string& host, int level) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  if (!profile_)
+    return;
+
   if (host.empty())
     return;
 
@@ -51,27 +72,76 @@ void HostZoomMap::SetZoomLevel(const std::string& host, int level) {
       host_zoom_levels_[host] = level;
   }
 
-  DictionaryValue* host_zoom_dictionary =
-      profile_->GetPrefs()->GetMutableDictionary(prefs::kPerHostZoomLevels);
-  std::wstring wide_host(UTF8ToWide(host));
-  if (level == 0) {
-    host_zoom_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
-  } else {
-    host_zoom_dictionary->SetWithoutPathExpansion(wide_host,
-        Value::CreateIntegerValue(level));
+  updating_preferences_ = true;
+  {
+    ScopedPrefUpdate update(profile_->GetPrefs(), prefs::kPerHostZoomLevels);
+    DictionaryValue* host_zoom_dictionary =
+        profile_->GetPrefs()->GetMutableDictionary(prefs::kPerHostZoomLevels);
+    std::wstring wide_host(UTF8ToWide(host));
+    if (level == 0) {
+      host_zoom_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
+    } else {
+      host_zoom_dictionary->SetWithoutPathExpansion(
+          wide_host,
+          Value::CreateIntegerValue(level));
+    }
   }
+  updating_preferences_ = false;
 }
 
 void HostZoomMap::ResetToDefaults() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  if (!profile_)
+    return;
 
   {
     AutoLock auto_lock(lock_);
     host_zoom_levels_.clear();
   }
 
+  updating_preferences_ = true;
   profile_->GetPrefs()->ClearPref(prefs::kPerHostZoomLevels);
+  updating_preferences_ = false;
+}
+
+void HostZoomMap::Shutdown() {
+  if (!profile_)
+    return;
+
+  registrar_.Remove(this,
+                    NotificationType::PROFILE_DESTROYED,
+                    Source<Profile>(profile_));
+  profile_->GetPrefs()->RemovePrefObserver(prefs::kPerHostZoomLevels, this);
+  profile_ = NULL;
+}
+
+void HostZoomMap::Observe(
+    NotificationType type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+
+  // If the profile is going away, we need to stop using it.
+  if (type == NotificationType::PROFILE_DESTROYED) {
+    Shutdown();
+    return;
+  }
+
+  if (type == NotificationType::PREF_CHANGED) {
+    // If we are updating our own preference, don't reload.
+    if (updating_preferences_)
+      return;
+
+    std::wstring* name = Details<std::wstring>(details).ptr();
+    if (prefs::kPerHostZoomLevels == *name) {
+      Load();
+      return;
+    }
+  }
+
+  NOTREACHED() << "Unexpected preference observed.";
 }
 
 HostZoomMap::~HostZoomMap() {
+  Shutdown();
 }
diff --git a/chrome/browser/host_zoom_map.h b/chrome/browser/host_zoom_map.h
index f64ca6b..87dee2c 100644
--- a/chrome/browser/host_zoom_map.h
+++ b/chrome/browser/host_zoom_map.h
@@ -14,11 +14,14 @@
 #include "base/basictypes.h"
 #include "base/lock.h"
 #include "base/ref_counted.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_registrar.h"
 
 class PrefService;
 class Profile;
 
-class HostZoomMap : public base::RefCountedThreadSafe<HostZoomMap> {
+class HostZoomMap : public NotificationObserver,
+                    public base::RefCountedThreadSafe<HostZoomMap> {
  public:
   explicit HostZoomMap(Profile* profile);
 
@@ -43,6 +46,11 @@ class HostZoomMap : public base::RefCountedThreadSafe<HostZoomMap> {
   // This should only be called on the UI thread.
   void ResetToDefaults();
 
+  // NotificationObserver implementation.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
  private:
   friend class base::RefCountedThreadSafe<HostZoomMap>;
 
@@ -50,6 +58,13 @@ class HostZoomMap : public base::RefCountedThreadSafe<HostZoomMap> {
 
   ~HostZoomMap();
 
+  // Reads the zoom levels from the preferences service.
+  void Load();
+
+  // Removes dependencies on the profile so we can live longer than
+  // the profile without crashing.
+  void Shutdown();
+
   // The profile we're associated with.
   Profile* profile_;
 
@@ -59,6 +74,12 @@ class HostZoomMap : public base::RefCountedThreadSafe<HostZoomMap> {
   // Used around accesses to |host_zoom_levels_| to guarantee thread safety.
   mutable Lock lock_;
 
+  // Whether we are currently updating preferences, this is used to ignore
+  // notifications from the preference service that we triggered ourself.
+  bool updating_preferences_;
+
+  NotificationRegistrar registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(HostZoomMap);
 };
 
diff --git a/chrome/browser/host_zoom_map_unittest.cc b/chrome/browser/host_zoom_map_unittest.cc
new file mode 100644
index 0000000..fb7b52d
--- /dev/null
+++ b/chrome/browser/host_zoom_map_unittest.cc
@@ -0,0 +1,110 @@
+// Copyright (c) 2010 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/message_loop.h"
+#include "base/ref_counted.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/host_zoom_map.h"
+#include "chrome/browser/pref_service.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_observer_mock.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/testing_profile.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::_;
+using testing::Pointee;
+using testing::Property;
+
+class HostZoomMapTest : public testing::Test {
+ public:
+  static const int kZoomLevel;
+  HostZoomMapTest()
+      : ui_thread_(ChromeThread::UI, &message_loop_),
+        prefs_(profile_.GetPrefs()),
+        per_host_zoom_levels_pref_(prefs::kPerHostZoomLevels),
+        host_name_("http://example/com/") {}
+
+ protected:
+  void SetPrefObserverExpectation() {
+    EXPECT_CALL(
+        pref_observer_,
+        Observe(NotificationType(NotificationType::PREF_CHANGED),
+                _,
+                Property(&Details<std::wstring>::ptr,
+                         Pointee(per_host_zoom_levels_pref_))));
+  }
+
+  MessageLoopForUI message_loop_;
+  ChromeThread ui_thread_;
+  TestingProfile profile_;
+  PrefService* prefs_;
+  std::wstring per_host_zoom_levels_pref_;  // For the observe matcher.
+  std::string host_name_;
+  NotificationObserverMock pref_observer_;
+};
+const int HostZoomMapTest::kZoomLevel = 42;
+
+TEST_F(HostZoomMapTest, LoadNoPrefs) {
+  scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
+  EXPECT_EQ(0, map->GetZoomLevel(host_name_));
+}
+
+TEST_F(HostZoomMapTest, Load) {
+  DictionaryValue* dict =
+      prefs_->GetMutableDictionary(prefs::kPerHostZoomLevels);
+  dict->SetWithoutPathExpansion(UTF8ToWide(host_name_),
+                                Value::CreateIntegerValue(kZoomLevel));
+  scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
+  EXPECT_EQ(kZoomLevel, map->GetZoomLevel(host_name_));
+}
+
+TEST_F(HostZoomMapTest, SetZoomLevel) {
+  scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
+  prefs_->AddPrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
+  SetPrefObserverExpectation();
+  map->SetZoomLevel(host_name_, kZoomLevel);
+  EXPECT_EQ(kZoomLevel, map->GetZoomLevel(host_name_));
+  const DictionaryValue* dict =
+      prefs_->GetDictionary(prefs::kPerHostZoomLevels);
+  int zoom_level = 0;
+  EXPECT_TRUE(dict->GetIntegerWithoutPathExpansion(UTF8ToWide(host_name_),
+                                                   &zoom_level));
+  EXPECT_EQ(kZoomLevel, zoom_level);
+
+  SetPrefObserverExpectation();
+  map->SetZoomLevel(host_name_, 0);
+  EXPECT_EQ(0, map->GetZoomLevel(host_name_));
+  EXPECT_FALSE(dict->HasKey(UTF8ToWide(host_name_)));
+  prefs_->RemovePrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
+}
+
+TEST_F(HostZoomMapTest, ResetToDefaults) {
+  scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
+  map->SetZoomLevel(host_name_, kZoomLevel);
+
+  prefs_->AddPrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
+  SetPrefObserverExpectation();
+  map->ResetToDefaults();
+  EXPECT_EQ(0, map->GetZoomLevel(host_name_));
+  EXPECT_EQ(NULL, prefs_->GetDictionary(prefs::kPerHostZoomLevels));
+  prefs_->RemovePrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
+}
+
+TEST_F(HostZoomMapTest, ReloadOnPrefChange) {
+  scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
+  map->SetZoomLevel(host_name_, kZoomLevel);
+
+  DictionaryValue dict;
+  dict.SetWithoutPathExpansion(UTF8ToWide(host_name_),
+                               Value::CreateIntegerValue(0));
+  prefs_->Set(prefs::kPerHostZoomLevels, dict);
+  EXPECT_EQ(0, map->GetZoomLevel(host_name_));
+}