Partial revert "Make IsSyncRequested false by default"

This reverts commit 8c872e02bbc75b817e036ed714ae652e3f638fa8.

Reason for revert:
https://bugs.chromium.org/p/chromium/issues/detail?id=973770

Note: This CL does not revert the changes the the test files as
instructed by treib@ who is the author of the original CL.


Original change's description:
> Make IsSyncRequested false by default
>
> Before this CL, the value of IsSyncRequested was essentially undefined
> while Sync was disabled: It was true by default, but false if the user
> had previously enabled Sync and then disabled it again.
> This CL resolves the inconsistency by making it false by default.
>
> Bug: 906034
> Change-Id: I2ce92f5df5ebb38a34ebde43d471839ad1184961
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1508453
> Reviewed-by: Jan Krcal <jkrcal@chromium.org>
> Commit-Queue: Marc Treib <treib@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#642447}

TBR=treib@chromium.org,jkrcal@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 906034, 973770
Change-Id: I501f563cae8ff63de21cd66d933281e0af9542ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1658571
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Kariah Davis <kariahda@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#307}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}

diff --git a/chrome/browser/sync/test/integration/single_client_standalone_transport_sync_test.cc b/chrome/browser/sync/test/integration/single_client_standalone_transport_sync_test.cc
index 53524bb..860657af 100644
--- a/chrome/browser/sync/test/integration/single_client_standalone_transport_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_standalone_transport_sync_test.cc

@@ -97,7 +97,9 @@
   // either by the Sync confirmation dialog or by the settings page if going
   // through the advanced settings flow.
   EXPECT_FALSE(GetSyncService(0)->GetUserSettings()->IsFirstSetupComplete());
-  EXPECT_FALSE(GetSyncService(0)->GetUserSettings()->IsSyncRequested());
+  // TODO(crbug.com/906034,crbug.com/973770): Sort out the proper default value
+  // for IsSyncRequested().
+  // EXPECT_FALSE(GetSyncService(0)->GetUserSettings()->IsSyncRequested());
 
   EXPECT_FALSE(GetSyncService(0)->IsSyncFeatureEnabled());
   EXPECT_FALSE(GetSyncService(0)->IsSyncFeatureActive());

diff --git a/chrome/browser/ui/webui/sync_internals_browsertest.js b/chrome/browser/ui/webui/sync_internals_browsertest.js
index ede5673..9a179ff 100644
--- a/chrome/browser/ui/webui/sync_internals_browsertest.js
+++ b/chrome/browser/ui/webui/sync_internals_browsertest.js

@@ -260,8 +260,9 @@
 // set.
 TEST_F('SyncInternalsWebUITest', 'SyncDisabledByDefault', function() {
   expectTrue(this.hasInDetails(true, 'Transport State', 'Disabled'));
-  expectTrue(
-      this.hasInDetails(true, 'Disable Reasons', 'Not signed in, User choice'));
+  // TODO(crbug.com/906034,crbug.com/973770): Sort out the proper default value
+  // for IsSyncRequested() and possibly add the "User choice" disable reason.
+  expectTrue(this.hasInDetails(true, 'Disable Reasons', 'Not signed in'));
   expectTrue(this.hasInDetails(true, 'Username', ''));
 });
 

diff --git a/components/sync/base/sync_prefs.cc b/components/sync/base/sync_prefs.cc
index e0233e2..db4744b 100644
--- a/components/sync/base/sync_prefs.cc
+++ b/components/sync/base/sync_prefs.cc

