[Sync] Add support for dynamically enabling/disabling types

The notion of Unready types is also introduced for those types which control whether
they should run, and therefore for which a configuration (which might
involve network access) should not even be performed.

BUG=368834

Review URL: https://codereview.chromium.org/312163004

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

9 files changed, 156 insertions, 40 deletions

diff --git a/components/sync_driver/backend_data_type_configurer.h b/components/sync_driver/backend_data_type_configurer.h
index dc91e34..0569745 100644
--- a/components/sync_driver/backend_data_type_configurer.h
+++ b/components/sync_driver/backend_data_type_configurer.h
@@ -32,6 +32,7 @@ class BackendDataTypeConfigurer {
     DISABLED,             // Not syncing. Disabled by user.
     FATAL,                // Not syncing due to unrecoverable error.
     CRYPTO,               // Not syncing due to a cryptographer error.
+    UNREADY,              // Not syncing due to transient error.
   };
   typedef std::map<syncer::ModelType, DataTypeConfigState>
       DataTypeConfigStateMap;
diff --git a/components/sync_driver/data_type_controller.cc b/components/sync_driver/data_type_controller.cc
index 3514a16..0d9d0aa 100644
--- a/components/sync_driver/data_type_controller.cc
+++ b/components/sync_driver/data_type_controller.cc
@@ -56,4 +56,8 @@ DataTypeController::disable_callback() {
   return disable_callback_;
 }
 
+bool DataTypeController::ReadyForStart() const {
+  return true;
+}
+
 }  // namespace browser_sync
diff --git a/components/sync_driver/data_type_controller.h b/components/sync_driver/data_type_controller.h
index be6e65a9..acceace 100644
--- a/components/sync_driver/data_type_controller.h
+++ b/components/sync_driver/data_type_controller.h
@@ -136,6 +136,13 @@ class DataTypeController
   // UserShare handle to associate model data with.
   void OnUserShareReady(syncer::UserShare* share);
 
+  // Whether the DataTypeController is ready to start. This is useful if the
+  // datatype itself must make the decision about whether it should be enabled
+  // at all (and therefore whether the initial download of the sync data for
+  // the type should be performed).
+  // Returns true by default.
+  virtual bool ReadyForStart() const;
+
  protected:
   friend class base::RefCountedDeleteOnMessageLoop<DataTypeController>;
   friend class base::DeleteHelper<DataTypeController>;
