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

chrome/browser/ui/prefs/prefs_tab_helper.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/ui/prefs/prefs_tab_helper.h"
 
 #include <string>
 
 #include "base/prefs/overlay_user_pref_store.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
 #include "chrome/common/pref_font_webkit_names.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_names_util.h"
 #include "chrome/grit/locale_settings.h"
 #include "components/pref_registry/pref_registry_syncable.h"
+#include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/renderer_preferences.h"
 #include "content/public/common/web_preferences.h"
 #include "grit/platform_locale_settings.h"
 #include "third_party/icu/source/common/unicode/uchar.h"
 #include "third_party/icu/source/common/unicode/uscript.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && defined(ENABLE_THEMES)
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #endif
@@ skipping 271 matching lines @@
   else
     NOTREACHED() << "Unknown generic font family: " << generic_family;
   (*map)[script] = base::UTF8ToUTF16(pref_value);
 }
 
 }  // namespace
 
 PrefsTabHelper::PrefsTabHelper(WebContents* contents)
     : web_contents_(contents),
       weak_ptr_factory_(this) {
+  // TODO(wjmaclean): Convert this to use the content-specific zoom-level
+  // prefs when HostZoomMap moves to StoragePartition.
+  PrefService* zoom_level_prefs = GetProfile()->GetZoomLevelPrefs();
+  if (zoom_level_prefs) {
+    zoom_level_pref_change_registrar_.Init(zoom_level_prefs);
+    base::Closure renderer_callback = base::Bind(
+        &PrefsTabHelper::UpdateRendererPreferences, base::Unretained(this));
+    zoom_level_pref_change_registrar_.Add(prefs::kDefaultZoomLevel,
+                                          renderer_callback);
+    if (zoom_level_prefs->GetInitializationStatus() ==
+        PrefService::INITIALIZATION_STATUS_WAITING) {
+      // Unlike per-profile prefs, which are guaranteed to have initialized by
+      // the time we get here, zoom-level prefs may not have
+      // completed their initialization. In this case, we need to re-sync the
+      // renderer prefs once their initialization completes so that the default
+      // zoom level information is updated.
+      // TODO(wjmaclean) We need to determine if there are possible lifetime
+      // issues here, namely, can 'this' disappear before the zoom_level_prefs
+      // have finished initializing, causing a stale pointer via the callback?

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

I would worry that it can; imagine shutting down Chrome as it's still starting
up.  Not sure though.

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

I've attempted to address this concern by using a WeakPtr in the callback. I'm
not sure how to definitively test this ... is it the correct approach?

[Peter Kasting, 2014/09/11 21:06:55]

Sadly, I think we really have to know definitively whether this is a potential
problem or not.

If we understand whether this can happen, it should become clearer whether it's
testable.  If we don't know for sure, we shouldn't try and introduce safety
measures on the off chance something goes wrong.

Maybe jam or avi know enough about startup control flow to know whether there's
a problem?

[Bernhard Bauer, 2014/09/12 10:28:49]

Well, I don't know of anything that would make this _not_ a problem... You're
bringing up your own PrefService which loads its preferences asynchronously, so
it might call you back at any time.

Of course, it's difficult to _prove_ that this actually happens (because it
might not happen every time). What you could do is try to (empirically) prove
the opposite via DCHECK(!weak_ptr_factory_.HasWeakPtrs()) when this object is
destroyed.

+      zoom_level_prefs->AddPrefInitObserver(
+          base::Bind(&PrefsTabHelper::UpdateAfterZoomLevelPrefsInitialized,
+                     base::Unretained(this)));
+    }
+  }
+
   PrefService* prefs = GetProfile()->GetPrefs();
   pref_change_registrar_.Init(prefs);
   if (prefs) {
     base::Closure renderer_callback = base::Bind(
         &PrefsTabHelper::UpdateRendererPreferences, base::Unretained(this));
     pref_change_registrar_.Add(prefs::kAcceptLanguages, renderer_callback);
-    pref_change_registrar_.Add(prefs::kDefaultZoomLevel, renderer_callback);
     pref_change_registrar_.Add(prefs::kEnableDoNotTrack, renderer_callback);
     pref_change_registrar_.Add(prefs::kEnableReferrers, renderer_callback);
 
 #if !defined(OS_MACOSX)
     pref_change_registrar_.Add(prefs::kFullscreenAllowed, renderer_callback);
 #endif
 
     PrefChangeRegistrar::NamedChangeCallback webkit_callback = base::Bind(
         &PrefsTabHelper::OnWebPrefChanged, base::Unretained(this));
     for (int i = 0; i < kPrefsToObserveLength; ++i) {
@@ skipping 17 matching lines @@
                                   prefs::kWebKitCursiveFontFamilyMap,
                                   webkit_callback);
     RegisterFontFamilyMapObserver(&pref_change_registrar_,
                                   prefs::kWebKitFantasyFontFamilyMap,
                                   webkit_callback);
     RegisterFontFamilyMapObserver(&pref_change_registrar_,
                                   prefs::kWebKitPictographFontFamilyMap,
                                   webkit_callback);
   }
 
