Adds metadata_fate to DataTypeController::Stop

Now ModelTypeController:Stop uses preferences to determine if data type
will be disable completely.
This CL introduce adds metadata_fate parameter which will be used
as signal for clearing metadata.

This is second CL in sequence.

Bug: 823721
Change-Id: Idfe4a70864e2f05aba9fc1ede3b0ccc2a108287a
Reviewed-on: https://chromium-review.googlesource.com/1093318
Commit-Queue: Aleksei Loshkarev <lixan@yandex-team.ru>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567913}

components/search_engines/search_engine_data_type_controller_unittest.cc

@@ -136,7 +136,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, StartAssociationFailed) {
   Start();
   EXPECT_EQ(syncer::DataTypeController::FAILED, search_engine_dtc_.state());
   EXPECT_FALSE(syncable_service_.syncing());
-  search_engine_dtc_.Stop();
+  search_engine_dtc_.Stop(syncer::KEEP_METADATA);
   EXPECT_EQ(syncer::DataTypeController::NOT_RUNNING,
             search_engine_dtc_.state());
   EXPECT_FALSE(syncable_service_.syncing());
@@ -153,7 +153,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, Stop) {
   Start();
   EXPECT_EQ(syncer::DataTypeController::RUNNING, search_engine_dtc_.state());
   EXPECT_TRUE(syncable_service_.syncing());
-  search_engine_dtc_.Stop();
+  search_engine_dtc_.Stop(syncer::KEEP_METADATA);
   EXPECT_EQ(syncer::DataTypeController::NOT_RUNNING,
             search_engine_dtc_.state());
   // AsyncDirectoryTypeController::Stop posts call to StopLocalService to model
@@ -171,7 +171,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, StopBeforeLoaded) {
   EXPECT_EQ(syncer::DataTypeController::MODEL_STARTING,
             search_engine_dtc_.state());
   EXPECT_FALSE(syncable_service_.syncing());
-  search_engine_dtc_.Stop();
+  search_engine_dtc_.Stop(syncer::KEEP_METADATA);
   EXPECT_EQ(nullptr, search_engine_dtc_.GetSubscriptionForTesting());
   EXPECT_EQ(syncer::DataTypeController::NOT_RUNNING,
             search_engine_dtc_.state());

components/sync/base/sync_stop_metadata_fate.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SYNC_BASE_SYNC_STOP_METADATA_FATE_H_
+#define COMPONENTS_SYNC_BASE_SYNC_STOP_METADATA_FATE_H_
+
+namespace syncer {
+
+// Passed as an argument when stopping sync to control whether models should
+// clear its metadata (e.g. sync disabled vs browser shutdown).
+// TODO(mastiz): Unify with SyncStopDataFate.
+enum SyncStopMetadataFate { KEEP_METADATA, CLEAR_METADATA };
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_BASE_SYNC_STOP_METADATA_FATE_H_

components/sync/driver/async_directory_type_controller.cc

@@ -120,7 +120,8 @@ void AsyncDirectoryTypeController::StartAssociating(
   }
 }
 
-void AsyncDirectoryTypeController::Stop() {
+// For directory datatypes metadata clears by SyncManager::PurgeDisabledTypes().
+void AsyncDirectoryTypeController::Stop(SyncStopMetadataFate metadata_fate) {
   DCHECK(CalledOnValidThread());
 
   if (state() == NOT_RUNNING)

components/sync/driver/async_directory_type_controller.h

@@ -37,7 +37,7 @@ class AsyncDirectoryTypeController : public DirectoryDataTypeController {
   // DataTypeController interface.
   void LoadModels(const ModelLoadCallback& model_load_callback) override;
   void StartAssociating(const StartCallback& start_callback) override;
-  void Stop() override;
+  void Stop(SyncStopMetadataFate metadata_fate) override;
   ChangeProcessor* GetChangeProcessor() const override;
   State state() const override;
 

components/sync/driver/async_directory_type_controller_mock.h

@@ -21,7 +21,7 @@ class AsyncDirectoryTypeControllerMock : public AsyncDirectoryTypeController {
   // DataTypeController mocks.
   MOCK_METHOD1(StartAssociating, void(const StartCallback& start_callback));
   MOCK_METHOD1(LoadModels, void(const ModelLoadCallback& model_load_callback));
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(SyncStopMetadataFate));
   MOCK_CONST_METHOD0(type, ModelType());
   MOCK_CONST_METHOD0(name, std::string());
   MOCK_CONST_METHOD0(state, State());

components/sync/driver/async_directory_type_controller_unittest.cc

@@ -303,7 +303,7 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, AbortDuringStartModels) {
                                      base::Unretained(&model_load_callback_)));
   WaitForDTC();
   EXPECT_EQ(DataTypeController::MODEL_STARTING, non_ui_dtc_->state());
-  non_ui_dtc_->Stop();
+  non_ui_dtc_->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
 }
 
@@ -328,7 +328,7 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, StartAssociationFailed) {
   Start();
   WaitForDTC();
   EXPECT_EQ(DataTypeController::FAILED, non_ui_dtc_->state());
-  non_ui_dtc_->Stop();
+  non_ui_dtc_->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
 }
 
