Client-side changes to support retry GU.

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/message_loop.h"
 #include "base/test/test_timeouts.h"
@@ skipping 16 matching lines @@
 using base::TimeDelta;
 using base::TimeTicks;
 using testing::_;
 using testing::AtLeast;
 using testing::DoAll;
 using testing::Invoke;
 using testing::Mock;
 using testing::Return;
 using testing::WithArg;
 using testing::WithArgs;
+using testing::WithoutArgs;
 
 namespace syncer {
 using sessions::SyncSession;
 using sessions::SyncSessionContext;
 using sync_pb::GetUpdatesCallerInfo;
 
 class MockSyncer : public Syncer {
  public:
   MockSyncer();
   MOCK_METHOD3(NormalSyncShare, bool(ModelTypeSet,
                                      const sessions::NudgeTracker&,
                                      sessions::SyncSession*));
   MOCK_METHOD3(ConfigureSyncShare,
                bool(ModelTypeSet,
                     sync_pb::GetUpdatesCallerInfo::GetUpdatesSource,
                     SyncSession*));
   MOCK_METHOD2(PollSyncShare, bool(ModelTypeSet, sessions::SyncSession*));
+  MOCK_METHOD2(RetrySyncShare, bool(ModelTypeSet, sessions::SyncSession*));
 };
 
 MockSyncer::MockSyncer()
   : Syncer(NULL) {}
 
 typedef std::vector<TimeTicks> SyncShareTimes;
 
 void QuitLoopNow() {
   // We use QuitNow() instead of Quit() as the latter may get stalled
   // indefinitely in the presence of repeated timers with low delays
@@ skipping 145 matching lines @@
     delay_ = new MockDelayProvider();
     scheduler_->delay_provider_.reset(delay_);
   }
 
   SyncSessionContext* context() { return context_.get(); }
 
   ModelTypeSet GetThrottledTypes() {
     return scheduler_->nudge_tracker_.GetThrottledTypes();
   }
 
+  base::TimeDelta GetRetryTimerDelay() {
+    EXPECT_TRUE(scheduler_->retry_timer_.IsRunning());
+    return scheduler_->retry_timer_.GetCurrentDelay();

[rlarocque, 2014/01/10 22:41:13]

This seems like a grey area in the API.  I'm not sure that it's safe to call
GetCurrentDelay() on the timer if the timer has already fired.  Though looking
at the implementation, I see no reason why it wouldn't work.

Since these checks are not really essential to your tests, I guess this is OK. 
You could always remove them if this breaks.

+  }
+
  private:
   syncable::Directory* directory() {
     return dir_maker_.directory();
   }
 
   base::MessageLoop loop_;
   TestDirectorySetterUpper dir_maker_;
   CancelationSignal cancelation_signal_;
   scoped_ptr<MockConnectionManager> connection_;
   scoped_ptr<SyncSessionContext> context_;
@@ skipping 309 matching lines @@
                       RecordSyncShare(&times2)));
   scheduler()->ScheduleInvalidationNudge(zero(), invalidations2, FROM_HERE);
   RunLoop();
 }
 
 // Test that polling works as expected.
 TEST_F(SyncSchedulerTest, Polling) {
   SyncShareTimes times;
   TimeDelta poll_interval(TimeDelta::FromMilliseconds(30));
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
-      .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           RecordSyncShareMultiple(&times, kMinNumSamples)));
+      .WillRepeatedly(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                RecordSyncShareMultiple(&times, kMinNumSamples)));
 
   scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
 
   TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
 
   StopSyncScheduler();
   AnalyzePollRun(times, kMinNumSamples, optimal_start, poll_interval);
 }
 
 // Test that the short poll interval is used.
 TEST_F(SyncSchedulerTest, PollNotificationsDisabled) {
   SyncShareTimes times;
   TimeDelta poll_interval(TimeDelta::FromMilliseconds(30));
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
-      .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           RecordSyncShareMultiple(&times, kMinNumSamples)));
+      .WillRepeatedly(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                RecordSyncShareMultiple(&times, kMinNumSamples)));
 
   scheduler()->OnReceivedShortPollIntervalUpdate(poll_interval);
   scheduler()->SetNotificationsEnabled(false);
 
   TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
 
   StopSyncScheduler();
   AnalyzePollRun(times, kMinNumSamples, optimal_start, poll_interval);
 }
 
 // Test that polling intervals are updated when needed.
 TEST_F(SyncSchedulerTest, PollIntervalUpdate) {
   SyncShareTimes times;
   TimeDelta poll1(TimeDelta::FromMilliseconds(120));
   TimeDelta poll2(TimeDelta::FromMilliseconds(30));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll1);
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
       .WillOnce(DoAll(
           WithArgs<0,1>(
               sessions::test_util::SimulatePollIntervalUpdate(poll2)),
           Return(true)))
       .WillRepeatedly(
-          DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
                 WithArg<1>(
                     RecordSyncShareMultiple(&times, kMinNumSamples))));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll1 + poll2;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
 
   StopSyncScheduler();
