sync: remove buggy freshness condition in SyncSchedulerImpl add defensive DCHECKs.

sync/engine/sync_scheduler_unittest.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/test/test_timeouts.h"
@@ skipping 86 matching lines @@
     }
 
     MOCK_METHOD1(GetDelay, TimeDelta(const TimeDelta&));
   };
 
   virtual void SetUp() {
     dir_maker_.SetUp();
     syncer_ = new MockSyncer();
     delay_ = NULL;
 
-    ModelSafeRoutingInfo routing_info;
-    routing_info[BOOKMARKS] = GROUP_UI;
-    routing_info[AUTOFILL] = GROUP_DB;
-    routing_info[THEMES] = GROUP_UI;
-    routing_info[NIGORI] = GROUP_PASSIVE;
+    routing_info_[BOOKMARKS] = GROUP_UI;
+    routing_info_[AUTOFILL] = GROUP_DB;
+    routing_info_[THEMES] = GROUP_UI;
+    routing_info_[NIGORI] = GROUP_PASSIVE;
 
     workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
     workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_DB)));
     workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE)));
 
     std::vector<ModelSafeWorker*> workers;
     for (std::vector<scoped_refptr<FakeModelWorker> >::iterator it =
          workers_.begin(); it != workers_.end(); ++it) {
       workers.push_back(it->get());
     }
 
     connection_.reset(new MockConnectionManager(directory()));
     connection_->SetServerReachable();
     throttled_data_type_tracker_.reset(new ThrottledDataTypeTracker(NULL));
     context_.reset(new SyncSessionContext(
             connection_.get(), directory(), workers,
             &extensions_activity_monitor_, throttled_data_type_tracker_.get(),
             std::vector<SyncEngineEventListener*>(), NULL, NULL,
             true  /* enable keystore encryption */));
-    context_->set_routing_info(routing_info);
+    context_->set_routing_info(routing_info_);
     context_->set_notifications_enabled(true);
     context_->set_account_name("Test");
     scheduler_.reset(
         new SyncSchedulerImpl("TestSyncScheduler",
             BackoffDelayProvider::FromDefaults(),
             context(),
             syncer_));
   }
 
   SyncSchedulerImpl* scheduler() { return scheduler_.get(); }
+  const ModelSafeRoutingInfo& routing_info() { return routing_info_; }
   MockSyncer* syncer() { return syncer_; }
   MockDelayProvider* delay() { return delay_; }
   MockConnectionManager* connection() { return connection_.get(); }
   TimeDelta zero() { return TimeDelta::FromSeconds(0); }
   TimeDelta timeout() {
     return TestTimeouts::action_timeout();
   }
 
   virtual void TearDown() {
     PumpLoop();
@@ skipping 24 matching lines @@
   }
 
   // This stops the scheduler synchronously.
   void StopSyncScheduler() {
     scheduler()->RequestStop(base::Bind(&SyncSchedulerTest::DoQuitLoopNow,
                              weak_ptr_factory_.GetWeakPtr()));
     RunLoop();
   }
 
   bool RunAndGetBackoff() {
-    ModelTypeSet nudge_types;
+    ModelTypeSet nudge_types(BOOKMARKS);
     StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
     scheduler()->ScheduleNudgeAsync(
         zero(), NUDGE_SOURCE_LOCAL, nudge_types, FROM_HERE);
     RunLoop();
 
     return scheduler()->IsBackingOff();
   }
 
   void UseMockDelayProvider() {
@@ skipping 28 matching lines @@
   MessageLoop message_loop_;
   TestDirectorySetterUpper dir_maker_;
   scoped_ptr<MockConnectionManager> connection_;
   scoped_ptr<SyncSessionContext> context_;
   scoped_ptr<SyncSchedulerImpl> scheduler_;
   MockSyncer* syncer_;
   MockDelayProvider* delay_;
   std::vector<scoped_refptr<FakeModelWorker> > workers_;
   FakeExtensionsActivityMonitor extensions_activity_monitor_;
   scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_;
+  ModelSafeRoutingInfo routing_info_;
 };
 
 void RecordSyncShareImpl(SyncSession* s, SyncShareRecords* record) {
   record->times.push_back(TimeTicks::Now());
   record->snapshots.push_back(s->TakeSnapshot());
 }
 
 ACTION_P(RecordSyncShare, record) {
   RecordSyncShareImpl(arg0, record);
   if (MessageLoop::current()->is_running())
@@ skipping 153 matching lines @@
       model_types,
       TypesToRoutingInfo(model_types),
       base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
   ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(0, counter.times_called());
   ASSERT_EQ(1U, records.snapshots.size());
 
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, 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(0, counter.times_called());
+  EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
+            records.snapshots[1].source().updates_source);
 
   RunLoop();
+  // This is the 3rd attempt, which we've set up to SimulateSuccess.
   ASSERT_EQ(3U, records.snapshots.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(CompareModelTypeSetToModelTypePayloadMap(model_types,
       records.snapshots[2].source().types));
@@ skipping 263 matching lines @@
 
 // Test that a sync session is run through to completion.
 TEST_F(SyncSchedulerTest, HasMoreToSync) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(Invoke(sessions::test_util::SimulateHasMoreToSync))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       QuitLoopNowAction()));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
+      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   RunLoop();
   // If more nudges are scheduled, they'll be waited on by TearDown, and would
   // cause our expectation to break.
 }
 
 // Test that continuations can go into backoff.
 TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
     .WillOnce(Invoke(sessions::test_util::SimulateHasMoreToSync))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                     QuitLoopNowAction()));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
