Removing PrefObserver usage, batch 4.

chrome/browser/extensions/api/font_settings/font_settings_api.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.
 
 // Font Settings Extension API implementation.
 
 #include "chrome/browser/extensions/api/font_settings/font_settings_api.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 67 matching lines @@
   if (!font_name.empty()) {
     gfx::Font font(font_name, 12);  // dummy font size
     return static_cast<gfx::PlatformFontWin*>(font.platform_font())->
         GetLocalizedFontName();
   }
 #endif
   return font_name;
 }
 
 // Registers |obs| to observe per-script font prefs under the path |map_name|.
-void RegisterFontFamilyMapObserver(PrefChangeRegistrar* registrar,
-                                   const char* map_name,
-                                   PrefObserver* obs) {
+void RegisterFontFamilyMapObserver(
+    PrefChangeRegistrar* registrar,
+    const char* map_name,
+    const PrefChangeRegistrar::NamedChangeCallback& callback) {
   for (size_t i = 0; i < prefs::kWebKitScriptsForFontFamilyMapsLength; ++i) {
     const char* script = prefs::kWebKitScriptsForFontFamilyMaps[i];
     std::string pref_name = base::StringPrintf("%s.%s", map_name, script);
-    registrar->Add(pref_name.c_str(), obs);
+    registrar->Add(pref_name.c_str(), callback);
   }
 }
 
 }  // namespace
 
 FontSettingsEventRouter::FontSettingsEventRouter(
     Profile* profile) : profile_(profile) {
   registrar_.Init(profile_->GetPrefs());
 
   AddPrefToObserve(prefs::kWebKitDefaultFixedFontSize,
                    kOnDefaultFixedFontSizeChanged,
                    kPixelSizeKey);
   AddPrefToObserve(prefs::kWebKitDefaultFontSize,
                    kOnDefaultFontSizeChanged,
                    kPixelSizeKey);
   AddPrefToObserve(prefs::kWebKitMinimumFontSize,
                    kOnMinimumFontSizeChanged,
                    kPixelSizeKey);
 
+  PrefChangeRegistrar::NamedChangeCallback callback = base::Bind(
+      &FontSettingsEventRouter::OnPreferenceChanged, base::Unretained(this));
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitStandardFontFamilyMap, this);
+                                prefs::kWebKitStandardFontFamilyMap, callback);
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitSerifFontFamilyMap, this);
+                                prefs::kWebKitSerifFontFamilyMap, callback);
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitSansSerifFontFamilyMap, this);
+                                prefs::kWebKitSansSerifFontFamilyMap, callback);
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitFixedFontFamilyMap, this);
+                                prefs::kWebKitFixedFontFamilyMap, callback);
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitCursiveFontFamilyMap, this);
+                                prefs::kWebKitCursiveFontFamilyMap, callback);
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitFantasyFontFamilyMap, this);
+                                prefs::kWebKitFantasyFontFamilyMap, callback);
   RegisterFontFamilyMapObserver(&registrar_,
-                                prefs::kWebKitPictographFontFamilyMap, this);
+                                prefs::kWebKitPictographFontFamilyMap,
+                                callback);

[Mattias Nissler (ping if slow), 2012/11/13 15:09:49]

All these could bind directly to the second half of OnPreferenceChanged
(extracting that to a separate function).

[Jói, 2012/11/13 15:16:45]

