Delay autofill commits to reduce client to server traffic.

chrome/browser/sync/engine/sync_scheduler.cc

Index: chrome/browser/sync/engine/sync_scheduler.cc
diff --git a/chrome/browser/sync/engine/sync_scheduler.cc b/chrome/browser/sync/engine/sync_scheduler.cc
index ce05fb1be4b065c094dd89677bd3a7d8502f548c..71db3daf7eeaec303e40cddccf58ac7131b5afb1 100644
--- a/chrome/browser/sync/engine/sync_scheduler.cc
+++ b/chrome/browser/sync/engine/sync_scheduler.cc
@@ -542,17 +542,15 @@ void SyncScheduler::ScheduleNudgeImpl(
     SDVLOG(2) << "Coalescing pending nudge";
     pending_nudge_->session->Coalesce(*(job.session.get()));
 
-    if (!IsBackingOff()) {
-      SDVLOG(2) << "Dropping a nudge because"
-                << " we are not in backoff and the job was coalesced";
-      return;
-    } else {
-      SDVLOG(2) << "Rescheduling pending nudge";
-      SyncSession* s = pending_nudge_->session.get();
-      job.session.reset(new SyncSession(s->context(), s->delegate(),
-          s->source(), s->routing_info(), s->workers()));
-      pending_nudge_.reset();
-    }
+    SDVLOG(2) << "Rescheduling pending nudge";
+    SyncSession* s = pending_nudge_->session.get();
+    job.session.reset(new SyncSession(s->context(), s->delegate(),
+        s->source(), s->routing_info(), s->workers()));
+
+    // Choose the start time as the earliest of the 2.
+    job.scheduled_start = std::min(job.scheduled_start,
+                                   pending_nudge_->scheduled_start);

[tim (not reviewing), 2011/12/06 20:15:57]

This was explicitly not the goal of the implementation. Why do we need this now?
E.g. for sessions, we have a delay of 11 seconds to coalesce. We actually only
want to sync once things have quiesced for 11 seconds, and this would break
that.

In either case, shouldn't this be a separate change?

[lipalani1, 2011/12/06 20:44:06]

This is a needed change and is deliberate. 
Need: When you put an autofill change to sync 2 hours later and then a bookmark
change comes along we want to sync right now rather than waiting for 2 hours.

Problem with existing impl: Since we keep pushing the new syncs to later time
there is a possibility that the user's browser does not sync for extended period
of time. If the user keeps opening new URLs or whatever continuously we would
not be syncing ANY change for that whole time period and in general this could
lead to bugs that say "my sync is not working". The new impl introduces
predictability which is important at this layer.

Compromise: I do believe the new impl is the way to go :) However I had a backup
plan cooked up in case this is not acceptable. For autofill type of changes we
need not schedule a sync at all. We can let the long poll interval take care of
it. Ideally some other change would occur and sync autofill. If the user has not
made any other changes it is probably OK to wait until the long poll interval.
But we should hash out why we should/should not use the new logic first.
On 2011/12/06 20:15:57, timsteele wrote:

+    pending_nudge_.reset();
   }
 
   // TODO(lipalani) - pass the job itself to ScheduleSyncSessionJob.