sync: GU retry with less explicit TimeTicks logic

sync/engine/sync_scheduler_impl.cc

Index: sync/engine/sync_scheduler_impl.cc
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index ac169ec18f4c394994c8230fffb3a2b757801138..0cc1db46837111f3e88f87f98a4d10e252812571 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -234,8 +234,7 @@ void SyncSchedulerImpl::Start(Mode mode) {
 
   if (old_mode != mode_ &&
       mode_ == NORMAL_MODE &&
-      (nudge_tracker_.IsSyncRequired() ||
-          nudge_tracker_.IsRetryRequired(base::TimeTicks::Now())) &&
+      (nudge_tracker_.IsSyncRequired() || nudge_tracker_.IsRetryRequired()) &&

[pavely, 2014/01/29 15:35:05]

SetSyncCycleStartTime isn't called in this case. If previous sync cycle called
SetRetryTime this call wouldn't notice it.

The semantic of variables is:
- current_retry_time : earliest unsatisfied retry time.
- next_retry_time : latest known retry time.

If the above semantic is right then SetNextRetryTime should also copy retry time
into current_retry_time if it is not set.

[rlarocque, 2014/01/29 18:27:42]

I had forgotten that this was a different code path.  I think this will need a
call to SetSyncCycleStartTime().  That would fix the behavior.

I'm not sure I agree with your suggestion about current_retry_time semantics.

I think the behavior we're trying to achieve is that the client should only act
on the most recently received retry.  There is a special case where the GU retry
attempt fails, in which case we should have the IsRetryRequired() flag set until
one of the exponential backoff attempts is successful.

It seems to me that if we set current_retry_time in SetNextRetryTime(), we won't
be able to override it if a new retry time arrives before it times out.

       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.
@@ -470,13 +469,12 @@ void SyncSchedulerImpl::DoNudgeSyncSessionJob(JobPriority priority) {
   if (success) {
     // That cycle took care of any outstanding work we had.
     SDVLOG(2) << "Nudge succeeded.";
-    nudge_tracker_.RecordSuccessfulSyncCycle(base::TimeTicks::Now());
+    nudge_tracker_.RecordSuccessfulSyncCycle();
     scheduled_nudge_time_ = base::TimeTicks();
 
     // If we're here, then we successfully reached the server.  End all backoff.
     wait_interval_.reset();
     NotifyRetryTime(base::Time());
-    return;
   } else {
     HandleFailure(session->status_controller().model_neutral_state());
   }
@@ -580,7 +578,7 @@ void SyncSchedulerImpl::DoRetrySyncSessionJob() {
                               session.get()) &&
       !sessions::HasSyncerError(
           session->status_controller().model_neutral_state())) {
-    nudge_tracker_.RecordSuccessfulSyncCycle(base::TimeTicks::Now());
+    nudge_tracker_.RecordSuccessfulSyncCycle();
   } else {
     HandleFailure(session->status_controller().model_neutral_state());
   }
@@ -687,6 +685,8 @@ void SyncSchedulerImpl::TrySyncSessionJobImpl() {
   JobPriority priority = next_sync_session_job_priority_;
   next_sync_session_job_priority_ = NORMAL_PRIORITY;
 
+  nudge_tracker_.SetSyncCycleStartTime(base::TimeTicks::Now());
+
   DCHECK(CalledOnValidThread());
   if (mode_ == CONFIGURATION_MODE) {
     if (pending_configure_params_) {
@@ -697,7 +697,7 @@ void SyncSchedulerImpl::TrySyncSessionJobImpl() {
     if (nudge_tracker_.IsSyncRequired()) {
       SDVLOG(2) << "Found pending nudge job";
       DoNudgeSyncSessionJob(priority);
-    } else if (nudge_tracker_.IsRetryRequired(base::TimeTicks::Now())) {
+    } else if (nudge_tracker_.IsRetryRequired()) {
       DoRetrySyncSessionJob();
     } else if (do_poll_after_credentials_updated_ ||
         ((base::TimeTicks::Now() - last_poll_reset_) >= GetPollInterval())) {
@@ -906,7 +906,7 @@ void SyncSchedulerImpl::OnSyncProtocolError(
 }
 
 void SyncSchedulerImpl::OnReceivedGuRetryDelay(const base::TimeDelta& delay) {
-  nudge_tracker_.set_next_retry_time(base::TimeTicks::Now() + delay);
+  nudge_tracker_.SetNextRetryTime(TimeTicks::Now() + delay);
   retry_timer_.Start(FROM_HERE, delay, this,
                      &SyncSchedulerImpl::RetryTimerCallback);
 }