[Sync] Remove CleanupDisabledTypes command and move purge logic into SyncManager.

sync/internal_api/sync_manager_impl.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/sync_manager_impl.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 184 matching lines @@
   // 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();
 
+  // Purges all disabled types.
+  // Warning: this is expensive as it will iterate over all entries in the
+  // directory. Only call if absolutely necessary.
+  bool CleanupDisabledTypes();
+
   // 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 607 matching lines @@
 void SyncManagerImpl::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) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!ready_task.is_null());
   DCHECK(!retry_task.is_null());
 
+  // Cleanup disabled types if necessary.
+  // Note: tests might not have a session context.
+  if (data_->session_context()) {
+    ModelTypeSet disabled_types =
+        Difference(

[tim (not reviewing), 2012/07/19 22:05:59]

nit: I'd move Difference( up one line

[Nicolas Zea, 2012/07/20 22:07:01]

Done.

+            GetRoutingInfoTypes(data_->session_context()->routing_info()),
+            GetRoutingInfoTypes(new_routing_info));
+    if (!disabled_types.Empty()) {
+      DVLOG(1) << "Purging disabled types: "
+               << ModelTypeSetToString(disabled_types);
+      data_->directory()->PurgeEntriesWithTypeIn(disabled_types);

[tim (not reviewing), 2012/07/19 22:05:59]

We should probably bail / callback / use a bigger nuke if this returns false?  

Also, can you merge this code with CleanupDisabledTypes (and call it here)?  I
think the only diff is using ModelTypeSet::All vs ctxt_routing_info.

[Nicolas Zea, 2012/07/20 22:07:01]

Done.

+    }
+  }
+
   // 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);
@@ skipping 119 matching lines @@
     // 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())
+    //
+    // Similarly, a type may have been disabled previously, but we didn't
+    // manage to purge. Ensure we cleanup any disabled types before starting up.
+    //
+    // Note: If either of these methods fail, we have directory corruption and
+    // cannot continue.
+    if (!PurgePartiallySyncedTypes() || !CleanupDisabledTypes())
       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));
 
   if (!success && testing_mode_ == NON_TEST)
     return false;
 
   sync_notifier_->AddObserver(this);
 
   return success;
 }
 
+bool SyncManagerImpl::SyncInternal::CleanupDisabledTypes() {
+  if (testing_mode_ != NON_TEST)
+    return true;
+
+  ModelTypeSet enabled_types = GetRoutingInfoTypes(
+      session_context()->routing_info());
+  ModelTypeSet disabled_types = Difference(ModelTypeSet::All(),
+                                           enabled_types);
+  if (disabled_types.Empty())
+    return true;
+
+  DVLOG(1) << "Purging disabled types "
+           << ModelTypeSetToString(disabled_types);
+  return directory()->PurgeEntriesWithTypeIn(disabled_types);
+}
+
+
 void SyncManagerImpl::SyncInternal::UpdateCryptographerAndNigori(
     const std::string& chrome_version,
     const base::Closure& done_callback) {
   DCHECK(initialized_);
   syncer::GetSessionName(
       blocking_task_runner_,
       base::Bind(
           &SyncManagerImpl::SyncInternal::UpdateCryptographerAndNigoriCallback,
           weak_ptr_factory_.GetWeakPtr(),
           chrome_version,
@@ skipping 1482 matching lines @@
 int SyncManagerImpl::GetDefaultNudgeDelay() {
   return kDefaultNudgeDelayMilliseconds;
 }
 
 // static.
 int SyncManagerImpl::GetPreferencesNudgeDelay() {
   return kPreferencesNudgeDelayMilliseconds;
 }
 
 }  // namespace syncer