[Sync] Inject startup dependencies into StartupController.

components/sync_driver/startup_controller_unittest.cc

Index: components/sync_driver/startup_controller_unittest.cc
diff --git a/components/sync_driver/startup_controller_unittest.cc b/components/sync_driver/startup_controller_unittest.cc
index c6de60f1180d37cae812d768e8386ffd79c540ed..f7b2289788b9adeb7a8df832f2a3bfc8530a486c 100644
--- a/components/sync_driver/startup_controller_unittest.cc
+++ b/components/sync_driver/startup_controller_unittest.cc
@@ -10,8 +10,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/time/time.h"
-#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
-#include "components/sync_driver/signin_manager_wrapper.h"
 #include "components/sync_driver/sync_driver_switches.h"
 #include "components/sync_driver/sync_prefs.h"
 #include "components/syncable_prefs/testing_pref_service_syncable.h"
@@ -19,9 +17,6 @@
 
 namespace browser_sync {
 
-static const char kTestUser[] = "test@gmail.com";
-static const char kTestToken[] = "testToken";
-
 // These are coupled to the implementation of StartupController's
 // GetBackendInitializationStateString which is used by about:sync. We use it
 // as a convenient way to verify internal state and that the class is
@@ -30,53 +25,32 @@ static const char kStateStringStarted[] = "Started";
 static const char kStateStringDeferred[] = "Deferred";
 static const char kStateStringNotStarted[] = "Not started";
 
-class FakeSigninManagerWrapper : public SigninManagerWrapper {
- public:
-  FakeSigninManagerWrapper() : SigninManagerWrapper(NULL) {}
-  std::string GetEffectiveUsername() const override { return std::string(); }
-
-  std::string GetAccountIdToUse() const override { return account_id_; }
-
-  void set_account_id(const std::string& account_id) {
-    account_id_ = account_id;
-  }
-
- private:
-  std::string account_id_;
-};
-
 class StartupControllerTest : public testing::Test {
  public:
-  StartupControllerTest() : started_(false) {}
+  StartupControllerTest() : can_start_(false), started_(false) {}
 
   void SetUp() override {
     sync_driver::SyncPrefs::RegisterProfilePrefs(pref_service_.registry());
     sync_prefs_.reset(new sync_driver::SyncPrefs(&pref_service_));
-    token_service_.reset(new FakeProfileOAuth2TokenService());
-    signin_.reset(new FakeSigninManagerWrapper());
-
-    SetUpController();
+    controller_.reset(new StartupController(
+        sync_prefs_.get(),
+        base::Bind(&StartupControllerTest::CanStart, base::Unretained(this)),
+        base::Bind(&StartupControllerTest::FakeStartBackend,
+                   base::Unretained(this))));
+    controller_->Reset(syncer::UserTypes());
+    controller_->OverrideFallbackTimeoutForTest(
+        base::TimeDelta::FromSeconds(0));
   }
 
   void TearDown() override {
     controller_.reset();

[Nicolas Zea, 2016/04/04 20:44:17]

is it necessary to have these here? These will automatically get destroyed when
after the test runs anyways right?

[maxbogue, 2016/04/04 22:56:55]

Good catch! They must have been explicitly called before to ensure some ordering
wrt the token service Shutdown() call. Shouldn't be needed now; I've removed the
whole method and made |controller_| the last member variable so it still gets
destroyed first.

-    signin_.reset();
-    token_service_->Shutdown();
-    token_service_.reset();
     sync_prefs_.reset();
     started_ = false;
   }
 
-  void SetUpController() {
-    started_ = false;
-    base::Closure fake_start_backend = base::Bind(
-        &StartupControllerTest::FakeStartBackend, base::Unretained(this));
-    controller_.reset(new StartupController(token_service(), sync_prefs_.get(),
-                                            signin_.get(), fake_start_backend));
-    controller_->Reset(syncer::UserTypes());
-    controller_->OverrideFallbackTimeoutForTest(
-        base::TimeDelta::FromSeconds(0));
-  }
+  bool CanStart() { return can_start_; }
+
+  void SetCanStart(bool can_start) { can_start_ = can_start; }
 
   void FakeStartBackend() {
     started_ = true;
@@ -107,19 +81,14 @@ class StartupControllerTest : public testing::Test {
   bool started() const { return started_; }
   void clear_started() { started_ = false; }
   StartupController* controller() { return controller_.get(); }
-  FakeSigninManagerWrapper* signin() { return signin_.get(); }
-  FakeProfileOAuth2TokenService* token_service() {
-    return token_service_.get();
-  }
   sync_driver::SyncPrefs* sync_prefs() { return sync_prefs_.get(); }
 
  private:
+  bool can_start_;
   bool started_;
   base::MessageLoop message_loop_;
   syncable_prefs::TestingPrefServiceSyncable pref_service_;
   scoped_ptr<StartupController> controller_;
-  scoped_ptr<FakeSigninManagerWrapper> signin_;
-  scoped_ptr<FakeProfileOAuth2TokenService> token_service_;
   scoped_ptr<sync_driver::SyncPrefs> sync_prefs_;
 };
 
@@ -128,45 +97,23 @@ TEST_F(StartupControllerTest, Basic) {
   controller()->TryStart();
   ExpectNotStarted();
 
-  sync_prefs()->SetFirstSetupComplete();
-  controller()->TryStart();
-  ExpectNotStarted();
-
-  signin()->set_account_id(kTestUser);
-  controller()->TryStart();
-  ExpectNotStarted();
-
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->TryStart();
-  ExpectStartDeferred();
-}
-
-// Test that sync doesn't start when not requested even if all other
-// conditons are met.
-TEST_F(StartupControllerTest, NotRequested) {
-  sync_prefs()->SetFirstSetupComplete();
-  sync_prefs()->SetSyncRequested(false);
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
-  controller()->TryStart();
-  ExpectNotStarted();
+  ExpectStarted();
 }
 
-// Test that sync doesn't when managed even if all other conditons are met.
-TEST_F(StartupControllerTest, Managed) {
+// Test that sync defers if first setup is complete.
+TEST_F(StartupControllerTest, DefersAfterFirstSetupComplete) {
   sync_prefs()->SetFirstSetupComplete();
-  sync_prefs()->SetManagedForTest(true);
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->TryStart();
-  ExpectNotStarted();
+  ExpectStartDeferred();
 }
 
 // Test that a data type triggering startup starts sync immediately.
 TEST_F(StartupControllerTest, NoDeferralDataTypeTrigger) {
   sync_prefs()->SetFirstSetupComplete();
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->OnDataTypeRequestsSyncStartup(syncer::SESSIONS);
   ExpectStarted();
 }
@@ -175,8 +122,7 @@ TEST_F(StartupControllerTest, NoDeferralDataTypeTrigger) {
 // sync immediately.
 TEST_F(StartupControllerTest, DataTypeTriggerInterruptsDeferral) {
   sync_prefs()->SetFirstSetupComplete();
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->TryStart();
   ExpectStartDeferred();
 
@@ -194,8 +140,7 @@ TEST_F(StartupControllerTest, DataTypeTriggerInterruptsDeferral) {
 // conditions are met and no data type requests sync.
 TEST_F(StartupControllerTest, FallbackTimer) {
   sync_prefs()->SetFirstSetupComplete();
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->TryStart();
   ExpectStartDeferred();
 
@@ -214,10 +159,9 @@ TEST_F(StartupControllerTest, NoDeferralWithoutSessionsSync) {
   sync_prefs()->SetKeepEverythingSynced(false);
   sync_prefs()->SetPreferredDataTypes(syncer::UserTypes(), types);
   controller()->Reset(syncer::UserTypes());
-  sync_prefs()->SetFirstSetupComplete();
 
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  sync_prefs()->SetFirstSetupComplete();
+  SetCanStart(true);
   controller()->TryStart();
   ExpectStarted();
 }
@@ -234,9 +178,7 @@ TEST_F(StartupControllerTest, FallbackTimerWaits) {
 // Test that sync starts immediately when setup in progress is true.
 TEST_F(StartupControllerTest, NoDeferralSetupInProgressTrigger) {
   sync_prefs()->SetFirstSetupComplete();
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
-
+  SetCanStart(true);
   controller()->SetSetupInProgress(true);
   ExpectStarted();
 }
@@ -245,8 +187,7 @@ TEST_F(StartupControllerTest, NoDeferralSetupInProgressTrigger) {
 // and starts sync immediately.
 TEST_F(StartupControllerTest, SetupInProgressTriggerInterruptsDeferral) {
   sync_prefs()->SetFirstSetupComplete();
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->TryStart();
   ExpectStartDeferred();
 
@@ -256,8 +197,7 @@ TEST_F(StartupControllerTest, SetupInProgressTriggerInterruptsDeferral) {
 
 // Test that start isn't deferred on the first start but is on restarts.
 TEST_F(StartupControllerTest, DeferralOnRestart) {
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
   controller()->TryStart();
   ExpectStarted();
 
@@ -270,8 +210,7 @@ TEST_F(StartupControllerTest, DeferralOnRestart) {
 
 // Test that setup-in-progress tracking is persistent across a Reset.
 TEST_F(StartupControllerTest, ResetDuringSetup) {
-  signin()->set_account_id(kTestUser);
-  token_service()->UpdateCredentials(kTestUser, kTestToken);
+  SetCanStart(true);
 
   // Simulate UI telling us setup is in progress.
   controller()->SetSetupInProgress(true);