sync: add a SyncSessionJobPurpose for clearing sync user data.

chrome/browser/sync/engine/syncer_thread2.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/syncer_thread2.h"
 
 #include <algorithm>
 
 #include "base/rand_util.h"
 #include "chrome/browser/sync/engine/syncer.h"
@@ skipping 84 matching lines @@
   mode_ = mode;
   AdjustPolling(NULL);  // Will kick start poll timer if needed.
 }
 
 bool SyncerThread::ShouldRunJob(SyncSessionJobPurpose purpose,
     const TimeTicks& scheduled_start) {
   DCHECK_EQ(MessageLoop::current(), thread_.message_loop());
 
   // Check wait interval.
   if (wait_interval_.get()) {
+    // TODO(tim): Consider different handling for CLEAR_USER_DATA (i.e. permit
+    // when throttled).
     if (wait_interval_->mode == WaitInterval::THROTTLED)
       return false;
 
     DCHECK_EQ(wait_interval_->mode, WaitInterval::EXPONENTIAL_BACKOFF);
     DCHECK(purpose == POLL ||
            purpose == NUDGE);
     if ((purpose != NUDGE) || wait_interval_->had_nudge)
       return false;
   }
 
   // Mode / purpose contract (See 'Mode' enum in header). Don't run jobs that
   // were intended for a normal sync if we are in configuration mode, and vice
   // versa.
   switch (mode_) {

[Raz Mathias, 2011/02/03 01:42:17]

It seems like you are switching on Purpose a bunch -- would it make make the
purpose a class and use polymorphic methods to get a better encapsulation?

[tim (not reviewing), 2011/02/03 18:18:52]

I like this suggestion, except that right now the code actually only switches on
|purpose| once!  There are a few if purpose == NUDGE branches though, but
they're so far pretty specific and I'm not sure right now is the best time to
build this encapsulation... will definitely keep it in mind though for next
time.  That sound ok?

[Raz Mathias, 2011/02/03 19:21:17]

What I meant here was implementing a method on the purpose class that took a
current_mode (and naming it so well that you could remove the comment above the
switch block :), something like shouldRunInMode().  The if checks and
switch(purpose) below would be additional candidates for encapsulation.

     case CONFIGURATION_MODE:
       if (purpose != CONFIGURATION)
         return false;
       break;
     case NORMAL_MODE:
-      if (purpose != POLL && purpose != NUDGE)
+      if (purpose == CONFIGURATION)
         return false;
       break;
     default:
       NOTREACHED() << "Unknown SyncerThread Mode: " << mode_;
       return false;
   }
 
   // Continuation NUDGE tasks have priority over POLLs because they are the
   // only tasks that trigger exponential backoff, so this prevents them from
   // being starved from running (e.g. due to a very, very low poll interval,
@@ skipping 16 matching lines @@
 namespace {
 GetUpdatesCallerInfo::GetUpdatesSource GetUpdatesFromNudgeSource(
     NudgeSource source) {
   switch (source) {
     case NUDGE_SOURCE_NOTIFICATION:
       return GetUpdatesCallerInfo::NOTIFICATION;
     case NUDGE_SOURCE_LOCAL:
       return GetUpdatesCallerInfo::LOCAL;
     case NUDGE_SOURCE_CONTINUATION:
       return GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION;
-    case NUDGE_SOURCE_CLEAR_PRIVATE_DATA:
-      return GetUpdatesCallerInfo::CLEAR_PRIVATE_DATA;
     case NUDGE_SOURCE_UNKNOWN:
+      return GetUpdatesCallerInfo::UNKNOWN;
     default:
+      NOTREACHED();
       return GetUpdatesCallerInfo::UNKNOWN;
   }
 }
 
 // Functor for std::find_if to search by ModelSafeGroup.
 struct WorkerGroupIs {
   explicit WorkerGroupIs(ModelSafeGroup group) : group(group) {}
   bool operator()(ModelSafeWorker* w) {
     return group == w->GetModelSafeGroup();
   }
   ModelSafeGroup group;
 };
 }  // namespace
 
+void SyncerThread::ScheduleClearUserData() {
+  if (!thread_.IsRunning()) {
+    NOTREACHED();
+    return;
+  }
+  thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+      this, &SyncerThread::ScheduleClearUserDataImpl));
+}
+
 void SyncerThread::ScheduleNudge(const TimeDelta& delay,
     NudgeSource source, const ModelTypeBitSet& types) {
   if (!thread_.IsRunning()) {
     NOTREACHED();
     return;
   }
 
   TypePayloadMap types_with_payloads =
       sessions::MakeTypePayloadMapFromBitSet(types, std::string());
   thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
       this, &SyncerThread::ScheduleNudgeImpl, delay, source,
       types_with_payloads));
 }
 
 void SyncerThread::ScheduleNudgeWithPayloads(const TimeDelta& delay,
     NudgeSource source, const TypePayloadMap& types_with_payloads) {
   if (!thread_.IsRunning()) {
     NOTREACHED();
     return;
   }
 
   thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
       this, &SyncerThread::ScheduleNudgeImpl, delay, source,
       types_with_payloads));
 }
 
