Delete BookmarkDataTypeController

This was part of the old bookmarks sync implementation and not used
anymore.

BookmarkDataTypeController was the only implementation of
FrontendDataTypeController, so that can go too.
SyncApiComponentsFactory::CreateBookmarkSyncComponents was also
unused and is removed.

Bug: 933756
Change-Id: I2c678e11a2030eb7aa37b52fed5ae6a363baad49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864684
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707776}

components/browser_sync/profile_sync_components_factory_impl.cc

@@ -38,8 +38,6 @@
 #include "components/sync/model/model_type_store_service.h"
 #include "components/sync/model_impl/forwarding_model_type_controller_delegate.h"
 #include "components/sync/model_impl/proxy_model_type_controller_delegate.h"
-#include "components/sync_bookmarks/bookmark_change_processor.h"
-#include "components/sync_bookmarks/bookmark_model_associator.h"
 #include "components/sync_bookmarks/bookmark_sync_service.h"
 #include "components/sync_device_info/device_info_sync_service.h"
 #include "components/sync_sessions/proxy_tabs_data_type_controller.h"
@@ -51,10 +49,6 @@
 #include "chromeos/constants/chromeos_features.h"
 #endif
 
-using base::FeatureList;
-using bookmarks::BookmarkModel;
-using sync_bookmarks::BookmarkChangeProcessor;
-using sync_bookmarks::BookmarkModelAssociator;
 using syncer::DataTypeController;
 using syncer::DataTypeManager;
 using syncer::DataTypeManagerImpl;
@@ -421,30 +415,6 @@ ProfileSyncComponentsFactoryImpl::CreateSyncEngine(
       sync_client_->GetModelTypeStoreService()->GetSyncDataPath());
 }
 
-syncer::SyncApiComponentFactory::SyncComponents
-ProfileSyncComponentsFactoryImpl::CreateBookmarkSyncComponents(
-    std::unique_ptr<syncer::DataTypeErrorHandler> error_handler,
-    syncer::UserShare* user_share) {
-  BookmarkModel* bookmark_model = sync_client_->GetBookmarkModel();
-// TODO(akalin): We may want to propagate this switch up eventually.
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  const bool kExpectMobileBookmarksFolder = true;
-#else
-  const bool kExpectMobileBookmarksFolder = false;
-#endif
-
-  auto model_associator = std::make_unique<BookmarkModelAssociator>(
-      bookmark_model, sync_client_->GetBookmarkUndoService(),
-      sync_client_->GetFaviconService(), user_share, error_handler->Copy(),
-      kExpectMobileBookmarksFolder);
-
-  SyncComponents components;
-  components.change_processor = std::make_unique<BookmarkChangeProcessor>(
-      model_associator.get(), std::move(error_handler));
-  components.model_associator = std::move(model_associator);
-  return components;
-}
-
 std::unique_ptr<syncer::ModelTypeControllerDelegate>
 ProfileSyncComponentsFactoryImpl::CreateForwardingControllerDelegate(
     syncer::ModelType type) {

components/browser_sync/profile_sync_components_factory_impl.h

@@ -78,9 +78,6 @@ class ProfileSyncComponentsFactoryImpl
       const std::string& name,
       invalidation::InvalidationService* invalidator,
       const base::WeakPtr<syncer::SyncPrefs>& sync_prefs) override;
-  syncer::SyncApiComponentFactory::SyncComponents CreateBookmarkSyncComponents(
-      std::unique_ptr<syncer::DataTypeErrorHandler> error_handler,
-      syncer::UserShare* user_share) override;
 
  private:
   // Factory function for ModelTypeController instances for models living on

components/sync/BUILD.gn

