[Sync] Sync types never recovers from throttle

components/sync/engine_impl/sync_scheduler_impl_unittest.cc

Index: components/sync/engine_impl/sync_scheduler_impl_unittest.cc
diff --git a/components/sync/engine_impl/sync_scheduler_impl_unittest.cc b/components/sync/engine_impl/sync_scheduler_impl_unittest.cc
index 0a770bb1936eb3cad5f2c2bfc95520f19d3ba306..da14eed1c1df41de5615098232dcdb7040c0fe02 100644
--- a/components/sync/engine_impl/sync_scheduler_impl_unittest.cc
+++ b/components/sync/engine_impl/sync_scheduler_impl_unittest.cc
@@ -280,6 +280,10 @@ class SyncSchedulerImplTest : public testing::Test {
     scheduler_->SetDefaultNudgeDelay(default_delay());
   }
 
+  bool BlockTimerIsRunning() const {
+    return scheduler_->pending_wakeup_timer_.IsRunning();
+  }
+
  private:
   syncable::Directory* directory() {
     return test_user_share_.user_share()->directory.get();
@@ -1002,6 +1006,7 @@ TEST_F(SyncSchedulerImplTest, TypeBackingOffAndThrottling) {
   PumpLoop();  // To get PerformDelayedNudge called.
   PumpLoop();  // To get TrySyncCycleJob called
   EXPECT_TRUE(GetBackedOffTypes().HasAll(types));
+  EXPECT_TRUE(BlockTimerIsRunning());
   EXPECT_FALSE(scheduler()->IsBackingOff());
   EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
 
@@ -1019,9 +1024,20 @@ TEST_F(SyncSchedulerImplTest, TypeBackingOffAndThrottling) {
   PumpLoop();  // To get TrySyncCycleJob called.
 
   EXPECT_TRUE(GetBackedOffTypes().HasAll(types));
+  EXPECT_TRUE(BlockTimerIsRunning());
   EXPECT_FALSE(scheduler()->IsBackingOff());
   EXPECT_TRUE(scheduler()->IsCurrentlyThrottled());
 
+  // Unthrottled client, but the backingoff datatype is still in backoff and
+  // scheduled.
+  EXPECT_CALL(*syncer(), NormalSyncShare(_, _, _))
+      .WillOnce(DoAll(Invoke(test_util::SimulateNormalSuccess),
+                      QuitLoopNowAction(true)));
+  RunLoop();
+  EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(types));
+  EXPECT_TRUE(BlockTimerIsRunning());
+
   StopSyncScheduler();
 }
 
@@ -1062,6 +1078,7 @@ TEST_F(SyncSchedulerImplTest, TypeThrottlingBackingOffBlocksNudge) {
 
   EXPECT_TRUE(GetThrottledTypes().HasAll(throttled_types));
   EXPECT_TRUE(GetBackedOffTypes().HasAll(backed_off_types));
+  EXPECT_TRUE(BlockTimerIsRunning());
   EXPECT_FALSE(scheduler()->IsBackingOff());
   EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
 
@@ -1760,4 +1777,126 @@ TEST_F(SyncSchedulerImplTest, PartialFailureWillExponentialBackoff) {
   StopSyncScheduler();
 }
 
+// If a datatype is in backoff or throttling, pending_wakeup_timer_ should
+// schedule a delay job for OnTypesUnblocked. SyncScheduler sometimes use
+// pending_wakeup_timer_ to schdule PerformDelayedNudge job before
+// OnTypesUnblocked got run. This test will verify after ran
+// PerformDelayedNudge, OnTypesUnblocked will be rescheduled if any datatype is
+// in backoff or throttling.
+TEST_F(SyncSchedulerImplTest, TypeBackoffAndSuccessfulSync) {
+  UseMockDelayProvider();
+  EXPECT_CALL(*delay(), GetDelay(_)).WillRepeatedly(Return(long_delay()));
+
+  TimeDelta poll(TimeDelta::FromDays(1));
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll);
+
+  const ModelTypeSet types(THEMES);
+
+  // Set backoff datatype.
+  ::testing::InSequence seq;
+  EXPECT_CALL(*syncer(), NormalSyncShare(_, _, _))
+      .WillOnce(DoAll(WithArg<2>(test_util::SimulatePartialFailure(types)),
+                      Return(true)))
+      .RetiresOnSaturation();
+
+  StartSyncScheduler(base::Time());
+  scheduler()->ScheduleLocalNudge(types, FROM_HERE);
+  PumpLoop();  // To get PerformDelayedNudge called.
+  PumpLoop();  // To get TrySyncCycleJob called
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(types));
+  EXPECT_TRUE(BlockTimerIsRunning());
+  EXPECT_FALSE(scheduler()->IsBackingOff());
+  EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
+
+  SyncShareTimes times;
+  EXPECT_CALL(*syncer(), NormalSyncShare(_, _, _))
+      .WillOnce(DoAll(Invoke(test_util::SimulateNormalSuccess),
+                      RecordSyncShare(&times, true)))
+      .RetiresOnSaturation();
+
+  // Do a successful Sync.
+  const ModelTypeSet unbacked_off_types(TYPED_URLS);
+  scheduler()->ScheduleLocalNudge(unbacked_off_types, FROM_HERE);
+  PumpLoop();  // TO get PerformDelayedNudge called.
+  PumpLoop();  // To get TrySyncCycleJob called.
+
+  // Timer is still running for backoff datatype after Sync success.
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(types));
+  EXPECT_TRUE(BlockTimerIsRunning());
+  EXPECT_FALSE(scheduler()->IsBackingOff());
+  EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
+
+  StopSyncScheduler();
+}
+
+// Verify that the timer is scheduled for an unblock job after one datatype is
+// unblocked, and there is another one still blocked.
+TEST_F(SyncSchedulerImplTest, TypeBackingOffAndFailureSync) {
+  UseMockDelayProvider();
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillOnce(Return(long_delay()))
+      .RetiresOnSaturation();
+
+  TimeDelta poll(TimeDelta::FromDays(1));
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll);
+
+  // Set a backoff datatype.
+  const ModelTypeSet themes_types(THEMES);
+  ::testing::InSequence seq;
+  EXPECT_CALL(*syncer(), NormalSyncShare(_, _, _))
+      .WillOnce(
+          DoAll(WithArg<2>(test_util::SimulatePartialFailure(themes_types)),
+                Return(true)))
+      .RetiresOnSaturation();
+
+  StartSyncScheduler(base::Time());
+  scheduler()->ScheduleLocalNudge(themes_types, FROM_HERE);
+  PumpLoop();  // To get PerformDelayedNudge called.
+  PumpLoop();  // To get TrySyncCycleJob called
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(themes_types));
+  EXPECT_TRUE(BlockTimerIsRunning());
+  EXPECT_FALSE(scheduler()->IsBackingOff());
+  EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
+
+  // Set anther backoff datatype.
+  const ModelTypeSet typed_urls_types(TYPED_URLS);
+  EXPECT_CALL(*syncer(), NormalSyncShare(_, _, _))
+      .WillOnce(
+          DoAll(WithArg<2>(test_util::SimulatePartialFailure(typed_urls_types)),
+                Return(true)))
+      .RetiresOnSaturation();
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillOnce(Return(default_delay()))
+      .RetiresOnSaturation();
+
+  scheduler()->ScheduleLocalNudge(typed_urls_types, FROM_HERE);
+  PumpLoop();  // TO get PerformDelayedNudge called.
+  PumpLoop();  // To get TrySyncCycleJob called.
+
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(themes_types));
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(typed_urls_types));
+  EXPECT_TRUE(BlockTimerIsRunning());
+  EXPECT_FALSE(scheduler()->IsBackingOff());
+  EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
+
+  // Unblock one datatype.
+  SyncShareTimes times;
+  EXPECT_CALL(*syncer(), NormalSyncShare(_, _, _))
+      .WillRepeatedly(DoAll(Invoke(test_util::SimulateNormalSuccess),
+                            RecordSyncShare(&times, true)));
+  EXPECT_CALL(*delay(), GetDelay(_)).WillRepeatedly(Return(long_delay()));
+
+  PumpLoop();  // TO get OnTypesUnblocked called.
+  PumpLoop();  // To get TrySyncCycleJob called.
+
+  // Timer is still scheduled for another backoff datatype.
+  EXPECT_TRUE(GetBackedOffTypes().HasAll(themes_types));
+  EXPECT_FALSE(GetBackedOffTypes().HasAll(typed_urls_types));
+  EXPECT_TRUE(BlockTimerIsRunning());
+  EXPECT_FALSE(scheduler()->IsBackingOff());
+  EXPECT_FALSE(scheduler()->IsCurrentlyThrottled());
+
+  StopSyncScheduler();
+}
+
 }  // namespace syncer