Delay autofill commits to reduce client to server traffic.

chrome/browser/sync/internal_api/sync_manager.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/internal_api/sync_manager.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/command_line.h"
@@ skipping 96 matching lines @@
       NOTREACHED();
   }
 
   return GetUpdatesCallerInfo::UNKNOWN;
 }
 
 } // namespace
 
 namespace sync_api {
 
+const int SyncManager::kDefaultNudgeDelayMilliseconds = 200;
+const int SyncManager::kPreferencesNudgeDelayMilliseconds = 2000;
+
+// Nudge delay for datatypes that piggy back their change with other changes.
+const int SyncManager::kPiggybackNudgeDelay = 2 * 60 * 60 * 1000;

[tim (not reviewing), 2011/12/09 23:07:22]

I'm a bit confused with the relation to the periodic sync interval.  It seems
like "accompany an immediate change or wait for periodic" is a clear
standby-like behavior.  Why did you choose to separate it?

[lipalani1, 2011/12/15 01:35:16]

I went back and forth. The advantage is that if it is not tied to polling
interval we could control this independently(like via a client command etc).
However I think unifying them makes more sense. If we actually find a use case
where they have to be changed dynamically we can change to it. But for now
unifying them reduces a variable and efficient sync wise. I am using the short
polling interval here in order to not wait too long.
On 2011/12/09 23:07:22, timsteele wrote:

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

Short-poll sounds good, but shouldn't we tie it to the actual variable, not
value, since it can change via ClientCommand

+
 //////////////////////////////////////////////////////////////////////////
 // SyncManager's implementation: SyncManager::SyncInternal
 class SyncManager::SyncInternal
     : public net::NetworkChangeNotifier::IPAddressObserver,
       public browser_sync::Cryptographer::Observer,
       public sync_notifier::SyncNotifierObserver,
       public JsBackend,
       public SyncEngineEventListener,
       public ServerConnectionEventListener,
       public syncable::DirectoryChangeDelegate {
-  static const int kDefaultNudgeDelayMilliseconds;
-  static const int kPreferencesNudgeDelayMilliseconds;
  public:
   explicit SyncInternal(const std::string& name)
       : name_(name),
         weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
         registrar_(NULL),
         change_delegate_(NULL),
         initialized_(false),
         setup_for_test_mode_(false),
         observing_ip_address_changes_(false),
         created_on_loop_(MessageLoop::current()) {
@@ skipping 156 matching lines @@
   }
 
   Status GetStatus();
 
   void RequestNudge(const tracked_objects::Location& nudge_location);
 
   void RequestNudgeForDataType(
       const tracked_objects::Location& nudge_location,
       const ModelType& type);
 
+  TimeDelta GetNudgeDelayTimeDelta(const ModelType& model_type);
+
   // See SyncManager::Shutdown* for information.
   void StopSyncingForShutdown(const base::Closure& callback);
   void ShutdownOnSyncThread();
 
   // If this is a deletion for a password, sets the legacy
   // ExtraPasswordChangeRecordData field of |buffer|. Otherwise sets
   // |buffer|'s specifics field to contain the unencrypted data.
   void SetExtraChangeRecordData(int64 id,
                                 syncable::ModelType type,
                                 ChangeReorderBuffer* buffer,
@@ skipping 145 matching lines @@
 
   // JS message handlers.
   JsArgList GetNotificationState(const JsArgList& args);
   JsArgList GetNotificationInfo(const JsArgList& args);
   JsArgList GetRootNodeDetails(const JsArgList& args);
   JsArgList GetNodeSummariesById(const JsArgList& args);
   JsArgList GetNodeDetailsById(const JsArgList& args);
   JsArgList GetChildNodeIds(const JsArgList& args);
   JsArgList FindNodesContainingString(const JsArgList& args);
 
+  TimeDelta GetNudgeDelayTimeDeltaFromType(

[tim (not reviewing), 2011/12/09 23:07:22]

I see an argument for a simple NudgeStrategy class that would encapsulate all
the code you've added here.  WDYT?

[lipalani1, 2011/12/15 01:35:16]

Done.

+      const NudgeDelayType& delay_type, const ModelType& model_type);
+
+
   const std::string name_;
 
   base::ThreadChecker thread_checker_;
 
   base::WeakPtrFactory<SyncInternal> weak_ptr_factory_;
 
   // Thread-safe handle used by
   // HandleCalculateChangesChangeEventFromSyncApi(), which can be
   // called from any thread.  Valid only between between calls to
   // Init() and Shutdown().
@@ skipping 62 matching lines @@
   JsMessageHandlerMap js_message_handlers_;
   WeakHandle<JsEventHandler> js_event_handler_;
   JsSyncManagerObserver js_sync_manager_observer_;
   JsMutationEventObserver js_mutation_event_observer_;
 
   // This is for keeping track of client events to send to the server.
   DebugInfoEventListener debug_info_event_listener_;
 
   MessageLoop* const created_on_loop_;
 };
-const int SyncManager::SyncInternal::kDefaultNudgeDelayMilliseconds = 200;
-const int SyncManager::SyncInternal::kPreferencesNudgeDelayMilliseconds = 2000;
 
 SyncManager::ChangeDelegate::~ChangeDelegate() {}
 
 SyncManager::ChangeObserver::~ChangeObserver() {}
 
 SyncManager::Observer::~Observer() {}
 
 SyncManager::SyncManager(const std::string& name)
     : data_(new SyncInternal(name)) {}
 
@@ skipping 973 matching lines @@
 }
 
 void SyncManager::SyncInternal::RequestNudge(
     const tracked_objects::Location& location) {
   if (scheduler())
      scheduler()->ScheduleNudge(
         TimeDelta::FromMilliseconds(0), browser_sync::NUDGE_SOURCE_LOCAL,
         ModelTypeBitSet(), location);
 }
 
+namespace {
+NudgeDelayType GetNudgeDelayType(const ModelType& type) {
+  switch (type) {
+    case syncable::AUTOFILL:
+    case syncable::AUTOFILL_PROFILE:
+      return PIGGY_BACK_WITH_ANOTHER_CHANGE;
+    case syncable::PREFERENCES:
+    case syncable::SESSIONS:
+      return CUSTOM;
+    default:
+      return IMMEDIATE;
+  }
+}
+}  // namespace
+
+TimeDelta SyncManager::SyncInternal::GetNudgeDelayTimeDeltaFromType(
+    const NudgeDelayType& delay_type, const ModelType& model_type) {
+
+  TimeDelta delay = TimeDelta::FromMilliseconds(
+      SyncManager::kDefaultNudgeDelayMilliseconds);
+  switch (delay_type) {
+    case IMMEDIATE:
+      delay = TimeDelta::FromMilliseconds(
+          SyncManager::kDefaultNudgeDelayMilliseconds);
+      break;
+    case PIGGY_BACK_WITH_ANOTHER_CHANGE:
+      delay = TimeDelta::FromMilliseconds(
+          SyncManager::kPiggybackNudgeDelay);
+      break;
+    case CUSTOM:
+      switch (model_type) {
+        case syncable::PREFERENCES:
+          delay = TimeDelta::FromMilliseconds(
+              SyncManager::kPreferencesNudgeDelayMilliseconds);
+          break;
+        case syncable::SESSIONS:
+          delay = scheduler()->sessions_commit_delay();
+          break;
+        default:
+          NOTREACHED();
+      }
+      break;
+    default:
+      NOTREACHED();
+  }
+
+  return delay;
+}
+
+TimeDelta SyncManager::SyncInternal::GetNudgeDelayTimeDelta(
+    const ModelType& model_type) {
+  NudgeDelayType delay_type = GetNudgeDelayType(model_type);
+  return GetNudgeDelayTimeDeltaFromType(delay_type,
+                                        model_type);
+}
+
+
 void SyncManager::SyncInternal::RequestNudgeForDataType(
     const tracked_objects::Location& nudge_location,
     const ModelType& type) {
   if (!scheduler()) {
     NOTREACHED();
     return;
   }
-  base::TimeDelta nudge_delay;
-  switch (type) {
-    case syncable::PREFERENCES:
-      nudge_delay =
-          TimeDelta::FromMilliseconds(kPreferencesNudgeDelayMilliseconds);
-      break;
-    case syncable::SESSIONS:
-      nudge_delay = scheduler()->sessions_commit_delay();
-      break;
-    default:
-      nudge_delay =
-          TimeDelta::FromMilliseconds(kDefaultNudgeDelayMilliseconds);
-      break;
-  }
+  base::TimeDelta nudge_delay = GetNudgeDelayTimeDelta(type);
   syncable::ModelTypeBitSet types;
   types.set(type);
   scheduler()->ScheduleNudge(nudge_delay,
                              browser_sync::NUDGE_SOURCE_LOCAL,
                              types,
                              nudge_location);
 }
 
 void SyncManager::SyncInternal::OnSyncEngineEvent(
     const SyncEngineEvent& event) {
@@ skipping 402 matching lines @@
 UserShare* SyncManager::GetUserShare() const {
   return data_->GetUserShare();
 }
 
 void SyncManager::RefreshEncryption() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (data_->UpdateCryptographerAndNigori())
     data_->RefreshEncryption();
 }
 
+TimeDelta SyncManager::GetNudgeDelayTimeDelta(
+    const ModelType& model_type) {
+  return data_->GetNudgeDelayTimeDelta(model_type);
+}
+
 syncable::ModelTypeSet SyncManager::GetEncryptedDataTypesForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return GetEncryptedTypes(&trans);
 }
 
 bool SyncManager::ReceivedExperimentalTypes(syncable::ModelTypeSet* to_add)
     const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   ReadNode node(&trans);
   if (!node.InitByTagLookup(kNigoriTag)) {
@@ skipping 82 matching lines @@
     lookup->GetDownloadProgress(*i, &marker);
 
     if (marker.token().empty())
       result.insert(*i);
 
   }
   return result;
 }
 
 }  // namespace sync_api