Revert 148792 - [Sync] Remove CleanupDisabledTypes command and move purge logic into SyncManager.

We were only ever performing a meaningful cleanup on reconfigurations
or restart, so we make that explicit by purging from within the SyncManager's
loading and configuration methods.

BUG=131433, 90868
TEST=manual

Review URL: https://chromiumcodereview.appspot.com/10541079

TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10823061

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148812 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 61 insertions, 5 deletions

diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 8072566..ff4787b 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -300,7 +300,9 @@ TEST_F(SyncSchedulerTest, Config) {
   SyncShareRecords records;
   const ModelTypeSet model_types(BOOKMARKS);
 
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+  EXPECT_CALL(*syncer(),
+              SyncShare(_,_,_))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))));
 
@@ -331,7 +333,9 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   SyncShareRecords records;
   const ModelTypeSet model_types(BOOKMARKS);
 
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+  EXPECT_CALL(*syncer(),
+              SyncShare(_,_,_))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                       WithArg<0>(RecordSyncShare(&records))))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
@@ -705,14 +709,19 @@ TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
-// Test that no syncing occurs when throttled.
+// Test that no syncing occurs when throttled (although CleanupDisabledTypes
+// is allowed).
 TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   const ModelTypeSet types(BOOKMARKS);
   TimeDelta poll(TimeDelta::FromMilliseconds(5));
   TimeDelta throttle(TimeDelta::FromMinutes(10));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+  EXPECT_CALL(*syncer(),
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+  EXPECT_CALL(*syncer(), SyncShare(_,Not(CLEANUP_DISABLED_TYPES),
+                                     Not(CLEANUP_DISABLED_TYPES)))
       .WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle)))
       .WillRepeatedly(AddFailureAndQuitLoopNow());
 
@@ -765,6 +774,7 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   SyncShareRecords records;
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))));
 
@@ -889,6 +899,10 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
   EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
             r.snapshots[1].source().updates_source);
 
+  // Cleanup is not affected by backoff, but it should not relieve it either.
+  EXPECT_CALL(*syncer(),
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
@@ -1062,7 +1076,10 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
-  // Configuration.
+  // Configuration (always includes a cleanup disabled types).
+  EXPECT_CALL(*syncer(),
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
       .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
@@ -1081,6 +1098,25 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
+  // Cleanup disabled types. Because no types are being configured, we just
+  // perform the cleanup.
+  EXPECT_CALL(*syncer(),
+              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES)).
+      WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+
+  CallbackCounter counter2;
+  ConfigurationParams params2(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      ModelTypeSet(),
+      ModelSafeRoutingInfo(),
+      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter2)));
+  ASSERT_TRUE(scheduler()->ScheduleConfiguration(params2));
+  ASSERT_EQ(1, counter2.times_called());
+  StopSyncScheduler();
+  Mock::VerifyAndClearExpectations(syncer());
+
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Poll.
@@ -1126,4 +1162,24 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   MessageLoop::current()->RunAllPending();
 }
 
+TEST_F(SyncSchedulerTest, SetsPreviousRoutingInfo) {
+  ModelSafeRoutingInfo info;
+  EXPECT_TRUE(info == context()->previous_session_routing_info());
+  ModelSafeRoutingInfo expected(context()->routing_info());
+  ASSERT_FALSE(expected.empty());
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1);
+
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+
+  scheduler()->ScheduleNudgeAsync(
+      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
+  PumpLoop();
+  // Pump again to run job.
+  PumpLoop();
+
+  StopSyncScheduler();
+
+  EXPECT_TRUE(expected == context()->previous_session_routing_info());
+}
+
 }  // namespace syncer