[Sync] Make BackendMigrator not wait for full sync cycles

chrome/browser/sync/glue/sync_backend_host.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 "build/build_config.h"
 
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/task.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/tracked.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/net/gaia/token_service.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/glue/change_processor.h"
 #include "chrome/browser/sync/glue/http_bridge.h"
 #include "chrome/browser/sync/internal_api/base_transaction.h"
+#include "chrome/browser/sync/internal_api/read_transaction.h"
 #include "chrome/browser/sync/internal_api/sync_manager.h"
 #include "chrome/browser/sync/glue/sync_backend_registrar.h"
 #include "chrome/browser/sync/notifier/sync_notifier.h"
+#include "chrome/browser/sync/protocol/sync.pb.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 // TODO(tim): Remove this! We should have a syncapi pass-thru instead.
 #include "chrome/browser/sync/syncable/directory_manager.h"  // Cryptographer.
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/net/gaia/gaia_constants.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/browser_thread.h"
 #include "content/common/notification_service.h"
 #include "webkit/glue/webkit_glue.h"
@@ skipping 11 matching lines @@
 using sessions::SyncSessionSnapshot;
 using sync_api::SyncCredentials;
 
 SyncBackendHost::SyncBackendHost(Profile* profile)
     : core_(new Core(profile,
                      ALLOW_THIS_IN_INITIALIZER_LIST(this))),
       initialization_state_(NOT_INITIALIZED),
       sync_thread_("Chrome_SyncThread"),
       frontend_loop_(MessageLoop::current()),
       profile_(profile),
+      name_(profile_->GetDebugName()),
       sync_notifier_factory_(webkit_glue::GetUserAgent(GURL()),
                              profile_->GetRequestContext(),
                              *CommandLine::ForCurrentProcess()),
       frontend_(NULL),
       sync_data_folder_path_(
           profile_->GetPath().Append(kSyncDataFolderName)),
       last_auth_error_(AuthError::None()) {
 }
 
 SyncBackendHost::SyncBackendHost()
@@ skipping 202 matching lines @@
   DCHECK_GT(initialization_state_, NOT_INITIALIZED);
 
   pending_config_mode_state_.reset(new PendingConfigureDataTypesState());
   pending_config_mode_state_->ready_task = ready_task;
   pending_config_mode_state_->types_to_add = types_to_add_with_nigori;
   pending_config_mode_state_->added_types =
       registrar_->ConfigureDataTypes(types_to_add_with_nigori,
                                      types_to_remove_with_nigori);
   pending_config_mode_state_->reason = reason;
 
-  // Cleanup disabled types before starting configuration so that
-  // callers can assume that the data types are cleaned up once
-  // configuration is done.
-  if (!types_to_remove.empty()) {
-    sync_thread_.message_loop()->PostTask(
-        FROM_HERE,
-        NewRunnableMethod(
-            core_.get(),
-            &SyncBackendHost::Core::DoRequestCleanupDisabledTypes));
-  }
-
-  StartConfiguration(NewCallback(core_.get(),
-      &SyncBackendHost::Core::FinishConfigureDataTypes));
+  StartConfiguration(!types_to_remove_with_nigori.empty());
 }
 
