[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 326 matching lines @@
 void SyncManagerImpl::ConfigureSyncer(
     ConfigureReason reason,
     const ModelTypeSet& types_to_config,
     const 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 any types that might have just been disabled.
+  if (!CleanupDisabledTypes(
+          GetRoutingInfoTypes(session_context_->routing_info()),
+          GetRoutingInfoTypes(new_routing_info))) {
+    // We failed to cleanup the types. Invoke the ready task without actually
+    // configuring any types. The caller should detect this as a configuration
+    // failure and act appropriately.
+    ready_task.Run();
+    return;
+  }
+
   // 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 98 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(

[tim (not reviewing), 2012/07/26 21:39:45]

nit - lets use the same verb (Purge/Cleanup) here, my vote goes to purge due to
the underlying API.

[Nicolas Zea, 2012/07/26 23:29:59]

Done.

+            ModelTypeSet::All(),
+            GetRoutingInfoTypes(session_context_->routing_info())))
       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);
   }
@@ skipping 227 matching lines @@
   partially_synced_types.RemoveAll(GetTypesWithEmptyProgressMarkerToken(
       ModelTypeSet::All()));
 
   UMA_HISTOGRAM_COUNTS("Sync.PartiallySyncedTypes",
                        partially_synced_types.Size());
   if (partially_synced_types.Empty())
     return true;
   return directory()->PurgeEntriesWithTypeIn(partially_synced_types);
 }
 
+bool SyncManagerImpl::CleanupDisabledTypes(
+    ModelTypeSet previously_enabled_types,
+    ModelTypeSet currently_enabled_types) {
+  ModelTypeSet disabled_types = Difference(previously_enabled_types,
+                                           currently_enabled_types);
+  if (disabled_types.Empty())
+    return true;
+
+  DVLOG(1) << "Purging disabled types "
+           << ModelTypeSetToString(disabled_types);
+  return directory()->PurgeEntriesWithTypeIn(disabled_types);
+}
+
 void SyncManagerImpl::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 1168 matching lines @@
 int SyncManagerImpl::GetDefaultNudgeDelay() {
   return kDefaultNudgeDelayMilliseconds;
 }
 
 // static.
 int SyncManagerImpl::GetPreferencesNudgeDelay() {
   return kPreferencesNudgeDelayMilliseconds;
 }
 
 }  // namespace syncer