@@ -388,7 +388,7 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, AbortDuringAssociation) {
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
   Start();
   wait_for_db_thread_pause.Wait();
-  non_ui_dtc_->Stop();
+  non_ui_dtc_->Stop(KEEP_METADATA);
   WaitForDTC();
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
 }
@@ -404,7 +404,7 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, StartAfterSyncShutdown) {
   SetStopExpectations();
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
   Start();
-  non_ui_dtc_->Stop();
+  non_ui_dtc_->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
   Mock::VerifyAndClearExpectations(change_processor_.get());
   Mock::VerifyAndClearExpectations(dtc_mock_.get());
@@ -422,7 +422,7 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, Stop) {
   Start();
   WaitForDTC();
   EXPECT_EQ(DataTypeController::RUNNING, non_ui_dtc_->state());
-  non_ui_dtc_->Stop();
+  non_ui_dtc_->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state());
 }
 
@@ -441,7 +441,7 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, StopStart) {
   EXPECT_EQ(DataTypeController::RUNNING, non_ui_dtc_->state());
 
   non_ui_dtc_->BlockBackendTasks();
-  non_ui_dtc_->Stop();
+  non_ui_dtc_->Stop(KEEP_METADATA);
   SetStartExpectations();
   SetAssociateExpectations();
   SetActivateExpectations(DataTypeController::OK);

components/sync/driver/data_type_controller.h

@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
 #include "components/sync/base/model_type.h"
+#include "components/sync/base/sync_stop_metadata_fate.h"
 #include "components/sync/base/unrecoverable_error_handler.h"
 #include "components/sync/engine/cycle/status_counters.h"
 #include "components/sync/model/data_type_error_handler.h"
@@ -134,7 +135,9 @@ class DataTypeController : public base::SupportsWeakPtr<DataTypeController> {
   // logic shuts down gracefully by flushing remaining changes and calling
   // StopSyncing on the SyncableService. This assumes no changes will ever
   // propagate from sync again from point where Stop() is called.
-  virtual void Stop() = 0;
+  // KEEP_METADATA is used when the engine just stops sync, and CLEAR_METADATA
+  // is used when the user disables sync for data type.
+  virtual void Stop(SyncStopMetadataFate metadata_fate) = 0;
 
   // Name of this data type.  For logging purposes only.
   std::string name() const { return ModelTypeToString(type()); }

components/sync/driver/data_type_manager_impl.cc

@@ -297,7 +297,7 @@ void DataTypeManagerImpl::Restart(ConfigureReason reason) {
   // If we're performing a "catch up", first stop the model types to ensure the
   // call to Initialize triggers model association.
   if (catch_up_in_progress_)
-    model_association_manager_.Stop();
+    model_association_manager_.Stop(KEEP_METADATA);
   download_started_ = false;
   model_association_manager_.Initialize(last_enabled_types_);
 }
@@ -786,9 +786,21 @@ void DataTypeManagerImpl::StopImpl(ShutdownReason reason) {
   // Invalidate weak pointer to drop download callbacks.
   weak_ptr_factory_.InvalidateWeakPtrs();
 
+  // Leave metadata If we do not disable sync completely.
+  SyncStopMetadataFate metadata_fate = KEEP_METADATA;
+  switch (reason) {
+    case STOP_SYNC:
+      break;
+    case DISABLE_SYNC:
+      metadata_fate = CLEAR_METADATA;
+      break;
+    case BROWSER_SHUTDOWN:
+      break;
+  }
+
   // Stop all data types. This may trigger association callback but the
   // callback will do nothing because state is set to STOPPING above.
-  model_association_manager_.Stop();
+  model_association_manager_.Stop(metadata_fate);
 
   state_ = STOPPED;
 }

components/sync/driver/data_type_manager_impl_unittest.cc

@@ -370,6 +370,7 @@ TEST_F(SyncDataTypeManagerImplTest, ConfigureOne) {
   EXPECT_EQ(DataTypeManager::STOPPED, dtm_->state());
   EXPECT_TRUE(configurer_.activated_types().Empty());
   EXPECT_TRUE(configurer_.registered_directory_types().Empty());
+  EXPECT_EQ(0, GetController(BOOKMARKS)->clear_metadata_call_count());
 }
 
 // Set up a DTM with a single controller, configure it, but stop it
@@ -792,6 +793,8 @@ TEST_F(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
   EXPECT_EQ(DataTypeManager::STOPPED, dtm_->state());
   EXPECT_TRUE(configurer_.activated_types().Empty());
   EXPECT_TRUE(configurer_.registered_directory_types().Empty());
+  EXPECT_EQ(0, GetController(BOOKMARKS)->clear_metadata_call_count());
+  EXPECT_EQ(0, GetController(PREFERENCES)->clear_metadata_call_count());
 }
 
 // Set up a DTM with two controllers.  Then:
@@ -1734,4 +1737,22 @@ TEST_F(SyncDataTypeManagerImplTest, RegisterWithBackendAfterLoadModelsError) {
   EXPECT_EQ(1, GetController(PASSWORDS)->register_with_backend_call_count());
 }
 
+// Test that Stop with DISABLE_SYNC calls DTC Stop with CLEAR_METADATA for
+// active data types.
+TEST_F(SyncDataTypeManagerImplTest, StopWithDisableSync) {
+  // Initiate configuration for two datatypes but block them at LoadModels.
+  AddController(BOOKMARKS, true, true);
+  SetConfigureStartExpectation();
+  SetConfigureDoneExpectation(DataTypeManager::ABORTED, DataTypeStatusTable());
+
+  Configure(ModelTypeSet(BOOKMARKS));
+  EXPECT_EQ(DataTypeController::MODEL_STARTING,
+            GetController(BOOKMARKS)->state());
+
+  dtm_->Stop(DISABLE_SYNC);
+  EXPECT_EQ(DataTypeManager::STOPPED, dtm_->state());
+  EXPECT_TRUE(configurer_.activated_types().Empty());
+  EXPECT_EQ(1, GetController(BOOKMARKS)->clear_metadata_call_count());
+}
+
 }  // namespace syncer

components/sync/driver/fake_data_type_controller.cc

@@ -22,7 +22,8 @@ FakeDataTypeController::FakeDataTypeController(ModelType type)
       model_load_delayed_(false),
       ready_for_start_(true),
       should_load_model_before_configure_(false),
-      register_with_backend_call_count_(0) {}
+      register_with_backend_call_count_(0),
+      clear_metadata_call_count_(0) {}
 
 FakeDataTypeController::~FakeDataTypeController() {}
 
@@ -100,14 +101,18 @@ void FakeDataTypeController::FinishStart(ConfigureResult result) {
 }
 
 // * -> NOT_RUNNING
-void FakeDataTypeController::Stop() {
+void FakeDataTypeController::Stop(SyncStopMetadataFate metadata_fate) {
   DCHECK(CalledOnValidThread());
-  if (!model_load_callback_.is_null()) {
+  if (state() == MODEL_STARTING) {
     // Real data type controllers run the callback and specify "ABORTED" as an
     // error.  We should probably find a way to use the real code and mock out
     // unnecessary pieces.
     SimulateModelLoadFinishing();
   }
+
+  if (metadata_fate == CLEAR_METADATA)
+    ++clear_metadata_call_count_;
+
   state_ = NOT_RUNNING;
 }
 

components/sync/driver/fake_data_type_controller.h

@@ -33,7 +33,7 @@ class FakeDataTypeController : public DirectoryDataTypeController {
   void RegisterWithBackend(base::Callback<void(bool)> set_downloaded,
                            ModelTypeConfigurer* configurer) override;
   void StartAssociating(const StartCallback& start_callback) override;
-  void Stop() override;
+  void Stop(SyncStopMetadataFate metadata_fate) override;
   ChangeProcessor* GetChangeProcessor() const override;
   State state() const override;
   bool ReadyForStart() const override;
@@ -55,6 +55,8 @@ class FakeDataTypeController : public DirectoryDataTypeController {
     return register_with_backend_call_count_;
   }
 
+  int clear_metadata_call_count() const { return clear_metadata_call_count_; }
+
  private:
   DataTypeController::State state_;
   bool model_load_delayed_;
@@ -64,6 +66,7 @@ class FakeDataTypeController : public DirectoryDataTypeController {
   bool ready_for_start_;
   bool should_load_model_before_configure_;
   int register_with_backend_call_count_;
+  int clear_metadata_call_count_;
 };
 
 }  // namespace syncer

components/sync/driver/frontend_data_type_controller.cc

@@ -76,7 +76,8 @@ void FrontendDataTypeController::StartAssociating(
                             base::AsWeakPtr(this)));
 }
 
-void FrontendDataTypeController::Stop() {
+// For directory datatypes metadata clears by SyncManager::PurgeDisabledTypes().
+void FrontendDataTypeController::Stop(SyncStopMetadataFate metadata_fate) {
   DCHECK(CalledOnValidThread());
 
   if (state_ == NOT_RUNNING)

components/sync/driver/frontend_data_type_controller.h

@@ -45,7 +45,7 @@ class FrontendDataTypeController : public DirectoryDataTypeController {
   // DataTypeController interface.
   void LoadModels(const ModelLoadCallback& model_load_callback) override;
   void StartAssociating(const StartCallback& start_callback) override;
-  void Stop() override;
+  void Stop(SyncStopMetadataFate metadata_fate) override;
   State state() const override;
 
  protected:

components/sync/driver/frontend_data_type_controller_mock.h

@@ -25,7 +25,7 @@ class FrontendDataTypeControllerMock : public FrontendDataTypeController {
   MOCK_METHOD1(StartAssociating, void(const StartCallback& start_callback));
   MOCK_METHOD1(LoadModels, void(const ModelLoadCallback& model_load_callback));
 
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(SyncStopMetadataFate));
   MOCK_CONST_METHOD0(type, ModelType());
   MOCK_CONST_METHOD0(name, std::string());
   MOCK_CONST_METHOD0(state, State());

components/sync/driver/frontend_data_type_controller_unittest.cc

@@ -176,8 +176,9 @@ TEST_F(SyncFrontendDataTypeControllerTest, StartStopBeforeAssociation) {
   EXPECT_CALL(model_load_callback_, Run(_, _));
   EXPECT_EQ(DataTypeController::NOT_RUNNING, frontend_dtc_->state());
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&FrontendDataTypeController::Stop,
-                            base::AsWeakPtr(frontend_dtc_.get())));
+      FROM_HERE,
+      base::BindOnce(&FrontendDataTypeController::Stop,
+                     base::AsWeakPtr(frontend_dtc_.get()), KEEP_METADATA));
   Start();
   EXPECT_EQ(DataTypeController::NOT_RUNNING, frontend_dtc_->state());
 }
@@ -189,7 +190,7 @@ TEST_F(SyncFrontendDataTypeControllerTest, AbortDuringStartModels) {
   frontend_dtc_->LoadModels(base::Bind(
       &ModelLoadCallbackMock::Run, base::Unretained(&model_load_callback_)));
   EXPECT_EQ(DataTypeController::MODEL_STARTING, frontend_dtc_->state());
-  frontend_dtc_->Stop();
+  frontend_dtc_->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, frontend_dtc_->state());
 }
 
