sync: Remove SyncSourceInfo

sync/engine/sync_scheduler_unittest.cc

Index: sync/engine/sync_scheduler_unittest.cc
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 3fb2042fe05b57a50ed09bed6c7eae5fcf8128de..00bd2531fe566a3a8767c336be1ef13c324d13f8 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -45,14 +45,19 @@ class MockSyncer : public Syncer {
   MOCK_METHOD3(NormalSyncShare, bool(ModelTypeSet,
                                      const sessions::NudgeTracker&,
                                      sessions::SyncSession*));
-  MOCK_METHOD2(ConfigureSyncShare, bool(ModelTypeSet, sessions::SyncSession*));
+  MOCK_METHOD3(ConfigureSyncShare,
+               bool(ModelTypeSet,
+                    sync_pb::GetUpdatesCallerInfo::GetUpdatesSource,
+                    SyncSession*));
   MOCK_METHOD2(PollSyncShare, bool(ModelTypeSet, sessions::SyncSession*));
 };
 
 // Used when tests want to record syncing activity to examine later.
+enum SyncShareSource { POLL, CONFIG, NORMAL };
+
 struct SyncShareRecords {
   std::vector<TimeTicks> times;
-  std::vector<SyncSessionSnapshot> snapshots;
+  std::vector<SyncShareSource> sources;
 };
 
 void QuitLoopNow() {
@@ -187,8 +192,7 @@ class SyncSchedulerTest : public testing::Test {
       SCOPED_TRACE(testing::Message() << "SyncShare # (" << i << ")");
       TimeTicks optimal_next_sync = optimal_start + poll_interval * i;
       EXPECT_GE(data[i], optimal_next_sync);
-      EXPECT_EQ(GetUpdatesCallerInfo::PERIODIC,
-                records.snapshots[i].source().updates_source);
+      EXPECT_EQ(POLL, records.sources[i]);
     }
   }
 
@@ -246,20 +250,20 @@ class SyncSchedulerTest : public testing::Test {
   ModelSafeRoutingInfo routing_info_;
 };
 
-void RecordSyncShareImpl(SyncSession* s, SyncShareRecords* record) {
+void RecordSyncShareImpl(SyncShareSource source, SyncShareRecords* record) {
   record->times.push_back(TimeTicks::Now());
-  record->snapshots.push_back(s->TakeSnapshot());
+  record->sources.push_back(source);
 }
 
-ACTION_P(RecordSyncShare, record) {
-  RecordSyncShareImpl(arg0, record);
+ACTION_P2(RecordSyncShare, source, record) {
+  RecordSyncShareImpl(source, record);
   if (base::MessageLoop::current()->is_running())
     QuitLoopNow();
   return true;
 }
 
-ACTION_P2(RecordSyncShareMultiple, record, quit_after) {
-  RecordSyncShareImpl(arg0, record);
+ACTION_P3(RecordSyncShareMultiple, source, record, quit_after) {
+  RecordSyncShareImpl(source, record);
   EXPECT_LE(record->times.size(), quit_after);
   if (record->times.size() >= quit_after &&
       base::MessageLoop::current()->is_running()) {
@@ -286,7 +290,7 @@ TEST_F(SyncSchedulerTest, Nudge) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&records))))
+                      RecordSyncShare(NORMAL, &records)))
       .RetiresOnSaturation();
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
@@ -294,12 +298,8 @@ TEST_F(SyncSchedulerTest, Nudge) {
   scheduler()->ScheduleLocalNudge(zero(), model_types, FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(1U, records.snapshots.size());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          model_types,
-          records.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            records.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, records.sources.size());
+  EXPECT_EQ(NORMAL, records.sources[0]);

[tim (not reviewing), 2013/08/01 17:16:38]

This expectation doesn't fill the shoes of the previous code. Previously, the
GetUpdatesCallerValue was set by scheduler / syncer code and we'd verify that;
now we're not really verifying anything at all besides our own test code (unless
I'm misunderstanding?).

For parity, one thing you could do is pre-bind an output GetUpdatesCallerInfo
parameter on SimulateNormalSuccess to a local variable, assign it in the test
utility, and verify *that*, to get the value that would have been sent to the
server.

[rlarocque, 2013/08/01 21:01:41]

You're right that this code tests less than it used to.

However, I don't think that checking the GetUpdatesCallerInfo value is as
important as it used to be.  The mocks can now assert that the scheduler calls
NormalSyncShare() and would fail if the scheduler decided to run a configure
instead.

Verifying the recorded cycles is still valuable in cases where the mock can't
make the sort of assertions we're looking for, as in the BackoffElevation test. 
You're right that it doesn't accomplish much in this test, though.

I think what we've lost here is the ability to detect errors where the
GetUpdatesCallerInfo is set wrong but it's still within the same category (ie.
NOTIFICATION instead of LOCAL), or where the SyncSource types are set wrong. 
The NudgeTracker unit tests should do a good job of detecting these kinds of
issues, so this CL shouldn't cost us too much coverage.

 
   Mock::VerifyAndClearExpectations(syncer());
 
