Remove ModelAssociationManager::Associate()

After recent cleanups, this method did nothing except maintain
|associated_types_| - which itself wasn't used at all, except for some
DCHECKs, so it's removed now as well.
While we're here, also remove ModelAssociationManager::state_ which
wasn't used (or useful) either anymore.

Bug: 1102837
Change-Id: Id62edd0f4be324fd4449dfd16e2a98530ee0c3d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494867Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820258}

components/sync/driver/data_type_manager_impl.cc

@@ -679,9 +679,6 @@ void DataTypeManagerImpl::StartNextAssociation(AssociationGroup group) {
     types_to_associate = association_types_queue_.front().types;
   }
 
-  DVLOG(1) << "Associating " << ModelTypeSetToString(types_to_associate);
-  model_association_manager_.Associate(types_to_associate);
-
   for (ModelType type : types_to_associate) {
     if (ProtocolTypes().Has(type)) {
       RecordConfigurationStats(type);

components/sync/driver/model_association_manager.cc

@@ -53,10 +53,7 @@ static_assert(base::size(kStartOrder) ==
 ModelAssociationManager::ModelAssociationManager(
     const DataTypeController::TypeMap* controllers,
     ModelAssociationManagerDelegate* processor)
-    : controllers_(controllers),
-      delegate_(processor),
-      state_(IDLE),
-      notified_about_ready_for_configure_(false) {}
+    : controllers_(controllers), delegate_(processor) {}
 
 ModelAssociationManager::~ModelAssociationManager() = default;
 
@@ -86,7 +83,6 @@ void ModelAssociationManager::Initialize(ModelTypeSet desired_types,
   DVLOG(1) << "ModelAssociationManager: Initializing for "
            << ModelTypeSetToString(desired_types_);
 
-  state_ = INITIALIZED;
   notified_about_ready_for_configure_ = false;
 
   DVLOG(1) << "ModelAssociationManager: Stopping disabled types.";
@@ -156,7 +152,6 @@ void ModelAssociationManager::StopDatatypeImpl(
     DataTypeController* dtc,
     DataTypeController::StopCallback callback) {
   loaded_types_.Remove(dtc->type());
-  associated_types_.Remove(dtc->type());
 
   DCHECK(error.IsSet() || (dtc->state() != DataTypeController::NOT_RUNNING));
 
@@ -179,7 +174,6 @@ void ModelAssociationManager::LoadDesiredTypes() {
     DCHECK_NE(DataTypeController::STOPPING, dtc->state());
     if (dtc->state() == DataTypeController::NOT_RUNNING) {
       DCHECK(!loaded_types_.Has(dtc->type()));
-      DCHECK(!associated_types_.Has(dtc->type()));
       dtc->LoadModels(
           configure_context_,
           base::BindRepeating(&ModelAssociationManager::ModelLoadCallback,
@@ -190,20 +184,6 @@ void ModelAssociationManager::LoadDesiredTypes() {
   NotifyDelegateIfReadyForConfigure();
 }
 
-void ModelAssociationManager::Associate(
-    const ModelTypeSet& types_to_associate) {
-  DCHECK_EQ(INITIALIZED, state_);
-  DCHECK(notified_about_ready_for_configure_);
-
-  ModelTypeSet associating_types = types_to_associate;
-  associating_types.RetainAll(desired_types_);
-  associating_types.RemoveAll(associated_types_);
-
-  DCHECK(loaded_types_.HasAll(associating_types));
-
-  associated_types_.PutAll(associating_types);
-}
-
 void ModelAssociationManager::Stop(ShutdownReason shutdown_reason) {
   // Ignore callbacks from controllers.
   weak_ptr_factory_.InvalidateWeakPtrs();
@@ -222,9 +202,6 @@ void ModelAssociationManager::Stop(ShutdownReason shutdown_reason) {
 
   desired_types_.Clear();
   loaded_types_.Clear();
-  associated_types_.Clear();
-
-  state_ = IDLE;
 }
 
 void ModelAssociationManager::ModelLoadCallback(ModelType type,

components/sync/driver/model_association_manager.h

@@ -49,11 +49,8 @@ class ModelAssociationManager {
   virtual ~ModelAssociationManager();
 
   // Stops any data types that are *not* in |desired_types|, then kicks off
-  // loading of all |desired_types|. A subsequent Initialize() call is only
-  // allowed after the ModelAssociationManager has invoked
-  // OnModelAssociationDone() on the delegate. After this call, there should be
-  // several calls to Associate() to associate subsets of |desired_types|, which
-  // itself must be a subset of |preferred_types|.
+  // loading of all |desired_types|.
+  // |desired_types| must be a subset of |preferred_types|.
   // |preferred_types| contains all types selected by the user.
   void Initialize(ModelTypeSet desired_types,
                   ModelTypeSet preferred_types,
@@ -62,12 +59,6 @@ class ModelAssociationManager {
   // Can be called at any time. Synchronously stops all datatypes.
   void Stop(ShutdownReason shutdown_reason);
 
-  // Must only be called after all data type models have been loaded, i.e. after
-  // OnAllDataTypesReadyForConfigure() has been called on the delegate.
-  // |types_to_associate| should be subset of |desired_types| in Initialize().
-  // Synchronously invokes |OnModelAssociationDone| on the delegate.
-  void Associate(const ModelTypeSet& types_to_associate);
-
   // Stops an individual datatype |type| for |shutdown_reason|. |error| must be
   // an actual error (i.e. not UNSET).
   void StopDatatype(ModelType type,
@@ -75,19 +66,11 @@ class ModelAssociationManager {
                     SyncError error);
 
  private:
-  enum State {
-    // No configuration is in progress.
-    IDLE,
-    // The model association manager has been initialized with a set of desired
-    // types.
-    INITIALIZED,
-  };
-
   // Start loading non-running types that are in |desired_types_|.
   void LoadDesiredTypes();
 
   // Callback that will be invoked when the model for |type| finishes loading.
-  // This callback is passed to |LoadModels| function.
+  // This callback is passed to the controller's |LoadModels| method.
   void ModelLoadCallback(ModelType type, const SyncError& error);
 
   // A helper to stop an individual datatype.
@@ -107,21 +90,15 @@ class ModelAssociationManager {
   // The delegate in charge of handling model association results.
   ModelAssociationManagerDelegate* const delegate_;
 
-  State state_;
-
   ConfigureContext configure_context_;
 
   // Data types that are enabled.
   ModelTypeSet desired_types_;
 
-  // Data types that are loaded, i.e. ready to associate.
+  // Data types that are loaded.
   ModelTypeSet loaded_types_;
 
-  // Data types that are associated, i.e. no more action needed during
-  // reconfiguration if not disabled.
-  ModelTypeSet associated_types_;
-
-  bool notified_about_ready_for_configure_;
+  bool notified_about_ready_for_configure_ = false;
 
   base::WeakPtrFactory<ModelAssociationManager> weak_ptr_factory_{this};
 

components/sync/driver/model_association_manager_unittest.cc

@@ -80,8 +80,6 @@ TEST_F(SyncModelAssociationManagerTest, SimpleModelStart) {
   EXPECT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
   EXPECT_EQ(GetController(APPS)->state(), DataTypeController::MODEL_LOADED);
-
-  model_association_manager.Associate(types);
 }
 
 // Start a type, let it finish and then call stop.
@@ -95,7 +93,6 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterFinish) {
   model_association_manager.Initialize(/*desired_types=*/types,
                                        /*preferred_types=*/types,
                                        BuildConfigureContext());
-  model_association_manager.Associate(types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -121,8 +118,6 @@ TEST_F(SyncModelAssociationManagerTest, ModelLoadFailBeforeAssociationStart) {
                                        BuildConfigureContext());
 
   EXPECT_EQ(DataTypeController::FAILED, GetController(BOOKMARKS)->state());
-  model_association_manager.Associate(types);
-  EXPECT_EQ(DataTypeController::FAILED, GetController(BOOKMARKS)->state());
 }
 
 // Test that a runtime error is handled by stopping the type.
@@ -135,7 +130,6 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterConfiguration) {
   model_association_manager.Initialize(/*desired_types=*/types,
                                        /*preferred_types=*/types,
                                        BuildConfigureContext());
-  model_association_manager.Associate(types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -353,7 +347,6 @@ TEST_F(SyncModelAssociationManagerTest, KeepsMetadataForPreferredDataType) {
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        BuildConfigureContext());
-  model_association_manager.Associate(desired_types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -389,7 +382,6 @@ TEST_F(SyncModelAssociationManagerTest, ClearsMetadataForNotPreferredDataType) {
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        BuildConfigureContext());
-  model_association_manager.Associate(desired_types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -432,7 +424,6 @@ TEST_F(SyncModelAssociationManagerTest,
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
-  model_association_manager.Associate(desired_types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -451,7 +442,6 @@ TEST_F(SyncModelAssociationManagerTest,
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
-  model_association_manager.Associate(desired_types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -481,7 +471,6 @@ TEST_F(SyncModelAssociationManagerTest,
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
-  model_association_manager.Associate(desired_types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);
@@ -500,7 +489,6 @@ TEST_F(SyncModelAssociationManagerTest,
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
-  model_association_manager.Associate(desired_types);
 
   ASSERT_EQ(GetController(BOOKMARKS)->state(),
             DataTypeController::MODEL_LOADED);