ProfileSyncServiceHarness: Replace IsSyncDisabled by IsSyncEnabledByUser

"IsSyncDisabled" described neither what the method was trying to do, nor
what it actually did. This CL flips it to "IsSyncEnabledByUser" (yay
less negation), and also fixes its implementation to actually check
that.

This was exposed as an actual problem by https://crrev.com/c/1148392.

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

chrome/browser/sync/test/integration/profile_sync_service_harness.cc

@@ -404,8 +404,10 @@ std::string ProfileSyncServiceHarness::GenerateFakeOAuth2RefreshTokenString() {
                             ++oauth2_refesh_token_number_);
 }
 
-bool ProfileSyncServiceHarness::IsSyncDisabled() const {
-  return !service()->IsSetupInProgress() && !service()->IsFirstSetupComplete();
+bool ProfileSyncServiceHarness::IsSyncEnabledByUser() const {
+  return service()->IsFirstSetupComplete() &&
+         !service()->HasDisableReason(
+             ProfileSyncService::DISABLE_REASON_USER_CHOICE);
 }
 
 void ProfileSyncServiceHarness::FinishSyncSetup() {
@@ -427,8 +429,12 @@ bool ProfileSyncServiceHarness::EnableSyncForDatatype(
       "EnableSyncForDatatype("
       + std::string(syncer::ModelTypeToString(datatype)) + ")");
 
-  if (IsSyncDisabled())
-    return SetupSync(syncer::ModelTypeSet(datatype));
+  if (!IsSyncEnabledByUser()) {
+    bool result = SetupSync(syncer::ModelTypeSet(datatype));
+    // If SetupSync() succeeded, then Sync must now be enabled.
+    DCHECK(!result || IsSyncEnabledByUser());
+    return result;
+  }
 
   if (service() == nullptr) {
     LOG(ERROR) << "EnableSyncForDatatype(): service() is null.";
@@ -505,8 +511,12 @@ bool ProfileSyncServiceHarness::DisableSyncForDatatype(
 bool ProfileSyncServiceHarness::EnableSyncForAllDatatypes() {
   DVLOG(1) << GetClientInfoString("EnableSyncForAllDatatypes");
 
-  if (IsSyncDisabled())
-    return SetupSync();
+  if (!IsSyncEnabledByUser()) {
+    bool result = SetupSync();
+    // If SetupSync() succeeded, then Sync must now be enabled.
+    DCHECK(!result || IsSyncEnabledByUser());
+    return result;
+  }
 
   if (service() == nullptr) {
     LOG(ERROR) << "EnableSyncForAllDatatypes(): service() is null.";

chrome/browser/sync/test/integration/profile_sync_service_harness.h

@@ -167,8 +167,9 @@ class ProfileSyncServiceHarness {
   // Gets detailed status from |service_| in pretty-printable form.
   std::string GetServiceStatus();
 
-  // Returns true if sync is disabled for this client.
-  bool IsSyncDisabled() const;
+  // Returns true if the user has enabled and configured sync for this client.
+  // Note that this does not imply sync is actually running.
+  bool IsSyncEnabledByUser() const;
 
   // Sync profile associated with this sync client.
   Profile* profile_;