Channel all low lever scheduling calls through single function

The idea of this change is to only call Do***SyncSessionJob from single
function TryJob and have all other functions in class call it. The hope
is that maybe someday we'll call it asynchronously.

Previously it would look like this:
http://www.plantuml.com/plantuml/svg/ZP5HJiCm38RVSue-0jAkW0JRa8OGJCquGDQuRKPg5pUTbjkJfXfQrzBnvlZ_REUtfLFiaiLZygGFRUEsPAt1GWYPbQ1R2s-KAVlUwWAIHQ3EwPZPLBKTWYda34sPmdSBZRjHiS6ZiaOhPmZd9eXHAe9Ru9ouMe5L7dJKnC14JuLuPohWfR9shx8lyOckP7BEzlDsx9SA5VjNselrgIO4T4RPvy1IdgVWs8_13vJ0nz2L427h7AoPYL_C2VIO5fUlAAyeLEQcQ3dQbnup2KqJmewVmAE_srXdzueIiZ_ntsry3LTBcGOdDZv3_vvjuFGVxiNePZFcPbl5khk7Up7vmmjrtFai_W00

Now it will look like this:
http://www.plantuml.com/plantuml/svg/XP5HJiGW48RVEKKVrMIlOAAxQxB6wARK0x1bsg9Jg5FOBBUNqbHRsTGtcE-3uH-u5MIppNts83zSRuy6KUcQCLus81t2Yoa5lZfPGxO9rbIEbKFRuLBCeIBecno-7FJsdnC7VHQefB3AwBKEB2b5yGXK6MftWCA3Z8mjGenMM1Ug1TeAn9CeFzcxjWqPQt7iLv3V2Yt8FviJsryweq5R9V0nkAQgSh3aK_5DSw1pkDLe3UMGO4zaw4bTGAOsc-ycF83Yjx8puQvArOxYGBZN9OU-3yj4Z5dNX27q7yxu-D6pthw3T1O3kJBPEVxvZyM35qOozGLTZHhDcxjRTZNdZAOX9_WD
The change should be roughly equivalent to today's behavior.

BUG=259913

Review URL: https://codereview.chromium.org/68013007

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

2 files changed, 48 insertions, 28 deletions

diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index f93a35b9..66460d3 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -237,7 +237,7 @@ void SyncSchedulerImpl::Start(Mode mode) {
       CanRunNudgeJobNow(NORMAL_PRIORITY)) {
     // We just got back to normal mode.  Let's try to run the work that was
     // queued up while we were configuring.
-    DoNudgeSyncSessionJob(NORMAL_PRIORITY);
+    TrySyncSessionJob(NORMAL_PRIORITY);
   }
 }
 
