Introduce PrefRegistrySyncable, simplifying PrefServiceSyncable.

chrome/browser/prefs/pref_service_syncable.cc

Index: chrome/browser/prefs/pref_service_syncable.cc
diff --git a/chrome/browser/prefs/pref_service_syncable.cc b/chrome/browser/prefs/pref_service_syncable.cc
index a64e6bf7ba7b01cbd764d7350c7d3c0b7931c21a..53f7ac5c36f5f31f22a85d1d900fbca561866449 100644
--- a/chrome/browser/prefs/pref_service_syncable.cc
+++ b/chrome/browser/prefs/pref_service_syncable.cc
@@ -13,63 +13,28 @@
 #include "chrome/browser/prefs/pref_model_associator.h"
 #include "chrome/browser/prefs/pref_notifier_impl.h"
 #include "chrome/browser/prefs/pref_registry.h"
-#include "chrome/browser/prefs/pref_registry_simple.h"
+#include "chrome/browser/prefs/pref_registry_syncable.h"
 #include "chrome/browser/prefs/pref_service_syncable_observer.h"
 #include "chrome/browser/prefs/pref_value_store.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/prefs/prefs_tab_helper.h"
-#include "ui/base/l10n/l10n_util.h"
 
-namespace {
-
-// A helper function for RegisterLocalized*Pref that creates a Value*
-// based on a localized resource.  Because we control the values in a
-// locale dll, this should always return a Value of the appropriate
-// type.
-Value* CreateLocaleDefaultValue(base::Value::Type type,
-                                int message_id) {
-  const std::string& resource_string = l10n_util::GetStringUTF8(message_id);
-  DCHECK(!resource_string.empty());
-  switch (type) {
-    case Value::TYPE_BOOLEAN: {
-      if ("true" == resource_string)
-        return Value::CreateBooleanValue(true);
-      if ("false" == resource_string)
-        return Value::CreateBooleanValue(false);
-      break;
-    }
-
-    case Value::TYPE_INTEGER: {
-      int val;
-      base::StringToInt(resource_string, &val);
-      return Value::CreateIntegerValue(val);
-    }
-
-    case Value::TYPE_DOUBLE: {
-      double val;
-      base::StringToDouble(resource_string, &val);
-      return Value::CreateDoubleValue(val);
-    }
-
-    case Value::TYPE_STRING: {
-      return Value::CreateStringValue(resource_string);
-    }
-
-    default: {
-      NOTREACHED() <<
-          "list and dictionary types cannot have default locale values";
-    }
-  }
-  NOTREACHED();
-  return Value::CreateNullValue();
+// static
+PrefServiceSyncable* PrefServiceSyncable::FromProfile(Profile* profile) {
+  return static_cast<PrefServiceSyncable*>(profile->GetPrefs());
 }
 
-}  // namespace
+// static
+PrefServiceSyncable* PrefServiceSyncable::IncognitoFromProfile(
+    Profile* profile) {
+  return static_cast<PrefServiceSyncable*>(profile->GetOffTheRecordPrefs());
+}
 
 PrefServiceSyncable::PrefServiceSyncable(
     PrefNotifierImpl* pref_notifier,
     PrefValueStore* pref_value_store,
     PersistentPrefStore* user_prefs,
-    PrefRegistry* pref_registry,
+    PrefRegistrySyncable* pref_registry,
     base::Callback<void(PersistentPrefStore::PrefReadError)>
         read_error_callback,
     bool async)
@@ -81,12 +46,33 @@ PrefServiceSyncable::PrefServiceSyncable(
                 async) {
   pref_sync_associator_.SetPrefService(this);
 
+  // Let PrefModelAssociator know about changes to preference values.
   pref_value_store->set_callback(
       base::Bind(&PrefModelAssociator::ProcessPrefChange,
                  base::Unretained(&pref_sync_associator_)));
+
+  // Add already-registered syncable preferences to PrefModelAssociator.
+  const std::set<std::string>& syncable_preferences =
+      pref_registry->syncable_preferences();
+  for (std::set<std::string>::const_iterator it = syncable_preferences.begin();
+       it != syncable_preferences.end();
+       ++it) {
+    AddRegisteredSyncablePreference(it->c_str());
+  }
+
+  // Watch for syncable preferences registered after this point.
+  pref_registry->SetSyncableRegistrationCallback(
+      base::Bind(&PrefServiceSyncable::AddRegisteredSyncablePreference,
+                 base::Unretained(this)));
 }
 
-PrefServiceSyncable::~PrefServiceSyncable() {}
+PrefServiceSyncable::~PrefServiceSyncable() {
+  // Remove our callback from the registry, since it may outlive us.
+  PrefRegistrySyncable* registry =
+      static_cast<PrefRegistrySyncable*>(pref_registry_.get());
+  registry->SetSyncableRegistrationCallback(
+      PrefRegistrySyncable::SyncableRegistrationCallback());
+}
 
 PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
     PrefStore* incognito_extension_prefs) {
@@ -96,17 +82,9 @@ PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
       new OverlayUserPrefStore(user_pref_store_.get());
   PrefsTabHelper::InitIncognitoUserPrefStore(incognito_pref_store);
 
-  // For the incognito service, we need a registry that shares the
-  // same default prefs, but does not interfere with callbacks on
-  // registration/unregistration made to the main service, or allow
-  // any registrations.
-  //
-  // TODO(joi): We can directly reuse the same PrefRegistry once
-  // PrefService no longer registers for callbacks on registration and
-  // unregistration.
-  scoped_refptr<PrefRegistry> incognito_registry = new PrefRegistry;
-  incognito_registry->defaults_ = pref_registry_->defaults_;
-
+  scoped_refptr<PrefRegistrySyncable> forked_registry =
+      static_cast<PrefRegistrySyncable*>(
+          pref_registry_.get())->ForkForIncognito();
   PrefServiceSyncable* incognito_service = new PrefServiceSyncable(
       pref_notifier,
       pref_value_store_->CloneAndSpecialize(
@@ -115,10 +93,10 @@ PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
           NULL,  // command_line_prefs
           incognito_pref_store,
           NULL,  // recommended
-          incognito_registry->defaults(),
+          forked_registry->defaults(),
           pref_notifier),
       incognito_pref_store,
-      incognito_registry,
+      forked_registry,
       read_error_callback_,
       false);
   return incognito_service;
@@ -137,138 +115,6 @@ void PrefServiceSyncable::RemoveObserver(
   observer_list_.RemoveObserver(observer);
 }
 
-void PrefServiceSyncable::UnregisterPreference(const char* path) {
-  // TODO(joi): Temporary until we have PrefRegistrySyncable.
-  static_cast<PrefRegistrySimple*>(
-      DeprecatedGetPrefRegistry())->DeprecatedUnregisterPreference(path);
-
-  if (pref_sync_associator_.IsPrefRegistered(path)) {
-    pref_sync_associator_.UnregisterPref(path);
-  }
-}
-
-void PrefServiceSyncable::RegisterBooleanPref(const char* path,
-                                              bool default_value,
-                                              PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path,
-                             Value::CreateBooleanValue(default_value),
-                             sync_status);
-}
-
-void PrefServiceSyncable::RegisterIntegerPref(const char* path,
-                                              int default_value,
-                                              PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path,
-                             Value::CreateIntegerValue(default_value),
-                             sync_status);
-}
-
-void PrefServiceSyncable::RegisterDoublePref(const char* path,
-                                             double default_value,
-                                             PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path,
-                             Value::CreateDoubleValue(default_value),
-                             sync_status);
-}
-
-void PrefServiceSyncable::RegisterStringPref(const char* path,
-                                             const std::string& default_value,
-                                             PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path,
-                             Value::CreateStringValue(default_value),
-                             sync_status);
-}
-
-void PrefServiceSyncable::RegisterFilePathPref(const char* path,
-                                               const FilePath& default_value,
-                                               PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path,
-                             Value::CreateStringValue(default_value.value()),
-                             sync_status);
-}
-
-void PrefServiceSyncable::RegisterListPref(const char* path,
-                                           PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path, new ListValue(), sync_status);
-}
-
-void PrefServiceSyncable::RegisterListPref(const char* path,
-                                           ListValue* default_value,
-                                           PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path, default_value, sync_status);
-}
-
-void PrefServiceSyncable::RegisterDictionaryPref(const char* path,
-                                                 PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path, new DictionaryValue(), sync_status);
-}
-
-void PrefServiceSyncable::RegisterDictionaryPref(const char* path,
-                                                 DictionaryValue* default_value,
-                                                 PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(path, default_value, sync_status);
-}
-
-void PrefServiceSyncable::RegisterLocalizedBooleanPref(
-    const char* path,
-    int locale_default_message_id,
-    PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(
-      path,
-      CreateLocaleDefaultValue(Value::TYPE_BOOLEAN, locale_default_message_id),
-      sync_status);
-}
-
-void PrefServiceSyncable::RegisterLocalizedIntegerPref(
-    const char* path,
-    int locale_default_message_id,
-    PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(
-      path,
-      CreateLocaleDefaultValue(Value::TYPE_INTEGER, locale_default_message_id),
-      sync_status);
-}
-
-void PrefServiceSyncable::RegisterLocalizedDoublePref(
-    const char* path,
-    int locale_default_message_id,
-    PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(
-      path,
-      CreateLocaleDefaultValue(Value::TYPE_DOUBLE, locale_default_message_id),
-      sync_status);
-}
-
-void PrefServiceSyncable::RegisterLocalizedStringPref(
-    const char* path,
-    int locale_default_message_id,
-    PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(
-      path,
-      CreateLocaleDefaultValue(Value::TYPE_STRING, locale_default_message_id),
-      sync_status);
-}
-
-void PrefServiceSyncable::RegisterInt64Pref(
-    const char* path,
-    int64 default_value,
-    PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(
-      path,
-      Value::CreateStringValue(base::Int64ToString(default_value)),
-      sync_status);
-}
-
-void PrefServiceSyncable::RegisterUint64Pref(
-    const char* path,
-    uint64 default_value,
-    PrefSyncStatus sync_status) {
-  RegisterSyncablePreference(
-      path,
-      Value::CreateStringValue(base::Uint64ToString(default_value)),
-      sync_status);
-}
-
 syncer::SyncableService* PrefServiceSyncable::GetSyncableService() {
   return &pref_sync_associator_;
 }
