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

sync/internal_api/sync_manager_impl.cc

Index: sync/internal_api/sync_manager_impl.cc
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 107ed606e743b2bdbd91d550541e84f238a5b35f..1a26fe048f9e196b8dc6e762637b0a06b43bb2a3 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -202,6 +202,11 @@ class SyncManagerImpl::SyncInternal
   // 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);
 
@@ -829,6 +834,20 @@ void SyncManagerImpl::ConfigureSyncer(
   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 =
@@ -968,7 +987,13 @@ bool SyncManagerImpl::SyncInternal::Init(
     // 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
@@ -998,6 +1023,23 @@ bool SyncManagerImpl::SyncInternal::Init(
   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) {