@@ skipping 70 matching lines @@
   TimeDelta throttle1(TimeDelta::FromMilliseconds(150));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
   ::testing::InSequence seq;
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillOnce(DoAll(
           WithArg<1>(sessions::test_util::SimulateThrottled(throttle1)),
           Return(true)))
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
-      .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           RecordSyncShareMultiple(&times, kMinNumSamples)));
+      .WillRepeatedly(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                RecordSyncShareMultiple(&times, kMinNumSamples)));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll + throttle1;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
 
   StopSyncScheduler();
   AnalyzePollRun(times, kMinNumSamples, optimal_start, poll);
 }
@@ skipping 411 matching lines @@
               RecordSyncShare(&times)));
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
   optimal_job_time = optimal_job_time + backoff;
   ASSERT_EQ(2U, times.size());
   EXPECT_GE(times[1], optimal_job_time);
 
   // Now let the Poll timer do its thing.
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(DoAll(
-              Invoke(sessions::test_util::SimulatePollSuccess),
+              Invoke(sessions::test_util::SimulatePollRetrySuccess),
               RecordSyncShareMultiple(&times, kMinNumSamples)));
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
   ASSERT_EQ(kMinNumSamples, times.size());
   for (size_t i = 2; i < times.size(); i++) {
     optimal_job_time = optimal_job_time + poll;
     SCOPED_TRACE(testing::Message() << "SyncShare # (" << i << ")");
     EXPECT_GE(times[i], optimal_job_time);
   }
 
   StopSyncScheduler();
 }
 
 // Test that poll failures are ignored.  They should have no effect on
 // subsequent poll attempts, nor should they trigger a backoff/retry.
 TEST_F(SyncSchedulerTest, TransientPollFailure) {
   SyncShareTimes times;
   const TimeDelta poll_interval(TimeDelta::FromMilliseconds(1));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
   UseMockDelayProvider(); // Will cause test failure if backoff is initiated.
 
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
-      .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollFailed),
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollRetryFailed),
                       RecordSyncShare(&times)))
-      .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
                       RecordSyncShare(&times)));
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run the unsucessful poll. The failed poll should not trigger backoff.
   RunLoop();
   EXPECT_FALSE(scheduler()->IsBackingOff());
 
   // Run the successful poll.
   RunLoop();
@@ skipping 112 matching lines @@
   PumpLoop();  // Run the nudge, that will fail and schedule a quick retry.
 }
 
 TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) {
   SyncShareTimes times;
   TimeDelta poll(TimeDelta::FromMilliseconds(15));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
   ::testing::InSequence seq;
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
-      .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-           RecordSyncShareMultiple(&times, kMinNumSamples)));
+      .WillRepeatedly(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                RecordSyncShareMultiple(&times, kMinNumSamples)));
 
   connection()->SetServerStatus(HttpResponse::SYNC_AUTH_ERROR);
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run to wait for polling.
   RunLoop();
 
   // Normally OnCredentialsUpdated calls TryCanaryJob that doesn't run Poll,
   // but after poll finished with auth error from poll timer it should retry
   // poll once more
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
-      .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
                       RecordSyncShare(&times)));
   scheduler()->OnCredentialsUpdated();
   connection()->SetServerStatus(HttpResponse::SERVER_CONNECTION_OK);
   RunLoop();
   StopSyncScheduler();
 }
 
