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

chrome/browser/ui/prefs/prefs_tab_helper.cc

Index: chrome/browser/ui/prefs/prefs_tab_helper.cc
diff --git a/chrome/browser/ui/prefs/prefs_tab_helper.cc b/chrome/browser/ui/prefs/prefs_tab_helper.cc
index 816e08b60f60092d0f97371f9dc9d6261dbc86bb..e80e41d726feb11718ce188828cd76ca92b730f9 100644
--- a/chrome/browser/ui/prefs/prefs_tab_helper.cc
+++ b/chrome/browser/ui/prefs/prefs_tab_helper.cc
@@ -20,10 +20,12 @@
 #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"
@@ -315,13 +317,37 @@ void OverrideFontFamily(WebPreferences* prefs,
 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);
 
@@ -359,8 +385,15 @@ PrefsTabHelper::PrefsTabHelper(WebContents* contents)
                                   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,
@@ -566,9 +599,19 @@ void PrefsTabHelper::UpdateWebPreferences() {
       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();
 }