Migrate various users of FakeModelTypeChangeProcessor

We'd like to get rid of the fake, in favor of the mock, since most tests
are anyway using the fake for mocking purposes (counting function calls
and verifying arguments).

Bug: 791939
Change-Id: Ie48fba806604af4aa622a56dc0e76e36c943f90e
Reviewed-on: https://chromium-review.googlesource.com/1109511Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569317}

components/sync/device_info/device_info_sync_bridge_unittest.cc

@@ -17,7 +17,6 @@
 #include "components/sync/model/data_batch.h"
 #include "components/sync/model/data_type_error_handler_mock.h"
 #include "components/sync/model/entity_data.h"
-#include "components/sync/model/fake_model_type_change_processor.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
 #include "components/sync/model/model_type_store_test_util.h"

components/sync/user_events/user_event_service_impl_unittest.cc

@@ -5,6 +5,7 @@
 #include "components/sync/user_events/user_event_service_impl.h"
 
 #include <utility>
+#include <vector>
 
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
@@ -12,8 +13,8 @@
 #include "components/sync/base/model_type.h"
 #include "components/sync/driver/fake_sync_service.h"
 #include "components/sync/driver/sync_driver_switches.h"
+#include "components/sync/model/mock_model_type_change_processor.h"
 #include "components/sync/model/model_type_store_test_util.h"
-#include "components/sync/model/recording_model_type_change_processor.h"
 #include "components/sync/protocol/sync.pb.h"
 #include "components/sync/user_events/user_event_sync_bridge.h"
 #include "components/variations/variations_associated_data.h"
@@ -21,6 +22,7 @@
 
 using base::test::ScopedFeatureList;
 using sync_pb::UserEventSpecifics;
