Introduce syncer::StartupController::State enum

This adds a State enum to StartupController and exposes it via GetState().
It's not strictly required right now, but
a) it'll help with sanitizing ProfileSyncService's state,
b) it makes the tests a bit nicer, now they can use the enum instead of
   relying on debug strings, and
c) it lets us move GetEngineInitializationStateString from
   StartupController to ProfileSyncService, where it's slightly less
   out of place (bonus: extra DCHECKs for consistent state).

Other semi-related test cleanups: don't call SetFirstSetupComplete from
FakeStartBackend (no idea why that call was there), and don't check for
the kSyncDisableDeferredStartup cmdline flag (if you run the tests with
a custom flag, it's your own fault if they fail).

Bug: 839834
Change-Id: I16f83be8ee828024329cbce899be4723241323db
Reviewed-on: https://chromium-review.googlesource.com/1109939
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569273}

components/browser_sync/profile_sync_service.cc

@@ -1189,7 +1189,19 @@ std::string ProfileSyncService::QuerySyncStatusSummaryString() {
 
 std::string ProfileSyncService::GetEngineInitializationStateString() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return startup_controller_->GetEngineInitializationStateString();
+  switch (startup_controller_->GetState()) {
+    case syncer::StartupController::State::NOT_STARTED:
+      DCHECK(!engine_);
+      return "Not started";
+    case syncer::StartupController::State::STARTING_DEFERRED:
+      DCHECK(!engine_);
+      return "Deferred";
+    case syncer::StartupController::State::STARTED:
+      DCHECK(engine_);
+      return "Started";
+  }
+  NOTREACHED();
+  return std::string();
 }
 
 bool ProfileSyncService::IsSetupInProgress() const {

components/sync/driver/startup_controller.cc

@@ -158,12 +158,12 @@ void StartupController::OnFallbackStartupTimerExpired() {
   TryStart();
 }
 
-std::string StartupController::GetEngineInitializationStateString() const {
+StartupController::State StartupController::GetState() const {
   if (!start_engine_time_.is_null())
-    return "Started";
+    return State::STARTED;
   if (!start_up_time_.is_null())
-    return "Deferred";
-  return "Not started";
+    return State::STARTING_DEFERRED;
+  return State::NOT_STARTED;
 }
 
 void StartupController::OnDataTypeRequestsSyncStartup(ModelType type) {

components/sync/driver/startup_controller.h

@@ -5,8 +5,6 @@
 #ifndef COMPONENTS_SYNC_DRIVER_STARTUP_CONTROLLER_H_
 #define COMPONENTS_SYNC_DRIVER_STARTUP_CONTROLLER_H_
 
-#include <string>
-
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
@@ -20,6 +18,17 @@ class SyncPrefs;
 // pertaining to initialization of the SyncEngine.
 class StartupController {
  public:
+  enum class State {
+    // Startup has not been triggered yet.
+    NOT_STARTED,
+    // Startup has been triggered but is deferred. The actual startup will
+    // happen once the deferred delay expires (or when immediate startup is
+    // requested, whichever happens first).
+    STARTING_DEFERRED,
+    // Startup has happened, i.e. |start_engine| has been run.
+    STARTED
+  };
+
   StartupController(const SyncPrefs* sync_prefs,
                     base::RepeatingCallback<bool()> can_start,
                     base::RepeatingClosure start_engine);
@@ -50,9 +59,10 @@ class StartupController {
   // Sets the setup in progress flag and tries to start sync if it's true.
   void SetSetupInProgress(bool setup_in_progress);
 
+  State GetState() const;
+
   bool IsSetupInProgress() const { return setup_in_progress_; }
   base::Time start_engine_time() const { return start_engine_time_; }
-  std::string GetEngineInitializationStateString() const;
 
  private:
   enum StartUpDeferredOption { STARTUP_DEFERRED, STARTUP_IMMEDIATE };

components/sync/driver/startup_controller_unittest.cc

@@ -16,14 +16,6 @@
 
 namespace syncer {
 
-// These are coupled to the implementation of StartupController's
-// GetEngineInitializationStateString which is used by about:sync. We use it
-// as a convenient way to verify internal state and that the class is
-// outputting the correct values for the debug string.
-static const char kStateStringStarted[] = "Started";
-static const char kStateStringDeferred[] = "Deferred";
-static const char kStateStringNotStarted[] = "Not started";
-
 class StartupControllerTest : public testing::Test {
  public:
   StartupControllerTest() : can_start_(false), started_(false) {}
@@ -46,23 +38,18 @@ class StartupControllerTest : public testing::Test {
 
   void ExpectStarted() {
     EXPECT_TRUE(started());
-    EXPECT_EQ(kStateStringStarted,
-              controller()->GetEngineInitializationStateString());
+    EXPECT_EQ(StartupController::State::STARTED, controller()->GetState());
   }
 
   void ExpectStartDeferred() {
-    const bool deferred_start =
-        !base::CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kSyncDisableDeferredStartup);
-    EXPECT_EQ(!deferred_start, started());
-    EXPECT_EQ(deferred_start ? kStateStringDeferred : kStateStringStarted,
-              controller()->GetEngineInitializationStateString());
+    EXPECT_FALSE(started());
+    EXPECT_EQ(StartupController::State::STARTING_DEFERRED,
+              controller()->GetState());
   }
 
   void ExpectNotStarted() {
     EXPECT_FALSE(started());
-    EXPECT_EQ(kStateStringNotStarted,
-              controller()->GetEngineInitializationStateString());
+    EXPECT_EQ(StartupController::State::NOT_STARTED, controller()->GetState());
   }
 
   bool started() const { return started_; }
@@ -72,11 +59,7 @@ class StartupControllerTest : public testing::Test {
 
  private:
   bool CanStart() { return can_start_; }
-
-  void FakeStartBackend() {
-    started_ = true;
-    sync_prefs()->SetFirstSetupComplete();
-  }
+  void FakeStartBackend() { started_ = true; }
 
   bool can_start_;
   bool started_;