diff --git a/components/sync_driver/data_type_manager.cc b/components/sync_driver/data_type_manager.cc
index 9bc9627..e3b6b32 100644
--- a/components/sync_driver/data_type_manager.cc
+++ b/components/sync_driver/data_type_manager.cc
@@ -29,9 +29,6 @@ DataTypeManager::ConfigureResult::ConfigureResult(
       failed_data_types(failed_data_types),
       unfinished_data_types(unfinished_data_types),
       needs_crypto(needs_crypto) {
-  if (!failed_data_types.empty() || !needs_crypto.Empty()) {
-    DCHECK_NE(OK, status);
-  }
 }
 
 DataTypeManager::ConfigureResult::~ConfigureResult() {
diff --git a/components/sync_driver/data_type_manager_impl.cc b/components/sync_driver/data_type_manager_impl.cc
index 1d4a96e..ba1709e 100644
--- a/components/sync_driver/data_type_manager_impl.cc
+++ b/components/sync_driver/data_type_manager_impl.cc
@@ -83,8 +83,26 @@ void DataTypeManagerImpl::Configure(syncer::ModelTypeSet desired_types,
   syncer::ModelTypeSet filtered_desired_types;
   for (syncer::ModelTypeSet::Iterator type = desired_types.First();
       type.Good(); type.Inc()) {
+    DataTypeController::TypeMap::const_iterator iter =
+        controllers_->find(type.Get());
     if (syncer::IsControlType(type.Get()) ||
-        controllers_->find(type.Get()) != controllers_->end()) {
+        iter != controllers_->end()) {
+      if (iter != controllers_->end()) {
+        if (!iter->second->ReadyForStart()) {
+          // Add the type to the unready types set to prevent purging it. It's
+          // up to the datatype controller to, if necessary, explicitly
+          // mark the type as broken to trigger a purge.
+          syncer::SyncError error(FROM_HERE,
+                                  syncer::SyncError::UNREADY_ERROR,
+                                  "Datatype not ready at config time.",
+                                  type.Get());
+          std::map<syncer::ModelType, syncer::SyncError> errors;
+          errors[type.Get()] = error;
+          failed_data_types_handler_->UpdateFailedDataTypes(errors);
+        } else {
+          failed_data_types_handler_->ResetUnreadyErrorFor(type.Get());
+        }
+      }
       filtered_desired_types.Put(type.Get());
     }
   }
@@ -111,19 +129,9 @@ void DataTypeManagerImpl::ConfigureImpl(
     return;
   }
 
-  if (state_ == CONFIGURED &&
-      last_requested_types_.Equals(desired_types) &&
-      reason == syncer::CONFIGURE_REASON_RECONFIGURATION &&
-      syncer::Intersection(failed_data_types_handler_->GetFailedTypes(),
-                           last_requested_types_).Empty()) {
-    // If the set of enabled types hasn't changed and there are no failing
-    // types, we can exit out early.
-    DVLOG(1) << "Reconfigure with same types, bypassing confguration.";
-    NotifyStart();
-    ConfigureResult result(OK, last_requested_types_);
-    NotifyDone(result);
-    return;
-  }
+  // TODO(zea): consider not performing a full configuration once there's a
+  // reliable way to determine if the requested set of enabled types matches the
+  // current set.
 
   last_requested_types_ = desired_types;
   last_configure_reason_ = reason;
@@ -141,18 +149,20 @@ void DataTypeManagerImpl::ConfigureImpl(
 BackendDataTypeConfigurer::DataTypeConfigStateMap
 DataTypeManagerImpl::BuildDataTypeConfigStateMap(
     const syncer::ModelTypeSet& types_being_configured) const {
-  // 1. Get the failed types (both due to fatal and crypto errors).
+  // 1. Get the failed types (due to fatal, crypto, and unready errors).
   // 2. Add the difference between last_requested_types_ and the failed types
   //    as CONFIGURE_INACTIVE.
   // 3. Flip |types_being_configured| to CONFIGURE_ACTIVE.
   // 4. Set non-enabled user types as DISABLED.
-  // 5. Set the fatal and crypto types to their respective states.
+  // 5. Set the fatal, crypto, and unready types to their respective states.
   syncer::ModelTypeSet error_types =
       failed_data_types_handler_->GetFailedTypes();
   syncer::ModelTypeSet fatal_types =
       failed_data_types_handler_->GetFatalErrorTypes();
   syncer::ModelTypeSet crypto_types =
       failed_data_types_handler_->GetCryptoErrorTypes();
+  syncer::ModelTypeSet unready_types=
+      failed_data_types_handler_->GetUnreadyErrorTypes();
 
   // Types with persistence errors are only purged/resynced when they're
   // actively being configured.
@@ -160,6 +170,9 @@ DataTypeManagerImpl::BuildDataTypeConfigStateMap(
       failed_data_types_handler_->GetPersistenceErrorTypes();
   persistence_types.RetainAll(types_being_configured);
 
+  // Types with unready errors do not count as unready if they've been disabled.
+  unready_types.RetainAll(last_requested_types_);
+
   syncer::ModelTypeSet enabled_types = last_requested_types_;
   enabled_types.RemoveAll(error_types);
   syncer::ModelTypeSet disabled_types =
@@ -191,6 +204,9 @@ DataTypeManagerImpl::BuildDataTypeConfigStateMap(
   BackendDataTypeConfigurer::SetDataTypesState(
       BackendDataTypeConfigurer::CRYPTO, crypto_types,
         &config_state_map);
+  BackendDataTypeConfigurer::SetDataTypesState(
+      BackendDataTypeConfigurer::UNREADY, unready_types,
+        &config_state_map);
   return config_state_map;
 }
 
diff --git a/components/sync_driver/data_type_manager_impl_unittest.cc b/components/sync_driver/data_type_manager_impl_unittest.cc
index ee88568..f501737 100644
--- a/components/sync_driver/data_type_manager_impl_unittest.cc
+++ b/components/sync_driver/data_type_manager_impl_unittest.cc
@@ -1154,4 +1154,46 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureForBackupRollback) {
                   syncer::CONFIGURE_REASON_BACKUP_ROLLBACK);
 }
 
+TEST_F(SyncDataTypeManagerImplTest, ReenableAfterDataTypeError) {
+  syncer::SyncError error(FROM_HERE,
+                          syncer::SyncError::DATATYPE_ERROR,
+                          "Datatype disabled",
+                          syncer::BOOKMARKS);
+  std::map<syncer::ModelType, syncer::SyncError> errors;
+  errors[syncer::BOOKMARKS] = error;
+  failed_data_types_handler_.UpdateFailedDataTypes(errors);
+
+  AddController(PREFERENCES);  // Will succeed.
+  AddController(BOOKMARKS);    // Will be disabled due to datatype error.
+
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::PARTIAL_SUCCESS);
+
+  Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES));
+  FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet());
+  FinishDownload(*dtm_, ModelTypeSet(PREFERENCES), ModelTypeSet());
+  GetController(PREFERENCES)->FinishStart(DataTypeController::OK);
+  EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state());
+  EXPECT_EQ(DataTypeController::RUNNING, GetController(PREFERENCES)->state());
+  EXPECT_EQ(DataTypeController::NOT_RUNNING, GetController(BOOKMARKS)->state());
+
+  Mock::VerifyAndClearExpectations(&observer_);
+
+  // Re-enable bookmarks.
+  failed_data_types_handler_.ResetDataTypeErrorFor(syncer::BOOKMARKS);
+
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::OK);
+
+  Configure(dtm_.get(), ModelTypeSet(BOOKMARKS, PREFERENCES));
+  EXPECT_EQ(DataTypeManager::DOWNLOAD_PENDING, dtm_->state());
+  FinishDownload(*dtm_, ModelTypeSet(), ModelTypeSet());
+  FinishDownload(*dtm_, ModelTypeSet(BOOKMARKS), ModelTypeSet());
+  EXPECT_EQ(DataTypeManager::CONFIGURING, dtm_->state());
+  GetController(BOOKMARKS)->FinishStart(DataTypeController::OK);
+  EXPECT_EQ(DataTypeManager::CONFIGURED, dtm_->state());
+  EXPECT_EQ(DataTypeController::RUNNING, GetController(PREFERENCES)->state());
+  EXPECT_EQ(DataTypeController::RUNNING, GetController(BOOKMARKS)->state());
+}
+
 }  // namespace browser_sync