@@ -309,16 +309,12 @@ TEST_F(SyncSchedulerTest, Nudge) {
   model_types.Put(AUTOFILL);
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&records2))));
+                      RecordSyncShare(NORMAL, &records2)));
   scheduler()->ScheduleLocalNudge(zero(), model_types, FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(1U, records2.snapshots.size());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          model_types,
-          records2.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            records2.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, records2.sources.size());
+  EXPECT_EQ(NORMAL, records2.sources[0]);
 }
 
 // Make sure a regular config command is scheduled fine in the absence of any
@@ -327,9 +323,9 @@ TEST_F(SyncSchedulerTest, Config) {
   SyncShareRecords records;
   const ModelTypeSet model_types(BOOKMARKS);
 
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      WithArg<1>(RecordSyncShare(&records))));
+                      RecordSyncShare(CONFIG, &records)));
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
@@ -342,12 +338,8 @@ TEST_F(SyncSchedulerTest, Config) {
   ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(1, counter.times_called());
 
-  ASSERT_EQ(1U, records.snapshots.size());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          model_types,
-          records.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
-            records.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, records.sources.size());
+  EXPECT_EQ(CONFIG, records.sources[0]);
 }
 
 // Simulate a failure and make sure the config request is retried.
@@ -358,15 +350,15 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   SyncShareRecords records;
   const ModelTypeSet model_types(BOOKMARKS);
 
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      WithArg<1>(RecordSyncShare(&records))))
+                      RecordSyncShare(CONFIG, &records)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      WithArg<1>(RecordSyncShare(&records))));
+                      RecordSyncShare(CONFIG, &records)));
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
-  ASSERT_EQ(0U, records.snapshots.size());
+  ASSERT_EQ(0U, records.sources.size());
   CallbackCounter counter;
   ConfigurationParams params(
       GetUpdatesCallerInfo::RECONFIGURATION,
@@ -376,16 +368,12 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(0, counter.times_called());
 
-  ASSERT_EQ(1U, records.snapshots.size());
+  ASSERT_EQ(1U, records.sources.size());
   RunLoop();
 
-  ASSERT_EQ(2U, records.snapshots.size());
+  ASSERT_EQ(2U, records.sources.size());
   ASSERT_EQ(1, counter.times_called());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          model_types,
-          records.snapshots[1].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
-            records.snapshots[1].source().updates_source);
+  EXPECT_EQ(CONFIG, records.sources[1]);
 }
 
 // Issue a nudge when the config has failed. Make sure both the config and