@@ -584,7 +584,6 @@ source_set("unit_tests") {
     "driver/async_directory_type_controller_unittest.cc",
     "driver/backend_migrator_unittest.cc",
     "driver/data_type_manager_impl_unittest.cc",
-    "driver/frontend_data_type_controller_unittest.cc",
     "driver/generic_change_processor_unittest.cc",
     "driver/glue/sync_engine_impl_unittest.cc",
     "driver/model_association_manager_unittest.cc",

components/sync/driver/BUILD.gn

@@ -33,8 +33,6 @@ jumbo_static_library("driver") {
     "data_type_status_table.h",
     "directory_data_type_controller.cc",
     "directory_data_type_controller.h",
-    "frontend_data_type_controller.cc",
-    "frontend_data_type_controller.h",
     "generic_change_processor.cc",
     "generic_change_processor.h",
     "generic_change_processor_factory.cc",
@@ -58,7 +56,6 @@ jumbo_static_library("driver") {
     "shared_change_processor_ref.h",
     "startup_controller.cc",
     "startup_controller.h",
-    "sync_api_component_factory.cc",
     "sync_api_component_factory.h",
     "sync_auth_manager.cc",
     "sync_auth_manager.h",
@@ -171,8 +168,6 @@ static_library("test_support") {
     "fake_generic_change_processor.h",
     "fake_sync_service.cc",
     "fake_sync_service.h",
-    "frontend_data_type_controller_mock.cc",
-    "frontend_data_type_controller_mock.h",
     "mock_sync_service.cc",
     "mock_sync_service.h",
     "model_associator_mock.cc",

components/sync/driver/frontend_data_type_controller.cc

-// Copyright (c) 2012 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.
-
-#include "components/sync/driver/frontend_data_type_controller.h"
-
-#include <utility>
-
-#include "base/bind.h"
-#include "base/logging.h"
-#include "base/threading/sequenced_task_runner_handle.h"
-#include "components/sync/base/data_type_histogram.h"
-#include "components/sync/base/model_type.h"
-#include "components/sync/driver/configure_context.h"
-#include "components/sync/driver/model_associator.h"
-#include "components/sync/model/change_processor.h"
-#include "components/sync/model/data_type_error_handler_impl.h"
-#include "components/sync/model/sync_error.h"
-#include "components/sync/model/sync_merge_result.h"
-
-namespace syncer {
-
-FrontendDataTypeController::FrontendDataTypeController(
-    ModelType type,
-    const base::Closure& dump_stack,
-    SyncService* sync_service)
-    : DirectoryDataTypeController(type, dump_stack, sync_service, GROUP_UI),
-      state_(NOT_RUNNING) {}
-
-void FrontendDataTypeController::LoadModels(
-    const ConfigureContext& configure_context,
-    const ModelLoadCallback& model_load_callback) {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(configure_context.sync_mode, SyncMode::kFull);
-
-  model_load_callback_ = model_load_callback;
-
-  if (state_ != NOT_RUNNING) {
-    model_load_callback_.Run(type(),
-                             SyncError(FROM_HERE, SyncError::DATATYPE_ERROR,
-                                       "Model already running", type()));
-    return;
-  }
-
-  state_ = MODEL_STARTING;
-  if (!StartModels()) {
-    // If we are waiting for some external service to load before associating
-    // or we failed to start the models, we exit early. state_ will control
-    // what we perform next.
-    DCHECK(state_ == NOT_RUNNING || state_ == MODEL_STARTING);
-    return;
-  }
-
-  OnModelLoaded();
-}
-
-void FrontendDataTypeController::OnModelLoaded() {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(state_, MODEL_STARTING);
-
-  state_ = MODEL_LOADED;
-  model_load_callback_.Run(type(), SyncError());
-}
-
-void FrontendDataTypeController::StartAssociating(
-    StartCallback start_callback) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(start_callback);
-  DCHECK_EQ(state_, MODEL_LOADED);
-
-  start_callback_ = std::move(start_callback);
-  state_ = ASSOCIATING;
-
-  base::SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(&FrontendDataTypeController::Associate,
-                                base::AsWeakPtr(this)));
-}
-
-void FrontendDataTypeController::Stop(ShutdownReason shutdown_reason) {
-  DCHECK(CalledOnValidThread());
-
-  if (state_ == NOT_RUNNING)
-    return;
-
-  State prev_state = state_;
-  state_ = STOPPING;
-
-  // If Stop() is called while Start() is waiting for the datatype model to
-  // load, abort the start.
-  if (prev_state == MODEL_STARTING) {
-    AbortModelLoad();
-    // We can just return here since we haven't performed association if we're
-    // still in MODEL_STARTING.
-    return;
-  }
-
-  CleanUpState();
-
-  if (model_associator()) {
-    SyncError error;  // Not used.
-    error = model_associator()->DisassociateModels();
-  }
-
-  set_model_associator(nullptr);
-  change_processor_.reset();
-
-  state_ = NOT_RUNNING;
-}
-
-DataTypeController::State FrontendDataTypeController::state() const {
-  return state_;
-}
-
-FrontendDataTypeController::FrontendDataTypeController()
-    : DirectoryDataTypeController(UNSPECIFIED,
-                                  base::Closure(),
-                                  nullptr,
-                                  GROUP_UI),
-      state_(NOT_RUNNING) {}
-
-FrontendDataTypeController::~FrontendDataTypeController() {}
-
-bool FrontendDataTypeController::StartModels() {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(state_, MODEL_STARTING);
-  // By default, no additional services need to be started before we can proceed
-  // with model association.
-  return true;
-}
-
-void FrontendDataTypeController::Associate() {
-  DCHECK(CalledOnValidThread());
-  if (state_ != ASSOCIATING) {
-    // Stop() must have been called while Associate() task have been waiting.
-    DCHECK_EQ(state_, NOT_RUNNING);
-    return;
-  }
-
-  SyncMergeResult local_merge_result(type());
-  SyncMergeResult syncer_merge_result(type());
-  CreateSyncComponents();
-  if (!model_associator()->CryptoReadyIfNecessary()) {
-    StartDone(NEEDS_CRYPTO, local_merge_result, syncer_merge_result);
-    return;
-  }
-
-  bool sync_has_nodes = false;
-  if (!model_associator()->SyncModelHasUserCreatedNodes(&sync_has_nodes)) {
-    SyncError error(FROM_HERE, SyncError::UNRECOVERABLE_ERROR,
-                    "Failed to load sync nodes", type());
-    local_merge_result.set_error(error);
-    StartDone(UNRECOVERABLE_ERROR, local_merge_result, syncer_merge_result);
-    return;
-  }
-
-  // TODO(zea): Have AssociateModels fill the local and syncer merge results.
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  SyncError error;
-  error = model_associator()->AssociateModels(&local_merge_result,
-                                              &syncer_merge_result);
-  // TODO(lipalani): crbug.com/122690 - handle abort.
-  RecordAssociationTime(base::TimeTicks::Now() - start_time);
-  if (error.IsSet()) {
-    local_merge_result.set_error(error);
-    StartDone(ASSOCIATION_FAILED, local_merge_result, syncer_merge_result);
-    return;
-  }
-
-  state_ = RUNNING;
-  // FinishStart() invokes the DataTypeManager callback, which can lead to a
-  // call to Stop() if one of the other data types being started generates an
-  // error.
-  StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, local_merge_result,
-            syncer_merge_result);
-}
-
-void FrontendDataTypeController::CleanUpState() {
-  // Do nothing by default.
-}
-
-void FrontendDataTypeController::CleanUp() {
-  CleanUpState();
-  set_model_associator(nullptr);
-  change_processor_.reset();
-}
-
-void FrontendDataTypeController::AbortModelLoad() {
-  DCHECK(CalledOnValidThread());
-  CleanUp();
-  state_ = NOT_RUNNING;
-}
-
-void FrontendDataTypeController::StartDone(
-    ConfigureResult start_result,
-    const SyncMergeResult& local_merge_result,
-    const SyncMergeResult& syncer_merge_result) {
-  DCHECK(CalledOnValidThread());
-  if (!IsSuccessfulResult(start_result)) {
-    CleanUp();
-    if (start_result == ASSOCIATION_FAILED) {
-      state_ = FAILED;
-    } else {
-      state_ = NOT_RUNNING;
-    }
-    RecordStartFailure(start_result);
-  }
-
-  std::move(start_callback_)
-      .Run(start_result, local_merge_result, syncer_merge_result);
-}
-
-std::unique_ptr<DataTypeErrorHandler>
-FrontendDataTypeController::CreateErrorHandler() {
-  return std::make_unique<DataTypeErrorHandlerImpl>(
-      base::SequencedTaskRunnerHandle::Get(), dump_stack_,
-      base::Bind(&FrontendDataTypeController::OnUnrecoverableError,
-                 base::AsWeakPtr(this)));
-}
-
-void FrontendDataTypeController::OnUnrecoverableError(const SyncError& error) {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type(), error.model_type());
-  if (model_load_callback_) {
-    model_load_callback_.Run(type(), error);
-  }
-}
-
-void FrontendDataTypeController::RecordAssociationTime(base::TimeDelta time) {
-  DCHECK(CalledOnValidThread());
-#define PER_DATA_TYPE_MACRO(type_str) \
-  UMA_HISTOGRAM_TIMES("Sync." type_str "AssociationTime", time);
-  SYNC_DATA_TYPE_HISTOGRAM(type());
-#undef PER_DATA_TYPE_MACRO
-}
-
-void FrontendDataTypeController::RecordStartFailure(ConfigureResult result) {
-  DCHECK(CalledOnValidThread());
-  UMA_HISTOGRAM_ENUMERATION("Sync.DataTypeStartFailures2",
-                            ModelTypeHistogramValue(type()));
-#define PER_DATA_TYPE_MACRO(type_str)                                    \
-  UMA_HISTOGRAM_ENUMERATION("Sync." type_str "ConfigureFailure", result, \
-                            MAX_CONFIGURE_RESULT);
-  SYNC_DATA_TYPE_HISTOGRAM(type());
-#undef PER_DATA_TYPE_MACRO
-}
-
-AssociatorInterface* FrontendDataTypeController::model_associator() const {
-  return model_associator_.get();
-}
-
-void FrontendDataTypeController::set_model_associator(
-    std::unique_ptr<AssociatorInterface> model_associator) {
-  model_associator_ = std::move(model_associator);
-}
-
-ChangeProcessor* FrontendDataTypeController::GetChangeProcessor() const {
-  return change_processor_.get();
-}
-
-void FrontendDataTypeController::set_change_processor(
-    std::unique_ptr<ChangeProcessor> change_processor) {
-  change_processor_ = std::move(change_processor);
-}
-
-}  // namespace syncer

