Delay autofill commits to reduce client to server traffic.

chrome/browser/sync/engine/sync_scheduler.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/engine/sync_scheduler.h"
 
 #include <algorithm>
 #include <cstring>
 
 #include "base/bind.h"
@@ skipping 524 matching lines @@
 
   if (pending_nudge_.get()) {
     if (IsBackingOff() && delay > TimeDelta::FromSeconds(1)) {
       SDVLOG(2) << "Dropping the nudge because we are in backoff";
       return;
     }
 
     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.
   ScheduleSyncSessionJob(delay, SyncSessionJob::NUDGE, job.session.release(),
       nudge_location);
 }
 
 // Helper to extract the routing info and workers corresponding to types in
 // |types| from |registrar|.
 void GetModelSafeParamsForTypes(const ModelTypeBitSet& types,
@@ skipping 621 matching lines @@
 
 #undef SDVLOG_LOC
 
 #undef SDVLOG
 
 #undef SLOG
 
 #undef ENUM_CASE
 
 }  // browser_sync