@@ -397,21 +385,21 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
       .WillRepeatedly(Return(TimeDelta::FromMilliseconds(50)));
   SyncShareRecords records;
 
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      WithArg<1>(RecordSyncShare(&records))))
+                      RecordSyncShare(CONFIG, &records)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      WithArg<1>(RecordSyncShare(&records))))
+                      RecordSyncShare(CONFIG, &records)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      WithArg<1>(RecordSyncShare(&records))));
+                      RecordSyncShare(CONFIG, &records)));
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&records))));
+                      RecordSyncShare(NORMAL, &records)));
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
-  ASSERT_EQ(0U, records.snapshots.size());
+  ASSERT_EQ(0U, records.sources.size());
   CallbackCounter counter;
   ConfigurationParams params(
       GetUpdatesCallerInfo::RECONFIGURATION,
@@ -420,40 +408,28 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
       base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
   ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(0, counter.times_called());
-  ASSERT_EQ(1U, records.snapshots.size());
+  ASSERT_EQ(1U, records.sources.size());
 
   scheduler()->ScheduleLocalNudge(zero(), model_types, FROM_HERE);
   RunLoop();
   // Note that we're not RunLoop()ing for the NUDGE we just scheduled, but
   // for the first retry attempt from the config job (after
   // waiting ~+/- 50ms).
-  ASSERT_EQ(2U, records.snapshots.size());
+  ASSERT_EQ(2U, records.sources.size());
   ASSERT_EQ(0, counter.times_called());
-  EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
-            records.snapshots[1].source().updates_source);
+  EXPECT_EQ(CONFIG, records.sources[1]);
 
   RunLoop();
   // This is the 3rd attempt, which we've set up to SimulateSuccess.
-  ASSERT_EQ(3U, records.snapshots.size());
+  ASSERT_EQ(3U, records.sources.size());
   ASSERT_EQ(1, counter.times_called());
 
   // Now change the mode so nudge can execute.
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
-  ASSERT_EQ(4U, records.snapshots.size());
-
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          model_types,
-          records.snapshots[2].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
-            records.snapshots[2].source().updates_source);
-
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          model_types,
-          records.snapshots[3].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            records.snapshots[3].source().updates_source);
-
+  ASSERT_EQ(4U, records.sources.size());
+  EXPECT_EQ(CONFIG, records.sources[2]);
+  EXPECT_EQ(NORMAL, records.sources[3]);
 }
 
 // Test that nudges are coalesced.
@@ -463,7 +439,7 @@ TEST_F(SyncSchedulerTest, NudgeCoalescing) {
   SyncShareRecords r;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&r))));
+                      RecordSyncShare(NORMAL, &r)));
   const ModelTypeSet types1(BOOKMARKS), types2(AUTOFILL), types3(THEMES);
   TimeDelta delay = zero();
   TimeTicks optimal_time = TimeTicks::Now() + delay;
@@ -471,29 +447,21 @@ TEST_F(SyncSchedulerTest, NudgeCoalescing) {
   scheduler()->ScheduleLocalNudge(zero(), types2, FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(1U, r.snapshots.size());
+  ASSERT_EQ(1U, r.sources.size());
   EXPECT_GE(r.times[0], optimal_time);
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          Union(types1, types2),
-          r.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            r.snapshots[0].source().updates_source);
+  EXPECT_EQ(NORMAL, r.sources[0]);
 
   Mock::VerifyAndClearExpectations(syncer());
 
   SyncShareRecords r2;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&r2))));
+                      RecordSyncShare(NORMAL, &r2)));
   scheduler()->ScheduleLocalNudge(zero(), types3, FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(1U, r2.snapshots.size());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          types3,
-          r2.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            r2.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, r2.sources.size());
+  EXPECT_EQ(NORMAL, r2.sources[0]);
 }
 
 // Test that nudges are coalesced.
@@ -503,7 +471,7 @@ TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) {
   SyncShareRecords r;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&r))));
+                      RecordSyncShare(NORMAL, &r)));
   ModelTypeSet types1(BOOKMARKS), types2(AUTOFILL), types3;
 
   // Create a huge time delay.
@@ -518,10 +486,7 @@ TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) {
   RunLoop();
 
   // Make sure the sync has happened.
-  ASSERT_EQ(1U, r.snapshots.size());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          Union(types1, types2),
-          r.snapshots[0].source().types));
+  ASSERT_EQ(1U, r.sources.size());
 
   // Make sure the sync happened at the right time.
   EXPECT_GE(r.times[0], min_time);