components/sync/driver/frontend_data_type_controller.h

-// Copyright (c) 2012 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_DRIVER_FRONTEND_DATA_TYPE_CONTROLLER_H__
-#define COMPONENTS_SYNC_DRIVER_FRONTEND_DATA_TYPE_CONTROLLER_H__
-
-#include <memory>
-#include <string>
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
-#include "components/sync/driver/directory_data_type_controller.h"
-#include "components/sync/model/data_type_error_handler.h"
-
-namespace base {
-class TimeDelta;
-}  // namespace base
-
-namespace syncer {
-
-class AssociatorInterface;
-class ChangeProcessor;
-class SyncError;
-class SyncService;
-
-// Implementation for datatypes that reside on the frontend thread
-// (UI thread). This is the same thread we perform initialization on, so we
-// don't have to worry about thread safety. The main start/stop funtionality is
-// implemented by default.
-// Derived classes must implement (at least):
-//    ModelType type() const
-//    void CreateSyncComponents();
-// NOTE: This class is deprecated! New sync datatypes should be using the
-// SyncableService API and the UIDataTypeController instead.
-// TODO(zea): Delete this once all types are on the new API.
-class FrontendDataTypeController : public DirectoryDataTypeController {
- public:
-  // |dump_stack| is called when an unrecoverable error occurs.
-  FrontendDataTypeController(ModelType type,
-                             const base::Closure& dump_stack,
-                             SyncService* sync_service);
-  ~FrontendDataTypeController() override;
-
-  // DataTypeController interface.
-  void LoadModels(const ConfigureContext& configure_context,
-                  const ModelLoadCallback& model_load_callback) override;
-  void StartAssociating(StartCallback start_callback) override;
-  void Stop(ShutdownReason shutdown_reason) override;
-  State state() const override;
-
- protected:
-  friend class FrontendDataTypeControllerMock;
-
-  // For testing only.
-  FrontendDataTypeController();
-
-  // Kick off any dependent services that need to be running before we can
-  // 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. Refer to Start(_) implementation.
-  virtual bool StartModels();
-
-  // Datatype specific creation of sync components.
-  virtual void CreateSyncComponents() = 0;
-
-  // Perform any DataType controller specific state cleanup before stopping
-  // the datatype controller. The default implementation is a no-op.
-  virtual void CleanUpState();
-
-  // Helper method for cleaning up state and running the start callback.
-  virtual void StartDone(ConfigureResult start_result,
-                         const SyncMergeResult& local_merge_result,
-                         const SyncMergeResult& syncer_merge_result);
-
-  // Record association time.
-  virtual void RecordAssociationTime(base::TimeDelta time);
-  // Record causes of start failure.
-  virtual void RecordStartFailure(ConfigureResult result);
-
-  virtual AssociatorInterface* model_associator() const;
-  virtual void set_model_associator(
-      std::unique_ptr<AssociatorInterface> associator);
-  ChangeProcessor* GetChangeProcessor() const override;
-  virtual void set_change_processor(std::unique_ptr<ChangeProcessor> processor);
-
-  // If the DTC is waiting for models to load, once the models are
-  // loaded the datatype service will call this function on DTC to let
-  // us know that it is safe to start associating.
-  void OnModelLoaded();
-
-  std::unique_ptr<DataTypeErrorHandler> CreateErrorHandler() override;
-
-  State state_;
-
-  StartCallback start_callback_;
-  ModelLoadCallback model_load_callback_;
-
-  // TODO(sync): transition all datatypes to SyncableService and deprecate
-  // AssociatorInterface.
-  std::unique_ptr<AssociatorInterface> model_associator_;
-  std::unique_ptr<ChangeProcessor> change_processor_;
-
- private:
-  // Build sync components and associate models.
-  virtual void Associate();
-
-  void AbortModelLoad();
-
-  // Clean up our state and state variables. Called in response
-  // to a failure or abort or stop.
-  void CleanUp();
-
-  // Handle an unrecoverable error.
-  void OnUnrecoverableError(const SyncError& error);
-
-  DISALLOW_COPY_AND_ASSIGN(FrontendDataTypeController);
-};
-
-}  // namespace syncer
-
-#endif  // COMPONENTS_SYNC_DRIVER_FRONTEND_DATA_TYPE_CONTROLLER_H__

