[Sync] Remove RecordingModelTypeChangeProcessor.

The only data type which used the class in unit tests is typed URL. All
its unit tests were rewritten to use MockModelTypeChangeProcessor.

Bug: 791939
Change-Id: I66b3ed34fad24e67a1d330a3ccc592cd8091c5c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2467901
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816594}

components/history/core/browser/sync/typed_url_sync_bridge_unittest.cc

@@ -21,7 +21,6 @@
 #include "components/history/core/test/test_history_database.h"
 #include "components/sync/model/data_batch.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
-#include "components/sync/model/recording_model_type_change_processor.h"
 #include "components/sync/model/sync_metadata_store.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -37,12 +36,12 @@ using syncer::KeyAndData;
 using syncer::MetadataBatch;
 using syncer::MetadataChangeList;
 using syncer::MockModelTypeChangeProcessor;
-using syncer::RecordingModelTypeChangeProcessor;
 using testing::_;
 using testing::AllOf;
 using testing::Mock;
 using testing::NiceMock;
 using testing::Pointee;
+using testing::Return;
 
 namespace history {
 
@@ -81,6 +80,14 @@ MATCHER_P(HasTitleInSpecifics, title, "") {
   return arg.specifics.typed_url().title() == title;
 }
 
+MATCHER(HasTypedUrlInSpecifics, "") {
+  return arg.specifics.has_typed_url();
+}
+
+MATCHER(IsValidStorageKey, "") {
+  return TypedURLSyncMetadataDatabase::StorageKeyToURLID(arg) > 0;
+}
+
 Time SinceEpoch(int64_t microseconds_since_epoch) {
   return Time::FromDeltaSinceWindowsEpoch(
       TimeDelta::FromMicroseconds(microseconds_since_epoch));
@@ -300,7 +307,6 @@ class TypedURLSyncBridgeTest : public testing::Test {
     ASSERT_TRUE(test_dir_.CreateUniqueTempDir());
     fake_history_backend_->Init(
         false, TestHistoryDatabaseParamsForPath(test_dir_.GetPath()));
-    mock_processor_.DelegateCallsByDefaultTo(&processor_);
     std::unique_ptr<TypedURLSyncBridge> bridge =
         std::make_unique<TypedURLSyncBridge>(
             fake_history_backend_.get(), fake_history_backend_->db(),
@@ -312,13 +318,13 @@ class TypedURLSyncBridgeTest : public testing::Test {
   }
 
   void TearDown() override {
-    VerifyProcessorReceivedValidEntityData();
     fake_history_backend_->Closing();
   }
 
   // Starts sync for |typed_url_sync_bridge_| with |initial_data| as the
   // initial sync data.
   void StartSyncing(const std::vector<TypedUrlSpecifics>& specifics) {
+    ON_CALL(mock_processor_, IsTrackingMetadata()).WillByDefault(Return(true));
     // Set change processor.
     const auto error =
         bridge()->MergeSyncData(bridge()->CreateMetadataChangeList(),
@@ -455,13 +461,6 @@ class TypedURLSyncBridgeTest : public testing::Test {
                       base::BindOnce(&VerifyDataBatch, ExpectedMap(expected)));
   }
 
-  void VerifyProcessorReceivedValidEntityData() {
-    for (const auto& it : processor_.put_multimap()) {
-      EXPECT_GT(TypedURLSyncMetadataDatabase::StorageKeyToURLID(it.first), 0);
-      EXPECT_TRUE(it.second->specifics.has_typed_url());
-    }
-  }
-
   static void DiffVisits(const VisitVector& history_visits,
                          const sync_pb::TypedUrlSpecifics& sync_specifics,
                          std::vector<VisitInfo>* new_visits,
@@ -518,12 +517,6 @@ class TypedURLSyncBridgeTest : public testing::Test {
   base::ScopedTempDir test_dir_;
   scoped_refptr<TestHistoryBackend> fake_history_backend_;
   TypedURLSyncBridge* typed_url_sync_bridge_ = nullptr;
-  // TODO(crbug.com/791939): should be removed after moving to
-  // |mock_processor_|.
-  RecordingModelTypeChangeProcessor processor_;
-
-  // |mock_processor_| is preferred to use instead of |processor_|. The
-  // |processor_| will be removed in following patches.
   NiceMock<MockModelTypeChangeProcessor> mock_processor_;
 };
 
@@ -791,7 +784,8 @@ TEST_F(TypedURLSyncBridgeTest, MergeUrlsWithUsernameAndPassword) {
   WriteToTypedUrlSpecifics(server_row, server_visits, typed_url);
 
   // Check username/password url is not synced.
-  EXPECT_CALL(mock_processor_, Put(_, _, _));
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _));
 
   // Make sure there is no crash when merge two urls.
   StartSyncing({*typed_url});
@@ -816,7 +810,8 @@ TEST_F(TypedURLSyncBridgeTest, SimpleMerge) {
   sync_pb::TypedUrlSpecifics* typed_url = entity_specifics.mutable_typed_url();
   WriteToTypedUrlSpecifics(row2, visits2, typed_url);
 
-  EXPECT_CALL(mock_processor_, Put(_, _, _));
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _));
   StartSyncing({*typed_url});
 
   // Check that the backend was updated correctly.
@@ -844,7 +839,7 @@ TEST_F(TypedURLSyncBridgeTest, AddLocalTypedUrl) {
   urls.push_back(kURL);
 
   EntityData entity_data;
-  EXPECT_CALL(mock_processor_, Put(_, _, _))
+  EXPECT_CALL(mock_processor_, Put(IsValidStorageKey(), _, _))
       .WillOnce(SaveArgPointeeMove<1>(&entity_data));
   StartSyncing(std::vector<TypedUrlSpecifics>());
   BuildAndPushLocalChanges(1, 0, urls, &url_rows, &visit_vectors);
@@ -918,7 +913,8 @@ TEST_F(TypedURLSyncBridgeTest, ReloadVisitLocalTypedUrl) {
 
   StartSyncing(std::vector<TypedUrlSpecifics>());
 
-  EXPECT_CALL(mock_processor_, Put(_, _, _));
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _));
   BuildAndPushLocalChanges(1, 0, {kURL}, &url_rows, &visit_vectors);
 
   // Check that Put method has been already called.
@@ -948,7 +944,8 @@ TEST_F(TypedURLSyncBridgeTest, LinkVisitLocalTypedUrl) {
   urls.push_back(kURL);
 
   StartSyncing(std::vector<TypedUrlSpecifics>());
-  EXPECT_CALL(mock_processor_, Put(_, _, _));
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _));
   BuildAndPushLocalChanges(1, 0, urls, &url_rows, &visit_vectors);
 
   // Check that Put method has been already called.
@@ -1022,7 +1019,9 @@ TEST_F(TypedURLSyncBridgeTest, DeleteLocalTypedUrl) {
   urls.push_back("http://foo.com/");
 
   StartSyncing(std::vector<TypedUrlSpecifics>());
-  EXPECT_CALL(mock_processor_, Put(_, _, _)).Times(4);
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _))
+      .Times(4);
   BuildAndPushLocalChanges(4, 0, urls, &url_rows, &visit_vectors);
 
   // Delete some urls from backend and create deleted row vector.
@@ -1088,7 +1087,9 @@ TEST_F(TypedURLSyncBridgeTest, ExpireLocalTypedUrl) {
   urls.push_back("http://bar.com/");
 
   // Add the URLs into the history db and notify the bridge.
-  EXPECT_CALL(mock_processor_, Put(_, _, _)).Times(urls.size());
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _))
+      .Times(urls.size());
   EXPECT_CALL(mock_processor_, UntrackEntityForStorageKey(_)).Times(0);
   BuildAndPushLocalChanges(urls.size(), 0, urls, &url_rows, &visit_vectors);
   // Store the typed_urls incl. metadata into the bridge's database.
@@ -1333,7 +1334,8 @@ TEST_F(TypedURLSyncBridgeTest, DeleteUrlAndVisits) {
   urls.push_back(kURL);
 
   StartSyncing(std::vector<TypedUrlSpecifics>());
-  EXPECT_CALL(mock_processor_, Put(_, _, _));
+  EXPECT_CALL(mock_processor_,
+              Put(IsValidStorageKey(), Pointee(HasTypedUrlInSpecifics()), _));
   BuildAndPushLocalChanges(1, 0, urls, &url_rows, &visit_vectors);
 
   Time visit_time = SinceEpoch(3);

components/sync/BUILD.gn

@@ -377,8 +377,6 @@ static_library("test_support_model") {
     "model/mock_model_type_change_processor.h",
     "model/model_type_store_test_util.cc",
     "model/model_type_store_test_util.h",
-    "model/recording_model_type_change_processor.cc",
-    "model/recording_model_type_change_processor.h",
     "model/stub_model_type_sync_bridge.cc",
     "model/stub_model_type_sync_bridge.h",
     "model/sync_change_processor_wrapper_for_test.cc",

components/sync/model/recording_model_type_change_processor.cc

-// Copyright 2017 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/recording_model_type_change_processor.h"
-
-#include <utility>
-
-#include "base/bind.h"
-#include "base/memory/ptr_util.h"
-#include "components/sync/model/fake_model_type_sync_bridge.h"
-#include "components/sync/model/metadata_batch.h"
-
-namespace syncer {
-
-RecordingModelTypeChangeProcessor::RecordingModelTypeChangeProcessor() {}
-
-RecordingModelTypeChangeProcessor::~RecordingModelTypeChangeProcessor() {}
-
-void RecordingModelTypeChangeProcessor::Put(
-    const std::string& storage_key,
-    std::unique_ptr<EntityData> entity_data,
-    MetadataChangeList* metadata_changes) {
-  put_multimap_.insert(std::make_pair(storage_key, std::move(entity_data)));
-}
-
-void RecordingModelTypeChangeProcessor::Delete(
-    const std::string& storage_key,
-    MetadataChangeList* metadata_changes) {
-  delete_set_.insert(storage_key);
-}
-
-void RecordingModelTypeChangeProcessor::UpdateStorageKey(
-    const EntityData& entity_data,
-    const std::string& storage_key,
-    MetadataChangeList* metadata_change_list) {
-  update_multimap_.insert(std::make_pair(
-      storage_key, FakeModelTypeSyncBridge::CopyEntityData(entity_data)));
-}
-
-void RecordingModelTypeChangeProcessor::UntrackEntityForStorageKey(
-    const std::string& storage_key) {
-  untrack_for_storage_key_set_.insert(storage_key);
-}
-
-void RecordingModelTypeChangeProcessor::UntrackEntityForClientTagHash(
-    const ClientTagHash& client_tag_hash) {
-  untrack_for_client_tag_hash_set_.insert(client_tag_hash);
-}
-
-void RecordingModelTypeChangeProcessor::ModelReadyToSync(
-    std::unique_ptr<MetadataBatch> batch) {
-  std::swap(metadata_, batch);
-}
-
-bool RecordingModelTypeChangeProcessor::IsTrackingMetadata() {
-  return true;
-}
-
-std::string RecordingModelTypeChangeProcessor::TrackedAccountId() {
-  return "";
-}
-
-}  //  namespace syncer

components/sync/model/recording_model_type_change_processor.h

-// Copyright 2017 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_RECORDING_MODEL_TYPE_CHANGE_PROCESSOR_H_
-#define COMPONENTS_SYNC_MODEL_RECORDING_MODEL_TYPE_CHANGE_PROCESSOR_H_
-
-#include <map>
-#include <memory>
-#include <set>
-#include <string>
-
-#include "components/sync/base/client_tag_hash.h"
-#include "components/sync/model/fake_model_type_change_processor.h"
-#include "components/sync/model/model_type_sync_bridge.h"
-
-namespace syncer {
-
-// Augmented FakeModelTypeChangeProcessor that accumulates all instructions in
-// members that can then be accessed for verification.
-class RecordingModelTypeChangeProcessor : public FakeModelTypeChangeProcessor {
- public:
-  RecordingModelTypeChangeProcessor();
-  ~RecordingModelTypeChangeProcessor() override;
-
-  // FakeModelTypeChangeProcessor overrides.
-  void Put(const std::string& storage_key,
-           std::unique_ptr<EntityData> entity_data,
-           MetadataChangeList* metadata_changes) override;
-  void Delete(const std::string& storage_key,
-              MetadataChangeList* metadata_changes) 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;
-  void ModelReadyToSync(std::unique_ptr<MetadataBatch> batch) override;
-  bool IsTrackingMetadata() override;
-  std::string TrackedAccountId() override;
-
-  const std::multimap<std::string, std::unique_ptr<EntityData>>& put_multimap()
-      const {
-    return put_multimap_;
-  }
-
-  const std::multimap<std::string, std::unique_ptr<EntityData>>&
-  update_multimap() const {
-    return update_multimap_;
-  }
-
-  const std::set<std::string>& delete_set() const { return delete_set_; }
-
-  const std::set<std::string>& untrack_for_storage_key_set() const {
-    return untrack_for_storage_key_set_;
-  }
-
-  const std::set<ClientTagHash>& untrack_for_client_tag_hash_set() const {
-    return untrack_for_client_tag_hash_set_;
-  }
-
-  MetadataBatch* metadata() const { return metadata_.get(); }
-
- private:
-  std::multimap<std::string, std::unique_ptr<EntityData>> put_multimap_;
-  std::multimap<std::string, std::unique_ptr<EntityData>> update_multimap_;
-  std::set<std::string> delete_set_;
-  std::set<std::string> untrack_for_storage_key_set_;
-  std::set<ClientTagHash> untrack_for_client_tag_hash_set_;
-  std::unique_ptr<MetadataBatch> metadata_;
-};
-
-}  //  namespace syncer
-
-#endif  // COMPONENTS_SYNC_MODEL_RECORDING_MODEL_TYPE_CHANGE_PROCESSOR_H_