+void SyncerThread::ScheduleClearUserDataImpl() {
+  DCHECK_EQ(MessageLoop::current(), thread_.message_loop());
+  SyncSession* session = new SyncSession(session_context_.get(), this,
+      SyncSourceInfo(), ModelSafeRoutingInfo(),
+      std::vector<ModelSafeWorker*>());
+  ScheduleSyncSessionJob(TimeDelta::FromSeconds(0), CLEAR_USER_DATA, session);
+}
+
 void SyncerThread::ScheduleNudgeImpl(const TimeDelta& delay,
     NudgeSource source, const TypePayloadMap& types_with_payloads) {
   DCHECK_EQ(MessageLoop::current(), thread_.message_loop());
   TimeTicks rough_start = TimeTicks::Now() + delay;
-  if (!ShouldRunJob(NUDGE, rough_start))
-    return;
 
   // Note we currently nudge for all types regardless of the ones incurring
   // the nudge.  Doing different would throw off some syncer commands like
   // CleanupDisabledTypes.  We may want to change this in the future.
   ModelSafeRoutingInfo routes;
   std::vector<ModelSafeWorker*> workers;
   session_context_->registrar()->GetModelSafeRoutingInfo(&routes);
   session_context_->registrar()->GetWorkers(&workers);
   SyncSourceInfo info(GetUpdatesFromNudgeSource(source),
                       types_with_payloads);
@@ skipping 80 matching lines @@
   SyncSessionJob job = {purpose, TimeTicks::Now() + delay,
                         make_linked_ptr(session)};
   if (purpose == NUDGE) {
     DCHECK(!pending_nudge_.get() || pending_nudge_->session.get() == session);
     pending_nudge_.reset(new SyncSessionJob(job));
   }
   MessageLoop::current()->PostDelayedTask(FROM_HERE, NewRunnableMethod(this,
       &SyncerThread::DoSyncSessionJob, job), delay.InMilliseconds());
 }
 