@@ -299,7 +299,7 @@ void SyncSchedulerImpl::ScheduleConfiguration(
   // Only reconfigure if we have types to download.
   if (!params.types_to_download.Empty()) {
     pending_configure_params_.reset(new ConfigurationParams(params));
-    DoConfigurationSyncSessionJob(NORMAL_PRIORITY);
+    TrySyncSessionJob(NORMAL_PRIORITY);
   } else {
     SDVLOG(2) << "No change in routing info, calling ready task directly.";
     params.ready_task.Run();
@@ -572,7 +572,7 @@ void SyncSchedulerImpl::DoPollSyncSessionJob() {
       GetEnabledAndUnthrottledTypes(),
       session.get());
 
-  AdjustPolling(UPDATE_INTERVAL);
+  AdjustPolling(FORCE_RESET);
 
   if (IsCurrentlyThrottled()) {
     SDVLOG(2) << "Poll request got us throttled.";
@@ -599,18 +599,25 @@ void SyncSchedulerImpl::UpdateNudgeTimeRecords(ModelTypeSet types) {
   }
 }
 
+TimeDelta SyncSchedulerImpl::GetPollInterval() {
+  return (!session_context_->notifications_enabled() ||
+          !session_context_->ShouldFetchUpdatesBeforeCommit()) ?
+      syncer_short_poll_interval_seconds_ :
+      syncer_long_poll_interval_seconds_;
+}
+
 void SyncSchedulerImpl::AdjustPolling(PollAdjustType type) {
   DCHECK(CalledOnValidThread());
 
-  TimeDelta poll  = (!session_context_->notifications_enabled() ||
-                     !session_context_->ShouldFetchUpdatesBeforeCommit()) ?
-      syncer_short_poll_interval_seconds_ :
-      syncer_long_poll_interval_seconds_;
+  TimeDelta poll = GetPollInterval();
   bool rate_changed = !poll_timer_.IsRunning() ||
                        poll != poll_timer_.GetCurrentDelay();
 
-  if (type == FORCE_RESET && !rate_changed)
-    poll_timer_.Reset();
+  if (type == FORCE_RESET) {
+    last_poll_reset_ = base::TimeTicks::Now();
+    if (!rate_changed)
+      poll_timer_.Reset();
+  }
 
   if (!rate_changed)
     return;
@@ -660,25 +667,28 @@ void SyncSchedulerImpl::Stop() {
 // This is the only place where we invoke DoSyncSessionJob with canary
 // privileges.  Everyone else should use NORMAL_PRIORITY.
 void SyncSchedulerImpl::TryCanaryJob() {
-  DCHECK(CalledOnValidThread());
+  TrySyncSessionJob(CANARY_PRIORITY);
+  // Don't run poll job till the next time poll timer fires.
+  do_poll_after_credentials_updated_ = false;
+}
 
-  if (mode_ == CONFIGURATION_MODE && pending_configure_params_) {
-    SDVLOG(2) << "Found pending configure job; will run as canary";
-    DoConfigurationSyncSessionJob(CANARY_PRIORITY);
-  } else if (mode_ == NORMAL_MODE && nudge_tracker_.IsSyncRequired() &&
-             CanRunNudgeJobNow(CANARY_PRIORITY)) {
-    SDVLOG(2) << "Found pending nudge job; will run as canary";
-    DoNudgeSyncSessionJob(CANARY_PRIORITY);
-  } else if (mode_ == NORMAL_MODE && CanRunJobNow(CANARY_PRIORITY) &&
-             do_poll_after_credentials_updated_) {
-    // Retry poll if poll timer recently fired and ProfileSyncService received
-    // fresh access token.
-    DoPollSyncSessionJob();
+void SyncSchedulerImpl::TrySyncSessionJob(JobPriority priority) {
+  DCHECK(CalledOnValidThread());
+  if (mode_ == CONFIGURATION_MODE) {
+    if (pending_configure_params_) {
+      SDVLOG(2) << "Found pending configure job";
+      DoConfigurationSyncSessionJob(priority);
+    }
   } else {
-    SDVLOG(2) << "Found no work to do; will not run a canary";
+    DCHECK(mode_ == NORMAL_MODE);
+    if (nudge_tracker_.IsSyncRequired() && CanRunNudgeJobNow(priority)) {
+      SDVLOG(2) << "Found pending nudge job";
+      DoNudgeSyncSessionJob(priority);
+    } else if (do_poll_after_credentials_updated_ ||
+        ((base::TimeTicks::Now() - last_poll_reset_) >= GetPollInterval())) {
+      DoPollSyncSessionJob();
+    }
   }
-  // Don't run poll job till the next time poll timer fires.
-  do_poll_after_credentials_updated_ = false;
 }
 
 void SyncSchedulerImpl::PollTimerCallback() {
@@ -694,7 +704,7 @@ void SyncSchedulerImpl::PollTimerCallback() {
     return;
   }
 
-  DoPollSyncSessionJob();
+  TrySyncSessionJob(NORMAL_PRIORITY);
   // Poll timer fires infrequently. Usually by this time access token is already
   // expired and poll job will fail with auth error. Set flag to retry poll once
   // ProfileSyncService gets new access token, TryCanaryJob will be called in
@@ -739,14 +749,14 @@ void SyncSchedulerImpl::TypeUnthrottle(base::TimeTicks unthrottle_time) {
 
   // Maybe this is a good time to run a nudge job.  Let's try it.
   if (nudge_tracker_.IsSyncRequired() && CanRunNudgeJobNow(NORMAL_PRIORITY))
-    DoNudgeSyncSessionJob(NORMAL_PRIORITY);
+    TrySyncSessionJob(NORMAL_PRIORITY);
 }
 
 void SyncSchedulerImpl::PerformDelayedNudge() {
   // Circumstances may have changed since we scheduled this delayed nudge.
   // We must check to see if it's OK to run the job before we do so.
   if (CanRunNudgeJobNow(NORMAL_PRIORITY))
-    DoNudgeSyncSessionJob(NORMAL_PRIORITY);
+    TrySyncSessionJob(NORMAL_PRIORITY);
 
   // We're not responsible for setting up any retries here.  The functions that
   // first put us into a state that prevents successful sync cycles (eg. global
diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h
index 60d1c38..4ffbf6f 100644
--- a/sync/engine/sync_scheduler_impl.h
+++ b/sync/engine/sync_scheduler_impl.h
@@ -167,6 +167,9 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   // Invoke the Syncer to perform a poll job.
   void DoPollSyncSessionJob();
 
+  // Helper function to calculate poll interval.
+  base::TimeDelta GetPollInterval();
+
   // Adjusts the poll timer to account for new poll interval, and possibly
   // resets the poll interval, depedning on the flag's value.
   void AdjustPolling(PollAdjustType type);
@@ -204,6 +207,8 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   // priority.
   void TryCanaryJob();
 
+  void TrySyncSessionJob(JobPriority priority);
+
   // Transitions out of the THROTTLED WaitInterval then calls TryCanaryJob().
   void Unthrottle();
 
@@ -309,6 +314,11 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   // after credentials are updated.
   bool do_poll_after_credentials_updated_;
 
+  // TryJob might get called for multiple reasons. It should only call
+  // DoPollSyncSessionJob after some time since the last attempt.
+  // last_poll_reset_ keeps track of when was last attempt.
+  base::TimeTicks last_poll_reset_;
+
   base::WeakPtrFactory<SyncSchedulerImpl> weak_ptr_factory_;
 
   // A second factory specially for weak_handle_this_, to allow the handle