make new syncer thread the default.

chrome/browser/sync/profile_sync_service_password_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 <vector>
 
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/synchronization/waitable_event.h"
 #include "base/task.h"
@@ skipping 135 matching lines @@
     profile_.CreateRequestContext();
     password_store_ = new MockPasswordStore();
     db_thread_.Start();
 
     notification_service_ = new ThreadNotificationService(&db_thread_);
     notification_service_->Init();
     registrar_.Add(&observer_,
         NotificationType::SYNC_CONFIGURE_DONE,
         NotificationService::AllSources());
     registrar_.Add(&observer_,
-        NotificationType::SYNC_PAUSED,
+        NotificationType::SYNC_CONFIGURE_UPDATES_DOWNLOADED,
         NotificationService::AllSources());
   }
 
   virtual void TearDown() {
     service_.reset();
     notification_service_->TearDown();
     db_thread_.Stop();
     {
       // The request context gets deleted on the I/O thread. To prevent a leak
       // supply one here.
@@ skipping 44 matching lines @@
 
       EXPECT_CALL(profile_, GetPasswordStore(_)).
           Times(AtLeast(2)).  // Can be more if we hit NEEDS_CRYPTO.
           WillRepeatedly(Return(password_store_.get()));
 
       EXPECT_CALL(observer_,
           Observe(
               NotificationType(NotificationType::SYNC_CONFIGURE_DONE),_,_));
       EXPECT_CALL(observer_,
           Observe(
-              NotificationType(NotificationType::SYNC_PAUSED),_,_))
+              NotificationType(
+              NotificationType::SYNC_CONFIGURE_UPDATES_DOWNLOADED),_,_))

[tim (not reviewing), 2011/04/18 16:11:47]

This was the kind of thing I expected to arise when "making the unit tests work"
and why I thought it'd be a little trickier than just pulling pause/resume out
:)

I'd prefer to not add a notification just for the test. But if we have to, note
that it's only a matter of time before code is built on top of it because "it
sounds like the right event", so I wouldn't pick "updates downloaded".  It's too
ambiguous and overloaded in this context.  

Additionally, it also seems racy because couldn't the cryptographer transition
to 'ready' due to the set passphrase call below?

Did you consider SYNC_CONFIGURE_BLOCKED instead?  I think it mitigates the two
issues above and also is nice because it teases out the fact that we only need
this for password tests, so hopefully it wouldn't be mistakenly included in
other tests that should be using SYNC_CONFIGURE_DONE in the future.

[lipalani1, 2011/04/18 20:36:41]

ha ha! yes and it took a while to figure out as well.

Any way the reason I put that in was to allow things to be built around
updates_downloaded if needed(similar to pause earlier). But I see your point as
well. If need arises others can add it later. I have made it very specific. Now
it is called sync_configure_needs_crypto. Look at the implementation.
On 2011/04/18 16:11:47, timsteele wrote:

+          .Times(2)
           .WillOnce(InvokeWithoutArgs(QuitMessageLoop));
 
       service_->RegisterDataTypeController(data_type_controller);
       service_->Initialize();
       MessageLoop::current()->Run();
       FlushLastDBTask();
 
       service_->SetPassphrase("foo", false, true);
       MessageLoop::current()->Run();
     }
@@ skipping 308 matching lines @@
   AddPasswordEntriesTask node_task(this, sync_forms);
 
   StartSyncService(&root_task, &node_task);
 
   std::vector<PasswordForm> new_sync_forms;
   GetPasswordEntriesFromSyncDB(&new_sync_forms);
 
   EXPECT_EQ(1U, new_sync_forms.size());
   EXPECT_TRUE(ComparePasswords(expected_forms[0], new_sync_forms[0]));
 }