Revert 146262 - Revert "Revert 142517 - [Sync] Refactor sync configuration logic."

sync/internal_api/sync_manager.cc

 // Copyright (c) 2012 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 "sync/internal_api/public/sync_manager.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 179 matching lines @@
             UnrecoverableErrorHandler* unrecoverable_error_handler,
             ReportUnrecoverableErrorFunction
                 report_unrecoverable_error_function);
 
   // Sign into sync with given credentials.
   // We do not verify the tokens given. After this call, the tokens are set
   // and the sync DB is open. True if successful, false if something
   // went wrong.
   bool SignIn(const SyncCredentials& credentials);
 
-  // Purge from the directory those types with non-empty progress markers
-  // but without initial synced ended set.
-  // Returns false if an error occurred, true otherwise.
-  bool PurgePartiallySyncedTypes();
-
   // Update tokens that we're using in Sync. Email must stay the same.
   void UpdateCredentials(const SyncCredentials& credentials);
 
   // Called when the user disables or enables a sync type.
   void UpdateEnabledTypes(const ModelTypeSet& enabled_types);
 
   // Tell the sync engine to start the syncing process.
   void StartSyncingNormally(
       const syncer::ModelSafeRoutingInfo& routing_info);
 
@@ skipping 140 matching lines @@
                                 syncer::ModelType type,
                                 ChangeReorderBuffer* buffer,
                                 Cryptographer* cryptographer,
                                 const syncable::EntryKernel& original,
                                 bool existed_before,
                                 bool exists_now);
 
   // Called only by our NetworkChangeNotifier.
   virtual void OnIPAddressChanged() OVERRIDE;
 
-  ModelTypeSet GetTypesWithEmptyProgressMarkerToken(ModelTypeSet types) {
-    DCHECK(initialized_);
-    syncer::ModelTypeSet result;
-    for (syncer::ModelTypeSet::Iterator i = types.First();
-         i.Good(); i.Inc()) {
-      sync_pb::DataTypeProgressMarker marker;
-      directory()->GetDownloadProgress(i.Get(), &marker);
-
-      if (marker.token().empty())
-        result.Put(i.Get());
-
-    }
-    return result;
-  }
-
   syncer::ModelTypeSet InitialSyncEndedTypes() {
     DCHECK(initialized_);
     return directory()->initial_sync_ended_types();
   }
 
   // SyncEngineEventListener implementation.
   virtual void OnSyncEngineEvent(const SyncEngineEvent& event) OVERRIDE;
 
   // ServerConnectionEventListener implementation.
   virtual void OnServerConnectionEvent(
       const ServerConnectionEvent& event) OVERRIDE;
 
   // JsBackend implementation.
   virtual void SetJsEventHandler(
       const WeakHandle<JsEventHandler>& event_handler) OVERRIDE;
   virtual void ProcessJsMessage(
       const std::string& name, const JsArgList& args,
       const WeakHandle<JsReplyHandler>& reply_handler) OVERRIDE;
 
-  void SetSyncSchedulerForTest(scoped_ptr<SyncScheduler> scheduler);
-
  private:
   struct NotificationInfo {
     int total_count;
     std::string payload;
 
     NotificationInfo() : total_count(0) {}
 
     ~NotificationInfo() {}
 
     // Returned pointer owned by the caller.
@@ skipping 353 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
   ReadTransaction trans(FROM_HERE, GetUserShare());
   trans.GetWrappedTrans()->OnUnrecoverableError(
       FROM_HERE, "Simulating unrecoverable error for testing purposes.");
 }
 
 syncer::ModelTypeSet SyncManager::InitialSyncEndedTypes() {
   return data_->InitialSyncEndedTypes();
 }
 