-  renderer_preferences_util::UpdateFromSystemSettings(
-      web_contents_->GetMutableRendererPrefs(), GetProfile());
+  content::RendererPreferences* render_prefs =
+      web_contents_->GetMutableRendererPrefs();
+  renderer_preferences_util::UpdateFromSystemSettings(render_prefs,
+                                                      GetProfile());
+  // TODO(wjmaclean): Convert this to use the HostZoomMap for the WebContents
+  // when HostZoomMap moves to StoragePartition.
+  render_prefs->default_zoom_level =
+      content::HostZoomMap::GetDefaultForBrowserContext(GetProfile())
+          ->GetDefaultZoomLevel();
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && defined(ENABLE_THEMES)
   registrar_.Add(this, chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
                  content::Source<ThemeService>(
                      ThemeServiceFactory::GetForProfile(GetProfile())));
 #endif
 #if defined(USE_AURA)
   registrar_.Add(this,
                  chrome::NOTIFICATION_BROWSER_FLING_CURVE_PARAMETERS_CHANGED,
                  content::NotificationService::AllSources());
@@ skipping 185 matching lines @@
 #endif  // defined(USE_AURA)
 
   NOTREACHED();
 }
 
 void PrefsTabHelper::UpdateWebPreferences() {
   web_contents_->GetRenderViewHost()->UpdateWebkitPreferences(
       web_contents_->GetRenderViewHost()->GetWebkitPreferences());
 }
 
+void PrefsTabHelper::UpdateAfterZoomLevelPrefsInitialized(bool success) {
+  if (success)
+    UpdateRendererPreferences();
+}
+
 void PrefsTabHelper::UpdateRendererPreferences() {
-  renderer_preferences_util::UpdateFromSystemSettings(
-      web_contents_->GetMutableRendererPrefs(), GetProfile());
+  content::RendererPreferences* prefs =
+      web_contents_->GetMutableRendererPrefs();
+  renderer_preferences_util::UpdateFromSystemSettings(prefs, GetProfile());
+  // TODO(wjmaclean): Convert this to use the HostZoomMap for the WebContents
+  // when HostZoomMap moves to StoragePartition.
+  prefs->default_zoom_level = content::HostZoomMap::GetDefaultForBrowserContext(
+                                  GetProfile())->GetDefaultZoomLevel();
   web_contents_->GetRenderViewHost()->SyncRendererPrefs();
 }
 
 Profile* PrefsTabHelper::GetProfile() {
   return Profile::FromBrowserContext(web_contents_->GetBrowserContext());
 }
 
 void PrefsTabHelper::OnFontFamilyPrefChanged(const std::string& pref_name) {
   // When a font family pref's value goes from non-empty to the empty string, we
   // must add it to the usual WebPreferences struct passed to the renderer.
@@ skipping 24 matching lines @@
   }
 }
 
 void PrefsTabHelper::OnWebPrefChanged(const std::string& pref_name) {
 #if !defined(OS_ANDROID)
   OnFontFamilyPrefChanged(pref_name);
 #endif
 
   web_contents_->GetRenderViewHost()->OnWebkitPreferencesChanged();
 }