[Sync] Fix ConfigureDataTypes bug that leads to extra nudge

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 <algorithm>
 
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 333 matching lines @@
     return;
   }
 }
 
 SyncBackendHost::PendingConfigureDataTypesState::
 PendingConfigureDataTypesState() : deleted_type(false) {}
 
 SyncBackendHost::PendingConfigureDataTypesState::
 ~PendingConfigureDataTypesState() {}
 
+// static
+SyncBackendHost::PendingConfigureDataTypesState*
+    SyncBackendHost::MakePendingConfigModeState(
+        const DataTypeController::TypeMap& data_type_controllers,
+        const syncable::ModelTypeSet& types,
+        CancelableTask* ready_task,
+        ModelSafeRoutingInfo* routing_info) {
+  PendingConfigureDataTypesState* state = new PendingConfigureDataTypesState();
+
+  for (DataTypeController::TypeMap::const_iterator it =
+           data_type_controllers.begin();
+       it != data_type_controllers.end(); ++it) {
+    syncable::ModelType type = it->first;
+
+    // If a type is not specified, remove it from the routing_info.
+    if (types.count(type) == 0) {
+      state->deleted_type = (routing_info->erase(type) > 0);

[tim (not reviewing), 2011/04/19 18:07:05]

this line is wrong.  we need to just set deleted_type to true here, because
DeactivateDataType will be called and is currently what yanks things from the
routing_info.

+    } else {
+      // Add a newly specified data type as GROUP_PASSIVE into the
+      // routing_info, if it does not already exist.
+      if (routing_info->count(type) == 0) {
+        (*routing_info)[type] = GROUP_PASSIVE;
+        state->added_types.set(type);
+      }
+    }
+  }
+
+  state->ready_task.reset(ready_task);
+  state->initial_types = types;
+  return state;
+}
+
 void SyncBackendHost::ConfigureDataTypes(
     const DataTypeController::TypeMap& data_type_controllers,
     const syncable::ModelTypeSet& types,
     CancelableTask* ready_task) {
   // Only one configure is allowed at a time.
   DCHECK(!pending_config_mode_state_.get());
   DCHECK(!pending_download_state_.get());
   DCHECK(syncapi_initialized_);
 
   if (types.count(syncable::AUTOFILL_PROFILE) != 0) {
     ConfigureAutofillMigration();
   }
 
-  scoped_ptr<PendingConfigureDataTypesState> state(new
-      PendingConfigureDataTypesState());
-
   {
     base::AutoLock lock(registrar_lock_);
-    for (DataTypeController::TypeMap::const_iterator it =
-             data_type_controllers.begin();
-         it != data_type_controllers.end(); ++it) {
-      syncable::ModelType type = (*it).first;
-
-      // If a type is not specified, remove it from the routing_info.
-      if (types.count(type) == 0) {
-        registrar_.routing_info.erase(type);
-        state->deleted_type = true;
-      } else {
-        // Add a newly specified data type as GROUP_PASSIVE into the
-        // routing_info, if it does not already exist.
-        if (registrar_.routing_info.count(type) == 0) {
-          registrar_.routing_info[type] = GROUP_PASSIVE;
-          state->added_types.set(type);
-        }
-      }
-    }
+    pending_config_mode_state_.reset(
+        MakePendingConfigModeState(data_type_controllers, types, ready_task,
+                                   &registrar_.routing_info));
   }
 
-  state->ready_task.reset(ready_task);
-  state->initial_types = types;
-  pending_config_mode_state_.reset(state.release());
-
   // If we're doing the first configure (at startup) this is redundant as the
   // syncer thread always must start in config mode.
   if (using_new_syncer_thread_) {
     core_->syncapi()->StartConfigurationMode(NewCallback(core_.get(),
         &SyncBackendHost::Core::FinishConfigureDataTypes));
   } else {
     FinishConfigureDataTypesOnFrontendLoop();
   }
 }
 
@@ skipping 781 matching lines @@
   DCHECK_EQ(MessageLoop::current(), host_->core_thread_.message_loop());
   syncapi_->GetJsBackend()->ProcessMessage(name, args, sender);
 }
 
 void SyncBackendHost::Core::DeferNudgeForCleanup() {
   DCHECK_EQ(MessageLoop::current(), host_->core_thread_.message_loop());
   deferred_nudge_for_cleanup_requested_ = true;
 }
 
 }  // namespace browser_sync