@@ -244,7 +245,7 @@ TEST_F(SyncFrontendDataTypeControllerTest, Stop) {
   EXPECT_EQ(DataTypeController::NOT_RUNNING, frontend_dtc_->state());
   Start();
   EXPECT_EQ(DataTypeController::RUNNING, frontend_dtc_->state());
-  frontend_dtc_->Stop();
+  frontend_dtc_->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, frontend_dtc_->state());
 }
 

components/sync/driver/model_association_manager.cc

@@ -127,6 +127,7 @@ void ModelAssociationManager::Initialize(ModelTypeSet desired_types) {
 }
 
 void ModelAssociationManager::StopDatatype(const SyncError& error,
+                                           SyncStopMetadataFate metadata_fate,
                                            DataTypeController* dtc) {
   loaded_types_.Remove(dtc->type());
   associated_types_.Remove(dtc->type());
@@ -135,7 +136,7 @@ void ModelAssociationManager::StopDatatype(const SyncError& error,
   if (error.IsSet() || dtc->state() != DataTypeController::NOT_RUNNING) {
     // If an error was set, the delegate must be informed of the error.
     delegate_->OnSingleDataTypeWillStop(dtc->type(), error);
-    dtc->Stop();
+    dtc->Stop(metadata_fate);
   }
 }
 
@@ -147,7 +148,7 @@ void ModelAssociationManager::StopDisabledTypes() {
     if (dtc->state() != DataTypeController::NOT_RUNNING &&
         !desired_types_.Has(dtc->type())) {
       DVLOG(1) << "ModelAssociationManager: stop " << dtc->name();
-      StopDatatype(SyncError(), dtc);
+      StopDatatype(SyncError(), KEEP_METADATA, dtc);
     }
   }
 }