They could, but we would lose the DCHECK at the end, which I think is a useful
check to ensure that you don't add stuff to observe without adding handling for
it at the same time.

 }
 
 FontSettingsEventRouter::~FontSettingsEventRouter() {}
 
 void FontSettingsEventRouter::AddPrefToObserve(const char* pref_name,
                                                const char* event_name,
                                                const char* key) {
-  registrar_.Add(pref_name, this);
+  registrar_.Add(pref_name,
+                 base::Bind(&FontSettingsEventRouter::OnPreferenceChanged,
+                            base::Unretained(this)));
   pref_event_map_[pref_name] = std::make_pair(event_name, key);

[Mattias Nissler (ping if slow), 2012/11/13 15:09:49]

it seems weird to keep this separate map and the lookup in the callback if we
could just bind to OnFontNamePrefChanged with |event_name| and |key| bound.
Memory-wise it should be OK I hope given that the strings are already stored in
a map, we'd just be moving them into the closures.

[Jói, 2012/11/13 15:16:45]

I think you're probably right.  Am looping in falken@ for feedback, who seems to
have written the OnFont(Name)PrefChanged methods.

[falken, 2012/11/14 03:35:31]

I think it's a good idea to bind directly to OnFontPrefChanged with |event_name|
and |key| bound. This would avoid the need for pref_event_map_.

On the other hand, I'm not sure if we want to bind directly to
OnFontNamePrefChanged for the font name prefs. There are about 1000 font name
prefs so that would create 1000 callbacks. Note that these font name strings
aren't stored in a map; they are parsed out by ParseFontNamePrefPath.

 }
 
 void FontSettingsEventRouter::OnPreferenceChanged(
-    PrefServiceBase* pref_service,
     const std::string& pref_name) {
-  bool incognito = (pref_service != profile_->GetPrefs());
+  bool incognito = (registrar_.prefs() != profile_->GetPrefs());
   // We're only observing pref changes on the regular profile.
   DCHECK(!incognito);
 
   PrefEventMap::iterator iter = pref_event_map_.find(pref_name);
   if (iter != pref_event_map_.end()) {
     const std::string& event_name = iter->second.first;
     const std::string& key = iter->second.second;
-    OnFontPrefChanged(pref_service, pref_name, event_name, key, incognito);
+    OnFontPrefChanged(pref_name, event_name, key, incognito);
     return;
   }
 
   std::string generic_family;
   std::string script;
   if (pref_names_util::ParseFontNamePrefPath(pref_name, &generic_family,
                                              &script)) {
-    OnFontNamePrefChanged(pref_service, pref_name, generic_family, script,
-                          incognito);
+    OnFontNamePrefChanged(pref_name, generic_family, script, incognito);
     return;
   }
 
   NOTREACHED();
 }
 
 void FontSettingsEventRouter::OnFontNamePrefChanged(
-    PrefServiceBase* pref_service,
     const std::string& pref_name,
     const std::string& generic_family,
     const std::string& script,
     bool incognito) {
-  const PrefServiceBase::Preference* pref = pref_service->FindPreference(
+  const PrefServiceBase::Preference* pref = registrar_.prefs()->FindPreference(
       pref_name.c_str());
   CHECK(pref);
 
   std::string font_name;
   if (!pref->GetValue()->GetAsString(&font_name)) {
     NOTREACHED();
     return;
   }
   font_name = MaybeGetLocalizedFontName(font_name);
 
   ListValue args;
   DictionaryValue* dict = new DictionaryValue();
   args.Append(dict);
   dict->SetString(kFontIdKey, font_name);
   dict->SetString(kGenericFamilyKey, generic_family);
   dict->SetString(kScriptKey, script);
 
   extensions::preference_helpers::DispatchEventToExtensions(
       profile_,
       kOnFontChanged,
       &args,
       APIPermission::kFontSettings,
       incognito,
       pref_name);
 }
 
 void FontSettingsEventRouter::OnFontPrefChanged(
-    PrefServiceBase* pref_service,
     const std::string& pref_name,
     const std::string& event_name,
     const std::string& key,
     bool incognito) {
-  const PrefServiceBase::Preference* pref = pref_service->FindPreference(
+  const PrefServiceBase::Preference* pref = registrar_.prefs()->FindPreference(
       pref_name.c_str());
   CHECK(pref);
 
   ListValue args;
   DictionaryValue* dict = new DictionaryValue();
   args.Append(dict);
   dict->Set(key, pref->GetValue()->DeepCopy());
 
   extensions::preference_helpers::DispatchEventToExtensions(
       profile_,
@@ skipping 238 matching lines @@
 
 const char* SetMinimumFontSizeFunction::GetPrefName() {
   return prefs::kWebKitMinimumFontSize;
 }
 
 const char* SetMinimumFontSizeFunction::GetKey() {
   return kPixelSizeKey;
 }
 
 }  // namespace extensions