[Sync] Simplify sync startup behavior.

components/browser_sync/browser/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 <utility>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 74 matching lines @@
   return new FakeDataTypeManager(configure_called);
 }
 
 using testing::Return;
 using testing::StrictMock;
 using testing::_;
 
 class TestSyncServiceObserver : public sync_driver::SyncServiceObserver {
  public:
   explicit TestSyncServiceObserver(ProfileSyncService* service)
-      : service_(service), first_setup_in_progress_(false) {}
+      : service_(service), setup_in_progress_(false) {}
   void OnStateChanged() override {
-    first_setup_in_progress_ = service_->IsFirstSetupInProgress();
+    setup_in_progress_ = service_->IsSetupInProgress();
   }
-  bool first_setup_in_progress() const { return first_setup_in_progress_; }
+  bool setup_in_progress() const { return setup_in_progress_; }
  private:
   ProfileSyncService* service_;
-  bool first_setup_in_progress_;
+  bool setup_in_progress_;
 };
 
 // A variant of the SyncBackendHostMock that won't automatically
 // call back when asked to initialized.  Allows us to test things
 // that could happen while backend init is in progress.
 class SyncBackendHostNoReturn : public SyncBackendHostMock {
   void Initialize(
       sync_driver::SyncFrontend* frontend,
       scoped_ptr<base::Thread> sync_thread,
       const scoped_refptr<base::SingleThreadTaskRunner>& db_thread,
@@ skipping 145 matching lines @@
 #endif
 
   void ShutdownAndDeleteService() {
     if (service_)
       service_->Shutdown();
     service_.reset();
   }
 
   void InitializeForNthSync() {
     // Set first sync time before initialize to simulate a complete sync setup.
-    sync_driver::SyncPrefs sync_prefs(
-        service_->GetSyncClient()->GetPrefService());
+    sync_driver::SyncPrefs sync_prefs(prefs());
     sync_prefs.SetFirstSyncTime(base::Time::Now());
     sync_prefs.SetFirstSetupComplete();
     sync_prefs.SetKeepEverythingSynced(true);
     service_->Initialize();
   }
 
   void InitializeForFirstSync() {
     service_->Initialize();
   }
 
@@ skipping 136 matching lines @@
 // Verify that the SetSetupInProgress function call updates state
 // and notifies observers.
 TEST_F(ProfileSyncServiceTest, SetupInProgress) {
   CreateService(browser_sync::AUTO_START);
   InitializeForFirstSync();
 
   TestSyncServiceObserver observer(service());
   service()->AddObserver(&observer);
 
   service()->SetSetupInProgress(true);
-  EXPECT_TRUE(observer.first_setup_in_progress());
+  EXPECT_TRUE(observer.setup_in_progress());
   service()->SetSetupInProgress(false);
-  EXPECT_FALSE(observer.first_setup_in_progress());
+  EXPECT_FALSE(observer.setup_in_progress());
 
   service()->RemoveObserver(&observer);
 }
 
 // Verify that disable by enterprise policy works.
 TEST_F(ProfileSyncServiceTest, DisabledByPolicyBeforeInit) {
   prefs()->SetManagedPref(sync_driver::prefs::kSyncManaged,
                           new base::FundamentalValue(true));
   IssueTestTokens();
   CreateService(browser_sync::AUTO_START);
@@ skipping 31 matching lines @@
   InitializeForNthSync();
   EXPECT_FALSE(service()->IsSyncActive());
 
   ShutdownAndDeleteService();
 }
 
 // Test RequestStop() before we've initialized the backend.
 TEST_F(ProfileSyncServiceTest, EarlyRequestStop) {
   CreateService(browser_sync::AUTO_START);
   IssueTestTokens();
+  ExpectDataTypeManagerCreation(1, GetDefaultConfigureCalledCallback());
+  ExpectSyncBackendHostCreation(1);
 
   service()->RequestStop(ProfileSyncService::KEEP_DATA);
-  EXPECT_TRUE(prefs()->GetBoolean(sync_driver::prefs::kSyncSuppressStart));
+  EXPECT_FALSE(service()->IsSyncRequested());
 
-  // Because of suppression, this should fail.
-  sync_driver::SyncPrefs sync_prefs(
-      service()->GetSyncClient()->GetPrefService());
-  sync_prefs.SetFirstSyncTime(base::Time::Now());
-  service()->Initialize();
+  // Because sync is not requested, this should fail.
+  InitializeForNthSync();
   EXPECT_FALSE(service()->IsSyncActive());

[Nicolas Zea, 2016/02/29 21:21:55]

Verify IsSyncRequested?

[maxbogue, 2016/03/09 02:00:59]

Done.

 
-  // Request start.  This should be enough to allow init to happen.
-  ExpectDataTypeManagerCreation(1, GetDefaultConfigureCalledCallback());
-  ExpectSyncBackendHostCreation(1);
+  // Request start. This should be enough to allow init to happen.
   service()->RequestStart();
+  EXPECT_TRUE(service()->IsSyncRequested());
   EXPECT_TRUE(service()->IsSyncActive());
-  EXPECT_FALSE(prefs()->GetBoolean(sync_driver::prefs::kSyncSuppressStart));
 }
 
 // Test RequestStop() after we've initialized the backend.
 TEST_F(ProfileSyncServiceTest, DisableAndEnableSyncTemporarily) {
   CreateService(browser_sync::AUTO_START);
   IssueTestTokens();
   ExpectDataTypeManagerCreation(1, GetDefaultConfigureCalledCallback());
   ExpectSyncBackendHostCreation(1);
   InitializeForNthSync();
 
@@ skipping 401 matching lines @@
 // dereferenced in OnSyncCycleCompleted. The fix is to use find() to check if
 // entry for sessions exists in map.
 TEST_F(ProfileSyncServiceTest, ValidPointersInDTCMap) {
   CreateService(browser_sync::AUTO_START);
   service()->OnSessionRestoreComplete();
   service()->OnSyncCycleCompleted();
 }
 
 }  // namespace
 }  // namespace browser_sync