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

whose values are determined by others (e.g. APP_NOTIFICATIONS on APPS).
Make the hand-written code use the system, and add {APP,EXTENSION}_SETTINGS.

R=akalin@chromium.org
TBR=estade@chromium.org
BUG=114974
TEST=unit_tests --gtest_filter=*Sync*


Review URL: http://codereview.chromium.org/9500005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124658 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync/sync_prefs.cc

@@ -20,6 +20,7 @@ SyncPrefObserver::~SyncPrefObserver() {}
 
 SyncPrefs::SyncPrefs(PrefService* pref_service)
     : pref_service_(pref_service) {
+  RegisterPrefGroups();
   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) &&
       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 ResolvePrefGroups(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);
-    }
-  }
-
-  for (syncable::ModelTypeSet::Iterator it = registered_types.First();
-       it.Good(); it.Inc()) {
-    SetDataTypePreferred(
-        it.Get(), preferred_types_with_dependents.Has(it.Get()));
+  preferred_types = ResolvePrefGroups(registered_types, preferred_types);
+  for (syncable::ModelTypeSet::Iterator i = registered_types.First();
+       i.Good(); i.Inc()) {
+    SetDataTypePreferred(i.Get(), preferred_types.Has(i.Get()));
   }
 }
 
@@ -405,6 +338,17 @@ const char* GetPrefNameForDataType(syncable::ModelType data_type) {
 
 }  // namespace
 
+void SyncPrefs::RegisterPrefGroups() {
+  pref_groups_[syncable::APPS].Put(syncable::APP_NOTIFICATIONS);
+  pref_groups_[syncable::APPS].Put(syncable::APP_SETTINGS);
+
+  pref_groups_[syncable::AUTOFILL].Put(syncable::AUTOFILL_PROFILE);
+
+  pref_groups_[syncable::EXTENSIONS].Put(syncable::EXTENSION_SETTINGS);
+
+  pref_groups_[syncable::PREFERENCES].Put(syncable::SEARCH_ENGINES);
+}
+
 void SyncPrefs::RegisterPreferences() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   CHECK(pref_service_);
@@ -518,4 +462,20 @@ void SyncPrefs::SetDataTypePreferred(
   pref_service_->SetBoolean(pref_name, is_preferred);
 }
 
+syncable::ModelTypeSet SyncPrefs::ResolvePrefGroups(
+    syncable::ModelTypeSet registered_types,
+    syncable::ModelTypeSet types) const {
+  DCHECK(registered_types.HasAll(types));
+  syncable::ModelTypeSet types_with_groups = types;
+  for (PrefGroupsMap::const_iterator i = pref_groups_.begin();
+      i != pref_groups_.end(); ++i) {
+    if (types.Has(i->first))
+      types_with_groups.PutAll(i->second);
+    else
+      types_with_groups.RemoveAll(i->second);
+  }
+  types_with_groups.RetainAll(registered_types);
+  return types_with_groups;
+}
+
 }  // namespace browser_sync

chrome/browser/sync/sync_prefs.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -118,6 +118,7 @@ class SyncPrefs : public base::SupportsWeakPtr<SyncPrefs>,
   syncable::ModelTypeSet GetAcknowledgeSyncedTypesForTest() const;
 
  private:
+  void RegisterPrefGroups();
   void RegisterPreferences();
 
   void RegisterDataTypePreferredPref(
@@ -125,6 +126,12 @@ class SyncPrefs : public base::SupportsWeakPtr<SyncPrefs>,
   bool GetDataTypePreferred(syncable::ModelType type) const;
   void SetDataTypePreferred(syncable::ModelType type, bool is_preferred);
 
+  // Returns a ModelTypeSet based on |types| expanded to include pref groups
+  // (see |pref_groups_|), but as a subset of |registered_types|.
+  syncable::ModelTypeSet ResolvePrefGroups(
+      syncable::ModelTypeSet registered_types,
+      syncable::ModelTypeSet types) const;
+
   base::NonThreadSafe non_thread_safe_;
 
   // May be NULL.
@@ -136,6 +143,16 @@ class SyncPrefs : public base::SupportsWeakPtr<SyncPrefs>,
   // configuration management.
   BooleanPrefMember pref_sync_managed_;
 
+  // Groups of prefs that always have the same value as a "master" pref.
+  // For example, the APPS group has {APP_NOTIFICATIONS, APP_SETTINGS}
+  // (as well as APPS, but that is implied), so
+  //   pref_groups_[syncable::APPS] =       { syncable::APP_NOTIFICATIONS,
+  //                                          syncable::APP_SETTINGS }
+  //   pref_groups_[syncable::EXTENSIONS] = { syncable::EXTENSION_SETTINGS }
+  // etc.
+  typedef std::map<syncable::ModelType, syncable::ModelTypeSet> PrefGroupsMap;
+  PrefGroupsMap pref_groups_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncPrefs);
 };
 

chrome/browser/sync/sync_prefs_unittest.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -36,9 +36,11 @@ syncable::ModelTypeSet GetNonPassiveTypes() {
 // Returns all types visible from the setup UI.
 syncable::ModelTypeSet GetUserVisibleTypes() {
   syncable::ModelTypeSet user_visible_types(GetNonPassiveTypes());
+  user_visible_types.Remove(syncable::APP_NOTIFICATIONS);
+  user_visible_types.Remove(syncable::APP_SETTINGS);
   user_visible_types.Remove(syncable::AUTOFILL_PROFILE);
+  user_visible_types.Remove(syncable::EXTENSION_SETTINGS);
   user_visible_types.Remove(syncable::SEARCH_ENGINES);
-  user_visible_types.Remove(syncable::APP_NOTIFICATIONS);
   return user_visible_types;
 }
 
@@ -112,6 +114,10 @@ TEST_F(SyncPrefsTest, PreferredTypesNotKeepEverythingSynced) {
     }
     if (it.Get() == syncable::APPS) {
       expected_preferred_types.Put(syncable::APP_NOTIFICATIONS);
+      expected_preferred_types.Put(syncable::APP_SETTINGS);
+    }
+    if (it.Get() == syncable::EXTENSIONS) {
+      expected_preferred_types.Put(syncable::EXTENSION_SETTINGS);
     }
     sync_prefs.SetPreferredDataTypes(non_passive_types, preferred_types);
     EXPECT_TRUE(expected_preferred_types.Equals(

chrome/browser/ui/webui/sync_setup_handler.cc

@@ -104,10 +104,8 @@ bool GetConfiguration(const std::string& json, SyncConfiguration* config) {
   bool sync_extensions;
   if (!result->GetBoolean("syncExtensions", &sync_extensions))
     return false;
-  if (sync_extensions) {
+  if (sync_extensions)
     config->data_types.Put(syncable::EXTENSIONS);
-    config->data_types.Put(syncable::EXTENSION_SETTINGS);
-  }
 
   bool sync_typed_urls;
   if (!result->GetBoolean("syncTypedUrls", &sync_typed_urls))
@@ -124,10 +122,8 @@ bool GetConfiguration(const std::string& json, SyncConfiguration* config) {
   bool sync_apps;
   if (!result->GetBoolean("syncApps", &sync_apps))
     return false;
-  if (sync_apps) {
+  if (sync_apps)
     config->data_types.Put(syncable::APPS);
-    config->data_types.Put(syncable::APP_SETTINGS);
-  }
 
   // Encryption settings.
   if (!result->GetBoolean("encryptAllData", &config->encrypt_all))