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

chrome/browser/profiles/profile_impl.cc

 // 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 "chrome/browser/profiles/profile_impl.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 55 matching lines @@
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/services/gcm/push_messaging_service_impl.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/signin/signin_ui_util.h"
 #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/chrome_zoom_level_prefs.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_settings.h"
@@ skipping 428 matching lines @@
   PrefService* prefs = GetPrefs();
   pref_change_registrar_.Init(prefs);
   pref_change_registrar_.Add(
       prefs::kGoogleServicesUsername,
       base::Bind(&ProfileImpl::UpdateProfileUserNameCache,
                  base::Unretained(this)));
   pref_change_registrar_.Add(
       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,
       base::Bind(&ProfileImpl::UpdateProfileAvatarCache,
                  base::Unretained(this)));
   pref_change_registrar_.Add(
       prefs::kProfileUsingDefaultAvatar,
       base::Bind(&ProfileImpl::UpdateProfileAvatarCache,
                  base::Unretained(this)));
@@ skipping 206 matching lines @@
   model->AddObserver(new BookmarkModelLoadedObserver(this));
 #endif
 
   gcm::PushMessagingServiceImpl::InitializeForProfile(this);
 
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_IOS)
   signin_ui_util::InitializePrefsForProfile(this);
 #endif
 }
 
 void ProfileImpl::InitHostZoomMap() {

[battre, 2014/10/08 11:27:10]

What do you think of moving the migration code to browser_prefs.cc ->
MigrateUserPrefs? This is where we typically trigger it.

I would then move the cleanup code out of Profile and into to the new
ChromeZoomLevelPrefs.

[wjmaclean, 2014/10/10 14:40:29]

Done.

I didn't realize there was similar code elsewhere I could group this with ...
thanks for mentioning it.

   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 migrate any per-profile zoom
+  // level prefs via zoom_level_prefs_.
+  // 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 several milestones after
+  // it goes stable. This means removing this entire function.
+  DCHECK(!zoom_level_prefs_);
+  zoom_level_prefs_.reset(
+      new chrome::ChromeZoomLevelPrefs(prefs_.get(), GetPath()));
+  zoom_level_prefs_->InitPrefsAndCopyToHostZoomMap(GetPath(), host_zoom_map);
+
+  bool migrated = false;
+  // Only migrate the default zoom level if it is not equal to the registered
+  // default for the preference.
+  const base::Value* per_profile_default_zoom_level_value =
+      prefs_->GetUserPrefValue(prefs::kDefaultZoomLevelDeprecated);
+  if (per_profile_default_zoom_level_value) {
+    DCHECK(per_profile_default_zoom_level_value->GetType() ==
+           base::Value::TYPE_DOUBLE);
+    double per_profile_default_zoom_level;

[battre, 2014/10/08 11:27:10]

nit: = 0.0;

[wjmaclean, 2014/10/10 14:40:29]

Done.

+    per_profile_default_zoom_level_value->GetAsDouble(
+        &per_profile_default_zoom_level);
+    zoom_level_prefs_->SetDefaultZoomLevelPref(per_profile_default_zoom_level);
+    prefs_->ClearPref(prefs::kDefaultZoomLevelDeprecated);
+    migrated = true;
+  }
 
   const base::DictionaryValue* host_zoom_dictionary =
-      prefs_->GetDictionary(prefs::kPerHostZoomLevels);
-  // 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());
-      double zoom_level = 0;
+      prefs_->GetDictionary(prefs::kPerHostZoomLevelsDeprecated);
+  // Collect stats on frequency with which migrations are occuring. This measure
+  // is not perfect, since it will consider an un-migrated user with only
+  // default value as being already migrated, but it will catch all non-trivial
+  // migrations.
+  migrated |= !host_zoom_dictionary->empty();
+  UMA_HISTOGRAM_BOOLEAN("Settings.ZoomLevelPreferencesMigrated", migrated);
 
-      bool success = i.value().GetAsDouble(&zoom_level);
-      DCHECK(success);
+  for (base::DictionaryValue::Iterator it(*host_zoom_dictionary); !it.IsAtEnd();
+       it.Advance()) {
+    const std::string& host(it.key());
+    double zoom_level = 0;
 
-      // Filter out A) the empty host, B) zoom levels equal to the default; and
-      // remember them, so that we can later erase them from Prefs.
-      // Values of type A and B could have been stored due to crbug.com/364399.
-      // Values of type B could further have been stored before the default zoom
-      // level was set to its current value. In either case, SetZoomLevelForHost
-      // will ignore type B values, thus, to have consistency with HostZoomMap's
-      // internal state, these values must also be removed from Prefs.
-      if (host.empty() ||
-          content::ZoomValuesEqual(zoom_level,
-                                   host_zoom_map->GetDefaultZoomLevel())) {
-        keys_to_remove.push_back(host);
-        continue;
-      }
+    bool success = it.value().GetAsDouble(&zoom_level);
+    DCHECK(success);
 
-      host_zoom_map->SetZoomLevelForHost(host, zoom_level);
+    // Filter out A) the empty host, B) zoom levels equal to the default; and
+    // remember them, so that we can later erase them from Prefs.
+    // Values of type A and B could have been stored due to crbug.com/364399.
+    // Values of type B could further have been stored before the default zoom
+    // level was set to its current value. In either case, SetZoomLevelForHost
+    // will ignore type B values, thus, to have consistency with HostZoomMap's
+    // internal state, these values must also be removed from Prefs.
+    if (host.empty() ||
+        content::ZoomValuesEqual(zoom_level,
+                                 host_zoom_map->GetDefaultZoomLevel())) {
+      continue;
     }
 
-    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 push the profile per-host levels in through the HostZoomMap
+    // directly, which will update the zoom_level_prefs_ indirectly
+    // through the subsequent ZoomLevelChanged events.
+    host_zoom_map->SetZoomLevelForHost(host, zoom_level);
   }
 
-  zoom_subscription_ = host_zoom_map->AddZoomLevelChangedCallback(
-      base::Bind(&ProfileImpl::OnZoomLevelChanged, base::Unretained(this)));
+  // We're done migrating the profile per-host zoom level values, so we clear
+  // them all.
+  DictionaryPrefUpdate host_zoom_dictionary_update(
+      prefs_.get(), prefs::kPerHostZoomLevelsDeprecated);
+  host_zoom_dictionary_update->Clear();
 }
 
 base::FilePath ProfileImpl::last_selected_directory() {
   return GetPrefs()->GetFilePath(prefs::kSelectFileLastDirectory);
 }
 
 void ProfileImpl::set_last_selected_directory(const base::FilePath& path) {
   GetPrefs()->SetFilePath(prefs::kSelectFileLastDirectory, path);
 }
 
