sync: implement configuration tasks in SyncerThread2

Required shuffling initial_sync_ended bit-setting from SyncerEnd to
ApplyUpdatesCommand.

BUG=26339
TEST=SyncerThread2Test

Review URL: http://codereview.chromium.org/6473008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@75015 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 66 insertions, 46 deletions

diff --git a/chrome/browser/sync/engine/apply_updates_command.cc b/chrome/browser/sync/engine/apply_updates_command.cc
index c14dc81..835c71a 100644
--- a/chrome/browser/sync/engine/apply_updates_command.cc
+++ b/chrome/browser/sync/engine/apply_updates_command.cc
@@ -36,6 +36,27 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
   applicator.SaveProgressIntoSessionState(
       session->status_controller()->mutable_conflict_progress(),
       session->status_controller()->mutable_update_progress());
+
+  // This might be the first time we've fully completed a sync cycle, for
+  // some subset of the currently synced datatypes.
+  sessions::StatusController* status(session->status_controller());
+  if (status->ServerSaysNothingMoreToDownload()) {
+    syncable::ScopedDirLookup dir(session->context()->directory_manager(),
+                                  session->context()->account_name());
+    if (!dir.good()) {
+      LOG(ERROR) << "Scoped dir lookup failed!";
+      return;
+    }
+
+    for (int i = syncable::FIRST_REAL_MODEL_TYPE;
+         i < syncable::MODEL_TYPE_COUNT; ++i) {
+      syncable::ModelType model_type = syncable::ModelTypeFromInt(i);
+      if (status->updates_request_types()[i]) {
+        // This gets persisted to the directory's backing store.
+        dir->set_initial_sync_ended_for_type(model_type, true);
+      }
+    }
+  }
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc
index 9ca668e..ac20600 100644
--- a/chrome/browser/sync/engine/syncer.cc
+++ b/chrome/browser/sync/engine/syncer.cc
@@ -268,13 +268,13 @@ void Syncer::SyncShare(sessions::SyncSession* session,
         ClearDataCommand clear_data_command;
         clear_data_command.Execute(session);
         next_step = SYNCER_END;
+        break;
       }
       case SYNCER_END: {
         VLOG(1) << "Syncer End";
         SyncerEndCommand syncer_end_command;
-        // This will set "syncing" to false, and send out a notification.
         syncer_end_command.Execute(session);
-        goto post_while;
+        break;
       }
       default:
         LOG(ERROR) << "Unknown command: " << current_step;
@@ -283,7 +283,12 @@ void Syncer::SyncShare(sessions::SyncSession* session,
       break;
     current_step = next_step;
   }
- post_while:
+
+  // Always send out a cycle ended notification, regardless of end-state.
+  SyncEngineEvent event(SyncEngineEvent::SYNC_CYCLE_ENDED);
+  sessions::SyncSessionSnapshot snapshot(session->TakeSnapshot());
+  event.snapshot = &snapshot;
+  session->context()->NotifyListeners(event);
   return;
 }
 
diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h
index b9deb23..3178f69 100644
--- a/chrome/browser/sync/engine/syncer.h
+++ b/chrome/browser/sync/engine/syncer.h
@@ -98,7 +98,7 @@ class Syncer {
 
  private:
   // Implements the PROCESS_CLIENT_COMMAND syncer step.
-  void ProcessClientCommand(sessions::SyncSession *session);
+  void ProcessClientCommand(sessions::SyncSession* session);
 
   bool early_exit_requested_;
   base::Lock early_exit_requested_lock_;
diff --git a/chrome/browser/sync/engine/syncer_end_command.cc b/chrome/browser/sync/engine/syncer_end_command.cc
index ff5c03e..dcdfcc1 100644
--- a/chrome/browser/sync/engine/syncer_end_command.cc
+++ b/chrome/browser/sync/engine/syncer_end_command.cc
@@ -20,31 +20,6 @@ void SyncerEndCommand::ExecuteImpl(sessions::SyncSession* session) {
   session->context()->set_previous_session_routing_info(
       session->routing_info());
   session->context()->set_last_snapshot(session->TakeSnapshot());
-
-  // This might be the first time we've fully completed a sync cycle, for
-  // some subset of the currently synced datatypes.
-  if (!session->HasMoreToSync() &&
-      status->ServerSaysNothingMoreToDownload()) {
-    syncable::ScopedDirLookup dir(session->context()->directory_manager(),
-                                  session->context()->account_name());
-    if (!dir.good()) {
-      LOG(ERROR) << "Scoped dir lookup failed!";
-      return;
-    }
-
-    for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
-      syncable::ModelType model_type = syncable::ModelTypeFromInt(i);
-      if (status->updates_request_types()[i]) {
-        // This gets persisted to the directory's backing store.
-        dir->set_initial_sync_ended_for_type(model_type, true);
-      }
-    }
-  }
-
-  SyncEngineEvent event(SyncEngineEvent::SYNC_CYCLE_ENDED);
-  sessions::SyncSessionSnapshot snapshot(session->TakeSnapshot());
-  event.snapshot = &snapshot;
-  session->context()->NotifyListeners(event);
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/syncer_thread2.cc b/chrome/browser/sync/engine/syncer_thread2.cc
index be49a39..46ec2f8 100644
--- a/chrome/browser/sync/engine/syncer_thread2.cc
+++ b/chrome/browser/sync/engine/syncer_thread2.cc
@@ -108,8 +108,6 @@ bool SyncerThread::ShouldRunJob(SyncSessionJobPurpose purpose,
       return false;
 
     DCHECK_EQ(wait_interval_->mode, WaitInterval::EXPONENTIAL_BACKOFF);
-    DCHECK(purpose == POLL ||
-           purpose == NUDGE);
     if ((purpose != NUDGE) || wait_interval_->had_nudge)
       return false;
   }
@@ -289,8 +287,7 @@ void GetModelSafeParamsForTypes(const ModelTypeBitSet& types,
     NOTREACHED();
 }
 
-void SyncerThread::ScheduleConfig(const TimeDelta& delay,
-    const ModelTypeBitSet& types) {
+void SyncerThread::ScheduleConfig(const ModelTypeBitSet& types) {
   if (!thread_.IsRunning()) {
     NOTREACHED();
     return;
@@ -302,14 +299,20 @@ void SyncerThread::ScheduleConfig(const TimeDelta& delay,
                              &routes, &workers);
 
   thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &SyncerThread::ScheduleConfigImpl, delay, routes, workers));
+      this, &SyncerThread::ScheduleConfigImpl, routes, workers));
 }
 