diff --git a/components/sync_driver/failed_data_types_handler.cc b/components/sync_driver/failed_data_types_handler.cc
index bf6dde1..7906844 100644
--- a/components/sync_driver/failed_data_types_handler.cc
+++ b/components/sync_driver/failed_data_types_handler.cc
@@ -38,9 +38,14 @@ bool FailedDataTypesHandler::UpdateFailedDataTypes(const TypeErrorMap& errors) {
        ++iter) {
     syncer::SyncError::ErrorType failure_type = iter->second.error_type();
     switch (failure_type) {
+      case syncer::SyncError::UNSET:
+        NOTREACHED();
+        break;
       case syncer::SyncError::UNRECOVERABLE_ERROR:
+        unrecoverable_errors_.insert(*iter);
+        break;
       case syncer::SyncError::DATATYPE_ERROR:
-        fatal_errors_.insert(*iter);
+        data_type_errors_.insert(*iter);
         break;
       case syncer::SyncError::CRYPTO_ERROR:
         crypto_errors_.insert(*iter);
@@ -48,17 +53,20 @@ bool FailedDataTypesHandler::UpdateFailedDataTypes(const TypeErrorMap& errors) {
       case syncer::SyncError::PERSISTENCE_ERROR:
         persistence_errors_.insert(*iter);
         break;
-      default:
-        NOTREACHED();
+      case syncer::SyncError::UNREADY_ERROR:
+        unready_errors_.insert(*iter);
+        break;
     }
   }
   return true;
 }
 
 void FailedDataTypesHandler::Reset() {
-  fatal_errors_.clear();
+  unrecoverable_errors_.clear();
+  data_type_errors_.clear();
   crypto_errors_.clear();
   persistence_errors_.clear();
+  unready_errors_.clear();
 }
 
 void FailedDataTypesHandler::ResetCryptoErrors() {
@@ -73,26 +81,37 @@ void FailedDataTypesHandler::ResetPersistenceErrorsFrom(
   }
 }
 