-void SyncBackendHost::StartConfiguration(Callback0::Type* callback) {
-  // Put syncer in the config mode. DTM will put us in normal mode once it is
-  // done. This is to ensure we dont do a normal sync when we are doing model
-  // association.
-  sync_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-      core_.get(), &SyncBackendHost::Core::DoStartConfiguration, callback));
+void SyncBackendHost::StartConfiguration(bool cleanup_disabled_types) {

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

I'll point out the bools vs enum style guide at
http://dev.chromium.org/developers/coding-style#Code_Formatting

I'll let you make the call.

+  sync_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncBackendHost::Core::DoStartConfiguration, core_.get(),
+                 cleanup_disabled_types));
 }
 
 void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop() {
   DCHECK_EQ(MessageLoop::current(), frontend_loop_);
   // Nudge the syncer. This is necessary for both datatype addition/deletion.
   //
   // Deletions need a nudge in order to ensure the deletion occurs in a timely
   // manner (see issue 56416).
   //
   // In the case of additions, on the next sync cycle, the syncer should
   // notice that the routing info has changed and start the process of
   // downloading updates for newly added data types.  Once this is
   // complete, the configure_state_.ready_task_ is run via an
   // OnInitializationComplete notification.
 
-  VLOG(1) << "Syncer in config mode. SBH executing"
+  VLOG(1) << "Syncer in config mode. SBH executing "
           << "FinishConfigureDataTypesOnFrontendLoop";
 
   if (pending_config_mode_state_->added_types.empty() &&
       !core_->sync_manager()->InitialSyncEndedForAllEnabledTypes()) {
     LOG(WARNING) << "No new types, but initial sync not finished."
                  << "Possible sync db corruption / removal.";
     // TODO(tim): Log / UMA / count this somehow?
     // TODO(tim): If no added types, we could (should?) config only for
     // types that are needed... but this is a rare corruption edge case or
     // implies the user mucked around with their syncdb, so for now do all.
     pending_config_mode_state_->added_types =
         pending_config_mode_state_->types_to_add;
   }
 
   // If we've added types, we always want to request a nudge/config (even if
   // the initial sync is ended), in case we could not decrypt the data.
   if (pending_config_mode_state_->added_types.empty()) {
-    VLOG(1) << "SyncBackendHost(" << this << "): No new types added. "
-            << "Calling ready_task directly";
+    VLOG(1) << name_ << ": No new types added; calling ready_task directly";
     // No new types - just notify the caller that the types are available.
     pending_config_mode_state_->ready_task.Run(true);
   } else {
     pending_download_state_.reset(pending_config_mode_state_.release());
 
     // Always configure nigori if it's enabled.
     syncable::ModelTypeSet types_to_config =
         pending_download_state_->added_types;
     if (IsNigoriEnabled()) {
       types_to_config.insert(syncable::NIGORI);
     }
-    VLOG(1) <<  "SyncBackendHost(" << this << "):New Types added. "
-            << "Calling DoRequestConfig";
+    VLOG(1) << name_ << ": types " << ModelTypeSetToString(types_to_config)
+            << "added; calling DoRequestConfig";
     sync_thread_.message_loop()->PostTask(FROM_HERE,
          NewRunnableMethod(core_.get(),
                            &SyncBackendHost::Core::DoRequestConfig,
                            syncable::ModelTypeBitSetFromSet(types_to_config),
                            pending_download_state_->reason));
   }
 
   pending_config_mode_state_.reset();
 
   // Notify the SyncManager about the new types.
   sync_thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(core_.get(),
                         &SyncBackendHost::Core::DoUpdateEnabledTypes));
 }
 
 void SyncBackendHost::EncryptDataTypes(
     const syncable::ModelTypeSet& encrypted_types) {
   sync_thread_.message_loop()->PostTask(FROM_HERE,
      NewRunnableMethod(core_.get(),
                        &SyncBackendHost::Core::DoEncryptDataTypes,
                        encrypted_types));
 }
 
 syncable::ModelTypeSet SyncBackendHost::GetEncryptedDataTypes() const {
   DCHECK_GT(initialization_state_, NOT_INITIALIZED);
   return core_->sync_manager()->GetEncryptedDataTypes();
 }
 
+syncable::ModelTypeSet SyncBackendHost::GetPurgedDataTypes() const {
+  DCHECK_GT(initialization_state_, NOT_INITIALIZED);
+  sync_api::ReadTransaction trans(FROM_HERE, GetUserShare());
+  syncable::ModelTypeSet purged_data_types;
+  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
+       i < syncable::MODEL_TYPE_COUNT; ++i) {
+    syncable::ModelType type = syncable::ModelTypeFromInt(i);
+    sync_pb::DataTypeProgressMarker progress_marker;
+    trans.GetLookup()->GetDownloadProgress(type, &progress_marker);

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

I like the GetDownloadProgress approach, but want to make sure there isn't a way
we can just let the initial snapshot (sent by
SyncScheduler::SendInitialSnapshot) bubble up so we can grab this without having
to add any accessors?  The point of that snapshot was intended to be a "here's
the initial state of the world", but I forget why we don't let it bubble up
here.  There be a valid / twisted reason.

If we do keep GetDownloadProgress, the only thing I'd like to change is this
conversion to a ModelTypeSet, since technically,  an empty progress marker does
not imply purged... it could simply be that the type wasn't ever enabled.  
Seems like we should just pass the sync_pb::DataTypeProgressMarker, but I could
be missing something.  Basically I don't like introducing the "purged types"
concept here (at least, at the SBH layer).

+    if (progress_marker.token().empty()) {
+      purged_data_types.insert(type);
+    }
+  }
+  return purged_data_types;
+}
+
 void SyncBackendHost::ActivateDataType(
     syncable::ModelType type, ModelSafeGroup group,
     ChangeProcessor* change_processor) {
   registrar_->ActivateDataType(type, group, change_processor, GetUserShare());
 }
 
 void SyncBackendHost::DeactivateDataType(syncable::ModelType type) {
   registrar_->DeactivateDataType(type);
 }
 
