Add a declarative way in c/b/s/sync_prefs.cc to say which data types have prefs

chrome/browser/sync/sync_prefs.cc

Index: chrome/browser/sync/sync_prefs.cc
diff --git a/chrome/browser/sync/sync_prefs.cc b/chrome/browser/sync/sync_prefs.cc
index b66d7c04b6e663b8f84f2b2537047c969b3d1182..6ade1f75e7bc50fac2137691c4a048a88e1d06d6 100644
--- a/chrome/browser/sync/sync_prefs.cc
+++ b/chrome/browser/sync/sync_prefs.cc
@@ -20,6 +20,7 @@ SyncPrefObserver::~SyncPrefObserver() {}
 
 SyncPrefs::SyncPrefs(PrefService* pref_service)
     : pref_service_(pref_service) {
+  RegisterDependentTypes();
   if (pref_service_) {
     RegisterPreferences();
     // Watch the preference that indicates sync is managed so we can take
@@ -124,8 +125,8 @@ syncable::ModelTypeSet SyncPrefs::GetPreferredDataTypes(
     return syncable::ModelTypeSet();
   }
 
-  // First remove any datatypes that are inconsistent with the current
-  // policies on the client.
+  // First remove any datatypes that are inconsistent with the current policies
+  // on the client (so that "keep everything synced" doesn't include them).
   if (pref_service_->HasPrefPath(prefs::kSavingBrowserHistoryDisabled) &&

[akalin, 2012/03/01 07:40:59]

Hmm...I'm a bit bothered by the fact that we filter registered types here but
not in the other function.  But I guess it's okay for now; don't ahve to deal
with it in this CL.

       pref_service_->GetBoolean(prefs::kSavingBrowserHistoryDisabled)) {
     registered_types.Remove(syncable::TYPED_URLS);
@@ -135,50 +136,14 @@ syncable::ModelTypeSet SyncPrefs::GetPreferredDataTypes(
     return registered_types;
   }
 
-  // Remove autofill_profile since it's controlled by autofill, and
-  // search_engines since it's controlled by preferences (see code below).
-  syncable::ModelTypeSet user_selectable_types(registered_types);
-  DCHECK(!user_selectable_types.Has(syncable::NIGORI));
-  user_selectable_types.Remove(syncable::AUTOFILL_PROFILE);
-  user_selectable_types.Remove(syncable::SEARCH_ENGINES);
-
-  // Remove app_notifications since it's controlled by apps (see
-  // code below).
-  // TODO(akalin): Centralize notion of all user selectable data types.
-  user_selectable_types.Remove(syncable::APP_NOTIFICATIONS);
-
   syncable::ModelTypeSet preferred_types;
-
-  for (syncable::ModelTypeSet::Iterator it = user_selectable_types.First();
+  for (syncable::ModelTypeSet::Iterator it = registered_types.First();
        it.Good(); it.Inc()) {
     if (GetDataTypePreferred(it.Get())) {
       preferred_types.Put(it.Get());
     }
   }
-
-  // Group the enabled/disabled state of autofill_profile with autofill, and
-  // search_engines with preferences (since only autofill and preferences are
-  // shown on the UI).
-  if (registered_types.Has(syncable::AUTOFILL) &&
-      registered_types.Has(syncable::AUTOFILL_PROFILE) &&
-      GetDataTypePreferred(syncable::AUTOFILL)) {
-    preferred_types.Put(syncable::AUTOFILL_PROFILE);
-  }
-  if (registered_types.Has(syncable::PREFERENCES) &&
-      registered_types.Has(syncable::SEARCH_ENGINES) &&
-      GetDataTypePreferred(syncable::PREFERENCES)) {
-    preferred_types.Put(syncable::SEARCH_ENGINES);
-  }
-
-  // Set app_notifications to the same enabled/disabled state as
-  // apps (since only apps is shown on the UI).
-  if (registered_types.Has(syncable::APPS) &&
-      registered_types.Has(syncable::APP_NOTIFICATIONS) &&
-      GetDataTypePreferred(syncable::APPS)) {
-    preferred_types.Put(syncable::APP_NOTIFICATIONS);
-  }
-
-  return preferred_types;
+  return ResolveDependentTypes(registered_types, preferred_types);
 }
 
 void SyncPrefs::SetPreferredDataTypes(
@@ -187,42 +152,10 @@ void SyncPrefs::SetPreferredDataTypes(
   DCHECK(non_thread_safe_.CalledOnValidThread());
   CHECK(pref_service_);
   DCHECK(registered_types.HasAll(preferred_types));
-  syncable::ModelTypeSet preferred_types_with_dependents(preferred_types);
-  // Set autofill_profile to the same enabled/disabled state as
-  // autofill (since only autofill is shown in the UI).
-  if (registered_types.Has(syncable::AUTOFILL) &&
-      registered_types.Has(syncable::AUTOFILL_PROFILE)) {
-    if (preferred_types_with_dependents.Has(syncable::AUTOFILL)) {
-      preferred_types_with_dependents.Put(syncable::AUTOFILL_PROFILE);
-    } else {
-      preferred_types_with_dependents.Remove(syncable::AUTOFILL_PROFILE);
-    }
-  }
-  // Set app_notifications to the same enabled/disabled state as
-  // apps (since only apps is shown in the UI).
-  if (registered_types.Has(syncable::APPS) &&
-      registered_types.Has(syncable::APP_NOTIFICATIONS)) {
-    if (preferred_types_with_dependents.Has(syncable::APPS)) {
-      preferred_types_with_dependents.Put(syncable::APP_NOTIFICATIONS);
-    } else {
-      preferred_types_with_dependents.Remove(syncable::APP_NOTIFICATIONS);
-    }
-  }
-  // Set search_engines to the same enabled/disabled state as
-  // preferences (since only preferences is shown in the UI).
-  if (registered_types.Has(syncable::PREFERENCES) &&
-      registered_types.Has(syncable::SEARCH_ENGINES)) {
-    if (preferred_types_with_dependents.Has(syncable::PREFERENCES)) {
-      preferred_types_with_dependents.Put(syncable::SEARCH_ENGINES);
-    } else {
-      preferred_types_with_dependents.Remove(syncable::SEARCH_ENGINES);
-    }
-  }
-
+  preferred_types = ResolveDependentTypes(registered_types, preferred_types);
   for (syncable::ModelTypeSet::Iterator it = registered_types.First();
        it.Good(); it.Inc()) {
-    SetDataTypePreferred(
-        it.Get(), preferred_types_with_dependents.Has(it.Get()));
+    SetDataTypePreferred(it.Get(), preferred_types.Has(it.Get()));
   }
 }
 
@@ -405,6 +338,14 @@ const char* GetPrefNameForDataType(syncable::ModelType data_type) {
 
 }  // namespace
 
+void SyncPrefs::RegisterDependentTypes() {
+  dependent_types_[syncable::APP_NOTIFICATIONS]  = syncable::APPS;
+  dependent_types_[syncable::APP_SETTINGS]       = syncable::APPS;
+  dependent_types_[syncable::AUTOFILL_PROFILE]   = syncable::AUTOFILL;
+  dependent_types_[syncable::EXTENSION_SETTINGS] = syncable::EXTENSIONS;
+  dependent_types_[syncable::SEARCH_ENGINES]     = syncable::PREFERENCES;
+}
+
 void SyncPrefs::RegisterPreferences() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   CHECK(pref_service_);
@@ -518,4 +459,17 @@ void SyncPrefs::SetDataTypePreferred(
   pref_service_->SetBoolean(pref_name, is_preferred);
 }
 
+syncable::ModelTypeSet SyncPrefs::ResolveDependentTypes(
+    const syncable::ModelTypeSet& registered_types,

[akalin, 2012/03/01 07:40:59]

don't use refs for ModelTypeSet (they're <64 bits)

[not at google - send to devlin, 2012/03/01 23:39:36]

Done.

+    const syncable::ModelTypeSet& types) const {
+  DCHECK(registered_types.HasAll(types));
+  syncable::ModelTypeSet with_dependent_types(types);

[akalin, 2012/03/01 07:40:59]

I think we still want to first remove the types in types that depend on other
types (e.g., APP_NOTIFICATIONS, AUTOFILL_PROFILES, etc.) before adding the
dependent types.

[not at google - send to devlin, 2012/03/01 23:39:36]

Done (the equivalent of).

+  for (DependentTypesMap::const_iterator i = dependent_types_.begin();
+      i != dependent_types_.end(); ++i) {
+    if (types.Has(i->second) && registered_types.Has(i->first))
+      with_dependent_types.Put(i->first);
+  }
+  return with_dependent_types;
+}
+
 }  // namespace browser_sync