Improve autofill unit test

With chron's help, I was able to get rid of the whole TestModelAssocator thing and use the low level syncapi directly to create the server tagged autofill node before model association starts.  Because of this, I was able to revert to the original AutofillModelAssociator::AssociateModels() code that zork had and it now works fine in a test.

Review URL: http://codereview.chromium.org/661410

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40524 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 61 insertions, 61 deletions

diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index 966a34f..df0f540 100755
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -1580,7 +1580,8 @@ void SyncManager::SyncInternal::HandleCalculateChangesChangeEventFromSyncApi(
     const syncable::DirectoryChangeEvent& event) {
   // We have been notified about a user action changing the bookmark model.
   DCHECK_EQ(event.todo, syncable::DirectoryChangeEvent::CALCULATE_CHANGES);
-  DCHECK_EQ(event.writer, syncable::SYNCAPI);
+  DCHECK(event.writer == syncable::SYNCAPI ||
+         event.writer == syncable::UNITTEST);
   LOG_IF(WARNING, !ChangeBuffersAreEmpty()) <<
       "CALCULATE_CHANGES called with unapplied old changes.";
 
@@ -1614,7 +1615,8 @@ void SyncManager::SyncInternal::HandleCalculateChangesChangeEventFromSyncer(
   // We only expect one notification per sync step, so change_buffers_ should
   // contain no pending entries.
   DCHECK_EQ(event.todo, syncable::DirectoryChangeEvent::CALCULATE_CHANGES);
-  DCHECK_EQ(event.writer, syncable::SYNCER);
+  DCHECK(event.writer == syncable::SYNCER ||
+         event.writer == syncable::UNITTEST);
   LOG_IF(WARNING, !ChangeBuffersAreEmpty()) <<
       "CALCULATE_CHANGES called with unapplied old changes.";
 
diff --git a/chrome/browser/sync/glue/autofill_model_associator.cc b/chrome/browser/sync/glue/autofill_model_associator.cc
index a7c4f7b..c9a349b 100755
--- a/chrome/browser/sync/glue/autofill_model_associator.cc
+++ b/chrome/browser/sync/glue/autofill_model_associator.cc
@@ -41,18 +41,10 @@ bool AutofillModelAssociator::AssociateModels() {
     return false;
   }
 
-  int64 root_id;
-  if (!GetSyncIdForTaggedNode(kAutofillTag, &root_id)) {
-    sync_service_->OnUnrecoverableError();
-    LOG(ERROR) << "Server did not create the top-level autofill node. We "
-               << "might be running against an out-of-date server.";
-    return false;
-  }
-
   sync_api::WriteTransaction trans(
       sync_service_->backend()->GetUserShareHandle());
   sync_api::ReadNode autofill_root(&trans);
-  if (!autofill_root.InitByIdLookup(root_id)) {
+  if (!autofill_root.InitByTagLookup(kAutofillTag)) {
     error_handler_->OnUnrecoverableError();
     LOG(ERROR) << "Server did not create the top-level autofill node. We "
                << "might be running against an out-of-date server.";
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 34dcbfe..fc0ddba 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -258,7 +258,6 @@ void ProfileSyncService::OnUnrecoverableError() {
 
 void ProfileSyncService::OnBackendInitialized() {
   backend_initialized_ = true;
-  StartProcessingChangesIfReady();
 
   // The very first time the backend initializes is effectively the first time
   // we can say we successfully "synced".  last_synced_time_ will only be null
@@ -266,6 +265,8 @@ void ProfileSyncService::OnBackendInitialized() {
   if (last_synced_time_.is_null())
     UpdateLastSyncedTime();
   FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
+
+  StartProcessingChangesIfReady();
 }
 
 void ProfileSyncService::OnSyncCycleCompleted() {
diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
index 107a54b..fbe5bd9 100644
--- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
@@ -20,9 +20,11 @@
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_test_util.h"
 #include "chrome/browser/sync/protocol/autofill_specifics.pb.h"
+#include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/webdata/autofill_entry.h"
 #include "chrome/browser/webdata/web_database.h"
 #include "chrome/common/notification_service.h"
+#include "chrome/test/sync/engine/test_id_factory.h"
 #include "chrome/test/testing_profile.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -32,11 +34,30 @@ using browser_sync::AutofillChangeProcessor;
 using browser_sync::AutofillDataTypeController;
 using browser_sync::AutofillModelAssociator;
 using browser_sync::SyncBackendHost;
+using browser_sync::TestIdFactory;
 using browser_sync::UnrecoverableErrorHandler;
 using sync_api::SyncManager;
+using sync_api::UserShare;
+using syncable::BASE_VERSION;
+using syncable::CREATE;
+using syncable::DirectoryManager;
+using syncable::ID;
+using syncable::IS_DEL;
+using syncable::IS_DIR;
+using syncable::IS_UNAPPLIED_UPDATE;
+using syncable::IS_UNSYNCED;
+using syncable::MutableEntry;
+using syncable::SERVER_IS_DIR;
+using syncable::SERVER_VERSION;
+using syncable::SPECIFICS;
+using syncable::ScopedDirLookup;
+using syncable::UNIQUE_SERVER_TAG;
+using syncable::UNITTEST;
+using syncable::WriteTransaction;
 using testing::_;
 using testing::DoAll;
 using testing::DoDefault;
+using testing::Invoke;
 using testing::Return;
 using testing::SetArgumentPointee;
 
@@ -69,17 +90,6 @@ class TestingProfileSyncService : public ProfileSyncService {
   }
 };
 
-class TestAutofillModelAssociator
-    : public TestModelAssociator<AutofillModelAssociator> {
- public:
-  TestAutofillModelAssociator(ProfileSyncService* service,
-                              WebDatabase* web_database,
-                              UnrecoverableErrorHandler* error_handler)
-      : TestModelAssociator<AutofillModelAssociator>(
-          service, web_database, error_handler) {
-  }
-};
-
 class WebDatabaseMock : public WebDatabase {
  public:
   MOCK_METHOD2(RemoveFormElement,
@@ -160,7 +170,7 @@ ACTION(QuitUIMessageLoop) {
 ACTION_P3(MakeAutofillSyncComponents, service, wd, dtc) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
   AutofillModelAssociator* model_associator =
-      new TestAutofillModelAssociator(service, wd, dtc);
+      new AutofillModelAssociator(service, wd, dtc);
   AutofillChangeProcessor* change_processor =
       new AutofillChangeProcessor(model_associator, wd, service);
   return ProfileSyncFactory::SyncComponents(model_associator, change_processor);
@@ -211,7 +221,8 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
 
       // State changes once for the backend init and once for startup done.
       EXPECT_CALL(observer_, OnStateChanged()).
-          WillOnce(DoDefault()).
+          WillOnce(Invoke(this,
+                          &ProfileSyncServiceAutofillTest::CreateAutofillRoot)).
           WillOnce(QuitUIMessageLoop());
       service_->RegisterDataTypeController(data_type_controller);
       service_->Initialize();
@@ -219,15 +230,37 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
     }
   }
 
-  void GetAutofillEntriesFromSyncDB(std::vector<AutofillEntry>* entries) {
-    sync_api::ReadTransaction trans(service_->backend()->GetUserShareHandle());
-    int64 autofill_root_id =
-        GetFakeServerTaggedNode(&trans, "google_chrome_autofill");
-    if (autofill_root_id == sync_api::kInvalidId)
+  void CreateAutofillRoot() {
+    UserShare* user_share = service_->backend()->GetUserShareHandle();
+    DirectoryManager* dir_manager = user_share->dir_manager.get();
+
+    ScopedDirLookup dir(dir_manager, user_share->authenticated_name);
+    if (!dir.good())
       return;
 
+    WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
+    MutableEntry node(&wtrans,
+                      CREATE,
+                      wtrans.root_id(),
+                      browser_sync::kAutofillTag);
+    node.Put(UNIQUE_SERVER_TAG, browser_sync::kAutofillTag);
+    node.Put(IS_DIR, true);
+    node.Put(SERVER_IS_DIR, false);
+    node.Put(IS_UNSYNCED, false);
+    node.Put(IS_UNAPPLIED_UPDATE, false);
+    node.Put(SERVER_VERSION, 20);
+    node.Put(BASE_VERSION, 20);
+    node.Put(IS_DEL, false);
+    node.Put(ID, ids_.MakeServer(browser_sync::kAutofillTag));
+    sync_pb::EntitySpecifics specifics;
+    specifics.MutableExtension(sync_pb::autofill);
+    node.Put(SPECIFICS, specifics);
+  }
+
+  void GetAutofillEntriesFromSyncDB(std::vector<AutofillEntry>* entries) {
+    sync_api::ReadTransaction trans(service_->backend()->GetUserShareHandle());
     sync_api::ReadNode autofill_root(&trans);
-    if (!autofill_root.InitByIdLookup(autofill_root_id))
+    if (!autofill_root.InitByTagLookup(browser_sync::kAutofillTag))
       return;
 
     int64 child_id = autofill_root.GetFirstChildId();
@@ -251,29 +284,6 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
     }
   }
 
-  int64 GetFakeServerTaggedNode(sync_api::ReadTransaction* trans,
-                                const std::string& tag) {
-    std::wstring tag_wide;
-    if (!UTF8ToWide(tag.c_str(), tag.length(), &tag_wide))
-      return sync_api::kInvalidId;
-
-    sync_api::ReadNode root(trans);
-    root.InitByRootLookup();
-
-    int64 last_child_id = sync_api::kInvalidId;
-    for (int64 id = root.GetFirstChildId(); id != sync_api::kInvalidId; /***/) {
-      sync_api::ReadNode child(trans);
-      child.InitByIdLookup(id);
-      last_child_id = id;
-      if (tag_wide == child.GetTitle()) {
-        return id;
-      }
-      id = child.GetSuccessorId();
-    }
-
-    return sync_api::kInvalidId;
-  }
-
   void SetIdleChangeProcessorExpectations() {
     EXPECT_CALL(web_database_, RemoveFormElement(_, _)).Times(0);
     EXPECT_CALL(web_database_, GetAutofillTimestamps(_, _, _)).Times(0);
@@ -289,7 +299,6 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
         AutofillKey(ASCIIToUTF16(name), ASCIIToUTF16(value)), timestamps);
   }
 
-
   MessageLoopForUI message_loop_;
   ChromeThread ui_thread_;
   ChromeThread db_thread_;
@@ -302,6 +311,8 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
   ProfileSyncServiceObserverMock observer_;
   WebDatabaseMock web_database_;
   scoped_refptr<WebDataService> web_data_service_;
+
+  TestIdFactory ids_;
 };
 
 // TODO(sync): Test unrecoverable error during MA.
diff --git a/chrome/browser/sync/profile_sync_test_util.h b/chrome/browser/sync/profile_sync_test_util.h
index e78ab09..7c73ed1 100644
--- a/chrome/browser/sync/profile_sync_test_util.h
+++ b/chrome/browser/sync/profile_sync_test_util.h
@@ -32,12 +32,6 @@ class TestModelAssociator : public ModelAssociatorImpl {
       : ModelAssociatorImpl(service, error_handler) {
   }
 
-  TestModelAssociator(ProfileSyncService* service,
-                      WebDatabase* web_database,
-                      browser_sync::UnrecoverableErrorHandler* error_handler)
-      : ModelAssociatorImpl(service, web_database, error_handler) {
-  }
-
   virtual bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id) {
     std::wstring tag_wide;
     if (!UTF8ToWide(tag.c_str(), tag.length(), &tag_wide)) {