@@ -539,15 +504,13 @@ TEST_F(SyncSchedulerTest, NudgeWithStates) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&records))))
+                      RecordSyncShare(NORMAL, &records)))
       .RetiresOnSaturation();
   scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(1U, records.snapshots.size());
-  EXPECT_THAT(invalidation_map, Eq(records.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::NOTIFICATION,
-            records.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, records.sources.size());
+  EXPECT_EQ(NORMAL, records.sources[0]);
 
   Mock::VerifyAndClearExpectations(syncer());
 
@@ -557,14 +520,12 @@ TEST_F(SyncSchedulerTest, NudgeWithStates) {
   invalidation_map[AUTOFILL].payload = "test2";
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&records2))));
+                      RecordSyncShare(NORMAL, &records2)));
   scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(1U, records2.snapshots.size());
-  EXPECT_THAT(invalidation_map, Eq(records2.snapshots[0].source().types));
-  EXPECT_EQ(GetUpdatesCallerInfo::NOTIFICATION,
-            records2.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, records2.sources.size());
+  EXPECT_EQ(NORMAL, records2.sources[0]);
 }
 
 // Test that polling works as expected.
@@ -573,7 +534,7 @@ TEST_F(SyncSchedulerTest, Polling) {
   TimeDelta poll_interval(TimeDelta::FromMilliseconds(30));
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples))));
+           RecordSyncShareMultiple(POLL, &records, kMinNumSamples)));
 
   scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
 
@@ -593,7 +554,7 @@ TEST_F(SyncSchedulerTest, PollNotificationsDisabled) {
   TimeDelta poll_interval(TimeDelta::FromMilliseconds(30));
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples))));
+           RecordSyncShareMultiple(POLL, &records, kMinNumSamples)));
 
   scheduler()->OnReceivedShortPollIntervalUpdate(poll_interval);
   scheduler()->SetNotificationsEnabled(false);
@@ -622,7 +583,7 @@ TEST_F(SyncSchedulerTest, PollIntervalUpdate) {
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
                 WithArg<1>(
-                    RecordSyncShareMultiple(&records, kMinNumSamples))));
+                    RecordSyncShareMultiple(POLL, &records, kMinNumSamples))));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll1 + poll2;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
@@ -669,9 +630,9 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   TimeDelta throttle(TimeDelta::FromMinutes(10));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(
-          WithArg<1>(sessions::test_util::SimulateThrottled(throttle)),
+          WithArg<2>(sessions::test_util::SimulateThrottled(throttle)),
           Return(true)))
       .WillRepeatedly(AddFailureAndQuitLoopNow());
 
@@ -707,7 +668,7 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromPoll) {
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples))));
+           RecordSyncShareMultiple(POLL, &records, kMinNumSamples)));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll + throttle1;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
@@ -754,12 +715,12 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromConfigure) {
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
   ::testing::InSequence seq;
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(
-          WithArg<1>(sessions::test_util::SimulateThrottled(throttle1)),
+          WithArg<2>(sessions::test_util::SimulateThrottled(throttle1)),
           Return(true)))
       .RetiresOnSaturation();
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
                       QuitLoopNowAction()));
 
@@ -836,12 +797,12 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) {
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                            WithArg<2>(RecordSyncShare(&records))));
+                            RecordSyncShare(NORMAL, &records)));
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
   scheduler()->ScheduleLocalNudge(zero(), throttled_types, FROM_HERE);
   PumpLoop();
-  EXPECT_EQ(0U, records.snapshots.size());
+  EXPECT_EQ(0U, records.sources.size());
   EXPECT_TRUE(GetThrottledTypes().HasAll(throttled_types));
 
   // Ignore invalidations for throttled types.
@@ -849,17 +810,17 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) {
       ModelTypeSetToInvalidationMap(throttled_types, "test");
   scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE);
   PumpLoop();
-  EXPECT_EQ(0U, records.snapshots.size());
+  EXPECT_EQ(0U, records.sources.size());
 
   // Ignore refresh requests for throttled types.
   scheduler()->ScheduleLocalRefreshRequest(zero(), throttled_types, FROM_HERE);
   PumpLoop();
