[Sync] Support open tabs experiment enabling before sync setup completion.

chrome/browser/sync/profile_sync_service.cc

Index: chrome/browser/sync/profile_sync_service.cc
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 9cbb4df057af99e1ca562bad4cce61a0084c61b5..97f781fa90ec4e0370c3b02927732ed9747e1d32 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -607,7 +607,8 @@ void ProfileSyncService::OnSyncCycleCompleted() {
 void ProfileSyncService::OnDataTypesChanged(
     const syncable::ModelTypeSet& to_add) {
   // We don't bother doing anything if the migrator is busy.
-  if (migrator_->state() != browser_sync::BackendMigrator::IDLE) {
+  if (migrator_.get() &&

[akalin, 2011/10/15 01:21:55]

can remove the _.get() check, or convert to CHECK

[Nicolas Zea, 2011/10/15 02:06:27]

This is getting called before the frontend is notified of the backend
initializing (else the sync setup screen shows before we enable the type), so
these conditions have to stay.

[akalin, 2011/10/18 19:13:42]

Can you add a comment here to that effect?

+      migrator_->state() != browser_sync::BackendMigrator::IDLE) {
     VLOG(1) << "Dropping OnDataTypesChanged due to migrator busy.";
     return;
   }
@@ -627,12 +628,13 @@ void ProfileSyncService::OnDataTypesChanged(
 
   for (syncable::ModelTypeSet::const_iterator it = to_register.begin();
        it != to_register.end(); ++it) {
-    // Received notice to enable session sync. Check if sessions are
+    // Received notice to enable experimental type. Check if the type is
     // registered, and if not register a new datatype controller.
     RegisterNewDataType(*it);
-    // Enable the about:flags switch for sessions so we don't have to always
-    // perform this reconfiguration. Once we set this, sessions will remain
-    // registered, so we will no longer go down this code path.
+    // Enable the about:flags switch for the experimental type so we don't have
+    // to always perform this reconfiguration. Once we set this, the type will
+    // remain registered on restart, so we will no longer go down this code
+    // path.
     std::string experiment_name = GetExperimentNameForDataType(*it);
     if (experiment_name.empty())
       continue;
@@ -642,15 +644,18 @@ void ProfileSyncService::OnDataTypesChanged(
   }
 
   // Check if the user has "Keep Everything Synced" enabled. If so, we want
-  // to turn on sessions if it's not already on. Otherwise we leave it off.
-  // Note: if sessions are already registered, we don't turn it on. This
+  // to turn on all experimental types if they're not already on. Otherwise we
+  // leave them off.
+  // Note: if any types are already registered, we don't turn them on. This
   // covers the case where we're already in the process of reconfiguring
-  // to turn sessions on.
+  // to turn an experimental type on.
   if (sync_prefs_.HasKeepEverythingSynced()) {
     // Mark all data types as preferred.
     sync_prefs_.SetPreferredDataTypes(registered_types, registered_types);
 
-    if (!to_register.empty()) {
+    // Only automatically turn on types if we have already finished set up.
+    // Otherwise, just leave the experimental types on by default.
+    if (!to_register.empty() && HasSyncSetupCompleted() && migrator_.get()) {

[akalin, 2011/10/15 01:21:55]

can remove migrator_.get() check

[akalin, 2011/10/15 01:21:55]

do we still need the HasSyncSetupCompleted check?

[Nicolas Zea, 2011/10/15 02:06:27]

see above.

[Nicolas Zea, 2011/10/15 02:06:27]

Theoretically the migrator check covers this case, and vice versa, but I'd
rather not make assumptions about both of them being there in case something
changes in the future. The main point here is that we don't want to reconfigure
if the user hasn't finished setting up yet, so HasSyncSetupCompleted is the main
condition.

       VLOG(1) << "Dynamically enabling new datatypes: "
               << syncable::ModelTypeSetToString(to_register);
       OnMigrationNeededForTypes(to_register);