+void SyncerThread::SetSyncerStepsForPurpose(SyncSessionJobPurpose purpose,

[Raz Mathias, 2011/02/03 01:42:17]

It seems a little strange that the syncer, which seems to be a general state
machine executor, hardcodes/controls the start/end states.  I wonder if it would
make sense to encapsulate this info in a separate class?  Without knowing more
about the design, it almost feels like the Purpose  itself could be that
class...

[tim (not reviewing), 2011/02/03 18:18:52]

Did you mean SyncerThread vs Syncer?  Because this patch actually deprecates the
Syncer method that hardcodes/controls the start/end states and makes public the
method to allow the controller (SyncerThread) to make the initial/end state
decisions.

[Raz Mathias, 2011/02/03 19:21:17]

Yes, sorry, SyncerThread -- This switch block with hard-coded constants seems to
belong elsewhere, perhaps in the purpose class itself (when it becomes a class
:)

+    SyncerStep* start, SyncerStep* end) {
+  *end = SYNCER_END;
+  switch (purpose) {
+    case CONFIGURATION:
+      *start = DOWNLOAD_UPDATES;
+      *end = APPLY_UPDATES;
+      return;
+    case CLEAR_USER_DATA:
+      *start = CLEAR_PRIVATE_DATA;
+       return;
+    case NUDGE:
+    case POLL:
+      *start = SYNCER_BEGIN;
+      return;
+    default:
+      NOTREACHED();
+  }
+}
+
 void SyncerThread::DoSyncSessionJob(const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), thread_.message_loop());
 
   if (job.purpose == NUDGE) {
     DCHECK(pending_nudge_.get());
     if (pending_nudge_->session != job.session)
       return;  // Another nudge must have been scheduled in in the meantime.
     pending_nudge_.reset();
-  } else if (job.purpose == CONFIGURATION) {
-    NOTIMPLEMENTED() << "TODO(tim): SyncShare [DOWNLOAD_UPDATES,APPLY_UPDATES]";
   }
 
+  SyncerStep begin(SYNCER_BEGIN);
+  SyncerStep end(SYNCER_END);
+  SetSyncerStepsForPurpose(job.purpose, &begin, &end);
+
   bool has_more_to_sync = true;
   bool did_job = false;
   while (ShouldRunJob(job.purpose, job.scheduled_start) && has_more_to_sync) {
     VLOG(1) << "SyncerThread: Calling SyncShare.";
     did_job = true;
     // Synchronously perform the sync session from this thread.
-    syncer_->SyncShare(job.session.get());
+    syncer_->SyncShare(job.session.get(), begin, end);
     has_more_to_sync = job.session->HasMoreToSync();
     if (has_more_to_sync)
       job.session->ResetTransientState();
   }
   VLOG(1) << "SyncerThread: Done SyncShare looping.";
   if (did_job)
     FinishSyncSessionJob(job);
 }
 
 void SyncerThread::FinishSyncSessionJob(const SyncSessionJob& job) {
@@ skipping 28 matching lines @@
   // 500, is likely to have occurred during commit).
   const bool work_to_do =
      old_job.session->status_controller()->num_server_changes_remaining() > 0
      || old_job.session->status_controller()->unsynced_handles().size() > 0;
   VLOG(1) << "syncer has work to do: " << work_to_do;
 
   AdjustPolling(&old_job);
 
   // TODO(tim): Old impl had special code if notifications disabled. Needed?
   if (!work_to_do) {
-    wait_interval_.reset();  // Success implies backoff relief.
+    // Success implies backoff relief.  Note that if this was a "one-off" job
+    // (i.e. purpose == CLEAR_USER_DATA), if there was work_to_do before it
+    // ran this wont have changed, as jobs like this don't run a full sync
+    // cycle.  So we don't need special code here.
+    wait_interval_.reset();
     return;
   }
 
   if (old_job.session->source().updates_source ==
       GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION) {
     // We don't seem to have made forward progress. Start or extend backoff.
     HandleConsecutiveContinuationError(old_job);
   } else if (IsBackingOff()) {
     // We weren't continuing but we're in backoff; must have been a nudge.
     DCHECK_EQ(NUDGE, old_job.purpose);
@@ skipping 146 matching lines @@
     const ServerConnectionEvent& event) {
   NOTIMPLEMENTED();
 }
 
 void SyncerThread::set_notifications_enabled(bool notifications_enabled) {
   session_context_->set_notifications_enabled(notifications_enabled);
 }
 
 }  // s3
 }  // browser_sync