[Sync] Make BackendMigrator not wait for full sync cycles

chrome/browser/sync/engine/sync_scheduler.h

 // 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.
 //
 // A class to schedule syncer tasks intelligently.
 #ifndef CHROME_BROWSER_SYNC_ENGINE_SYNC_SCHEDULER_H_
 #define CHROME_BROWSER_SYNC_ENGINE_SYNC_SCHEDULER_H_
 #pragma once
 
 #include <string>
@@ skipping 44 matching lines @@
   // (except for RequestEarlyExit()).
 
   // |name| is a display string to identify the syncer thread.  Takes
   // |ownership of both |context| and |syncer|.
   SyncScheduler(const std::string& name,
                 sessions::SyncSessionContext* context, Syncer* syncer);
 
   // Calls Stop().
   virtual ~SyncScheduler();
 
-  typedef Callback0::Type ModeChangeCallback;
-
   // Start the scheduler with the given mode.  If the scheduler is
   // already started, switch to the given mode, although some
   // scheduled tasks from the old mode may still run.  If non-NULL,
   // |callback| will be invoked when the mode has been changed to
   // |mode|.  Takes ownership of |callback|.
-  void Start(Mode mode, ModeChangeCallback* callback);
+  void Start(Mode mode, const base::Closure& callback);
 
   // Request that any running syncer task stop as soon as possible.
   // This function can be called from any thread.  Stop must still be
   // called to stop future schedule tasks.
   //
   // TODO(akalin): This function is awkward.  Find a better way to let
   // the UI thread stop the syncer thread.
   void RequestEarlyExit();
 
   // Cancel all scheduled tasks.  Can be called even if already stopped.
   void Stop();
 
+  // TODO(akalin): Add callback parameters to Schedule* functions as
+  // needed.  In particular, we'd like to know when a nudge completes
+  // (or fails) so that we can acknowledge any invalidations that
+  // triggered the nudge.

[tim (not reviewing), 2011/08/26 12:41:40]

So, one of the "future todos / open questions" in the doc for SyncerThread2 was
about connecting nudges back to the originating sites. But with coalescing, that
gets pretty tricky, and I'm not convinced it's worth the complexity.  There are
some cases it would help, e.g. with ClearUserData, but for straight nudges, I
don't know.

Instead of "TODO: add Foo as needed" can you be more specific with TODOs on the
individual functions and/or file a bug?

+
   // The meat and potatoes.
   void ScheduleNudge(const base::TimeDelta& delay, NudgeSource source,
                      const syncable::ModelTypeBitSet& types,
                      const tracked_objects::Location& nudge_location);
   void ScheduleNudgeWithPayloads(
       const base::TimeDelta& delay, NudgeSource source,
       const syncable::ModelTypePayloadMap& types_with_payloads,
       const tracked_objects::Location& nudge_location);
   void ScheduleConfig(const syncable::ModelTypeBitSet& types,
       sync_api::ConfigureReason reason);
   void ScheduleClearUserData();
-  void ScheduleCleanupDisabledTypes();
+  void ScheduleCleanupDisabledTypes(const base::Closure& callback);

[tim (not reviewing), 2011/08/26 12:41:40]

Please comment the new parameter.

 
   // Change status of notifications in the SyncSessionContext.
   void set_notifications_enabled(bool notifications_enabled);
 
   base::TimeDelta sessions_commit_delay() const;
 
   // DDOS avoidance function.  Calculates how long we should wait before trying
   // again after a failed sync attempt, where the last delay was |base_delay|.
   // TODO(tim): Look at URLRequestThrottlerEntryInterface.
   static base::TimeDelta GetRecommendedDelay(const base::TimeDelta& base_delay);