-syncer::ModelTypeSet SyncManager::GetTypesWithEmptyProgressMarkerToken(
-    syncer::ModelTypeSet types) {
-  return data_->GetTypesWithEmptyProgressMarkerToken(types);
-}
-
-bool SyncManager::PurgePartiallySyncedTypes() {
-  return data_->PurgePartiallySyncedTypes();
-}
-
 void SyncManager::StartSyncingNormally(
     const syncer::ModelSafeRoutingInfo& routing_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
   data_->StartSyncingNormally(routing_info);
 }
 
 void SyncManager::SetEncryptionPassphrase(const std::string& passphrase,
                                           bool is_explicit) {
   DCHECK(thread_checker_.CalledOnValidThread());
   data_->SetEncryptionPassphrase(passphrase, is_explicit);
@@ skipping 24 matching lines @@
 
 bool SyncManager::EncryptEverythingEnabledForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return trans.GetCryptographer()->encrypt_everything();
 }
 
 bool SyncManager::IsUsingExplicitPassphrase() {
   return data_ && data_->IsUsingExplicitPassphrase();
 }
 
-void SyncManager::ConfigureSyncer(
-    ConfigureReason reason,
-    const syncer::ModelTypeSet& types_to_config,
-    const syncer::ModelSafeRoutingInfo& new_routing_info,
-    const base::Closure& ready_task,
-    const base::Closure& retry_task) {
+void SyncManager::RequestCleanupDisabledTypes(
+    const syncer::ModelSafeRoutingInfo& routing_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!ready_task.is_null());
-  DCHECK(!retry_task.is_null());
+  if (data_->scheduler()) {
+    data_->session_context()->set_routing_info(routing_info);
+    data_->scheduler()->CleanupDisabledTypes();
+  }
+}
 
-  // TODO(zea): set this based on whether cryptographer has keystore
-  // encryption key or not (requires opening a transaction). crbug.com/129665.
-  ConfigurationParams::KeystoreKeyStatus keystore_key_status =
-      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY;
-
-  ConfigurationParams params(GetSourceFromReason(reason),
-                             types_to_config,
-                             new_routing_info,
-                             keystore_key_status,
-                             ready_task);
-
+void SyncManager::RequestConfig(
+    const syncer::ModelSafeRoutingInfo& routing_info,
+    const ModelTypeSet& types, ConfigureReason reason) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!data_->scheduler()) {
     LOG(INFO)
-        << "SyncManager::ConfigureSyncer: could not configure because "
-        << "scheduler is null";
-    params.ready_task.Run();
+        << "SyncManager::RequestConfig: bailing out because scheduler is "
+        << "null";
     return;
   }
+  StartConfigurationMode(base::Closure());
+  data_->session_context()->set_routing_info(routing_info);
+  data_->scheduler()->ScheduleConfiguration(types, GetSourceFromReason(reason));
+}
 
-  data_->scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE);
-  if (!data_->scheduler()->ScheduleConfiguration(params))
-    retry_task.Run();
-
+void SyncManager::StartConfigurationMode(const base::Closure& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!data_->scheduler()) {
+    LOG(INFO)
+        << "SyncManager::StartConfigurationMode: could not start "
+        << "configuration mode because because scheduler is null";
+    return;
+  }
+  data_->scheduler()->Start(
+      syncer::SyncScheduler::CONFIGURATION_MODE, callback);
 }
 
 bool SyncManager::SyncInternal::Init(
     const FilePath& database_location,
     const WeakHandle<JsEventHandler>& event_handler,
     const std::string& sync_server_and_path,
     int port,
     bool use_ssl,
     const scoped_refptr<base::TaskRunner>& blocking_task_runner,
     HttpPostProviderFactory* post_factory,
@@ skipping 76 matching lines @@
         workers,
         extensions_activity_monitor,
         &throttled_data_type_tracker_,
         listeners,
         &debug_info_event_listener_,
         &traffic_recorder_));
     session_context()->set_account_name(credentials.email);
     scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer()));
   }
 
-  bool success = SignIn(credentials);
+  bool signed_in = SignIn(credentials);
 
