sync: Fix handling of data type throttling

This change attempts to fix two semi-related issues in data type
throttling that were introduced in r194766.  Data type throttling has
always had a few rough edge cases, but that commit introduced some
issues that must be fixed.

1:
Prior to r194766, data type throttling would block sync cycles only if
the source was LOCAL and all types were throttled. This meant that a
refresh request or notification could override data type throttling, if
it arrived at the right time.  

That change made this behaviour much more deterministic.  It would fail
to perform any sync cycles as long as the only local nudges that were
currently tracked were for throttled types.  Even notificiations and
local refresh requests for these types will not be sufficient to force a
sync cycle.

This change tries to find a better balance.  It will allow sync cycles
in the presence of unthrottled data types if one of the following is
true:
- There is an unserviced invalidation for any type.
- There is an unservice refresh request for any type.
- There is a local nudge for any non-throttled type.

2:
The other problem has to do with exponential backoff in the presence of
per-type throttling.  In r194766, we made the assumption that we can
early-exit from attempted sync cycle only if there was some other reason
to perform a sync cycle later.  Common causes of early-exit include
throttling, exponential backoff, and network connectivity issues, all of
which should be able to get back into a normal state on their own.

However, one of those early-exit causes is data type throttling.  In
that case, there is no timer or notification that will restart sync once
the issue is resolved.  This is a problem because DoNudgeSyncSessionJob
assumes that it can return without scheduling a retry in the "if
(!CanRunNudgeJobNow())" case, while the code that manages the
exponential backoff assumes the function that invokes
DoNudgeSyncSessionJob, TryCanaryJob(), will always either clear or
restart the wait interval.

To fix this issue, this change adds a small function to invoke the
exponential backoff retry callback, and gives it responsibility for
making sure the WaitInterval is always managed correctly.

BUG=239693

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

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

1 files changed, 87 insertions, 0 deletions

diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 21e1430..3820102 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -225,6 +225,10 @@ class SyncSchedulerTest : public testing::Test {
 
   SyncSessionContext* context() { return context_.get(); }
 
+  ThrottledDataTypeTracker* throttled_data_type_tracker() {
+    return throttled_data_type_tracker_.get();
+  }
+
  private:
   syncable::Directory* directory() {
     return dir_maker_.directory();
@@ -778,6 +782,89 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromConfigure) {
   StopSyncScheduler();
 }
 
+TEST_F(SyncSchedulerTest, TypeThrottlingBlocksNudge) {
+  UseMockDelayProvider();
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillRepeatedly(Return(zero()));
+
+  TimeDelta poll(TimeDelta::FromDays(1));
+  TimeDelta throttle1(TimeDelta::FromSeconds(60));
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll);
+
+  const ModelTypeSet types(BOOKMARKS);
+
+  ::testing::InSequence seq;
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillOnce(DoAll(
+          WithArg<0>(
+              sessions::test_util::SimulateTypesThrottled(types, throttle1)),
+          Return(true)))
+      .RetiresOnSaturation();
+
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  scheduler()->ScheduleLocalNudge(zero(), types, FROM_HERE);
+  PumpLoop();
+  EXPECT_TRUE(throttled_data_type_tracker()->GetThrottledTypes().HasAll(types));
+
+  // This won't cause a sync cycle because the types are throttled.
+  scheduler()->ScheduleLocalNudge(zero(), types, FROM_HERE);
+  PumpLoop();
+
+  StopSyncScheduler();
+}
+
+TEST_F(SyncSchedulerTest, TypeThrottlingDoesntBlockOtherSources) {
+  UseMockDelayProvider();
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillRepeatedly(Return(zero()));
+
+  SyncShareRecords records;
+  TimeDelta poll(TimeDelta::FromDays(1));
+  TimeDelta throttle1(TimeDelta::FromSeconds(60));
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll);
+
+  const ModelTypeSet throttled_types(BOOKMARKS);
+  const ModelTypeSet unthrottled_types(PREFERENCES);
+
+  ::testing::InSequence seq;
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillOnce(DoAll(
+          WithArg<0>(
+              sessions::test_util::SimulateTypesThrottled(
+                  throttled_types, throttle1)),
+          Return(true)))
+      .RetiresOnSaturation();
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+                            WithArg<0>(RecordSyncShare(&records))));
+
+  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  scheduler()->ScheduleLocalNudge(zero(), throttled_types, FROM_HERE);
+  PumpLoop();
+  EXPECT_EQ(0U, records.snapshots.size());
+  EXPECT_TRUE(throttled_data_type_tracker()->GetThrottledTypes().HasAll(
+          throttled_types));
+
+  // This invalidaiton will cause a sync even though the types are throttled.
+  ModelTypeInvalidationMap invalidation_map =
+      ModelTypeSetToInvalidationMap(throttled_types, "test");
+  scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE);
+  RunLoop();
+  EXPECT_EQ(1U, records.snapshots.size());
+
+  // Refresh requests will cause a sync, too.
+  scheduler()->ScheduleLocalRefreshRequest(zero(), throttled_types, FROM_HERE);
+  RunLoop();
+  EXPECT_EQ(2U, records.snapshots.size());
+
+  // Even local nudges for other data types will trigger a sync.
+  scheduler()->ScheduleLocalNudge(zero(), unthrottled_types, FROM_HERE);
+  RunLoop();
+  EXPECT_EQ(3U, records.snapshots.size());
+
+  StopSyncScheduler();
+}
+
 // Test nudges / polls don't run in config mode and config tasks do.
 TEST_F(SyncSchedulerTest, ConfigurationMode) {
   TimeDelta poll(TimeDelta::FromMilliseconds(15));