[Sync] Remove FakeModelTypeChangeProcessor.

This is very similar to MockModelTypeChangeProcessor and used in few
places.

This CL also changes the interface of UserEventService to not return the
raw bridge. It was changed to more common approach with returning
controller delegate from the service.

Bug: 791939
Change-Id: I10df56156023a21102cb13f418b17d915e823f76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468097
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Luum Habtemariam <luum@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818240}

chrome/browser/chromeos/printing/enterprise_printers_provider_unittest.cc

@@ -21,7 +21,6 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
-#include "components/sync/model/fake_model_type_change_processor.h"
 #include "components/sync/model/model_type_store.h"
 #include "components/sync/model/model_type_store_test_util.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"

chrome/browser/chromeos/printing/synced_printers_manager_unittest.cc

@@ -20,7 +20,6 @@
 #include "chrome/browser/chromeos/printing/synced_printers_manager_factory.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
-#include "components/sync/model/fake_model_type_change_processor.h"
 #include "components/sync/model/model_type_store.h"
 #include "components/sync/model/model_type_store_test_util.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"

chrome/browser/sync/chrome_sync_client.cc

@@ -652,8 +652,6 @@ ChromeSyncClient::GetControllerDelegateForModelType(syncer::ModelType type) {
           ->GetControllerDelegate();
     case syncer::USER_EVENTS:
       return browser_sync::UserEventServiceFactory::GetForProfile(profile_)
-          ->GetSyncBridge()
-          ->change_processor()
           ->GetControllerDelegate();
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     case syncer::WEB_APPS: {

components/sync/BUILD.gn

@@ -355,8 +355,6 @@ static_library("test_support_model") {
   sources = [
     "model/data_type_error_handler_mock.cc",
     "model/data_type_error_handler_mock.h",
-    "model/fake_model_type_change_processor.cc",
-    "model/fake_model_type_change_processor.h",
     "model/fake_model_type_controller_delegate.cc",
     "model/fake_model_type_controller_delegate.h",
     "model/fake_model_type_sync_bridge.cc",

components/sync/model/fake_model_type_change_processor.cc

-// Copyright 2016 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/model/fake_model_type_change_processor.h"
-
-#include <utility>
-
-#include "base/callback.h"
-#include "base/memory/ptr_util.h"
-#include "components/sync/model/metadata_batch.h"
-#include "components/sync/model/model_type_sync_bridge.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace syncer {
-
-FakeModelTypeChangeProcessor::FakeModelTypeChangeProcessor()
-    : FakeModelTypeChangeProcessor(nullptr) {}
-
-FakeModelTypeChangeProcessor::FakeModelTypeChangeProcessor(
-    base::WeakPtr<ModelTypeControllerDelegate> delegate)
-    : delegate_(delegate) {}
-
-FakeModelTypeChangeProcessor::~FakeModelTypeChangeProcessor() {}
-
-void FakeModelTypeChangeProcessor::Put(
-    const std::string& client_tag,
-    std::unique_ptr<EntityData> entity_data,
-    MetadataChangeList* metadata_change_list) {}
-
-void FakeModelTypeChangeProcessor::Delete(
-    const std::string& client_tag,
-    MetadataChangeList* metadata_change_list) {}
-
-void FakeModelTypeChangeProcessor::UpdateStorageKey(
-    const EntityData& entity_data,
-    const std::string& storage_key,
-    MetadataChangeList* metadata_change_list) {}
-
-void FakeModelTypeChangeProcessor::UntrackEntityForStorageKey(
-    const std::string& storage_key) {}
-
-void FakeModelTypeChangeProcessor::UntrackEntityForClientTagHash(
-    const ClientTagHash& client_tag_hash) {}
-
-bool FakeModelTypeChangeProcessor::IsEntityUnsynced(
-    const std::string& storage_key) {
-  return false;
-}
-
-base::Time FakeModelTypeChangeProcessor::GetEntityCreationTime(
-    const std::string& storage_key) const {
-  return base::Time();
-}
-
-base::Time FakeModelTypeChangeProcessor::GetEntityModificationTime(
-    const std::string& storage_key) const {
-  return base::Time();
-}
-
-void FakeModelTypeChangeProcessor::OnModelStarting(
-    ModelTypeSyncBridge* bridge) {}
-
-void FakeModelTypeChangeProcessor::ModelReadyToSync(
-    std::unique_ptr<MetadataBatch> batch) {}
-
-bool FakeModelTypeChangeProcessor::IsTrackingMetadata() {
-  return true;
-}
-
-std::string FakeModelTypeChangeProcessor::TrackedAccountId() {
-  return "";
-}
-
-std::string FakeModelTypeChangeProcessor::TrackedCacheGuid() {
-  return "";
-}
-
-void FakeModelTypeChangeProcessor::ReportError(const ModelError& error) {
-  error_ = error;
-}
-
-base::Optional<ModelError> FakeModelTypeChangeProcessor::GetError() const {
-  return error_;
-}
-
-base::WeakPtr<ModelTypeControllerDelegate>
-FakeModelTypeChangeProcessor::GetControllerDelegate() {
-  return delegate_;
-}
-
-}  // namespace syncer

components/sync/model/fake_model_type_change_processor.h

-// Copyright 2016 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_MODEL_FAKE_MODEL_TYPE_CHANGE_PROCESSOR_H_
-#define COMPONENTS_SYNC_MODEL_FAKE_MODEL_TYPE_CHANGE_PROCESSOR_H_
-
-#include <memory>
-#include <string>
-
-#include "base/memory/weak_ptr.h"
-#include "components/sync/base/model_type.h"
-#include "components/sync/model/metadata_change_list.h"
-#include "components/sync/model/model_error.h"
-#include "components/sync/model/model_type_change_processor.h"
-
-namespace syncer {
-
-class ModelTypeSyncBridge;
-
-// A ModelTypeChangeProcessor implementation for tests.
-class FakeModelTypeChangeProcessor : public ModelTypeChangeProcessor {
- public:
-  FakeModelTypeChangeProcessor();
-  explicit FakeModelTypeChangeProcessor(
-      base::WeakPtr<ModelTypeControllerDelegate> delegate);
-  ~FakeModelTypeChangeProcessor() override;
-
-  // ModelTypeChangeProcessor overrides
-  void Put(const std::string& client_tag,
-           std::unique_ptr<EntityData> entity_data,
-           MetadataChangeList* metadata_change_list) override;
-  void Delete(const std::string& client_tag,
-              MetadataChangeList* metadata_change_list) override;
-  void UpdateStorageKey(const EntityData& entity_data,
-                        const std::string& storage_key,
-                        MetadataChangeList* metadata_change_list) override;
-  void UntrackEntityForStorageKey(const std::string& storage_key) override;
-  void UntrackEntityForClientTagHash(
-      const ClientTagHash& client_tag_hash) override;
-  bool IsEntityUnsynced(const std::string& storage_key) override;
-  base::Time GetEntityCreationTime(
-      const std::string& storage_key) const override;
-  base::Time GetEntityModificationTime(
-      const std::string& storage_key) const override;
-  void OnModelStarting(ModelTypeSyncBridge* bridge) override;
-  void ModelReadyToSync(std::unique_ptr<MetadataBatch> batch) override;
-  bool IsTrackingMetadata() override;
-  std::string TrackedAccountId() override;
-  std::string TrackedCacheGuid() override;
-  void ReportError(const ModelError& error) override;
-  base::Optional<ModelError> GetError() const override;
-  base::WeakPtr<ModelTypeControllerDelegate> GetControllerDelegate() override;
-
- private:
-  base::Optional<ModelError> error_;
-  base::WeakPtr<ModelTypeControllerDelegate> delegate_;
-};
-
-}  // namespace syncer
-
-#endif  // COMPONENTS_SYNC_MODEL_FAKE_MODEL_TYPE_CHANGE_PROCESSOR_H_

components/sync/model/stub_model_type_sync_bridge.cc

@@ -7,14 +7,10 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "components/sync/model/fake_model_type_change_processor.h"
+#include "components/sync/model/metadata_change_list.h"
 
 namespace syncer {
 
-StubModelTypeSyncBridge::StubModelTypeSyncBridge()
-    : StubModelTypeSyncBridge(
-          std::make_unique<FakeModelTypeChangeProcessor>()) {}
-
 StubModelTypeSyncBridge::StubModelTypeSyncBridge(
     std::unique_ptr<ModelTypeChangeProcessor> change_processor)
     : ModelTypeSyncBridge(std::move(change_processor)) {}

components/sync/model/stub_model_type_sync_bridge.h

@@ -17,7 +17,6 @@ namespace syncer {
 // testing purposes.
 class StubModelTypeSyncBridge : public ModelTypeSyncBridge {
  public:
-  StubModelTypeSyncBridge();
   explicit StubModelTypeSyncBridge(
       std::unique_ptr<ModelTypeChangeProcessor> change_processor);
   ~StubModelTypeSyncBridge() override;

components/sync_user_events/fake_user_event_service.cc

@@ -4,14 +4,13 @@
 
 #include "components/sync_user_events/fake_user_event_service.h"
 
-#include "components/sync/model/fake_model_type_change_processor.h"
 
 using sync_pb::UserEventSpecifics;
 
 namespace syncer {
 
 FakeUserEventService::FakeUserEventService()
-    : fake_bridge_(std::make_unique<FakeModelTypeChangeProcessor>()) {}
+    : fake_controller_delegate_(syncer::USER_EVENTS) {}
 
 FakeUserEventService::~FakeUserEventService() {}
 
@@ -26,8 +25,9 @@ void FakeUserEventService::RecordUserEvent(
   recorded_user_events_.push_back(specifics);
 }
 
-ModelTypeSyncBridge* FakeUserEventService::GetSyncBridge() {
-  return &fake_bridge_;
+base::WeakPtr<syncer::ModelTypeControllerDelegate>
+FakeUserEventService::GetControllerDelegate() {
+  return fake_controller_delegate_.GetWeakPtr();
 }
 
 const std::vector<UserEventSpecifics>&

components/sync_user_events/fake_user_event_service.h

@@ -10,7 +10,7 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "components/sync/model/fake_model_type_sync_bridge.h"
+#include "components/sync/model/fake_model_type_controller_delegate.h"
 #include "components/sync/protocol/user_event_specifics.pb.h"
 #include "components/sync_user_events/user_event_service.h"
 
@@ -27,18 +27,14 @@ class FakeUserEventService : public UserEventService {
   void RecordUserEvent(
       std::unique_ptr<sync_pb::UserEventSpecifics> specifics) override;
   void RecordUserEvent(const sync_pb::UserEventSpecifics& specifics) override;
-  // TODO(crbug.com/895340): This is hard to mock, replace it (in the base
-  // class) by GetControllerDelegate(), then we can get rid of |fake_bridge_|.
-  // Maybe we can also expose a raw pointer to be consumed by
-  // ForwardingModelTypeControllerDelegate and not care about WeakPtrs anymore
-  // (but we need a nice solution for SyncClient).
-  ModelTypeSyncBridge* GetSyncBridge() override;
+  base::WeakPtr<syncer::ModelTypeControllerDelegate> GetControllerDelegate()
+      override;
 
   const std::vector<sync_pb::UserEventSpecifics>& GetRecordedUserEvents() const;
 
  private:
   std::vector<sync_pb::UserEventSpecifics> recorded_user_events_;
-  FakeModelTypeSyncBridge fake_bridge_;
+  FakeModelTypeControllerDelegate fake_controller_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeUserEventService);
 };

components/sync_user_events/no_op_user_event_service.cc

@@ -20,7 +20,8 @@ void NoOpUserEventService::RecordUserEvent(
 void NoOpUserEventService::RecordUserEvent(
     const UserEventSpecifics& specifics) {}
 
-ModelTypeSyncBridge* NoOpUserEventService::GetSyncBridge() {
+base::WeakPtr<syncer::ModelTypeControllerDelegate>
+NoOpUserEventService::GetControllerDelegate() {
   return nullptr;
 }
 

components/sync_user_events/no_op_user_event_service.h

@@ -13,8 +13,6 @@
 
 namespace syncer {
 
-class ModelTypeSyncBridge;
-
 // This implementation is used when we know event should never be recorded,
 // such as in incognito mode.
 class NoOpUserEventService : public UserEventService {
@@ -26,7 +24,8 @@ class NoOpUserEventService : public UserEventService {
   void RecordUserEvent(
       std::unique_ptr<sync_pb::UserEventSpecifics> specifics) override;
   void RecordUserEvent(const sync_pb::UserEventSpecifics& specifics) override;
-  ModelTypeSyncBridge* GetSyncBridge() override;
+  base::WeakPtr<syncer::ModelTypeControllerDelegate> GetControllerDelegate()
+      override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NoOpUserEventService);

components/sync_user_events/user_event_service.h

@@ -15,7 +15,7 @@
 
 namespace syncer {
 
-class ModelTypeSyncBridge;
+class ModelTypeControllerDelegate;
 
 class UserEventService : public KeyedService {
  public:
@@ -31,7 +31,8 @@ class UserEventService : public KeyedService {
       const sync_pb::UserEventSpecifics& specifics) = 0;
 
   // Returns the underlying Sync integration point.
-  virtual ModelTypeSyncBridge* GetSyncBridge() = 0;
+  virtual base::WeakPtr<syncer::ModelTypeControllerDelegate>
+  GetControllerDelegate() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(UserEventService);

components/sync_user_events/user_event_service_impl.cc

@@ -79,8 +79,9 @@ void UserEventServiceImpl::RecordUserEvent(
   RecordUserEvent(std::make_unique<UserEventSpecifics>(specifics));
 }
 
-ModelTypeSyncBridge* UserEventServiceImpl::GetSyncBridge() {
-  return bridge_.get();
+base::WeakPtr<syncer::ModelTypeControllerDelegate>
+UserEventServiceImpl::GetControllerDelegate() {
+  return bridge_->change_processor()->GetControllerDelegate();
 }
 
 bool UserEventServiceImpl::ShouldRecordEvent(

components/sync_user_events/user_event_service_impl.h

@@ -16,7 +16,6 @@
 
 namespace syncer {
 
-class ModelTypeSyncBridge;
 class UserEventSyncBridge;
 
 class UserEventServiceImpl : public UserEventService {
@@ -31,7 +30,8 @@ class UserEventServiceImpl : public UserEventService {
   void RecordUserEvent(
       std::unique_ptr<sync_pb::UserEventSpecifics> specifics) override;
   void RecordUserEvent(const sync_pb::UserEventSpecifics& specifics) override;
-  ModelTypeSyncBridge* GetSyncBridge() override;
+  base::WeakPtr<syncer::ModelTypeControllerDelegate> GetControllerDelegate()
+      override;
 
  private:
   // Checks dynamic or event specific conditions.

ios/chrome/browser/sync/ios_chrome_sync_client.mm

@@ -229,8 +229,6 @@ IOSChromeSyncClient::GetControllerDelegateForModelType(syncer::ModelType type) {
           ->GetControllerDelegate();
     case syncer::USER_EVENTS:
       return IOSUserEventServiceFactory::GetForBrowserState(browser_state_)
-          ->GetSyncBridge()
-          ->change_processor()
           ->GetControllerDelegate();
 
     // We don't exercise this function for certain datatypes, because their