Refactor preference syncing.

chrome/browser/prefs/pref_service.h

Index: chrome/browser/prefs/pref_service.h
diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h
index bf904a4cf176d0c06a5120531f2d41c10df1fc9c..854086a6accc141dd49a45e76fabe835b65ef915 100644
--- a/chrome/browser/prefs/pref_service.h
+++ b/chrome/browser/prefs/pref_service.h
@@ -21,12 +21,13 @@ class DefaultPrefStore;
 class FilePath;
 class NotificationObserver;
 class PersistentPrefStore;
-class PrefChangeObserver;
+class PrefModelAssociator;
 class PrefNotifier;
 class PrefNotifierImpl;
 class PrefStore;
 class PrefValueStore;
 class Profile;
+class SyncableService;
 
 namespace subtle {
 class PrefMemberBase;
@@ -171,27 +172,46 @@ class PrefService : public base::NonThreadSafe,
   // Lands pending writes to disk.
   void CommitPendingWrite();
 
-  // Make the PrefService aware of a pref.
-  void RegisterBooleanPref(const char* path, bool default_value);
-  void RegisterIntegerPref(const char* path, int default_value);
-  void RegisterDoublePref(const char* path, double default_value);
-  void RegisterStringPref(const char* path, const std::string& default_value);
-  void RegisterFilePathPref(const char* path, const FilePath& default_value);
-  void RegisterListPref(const char* path);
-  void RegisterDictionaryPref(const char* path);
+  // Make the PrefService aware of a pref, and inform the Sync model associator
+  // if we want to synchronize it.
+  void RegisterBooleanPref(const char* path,
+                           bool default_value,
+                           bool syncable);
+  void RegisterIntegerPref(const char* path,
+                           int default_value,
+                           bool syncable);
+  void RegisterDoublePref(const char* path,
+                          double default_value,
+                          bool syncable);
+  void RegisterStringPref(const char* path,
+                          const std::string& default_value,
+                          bool syncable);
+  void RegisterFilePathPref(const char* path,
+                            const FilePath& default_value,
+                            bool syncable);
+  void RegisterListPref(const char* path, bool syncable);
+  void RegisterDictionaryPref(const char* path, bool syncable);
   // These take ownership of the default_value:
-  void RegisterListPref(const char* path, ListValue* default_value);
-  void RegisterDictionaryPref(const char* path, DictionaryValue* default_value);
+  void RegisterListPref(const char* path,
+                        ListValue* default_value,
+                        bool syncable);
+  void RegisterDictionaryPref(const char* path,
+                              DictionaryValue* default_value,
+                              bool syncable);
 
   // These variants use a default value from the locale dll instead.
   void RegisterLocalizedBooleanPref(const char* path,
-                                    int locale_default_message_id);
+                                    int locale_default_message_id,
+                                    bool syncable);
   void RegisterLocalizedIntegerPref(const char* path,
-                                    int locale_default_message_id);
+                                    int locale_default_message_id,
+                                    bool syncable);
   void RegisterLocalizedDoublePref(const char* path,
-                                   int locale_default_message_id);
+                                   int locale_default_message_id,
+                                   bool syncable);
   void RegisterLocalizedStringPref(const char* path,
-                                   int locale_default_message_id);
+                                   int locale_default_message_id,
+                                   bool syncable);
 
   // If the path is valid and the value at the end of the path matches the type
   // specified, it will return the specified value.  Otherwise, the default
@@ -228,7 +248,7 @@ class PrefService : public base::NonThreadSafe,
   // Value type will be TYPE_STRING.
   void SetInt64(const char* path, int64 value);
   int64 GetInt64(const char* path) const;
-  void RegisterInt64Pref(const char* path, int64 default_value);
+  void RegisterInt64Pref(const char* path, int64 default_value, bool syncable);
 
   // Returns true if a value has been set for the specified path.
   // NOTE: this is NOT the same as FindPreference. In particular
@@ -246,6 +266,10 @@ class PrefService : public base::NonThreadSafe,
 
   bool ReadOnly() const;
 
+  // SyncableService getter.
+  // TODO(zea): Have PrefService implement SyncableService directly.
+  SyncableService* GetSyncableService();
+
  protected:
   // Construct a new pref service, specifying the pref sources as explicit
   // PrefStore pointers. This constructor is what CreatePrefService() ends up
@@ -308,7 +332,9 @@ class PrefService : public base::NonThreadSafe,
   // NULL as it determines the preference value's type.
   // RegisterPreference must not be called twice for the same path.
   // This method takes ownership of |default_value|.
-  void RegisterPreference(const char* path, Value* default_value);
+  void RegisterPreference(const char* path,
+                          Value* default_value,
+                          bool syncable);
 
   // Sets the value for this pref path in the user pref store and informs the
   // PrefNotifier of the change.
@@ -344,6 +370,9 @@ class PrefService : public base::NonThreadSafe,
   // is used.
   PrefServiceDelegate* delegate_;
 
+  // The model associator that maintains the links with the sync db.
+  scoped_ptr<PrefModelAssociator> pref_sync_associator_;

[Mattias Nissler (ping if slow), 2011/05/06 04:00:02]

What's the advantage of adding the PrefModelAssociator into the internals of the
already non-trivial pref system? It seems like PrefModelAssociator needs two
things:
(a) check whether a pref is to be synced
(b) respond to pref notifications
We could achieve (a) by adding a state bit that can be queried through the
PrefService::Preference and (b) is already observable through the public
interface, so there'd be no need to host the associator within the PrefService.

[tim (not reviewing), 2011/05/06 04:27:54]

Part of this is we're getting rid of our "glue" layer in sync and organizing
per-data type glue code alongside the main services as part of the "think sync"
effort.

Perhaps we should discuss over VC / in person? Nicolas / myself would be happy
to discuss how we anticipate this making life easier for maintainers of prefs.

We were assuming that prefs ownership was as decentralized as use of
RegisterPref :)  It's a relief to know there's someone we can talk to.

+
   DISALLOW_COPY_AND_ASSIGN(PrefService);
 };