Add tests for the full ProfileSyncService startup sequence

This adds tests FullStartupSequenceFirstTime and
FullStartupSequenceNthTime, based on the new SyncService::State enum.
To be able to properly test the full flow, this also introduces a
MockSyncEngine (previous tests use a Fake, which bypasses some steps).

Bug: 839834
Change-Id: I145becf840f476d73815526b8abd43a9bfa159b8
Reviewed-on: https://chromium-review.googlesource.com/1118383
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572170}

components/browser_sync/profile_sync_service_startup_unittest.cc

@@ -12,6 +12,7 @@
 #include "components/sync/driver/fake_data_type_controller.h"
 #include "components/sync/driver/sync_api_component_factory_mock.h"
 #include "components/sync/engine/fake_sync_engine.h"
+#include "components/sync/engine/mock_sync_engine.h"
 #include "services/identity/public/cpp/identity_test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -19,9 +20,11 @@
 using syncer::DataTypeManager;
 using syncer::DataTypeManagerMock;
 using syncer::FakeSyncEngine;
+using syncer::MockSyncEngine;
 using testing::_;
 using testing::ByMove;
 using testing::DoAll;
+using testing::InvokeWithoutArgs;
 using testing::Mock;
 using testing::NiceMock;
 using testing::Return;
@@ -56,7 +59,9 @@ ACTION_P3(InvokeOnConfigureDone, sync_service, error_callback, result) {
 
 class ProfileSyncServiceStartupTest : public testing::Test {
  public:
-  ProfileSyncServiceStartupTest() {
+  ProfileSyncServiceStartupTest()
+      : scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME) {
     profile_sync_service_bundle_.auth_service()
         ->set_auto_post_fetch_response_on_message_loop(true);
   }
@@ -65,7 +70,9 @@ class ProfileSyncServiceStartupTest : public testing::Test {
     sync_service_->Shutdown();
   }
 
-  void CreateSyncService(ProfileSyncService::StartBehavior start_behavior) {
+  void CreateSyncService(ProfileSyncService::StartBehavior start_behavior,
+                         syncer::ModelTypeSet registered_types =
+                             syncer::ModelTypeSet(syncer::BOOKMARKS)) {
     ProfileSyncServiceBundle::SyncClientBuilder builder(
         &profile_sync_service_bundle_);
     ProfileSyncService::InitParams init_params =
@@ -73,11 +80,13 @@ class ProfileSyncServiceStartupTest : public testing::Test {
                                                            builder.Build());
 
     ON_CALL(*component_factory(), CreateCommonDataTypeControllers(_, _))
-        .WillByDefault(testing::InvokeWithoutArgs([=]() {
+        .WillByDefault(InvokeWithoutArgs([=]() {
           syncer::DataTypeController::TypeVector controllers;
-          controllers.push_back(
-              std::make_unique<syncer::FakeDataTypeController>(
-                  syncer::BOOKMARKS));
+          for (syncer::ModelTypeSet::Iterator it = registered_types.First();
+               it.Good(); it.Inc()) {
+            controllers.push_back(
+                std::make_unique<syncer::FakeDataTypeController>(it.Get()));
+          }
           return controllers;
         }));
 
@@ -121,6 +130,14 @@ class ProfileSyncServiceStartupTest : public testing::Test {
     return sync_engine_raw;
   }
 
+  MockSyncEngine* SetUpMockSyncEngine() {
+    auto sync_engine = std::make_unique<NiceMock<MockSyncEngine>>();
+    MockSyncEngine* sync_engine_raw = sync_engine.get();
+    ON_CALL(*component_factory(), CreateSyncEngine(_, _, _, _))
+        .WillByDefault(Return(ByMove(std::move(sync_engine))));
+    return sync_engine_raw;
+  }
+
   ProfileSyncService* sync_service() { return sync_service_.get(); }
 
   PrefService* pref_service() {
@@ -131,6 +148,10 @@ class ProfileSyncServiceStartupTest : public testing::Test {
     return profile_sync_service_bundle_.component_factory();
   }
 
+  void FastForwardUntilNoTasksRemain() {
+    scoped_task_environment_.FastForwardUntilNoTasksRemain();
+  }
+
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   ProfileSyncServiceBundle profile_sync_service_bundle_;
@@ -538,4 +559,138 @@ TEST_F(ProfileSyncServiceStartupTest, StartDownloadFailed) {
   EXPECT_EQ(syncer::SyncService::State::DISABLED, sync_service()->GetState());
 }
 
+// ChromeOS does not support sign-in after startup (in particular,
+// IdentityManager::Observer::OnPrimaryAccountSet never gets called).
+#if !defined(OS_CHROMEOS)
+TEST_F(ProfileSyncServiceStartupTest, FullStartupSequenceFirstTime) {
+  // We've never completed startup.
+  pref_service()->ClearPref(syncer::prefs::kSyncFirstSetupComplete);
+
+  MockSyncEngine* sync_engine = SetUpMockSyncEngine();
+  EXPECT_CALL(*sync_engine, Initialize(_)).Times(0);
+
+  DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
+  EXPECT_CALL(*data_type_manager, Configure(_, _)).Times(0);
+  ON_CALL(*data_type_manager, state())
+      .WillByDefault(Return(DataTypeManager::STOPPED));
+
+  // Note: Deferred startup is only enabled if SESSIONS is among the preferred
+  // data types.
+  CreateSyncService(ProfileSyncService::MANUAL_START,
+                    syncer::ModelTypeSet(syncer::SESSIONS));
+  sync_service()->Initialize();
+
+  // There is no signed-in user, but nothing else prevents Sync from starting.
+  EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN,
+            sync_service()->GetDisableReasons());
+  EXPECT_EQ(syncer::SyncService::State::DISABLED, sync_service()->GetState());
+
+  // Sign in. Now Sync is ready to start, just waiting for a prod.
+  SimulateTestUserSignin();
+  EXPECT_EQ(syncer::SyncService::State::WAITING_FOR_START_REQUEST,
+            sync_service()->GetState());
+
+  // Once we give the service a prod by initiating Sync setup, it'll start and
+  // initialize the engine. Since this is the initial Sync start, this will not
+  // be deferred.
+  EXPECT_CALL(*sync_engine, Initialize(_));
+  auto setup_in_progress_handle = sync_service()->GetSetupInProgressHandle();
+  EXPECT_EQ(syncer::SyncService::State::INITIALIZING,
+            sync_service()->GetState());
+
+  // Once the engine calls back and says it's initialized, we're just waiting
+  // for the user to finish the initial configuration (choosing data types etc.)
+  // before actually syncing data.
+  sync_service()->OnEngineInitialized(
+      syncer::ModelTypeSet(), syncer::WeakHandle<syncer::JsBackend>(),
+      syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), "test-guid",
+      /*success=*/true);
+  EXPECT_EQ(syncer::SyncService::State::WAITING_FOR_CONSENT,
+            sync_service()->GetState());
+
+  // Once the user finishes the initial setup, the service can actually start
+  // configuring the data types. It won't actually call the DataTypeManager yet
+  // though, because setup is still considered in progress (we haven't released
+  // the setup-in-progress handle).
+  sync_service()->SetFirstSetupComplete();
+  EXPECT_EQ(syncer::SyncService::State::CONFIGURING,
+            sync_service()->GetState());
+
+  // Releasing the setup in progress handle lets the service actually configure
+  // the DataTypeManager.
+  EXPECT_CALL(*data_type_manager, Configure(_, _))
+      .WillOnce(InvokeWithoutArgs(sync_service(),
+                                  &ProfileSyncService::OnConfigureStart));
+  ON_CALL(*data_type_manager, state())
+      .WillByDefault(Return(DataTypeManager::CONFIGURING));
+  setup_in_progress_handle.reset();
+  // While DataTypeManager configuration is ongoing, the overall state is still
+  // CONFIGURING.
+  EXPECT_EQ(syncer::SyncService::State::CONFIGURING,
+            sync_service()->GetState());
+
+  // Finally, once the DataTypeManager says it's done with configuration, Sync
+  // is actually fully up and running.
+  DataTypeManager::ConfigureResult configure_result(
+      DataTypeManager::OK, syncer::ModelTypeSet(syncer::SESSIONS));
+  ON_CALL(*data_type_manager, state())
+      .WillByDefault(Return(DataTypeManager::CONFIGURED));
+  sync_service()->OnConfigureDone(configure_result);
+  EXPECT_EQ(syncer::SyncService::State::ACTIVE, sync_service()->GetState());
+}
+#endif  // OS_CHROMEOS
+
+TEST_F(ProfileSyncServiceStartupTest, FullStartupSequenceNthTime) {
+  // The user is already signed in and has completed Sync setup before.
+  SimulateTestUserSignin();
+  pref_service()->SetBoolean(syncer::prefs::kSyncFirstSetupComplete, true);
+
+  MockSyncEngine* sync_engine = SetUpMockSyncEngine();
+  EXPECT_CALL(*sync_engine, Initialize(_)).Times(0);
+
+  DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
+  EXPECT_CALL(*data_type_manager, Configure(_, _)).Times(0);
+  ON_CALL(*data_type_manager, state())
+      .WillByDefault(Return(DataTypeManager::STOPPED));
+
+  // Note: Deferred startup is only enabled if SESSIONS is among the preferred
+  // data types.
+  CreateSyncService(ProfileSyncService::MANUAL_START,
+                    syncer::ModelTypeSet(syncer::SESSIONS));
+  sync_service()->Initialize();
+
+  // Nothing is preventing Sync from starting, but it should be deferred so as
+  // to now slow down browser startup.
+  EXPECT_EQ(syncer::SyncService::State::START_DEFERRED,
+            sync_service()->GetState());
+
+  // Wait for the deferred startup timer to expire. The Sync service will start
+  // and initialize the engine.
+  EXPECT_CALL(*sync_engine, Initialize(_));
+  FastForwardUntilNoTasksRemain();
+  EXPECT_EQ(syncer::SyncService::State::INITIALIZING,
+            sync_service()->GetState());
+
+  // Once the engine calls back and says it's initialized, the DataTypeManager
+  // will get configured, since initial setup is already done.
+  EXPECT_CALL(*data_type_manager, Configure(_, _));
+  sync_service()->OnEngineInitialized(
+      syncer::ModelTypeSet(), syncer::WeakHandle<syncer::JsBackend>(),
+      syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), "test-guid",
+      /*success=*/true);
+  ON_CALL(*data_type_manager, state())
+      .WillByDefault(Return(DataTypeManager::CONFIGURING));
+  EXPECT_EQ(syncer::SyncService::State::CONFIGURING,
+            sync_service()->GetState());
+
+  // Finally, once the DataTypeManager says it's done with configuration, Sync
+  // is actually fully up and running.
+  DataTypeManager::ConfigureResult configure_result(
+      DataTypeManager::OK, syncer::ModelTypeSet(syncer::SESSIONS));
+  ON_CALL(*data_type_manager, state())
+      .WillByDefault(Return(DataTypeManager::CONFIGURED));
+  sync_service()->OnConfigureDone(configure_result);
+  EXPECT_EQ(syncer::SyncService::State::ACTIVE, sync_service()->GetState());
+}
+
 }  // namespace browser_sync

components/sync/BUILD.gn

@@ -782,6 +782,8 @@ static_library("test_support_driver") {
     "driver/sync_client_mock.h",
     "engine/fake_sync_engine.cc",
     "engine/fake_sync_engine.h",
+    "engine/mock_sync_engine.cc",
+    "engine/mock_sync_engine.h",
     "model/change_processor_mock.cc",
     "model/change_processor_mock.h",
   ]

components/sync/engine/mock_sync_engine.cc

+// Copyright 2018 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 "components/sync/engine/mock_sync_engine.h"
+
+namespace syncer {
+
+MockSyncEngine::MockSyncEngine() {}
+
+MockSyncEngine::~MockSyncEngine() {}
+
+}  // namespace syncer

components/sync/engine/mock_sync_engine.h

+// Copyright 2018 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.
+
+#ifndef COMPONENTS_SYNC_ENGINE_MOCK_SYNC_ENGINE_H_
+#define COMPONENTS_SYNC_ENGINE_MOCK_SYNC_ENGINE_H_
+
+#include <memory>
+#include <string>
+
+#include "components/sync/engine/data_type_activation_response.h"
+#include "components/sync/engine/sync_engine.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace syncer {
+
+// A mock of the SyncEngine.
+//
+// Note: If you don't really care about all the exact details, FakeSyncEngine is
+// probably better.
+class MockSyncEngine : public SyncEngine {
+ public:
+  MockSyncEngine();
+  ~MockSyncEngine() override;
+
+  // ModelTypeConfigurer:
+  MOCK_METHOD1(ConfigureDataTypes, void(ConfigureParams));
+  MOCK_METHOD2(RegisterDirectoryDataType, void(ModelType, ModelSafeGroup));
+  MOCK_METHOD1(UnregisterDirectoryDataType, void(ModelType));
+  MOCK_METHOD3(ActivateDirectoryDataType,
+               void(ModelType, ModelSafeGroup, ChangeProcessor*));
+  MOCK_METHOD1(DeactivateDirectoryDataType, void(ModelType));
+  MOCK_METHOD2(ActivateNonBlockingDataType,
+               void(ModelType, std::unique_ptr<DataTypeActivationResponse>));
+  MOCK_METHOD1(DeactivateNonBlockingDataType, void(ModelType));
+
+  // SyncEngine:
+  MOCK_METHOD1(Initialize, void(InitParams));
+  MOCK_METHOD1(TriggerRefresh, void(const ModelTypeSet&));
+  MOCK_METHOD1(UpdateCredentials, void(const SyncCredentials&));
+  MOCK_METHOD0(InvalidateCredentials, void());
+  MOCK_METHOD0(StartConfiguration, void());
+  MOCK_METHOD0(StartSyncingWithServer, void());
+  MOCK_METHOD2(SetEncryptionPassphrase, void(const std::string&, bool));
+  MOCK_METHOD1(SetDecryptionPassphrase, void(const std::string&));
+  MOCK_METHOD0(StopSyncingForShutdown, void());
+  MOCK_METHOD1(Shutdown, void(ShutdownReason));
+  MOCK_METHOD0(EnableEncryptEverything, void());
+  MOCK_CONST_METHOD0(GetUserShare, UserShare*());
+  MOCK_METHOD0(GetDetailedStatus, Status());
+  MOCK_CONST_METHOD1(HasUnsyncedItemsForTest,
+                     void(base::OnceCallback<void(bool)>));
+  MOCK_CONST_METHOD1(IsCryptographerReady, bool(const BaseTransaction*));
+  MOCK_CONST_METHOD1(GetModelSafeRoutingInfo, void(ModelSafeRoutingInfo*));
+  MOCK_CONST_METHOD0(FlushDirectory, void());
+  MOCK_METHOD0(RequestBufferedProtocolEventsAndEnableForwarding, void());
+  MOCK_METHOD0(DisableProtocolEventForwarding, void());
+  MOCK_METHOD0(EnableDirectoryTypeDebugInfoForwarding, void());
+  MOCK_METHOD0(DisableDirectoryTypeDebugInfoForwarding, void());
+  MOCK_METHOD1(ClearServerData, void(const base::Closure&));
+  MOCK_METHOD3(OnCookieJarChanged, void(bool, bool, const base::Closure&));
+};
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_ENGINE_MOCK_SYNC_ENGINE_H_