Clean up TestProfileSyncService and related tests

chrome/browser/sync/profile_sync_service_unittest.cc

 // Copyright (c) 2012 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 "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
 #include "base/values.h"
 #include "chrome/browser/invalidation/invalidation_service_factory.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/browser/sync/fake_oauth2_token_service.h"
-#include "chrome/browser/sync/glue/bookmark_data_type_controller.h"
-#include "chrome/browser/sync/glue/data_type_controller.h"
+#include "chrome/browser/sync/glue/data_type_manager_impl.h"
 #include "chrome/browser/sync/glue/sync_backend_host_mock.h"
 #include "chrome/browser/sync/profile_sync_components_factory_mock.h"
-#include "chrome/browser/sync/test_profile_sync_service.h"
-#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread_bundle.h"
-#include "content/public/test/test_utils.h"
-#include "google/cacheinvalidation/include/types.h"
 #include "google_apis/gaia/gaia_constants.h"
-#include "sync/js/js_arg_list.h"
-#include "sync/js/js_event_details.h"
-#include "sync/js/js_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// TODO(akalin): Add tests here that exercise the whole
-// ProfileSyncService/SyncBackendHost stack while mocking out as
-// little as possible.
-
 namespace browser_sync {
 
 namespace {
 
-using testing::_;
-using testing::AtLeast;
-using testing::AtMost;
-using testing::Mock;
-using testing::Return;
-using testing::StrictMock;
-
-void SignalDone(base::WaitableEvent* done) {
-  done->Signal();
+ACTION(ReturnNewDataTypeManager) {

[pavely, 2013/12/02 19:00:55]

I don't think you are using ReturnNewDataTypeManager anywhere. Could you check?

[rlarocque, 2013/12/02 21:08:59]

It's used in ExpectDataTypeManagerCreation() on line 144.

+  return new browser_sync::DataTypeManagerImpl(arg0,
+                                               arg1,
+                                               arg2,
+                                               arg3,
+                                               arg4,
+                                               arg5);
 }
 
-class ProfileSyncServiceTest : public testing::Test {
- protected:
-  ProfileSyncServiceTest()
-      : thread_bundle_(content::TestBrowserThreadBundle::REAL_DB_THREAD |
-                       content::TestBrowserThreadBundle::REAL_FILE_THREAD |
-                       content::TestBrowserThreadBundle::REAL_IO_THREAD) {
-   }
-
-  virtual ~ProfileSyncServiceTest() {}
-
-  virtual void SetUp() OVERRIDE {
-    TestingProfile::Builder builder;
-    builder.AddTestingFactory(ProfileOAuth2TokenServiceFactory::GetInstance(),
-                              FakeOAuth2TokenService::BuildTokenService);
-    profile_ = builder.Build().Pass();
-    invalidation::InvalidationServiceFactory::GetInstance()->
-        SetBuildOnlyFakeInvalidatorsForTest(true);
-  }
-
-  virtual void TearDown() OVERRIDE {
-    // Kill the service before the profile.
-    if (service_)
-      service_->Shutdown();
-
-    service_.reset();
-    profile_.reset();
-
-    // Pump messages posted by the sync thread (which may end up
-    // posting on the IO thread).
-    base::RunLoop().RunUntilIdle();
-    content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
-    base::RunLoop().RunUntilIdle();
-  }
-
-  void StartSyncServiceAndSetInitialSyncEnded() {
-    if (service_)
-      return;
-
-    SigninManagerBase* signin =
-        SigninManagerFactory::GetForProfile(profile_.get());
-    signin->SetAuthenticatedUsername("test");
-    ProfileOAuth2TokenService* oauth2_token_service =
-        ProfileOAuth2TokenServiceFactory::GetForProfile(profile_.get());
-    ProfileSyncComponentsFactoryMock* factory =
-        new ProfileSyncComponentsFactoryMock();
-    service_.reset(new TestProfileSyncService(
-        factory,
-        profile_.get(),
-        signin,
-        oauth2_token_service,
-        ProfileSyncService::AUTO_START,
-        true));
-
-
-    // Register the bookmark data type.
-    ON_CALL(*factory, CreateDataTypeManager(_, _, _, _, _, _)).
-        WillByDefault(ReturnNewDataTypeManager());
-
-    service_->Initialize();
-  }
-
-  void WaitForBackendInitDone() {
-    for (int i = 0; i < 5; ++i) {
-      base::WaitableEvent done(false, false);
-      service_->GetBackendForTest()->GetSyncLoopForTesting()
-          ->PostTask(FROM_HERE, base::Bind(&SignalDone, &done));
-      done.Wait();
-      base::RunLoop().RunUntilIdle();
-      if (service_->sync_initialized()) {
-        return;
-      }
-    }
-    LOG(ERROR) << "Backend not initialized.";
-  }
-
-  void IssueTestTokens() {
-    ProfileOAuth2TokenServiceFactory::GetForProfile(profile_.get())
-        ->UpdateCredentials("test", "oauth2_login_token");
-  }
-
-  scoped_ptr<TestProfileSyncService> service_;
-  scoped_ptr<TestingProfile> profile_;
-
- private:
-  content::TestBrowserThreadBundle thread_bundle_;
-};
+using testing::_;
+using testing::StrictMock;
 
 class TestProfileSyncServiceObserver : public ProfileSyncServiceObserver {
  public:
   explicit TestProfileSyncServiceObserver(ProfileSyncService* service)
       : service_(service), first_setup_in_progress_(false) {}
   virtual void OnStateChanged() OVERRIDE {
     first_setup_in_progress_ = service_->FirstSetupInProgress();
   }
   bool first_setup_in_progress() const { return first_setup_in_progress_; }
  private:
@@ skipping 23 matching lines @@
 }
 
 ACTION(ReturnNewSyncBackendHostNoReturn) {
   return new browser_sync::SyncBackendHostNoReturn();
 }
 
 // A test harness that uses a real ProfileSyncService and in most cases a
 // MockSyncBackendHost.
 //
 // This is useful if we want to test the ProfileSyncService and don't care about
-// testing the SyncBackendHost.  It's easier to use than the other tests, since
-// it doesn't involve any threads.
-class ProfileSyncServiceSimpleTest : public ::testing::Test {
+// testing the SyncBackendHost.
+class ProfileSyncServiceTest : public ::testing::Test {

[rlarocque, 2013/11/19 20:41:09]

The diffs here may be a bit confusing.

In a previous commit, I added the ProfileSyncServiceSimpleTest harness to
supplement the ProfileSyncServiceTest.  This CL converts all tests to use
ProfileSyncServiceSimpleTest.  Then, to make things extra confusing, it renames
ProfileSyncServiceSimpleTest to ProfileSyncServiceTest.

  protected:
-  ProfileSyncServiceSimpleTest()
+  ProfileSyncServiceTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}
-  virtual ~ProfileSyncServiceSimpleTest() {}
+  virtual ~ProfileSyncServiceTest() {}
 
   virtual void SetUp() OVERRIDE {
     TestingProfile::Builder builder;
 
     builder.AddTestingFactory(ProfileOAuth2TokenServiceFactory::GetInstance(),
                               FakeOAuth2TokenService::BuildTokenService);
     invalidation::InvalidationServiceFactory::GetInstance()->
         SetBuildOnlyFakeInvalidatorsForTest(true);
 
     profile_ = builder.Build().Pass();
@@ skipping 69 matching lines @@
   scoped_ptr<TestingProfile> profile_;
   scoped_ptr<ProfileSyncService> service_;
 
   // Pointer to the components factory.  Not owned.  May be null.
   ProfileSyncComponentsFactoryMock* components_factory_;
 
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
 // Verify that the server URLs are sane.
-TEST_F(ProfileSyncServiceSimpleTest, InitialState) {
+TEST_F(ProfileSyncServiceTest, InitialState) {
   CreateService(ProfileSyncService::AUTO_START);
   Initialize();
   const std::string& url = service()->sync_service_url().spec();
   EXPECT_TRUE(url == ProfileSyncService::kSyncServerUrl ||
               url == ProfileSyncService::kDevServerUrl);
 }
 
 // Verify a successful initialization.
-TEST_F(ProfileSyncServiceSimpleTest, SuccessfulInitialization) {
+TEST_F(ProfileSyncServiceTest, SuccessfulInitialization) {
   profile()->GetTestingPrefService()->SetManagedPref(
       prefs::kSyncManaged,
       Value::CreateBooleanValue(false));
   IssueTestTokens();
   CreateService(ProfileSyncService::AUTO_START);
   ExpectDataTypeManagerCreation();
   ExpectSyncBackendHostCreation();
   Initialize();
   EXPECT_FALSE(service()->IsManaged());
   EXPECT_TRUE(service()->sync_initialized());
 }
 
 
 // Verify that the SetSetupInProgress function call updates state
 // and notifies observers.
-TEST_F(ProfileSyncServiceSimpleTest, SetupInProgress) {
+TEST_F(ProfileSyncServiceTest, SetupInProgress) {
   CreateService(ProfileSyncService::MANUAL_START);
   Initialize();
 
   TestProfileSyncServiceObserver observer(service());
   service()->AddObserver(&observer);
 
   service()->SetSetupInProgress(true);
   EXPECT_TRUE(observer.first_setup_in_progress());
   service()->SetSetupInProgress(false);
   EXPECT_FALSE(observer.first_setup_in_progress());
 
   service()->RemoveObserver(&observer);
 }
 
 // Verify that disable by enterprise policy works.
-TEST_F(ProfileSyncServiceSimpleTest, DisabledByPolicyBeforeInit) {
+TEST_F(ProfileSyncServiceTest, DisabledByPolicyBeforeInit) {
   profile()->GetTestingPrefService()->SetManagedPref(
       prefs::kSyncManaged,
       Value::CreateBooleanValue(true));
   IssueTestTokens();
   CreateService(ProfileSyncService::AUTO_START);
   Initialize();
   EXPECT_TRUE(service()->IsManaged());
   EXPECT_FALSE(service()->sync_initialized());
 }
 
 // Verify that disable by enterprise policy works even after the backend has
 // been initialized.
-TEST_F(ProfileSyncServiceSimpleTest, DisabledByPolicyAfterInit) {
+TEST_F(ProfileSyncServiceTest, DisabledByPolicyAfterInit) {
   IssueTestTokens();
   CreateService(ProfileSyncService::AUTO_START);
   ExpectDataTypeManagerCreation();
   ExpectSyncBackendHostCreation();
   Initialize();
 
   EXPECT_FALSE(service()->IsManaged());
   EXPECT_TRUE(service()->sync_initialized());
 
   profile()->GetTestingPrefService()->SetManagedPref(
       prefs::kSyncManaged,
       Value::CreateBooleanValue(true));
 
   EXPECT_TRUE(service()->IsManaged());
   EXPECT_FALSE(service()->sync_initialized());
 }
 
 // Exercies the ProfileSyncService's code paths related to getting shut down
 // before the backend initialize call returns.
-TEST_F(ProfileSyncServiceSimpleTest, AbortedByShutdown) {
+TEST_F(ProfileSyncServiceTest, AbortedByShutdown) {
   CreateService(ProfileSyncService::AUTO_START);
   PrepareDelayedInitSyncBackendHost();
 
   IssueTestTokens();
   Initialize();
   EXPECT_FALSE(service()->sync_initialized());
 
   ShutdownAndDeleteService();
 }
 
 // Test StopAndSuppress() before we've initialized the backend.
-TEST_F(ProfileSyncServiceSimpleTest, EarlyStopAndSuppress) {
+TEST_F(ProfileSyncServiceTest, EarlyStopAndSuppress) {
   CreateService(ProfileSyncService::AUTO_START);
   IssueTestTokens();
 
   service()->StopAndSuppress();
   EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart));
 
   // Because of supression, this should fail.
   Initialize();
   EXPECT_FALSE(service()->sync_initialized());
 
   // Remove suppression.  This should be enough to allow init to happen.
   ExpectDataTypeManagerCreation();
   ExpectSyncBackendHostCreation();
   service()->UnsuppressAndStart();
   EXPECT_TRUE(service()->sync_initialized());
   EXPECT_FALSE(
       profile()->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart));
 }
 
 // Test StopAndSuppress() after we've initialized the backend.
-TEST_F(ProfileSyncServiceSimpleTest, DisableAndEnableSyncTemporarily) {
+TEST_F(ProfileSyncServiceTest, DisableAndEnableSyncTemporarily) {
   CreateService(ProfileSyncService::AUTO_START);
   IssueTestTokens();
   ExpectDataTypeManagerCreation();
   ExpectSyncBackendHostCreation();
   Initialize();
 
   EXPECT_TRUE(service()->sync_initialized());
   EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart));
 
   testing::Mock::VerifyAndClearExpectations(components_factory());
 
   service()->StopAndSuppress();
   EXPECT_FALSE(service()->sync_initialized());
   EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart));
 
   ExpectDataTypeManagerCreation();
   ExpectSyncBackendHostCreation();
 
   service()->UnsuppressAndStart();
   EXPECT_TRUE(service()->sync_initialized());
   EXPECT_FALSE(
       profile()->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart));
 }
 
 // Certain ProfileSyncService tests don't apply to Chrome OS, for example
 // things that deal with concepts like "signing out" and policy.
 #if !defined (OS_CHROMEOS)
 
-TEST_F(ProfileSyncServiceSimpleTest, EnableSyncAndSignOut) {
+TEST_F(ProfileSyncServiceTest, EnableSyncAndSignOut) {
   CreateService(ProfileSyncService::AUTO_START);
   ExpectDataTypeManagerCreation();
   ExpectSyncBackendHostCreation();
   IssueTestTokens();
   Initialize();
 
   EXPECT_TRUE(service()->sync_initialized());
   EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(prefs::kSyncSuppressStart));
 
   SigninManagerFactory::GetForProfile(profile())->SignOut();
   EXPECT_FALSE(service()->sync_initialized());
 }
 
 #endif  // !defined(OS_CHROMEOS)
 
-TEST_F(ProfileSyncServiceTest, JsControllerHandlersBasic) {
-  StartSyncServiceAndSetInitialSyncEnded();
+TEST_F(ProfileSyncServiceTest, GetSyncTokenStatus) {
+  CreateService(ProfileSyncService::AUTO_START);
   IssueTestTokens();
-  EXPECT_TRUE(service_->sync_initialized());
-  EXPECT_TRUE(service_->GetBackendForTest() != NULL);
+  ExpectDataTypeManagerCreation();
+  ExpectSyncBackendHostCreation();
+  Initialize();
 
-  base::WeakPtr<syncer::JsController> js_controller =
-      service_->GetJsController();
-  StrictMock<syncer::MockJsEventHandler> event_handler;
-  js_controller->AddJsEventHandler(&event_handler);
-  js_controller->RemoveJsEventHandler(&event_handler);
-}
-
-TEST_F(ProfileSyncServiceTest,
-       JsControllerHandlersDelayedBackendInitialization) {
-  StartSyncServiceAndSetInitialSyncEnded();
-
-  StrictMock<syncer::MockJsEventHandler> event_handler;
-  EXPECT_CALL(event_handler, HandleJsEvent(_, _)).Times(AtLeast(1));
-
-  EXPECT_EQ(NULL, service_->GetBackendForTest());
-  EXPECT_FALSE(service_->sync_initialized());
-
-  base::WeakPtr<syncer::JsController> js_controller =
-      service_->GetJsController();
-  js_controller->AddJsEventHandler(&event_handler);
-  // Since we're doing synchronous initialization, backend should be
-  // initialized by this call.
-  IssueTestTokens();
-  EXPECT_TRUE(service_->sync_initialized());
-  js_controller->RemoveJsEventHandler(&event_handler);
-}
-
-TEST_F(ProfileSyncServiceTest, JsControllerProcessJsMessageBasic) {
-  StartSyncServiceAndSetInitialSyncEnded();
-  IssueTestTokens();
-  WaitForBackendInitDone();
-
-  StrictMock<syncer::MockJsReplyHandler> reply_handler;
-
-  ListValue arg_list1;
-  arg_list1.Append(Value::CreateStringValue("INVALIDATIONS_ENABLED"));
-  syncer::JsArgList args1(&arg_list1);
-  EXPECT_CALL(reply_handler,
-              HandleJsReply("getNotificationState", HasArgs(args1)));
-
-  {
-    base::WeakPtr<syncer::JsController> js_controller =
-        service_->GetJsController();
-    js_controller->ProcessJsMessage("getNotificationState", args1,
-                                    reply_handler.AsWeakHandle());
-  }
-
-  // This forces the sync thread to process the message and reply.
-  base::WaitableEvent done(false, false);
-  service_->GetBackendForTest()->GetSyncLoopForTesting()
-      ->PostTask(FROM_HERE,
-                 base::Bind(&SignalDone, &done));
-  done.Wait();
-
-  // Call TearDown() to flush the message loops before the mock is destroyed.
-  // TearDown() is idempotent, so it's not a problem that it gets called by the
-  // test fixture again later.
-  TearDown();
-}
-
-TEST_F(ProfileSyncServiceTest,
-       JsControllerProcessJsMessageBasicDelayedBackendInitialization) {
-  StartSyncServiceAndSetInitialSyncEnded();
-
-  StrictMock<syncer::MockJsReplyHandler> reply_handler;
-
-  ListValue arg_list1;
-  arg_list1.Append(Value::CreateStringValue("INVALIDATIONS_ENABLED"));
-  syncer::JsArgList args1(&arg_list1);
-  EXPECT_CALL(reply_handler,
-              HandleJsReply("getNotificationState", HasArgs(args1)));
-
-  {
-    base::WeakPtr<syncer::JsController> js_controller =
-        service_->GetJsController();
-    js_controller->ProcessJsMessage("getNotificationState",
-                                    args1, reply_handler.AsWeakHandle());
-  }
-
-  IssueTestTokens();
-  WaitForBackendInitDone();
-
-  // This forces the sync thread to process the message and reply.
-  base::WaitableEvent done(false, false);
-  service_->GetBackendForTest()->GetSyncLoopForTesting()
-      ->PostTask(FROM_HERE,
-                 base::Bind(&SignalDone, &done));
-  done.Wait();
-}
-
-TEST_F(ProfileSyncServiceTest, GetSyncTokenStatus) {
-  StartSyncServiceAndSetInitialSyncEnded();
-  IssueTestTokens();
+  // Initial status.
   ProfileSyncService::SyncTokenStatus token_status =
-      service_->GetSyncTokenStatus();
+      service()->GetSyncTokenStatus();
   EXPECT_EQ(syncer::CONNECTION_NOT_ATTEMPTED, token_status.connection_status);
   EXPECT_TRUE(token_status.connection_status_update_time.is_null());
   EXPECT_TRUE(token_status.token_request_time.is_null());
   EXPECT_TRUE(token_status.token_receive_time.is_null());
 
-  // Sync engine is given invalid token at initialization and will report
-  // auth error when trying to connect.
-  service_->OnConnectionStatusChange(syncer::CONNECTION_AUTH_ERROR);
-  token_status = service_->GetSyncTokenStatus();
+  // Simulate an auth error.
+  service()->OnConnectionStatusChange(syncer::CONNECTION_AUTH_ERROR);
+
+  // The token request will take the form of a posted task.  Run it.
+  base::RunLoop loop;
+  loop.RunUntilIdle();
+
+  token_status = service()->GetSyncTokenStatus();
   EXPECT_EQ(syncer::CONNECTION_AUTH_ERROR, token_status.connection_status);
   EXPECT_FALSE(token_status.connection_status_update_time.is_null());
   EXPECT_FALSE(token_status.token_request_time.is_null());
   EXPECT_FALSE(token_status.token_receive_time.is_null());
   EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
             token_status.last_get_token_error);
   EXPECT_TRUE(token_status.next_token_request_time.is_null());
 
   // Simulate successful connection.
-  service_->OnConnectionStatusChange(syncer::CONNECTION_OK);
-  token_status = service_->GetSyncTokenStatus();
+  service()->OnConnectionStatusChange(syncer::CONNECTION_OK);
+  token_status = service()->GetSyncTokenStatus();
   EXPECT_EQ(syncer::CONNECTION_OK, token_status.connection_status);
 }
 
 }  // namespace
 }  // namespace browser_sync