@@ skipping 42 matching lines @@
       // Typically used for fetching updates for a subset of the enabled types
       // during initial sync or reconfiguration.  We don't run all steps of
       // the sync cycle for these (e.g. CleanupDisabledTypes is skipped).
       CONFIGURATION,
       // The user disabled some types and we have to clean up the data
       // for those.
       CLEANUP_DISABLED_TYPES,
     };
     SyncSessionJob();
     SyncSessionJob(SyncSessionJobPurpose purpose, base::TimeTicks start,
-        linked_ptr<sessions::SyncSession> session, bool is_canary_job,
-        const tracked_objects::Location& nudge_location);
+                   linked_ptr<sessions::SyncSession> session,
+                   bool is_canary_job, const base::Closure& on_success,
+                   const tracked_objects::Location& nudge_location);
     ~SyncSessionJob();
     static const char* GetPurposeString(SyncSessionJobPurpose purpose);
 
     SyncSessionJobPurpose purpose;
     base::TimeTicks scheduled_start;
     linked_ptr<sessions::SyncSession> session;
     bool is_canary_job;
+    base::Closure on_success;

[tim (not reviewing), 2011/08/26 12:41:40]

I like this.  I'm still wrapping my head around what the outcome is with all the
coalescing we do, but it seems valuable in general.

 
     // This is the location the job came from.  Used for debugging.
     // In case of multiple nudges getting coalesced this stores the
     // first location that came in.
     tracked_objects::Location from_here;
   };
   friend class SyncSchedulerTest;
   friend class SyncSchedulerWhiteboxTest;
 
   FRIEND_TEST_ALL_PREFIXES(SyncSchedulerWhiteboxTest,
@@ skipping 66 matching lines @@
   void PostTask(const tracked_objects::Location& from_here,
                 const char* name, Task* task);
   void PostDelayedTask(const tracked_objects::Location& from_here,
                        const char* name, Task* task, int64 delay_ms);
 
   // Helper to assemble a job and post a delayed task to sync.
   void ScheduleSyncSessionJob(
       const base::TimeDelta& delay,
       SyncSessionJob::SyncSessionJobPurpose purpose,
       sessions::SyncSession* session,
+      const base::Closure& on_success,
       const tracked_objects::Location& from_here);
 
   // Invoke the Syncer to perform a sync.
   void DoSyncSessionJob(const SyncSessionJob& job);
 
   // Called after the Syncer has performed the sync represented by |job|, to
   // reset our state.
   void FinishSyncSessionJob(const SyncSessionJob& job);
 
   // Record important state that might be needed in future syncs, such as which
@@ skipping 25 matching lines @@
   JobProcessDecision DecideWhileInWaitInterval(const SyncSessionJob& job);
 
   // Saves the job for future execution. Note: It drops all the poll jobs.
   void SaveJob(const SyncSessionJob& job);
 
   // Coalesces the current job with the pending nudge.
   void InitOrCoalescePendingJob(const SyncSessionJob& job);
 
   // 'Impl' here refers to real implementation of public functions, running on
   // |thread_|.
-  void StartImpl(Mode mode, ModeChangeCallback* callback);
+  void StartImpl(Mode mode, const base::Closure& callback);
   void ScheduleNudgeImpl(
       const base::TimeDelta& delay,
       sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source,
       const syncable::ModelTypePayloadMap& types_with_payloads,
       bool is_canary_job, const tracked_objects::Location& nudge_location);
   void ScheduleConfigImpl(const ModelSafeRoutingInfo& routing_info,
       const std::vector<ModelSafeWorker*>& workers,
       const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source);
   void ScheduleClearUserDataImpl();
-  void ScheduleCleanupDisabledTypesImpl();
+  void ScheduleCleanupDisabledTypesImpl(const base::Closure& callback);
 
   // Returns true if the client is currently in exponential backoff.
   bool IsBackingOff() const;
 
   // Helper to signal all listeners registered with |session_context_|.
   void Notify(SyncEngineEvent::EventCause cause);
 
   // Callback to change backoff state.
   void DoCanaryJob();
   void Unthrottle();
@@ skipping 73 matching lines @@
   scoped_ptr<Syncer> syncer_;
 
   scoped_ptr<sessions::SyncSessionContext> session_context_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncScheduler);
 };
 
 }  // namespace browser_sync
 
 #endif  // CHROME_BROWSER_SYNC_ENGINE_SYNC_SCHEDULER_H_