Fixes to (formerly) crashing test ProfileSyncServiceAutofillTest.ServerChangeRace

chrome/browser/sync/profile_sync_service_autofill_unittest.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 <set>
 #include <string>
 #include <vector>
 
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_common_test.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/history/history.h"

[Nicolas Zea, 2011/12/07 00:46:43]

can you just forward declare the history service instead of including it?

[GeorgeY, 2011/12/07 00:55:08]

Is it enough for static_cast<>? Trying...

Yes, compiles - changed.

 #include "chrome/browser/sync/abstract_profile_sync_service_test.h"
 #include "chrome/browser/sync/engine/model_changing_syncer_command.h"
 #include "chrome/browser/sync/glue/autofill_data_type_controller.h"
 #include "chrome/browser/sync/glue/autofill_profile_data_type_controller.h"
 #include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/browser/sync/glue/generic_change_processor.h"
 #include "chrome/browser/sync/glue/shared_change_processor.h"
 #include "chrome/browser/sync/glue/syncable_service_adapter.h"
 #include "chrome/browser/sync/internal_api/read_node.h"
 #include "chrome/browser/sync/internal_api/read_transaction.h"
@@ skipping 89 matching lines @@
       : autofill_table_(autofill_table) {}
 
   virtual AutofillTable* GetAutofillTable() OVERRIDE {
     return autofill_table_;
   }
 
  private:
   AutofillTable* autofill_table_;
 };
 
-
 class ProfileSyncServiceAutofillTest;
 
 template<class AutofillProfile>
 syncable::ModelType GetModelType() {
   return syncable::UNSPECIFIED;
 }
 
 template<>
 syncable::ModelType GetModelType<AutofillEntry>() {
   return syncable::AUTOFILL;
@@ skipping 939 matching lines @@
   notifier->Notify(chrome::NOTIFICATION_AUTOFILL_PROFILE_CHANGED,
                    content::Source<WebDataService>(web_data_service_.get()),
                    content::Details<AutofillProfileChange>(&change));
 
   std::vector<AutofillProfile> new_sync_profiles;
   ASSERT_TRUE(GetAutofillProfilesFromSyncDBUnderProfileNode(
       &new_sync_profiles));
   ASSERT_EQ(0U, new_sync_profiles.size());
 }
 
-// Crashy, http://crbug.com/57884
-TEST_F(ProfileSyncServiceAutofillTest, DISABLED_ServerChangeRace) {
-  EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).WillOnce(Return(true));
-  EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+TEST_F(ProfileSyncServiceAutofillTest, ServerChangeRace) {
+  // GetHistoryService() gets called indirectly, but the result is ignored, so
+  // it is safe to return NULL.
+  EXPECT_CALL(profile_, GetHistoryService(_)).
+      WillRepeatedly(Return(static_cast<HistoryService*>(NULL)));
+  // Once for MergeDataAndStartSyncing() and twice for ProcessSyncChanges(), via
+  // LoadAutofillData().
+  EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+      Times(3).WillRepeatedly(Return(true));
+  // On the other hand Autofill and Autocomplete are separated now, so
+  // GetAutofillProfiles() should not be called.
+  EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).Times(0);
   EXPECT_CALL(autofill_table_, UpdateAutofillEntries(_)).
       WillRepeatedly(Return(true));
   EXPECT_CALL(*personal_data_manager_, Refresh()).Times(3);
   CreateRootHelper create_root(this, syncable::AUTOFILL);
   StartSyncService(create_root.callback(), false, syncable::AUTOFILL);
   ASSERT_TRUE(create_root.success());
 
   // (true, false) means we have to reset after |Signal|, init to unsignaled.
   scoped_ptr<WaitableEvent> wait_for_start(new WaitableEvent(true, false));
   scoped_ptr<WaitableEvent> wait_for_syncapi(new WaitableEvent(true, false));
@@ skipping 19 matching lines @@
   std::vector<AutofillEntry> sync_entries;
   std::vector<AutofillProfile> sync_profiles;
   ASSERT_TRUE(GetAutofillEntriesFromSyncDB(&sync_entries, &sync_profiles));
   EXPECT_EQ(3U, sync_entries.size());
   EXPECT_EQ(0U, sync_profiles.size());
   for (size_t i = 0; i < sync_entries.size(); i++) {
     DVLOG(1) << "Entry " << i << ": " << sync_entries[i].key().name()
              << ", " << sync_entries[i].key().value();
   }
 }