sync: repairs + improvement to migration code path

BUG=80290
TEST=BackendMigratorTest, manual test of migration.  Integration tests are coming

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

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

12 files changed, 24 insertions, 47 deletions

diff --git a/chrome/browser/sync/backend_migrator.cc b/chrome/browser/sync/backend_migrator.cc
index 9ce0203..481d455 100644
--- a/chrome/browser/sync/backend_migrator.cc
+++ b/chrome/browser/sync/backend_migrator.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/string_number_conversions.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/glue/data_type_manager.h"
 #include "chrome/browser/sync/sessions/session_state.h"
@@ -46,7 +47,7 @@ void BackendMigrator::MigrateTypes(const syncable::ModelTypeSet& types) {
     to_migrate_ = temp;
   }
 
-  if (state_ >= STARTED_MIGRATING) {
+  if (HasStartedMigrating()) {
     VLOG(1) << "BackendMigrator::MigrateTypes: STARTED_MIGRATING early-out.";
     restart_migration_ = true;
     return;
@@ -159,6 +160,11 @@ void BackendMigrator::Observe(NotificationType type,
   } else if (state_ == REENABLING_TYPES) {
     // We're done!
     state_ = IDLE;
+
+    std::stringstream ss;
+    std::copy(to_migrate_.begin(), to_migrate_.end(),
+              std::ostream_iterator<syncable::ModelType>(ss, ","));
+    VLOG(1) << "BackendMigrator: Migration complete for: " << ss.str();
     to_migrate_.clear();
   }
 }
diff --git a/chrome/browser/sync/backend_migrator.h b/chrome/browser/sync/backend_migrator.h
index 230e7d8..2f2a175 100644
--- a/chrome/browser/sync/backend_migrator.h
+++ b/chrome/browser/sync/backend_migrator.h
@@ -27,7 +27,6 @@ class BackendMigrator : public NotificationObserver,
     WAITING_TO_START,   // Waiting for previous configuration to finish.
     DISABLING_TYPES,    // Exit criteria: SYNC_CONFIGURE_DONE for enabled
                         // types _excluding_ |to_migrate_|.
-    STARTED_MIGRATING = DISABLING_TYPES,
     WAITING_FOR_PURGE,  // Exit criteria: SyncCycleEnded for enabled types
                         // excluding |to_migrate|
     REENABLING_TYPES,   // Exit criteria: SYNC_CONFIGURE_DONE for enabled
diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc
index 575113e..dea0085 100644
--- a/chrome/browser/sync/engine/all_status.cc
+++ b/chrome/browser/sync/engine/all_status.cc
@@ -110,7 +110,6 @@ void AllStatus::OnSyncEngineEvent(const SyncEngineEvent& event) {
     case SyncEngineEvent::UPDATED_TOKEN:
     case SyncEngineEvent::CLEAR_SERVER_DATA_FAILED:
     case SyncEngineEvent::CLEAR_SERVER_DATA_SUCCEEDED:
-    case SyncEngineEvent::MIGRATION_NEEDED_FOR_TYPES:
        break;
     default:
       LOG(ERROR) << "Unrecognized Syncer Event: " << event.what_happened;
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index b701c00..d52326f 100644
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -2594,12 +2594,6 @@ void SyncManager::SyncInternal::OnSyncEngineEvent(
                       OnUpdatedToken(event.updated_token));
     return;
   }
-
-  if (event.what_happened == SyncEngineEvent::MIGRATION_NEEDED_FOR_TYPES) {
-    FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
-                      OnMigrationNeededForTypes(event.types_to_migrate));
-    return;
-  }
 }
 
 void SyncManager::SyncInternal::SetParentJsEventRouter(
diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h
index ff3a1b4..1011a29 100644
--- a/chrome/browser/sync/engine/syncapi.h
+++ b/chrome/browser/sync/engine/syncapi.h
@@ -778,10 +778,6 @@ class SyncManager {
     // Called when a new auth token is provided by the sync server.
     virtual void OnUpdatedToken(const std::string& token) = 0;
 
-    // Called when migration is required for |types|.
-    virtual void OnMigrationNeededForTypes(
-        const syncable::ModelTypeSet& types) = 0;
-
     // Called when user interaction is required to obtain a valid passphrase.
     // If the passphrase is required to decrypt something that has
     // already been encrypted (and thus has to match the existing key),
diff --git a/chrome/browser/sync/engine/syncer_proto_util.cc b/chrome/browser/sync/engine/syncer_proto_util.cc
index 63facd4..a3a6177 100644
--- a/chrome/browser/sync/engine/syncer_proto_util.cc
+++ b/chrome/browser/sync/engine/syncer_proto_util.cc
@@ -90,9 +90,7 @@ void SyncerProtoUtil::HandleMigrationDoneResponse(
     to_migrate.insert(syncable::GetModelTypeFromExtensionFieldNumber(
         response->migrated_data_type_id(i)));
   }
-  SyncEngineEvent event(SyncEngineEvent::MIGRATION_NEEDED_FOR_TYPES);
-  event.types_to_migrate = to_migrate;
-  session->context()->NotifyListeners(event);
+  session->status_controller()->set_types_needing_local_migration(to_migrate);
 }
 
 // static
diff --git a/chrome/browser/sync/engine/syncer_types.h b/chrome/browser/sync/engine/syncer_types.h
index 6f45395..bcb59e0 100644
--- a/chrome/browser/sync/engine/syncer_types.h
+++ b/chrome/browser/sync/engine/syncer_types.h
@@ -89,9 +89,6 @@ struct SyncEngineEvent {
     // New token in updated_token.
     UPDATED_TOKEN,
 
-    // A list of types to migrate is in |types_to_migrate|.
-    MIGRATION_NEEDED_FOR_TYPES,
-
     // This is sent after the Syncer (and SyncerThread) have initiated self
     // halt due to no longer being permitted to communicate with the server.
     // The listener should sever the sync / browser connections and delete sync
@@ -114,8 +111,6 @@ struct SyncEngineEvent {
 
   // Update-Client-Auth returns a new token for sync use.
   std::string updated_token;
-
-  syncable::ModelTypeSet types_to_migrate;
 };
 
 class SyncEngineEventListener {
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 787a4e0..6e143a3 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -893,6 +893,11 @@ void SyncBackendHost::Core::HandleSyncCycleCompletedOnFrontendLoop(
 
   host_->last_snapshot_.reset(snapshot);
 
+  const syncable::ModelTypeSet& to_migrate =
+      snapshot->syncer_status.types_needing_local_migration;
+  if (!to_migrate.empty())
+    host_->frontend_->OnMigrationNeededForTypes(to_migrate);
+
   // If we are waiting for a configuration change, check here to see
   // if this sync cycle has initialized all of the types we've been
   // waiting for.
@@ -906,10 +911,7 @@ void SyncBackendHost::Core::HandleSyncCycleCompletedOnFrontendLoop(
         found_all_added &= snapshot->initial_sync_ended.test(*it);
     }
     if (!found_all_added) {
-      LOG(WARNING) << "Update didn't return updates for all types requested.";
-      // This is typically an error case and sync won't make forward progress.
-      // The exception is backend migration-during-configuration. We'll reset
-      // state below to allow for a future call to ConfigureDataTypes.
+      NOTREACHED() << "Update didn't return updates for all types requested.";
     } else {
       host_->pending_download_state_->ready_task->Run();
     }
@@ -997,12 +999,6 @@ void SyncBackendHost::Core::OnUpdatedToken(const std::string& token) {
       &Core::NotifyUpdatedToken, token));
 }
 
-void SyncBackendHost::Core::OnMigrationNeededForTypes(
-    const syncable::ModelTypeSet& types) {
-  host_->frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
-      &Core::HandleMigrationNeededOnFrontendLoop, types));
-}
-
 void SyncBackendHost::Core::OnClearServerDataSucceeded() {
   host_->frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
       &Core::HandleClearServerDataSucceededOnFrontendLoop));