components/sync/driver/frontend_data_type_controller_mock.cc

-// Copyright (c) 2011 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.
-
-#include "components/sync/driver/frontend_data_type_controller_mock.h"
-
-#include "components/sync/driver/configure_context.h"
-
-namespace syncer {
-
-FrontendDataTypeControllerMock::FrontendDataTypeControllerMock() {}
-
-FrontendDataTypeControllerMock::~FrontendDataTypeControllerMock() {}
-
-}  // namespace syncer

components/sync/driver/frontend_data_type_controller_mock.h

-// Copyright (c) 2012 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_DRIVER_FRONTEND_DATA_TYPE_CONTROLLER_MOCK_H__
-#define COMPONENTS_SYNC_DRIVER_FRONTEND_DATA_TYPE_CONTROLLER_MOCK_H__
-
-#include <memory>
-#include <string>
-
-#include "components/sync/driver/frontend_data_type_controller.h"
-#include "components/sync/driver/model_associator.h"
-#include "components/sync/model/change_processor.h"
-#include "components/sync/model/sync_error.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace syncer {
-
-class FrontendDataTypeControllerMock : public FrontendDataTypeController {
- public:
-  FrontendDataTypeControllerMock();
-  ~FrontendDataTypeControllerMock() override;
-
-  // DataTypeController mocks.
-  MOCK_METHOD1(StartAssociating, void(StartCallback start_callback));
-  MOCK_METHOD2(LoadModels,
-               void(const ConfigureContext& configure_context,
-                    const ModelLoadCallback& model_load_callback));
-  MOCK_METHOD1(Stop, void(ShutdownReason));
-  MOCK_CONST_METHOD0(type, ModelType());
-  MOCK_CONST_METHOD0(name, std::string());
-  MOCK_CONST_METHOD0(state, State());
-
-  // FrontendDataTypeController mocks.
-  MOCK_METHOD0(StartModels, bool());
-  MOCK_METHOD0(Associate, void());
-  MOCK_METHOD0(CreateSyncComponents, void());
-  MOCK_METHOD0(CleanUpState, void());
-  MOCK_CONST_METHOD0(model_associator, AssociatorInterface*());
-  MOCK_METHOD1(set_model_associator,
-               void(std::unique_ptr<AssociatorInterface> associator));
-  MOCK_CONST_METHOD0(change_processor, ChangeProcessor*());
-  MOCK_METHOD1(set_change_processor,
-               void(std::unique_ptr<ChangeProcessor> processor));
-  MOCK_METHOD1(RecordAssociationTime, void(base::TimeDelta time));
-  MOCK_METHOD1(RecordStartFailure, void(ConfigureResult result));
-};
-
-}  // namespace syncer
-
-#endif  // COMPONENTS_SYNC_DRIVER_FRONTEND_DATA_TYPE_CONTROLLER_MOCK_H__