-  EXPECT_EQ(0U, records.snapshots.size());
+  EXPECT_EQ(0U, records.sources.size());
 
   // Local nudges for non-throttled types will trigger a sync.
   scheduler()->ScheduleLocalNudge(zero(), unthrottled_types, FROM_HERE);
   RunLoop();
-  EXPECT_EQ(1U, records.snapshots.size());
+  EXPECT_EQ(1U, records.sources.size());
 
   StopSyncScheduler();
 }
@@ -869,9 +830,9 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   TimeDelta poll(TimeDelta::FromMilliseconds(15));
   SyncShareRecords records;
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      WithArg<1>(RecordSyncShare(&records))))
+                      RecordSyncShare(CONFIG, &records)))
       .RetiresOnSaturation();
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
@@ -891,10 +852,7 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(1, counter.times_called());
 
-  ASSERT_EQ(1U, records.snapshots.size());
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          config_types,
-          records.snapshots[0].source().types));
+  ASSERT_EQ(1U, records.sources.size());
 
   // Switch to NORMAL_MODE to ensure NUDGES were properly saved and run.
   // SyncSchedulerWhiteboxTest also provides coverage for this, but much
@@ -903,19 +861,15 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   SyncShareRecords records2;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      WithArg<2>(RecordSyncShare(&records2))));
+                      RecordSyncShare(NORMAL, &records2)));
 
   // TODO(tim): Figure out how to remove this dangerous need to reset
   // routing info between mode switches.
   context()->set_routing_info(routing_info());
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
-  ASSERT_EQ(1U, records2.snapshots.size());
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            records2.snapshots[0].source().updates_source);
-  EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap(
-          nudge_types,
-          records2.snapshots[0].source().types));
+  ASSERT_EQ(1U, records2.sources.size());
+  EXPECT_EQ(NORMAL, records2.sources[0]);
   PumpLoop();
 }
 
@@ -982,7 +936,7 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadTwice) {
 // Have the syncer fail to get the encryption key yet succeed in downloading
 // updates. Expect this will leave the scheduler in backoff.
 TEST_F(BackoffTriggersSyncSchedulerTest, FailGetEncryptionKey) {
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(
           Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed),
           Return(true)))
@@ -1014,7 +968,7 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                            WithArg<2>(RecordSyncShareMultiple(&r, 1U))));
+                            RecordSyncShareMultiple(NORMAL, &r, 1U)));
   EXPECT_CALL(*delay(), GetDelay(_)).
       WillRepeatedly(Return(TimeDelta::FromDays(1)));
 
@@ -1026,9 +980,8 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
   RunLoop();
 
   Mock::VerifyAndClearExpectations(syncer());
-  ASSERT_EQ(1U, r.snapshots.size());
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            r.snapshots[0].source().updates_source);
+  ASSERT_EQ(1U, r.sources.size());
+  EXPECT_EQ(NORMAL, r.sources[0]);
 
   // Wait a while (10x poll interval) so a few poll jobs will be attempted.
   PumpLoopFor(poll * 10);
@@ -1042,7 +995,7 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
   Mock::VerifyAndClearExpectations(syncer());
   Mock::VerifyAndClearExpectations(delay());
 
-  ASSERT_EQ(1U, r.snapshots.size());
+  ASSERT_EQ(1U, r.sources.size());
 
   EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
 
@@ -1065,7 +1018,7 @@ TEST_F(SyncSchedulerTest, BackoffElevation) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)).Times(kMinNumSamples)
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-          WithArg<2>(RecordSyncShareMultiple(&r, kMinNumSamples))));
+          RecordSyncShareMultiple(NORMAL, &r, kMinNumSamples)));
 
   const TimeDelta first = TimeDelta::FromSeconds(kInitialBackoffRetrySeconds);
   const TimeDelta second = TimeDelta::FromMilliseconds(2);