-  if (success) {
+  if (signed_in) {
     if (scheduler()) {
-      scheduler()->Start(syncer::SyncScheduler::CONFIGURATION_MODE);
+      scheduler()->Start(
+          syncer::SyncScheduler::CONFIGURATION_MODE, base::Closure());
     }
 
     initialized_ = true;
 
-    // Unapplied datatypes (those that do not have initial sync ended set) get
-    // re-downloaded during any configuration. But, it's possible for a datatype
-    // to have a progress marker but not have initial sync ended yet, making
-    // it a candidate for migration. This is a problem, as the DataTypeManager
-    // does not support a migration while it's already in the middle of a
-    // configuration. As a result, any partially synced datatype can stall the
-    // DTM, waiting for the configuration to complete, which it never will due
-    // to the migration error. In addition, a partially synced nigori will
-    // trigger the migration logic before the backend is initialized, resulting
-    // in crashes. We therefore detect and purge any partially synced types as
-    // part of initialization.
-    if (!PurgePartiallySyncedTypes())
-      success = false;
-
     // Cryptographer should only be accessed while holding a
     // transaction.  Grabbing the user share for the transaction
     // checks the initialization state, so this must come after
     // |initialized_| is set to true.
     ReadTransaction trans(FROM_HERE, GetUserShare());
     trans.GetCryptographer()->Bootstrap(restored_key_for_bootstrapping);
     trans.GetCryptographer()->AddObserver(this);
   }
 
   // Notify that initialization is complete. Note: This should be the last to
   // execute if |signed_in| is false. Reason being in that case we would
   // post a task to shutdown sync. But if this function posts any other tasks
   // on the UI thread and if shutdown wins then that tasks would execute on
   // a freed pointer. This is because UI thread is not shut down.
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                     OnInitializationComplete(
                         MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()),
-                        success));
+                        signed_in));
 
-  if (!success && testing_mode_ == NON_TEST)
+  if (!signed_in && testing_mode_ == NON_TEST)
     return false;
 
   sync_notifier_->AddObserver(this);
 