+bool FailedDataTypesHandler::ResetDataTypeErrorFor(syncer::ModelType type) {
+  return data_type_errors_.erase(type) > 0;
+}
+
+bool FailedDataTypesHandler::ResetUnreadyErrorFor(syncer::ModelType type) {
+  return unready_errors_.erase(type) > 0;
+}
+
 FailedDataTypesHandler::TypeErrorMap FailedDataTypesHandler::GetAllErrors()
     const {
   TypeErrorMap result;
-
-  if (AnyFailedDataType()) {
-    result = fatal_errors_;
-    result.insert(crypto_errors_.begin(), crypto_errors_.end());
-    result.insert(persistence_errors_.begin(), persistence_errors_.end());
-  }
+  result = unrecoverable_errors_;
+  result.insert(data_type_errors_.begin(), data_type_errors_.end());
+  result.insert(crypto_errors_.begin(), crypto_errors_.end());
+  result.insert(persistence_errors_.begin(), persistence_errors_.end());
+  result.insert(unready_errors_.begin(), unready_errors_.end());
   return result;
 }
 
 syncer::ModelTypeSet FailedDataTypesHandler::GetFailedTypes() const {
   syncer::ModelTypeSet result = GetFatalErrorTypes();
   result.PutAll(GetCryptoErrorTypes());
+  result.PutAll(GetUnreadyErrorTypes());
   return result;
 }
 