-void SyncerThread::ScheduleConfigImpl(const TimeDelta& delay,
-    const ModelSafeRoutingInfo& routing_info,
+void SyncerThread::ScheduleConfigImpl(const ModelSafeRoutingInfo& routing_info,
     const std::vector<ModelSafeWorker*>& workers) {
   DCHECK_EQ(MessageLoop::current(), thread_.message_loop());
-  NOTIMPLEMENTED() << "TODO(tim)";
+
+  // TODO(tim): config-specific GetUpdatesCallerInfo value?
+  SyncSession* session = new SyncSession(session_context_.get(), this,
+      SyncSourceInfo(GetUpdatesCallerInfo::FIRST_UPDATE,
+          sessions::MakeTypePayloadMapFromRoutingInfo(
+              routing_info, std::string())),
+      routing_info, workers);
+  ScheduleSyncSessionJob(TimeDelta::FromSeconds(0), CONFIGURATION, session);
 }
 
 void SyncerThread::ScheduleSyncSessionJob(const base::TimeDelta& delay,
diff --git a/chrome/browser/sync/engine/syncer_thread2.h b/chrome/browser/sync/engine/syncer_thread2.h
index a9848ff..89e0c96 100644
--- a/chrome/browser/sync/engine/syncer_thread2.h
+++ b/chrome/browser/sync/engine/syncer_thread2.h
@@ -63,8 +63,7 @@ class SyncerThread : public sessions::SyncSession::Delegate {
   void ScheduleNudgeWithPayloads(
       const base::TimeDelta& delay, NudgeSource source,
       const sessions::TypePayloadMap& types_with_payloads);
-  void ScheduleConfig(const base::TimeDelta& delay,
-                      const syncable::ModelTypeBitSet& types);
+  void ScheduleConfig(const syncable::ModelTypeBitSet& types);
   void ScheduleClearUserData();
 
   // Change status of notifications in the SyncSessionContext.
@@ -154,8 +153,7 @@ class SyncerThread : public sessions::SyncSession::Delegate {
   void ScheduleNudgeImpl(const base::TimeDelta& delay,
                          NudgeSource source,
                          const sessions::TypePayloadMap& types_with_payloads);
-  void ScheduleConfigImpl(const base::TimeDelta& delay,
-                          const ModelSafeRoutingInfo& routing_info,
+  void ScheduleConfigImpl(const ModelSafeRoutingInfo& routing_info,
                           const std::vector<ModelSafeWorker*>& workers);
   void ScheduleClearUserDataImpl();
 
diff --git a/chrome/browser/sync/engine/syncer_thread2_unittest.cc b/chrome/browser/sync/engine/syncer_thread2_unittest.cc
index 1268593..0a54d02 100644
--- a/chrome/browser/sync/engine/syncer_thread2_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_thread2_unittest.cc
@@ -424,7 +424,7 @@ TEST_F(SyncerThread2Test, ThrottlingDoesThrottle) {
   FlushLastTask(&done);
 
   syncer_thread()->Start(SyncerThread::CONFIGURATION_MODE);
-  syncer_thread()->ScheduleConfig(zero(), types);
+  syncer_thread()->ScheduleConfig(types);
   FlushLastTask(&done);
 }
 
@@ -455,9 +455,13 @@ TEST_F(SyncerThread2Test, ThrottlingExpires) {
 // Test nudges / polls don't run in config mode and config tasks do.
 TEST_F(SyncerThread2Test, ConfigurationMode) {
   TimeDelta poll(TimeDelta::FromMilliseconds(15));
+  SyncShareRecords records;
   base::WaitableEvent done(false, false);
+  base::WaitableEvent* dummy = NULL;
   syncer_thread()->OnReceivedLongPollIntervalUpdate(poll);
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(0);
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+           WithArg<0>(RecordSyncShare(&records, 1U, dummy))));
   syncer_thread()->Start(SyncerThread::CONFIGURATION_MODE);
   syncable::ModelTypeBitSet nudge_types;
   nudge_types[syncable::AUTOFILL] = true;
@@ -468,8 +472,13 @@ TEST_F(SyncerThread2Test, ConfigurationMode) {
   config_types[syncable::BOOKMARKS] = true;
   // TODO(tim): This will fail once CONFIGURATION tasks are implemented. Update
   // the EXPECT when that happens.
-  syncer_thread()->ScheduleConfig(zero(), config_types);
+  syncer_thread()->ScheduleConfig(config_types);
   FlushLastTask(&done);
+  syncer_thread()->Stop();
+
+  EXPECT_EQ(1U, records.snapshots.size());
+  EXPECT_TRUE(CompareModelTypeBitSetToTypePayloadMap(config_types,
+      records.snapshots[0]->source.types));
 }
 
 // Test that exponential backoff is properly triggered.
@@ -560,7 +569,7 @@ TEST_F(SyncerThread2Test, BackoffDropsJobs) {
   EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
 
   syncer_thread()->Start(SyncerThread::CONFIGURATION_MODE);
-  syncer_thread()->ScheduleConfig(zero(), types);
+  syncer_thread()->ScheduleConfig(types);
   FlushLastTask(&done);
 
   syncer_thread()->Start(SyncerThread::NORMAL_MODE);
@@ -677,6 +686,14 @@ TEST_F(SyncerThread2Test, SyncerSteps) {
   syncer_thread()->Stop();
   Mock::VerifyAndClearExpectations(syncer());
 
+  // Configuration.
+  EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES));
+  syncer_thread()->Start(SyncerThread::CONFIGURATION_MODE);
+  syncer_thread()->ScheduleConfig(ModelTypeBitSet());
+  FlushLastTask(&done);
+  syncer_thread()->Stop();
+  Mock::VerifyAndClearExpectations(syncer());
+
   // Poll.
   EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END))
       .Times(AtLeast(1))
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 681f8a9..80564de 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -392,10 +392,11 @@ void SyncBackendHost::ConfigureDataTypes(
   // downloading updates for newly added data types.  Once this is
   // complete, the configure_ready_task_ is run via an
   // OnInitializationComplete notification.
-  if (deleted_type || !core_->syncapi()->InitialSyncEndedForAllEnabledTypes())
+  if (deleted_type || !core_->syncapi()->InitialSyncEndedForAllEnabledTypes()) {
     // We can only nudge when we've either deleted a dataype or added one, else
     // we break all the profile sync unit tests.
     RequestNudge();
+  }
 }
 
 void SyncBackendHost::RequestNudge() {