@@ -1090,7 +1043,7 @@ TEST_F(SyncSchedulerTest, BackoffElevation) {
   scheduler()->ScheduleLocalNudge(zero(), ModelTypeSet(BOOKMARKS), FROM_HERE);
   RunLoop();
 
-  ASSERT_EQ(kMinNumSamples, r.snapshots.size());
+  ASSERT_EQ(kMinNumSamples, r.sources.size());
   EXPECT_GE(r.times[1] - r.times[0], second);
   EXPECT_GE(r.times[2] - r.times[1], third);
   EXPECT_GE(r.times[3] - r.times[2], fourth);
@@ -1108,14 +1061,14 @@ TEST_F(SyncSchedulerTest, BackoffRelief) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                      WithArg<2>(RecordSyncShareMultiple(&r, kMinNumSamples))))
+                      RecordSyncShareMultiple(NORMAL, &r, kMinNumSamples)))
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulateNormalSuccess),
-              WithArg<2>(RecordSyncShareMultiple(&r, kMinNumSamples))));
+              RecordSyncShareMultiple(NORMAL, &r, kMinNumSamples)));
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulatePollSuccess),
-              WithArg<1>(RecordSyncShareMultiple(&r, kMinNumSamples))));
+              RecordSyncShareMultiple(POLL, &r, kMinNumSamples)));
   EXPECT_CALL(*delay(), GetDelay(_)).WillOnce(Return(backoff));
 
   // Optimal start for the post-backoff poll party.
@@ -1133,21 +1086,19 @@ TEST_F(SyncSchedulerTest, BackoffRelief) {
   // The first nudge ran as soon as possible.  It failed.
   TimeTicks optimal_job_time = optimal_start;
   EXPECT_GE(r.times[0], optimal_job_time);
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            r.snapshots[0].source().updates_source);
+  EXPECT_EQ(NORMAL, r.sources[0]);
 
   // It was followed by a successful retry nudge shortly afterward.
   optimal_job_time = optimal_job_time + backoff;
   EXPECT_GE(r.times[1], optimal_job_time);
-  EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
-            r.snapshots[1].source().updates_source);
+  EXPECT_EQ(NORMAL, r.sources[1]);
+
   // After that, we went back to polling.
-  for (size_t i = 2; i < r.snapshots.size(); i++) {
+  for (size_t i = 2; i < r.sources.size(); i++) {
     optimal_job_time = optimal_job_time + poll;
     SCOPED_TRACE(testing::Message() << "SyncShare # (" << i << ")");
     EXPECT_GE(r.times[i], optimal_job_time);
-    EXPECT_EQ(GetUpdatesCallerInfo::PERIODIC,
-              r.snapshots[i].source().updates_source);
+    EXPECT_EQ(POLL, r.sources[i]);
   }
 }
 
@@ -1161,9 +1112,9 @@ TEST_F(SyncSchedulerTest, TransientPollFailure) {
 
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollFailed),
-                      WithArg<1>(RecordSyncShare(&r))))
+                      RecordSyncShare(POLL, &r)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                      WithArg<1>(RecordSyncShare(&r))));
+                      RecordSyncShare(POLL, &r)));
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
@@ -1261,7 +1212,7 @@ TEST_F(SyncSchedulerTest, ConnectionChangeCanaryPreemptedByNudge) {
 // Tests that we don't crash trying to run two canaries at once if we receive
 // extra connection status change notifications.  See crbug.com/190085.
 TEST_F(SyncSchedulerTest, DoubleCanaryInConfigure) {
-  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_))
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulateConfigureConnectionFailure),
               Return(true)));
@@ -1292,7 +1243,7 @@ TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) {
   ::testing::InSequence seq;
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples))));
+           RecordSyncShareMultiple(POLL, &records, kMinNumSamples)));
 
   connection()->SetServerStatus(HttpResponse::SYNC_AUTH_ERROR);
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
@@ -1305,7 +1256,7 @@ TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) {
   // poll once more
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                      WithArg<1>(RecordSyncShare(&records))));
+                      RecordSyncShare(POLL, &records)));
   scheduler()->OnCredentialsUpdated();
   connection()->SetServerStatus(HttpResponse::SERVER_CONNECTION_OK);
   StopSyncScheduler();