-syncer::ModelTypeSet FailedDataTypesHandler::GetFatalErrorTypes() const {
-  return GetTypesFromErrorMap(fatal_errors_);
+syncer::ModelTypeSet FailedDataTypesHandler::GetFatalErrorTypes()
+    const {
+  syncer::ModelTypeSet result = GetTypesFromErrorMap(unrecoverable_errors_);
+  result.PutAll(GetTypesFromErrorMap(data_type_errors_));
+  return result;
 }
 
 syncer::ModelTypeSet FailedDataTypesHandler::GetCryptoErrorTypes() const {
@@ -105,10 +124,17 @@ syncer::ModelTypeSet FailedDataTypesHandler::GetPersistenceErrorTypes() const {
   return result;
 }
 
+syncer::ModelTypeSet FailedDataTypesHandler::GetUnreadyErrorTypes() const {
+  syncer::ModelTypeSet result = GetTypesFromErrorMap(unready_errors_);
+  return result;
+}
+
 bool FailedDataTypesHandler::AnyFailedDataType() const {
   // Note: persistence errors are not failed types. They just trigger automatic
   // unapply + getupdates, at which point they are associated like normal.
-  return !fatal_errors_.empty() || !crypto_errors_.empty();
+  return !unrecoverable_errors_.empty() ||
+         !data_type_errors_.empty() ||
+         !crypto_errors_.empty();
 }
 
 }  // namespace browser_sync
diff --git a/components/sync_driver/failed_data_types_handler.h b/components/sync_driver/failed_data_types_handler.h
index bff7b15..3820a24 100644
--- a/components/sync_driver/failed_data_types_handler.h
+++ b/components/sync_driver/failed_data_types_handler.h
@@ -32,13 +32,24 @@ class FailedDataTypesHandler {
   // Resets those persistence errors that intersect with |purged_types|.
   void ResetPersistenceErrorsFrom(syncer::ModelTypeSet purged_types);
 
+  // Removes |type| from the data_type_errors_ set. Returns true if the type
+  // was removed from the error set, false if the type did not have a data type
+  // error to begin with.
+  bool ResetDataTypeErrorFor(syncer::ModelType type);
+
+  // Removes |type| from the unread_errors_ set. Returns true if the type
+  // was removed from the error set, false if the type did not have an unready
+  // error to begin with.
+  bool ResetUnreadyErrorFor(syncer::ModelType type);
+
   // Returns a list of all the errors this class has recorded.
   TypeErrorMap GetAllErrors() const;
 
-  // Returns all types with errors.
+  // Returns all types with failure errors. This includes, fatal, crypto, and
+  // unready types.`
   syncer::ModelTypeSet GetFailedTypes() const;
 
-  // Returns the types that are failing due to startup or runtime errors.
+  // Returns the types that are failing due to unrecoverable or datatype errors.
   syncer::ModelTypeSet GetFatalErrorTypes() const;
 
   // Returns the types that are failing due to cryptographer errors.
@@ -47,21 +58,33 @@ class FailedDataTypesHandler {
   // Returns the types that are failing due to persistence errors.
   syncer::ModelTypeSet GetPersistenceErrorTypes() const;
 
+  // Returns the types that cannot be configured due to not being ready.
+  syncer::ModelTypeSet GetUnreadyErrorTypes() const;
+
  private:
   // Returns true if there are any types with errors.
   bool AnyFailedDataType() const;
 
-  // List of data types that failed at startup due to association errors or
-  // runtime due to data type errors.
-  TypeErrorMap fatal_errors_;
+  // List of data types that failed due to unrecoverable errors and should
+  // be disabled.
+  TypeErrorMap unrecoverable_errors_;
+
+  // List of data types that failed due to runtime errors and should be
+  // disabled. These are different from unrecoverable_errors_ in that
+  // ResetDataTypeError can remove them from this list.
+  TypeErrorMap data_type_errors_;
 
   // List of data types that failed due to the cryptographer not being ready.
   TypeErrorMap crypto_errors_;
 
-  // List of data type that failed because sync did not persist the newest
+  // List of data types that failed because sync did not persist the newest
   // version of their data.
   TypeErrorMap persistence_errors_;
 
+  // List of data types that could not start due to not being ready. These can
+  // be marked as ready by calling ResetUnreadyErrorFor(..).
+  TypeErrorMap unready_errors_;
+
   DISALLOW_COPY_AND_ASSIGN(FailedDataTypesHandler);
 };
 
diff --git a/components/sync_driver/ui_data_type_controller.h b/components/sync_driver/ui_data_type_controller.h
index 547aac7..c1b80a9 100644
--- a/components/sync_driver/ui_data_type_controller.h
+++ b/components/sync_driver/ui_data_type_controller.h
@@ -69,8 +69,8 @@ class UIDataTypeController : public DataTypeController {
   // associate models. The default implementation is a no-op.
   // Return value:
   //   True - if models are ready and association can proceed.
-  //   False - if models are not ready. Associate() should be called when the
-  //           models are ready.
+  //   False - if models are not ready. OnModelLoaded() should be called when
+  //           the models are ready.
   virtual bool StartModels();
 
   // Perform any DataType controller specific state cleanup before stopping