components/sync/driver/sync_api_component_factory.cc

-// 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.
-
-#include "components/sync/driver/sync_api_component_factory.h"
-
-#include "components/sync/driver/model_associator.h"
-#include "components/sync/model/change_processor.h"
-
-namespace syncer {
-
-SyncApiComponentFactory::SyncComponents::SyncComponents() = default;
-
-SyncApiComponentFactory::SyncComponents::SyncComponents(SyncComponents&&) =
-    default;
-
-SyncApiComponentFactory::SyncComponents::~SyncComponents() = default;
-
-}  // namespace syncer

components/sync/driver/sync_api_component_factory.h

@@ -10,9 +10,8 @@
 
 #include "base/memory/weak_ptr.h"
 #include "components/sync/base/model_type.h"
+#include "components/sync/base/weak_handle.h"
 #include "components/sync/driver/data_type_controller.h"
-#include "components/sync/model/data_type_error_handler.h"
-#include "components/sync/model/syncable_service.h"
 
 namespace invalidation {
 class InvalidationService;
@@ -20,15 +19,12 @@ class InvalidationService;
 
 namespace syncer {
 
-class AssociatorInterface;
-class ChangeProcessor;
 class DataTypeDebugInfoListener;
 class DataTypeEncryptionHandler;
 class DataTypeManager;
 class DataTypeManagerObserver;
 class SyncEngine;
 class SyncPrefs;
-struct UserShare;
 
 // This factory provides sync driver code with the model type specific sync/api
 // service (like SyncableService) implementations.
@@ -36,25 +32,6 @@ class SyncApiComponentFactory {
  public:
   virtual ~SyncApiComponentFactory() {}
 
-  // The various factory methods for the data type model associators
-  // and change processors all return this struct.  This is needed
-  // because the change processors typically require a type-specific
-  // model associator at construction time.
-  //
-  // Note: This interface is deprecated in favor of the SyncableService API.
-  // New datatypes that do not live on the UI thread should directly return a
-  // weak pointer to a SyncableService. All others continue to return
-  // SyncComponents. It is safe to assume that the factory methods below are
-  // called on the same thread in which the datatype resides.
-  struct SyncComponents {
-    SyncComponents();
-    SyncComponents(SyncComponents&&);
-    ~SyncComponents();
-
-    std::unique_ptr<AssociatorInterface> model_associator;
-    std::unique_ptr<ChangeProcessor> change_processor;
-  };
-
   virtual std::unique_ptr<DataTypeManager> CreateDataTypeManager(
       ModelTypeSet initial_types,
       const WeakHandle<DataTypeDebugInfoListener>& debug_info_listener,
@@ -68,11 +45,6 @@ class SyncApiComponentFactory {
       const std::string& name,
       invalidation::InvalidationService* invalidator,
       const base::WeakPtr<SyncPrefs>& sync_prefs) = 0;
-
-  // Legacy datatypes that need to be converted to the SyncableService API.
-  virtual SyncComponents CreateBookmarkSyncComponents(
-      std::unique_ptr<DataTypeErrorHandler> error_handler,
-      UserShare* user_share) = 0;
 };
 
 }  // namespace syncer

components/sync/driver/sync_api_component_factory_mock.h

@@ -11,11 +11,8 @@
 #include "components/sync/base/model_type.h"
 #include "components/sync/driver/data_type_controller.h"
 #include "components/sync/driver/data_type_manager.h"
-#include "components/sync/driver/model_associator.h"
 #include "components/sync/driver/sync_api_component_factory.h"
 #include "components/sync/engine/sync_engine.h"
-#include "components/sync/model/change_processor.h"
-#include "components/sync/model/data_type_error_handler.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace syncer {
@@ -40,10 +37,6 @@ class SyncApiComponentFactoryMock : public SyncApiComponentFactory {
                    const std::string& name,
                    invalidation::InvalidationService* invalidator,
                    const base::WeakPtr<SyncPrefs>& sync_prefs));
-  MOCK_METHOD2(
-      CreateBookmarkSyncComponents,
-      SyncComponents(std::unique_ptr<DataTypeErrorHandler> error_handler,
-                     UserShare* user_share));
 };
 
 }  // namespace syncer

