sync: fix reentrancy crash in ModelAssociationManager

chrome/browser/sync/glue/model_association_manager_unittest.cc

Index: chrome/browser/sync/glue/model_association_manager_unittest.cc
diff --git a/chrome/browser/sync/glue/model_association_manager_unittest.cc b/chrome/browser/sync/glue/model_association_manager_unittest.cc
index 647184af8b0c106d57d52ac34c53d433746ce8fd..6c6dfed3c543c3a95c19189d5214942a093475b3 100644
--- a/chrome/browser/sync/glue/model_association_manager_unittest.cc
+++ b/chrome/browser/sync/glue/model_association_manager_unittest.cc
@@ -211,6 +211,81 @@ TEST_F(ModelAssociationManagerTest, TypeReturnUnrecoverableError) {
       DataTypeController::UNRECOVERABLE_ERROR);
 }
 
+TEST_F(ModelAssociationManagerTest, InitializeAbortsLoad) {
+  controllers_[syncable::BOOKMARKS] =
+      new FakeDataTypeController(syncable::BOOKMARKS);
+  controllers_[syncable::THEMES] =
+      new FakeDataTypeController(syncable::THEMES);
+
+  GetController(controllers_, syncable::BOOKMARKS)->SetDelayModelLoad();

[Nicolas Zea, 2012/06/27 17:52:09]

it's kindda weird to me that  we use GetController for something we just created
and require exists. But I guess it's consistent with the other tests.

+  ModelAssociationManager model_association_manager(&controllers_,
+                                                    &result_processor_);
+  syncable::ModelTypeSet types;
+  types.Put(syncable::BOOKMARKS);
+  types.Put(syncable::THEMES);
+
+  syncable::ModelTypeSet expected_types_waiting_to_load;

[Nicolas Zea, 2012/06/27 17:52:09]

expected_types_waiting_to_load(syncable::BOOKMARKS);

can do that above too for |types|

+  expected_types_waiting_to_load.Put(syncable::BOOKMARKS);
+  DataTypeManager::ConfigureResult expected_result_partially_done(
+      DataTypeManager::PARTIAL_SUCCESS,
+      types,
+      std::list<csync::SyncError>(),
+      expected_types_waiting_to_load);
+
+  model_association_manager.Initialize(types);
+  model_association_manager.StopDisabledTypes();
+
+  model_association_manager.StartAssociationAsync();
+
+  EXPECT_CALL(result_processor_, OnModelAssociationDone(_)).
+              WillOnce(VerifyResult(expected_result_partially_done));
+
+  base::OneShotTimer<ModelAssociationManager>* timer =
+      model_association_manager.GetTimerForTesting();
+
+  // Note: Independent of the timeout value this test is not flaky.
+  // The reason is timer posts a task which would never be executed
+  // as we dont let the message loop run.
+  base::Closure task = timer->user_task();
+  timer->Stop();
+  task.Run();  // Bookmark load times out here.
+
+  // Apps finishes associating here.
+  GetController(controllers_, syncable::THEMES)->FinishStart(
+      DataTypeController::OK);
+
+  // At this point, BOOKMARKS is still waiting to load (as evidenced by
+  // expected_result_partially_done). If we schedule another Initialize (which
+  // could happen in practice due to reconfiguration), this should abort
+  // BOOKMARKS. Aborting will call ModelLoadCallback, but the
+  // ModelAssociationManager should be smart enough to know that this is not due
+  // to the type having completed loading.
+  EXPECT_CALL(result_processor_, OnTypesLoaded()).Times(0);
+
+  EXPECT_EQ(GetController(controllers_, syncable::BOOKMARKS)->state(),
+            DataTypeController::MODEL_STARTING);
+
+  model_association_manager.Initialize(types);
+  EXPECT_EQ(GetController(controllers_, syncable::BOOKMARKS)->state(),
+            DataTypeController::NOT_RUNNING);
+
+  DataTypeManager::ConfigureResult expected_result_done(
+      DataTypeManager::OK,
+      types,
+      std::list<csync::SyncError>(),
+      syncable::ModelTypeSet());
+  EXPECT_CALL(result_processor_, OnModelAssociationDone(_)).
+              WillOnce(VerifyResult(expected_result_done));
+
+  model_association_manager.StopDisabledTypes();
+  model_association_manager.StartAssociationAsync();
+
+  GetController(controllers_,
+                syncable::BOOKMARKS)->SimulateModelLoadFinishing();
+  GetController(controllers_, syncable::BOOKMARKS)->FinishStart(
+      DataTypeController::OK);
+}
+
 // Start 2 types. One of which timeout loading. Ensure that type is
 // fully configured eventually.
 TEST_F(ModelAssociationManagerTest, ModelStartWithSlowLoadingType) {
@@ -281,4 +356,3 @@ TEST_F(ModelAssociationManagerTest, ModelStartWithSlowLoadingType) {
 
 
 }  // namespace browser_sync
-