+using testing::_;
 
 namespace syncer {
 
@@ -61,6 +63,13 @@ std::unique_ptr<UserEventSpecifics> WithNav(
   return specifics;
 }
 
+MATCHER_P(HasFieldTrialVariationIds, expected_variation_id, "") {
+  const UserEventSpecifics& specifics = arg->specifics.user_event();
+  return specifics.field_trial_event().variation_ids_size() == 1 &&
+         specifics.field_trial_event().variation_ids(0) ==
+             expected_variation_id;
+}
+
 // TODO(vitaliii): Merge this into FakeSyncService and use it instead.
 class TestSyncService : public FakeSyncService {
  public:
@@ -95,24 +104,25 @@ class UserEventServiceImplTest : public testing::Test {
  protected:
   UserEventServiceImplTest()
       : field_trial_list_(nullptr),
-        sync_service_(true, false, {HISTORY_DELETE_DIRECTIVES}) {}
+        sync_service_(true, false, {HISTORY_DELETE_DIRECTIVES}) {
+    ON_CALL(mock_processor_, IsTrackingMetadata())
+        .WillByDefault(testing::Return(true));
+  }
 
   std::unique_ptr<UserEventSyncBridge> MakeBridge() {
     return std::make_unique<UserEventSyncBridge>(
         ModelTypeStoreTestUtil::FactoryForInMemoryStoreForTest(),
-        RecordingModelTypeChangeProcessor::CreateProcessorAndAssignRawPointer(
-            &processor_),
-        &mapper_, &sync_service_);
+        mock_processor_.CreateForwardingProcessor(), &mapper_, &sync_service_);
   }
 
   TestSyncService* sync_service() { return &sync_service_; }
-  const RecordingModelTypeChangeProcessor& processor() { return *processor_; }
+  MockModelTypeChangeProcessor* mock_processor() { return &mock_processor_; }
 
  private:
   base::MessageLoop message_loop_;
   base::FieldTrialList field_trial_list_;
   TestSyncService sync_service_;
-  RecordingModelTypeChangeProcessor* processor_;
+  testing::NiceMock<MockModelTypeChangeProcessor> mock_processor_;
   TestGlobalIdMapper mapper_;
 };
 
@@ -134,8 +144,8 @@ TEST_F(UserEventServiceImplTest, MightRecordEventsFeatureDisabled) {
 
 TEST_F(UserEventServiceImplTest, ShouldRecord) {
   UserEventServiceImpl service(sync_service(), MakeBridge());
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(AsTest(Event()));
-  EXPECT_EQ(1u, processor().put_multimap().size());
 }
 
 TEST_F(UserEventServiceImplTest, ShouldRecordNoHistory) {
@@ -143,17 +153,19 @@ TEST_F(UserEventServiceImplTest, ShouldRecordNoHistory) {
   UserEventServiceImpl service(&no_history_sync_service, MakeBridge());
 
   // Only record events without navigation ids when history sync is off.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _)).Times(0);
   service.RecordUserEvent(WithNav(AsTest(Event())));
-  EXPECT_EQ(0u, processor().put_multimap().size());
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(AsTest(Event()));
 }
 
 TEST_F(UserEventServiceImplTest, ShouldRecordUserConsentNoHistory) {
   TestSyncService no_history_sync_service(true, false, ModelTypeSet());
   UserEventServiceImpl service(&no_history_sync_service, MakeBridge());
-  service.RecordUserEvent(AsConsent(Event()));
+
   // UserConsent recording doesn't need history sync to be enabled.
-  EXPECT_EQ(1u, processor().put_multimap().size());
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
+  service.RecordUserEvent(AsConsent(Event()));
 }
 
 TEST_F(UserEventServiceImplTest, ShouldRecordPassphrase) {
@@ -162,10 +174,11 @@ TEST_F(UserEventServiceImplTest, ShouldRecordPassphrase) {
   UserEventServiceImpl service(&passphrase_sync_service, MakeBridge());
 
   // Only record events without navigation ids when a passphrase is used.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _)).Times(0);
   service.RecordUserEvent(WithNav(AsTest(Event())));
-  EXPECT_EQ(0u, processor().put_multimap().size());
+
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(AsTest(Event()));
-  EXPECT_EQ(1u, processor().put_multimap().size());
 }
 
 TEST_F(UserEventServiceImplTest, ShouldRecordEngineOff) {
@@ -175,59 +188,62 @@ TEST_F(UserEventServiceImplTest, ShouldRecordEngineOff) {
                                MakeBridge());
 
   // Only record events without navigation ids when the engine is off.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _)).Times(0);
   service.RecordUserEvent(WithNav(AsTest(Event())));
-  EXPECT_EQ(0u, processor().put_multimap().size());
+
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(AsTest(Event()));
-  EXPECT_EQ(1u, processor().put_multimap().size());
 }
 
 TEST_F(UserEventServiceImplTest, ShouldRecordEmpty) {
   UserEventServiceImpl service(sync_service(), MakeBridge());
 
   // All untyped events should always be ignored.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _)).Times(0);
   service.RecordUserEvent(Event());
-  EXPECT_EQ(0u, processor().put_multimap().size());
   service.RecordUserEvent(WithNav(Event()));
-  EXPECT_EQ(0u, processor().put_multimap().size());
 }
 
 TEST_F(UserEventServiceImplTest, ShouldRecordHasNavigationId) {
   UserEventServiceImpl service(sync_service(), MakeBridge());
 
   // Verify logic for types that might or might not have a navigation id.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(AsTest(Event()));
-  EXPECT_EQ(1u, processor().put_multimap().size());
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(WithNav(AsTest(Event())));
-  EXPECT_EQ(2u, processor().put_multimap().size());
 
   // Verify logic for types that must have a navigation id.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _)).Times(0);
   service.RecordUserEvent(AsDetection(Event()));
-  EXPECT_EQ(2u, processor().put_multimap().size());
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(WithNav(AsDetection(Event())));
-  EXPECT_EQ(3u, processor().put_multimap().size());
 
   // Verify logic for types that cannot have a navigation id.
+  EXPECT_CALL(*mock_processor(), Put(_, _, _));
   service.RecordUserEvent(AsTrial(Event()));
-  EXPECT_EQ(4u, processor().put_multimap().size());
+  EXPECT_CALL(*mock_processor(), Put(_, _, _)).Times(0);
   service.RecordUserEvent(WithNav(AsTrial(Event())));
-  EXPECT_EQ(4u, processor().put_multimap().size());
 }
 
 TEST_F(UserEventServiceImplTest, SessionIdIsDifferent) {
+  std::vector<int64_t> put_session_ids;
+  ON_CALL(*mock_processor(), Put(_, _, _))
+      .WillByDefault([&](const std::string& storage_key,
+                         const std::unique_ptr<EntityData> entity_data,
+                         MetadataChangeList* metadata_change_list) {
+        put_session_ids.push_back(
+            entity_data->specifics.user_event().session_id());
+      });
+
   UserEventServiceImpl service1(sync_service(), MakeBridge());
   service1.RecordUserEvent(AsTest(Event()));
-  ASSERT_EQ(1u, processor().put_multimap().size());
-  auto put1 = processor().put_multimap().begin();
-  int64_t session_id1 = put1->second->specifics.user_event().session_id();
 
   UserEventServiceImpl service2(sync_service(), MakeBridge());
   service2.RecordUserEvent(AsTest(Event()));
-  // The object processor() points to has changed to be |service2|'s processor.
-  ASSERT_EQ(1u, processor().put_multimap().size());
-  auto put2 = processor().put_multimap().begin();
-  int64_t session_id2 = put2->second->specifics.user_event().session_id();
 
-  EXPECT_NE(session_id1, session_id2);
+  ASSERT_EQ(2U, put_session_ids.size());
+  EXPECT_NE(put_session_ids[0], put_session_ids[1]);
 }
 
 TEST_F(UserEventServiceImplTest, FieldTrial) {
@@ -236,12 +252,8 @@ TEST_F(UserEventServiceImplTest, FieldTrial) {
   base::FieldTrialList::CreateFieldTrial("trial", "group");
   base::FieldTrialList::FindFullName("trial");
 
+  EXPECT_CALL(*mock_processor(), Put(_, HasFieldTrialVariationIds(123u), _));
   UserEventServiceImpl service(sync_service(), MakeBridge());
-  ASSERT_EQ(1u, processor().put_multimap().size());
-  const UserEventSpecifics specifics =
-      processor().put_multimap().begin()->second->specifics.user_event();
-  ASSERT_EQ(1, specifics.field_trial_event().variation_ids_size());
-  EXPECT_EQ(123u, specifics.field_trial_event().variation_ids(0));
 }
 
 }  // namespace