@@ skipping 200 matching lines @@
   // it to be set by asking for the prefs.
   GetPrefs();
   return last_session_exit_type_;
 }
 
 PrefService* ProfileImpl::GetPrefs() {
   DCHECK(prefs_);  // Should explicitly be initialized.
   return prefs_.get();
 }
 
+chrome::ChromeZoomLevelPrefs* ProfileImpl::GetZoomLevelPrefs() {
+  return zoom_level_prefs_.get();
+}
+
 PrefService* ProfileImpl::GetOffTheRecordPrefs() {
   DCHECK(prefs_);
   if (!otr_prefs_) {
     // The new ExtensionPrefStore is ref_counted and the new PrefService
     // stores a reference so that we do not leak memory here.
     otr_prefs_.reset(prefs_->CreateIncognitoPrefService(
         CreateExtensionPrefStore(this, true)));
   }
   return otr_prefs_.get();
 }
@@ skipping 128 matching lines @@
     top_sites_ = history::TopSites::Create(
         this, GetPath().Append(chrome::kTopSitesFilename));
   }
   return top_sites_.get();
 }
 
 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();
 }
 
 void ProfileImpl::EnsureSessionServiceCreated() {
   SessionServiceFactory::GetForProfile(this);
 }
 #endif
 
@@ skipping 238 matching lines @@
       domain_reliability::DomainReliabilityServiceFactory::GetInstance()->
           GetForBrowserContext(this);
   if (!service)
     return scoped_ptr<domain_reliability::DomainReliabilityMonitor>();
 
   return service->CreateMonitor(
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
       local_state,
       prefs::kMetricsReportingEnabled);
 }