@@ -231,7 +232,7 @@ void ModelAssociationManager::StartAssociationAsync(
   }
 }
 
-void ModelAssociationManager::Stop() {
+void ModelAssociationManager::Stop(SyncStopMetadataFate metadata_fate) {
   // Ignore callbacks from controllers.
   weak_ptr_factory_.InvalidateWeakPtrs();
 
@@ -240,7 +241,7 @@ void ModelAssociationManager::Stop() {
        it != controllers_->end(); ++it) {
     DataTypeController* dtc = (*it).second.get();
     if (dtc->state() != DataTypeController::NOT_RUNNING) {
-      StopDatatype(SyncError(), dtc);
+      StopDatatype(SyncError(), metadata_fate, dtc);
       DVLOG(1) << "ModelAssociationManager: Stopped " << dtc->name();
     }
   }
@@ -310,7 +311,7 @@ void ModelAssociationManager::TypeStartCallback(
     DVLOG(1) << "ModelAssociationManager: Type encountered an error.";
     desired_types_.Remove(type);
     DataTypeController* dtc = controllers_->find(type)->second.get();
-    StopDatatype(local_merge_result.error(), dtc);
+    StopDatatype(local_merge_result.error(), KEEP_METADATA, dtc);
     NotifyDelegateIfReadyForConfigure();
 
     // Update configuration result.
@@ -384,7 +385,7 @@ void ModelAssociationManager::ModelAssociationDone(State new_state) {
                                 static_cast<int>(MODEL_TYPE_COUNT));
       StopDatatype(SyncError(FROM_HERE, SyncError::DATATYPE_ERROR,
                              "Association timed out.", dtc->type()),
-                   dtc);
+                   KEEP_METADATA, dtc);
     }
   }
 