+TEST_F(SyncSchedulerTest, SuccessfulRetry) {
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+
+  SyncShareTimes times;
+  base::TimeDelta delay = base::TimeDelta::FromMilliseconds(1);
+  scheduler()->OnReceivedGuRetryDelay(delay);
+  EXPECT_EQ(delay, GetRetryTimerDelay());
+
+  EXPECT_CALL(*syncer(), RetrySyncShare(_,_))
+      .WillOnce(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                RecordSyncShare(&times)));
+
+  // Run to wait for retrying.
+  RunLoop();
+
+  StopSyncScheduler();
+}
+
+TEST_F(SyncSchedulerTest, FailedRetry) {
+  UseMockDelayProvider();
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillRepeatedly(Return(TimeDelta::FromMilliseconds(1)));
+
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+
+  base::TimeDelta delay = base::TimeDelta::FromMilliseconds(1);
+  scheduler()->OnReceivedGuRetryDelay(delay);
+
+  EXPECT_CALL(*syncer(), RetrySyncShare(_,_))
+      .WillOnce(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetryFailed),
+                QuitLoopNowAction()));
+
+  // Run to wait for retrying.
+  RunLoop();
+
+  EXPECT_TRUE(scheduler()->IsBackingOff());
+  EXPECT_CALL(*syncer(), RetrySyncShare(_,_))
+      .WillOnce(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                QuitLoopNowAction()));
+
+  // Run to wait for second retrying.
+  RunLoop();
+
+  StopSyncScheduler();
+}
+
+ACTION_P2(VerifyRetryTimerDelay, scheduler_test, expected_delay) {
+  EXPECT_EQ(expected_delay, scheduler_test->GetRetryTimerDelay());
+}
+
+TEST_F(SyncSchedulerTest, ReceiveNewRetryDelay) {
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+
+  SyncShareTimes times;
+  base::TimeDelta delay1 = base::TimeDelta::FromMilliseconds(10);
+  base::TimeDelta delay2 = base::TimeDelta::FromMilliseconds(20);
+
+  scheduler()->OnReceivedGuRetryDelay(delay1);
+  scheduler()->ScheduleLocalRefreshRequest(zero(), ModelTypeSet(BOOKMARKS),
+                                           FROM_HERE);
+
+  EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
+      .WillOnce(DoAll(
+          WithoutArgs(VerifyRetryTimerDelay(this, delay1)),

[rlarocque, 2014/01/10 22:41:13]

nit: Maybe put this into an expectation just before the sync cycle is allowed to
run (ie. just before line 1382).

[haitaol1, 2014/01/10 23:45:03]

I'd like to verify pre/post conditions tightly around NormalSyncShare to avoid
interference of other code.

[rlarocque, 2014/01/10 23:47:59]

That's valid.  I suggested alternatives because:
- I dislike GMock, and would prefer to rely on it as little as possible.
- Seeing the retry timer get verified against two different values within two
lines of each other looks a bit suspicious.

Neither argument is very strong.  You can leave it as is if you feel strongly
about it.

+          WithArg<2>(sessions::test_util::SimulateGuRetryDelayCommand(delay2)),
+          WithoutArgs(VerifyRetryTimerDelay(this, delay2)),

[rlarocque, 2014/01/10 22:41:13]

nit: I think it would be more clear to perform this second timer delay
verification a few lines below.  (ie. just after line 1382).

+          RecordSyncShare(&times)));
+
+  // Run nudge GU.
+  RunLoop();
+
+  EXPECT_CALL(*syncer(), RetrySyncShare(_,_))
+      .WillOnce(
+          DoAll(Invoke(sessions::test_util::SimulatePollRetrySuccess),
+                RecordSyncShare(&times)));
+
+  // Run to wait for retrying.
+  RunLoop();
+
+  StopSyncScheduler();
+}
+
 }  // namespace syncer