@@ -148,7 +148,7 @@
     user_prefs::PrefRegistrySyncable* registry) {
   // Actual user-controlled preferences.
   registry->RegisterBooleanPref(prefs::kSyncFirstSetupComplete, false);
-  registry->RegisterBooleanPref(prefs::kSyncSuppressStart, true);
+  registry->RegisterBooleanPref(prefs::kSyncSuppressStart, false);
   registry->RegisterBooleanPref(prefs::kSyncKeepEverythingSynced, true);
   for (UserSelectableType type : UserSelectableTypeSet::All()) {
     RegisterTypeSelectedPref(registry, type);

diff --git a/components/sync/base/sync_prefs_unittest.cc b/components/sync/base/sync_prefs_unittest.cc
index 737ec95..529c387 100644
--- a/components/sync/base/sync_prefs_unittest.cc
+++ b/components/sync/base/sync_prefs_unittest.cc

@@ -77,12 +77,12 @@
   EXPECT_CALL(mock_sync_pref_observer, OnSyncManagedPrefChange(false));
   EXPECT_CALL(mock_sync_pref_observer, OnFirstSetupCompletePrefChange(true));
   EXPECT_CALL(mock_sync_pref_observer, OnFirstSetupCompletePrefChange(false));
-  EXPECT_CALL(mock_sync_pref_observer, OnSyncRequestedPrefChange(true));
   EXPECT_CALL(mock_sync_pref_observer, OnSyncRequestedPrefChange(false));
+  EXPECT_CALL(mock_sync_pref_observer, OnSyncRequestedPrefChange(true));
 
   ASSERT_FALSE(sync_prefs_->IsManaged());
   ASSERT_FALSE(sync_prefs_->IsFirstSetupComplete());
-  ASSERT_FALSE(sync_prefs_->IsSyncRequested());
+  ASSERT_TRUE(sync_prefs_->IsSyncRequested());
 
   sync_prefs_->AddSyncPrefObserver(&mock_sync_pref_observer);
 
@@ -98,10 +98,10 @@
   sync_prefs_->ClearPreferences();
   EXPECT_FALSE(sync_prefs_->IsFirstSetupComplete());
 
-  sync_prefs_->SetSyncRequested(true);
-  EXPECT_TRUE(sync_prefs_->IsSyncRequested());
   sync_prefs_->SetSyncRequested(false);
   EXPECT_FALSE(sync_prefs_->IsSyncRequested());
+  sync_prefs_->SetSyncRequested(true);
+  EXPECT_TRUE(sync_prefs_->IsSyncRequested());
 
   sync_prefs_->RemoveSyncPrefObserver(&mock_sync_pref_observer);
 }
@@ -135,11 +135,11 @@
   sync_prefs_->SetFirstSetupComplete();
   EXPECT_TRUE(sync_prefs_->IsFirstSetupComplete());
 
-  EXPECT_FALSE(sync_prefs_->IsSyncRequested());
-  sync_prefs_->SetSyncRequested(true);
   EXPECT_TRUE(sync_prefs_->IsSyncRequested());
   sync_prefs_->SetSyncRequested(false);
   EXPECT_FALSE(sync_prefs_->IsSyncRequested());
+  sync_prefs_->SetSyncRequested(true);
+  EXPECT_TRUE(sync_prefs_->IsSyncRequested());
 
   EXPECT_EQ(base::Time(), sync_prefs_->GetLastSyncedTime());
   const base::Time& now = base::Time::Now();

diff --git a/components/sync/driver/profile_sync_service_startup_unittest.cc b/components/sync/driver/profile_sync_service_startup_unittest.cc
index a5ca326..86c6928 100644
--- a/components/sync/driver/profile_sync_service_startup_unittest.cc
+++ b/components/sync/driver/profile_sync_service_startup_unittest.cc

@@ -165,8 +165,7 @@
   // Should not actually start, rather just clean things up and wait
   // to be enabled.
   sync_service()->Initialize();
-  EXPECT_EQ(SyncService::DISABLE_REASON_NOT_SIGNED_IN |
-                SyncService::DISABLE_REASON_USER_CHOICE,
+  EXPECT_EQ(SyncService::DISABLE_REASON_NOT_SIGNED_IN,
             sync_service()->GetDisableReasons());
   EXPECT_EQ(SyncService::TransportState::DISABLED,
             sync_service()->GetTransportState());
@@ -604,8 +603,7 @@
 
   // There is no signed-in user, so also nobody has decided that Sync should be
   // started.
-  EXPECT_EQ(SyncService::DISABLE_REASON_NOT_SIGNED_IN |
-                SyncService::DISABLE_REASON_USER_CHOICE,
+  EXPECT_EQ(SyncService::DISABLE_REASON_NOT_SIGNED_IN,
             sync_service()->GetDisableReasons());
   EXPECT_EQ(SyncService::TransportState::DISABLED,
             sync_service()->GetTransportState());
@@ -613,7 +611,7 @@
   // Sign in. Now Sync-the-transport could start, but gets deferred by default.
   // Sync-the-feature still doesn't start until the user says they want it.
   SimulateTestUserSignin();
-  EXPECT_EQ(SyncService::DISABLE_REASON_USER_CHOICE,
+  EXPECT_EQ(SyncService::DISABLE_REASON_NONE,
             sync_service()->GetDisableReasons());
   EXPECT_EQ(SyncService::TransportState::START_DEFERRED,
             sync_service()->GetTransportState());