components/sync/driver/model_association_manager.h

@@ -11,6 +11,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 
+#include "components/sync/base/sync_stop_metadata_fate.h"
 #include "components/sync/base/weak_handle.h"
 #include "components/sync/driver/data_type_manager.h"
 #include "components/sync/engine/data_type_association_stats.h"
@@ -87,7 +88,9 @@ class ModelAssociationManager {
   void Initialize(ModelTypeSet desired_types);
 
   // Can be called at any time. Synchronously stops all datatypes.
-  void Stop();
+  // If |metadata_fate| equals  CLEAR_METADATA controllers should clear sync
+  // metadata.
+  void Stop(SyncStopMetadataFate metadata_fate);
 
   // Should only be called after Initialize to start the actual association.
   // |types_to_associate| should be subset of |desired_types| in Initialize().
@@ -131,7 +134,9 @@ class ModelAssociationManager {
   void ModelAssociationDone(State new_state);
 
   // A helper to stop an individual datatype.
-  void StopDatatype(const SyncError& error, DataTypeController* dtc);
+  void StopDatatype(const SyncError& error,
+                    SyncStopMetadataFate metadata_fate,
+                    DataTypeController* dtc);
 
   // Calls delegate's OnAllDataTypesReadyForConfigure when all datatypes from
   // desired_types_ are ready for configure. Ensures that for every call to

components/sync/driver/model_association_manager_unittest.cc

@@ -115,9 +115,11 @@ TEST_F(SyncModelAssociationManagerTest, StopModelBeforeFinish) {
 
   EXPECT_EQ(GetController(controllers_, BOOKMARKS)->state(),
             DataTypeController::ASSOCIATING);
-  model_association_manager.Stop();
+  model_association_manager.Stop(KEEP_METADATA);
   EXPECT_EQ(GetController(controllers_, BOOKMARKS)->state(),
             DataTypeController::NOT_RUNNING);
+  EXPECT_EQ(
+      0, GetController(controllers_, BOOKMARKS)->clear_metadata_call_count());
 }
 
 // Start a type, let it finish and then call stop.
@@ -139,9 +141,11 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterFinish) {
             DataTypeController::ASSOCIATING);
   GetController(controllers_, BOOKMARKS)->FinishStart(DataTypeController::OK);
 
-  model_association_manager.Stop();
+  model_association_manager.Stop(KEEP_METADATA);
   EXPECT_EQ(GetController(controllers_, BOOKMARKS)->state(),
             DataTypeController::NOT_RUNNING);
+  EXPECT_EQ(
+      0, GetController(controllers_, BOOKMARKS)->clear_metadata_call_count());
 }
 
 // Make a type fail model association and verify correctness.