components/sync_bookmarks/BUILD.gn

@@ -8,8 +8,6 @@ static_library("sync_bookmarks") {
   sources = [
     "bookmark_change_processor.cc",
     "bookmark_change_processor.h",
-    "bookmark_data_type_controller.cc",
-    "bookmark_data_type_controller.h",
     "bookmark_local_changes_builder.cc",
     "bookmark_local_changes_builder.h",
     "bookmark_model_associator.cc",
@@ -47,7 +45,6 @@ source_set("unit_tests") {
   testonly = true
 
   sources = [
-    "bookmark_data_type_controller_unittest.cc",
     "bookmark_model_merger_unittest.cc",
     "bookmark_model_observer_impl_unittest.cc",
     "bookmark_model_type_processor_unittest.cc",

components/sync_bookmarks/bookmark_data_type_controller.cc

-// Copyright (c) 2012 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.
-
-#include "components/sync_bookmarks/bookmark_data_type_controller.h"
-
-#include <utility>
-
-#include "base/metrics/histogram.h"
-#include "components/sync/driver/model_associator.h"
-#include "components/sync/driver/sync_api_component_factory.h"
-#include "components/sync/driver/sync_service.h"
-#include "components/sync/model/change_processor.h"
-
-using bookmarks::BookmarkModel;
-
-namespace sync_bookmarks {
-
-BookmarkDataTypeController::BookmarkDataTypeController(
-    const base::RepeatingClosure& dump_stack,
-    syncer::SyncService* sync_service,
-    bookmarks::BookmarkModel* bookmark_model,
-    history::HistoryService* history_service,
-    syncer::SyncApiComponentFactory* component_factory)
-    : syncer::FrontendDataTypeController(syncer::BOOKMARKS,
-                                         dump_stack,
-                                         sync_service),
-      bookmark_model_(bookmark_model),
-      history_service_(history_service),
-      component_factory_(component_factory) {}
-
-BookmarkDataTypeController::~BookmarkDataTypeController() {}
-
-bool BookmarkDataTypeController::StartModels() {
-  DCHECK(CalledOnValidThread());
-  if (!DependentsLoaded()) {
-    bookmark_model_observer_.Add(bookmark_model_);
-    history_service_observer_.Add(history_service_);
-    return false;
-  }
-  return true;
-}
-
-void BookmarkDataTypeController::CleanUpState() {
-  DCHECK(CalledOnValidThread());
-  history_service_observer_.RemoveAll();
-  bookmark_model_observer_.RemoveAll();
-}
-
-void BookmarkDataTypeController::CreateSyncComponents() {
-  DCHECK(CalledOnValidThread());
-  syncer::SyncApiComponentFactory::SyncComponents sync_components =
-      component_factory_->CreateBookmarkSyncComponents(
-          CreateErrorHandler(), sync_service()->GetUserShare());
-  set_model_associator(std::move(sync_components.model_associator));
-  set_change_processor(std::move(sync_components.change_processor));
-}
-
-void BookmarkDataTypeController::BookmarkModelChanged() {
-}
-
-void BookmarkDataTypeController::BookmarkModelLoaded(BookmarkModel* model,
-                                                     bool ids_reassigned) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(model->loaded());
-  bookmark_model_observer_.RemoveAll();
-
-  if (!DependentsLoaded())
-    return;
-
-  history_service_observer_.RemoveAll();
-  OnModelLoaded();
-}
-
-void BookmarkDataTypeController::BookmarkModelBeingDeleted(
-    BookmarkModel* model) {
-  CleanUpState();
-}
-
-// Check that both the bookmark model and the history service (for favicons)
-// are loaded.
-bool BookmarkDataTypeController::DependentsLoaded() {
-  DCHECK(CalledOnValidThread());
-  if (!bookmark_model_ || !bookmark_model_->loaded())
-    return false;
-
-  if (!history_service_ || !history_service_->BackendLoaded())
-    return false;
-
-  // All necessary services are loaded.
-  return true;
-}
-
-void BookmarkDataTypeController::OnHistoryServiceLoaded(
-    history::HistoryService* service) {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(state_, MODEL_STARTING);
-  history_service_observer_.RemoveAll();
-
-  if (!DependentsLoaded())
-    return;
-
-  bookmark_model_observer_.RemoveAll();
-  OnModelLoaded();
-}
-
-void BookmarkDataTypeController::HistoryServiceBeingDeleted(
-    history::HistoryService* history_service) {
-  CleanUpState();
-}
-
-}  // namespace sync_bookmarks

components/sync_bookmarks/bookmark_data_type_controller.h

-// Copyright (c) 2012 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_BOOKMARKS_BOOKMARK_DATA_TYPE_CONTROLLER_H__
-#define COMPONENTS_SYNC_BOOKMARKS_BOOKMARK_DATA_TYPE_CONTROLLER_H__
-
-#include <string>
-
-#include "base/macros.h"
-#include "base/scoped_observer.h"
-#include "components/bookmarks/browser/base_bookmark_model_observer.h"
-#include "components/bookmarks/browser/bookmark_model.h"
-#include "components/history/core/browser/history_service.h"
-#include "components/history/core/browser/history_service_observer.h"
-#include "components/sync/driver/frontend_data_type_controller.h"
-
-namespace syncer {
-class SyncApiComponentFactory;
-class SyncService;
-}  // namespace syncer
-
-namespace sync_bookmarks {
-
-// A class that manages the startup and shutdown of bookmark sync.
-class BookmarkDataTypeController : public syncer::FrontendDataTypeController,
-                                   public bookmarks::BaseBookmarkModelObserver,
-                                   public history::HistoryServiceObserver {
- public:
-  // |dump_stack| is called when an unrecoverable error occurs.
-  BookmarkDataTypeController(
-      const base::RepeatingClosure& dump_stack,
-      syncer::SyncService* sync_service,
-      bookmarks::BookmarkModel* bookmark_model,
-      history::HistoryService* history_service,
-      syncer::SyncApiComponentFactory* component_factory);
-  ~BookmarkDataTypeController() override;
-
- private:
-  // syncer::FrontendDataTypeController:
-  bool StartModels() override;
-  void CleanUpState() override;
-  void CreateSyncComponents() override;
-
-  // bookmarks::BaseBookmarkModelObserver:
-  void BookmarkModelChanged() override;
-  void BookmarkModelLoaded(bookmarks::BookmarkModel* model,
-                           bool ids_reassigned) override;
-  void BookmarkModelBeingDeleted(bookmarks::BookmarkModel* model) override;
-
-  // Helper that returns true iff both the bookmark model and the history
-  // service have finished loading.
-  bool DependentsLoaded();
-
-  // history::HistoryServiceObserver:
-  void OnHistoryServiceLoaded(history::HistoryService* service) override;
-  void HistoryServiceBeingDeleted(
-      history::HistoryService* history_service) override;
-
-  bookmarks::BookmarkModel* const bookmark_model_;
-  history::HistoryService* const history_service_;
-  syncer::SyncApiComponentFactory* const component_factory_;
-
-  ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
-      history_service_observer_{this};
-  ScopedObserver<bookmarks::BookmarkModel, bookmarks::BookmarkModelObserver>
-      bookmark_model_observer_{this};
-
-  DISALLOW_COPY_AND_ASSIGN(BookmarkDataTypeController);
-};
-
-}  // namespace sync_bookmarks
-
-#endif  // COMPONENTS_SYNC_BOOKMARKS_BOOKMARK_DATA_TYPE_CONTROLLER_H__