Cleanup: Remove ModelAssociationManagerDelegate::OnModelAssociationDone

This method was always called synchronously from
ModelAssociationManager::Associate(), so no need for a delegate
method - instead, the one implementation is inlined at the relevant
place.

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

components/sync/driver/data_type_manager_impl.cc

@@ -544,8 +544,7 @@ void DataTypeManagerImpl::StartNextDownload(
   association_info.high_priority_types_before = high_priority_types_before;
   association_types_queue_.push(association_info);
 
-  // Start associating those types that are already downloaded (does nothing
-  // if model associator is busy).
+  // Start associating those types that are already downloaded.
   StartNextAssociation(READY_AT_CONFIG);
 }
 
@@ -682,6 +681,33 @@ void DataTypeManagerImpl::StartNextAssociation(AssociationGroup group) {
 
   DVLOG(1) << "Associating " << ModelTypeSetToString(types_to_associate);
   model_association_manager_.Associate(types_to_associate);
+
+  DCHECK(state_ == STOPPING || state_ == CONFIGURING);
+
+  if (state_ == STOPPING)
+    return;
+
+  if (needs_reconfigure_) {
+    ProcessReconfigure();
+    return;
+  }
+
+  // If this model association was for the full set of types, then this priority
+  // set is done. Otherwise it was just the ready types and the unready types
+  // still need to be associated.
+  if (types_to_associate == association_types_queue_.front().types) {
+    association_types_queue_.pop();
+    if (!association_types_queue_.empty()) {
+      StartNextAssociation(READY_AT_CONFIG);
+    } else if (download_types_queue_.empty()) {
+      state_ = CONFIGURED;
+      NotifyDone(ConfigureResult(OK, types_to_associate));
+    }
+  } else {
+    DCHECK_EQ(association_types_queue_.front().ready_types, types_to_associate);
+    // Will do nothing if the types are still downloading.
+    StartNextAssociation(UNREADY_AT_CONFIG);
+  }
 }
 
 void DataTypeManagerImpl::OnSingleDataTypeWillStop(ModelType type,
@@ -742,40 +768,6 @@ void DataTypeManagerImpl::OnSingleDataTypeAssociationDone(ModelType type) {
   }
 }
 
-void DataTypeManagerImpl::OnModelAssociationDone(const ModelTypeSet& types) {
-  DCHECK(state_ == STOPPING || state_ == CONFIGURING);
-
-  if (state_ == STOPPING)
-    return;
-
-  // Ignore abort/unrecoverable error if we need to reconfigure anyways.
-  if (needs_reconfigure_) {
-    ProcessReconfigure();
-    return;
-  }
-
-  DCHECK(!association_types_queue_.empty());
-
-  // If this model association was for the full set of types, then this priority
-  // set is done. Otherwise it was just the ready types and the unready types
-  // still need to be associated.
-  ConfigureResult result(OK, types);
-  if (result.requested_types == association_types_queue_.front().types) {
-    association_types_queue_.pop();
-    if (!association_types_queue_.empty()) {
-      StartNextAssociation(READY_AT_CONFIG);
-    } else if (download_types_queue_.empty()) {
-      state_ = CONFIGURED;
-      NotifyDone(result);
-    }
-  } else {
-    DCHECK_EQ(association_types_queue_.front().ready_types,
-              result.requested_types);
-    // Will do nothing if the types are still downloading.
-    StartNextAssociation(UNREADY_AT_CONFIG);
-  }
-}
-
 void DataTypeManagerImpl::Stop(ShutdownReason reason) {
   if (state_ == STOPPED)
     return;

components/sync/driver/data_type_manager_impl.h

@@ -58,7 +58,6 @@ class DataTypeManagerImpl : public DataTypeManager,
   // |ModelAssociationManagerDelegate| implementation.
   void OnAllDataTypesReadyForConfigure() override;
   void OnSingleDataTypeAssociationDone(ModelType type) override;
-  void OnModelAssociationDone(const ModelTypeSet& types) override;
   void OnSingleDataTypeWillStop(ModelType type,
                                 const SyncError& error) override;
 

components/sync/driver/model_association_manager.cc

@@ -215,8 +215,6 @@ void ModelAssociationManager::Associate(
     }
   }
   DCHECK(associating_types.Empty());