@@ -514,4 +518,28 @@ TEST_F(SyncModelAssociationManagerTest, TypeRegistrationCallSequence) {
             GetController(controllers_, APPS)->state());
 }
 
+// Test that Stop clears metadata for disabled type.
+TEST_F(SyncModelAssociationManagerTest, StopClearMetadata) {
+  controllers_[BOOKMARKS] = std::make_unique<FakeDataTypeController>(BOOKMARKS);
+  ModelAssociationManager model_association_manager(&controllers_, &delegate_);
+
+  ASSERT_EQ(GetController(controllers_, BOOKMARKS)->state(),
+            DataTypeController::NOT_RUNNING);
+
+  ModelTypeSet desired_types(BOOKMARKS);
+
+  // Initialize() kicks off model loading.
+  model_association_manager.Initialize(desired_types);
+
+  ASSERT_EQ(GetController(controllers_, BOOKMARKS)->state(),
+            DataTypeController::MODEL_LOADED);
+
+  model_association_manager.Stop(CLEAR_METADATA);
+
+  EXPECT_EQ(GetController(controllers_, BOOKMARKS)->state(),
+            DataTypeController::NOT_RUNNING);
+  EXPECT_EQ(
+      1, GetController(controllers_, BOOKMARKS)->clear_metadata_call_count());
+}
+
 }  // namespace syncer