@@ -1058,16 +1054,6 @@ void SyncBackendHost::Core::HandleAuthErrorEventOnFrontendLoop(
   host_->frontend_->OnAuthError();
 }
 
-void SyncBackendHost::Core::HandleMigrationNeededOnFrontendLoop(
-    const syncable::ModelTypeSet& types) {
-  if (!host_ || !host_->frontend_)
-    return;
-
-  DCHECK_EQ(MessageLoop::current(), host_->frontend_loop_);
-
-  host_->frontend_->OnMigrationNeededForTypes(types);
-}
-
 void SyncBackendHost::Core::RouteJsEventOnFrontendLoop(
     const std::string& name, const JsArgList& args,
     const JsEventHandler* target) {
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 3f8d960..b5c2b70 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -282,8 +282,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
     virtual void OnPassphraseAccepted(const std::string& bootstrap_token);
     virtual void OnStopSyncingPermanently();
     virtual void OnUpdatedToken(const std::string& token);
-    virtual void OnMigrationNeededForTypes(
-        const syncable::ModelTypeSet& types);
     virtual void OnClearServerDataFailed();
     virtual void OnClearServerDataSucceeded();
     virtual void OnEncryptionComplete(
@@ -483,10 +481,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
     void HandleClearServerDataSucceededOnFrontendLoop();
     void HandleClearServerDataFailedOnFrontendLoop();
 
-    // Called to handle a migration for |types|.
-    void HandleMigrationNeededOnFrontendLoop(
-        const syncable::ModelTypeSet& types);
-
     // Called from Core::OnInitializationComplete to handle updating
     // frontend thread components.
     void HandleInitalizationCompletedOnFrontendLoop();
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index 2a2c94a..29655a8 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -75,6 +75,10 @@ struct SyncerStatus {
   // Download event counters.
   int num_updates_downloaded_total;
   int num_tombstone_updates_downloaded_total;
+
+  // If the syncer encountered a MIGRATION_DONE code, these are the types that
+  // the client must now "migrate", by purging and re-downloading all updates.
+  syncable::ModelTypeSet types_needing_local_migration;
 };
 
 // Counters for various errors that can occur repeatedly during a sync session.
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index c8d6866..8266f0f 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -51,6 +51,11 @@ void StatusController::increment_num_updates_downloaded_by(int value) {
   shared_.syncer_status.mutate()->num_updates_downloaded_total += value;
 }
 
+void StatusController::set_types_needing_local_migration(
+    const syncable::ModelTypeSet& types) {
+  shared_.syncer_status.mutate()->types_needing_local_migration = types;
+}
+
 void StatusController::increment_num_tombstone_updates_downloaded_by(
     int value) {
   shared_.syncer_status.mutate()->num_tombstone_updates_downloaded_total +=
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index 805ff28..d4c912a 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -212,6 +212,7 @@ class StatusController {
   void increment_num_successful_bookmark_commits();
   void increment_num_updates_downloaded_by(int value);
   void increment_num_tombstone_updates_downloaded_by(int value);
+  void set_types_needing_local_migration(const syncable::ModelTypeSet& types);
   void set_unsynced_handles(const std::vector<int64>& unsynced_handles);
 
   void set_commit_set(const OrderedCommitSet& commit_set);