@@ -281,17 +127,20 @@ void PrefServiceSyncable::UpdateCommandLinePrefStore(
   PrefService::UpdateCommandLinePrefStore(cmd_line_store);
 }
 
+void PrefServiceSyncable::AddRegisteredSyncablePreference(const char* path) {
+  DCHECK(FindPreference(path));
+  pref_sync_associator_.RegisterPref(path);
+}
+
+void PrefServiceSyncable::RemoveRegisteredPreference(const char* path) {
+  PrefService::RemoveRegisteredPreference(path);
+
+  if (pref_sync_associator_.IsPrefRegistered(path)) {

[Mattias Nissler (ping if slow), 2013/02/06 17:53:33]

nit: no curlies required

[Jói, 2013/02/07 14:52:32]

Done.

+    pref_sync_associator_.UnregisterPref(path);
+  }
+}
+
 void PrefServiceSyncable::OnIsSyncingChanged() {
   FOR_EACH_OBSERVER(PrefServiceSyncableObserver, observer_list_,
                     OnIsSyncingChanged());
 }
-
-void PrefServiceSyncable::RegisterSyncablePreference(
-    const char* path, Value* default_value, PrefSyncStatus sync_status) {
-  // TODO(joi): Temporary until we have PrefRegistrySyncable.
-  static_cast<PrefRegistrySimple*>(
-      DeprecatedGetPrefRegistry())->RegisterPreference(path, default_value);
-  // Register with sync if necessary.
-  if (sync_status == SYNCABLE_PREF)
-    pref_sync_associator_.RegisterPref(path);
-}