Move IsEngineInitialized implementation into SyncService

IsEngineInitialized can now be implemented in terms of GetState (as an
intermediate step before fully removing it). Moving the implementation
into the base class makes sure tests can't expose inconsistent state.

TBR=marq for ios/
who did say LGTM, but forgot to press "+1" :)

Bug: 839834
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I67b5174d4a0bdb0b286526a6deecfed58d01267f
Reviewed-on: https://chromium-review.googlesource.com/1141844
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577119}

chrome/browser/sync/sync_startup_tracker_unittest.cc

@@ -54,10 +54,10 @@ class SyncStartupTrackerTest : public testing::Test {
   void SetupNonInitializedPSS() {
     EXPECT_CALL(*mock_pss_, GetAuthError())
         .WillRepeatedly(ReturnRef(no_error_));
-    EXPECT_CALL(*mock_pss_, IsEngineInitialized())
-        .WillRepeatedly(Return(false));
     EXPECT_CALL(*mock_pss_, GetDisableReasons())
         .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
+    EXPECT_CALL(*mock_pss_, GetState())
+        .WillRepeatedly(Return(syncer::SyncService::State::INITIALIZING));
   }
 
   content::TestBrowserThreadBundle thread_bundle_;
@@ -68,9 +68,10 @@ class SyncStartupTrackerTest : public testing::Test {
 };
 
 TEST_F(SyncStartupTrackerTest, SyncAlreadyInitialized) {
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized()).WillRepeatedly(Return(true));
   EXPECT_CALL(*mock_pss_, GetDisableReasons())
       .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::ACTIVE));
   EXPECT_CALL(observer_, SyncStartupCompleted());
   SyncStartupTracker tracker(profile_.get(), &observer_);
 }
@@ -78,10 +79,11 @@ TEST_F(SyncStartupTrackerTest, SyncAlreadyInitialized) {
 TEST_F(SyncStartupTrackerTest, SyncNotSignedIn) {
   // Make sure that we get a SyncStartupFailed() callback if sync is not logged
   // in.
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized()).WillRepeatedly(Return(false));
   EXPECT_CALL(*mock_pss_, GetDisableReasons())
       .WillRepeatedly(
           Return(syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::DISABLED));
   EXPECT_CALL(observer_, SyncStartupFailed());
   SyncStartupTracker tracker(profile_.get(), &observer_);
 }
@@ -89,9 +91,10 @@ TEST_F(SyncStartupTrackerTest, SyncNotSignedIn) {
 TEST_F(SyncStartupTrackerTest, SyncAuthError) {
   // Make sure that we get a SyncStartupFailed() callback if sync gets an auth
   // error.
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized()).WillRepeatedly(Return(false));
   EXPECT_CALL(*mock_pss_, GetDisableReasons())
       .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::INITIALIZING));
   GoogleServiceAuthError error(
       GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
   EXPECT_CALL(*mock_pss_, GetAuthError()).WillRepeatedly(ReturnRef(error));
@@ -107,7 +110,8 @@ TEST_F(SyncStartupTrackerTest, SyncDelayedInitialization) {
   SyncStartupTracker tracker(profile_.get(), &observer_);
   Mock::VerifyAndClearExpectations(&observer_);
   // Now, mark the PSS as initialized.
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized()).WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::ACTIVE));
   EXPECT_CALL(observer_, SyncStartupCompleted());
   tracker.OnStateChanged(mock_pss_);
 }
@@ -122,9 +126,10 @@ TEST_F(SyncStartupTrackerTest, SyncDelayedAuthError) {
   Mock::VerifyAndClearExpectations(mock_pss_);
 
   // Now, mark the PSS as having an auth error.
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized()).WillRepeatedly(Return(false));
   EXPECT_CALL(*mock_pss_, GetDisableReasons())
       .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::INITIALIZING));
   GoogleServiceAuthError error(
       GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
   EXPECT_CALL(*mock_pss_, GetAuthError()).WillRepeatedly(ReturnRef(error));
@@ -142,12 +147,11 @@ TEST_F(SyncStartupTrackerTest, SyncDelayedUnrecoverableError) {
   Mock::VerifyAndClearExpectations(mock_pss_);
 
   // Now, mark the PSS as having an unrecoverable error.
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized()).WillRepeatedly(Return(false));
   EXPECT_CALL(*mock_pss_, GetDisableReasons())
-      .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
-  GoogleServiceAuthError error(
-      GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
-  EXPECT_CALL(*mock_pss_, GetAuthError()).WillRepeatedly(ReturnRef(error));
+      .WillRepeatedly(
+          Return(syncer::SyncService::DISABLE_REASON_UNRECOVERABLE_ERROR));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::DISABLED));
   EXPECT_CALL(observer_, SyncStartupFailed());
   tracker.OnStateChanged(mock_pss_);
 }

chrome/browser/ui/webui/settings/people_handler_unittest.cc