+      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
 
   // We should detect the failure on the second sync share, and go into backoff.
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
 // Test that no syncing occurs when throttled.
 TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   const ModelTypeSet types(BOOKMARKS);
   TimeDelta poll(TimeDelta::FromMilliseconds(5));
   TimeDelta throttle(TimeDelta::FromMinutes(10));
@@ skipping 45 matching lines @@
   AnalyzePollRun(records, kMinNumSamples, optimal_start, poll);
 }
 
 // Test nudges / polls don't run in config mode and config tasks do.
 TEST_F(SyncSchedulerTest, ConfigurationMode) {
   TimeDelta poll(TimeDelta::FromMilliseconds(15));
   SyncShareRecords records;
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
-                      WithArg<0>(RecordSyncShare(&records))));
+                      WithArg<0>(RecordSyncShare(&records))))
+      .RetiresOnSaturation();
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
   const ModelTypeSet nudge_types(AUTOFILL);
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, nudge_types, FROM_HERE);
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, nudge_types, FROM_HERE);
 
   const ModelTypeSet config_types(BOOKMARKS);
 
   CallbackCounter counter;
   ConfigurationParams params(
       GetUpdatesCallerInfo::RECONFIGURATION,
       config_types,
       TypesToRoutingInfo(config_types),
       base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
   ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(1, counter.times_called());
 
   ASSERT_EQ(1U, records.snapshots.size());
   EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(config_types,
       records.snapshots[0].source().types));
+
+  // Switch to NORMAL_MODE to ensure NUDGES were properly saved and run.
+  // SyncSchedulerWhiteboxTest also tests this, but here, we also wind up
+  // up testing that the freshness condition check doesn't cancel nudges
+  // that were scheduled to start before the config finished.
+  scheduler()->OnReceivedLongPollIntervalUpdate(TimeDelta::FromDays(1));
+  SyncShareRecords records2;
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+                      WithArg<0>(RecordSyncShare(&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(CompareModelTypeSetToModelTypePayloadMap(nudge_types,
+      records2.snapshots[0].source().types));
+  PumpLoop();
 }
 
 class BackoffTriggersSyncSchedulerTest : public SyncSchedulerTest {
   void SetUp() {
     SyncSchedulerTest::SetUp();
     UseMockDelayProvider();
     EXPECT_CALL(*delay(), GetDelay(_))
         .WillRepeatedly(Return(TimeDelta::FromMilliseconds(1)));
   }
 
@@ skipping 101 matching lines @@
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
   CallbackCounter counter;
   ConfigurationParams params(
       GetUpdatesCallerInfo::RECONFIGURATION,
       types,
       TypesToRoutingInfo(types),
       base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
   ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(0, counter.times_called());
-
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
-
-  scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, types, FROM_HERE);
-  scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, types, FROM_HERE);
-  PumpLoop();
 }
 
 // Test that backoff is shaping traffic properly with consecutive errors.
 TEST_F(SyncSchedulerTest, BackoffElevation) {
   SyncShareRecords r;
   UseMockDelayProvider();
 
   EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(kMinNumSamples)
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
           RecordSyncShareMultiple(&r, kMinNumSamples)));
@@ skipping 12 matching lines @@
   EXPECT_CALL(*delay(), GetDelay(Eq(third))).WillOnce(Return(fourth))
           .RetiresOnSaturation();
   EXPECT_CALL(*delay(), GetDelay(Eq(fourth))).WillOnce(Return(fifth))
           .RetiresOnSaturation();
   EXPECT_CALL(*delay(), GetDelay(Eq(fifth))).WillOnce(Return(sixth));
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again with a nudge.
   scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
+      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   RunLoop();
 
   ASSERT_EQ(kMinNumSamples, r.snapshots.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);
   EXPECT_GE(r.times[4] - r.times[3], fifth);
 }
 
 // Test that things go back to normal once a retry makes forward progress.
@@ skipping 11 matching lines @@
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                             RecordSyncShareMultiple(&r, kMinNumSamples)));
   EXPECT_CALL(*delay(), GetDelay(_)).WillOnce(Return(backoff));
 
   // Optimal start for the post-backoff poll party.
   TimeTicks optimal_start = TimeTicks::Now();
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   scheduler()->ScheduleNudgeAsync(zero(), NUDGE_SOURCE_LOCAL,
-                                  ModelTypeSet(), FROM_HERE);
+                                  ModelTypeSet(BOOKMARKS), FROM_HERE);
   RunLoop();
 
   StopSyncScheduler();
 
   EXPECT_EQ(kMinNumSamples, r.times.size());
 
   // 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,
@@ skipping 40 matching lines @@
 }
 
 // Test that appropriate syncer steps are requested for each job type.
 TEST_F(SyncSchedulerTest, SyncerSteps) {
   // Nudges.
   EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END))
       .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
+      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   PumpLoop();
   // Pump again to run job.
   PumpLoop();
 
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
   // Configuration.
   EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
       .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
@@ skipping 24 matching lines @@
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
 
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 }
 
-// Test config tasks don't run during normal mode.
-// TODO(tim): Implement this test and then the functionality!
-TEST_F(SyncSchedulerTest, DISABLED_NoConfigDuringNormal) {
-}
-
 // Test that starting the syncer thread without a valid connection doesn't
 // break things when a connection is detected.
 TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   connection()->SetServerNotReachable();
   connection()->UpdateConnectionStatus();
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
     .WillOnce(Invoke(sessions::test_util::SimulateConnectionFailure))
     .WillOnce(QuitLoopNowAction());
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
+      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   // Should save the nudge for until after the server is reachable.
   MessageLoop::current()->RunAllPending();
 
   connection()->SetServerReachable();
   connection()->UpdateConnectionStatus();
   scheduler()->OnConnectionStatusChange();
   MessageLoop::current()->RunAllPending();
 }
 
 }  // namespace syncer