components/sync/driver/model_type_controller.cc

@@ -231,7 +231,7 @@ void ModelTypeController::DeactivateDataType(ModelTypeConfigurer* configurer) {
   }
 }
 
-void ModelTypeController::Stop() {
+void ModelTypeController::Stop(SyncStopMetadataFate metadata_fate) {
   DCHECK(CalledOnValidThread());
 
   if (state() == NOT_RUNNING)

components/sync/driver/model_type_controller.h

@@ -47,7 +47,7 @@ class ModelTypeController : public DataTypeController {
   void StartAssociating(const StartCallback& start_callback) override;
   void ActivateDataType(ModelTypeConfigurer* configurer) override;
   void DeactivateDataType(ModelTypeConfigurer* configurer) override;
-  void Stop() override;
+  void Stop(SyncStopMetadataFate metadata_fate) override;
   State state() const override;
   void GetAllNodes(const AllNodesCallback& callback) override;
   void GetStatusCounters(const StatusCountersCallback& callback) override;

components/sync/driver/model_type_controller_unittest.cc

@@ -216,7 +216,7 @@ class ModelTypeControllerTest : public testing::Test {
 
   void DeactivateDataTypeAndStop() {
     controller_->DeactivateDataType(&configurer_);
-    controller_->Stop();
+    controller_->Stop(KEEP_METADATA);
   }
 
   // These threads can ping-pong for a bit so we run the model thread twice.
@@ -450,7 +450,7 @@ TEST_F(ModelTypeControllerTest, StopBeforeLoadModels) {
 
   // Clearing preferences emulates signing out.
   sync_prefs()->ClearPreferences();
-  controller()->Stop();
+  controller()->Stop(KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, controller()->state());
   // Ensure that DisableSync is not called.
   EXPECT_EQ(0, disable_sync_call_count());

components/sync/driver/proxy_data_type_controller.cc

@@ -53,7 +53,7 @@ void ProxyDataTypeController::StartAssociating(
                      syncer_merge_result);
 }
 
-void ProxyDataTypeController::Stop() {
+void ProxyDataTypeController::Stop(SyncStopMetadataFate metadata_fate) {
   state_ = NOT_RUNNING;
 }
 

components/sync/driver/proxy_data_type_controller.h

@@ -28,7 +28,7 @@ class ProxyDataTypeController : public DataTypeController {
   void RegisterWithBackend(base::Callback<void(bool)> set_downloaded,
                            ModelTypeConfigurer* configurer) override;
   void StartAssociating(const StartCallback& start_callback) override;
-  void Stop() override;
+  void Stop(SyncStopMetadataFate metadata_fate) override;
   State state() const override;
   void ActivateDataType(ModelTypeConfigurer* configurer) override;
   void DeactivateDataType(ModelTypeConfigurer* configurer) override;

components/sync_bookmarks/bookmark_data_type_controller_unittest.cc

@@ -292,7 +292,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, StartAborted) {
       base::Bind(&ModelLoadCallbackMock::Run,
                  base::Unretained(&model_load_callback_)));
 
-  bookmark_dtc_->Stop();
+  bookmark_dtc_->Stop(syncer::KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
 }
 
@@ -307,6 +307,6 @@ TEST_F(SyncBookmarkDataTypeControllerTest, Stop) {
   EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _));
   Start();
   EXPECT_EQ(DataTypeController::RUNNING, bookmark_dtc_->state());
-  bookmark_dtc_->Stop();
+  bookmark_dtc_->Stop(syncer::KEEP_METADATA);
   EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state());
 }