-
-  delegate_->OnModelAssociationDone(types_to_associate);
 }
 
 void ModelAssociationManager::Stop(ShutdownReason shutdown_reason) {

components/sync/driver/model_association_manager.h

@@ -36,10 +36,6 @@ class ModelAssociationManagerDelegate {
   virtual void OnSingleDataTypeWillStop(ModelType type,
                                         const SyncError& error) = 0;
 
-  // Called when the ModelAssociationManager has tried to perform model
-  // association for all desired types and has nothing left to do.
-  virtual void OnModelAssociationDone(const ModelTypeSet& types) = 0;
-
   virtual ~ModelAssociationManagerDelegate() = default;
 };
 

components/sync/driver/model_association_manager_unittest.cc

@@ -43,10 +43,6 @@ class MockModelAssociationManagerDelegate
               OnSingleDataTypeWillStop,
               (ModelType, const SyncError& error),
               (override));
-  MOCK_METHOD(void,
-              OnModelAssociationDone,
-              (const ModelTypeSet& result),
-              (override));
 };
 
 class SyncModelAssociationManagerTest : public testing::Test {
@@ -76,7 +72,6 @@ TEST_F(SyncModelAssociationManagerTest, SimpleModelStart) {
   ModelAssociationManager model_association_manager(&controllers_, &delegate_);
   ModelTypeSet types(BOOKMARKS, APPS);
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
-  EXPECT_CALL(delegate_, OnModelAssociationDone(types));
 
   EXPECT_EQ(GetController(BOOKMARKS)->state(), DataTypeController::NOT_RUNNING);
   EXPECT_EQ(GetController(APPS)->state(), DataTypeController::NOT_RUNNING);
@@ -99,7 +94,6 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterFinish) {
   ModelAssociationManager model_association_manager(&controllers_, &delegate_);
   ModelTypeSet types;
   types.Put(BOOKMARKS);
-  EXPECT_CALL(delegate_, OnModelAssociationDone(types));
   EXPECT_CALL(delegate_, OnSingleDataTypeWillStop(BOOKMARKS, _));
 
   model_association_manager.Initialize(/*desired_types=*/types,
@@ -125,7 +119,6 @@ TEST_F(SyncModelAssociationManagerTest, ModelLoadFailBeforeAssociationStart) {
   ModelTypeSet types;
   types.Put(BOOKMARKS);
   EXPECT_CALL(delegate_, OnSingleDataTypeWillStop(BOOKMARKS, _));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(types));
 
   model_association_manager.Initialize(/*desired_types=*/types,
                                        /*preferred_types=*/types,
@@ -142,7 +135,6 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterConfiguration) {
   ModelAssociationManager model_association_manager(&controllers_, &delegate_);
   ModelTypeSet types;
   types.Put(BOOKMARKS);
-  EXPECT_CALL(delegate_, OnModelAssociationDone(types));
 
   model_association_manager.Initialize(/*desired_types=*/types,
                                        /*preferred_types=*/types,
@@ -364,7 +356,6 @@ TEST_F(SyncModelAssociationManagerTest, KeepsMetadataForPreferredDataType) {
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(BOOKMARKS));
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(APPS));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(desired_types));
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        BuildConfigureContext());
@@ -403,7 +394,6 @@ TEST_F(SyncModelAssociationManagerTest, ClearsMetadataForNotPreferredDataType) {
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(BOOKMARKS));
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(APPS));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(desired_types));
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        BuildConfigureContext());
@@ -449,7 +439,6 @@ TEST_F(SyncModelAssociationManagerTest,
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(BOOKMARKS));
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(APPS));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(desired_types));
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
@@ -470,7 +459,6 @@ TEST_F(SyncModelAssociationManagerTest,
   EXPECT_CALL(delegate_, OnSingleDataTypeWillStop(BOOKMARKS, _));
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(BOOKMARKS));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(desired_types));
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
@@ -503,7 +491,6 @@ TEST_F(SyncModelAssociationManagerTest,
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(BOOKMARKS));
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(APPS));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(desired_types));
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);
@@ -524,7 +511,6 @@ TEST_F(SyncModelAssociationManagerTest,
   EXPECT_CALL(delegate_, OnSingleDataTypeWillStop(BOOKMARKS, _));
   EXPECT_CALL(delegate_, OnAllDataTypesReadyForConfigure());
   EXPECT_CALL(delegate_, OnSingleDataTypeAssociationDone(BOOKMARKS));
-  EXPECT_CALL(delegate_, OnModelAssociationDone(desired_types));
 
   model_association_manager.Initialize(desired_types, preferred_types,
                                        configure_context);