@@ skipping 242 matching lines @@
   DCHECK(MessageLoop::current() == host_->sync_thread_.message_loop());
   sync_manager_->EncryptDataTypes(encrypted_types);
 }
 
 void SyncBackendHost::Core::DoRequestConfig(
     const syncable::ModelTypeBitSet& types_to_config,
     sync_api::ConfigureReason reason) {
   sync_manager_->RequestConfig(types_to_config, reason);
 }
 
-void SyncBackendHost::Core::DoStartConfiguration(Callback0::Type* callback) {
-  sync_manager_->StartConfigurationMode(callback);
+void SyncBackendHost::Core::DoStartConfiguration(bool cleanup_disabled_types) {
+  base::Closure done_callback =
+      base::Bind(&SyncBackendHost::Core::FinishConfigureDataTypes,this);
+  base::Closure on_config_done_callback =
+      cleanup_disabled_types ?
+      base::Bind(&SyncBackendHost::Core::DoRequestCleanupDisabledTypes,
+                 this, done_callback) :
+      done_callback;
+  sync_manager_->StartConfigurationMode(on_config_done_callback);
 }
 
 void SyncBackendHost::Core::DoShutdown(bool sync_disabled) {
   DCHECK(MessageLoop::current() == host_->sync_thread_.message_loop());
 
   save_changes_timer_.Stop();
   sync_manager_->Shutdown();  // Stops the SyncerThread.
   sync_manager_->RemoveObserver(this);
   sync_manager_.reset();
   registrar_->OnSyncerShutdownComplete();
@@ skipping 67 matching lines @@
 
   // 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.
   if (host_->pending_download_state_.get()) {
     scoped_ptr<PendingConfigureDataTypesState> state(
         host_->pending_download_state_.release());
     DCHECK(
         std::includes(state->types_to_add.begin(), state->types_to_add.end(),
                       state->added_types.begin(), state->added_types.end()));
+    VLOG(1)
+        << "Added types: "
+        << syncable::ModelTypeSetToString(state->added_types)
+        << ", configured types: "
+        << syncable::ModelTypeBitSetToString(snapshot->initial_sync_ended);
     syncable::ModelTypeBitSet added_types =
         syncable::ModelTypeBitSetFromSet(state->added_types);
     bool found_all_added =
         (added_types & snapshot->initial_sync_ended) == added_types;
     state->ready_task.Run(found_all_added);
     if (!found_all_added)
       return;
   }
 
   if (host_->initialized())
@@ skipping 140 matching lines @@
 void SyncBackendHost::Core::StartSavingChanges() {
   save_changes_timer_.Start(
       base::TimeDelta::FromSeconds(kSaveChangesIntervalSeconds),
       this, &Core::SaveChanges);
 }
 
 void SyncBackendHost::Core::DoRequestClearServerData() {
   sync_manager_->RequestClearServerData();
 }
 
-void SyncBackendHost::Core::DoRequestCleanupDisabledTypes() {
-  sync_manager_->RequestCleanupDisabledTypes();
+void SyncBackendHost::Core::DoRequestCleanupDisabledTypes(
+    const base::Closure& callback) {
+  sync_manager_->RequestCleanupDisabledTypes(callback);
 }
 
 void SyncBackendHost::Core::SaveChanges() {
   sync_manager_->SaveChanges();
 }
 
 void SyncBackendHost::Core::DeleteSyncDataFolder() {
   if (file_util::DirectoryExists(host_->sync_data_folder_path())) {
     if (!file_util::Delete(host_->sync_data_folder_path(), true))
       LOG(DFATAL) << "Could not delete the Sync Data folder.";
   }
 }
 
 }  // namespace browser_sync