Introduce ChromeZoomLevelPref, make zoom level prefs independent of profile prefs.

chrome/browser/profiles/profile_impl.cc

Index: chrome/browser/profiles/profile_impl.cc
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 9cfe5359a26af2cbf1c6b47bb0931650131c5988..946d391c09ca333b495832b19d2b165f01d05578 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -73,6 +73,7 @@
 #include "chrome/browser/ssl/chrome_ssl_host_state_delegate.h"
 #include "chrome/browser/ssl/chrome_ssl_host_state_delegate_factory.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
+#include "chrome/browser/ui/zoom/zoom_level_prefs_store_impl.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
@@ -547,11 +548,6 @@ void ProfileImpl::DoFinalInit() {
       prefs::kSupervisedUserId,
       base::Bind(&ProfileImpl::UpdateProfileSupervisedUserIdCache,
                  base::Unretained(this)));
-  pref_change_registrar_.Add(
-      prefs::kDefaultZoomLevel,
-      base::Bind(&ProfileImpl::OnDefaultZoomLevelChanged,
-                 base::Unretained(this)));
-
   // Changes in the profile avatar.
   pref_change_registrar_.Add(
       prefs::kProfileAvatarIndex,
@@ -770,14 +766,46 @@ void ProfileImpl::DoFinalInit() {
 
 void ProfileImpl::InitHostZoomMap() {
   HostZoomMap* host_zoom_map = HostZoomMap::GetDefaultForBrowserContext(this);
-  host_zoom_map->SetDefaultZoomLevel(
-      prefs_->GetDouble(prefs::kDefaultZoomLevel));
+  // As part of the migration from per-profile to per-partition HostZoomMaps,
+  // we need to detect if an existing per-profile set of preferences exist, and
+  // if so convert them to be per-partition. We defer running this function
+  // until zoom_level_prefs_store_ has initialized, and then migrate any per-
+  // profile zoom level prefs via zoom_level_prefs_store_.
+  // Code that updates zoom prefs in the profile prefs store has been removed,
+  // so once we clear these values here, they should never get set again.
+  // TODO(wjmaclean): Remove this migration machinery one (?) milestone after
+  // it goes stable. This means removing this entire function.

[Peter Kasting, 2014/09/11 00:45:47]

You're going to want to wait quite a bit longer than one milestone.  In general
we tend to leave migration code in for many milestones in case people don't use
Chrome for a while and then come back to it.

[wjmaclean, 2014/09/11 21:00:51]

Sure, I was wondering about that. Is there any accepted guideline?

Done.

+  if (!zoom_level_prefs_store_) {
+    chrome::ZoomLevelPrefsStoreImpl* zoom_level_prefs_store_impl =
+        new chrome::ZoomLevelPrefsStoreImpl;
+    zoom_level_prefs_store_impl->SetInitCallback(
+        base::Bind(&ProfileImpl::InitHostZoomMap, base::Unretained(this)));
+    zoom_level_prefs_store_.reset(zoom_level_prefs_store_impl);
+    zoom_level_prefs_store_->InitPrefsAndCopyToHostZoomMap(GetPath(),
+                                                           host_zoom_map);
+    return;
+  }
+
+  // Only migrate the default zoom level if it is not equal to the registered
+  // default for the preference.
+  double default_default_zoom_level = 0.0;
+  prefs_->GetDefaultPrefValue(prefs::kProfileDefaultZoomLevel)
+      ->GetAsDouble(&default_default_zoom_level);
+  double per_profile_default_zoom_level =
+      prefs_->GetDouble(prefs::kProfileDefaultZoomLevel);
+  if (per_profile_default_zoom_level != default_default_zoom_level) {
+    // We cannot un-register this pref name, but we can reset it to a
+    // default sentinel value.
+    prefs_->SetDouble(prefs::kProfileDefaultZoomLevel,
+                      default_default_zoom_level);
+    zoom_level_prefs_store_->GetPrefs()->SetDouble(
+        prefs::kDefaultZoomLevel, per_profile_default_zoom_level);
+  }
 
   const base::DictionaryValue* host_zoom_dictionary =
-      prefs_->GetDictionary(prefs::kPerHostZoomLevels);
+      prefs_->GetDictionary(prefs::kProfilePerHostZoomLevels);
   // Careful: The returned value could be NULL if the pref has never been set.
   if (host_zoom_dictionary != NULL) {
-    std::vector<std::string> keys_to_remove;
     for (base::DictionaryValue::Iterator i(*host_zoom_dictionary); !i.IsAtEnd();
          i.Advance()) {
       const std::string& host(i.key());
@@ -796,23 +824,21 @@ void ProfileImpl::InitHostZoomMap() {
       if (host.empty() ||
           content::ZoomValuesEqual(zoom_level,
                                    host_zoom_map->GetDefaultZoomLevel())) {
-        keys_to_remove.push_back(host);
         continue;
       }
 
+      // We push the profile per-host levels in through the HostZoomMap
+      // directly, which will update the zoom_level_prefs_store_ indirectly
+      // through the subsequent ZoomLevelChanged events.
       host_zoom_map->SetZoomLevelForHost(host, zoom_level);
     }
 
-    DictionaryPrefUpdate update(prefs_.get(), prefs::kPerHostZoomLevels);
-    base::DictionaryValue* host_zoom_dictionary = update.Get();
-    for (std::vector<std::string>::const_iterator it = keys_to_remove.begin();
-         it != keys_to_remove.end(); ++it) {
-      host_zoom_dictionary->RemoveWithoutPathExpansion(*it, NULL);
-    }
+    // We're done imigrating the profile per-host zoom level values, so we clear

[Peter Kasting, 2014/09/11 00:45:47]

Nit: Spelling.  (I'd just say "migrating", that's clearer and easier to spell.)

[wjmaclean, 2014/09/11 21:00:51]

Ha ha ... the 'i' was from vi ... 'migrating was intended all along ... no need
to get immigration involved in all this ;-)

+    // them all.
+    DictionaryPrefUpdate update(prefs_.get(), prefs::kProfilePerHostZoomLevels);
+    base::DictionaryValue* host_zoom_dictionary_update = update.Get();
+    host_zoom_dictionary_update->Clear();
   }
-
-  zoom_subscription_ = host_zoom_map->AddZoomLevelChangedCallback(
-      base::Bind(&ProfileImpl::OnZoomLevelChanged, base::Unretained(this)));
 }
 
 base::FilePath ProfileImpl::last_selected_directory() {
@@ -1033,6 +1059,10 @@ PrefService* ProfileImpl::GetPrefs() {
   return prefs_.get();
 }
 
+PrefService* ProfileImpl::GetZoomLevelPrefs() {
+  return zoom_level_prefs_store_ ? zoom_level_prefs_store_->GetPrefs() : NULL;
+}
+
 PrefService* ProfileImpl::GetOffTheRecordPrefs() {
   DCHECK(prefs_);
   if (!otr_prefs_) {
@@ -1181,26 +1211,6 @@ history::TopSites* ProfileImpl::GetTopSitesWithoutCreating() {
   return top_sites_.get();
 }
 
-void ProfileImpl::OnDefaultZoomLevelChanged() {
-  HostZoomMap::GetDefaultForBrowserContext(this)->SetDefaultZoomLevel(
-      pref_change_registrar_.prefs()->GetDouble(prefs::kDefaultZoomLevel));
-}
-
-void ProfileImpl::OnZoomLevelChanged(
-    const HostZoomMap::ZoomLevelChange& change) {
-
-  if (change.mode != HostZoomMap::ZOOM_CHANGED_FOR_HOST)
-    return;
-  HostZoomMap* host_zoom_map = HostZoomMap::GetDefaultForBrowserContext(this);
-  double level = change.zoom_level;
-  DictionaryPrefUpdate update(prefs_.get(), prefs::kPerHostZoomLevels);
-  base::DictionaryValue* host_zoom_dictionary = update.Get();
-  if (content::ZoomValuesEqual(level, host_zoom_map->GetDefaultZoomLevel()))
-    host_zoom_dictionary->RemoveWithoutPathExpansion(change.host, NULL);
-  else
-    host_zoom_dictionary->SetDoubleWithoutPathExpansion(change.host, level);
-}
-
 #if defined(ENABLE_SESSION_SERVICE)
 void ProfileImpl::StopCreateSessionServiceTimer() {
   create_session_service_timer_.Stop();