Move enable/disable storage to prefs.

The ProfileSyncService will manage the enabled/disabled types.  Remove
CustomizeSyncWindowView's DataTypeManager dependency; it should really
just be operating on the preferences via the ProfileSyncService.

Clarify the difference between types being enabled (meaning the user
wants to sync them) versus being advertisable (meaning we support the
datatype, and advertise that the user has the option of syncing it).

BUG=34209,38340
TEST=After syncing, open the options dialog and recustomize your
options.  Verify that the enabled types from the wizard match the
initial state of the options window.  After changing the options and
hitting "accept", observe that a breakpoint in DTM::Configure gets
triggered, with exactly the new configuration set.  Verify that the
preferences are preserved on restart, as well as across Stop
Syncing / Start Syncing.

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

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

8 files changed, 181 insertions, 35 deletions

diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc
index 23e5ab1..787d9b6 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl.cc
@@ -86,11 +86,12 @@ void DataTypeManagerImpl::Configure(const TypeSet& desired_types) {
   needs_start_.clear();
   for (TypeSet::const_iterator it = desired_types.begin();
        it != desired_types.end(); ++it) {
-    DataTypeController* dtc = controllers_[*it];
-    if (dtc && (dtc->state() == DataTypeController::NOT_RUNNING ||
-                dtc->state() == DataTypeController::STOPPING)) {
-      needs_start_.push_back(dtc);
-      LOG(INFO) << "Will start " << dtc->name();
+    DataTypeController::TypeMap::const_iterator dtc = controllers_.find(*it);
+    if (dtc != controllers_.end() &&
+        (dtc->second->state() == DataTypeController::NOT_RUNNING ||
+         dtc->second->state() == DataTypeController::STOPPING)) {
+      needs_start_.push_back(dtc->second.get());
+      LOG(INFO) << "Will start " << dtc->second->name();
     }
   }
   // Sort these according to kStartOrder.
diff --git a/chrome/browser/sync/glue/data_type_manager_impl.h b/chrome/browser/sync/glue/data_type_manager_impl.h
index ef4ecba..92d2e1e 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl.h
+++ b/chrome/browser/sync/glue/data_type_manager_impl.h
@@ -93,7 +93,7 @@ class DataTypeManagerImpl : public DataTypeManager,
   SyncBackendHost* backend_;
   // Map of all data type controllers that are available for sync.
   // This list is determined at startup by various command line flags.
-  DataTypeController::TypeMap controllers_;
+  const DataTypeController::TypeMap controllers_;
   State state_;
   DataTypeController* current_dtc_;
   CancelableTask* download_ready_task_;
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index c5dc382..8dabc4b 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -147,10 +147,16 @@ void ProfileSyncService::RegisterPreferences() {
   pref_service->RegisterBooleanPref(prefs::kSyncHasSetupCompleted, false);
 
   // If you've never synced before, all datatypes are on by default.
+  // TODO(nick): Perhaps a better model would be to always default to false,
+  // and explicitly call SetDataTypes() when the user shows the wizard.
+  bool enable_by_default =
+      !pref_service->HasPrefPath(prefs::kSyncHasSetupCompleted);
+
   pref_service->RegisterBooleanPref(prefs::kSyncBookmarks, true);
-  pref_service->RegisterBooleanPref(prefs::kSyncPreferences, true);
-  pref_service->RegisterBooleanPref(prefs::kSyncAutofill, true);
-  pref_service->RegisterBooleanPref(prefs::kSyncThemes, true);
+  pref_service->RegisterBooleanPref(prefs::kSyncPreferences, enable_by_default);
+  pref_service->RegisterBooleanPref(prefs::kSyncAutofill, enable_by_default);
+  pref_service->RegisterBooleanPref(prefs::kSyncThemes, enable_by_default);
+  pref_service->RegisterBooleanPref(prefs::kSyncTypedUrls, enable_by_default);
 
   // TODO(albertb): Consider getting rid of this preference once we have a UI
   // for per-data type disabling.
@@ -163,6 +169,8 @@ void ProfileSyncService::ClearPreferences() {
   PrefService* pref_service = profile_->GetPrefs();
   pref_service->ClearPref(prefs::kSyncLastSyncedTime);
   pref_service->ClearPref(prefs::kSyncHasSetupCompleted);
+  // TODO(nick): The current behavior does not clear e.g. prefs::kSyncBookmarks.
+  // Is that really what we want?
   pref_service->ScheduleSavePersistentPrefs();
 }
 
@@ -199,6 +207,11 @@ void ProfileSyncService::InitializeBackend(bool delete_sync_data_folder) {
   invalidate_sync_xmpp_login = CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kInvalidateSyncXmppLogin);
 #endif
+
+  // TODO(nick): Pass |types| to Initialize when supported.
+  syncable::ModelTypeSet types;
+  GetPreferredDataTypes(&types);
+
   backend_->Initialize(sync_service_url_, profile_->GetRequestContext(),
                        GetLsidForAuthBootstraping(), delete_sync_data_folder,
                        invalidate_sync_login,
@@ -251,6 +264,15 @@ void ProfileSyncService::EnableForUser() {
   }
   expecting_first_run_auth_needed_event_ = true;
 
+  // Lock in the preference values for the datatype's enable/disable state.
+  // This won't change any values, but it will cause the current default
+  // values to persist as explicit preferences, even if the user doesn't
+  // click the "configure" button during the setup process.
+  DCHECK(!data_type_manager_.get());
+  syncable::ModelTypeSet preferred_types;
+  GetPreferredDataTypes(&preferred_types);
+  ChangePreferredDataTypes(preferred_types);
+
   StartUp();
   FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
 }
@@ -283,6 +305,26 @@ void ProfileSyncService::UpdateLastSyncedTime() {
   profile_->GetPrefs()->ScheduleSavePersistentPrefs();
 }
 
+// static
+const wchar_t* ProfileSyncService::GetPrefNameForDataType(
+    syncable::ModelType data_type) {
+  switch (data_type) {
+    case syncable::BOOKMARKS:
+      return prefs::kSyncBookmarks;
+    case syncable::PREFERENCES:
+      return prefs::kSyncPreferences;
+    case syncable::AUTOFILL:
+      return prefs::kSyncAutofill;
+    case syncable::THEMES:
+      return prefs::kSyncThemes;
+    case syncable::TYPED_URLS:
+      return prefs::kSyncTypedUrls;
+    default:
+      NOTREACHED();
+      return NULL;
+  }
+}
+
 // An invariant has been violated.  Transition to an error state where we try
 // to do as little work as possible, to avoid further corruption or crashes.
 void ProfileSyncService::OnUnrecoverableError() {
@@ -407,12 +449,10 @@ string16 ProfileSyncService::GetAuthenticatedUsername() const {
 }
 
 void ProfileSyncService::OnUserClickedCustomize() {
-  // This is coming from the gaia_login, so set configure_on_accept=false
-  // (because when the user accepts, he/she will not have signed in yet).
 #if defined(OS_WIN)
-  CustomizeSyncWindowView::Show(NULL, profile_, false);
+  CustomizeSyncWindowView::Show(NULL, profile_);
 #elif defined(OS_LINUX)
-  ShowCustomizeSyncWindow(profile_, false);
+  ShowCustomizeSyncWindow(profile_);
 #endif
 }
 
@@ -437,9 +477,61 @@ void ProfileSyncService::OnUserCancelledDialog() {
   FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
 }
 
-void ProfileSyncService::ChangeDataTypes(
-  const browser_sync::DataTypeManager::TypeSet& desired_types) {
-  data_type_manager_->Configure(desired_types);
+void ProfileSyncService::ChangePreferredDataTypes(
+    const syncable::ModelTypeSet& preferred_types) {
+
+  // Filter out any datatypes which aren't registered, or for which
+  // the preference can't be set.
+  syncable::ModelTypeSet registered_types;
+  GetRegisteredDataTypes(&registered_types);
+  for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
+    syncable::ModelType model_type = syncable::ModelTypeFromInt(i);
+    if (!registered_types.count(model_type))
+      continue;
+    const wchar_t* pref_name = GetPrefNameForDataType(model_type);
+    if (!pref_name)
+      continue;
+    profile_->GetPrefs()->SetBoolean(pref_name,
+        preferred_types.count(model_type) != 0);
+  }
+
+  if (data_type_manager_.get()) {
+    syncable::ModelTypeSet validated_preferred_types;
+    GetPreferredDataTypes(&validated_preferred_types);
+    data_type_manager_->Configure(validated_preferred_types);
+  }
+}
+
+void ProfileSyncService::GetPreferredDataTypes(
+    syncable::ModelTypeSet* preferred_types) const {
+ preferred_types->clear();
+
+  // Filter out any datatypes which aren't registered, or for which
+  // the preference can't be read.
+  syncable::ModelTypeSet registered_types;
+  GetRegisteredDataTypes(&registered_types);
+  for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
+    syncable::ModelType model_type = syncable::ModelTypeFromInt(i);
+    if (!registered_types.count(model_type))
+      continue;
+    const wchar_t* pref_name = GetPrefNameForDataType(model_type);
+    if (!pref_name)
+      continue;
+    if (profile_->GetPrefs()->GetBoolean(pref_name))
+      preferred_types->insert(model_type);
+  }
+}
+
+void ProfileSyncService::GetRegisteredDataTypes(
+    syncable::ModelTypeSet* registered_types) const {
+  registered_types->clear();
+  // The data_type_controllers_ are determined by command-line flags; that's
+  // effectively what controls the values returned here.
+  for (DataTypeController::TypeMap::const_iterator it =
+       data_type_controllers_.begin();
+       it != data_type_controllers_.end(); ++it) {
+    registered_types->insert((*it).first);
+  }
 }
 
 void ProfileSyncService::StartProcessingChangesIfReady() {
@@ -449,15 +541,11 @@ void ProfileSyncService::StartProcessingChangesIfReady() {
     SetSyncSetupCompleted();
   }
 
-  std::set<syncable::ModelType> types;
-  for (DataTypeController::TypeMap::const_iterator it =
-           data_type_controllers_.begin();
-       it != data_type_controllers_.end(); ++it) {
-    types.insert((*it).first);
-  }
-
   data_type_manager_.reset(
       factory_->CreateDataTypeManager(backend_.get(), data_type_controllers_));
+
+  syncable::ModelTypeSet types;
+  GetPreferredDataTypes(&types);
   data_type_manager_->Configure(types);
 }
 
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index a79440c..ceacc59 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -46,7 +46,47 @@ class ProfileSyncServiceObserver {
 };
 
 // ProfileSyncService is the layer between browser subsystems like bookmarks,
-// and the sync backend.
+// and the sync backend.  Each subsystem is logically thought of as being
+// a sync datatype.
+//
+// Individual datatypes can, at any point, be in a variety of stages of being
+// "enabled".  Here are some specific terms for concepts used in this class:
+//
+//   'Registered' (feature suppression for a datatype)
+//
+//      When a datatype is registered, the user has the option of syncing it.
+//      The sync opt-in UI will show only registered types; a checkbox should
+//      never be shown for an unregistered type, and nor should it ever be
+//      synced.
+//
+//      A datatype is considered registered once RegisterDataTypeController
+//      has been called with that datatype's DataTypeController.
+//
+//   'Preferred' (user preferences and opt-out for a datatype)
+//
+//      This means the user's opt-in or opt-out preference on a per-datatype
+//      basis.  The sync service will try to make active exactly these types.
+//      If a user has opted out of syncing a particular datatype, it will
+//      be registered, but not preferred.
+//
+//      This state is controlled by the ConfigurePreferredDataTypes and
+//      GetPreferredDataTypes.  They are stored in the preferences system,
+//      and persist; though if a datatype is not registered, it cannot
+//      be a preferred datatype.
+//
+//   'Active' (run-time initialization of sync system for a datatype)
+//
+//      An active datatype is a preferred datatype that is actively being
+//      synchronized: the syncer has been instructed to querying the server
+//      for this datatype, first-time merges have finished, and there is an
+//      actively installed ChangeProcessor that listens for changes to this
+//      datatype, propagating such changes into and out of the sync backend
+//      as necessary.
+//
+//      When a datatype is in the process of becoming active, it may be
+//      in some intermediate state.  Those finer-grained intermediate states
+//      are differentiated by the DataTypeController state.
+//
 class ProfileSyncService : public browser_sync::SyncFrontend,
                            public browser_sync::UnrecoverableErrorHandler,
                            public NotificationObserver {
@@ -212,9 +252,25 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  // Changes which data types we're going to be syncing to |desired_types|.
-  virtual void ChangeDataTypes(
-    const browser_sync::DataTypeManager::TypeSet& desired_types);
+  // Changes which data types we're going to be syncing to |preferred_types|.
+  // If it is running, the DataTypeManager will be instructed to reconfigure
+  // the sync backend so that exactly these datatypes are actively synced.  See
+  // class comment for more on what it means for a datatype to be Preferred.
+  virtual void ChangePreferredDataTypes(
+      const syncable::ModelTypeSet& preferred_types);
+
+  // Get the set of currently enabled data types (as chosen or configured by
+  // the user).  See class comment for more on what it means for a datatype
+  // to be Preferred.
+  virtual void GetPreferredDataTypes(
+      syncable::ModelTypeSet* preferred_types) const;
+
+  // Gets the set of all data types that could be allowed (the set that
+  // should be advertised to the user).  These will typically only change
+  // via a command-line option.  See class comment for more on what it means
+  // for a datatype to be Registered.
+  virtual void GetRegisteredDataTypes(
+      syncable::ModelTypeSet* registered_types) const;
 
  protected:
   // Call this after any of the subsystems being synced (the bookmark
@@ -263,6 +319,8 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // Sets the last synced time to the current time.
   void UpdateLastSyncedTime();
 
+  static const wchar_t* GetPrefNameForDataType(syncable::ModelType data_type);
+
   // When running inside Chrome OS, extract the LSID cookie from the cookie
   // store to bootstrap the authentication process.
   virtual std::string GetLsidForAuthBootstraping();
diff --git a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
index 716cbe2..e784b49 100644
--- a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
@@ -353,7 +353,7 @@ TEST_F(ProfileSyncServiceTypedUrlTest, HasNativeEmptySync) {
   StartSyncService(&task);
   std::vector<history::URLRow> sync_entries;
   GetTypedUrlsFromSyncDB(&sync_entries);
-  ASSERT_EQ(1U, entries.size());
+  ASSERT_EQ(1U, sync_entries.size());
   EXPECT_TRUE(URLsEqual(entries[0], sync_entries[0]));
 }
 
diff --git a/chrome/browser/sync/sync_setup_flow.cc b/chrome/browser/sync/sync_setup_flow.cc
index 3d55eb9..5ddf892 100644
--- a/chrome/browser/sync/sync_setup_flow.cc
+++ b/chrome/browser/sync/sync_setup_flow.cc
@@ -263,12 +263,9 @@ void SyncSetupFlow::GetArgsForGaiaLogin(const ProfileSyncService* service,
 
   // TODO(dantasse) Remove this when multi-datatype sync is live.
 #if defined(OS_WIN) || defined(OS_LINUX)
-  browser_sync::DataTypeController::StateMap states;
-  browser_sync::DataTypeController::StateMap* controller_states = &states;
-  service->GetDataTypeControllerStates(controller_states);
-  args->SetBoolean(L"showCustomize",
-    controller_states->count(syncable::PREFERENCES) ||
-    controller_states->count(syncable::AUTOFILL));
+  syncable::ModelTypeSet registered_datatypes;
+  service->GetRegisteredDataTypes(&registered_datatypes);
+  args->SetBoolean(L"showCustomize", registered_datatypes.size() > 1);
 #else
   args->SetBoolean(L"showCustomize", false);
 #endif
diff --git a/chrome/browser/sync/sync_setup_wizard.cc b/chrome/browser/sync/sync_setup_wizard.cc
index 660ac9f..7b4d940 100644
--- a/chrome/browser/sync/sync_setup_wizard.cc
+++ b/chrome/browser/sync/sync_setup_wizard.cc
@@ -112,7 +112,7 @@ void SyncResourcesSource::StartDataRequest(const std::string& path_raw,
     localized_strings.SetString(L"cancel",
         l10n_util::GetString(IDS_CANCEL));
     localized_strings.SetString(L"customize",
-      l10n_util::GetString(IDS_SYNC_LOGIN_CUSTOMIZE));
+        l10n_util::GetString(IDS_SYNC_LOGIN_CUSTOMIZE));
     localized_strings.SetString(L"settingup",
         l10n_util::GetString(IDS_SYNC_LOGIN_SETTING_UP));
     localized_strings.SetString(L"success",
diff --git a/chrome/browser/sync/syncable/model_type.h b/chrome/browser/sync/syncable/model_type.h
index 61a5104..dc7aceb 100644
--- a/chrome/browser/sync/syncable/model_type.h
+++ b/chrome/browser/sync/syncable/model_type.h
@@ -10,6 +10,7 @@
 #define CHROME_BROWSER_SYNC_SYNCABLE_MODEL_TYPE_H_
 
 #include <bitset>
+#include <set>
 
 #include "base/logging.h"
 
@@ -54,6 +55,7 @@ enum ModelType {
 };
 
 typedef std::bitset<MODEL_TYPE_COUNT> ModelTypeBitSet;
+typedef std::set<ModelType> ModelTypeSet;
 
 inline ModelType ModelTypeFromInt(int i) {
   DCHECK_GE(i, 0);