@@ -253,7 +253,8 @@ class PeopleHandlerTest : public ChromeRenderViewHostTestHarness {
     // An initialized ProfileSyncService will have already completed sync setup
     // and will have an initialized sync engine.
     ASSERT_TRUE(mock_signin_->IsInitialized());
-    ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(true));
+    ON_CALL(*mock_pss_, GetState())
+        .WillByDefault(Return(syncer::SyncService::State::ACTIVE));
   }
 
   void ExpectPageStatusResponse(const std::string& expected_status) {
@@ -396,7 +397,8 @@ TEST_F(PeopleHandlerTest, DisplayConfigureWithEngineDisabledAndCancel) {
       .WillByDefault(Return(syncer::SyncService::DISABLE_REASON_NONE));
   ON_CALL(*mock_pss_, IsFirstSetupComplete()).WillByDefault(Return(false));
   error_ = GoogleServiceAuthError::AuthErrorNone();
-  ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(false));
+  ON_CALL(*mock_pss_, GetState())
+      .WillByDefault(Return(syncer::SyncService::State::INITIALIZING));
   EXPECT_CALL(*mock_pss_, RequestStart());
 
   // We're simulating a user setting up sync, which would cause the engine to
@@ -422,7 +424,9 @@ TEST_F(PeopleHandlerTest,
       .WillByDefault(Return(syncer::SyncService::DISABLE_REASON_NONE));
   error_ = GoogleServiceAuthError::AuthErrorNone();
   // Sync engine is stopped initially, and will start up.
-  ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(false));
+  ON_CALL(*mock_pss_, GetState())
+      .WillByDefault(
+          Return(syncer::SyncService::State::WAITING_FOR_START_REQUEST));
   EXPECT_CALL(*mock_pss_, RequestStart());
   SetDefaultExpectationsForConfigPage();
 
@@ -434,7 +438,8 @@ TEST_F(PeopleHandlerTest,
   Mock::VerifyAndClearExpectations(mock_pss_);
   // Now, act as if the ProfileSyncService has started up.
   SetDefaultExpectationsForConfigPage();
-  ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(true));
+  ON_CALL(*mock_pss_, GetState())
+      .WillByDefault(Return(syncer::SyncService::State::ACTIVE));
   error_ = GoogleServiceAuthError::AuthErrorNone();
   ON_CALL(*mock_pss_, GetAuthError()).WillByDefault(ReturnRef(error_));
   handler_->SyncStartupCompleted();
@@ -459,9 +464,9 @@ TEST_F(PeopleHandlerTest,
       .WillByDefault(Return(syncer::SyncService::DISABLE_REASON_NONE));
   ON_CALL(*mock_pss_, IsFirstSetupComplete()).WillByDefault(Return(false));
   error_ = GoogleServiceAuthError::AuthErrorNone();
-  EXPECT_CALL(*mock_pss_, IsEngineInitialized())
-      .WillOnce(Return(false))
-      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_pss_, GetState())
+      .WillOnce(Return(syncer::SyncService::State::INITIALIZING))
+      .WillRepeatedly(Return(syncer::SyncService::State::ACTIVE));
   EXPECT_CALL(*mock_pss_, RequestStart());
   SetDefaultExpectationsForConfigPage();
   handler_->HandleShowSetupUI(nullptr);
@@ -483,7 +488,8 @@ TEST_F(PeopleHandlerTest, DisplayConfigureWithEngineDisabledAndSigninFailed) {
       .WillByDefault(Return(syncer::SyncService::DISABLE_REASON_NONE));
   ON_CALL(*mock_pss_, IsFirstSetupComplete()).WillByDefault(Return(false));
   error_ = GoogleServiceAuthError::AuthErrorNone();
-  ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(false));
+  ON_CALL(*mock_pss_, GetState())
+      .WillByDefault(Return(syncer::SyncService::State::INITIALIZING));
   EXPECT_CALL(*mock_pss_, RequestStart());
 
   handler_->HandleShowSetupUI(nullptr);
@@ -503,7 +509,8 @@ TEST_F(PeopleHandlerTest, DisplayConfigureWithEngineDisabledAndSigninFailed) {
 // Tests that signals not related to user intention to configure sync don't
 // trigger sync engine start.
 TEST_F(PeopleHandlerTest, OnlyStartEngineWhenConfiguringSync) {
-  ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(false));
+  ON_CALL(*mock_pss_, GetState())
+      .WillByDefault(Return(syncer::SyncService::State::INITIALIZING));
   EXPECT_CALL(*mock_pss_, RequestStart()).Times(0);
   NotifySyncStateChanged();
 }
@@ -767,7 +774,8 @@ TEST_F(PeopleHandlerTest, ShowSigninOnAuthError) {
   ON_CALL(*mock_pss_, IsPassphraseRequired()).WillByDefault(Return(false));
   ON_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
       .WillByDefault(Return(false));
-  ON_CALL(*mock_pss_, IsEngineInitialized()).WillByDefault(Return(false));
+  ON_CALL(*mock_pss_, GetState())
+      .WillByDefault(Return(syncer::SyncService::State::INITIALIZING));
 
 #if defined(OS_CHROMEOS)
   // On ChromeOS, auth errors are ignored - instead we just try to start the

chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc

@@ -262,8 +262,8 @@ class DiceTurnSyncOnHelperTestBase : public testing::Test {
     EXPECT_CALL(*sync_service_mock, GetSetupInProgressHandle()).Times(1);
     ON_CALL(*sync_service_mock, GetDisableReasons())
         .WillByDefault(Return(syncer::SyncService::DISABLE_REASON_NONE));
-    ON_CALL(*sync_service_mock, IsEngineInitialized())
-        .WillByDefault(Return(true));
+    ON_CALL(*sync_service_mock, GetState())
+        .WillByDefault(Return(syncer::SyncService::State::ACTIVE));
   }
 
   void SetExpectationsForSyncStartupPending() {
@@ -272,8 +272,8 @@ class DiceTurnSyncOnHelperTestBase : public testing::Test {
     EXPECT_CALL(*sync_service_mock, GetSetupInProgressHandle()).Times(1);
     ON_CALL(*sync_service_mock, GetDisableReasons())
         .WillByDefault(Return(syncer::SyncService::DISABLE_REASON_NONE));
-    ON_CALL(*sync_service_mock, IsEngineInitialized())
-        .WillByDefault(Return(false));
+    ON_CALL(*sync_service_mock, GetState())
+        .WillByDefault(Return(syncer::SyncService::State::INITIALIZING));
     ON_CALL(*sync_service_mock, GetAuthError())
         .WillByDefault(ReturnRef(kNoAuthError));
   }

components/autofill/core/browser/test_sync_service.cc

@@ -30,10 +30,6 @@ syncer::ModelTypeSet TestSyncService::GetActiveDataTypes() const {
   return data_types_;
 }
 
-bool TestSyncService::IsEngineInitialized() const {
-  return is_engine_initialized_;
-}
-
 bool TestSyncService::IsFirstSetupComplete() const {
   return true;
 }

components/autofill/core/browser/test_sync_service.h

@@ -19,7 +19,6 @@ class TestSyncService : public syncer::FakeSyncService {
   int GetDisableReasons() const override;
   syncer::ModelTypeSet GetPreferredDataTypes() const override;
   syncer::ModelTypeSet GetActiveDataTypes() const override;
-  bool IsEngineInitialized() const override;
   bool IsFirstSetupComplete() const override;
   bool IsUsingSecondaryPassphrase() const override;
   syncer::SyncCycleSnapshot GetLastCycleSnapshot() const override;
@@ -34,10 +33,6 @@ class TestSyncService : public syncer::FakeSyncService {
     data_types_ = data_types;
   }
 
-  void SetIsEngineInitialized(bool is_engine_initialized) {
-    is_engine_initialized_ = is_engine_initialized;
-  }
-
   void SetIsUsingSecondaryPassphrase(bool is_using_secondary_passphrase) {
     is_using_secondary_passphrase_ = is_using_secondary_passphrase;
   }
@@ -50,7 +45,6 @@ class TestSyncService : public syncer::FakeSyncService {
   int disable_reasons_ = DISABLE_REASON_NONE;
   // Used as both "preferred" and "active" data types.
   syncer::ModelTypeSet data_types_;
-  bool is_engine_initialized_ = true;
   bool is_using_secondary_passphrase_ = false;
   bool sync_cycle_complete_ = true;
   GoogleServiceAuthError auth_error_;

components/browser_sync/profile_sync_service.cc

@@ -774,7 +774,7 @@ syncer::SyncService::State ProfileSyncService::GetState() const {
   // Typically, Sync won't start until the initial setup is at least in
   // progress. StartupController::TryStartImmediately bypasses the first setup
   // check though, so we first have to check whether the engine is initialized.
-  if (!IsEngineInitialized()) {
+  if (!engine_initialized_) {
     switch (startup_controller_->GetState()) {
       case syncer::StartupController::State::NOT_STARTED:
         DCHECK(!engine_);
@@ -826,7 +826,7 @@ bool ProfileSyncService::IsFirstSetupComplete() const {
 void ProfileSyncService::SetFirstSetupComplete() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   sync_prefs_.SetFirstSetupComplete();
-  if (IsEngineInitialized()) {
+  if (engine_initialized_) {
     ReconfigureDatatypeManager();
   }
 }
@@ -1319,11 +1319,6 @@ bool ProfileSyncService::IsSignedIn() const {
   return !GetAuthenticatedAccountInfo().account_id.empty();
 }
 
-bool ProfileSyncService::IsEngineInitialized() const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return engine_initialized_;
-}
-
 bool ProfileSyncService::IsPassphraseRequired() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return crypto_->passphrase_required_reason() !=
@@ -1750,7 +1745,7 @@ void ProfileSyncService::OnGaiaAccountsInCookieUpdatedWithCallback(
     const std::vector<gaia::ListedAccount>& accounts,
     const base::Closure& callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (!IsEngineInitialized())
+  if (!engine_initialized_)
     return;
 
   bool cookie_jar_mismatch = HasCookieJarMismatch(accounts);
@@ -2187,7 +2182,7 @@ void ProfileSyncService::OnSetupInProgressHandleDestroyed() {
   if (--outstanding_setup_in_progress_handles_ != 0)
     return;
 
-  if (IsEngineInitialized())
+  if (engine_initialized_)
     ReconfigureDatatypeManager();
   NotifyObservers();
 }

components/browser_sync/profile_sync_service.h

@@ -252,7 +252,6 @@ class ProfileSyncService : public syncer::SyncService,
       override;
   bool IsSetupInProgress() const override;
   const GoogleServiceAuthError& GetAuthError() const override;
-  bool IsEngineInitialized() const override;
   sync_sessions::OpenTabsUIDelegate* GetOpenTabsUIDelegate() override;
   bool IsPassphraseRequiredForDecryption() const override;
   base::Time GetExplicitPassphraseTime() const override;

components/browser_sync/profile_sync_service_mock.h

@@ -76,7 +76,6 @@ class ProfileSyncServiceMock : public ProfileSyncService {
   MOCK_CONST_METHOD0(GetAuthError, const GoogleServiceAuthError&());
   MOCK_CONST_METHOD0(IsSetupInProgress, bool());
   MOCK_CONST_METHOD0(GetLastSyncedTime, base::Time());
-  MOCK_CONST_METHOD0(IsEngineInitialized, bool());
   MOCK_CONST_METHOD0(IsSyncConfirmationNeeded, bool());
   MOCK_METHOD1(OnActionableError, void(const syncer::SyncProtocolError&));
   MOCK_CONST_METHOD1(IsDataTypeControllerRunning, bool(syncer::ModelType));

components/suggestions/suggestions_service_impl_unittest.cc

@@ -86,9 +86,7 @@ class MockSyncService : public syncer::FakeSyncService {
   MockSyncService() {}
   ~MockSyncService() override {}
   MOCK_CONST_METHOD0(GetDisableReasons, int());
-  MOCK_CONST_METHOD0(IsEngineInitialized, bool());
-  MOCK_CONST_METHOD0(IsFirstSetupComplete, bool());
-  MOCK_CONST_METHOD0(ConfigurationDone, bool());
+  MOCK_CONST_METHOD0(GetState, State());
   MOCK_CONST_METHOD0(IsLocalSyncEnabled, bool());
   MOCK_CONST_METHOD0(IsUsingSecondaryPassphrase, bool());
   MOCK_CONST_METHOD0(GetPreferredDataTypes, syncer::ModelTypeSet());
@@ -157,15 +155,9 @@ class SuggestionsServiceTest : public testing::Test {
     EXPECT_CALL(*sync_service(), GetDisableReasons())
         .Times(AnyNumber())
         .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
-    EXPECT_CALL(*sync_service(), IsEngineInitialized())
+    EXPECT_CALL(*sync_service(), GetState())
         .Times(AnyNumber())
-        .WillRepeatedly(Return(true));
-    EXPECT_CALL(*sync_service(), IsFirstSetupComplete())
-        .Times(AnyNumber())
-        .WillRepeatedly(Return(true));
-    EXPECT_CALL(*sync_service(), ConfigurationDone())
-        .Times(AnyNumber())
-        .WillRepeatedly(Return(true));
+        .WillRepeatedly(Return(syncer::SyncService::State::ACTIVE));
     EXPECT_CALL(*sync_service(), IsLocalSyncEnabled())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
@@ -348,8 +340,8 @@ TEST_F(SuggestionsServiceTest, IgnoresUninterestingSyncChange) {
 // This should *not* result in an automatic fetch.
 TEST_F(SuggestionsServiceTest, DoesNotFetchOnStartup) {
   // The sync service starts out inactive.
-  EXPECT_CALL(*sync_service(), IsEngineInitialized())
-      .WillRepeatedly(Return(false));
+  EXPECT_CALL(*sync_service(), GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::INITIALIZING));
   static_cast<SyncServiceObserver*>(suggestions_service())
       ->OnStateChanged(sync_service());
 
@@ -357,8 +349,8 @@ TEST_F(SuggestionsServiceTest, DoesNotFetchOnStartup) {
   ASSERT_FALSE(suggestions_service()->HasPendingRequestForTesting());
 
   // Sync getting enabled should not result in a fetch.
-  EXPECT_CALL(*sync_service(), IsEngineInitialized())
-      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*sync_service(), GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::ACTIVE));
   static_cast<SyncServiceObserver*>(suggestions_service())
       ->OnStateChanged(sync_service());
 
@@ -391,8 +383,8 @@ TEST_F(SuggestionsServiceTest, BuildUrlWithDefaultMinZeroParamForFewFeature) {
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncNotInitializedEnabled) {
-  EXPECT_CALL(*sync_service(), IsEngineInitialized())
-      .WillRepeatedly(Return(false));
+  EXPECT_CALL(*sync_service(), GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::INITIALIZING));
   static_cast<SyncServiceObserver*>(suggestions_service())
       ->OnStateChanged(sync_service());
 
@@ -418,6 +410,8 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
   EXPECT_CALL(*sync_service(), GetDisableReasons())
       .Times(AnyNumber())
       .WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_USER_CHOICE));
+  EXPECT_CALL(*sync_service(), GetState())
+      .WillRepeatedly(Return(syncer::SyncService::State::DISABLED));
 
   base::MockCallback<SuggestionsService::ResponseCallback> callback;
   auto subscription = suggestions_service()->AddCallback(callback.Get());

components/sync/driver/fake_sync_service.cc

@@ -49,11 +49,6 @@ syncer::SyncService::State FakeSyncService::GetState() const {
   }
   // From this point on, Sync can start in principle.
   DCHECK(CanSyncStart());
-  // Note: We don't distinguish here if the engine doesn't exist at all, or
-  // exists but hasn't finished initializing.
-  if (!IsEngineInitialized()) {
-    return State::INITIALIZING;
-  }
   if (!IsFirstSetupComplete()) {
     return State::PENDING_DESIRED_CONFIGURATION;
   }
@@ -117,10 +112,6 @@ const GoogleServiceAuthError& FakeSyncService::GetAuthError() const {
   return error_;
 }
 
-bool FakeSyncService::IsEngineInitialized() const {
-  return false;
-}
-
 sync_sessions::OpenTabsUIDelegate* FakeSyncService::GetOpenTabsUIDelegate() {
   return nullptr;
 }

components/sync/driver/fake_sync_service.h

@@ -60,7 +60,6 @@ class FakeSyncService : public SyncService {
       override;
   bool IsSetupInProgress() const override;
   const GoogleServiceAuthError& GetAuthError() const override;
-  bool IsEngineInitialized() const override;
   sync_sessions::OpenTabsUIDelegate* GetOpenTabsUIDelegate() override;
   bool IsPassphraseRequiredForDecryption() const override;
   base::Time GetExplicitPassphraseTime() const override;

components/sync/driver/sync_service.cc

@@ -22,9 +22,36 @@ bool SyncService::IsSyncAllowed() const {
          !HasDisableReason(DISABLE_REASON_ENTERPRISE_POLICY);
 }
 
+bool SyncService::IsEngineInitialized() const {
+  switch (GetState()) {
+    case State::DISABLED:
+    case State::WAITING_FOR_START_REQUEST:
+    case State::START_DEFERRED:
+    case State::INITIALIZING:
+      return false;
+    case State::PENDING_DESIRED_CONFIGURATION:
+    case State::CONFIGURING:
+    case State::ACTIVE:
+      return true;
+  }
+  NOTREACHED();
+  return false;
+}
+
 bool SyncService::IsSyncActive() const {
-  State state = GetState();
-  return state == State::CONFIGURING || state == State::ACTIVE;
+  switch (GetState()) {
+    case State::DISABLED:
+    case State::WAITING_FOR_START_REQUEST:
+    case State::START_DEFERRED:
+    case State::INITIALIZING:
+    case State::PENDING_DESIRED_CONFIGURATION:
+      return false;
+    case State::CONFIGURING:
+    case State::ACTIVE:
+      return true;
+  }
+  NOTREACHED();
+  return false;
 }
 
 bool SyncService::IsFirstSetupInProgress() const {

components/sync/driver/sync_service.h

@@ -265,9 +265,10 @@ class SyncService : public DataTypeEncryptionHandler, public KeyedService {
   // Equivalent to "HasDisableReason(DISABLE_REASON_UNRECOVERABLE_ERROR)".
   bool HasUnrecoverableError() const;
 
-  // Returns true if the SyncEngine has told us it's ready to accept changes.
-  // DEPRECATED! Use GetState instead.
-  virtual bool IsEngineInitialized() const = 0;
+  // DEPRECATED! Use GetState instead. Equivalent to
+  // "GetState() == State::PENDING_DESIRED_CONFIGURATION ||
+  // GetState() == State::CONFIGURING || GetState() == State::ACTIVE".
+  bool IsEngineInitialized() const;
 
   // Return the active OpenTabsUIDelegate. If open/proxy tabs is not enabled or
   // not currently syncing, returns nullptr.

components/sync/driver/sync_service_utils_unittest.cc

@@ -14,15 +14,13 @@ namespace syncer {
 
 class TestSyncService : public FakeSyncService {
  public:
-  TestSyncService() { SetConfigurationDone(false); }
+  TestSyncService() = default;
   ~TestSyncService() override = default;
 
   void SetDisableReasons(int disable_reasons) {
     disable_reasons_ = disable_reasons;
   }
-  void SetEngineInitialized(bool initialized) {
-    engine_initialized_ = initialized;
-  }
+  void SetState(State state) { state_ = state; }
   void SetLocalSyncEnabled(bool local) { local_sync_enabled_ = local; }
   void SetPreferredDataTypes(const ModelTypeSet& types) {
     preferred_data_types_ = types;
@@ -37,7 +35,7 @@ class TestSyncService : public FakeSyncService {
 
   // SyncService implementation.
   int GetDisableReasons() const override { return disable_reasons_; }
-  bool IsEngineInitialized() const override { return engine_initialized_; }
+  State GetState() const override { return state_; }
   bool IsLocalSyncEnabled() const override { return local_sync_enabled_; }
   bool IsFirstSetupComplete() const override { return true; }
   ModelTypeSet GetPreferredDataTypes() const override {
@@ -78,7 +76,7 @@ class TestSyncService : public FakeSyncService {
 
  private:
   int disable_reasons_ = DISABLE_REASON_PLATFORM_OVERRIDE;
-  bool engine_initialized_ = false;
+  State state_ = State::DISABLED;
   bool sync_cycle_complete_ = false;
   bool local_sync_enabled_ = false;
   ModelTypeSet preferred_data_types_;
@@ -89,12 +87,11 @@ class TestSyncService : public FakeSyncService {
 TEST(SyncServiceUtilsTest, UploadToGoogleDisabledIfSyncNotAllowed) {
   TestSyncService service;
 
-  // If sync is not allowed, uploading should never be enabled, even if
-  // configuration is done and all the data types are enabled.
+  // If sync is not allowed, uploading should never be enabled, even if all the
+  // data types are enabled.
   service.SetDisableReasons(
       syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY);
 
-  service.SetConfigurationDone(true);
   service.SetPreferredDataTypes(ProtocolTypes());
   service.SetActiveDataTypes(ProtocolTypes());
 
@@ -104,6 +101,7 @@ TEST(SyncServiceUtilsTest, UploadToGoogleDisabledIfSyncNotAllowed) {
   // Once sync gets allowed (e.g. policy is updated), uploading should not be
   // disabled anymore (though not necessarily active yet).
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
+  service.SetState(syncer::SyncService::State::WAITING_FOR_START_REQUEST);
 
   EXPECT_NE(UploadState::NOT_ACTIVE,
             GetUploadToGoogleState(&service, syncer::BOOKMARKS));
@@ -113,6 +111,7 @@ TEST(SyncServiceUtilsTest,
      UploadToGoogleInitializingUntilConfiguredAndActiveAndSyncCycleComplete) {
   TestSyncService service;
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
+  service.SetState(syncer::SyncService::State::WAITING_FOR_START_REQUEST);
   service.SetPreferredDataTypes(ProtocolTypes());
   service.SetActiveDataTypes(ProtocolTypes());
 
@@ -121,15 +120,11 @@ TEST(SyncServiceUtilsTest,
             GetUploadToGoogleState(&service, syncer::BOOKMARKS));
 
   // Finished configuration is not enough, still INITIALIZING.
-  service.SetConfigurationDone(true);
-  EXPECT_EQ(UploadState::INITIALIZING,
-            GetUploadToGoogleState(&service, syncer::BOOKMARKS));
-
-  service.SetEngineInitialized(true);
+  service.SetState(syncer::SyncService::State::ACTIVE);
   EXPECT_EQ(UploadState::INITIALIZING,
             GetUploadToGoogleState(&service, syncer::BOOKMARKS));
 
-  // Only after sync is both configured and active is upload actually ACTIVE.
+  // Only after a sync cycle has been completed is upload actually ACTIVE.
   service.SetSyncCycleComplete(true);
   EXPECT_EQ(UploadState::ACTIVE,
             GetUploadToGoogleState(&service, syncer::BOOKMARKS));
@@ -138,8 +133,7 @@ TEST(SyncServiceUtilsTest,
 TEST(SyncServiceUtilsTest, UploadToGoogleDisabledForModelType) {
   TestSyncService service;
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
-  service.SetConfigurationDone(true);
-  service.SetEngineInitialized(true);
+  service.SetState(syncer::SyncService::State::ACTIVE);
   service.SetSyncCycleComplete(true);
 
   // Sync is enabled only for a specific model type.
@@ -162,8 +156,7 @@ TEST(SyncServiceUtilsTest,
      UploadToGoogleDisabledForModelTypeThatFailedToStart) {
   TestSyncService service;
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
-  service.SetConfigurationDone(true);
-  service.SetEngineInitialized(true);
+  service.SetState(syncer::SyncService::State::ACTIVE);
   service.SetSyncCycleComplete(true);
 
   // Sync is enabled for some model types.
@@ -186,8 +179,7 @@ TEST(SyncServiceUtilsTest, UploadToGoogleDisabledIfLocalSyncEnabled) {
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
   service.SetPreferredDataTypes(ProtocolTypes());
   service.SetActiveDataTypes(ProtocolTypes());
-  service.SetEngineInitialized(true);
-  service.SetConfigurationDone(true);
+  service.SetState(syncer::SyncService::State::ACTIVE);
   service.SetSyncCycleComplete(true);
 
   // Sanity check: Upload is active now.
@@ -207,8 +199,7 @@ TEST(SyncServiceUtilsTest, UploadToGoogleDisabledOnPersistentAuthError) {
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
   service.SetPreferredDataTypes(ProtocolTypes());
   service.SetActiveDataTypes(ProtocolTypes());
-  service.SetEngineInitialized(true);
-  service.SetConfigurationDone(true);
+  service.SetState(syncer::SyncService::State::ACTIVE);
   service.SetSyncCycleComplete(true);
 
   // Sanity check: Upload is active now.
@@ -237,6 +228,7 @@ TEST(SyncServiceUtilsTest, UploadToGoogleDisabledOnPersistentAuthError) {
   // Once the auth error is resolved (e.g. user re-authenticated), uploading is
   // active again.
   service.set_auth_error(GoogleServiceAuthError(GoogleServiceAuthError::NONE));
+  service.SetState(syncer::SyncService::State::ACTIVE);
 
   EXPECT_EQ(UploadState::ACTIVE,
             GetUploadToGoogleState(&service, syncer::BOOKMARKS));
@@ -247,8 +239,7 @@ TEST(SyncServiceUtilsTest, UploadToGoogleDisabledIfCustomPassphraseInUse) {
   service.SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
   service.SetPreferredDataTypes(ProtocolTypes());
   service.SetActiveDataTypes(ProtocolTypes());
-  service.SetEngineInitialized(true);
-  service.SetConfigurationDone(true);
+  service.SetState(syncer::SyncService::State::ACTIVE);
   service.SetSyncCycleComplete(true);
 
   // Sanity check: Upload is ACTIVE, even for data types that are always

components/sync/user_events/user_event_service_impl_unittest.cc

@@ -80,7 +80,10 @@ class TestSyncService : public FakeSyncService {
         is_using_secondary_passphrase_(is_using_secondary_passphrase),
         preferred_data_types_(preferred_data_types) {}
 
-  bool IsEngineInitialized() const override { return is_engine_initialized_; }
+  State GetState() const override {
+    return is_engine_initialized_ ? State::ACTIVE : State::INITIALIZING;
+  }
+
   bool IsUsingSecondaryPassphrase() const override {
     return is_using_secondary_passphrase_;
   }

components/ukm/observers/sync_disable_observer_unittest.cc

@@ -53,7 +53,9 @@ class MockSyncService : public syncer::FakeSyncService {
   void RemoveObserver(syncer::SyncServiceObserver* observer) override {
     observers_.RemoveObserver(observer);
   }
-  bool IsEngineInitialized() const override { return initialized_; }
+  State GetState() const override {
+    return initialized_ ? State::ACTIVE : State::INITIALIZING;
+  }
   bool IsUsingSecondaryPassphrase() const override { return has_passphrase_; }
   syncer::ModelTypeSet GetPreferredDataTypes() const override {
     return preferred_data_types_;

components/unified_consent/unified_consent_service_unittest.cc

@@ -22,8 +22,8 @@ namespace {
 class TestSyncService : public syncer::FakeSyncService {
  public:
   int GetDisableReasons() const override { return DISABLE_REASON_NONE; }
+  State GetState() const override { return state_; }
   bool IsFirstSetupComplete() const override { return true; }
-  bool IsEngineInitialized() const override { return engine_initialized_; }
   void AddObserver(syncer::SyncServiceObserver* observer) override {
     observer_ = observer;
   }
@@ -32,9 +32,7 @@ class TestSyncService : public syncer::FakeSyncService {
     is_syncing_everything_ = sync_everything;
   }
 
-  void SetEngineInitialized(bool engine_initialized) {
-    engine_initialized_ = engine_initialized;
-  }
+  void SetState(State state) { state_ = state; }
   void FireStateChanged() {
     if (observer_)
       observer_->OnStateChanged(this);
@@ -46,7 +44,7 @@ class TestSyncService : public syncer::FakeSyncService {
 
  private:
   syncer::SyncServiceObserver* observer_ = nullptr;
-  bool engine_initialized_ = true;
+  State state_ = State::ACTIVE;
   bool is_syncing_everything_ = false;
 };
 
@@ -167,7 +165,7 @@ TEST_F(UnifiedConsentServiceTest, EnableUnfiedConsent_SyncNotActive) {
   EXPECT_FALSE(consent_service_->IsUnifiedConsentGiven());
 
   // Make sure sync is not active.
-  sync_service_.SetEngineInitialized(false);
+  sync_service_.SetState(syncer::SyncService::State::INITIALIZING);
   EXPECT_FALSE(sync_service_.IsEngineInitialized());
   EXPECT_NE(sync_service_.GetState(), syncer::SyncService::State::ACTIVE);
 
@@ -179,8 +177,7 @@ TEST_F(UnifiedConsentServiceTest, EnableUnfiedConsent_SyncNotActive) {
   EXPECT_FALSE(sync_service_.IsSyncingEverything());
 
   // Initalize sync engine and therefore activate sync.
-  sync_service_.SetEngineInitialized(true);
-  EXPECT_EQ(sync_service_.GetState(), syncer::SyncService::State::ACTIVE);
+  sync_service_.SetState(syncer::SyncService::State::ACTIVE);
   sync_service_.FireStateChanged();
 
   // UnifiedConsentService starts syncing everything.

components/unified_consent/url_keyed_data_collection_consent_helper_unittest.cc

@@ -31,12 +31,12 @@ class TestSyncService : public syncer::FakeSyncService {
 
   // syncer::FakeSyncService:
   int GetDisableReasons() const override { return DISABLE_REASON_NONE; }
+  State GetState() const override { return State::ACTIVE; }
   syncer::ModelTypeSet GetPreferredDataTypes() const override {
     return syncer::ModelTypeSet(syncer::ModelType::HISTORY_DELETE_DIRECTIVES,
                                 syncer::ModelType::USER_EVENTS);
   }
   bool IsFirstSetupComplete() const override { return true; }
-  bool IsEngineInitialized() const override { return true; }
 
   syncer::SyncCycleSnapshot GetLastCycleSnapshot() const override {
     if (!sync_initialized_)

ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_coordinator_unittest.mm

@@ -117,8 +117,8 @@ class RecentTabsTableCoordinatorTest : public BlockCleanupTest {
     ON_CALL(*sync_service, GetRegisteredDataTypes())
         .WillByDefault(Return(syncer::ModelTypeSet()));
     sync_service->Initialize();
-    EXPECT_CALL(*sync_service, IsEngineInitialized())
-        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*sync_service, GetState())
+        .WillRepeatedly(Return(syncer::SyncService::State::ACTIVE));
     EXPECT_CALL(*sync_service, GetOpenTabsUIDelegate())
         .WillRepeatedly(Return(nullptr));
 

ios/chrome/browser/ui/settings/passphrase_collection_view_controller_test.mm

@@ -87,8 +87,8 @@ void PassphraseCollectionViewControllerTest::SetUp() {
   // Set up non-default return values for our sync service mock.
   ON_CALL(*fake_sync_service_, IsPassphraseRequired())
       .WillByDefault(Return(true));
-  ON_CALL(*fake_sync_service_, IsEngineInitialized())
-      .WillByDefault(Return(true));
+  ON_CALL(*fake_sync_service_, GetState())
+      .WillByDefault(Return(syncer::SyncService::State::ACTIVE));
 
   ios::FakeChromeIdentityService* identityService =
       ios::FakeChromeIdentityService::GetInstanceFromChromeProvider();

ios/chrome/browser/ui/settings/sync_encryption_collection_view_controller_unittest.mm

@@ -53,8 +53,8 @@ class SyncEncryptionCollectionViewControllerTest
         static_cast<browser_sync::ProfileSyncServiceMock*>(
             ProfileSyncServiceFactory::GetForBrowserState(
                 chrome_browser_state_.get()));
-    ON_CALL(*mock_profile_sync_service_, IsEngineInitialized())
-        .WillByDefault(Return(true));
+    ON_CALL(*mock_profile_sync_service_, GetState())
+        .WillByDefault(Return(syncer::SyncService::State::ACTIVE));
     ON_CALL(*mock_profile_sync_service_, IsUsingSecondaryPassphrase())
         .WillByDefault(Return(true));
 

ios/chrome/browser/ui/settings/sync_settings_collection_view_controller_unittest.mm

@@ -187,8 +187,8 @@ class SyncSettingsCollectionViewControllerTest
         static_cast<browser_sync::ProfileSyncServiceMock*>(
             ProfileSyncServiceFactory::GetForBrowserState(
                 chrome_browser_state_.get()));
-    ON_CALL(*mock_profile_sync_service_, IsEngineInitialized())
-        .WillByDefault(Return(true));
+    ON_CALL(*mock_profile_sync_service_, GetState())
+        .WillByDefault(Return(syncer::SyncService::State::ACTIVE));
     ON_CALL(*mock_profile_sync_service_, GetRegisteredDataTypes())
         .WillByDefault(Return(syncer::ModelTypeSet()));
     mock_profile_sync_service_->Initialize();