-  return success;
+  return signed_in;
 }
 
 void SyncManager::SyncInternal::UpdateCryptographerAndNigori(
     const std::string& chrome_version,
     const base::Closure& done_callback) {
   DCHECK(initialized_);
   syncer::GetSessionName(
       blocking_task_runner_,
       base::Bind(
           &SyncManager::SyncInternal::UpdateCryptographerAndNigoriCallback,
@@ skipping 124 matching lines @@
   allstatus_.SetCryptographerReady(cryptographer->is_ready());
   allstatus_.SetCryptoHasPendingKeys(cryptographer->has_pending_keys());
   debug_info_event_listener_.SetCryptographerReady(cryptographer->is_ready());
   debug_info_event_listener_.SetCrytographerHasPendingKeys(
       cryptographer->has_pending_keys());
 }
 
 void SyncManager::SyncInternal::StartSyncingNormally(
     const syncer::ModelSafeRoutingInfo& routing_info) {
   // Start the sync scheduler.
-  if (scheduler()) {  // NULL during certain unittests.
-    // TODO(sync): We always want the newest set of routes when we switch back
-    // to normal mode. Figure out how to enforce set_routing_info is always
-    // appropriately set and that it's only modified when switching to normal
-    // mode.
+  if (scheduler()) { // NULL during certain unittests.
     session_context()->set_routing_info(routing_info);
-    scheduler()->Start(SyncScheduler::NORMAL_MODE);
+    scheduler()->Start(SyncScheduler::NORMAL_MODE, base::Closure());
   }
 }
 
 bool SyncManager::SyncInternal::OpenDirectory() {
   DCHECK(!initialized_) << "Should only happen once";
 
   // Set before Open().
   change_observer_ =
       syncer::MakeWeakHandle(js_mutation_event_observer_.AsWeakPtr());
   WeakHandle<syncable::TransactionObserver> transaction_observer(
@@ skipping 33 matching lines @@
   allstatus_.SetUniqueId(unique_id);
   sync_notifier_->SetUniqueId(unique_id);
   // TODO(tim): Remove once invalidation state has been migrated to new
   // InvalidationStateTracker store. Bug 124140.
   sync_notifier_->SetStateDeprecated(state);
 
   UpdateCredentials(credentials);
   return true;
 }
 
-bool SyncManager::SyncInternal::PurgePartiallySyncedTypes() {
-  syncer::ModelTypeSet partially_synced_types =
-      syncer::ModelTypeSet::All();
-  partially_synced_types.RemoveAll(InitialSyncEndedTypes());
-  partially_synced_types.RemoveAll(GetTypesWithEmptyProgressMarkerToken(
-      syncer::ModelTypeSet::All()));
-
-  UMA_HISTOGRAM_COUNTS("Sync.PartiallySyncedTypes",
-                       partially_synced_types.Size());
-  if (partially_synced_types.Empty())
-    return true;
-  return directory()->PurgeEntriesWithTypeIn(partially_synced_types);
-}
-
 void SyncManager::SyncInternal::UpdateCredentials(
     const SyncCredentials& credentials) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(credentials.email, share_.name);
   DCHECK(!credentials.email.empty());
   DCHECK(!credentials.sync_token.empty());
 
   observing_ip_address_changes_ = true;
   if (connection_manager()->set_auth_token(credentials.sync_token)) {
     sync_notifier_->UpdateCredentials(
@@ skipping 1162 matching lines @@
 void SyncManager::SyncInternal::AddObserver(
     SyncManager::Observer* observer) {
   observers_.AddObserver(observer);
 }
 
 void SyncManager::SyncInternal::RemoveObserver(
     SyncManager::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void SyncManager::SyncInternal::SetSyncSchedulerForTest(
-    scoped_ptr<SyncScheduler> sync_scheduler) {
-  scheduler_ = sync_scheduler.Pass();
-}
-
 SyncStatus SyncManager::GetDetailedStatus() const {
   return data_->GetStatus();
 }
 
 void SyncManager::SaveChanges() {
   DCHECK(thread_checker_.CalledOnValidThread());
   data_->SaveChanges();
 }
 
 void SyncManager::SyncInternal::SaveChanges() {
@@ skipping 10 matching lines @@
   data_->UpdateCryptographerAndNigori(
       chrome_version,
       done_callback);
 }
 
 TimeDelta SyncManager::GetNudgeDelayTimeDelta(
     const ModelType& model_type) {
   return data_->GetNudgeDelayTimeDelta(model_type);
 }
 
-void SyncManager::SetSyncSchedulerForTest(scoped_ptr<SyncScheduler> scheduler) {
-  data_->SetSyncSchedulerForTest(scheduler.Pass());
-}
-
 syncer::ModelTypeSet SyncManager::GetEncryptedDataTypesForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return GetEncryptedTypes(&trans);
 }
 
 bool SyncManager::ReceivedExperiment(syncer::Experiments* experiments)
     const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   ReadNode node(&trans);
   if (node.InitByTagLookup(kNigoriTag) != syncer::BaseNode::INIT_OK) {
@@ skipping 69 matching lines @@
 bool InitialSyncEndedForTypes(syncer::ModelTypeSet types,
                               syncer::UserShare* share) {
   for (syncer::ModelTypeSet::Iterator i = types.First();
        i.Good(); i.Inc()) {
     if (!share->directory->initial_sync_ended_for_type(i.Get()))
       return false;
   }
   return true;
 }
 
+syncer::ModelTypeSet GetTypesWithEmptyProgressMarkerToken(
+    syncer::ModelTypeSet types,
+    syncer::UserShare* share) {
+  syncer::ModelTypeSet result;
+  for (syncer::ModelTypeSet::Iterator i = types.First();
+       i.Good(); i.Inc()) {
+    sync_pb::DataTypeProgressMarker marker;
+    share->directory->GetDownloadProgress(i.Get(), &marker);
+
+    if (marker.token().empty())
+      result.Put(i.Get());
+
+  }
+  return result;
+}
+
 }  // namespace syncer