Allow roaming profiles to work even if sync is disabled by policy.

Bug: 1146509
Change-Id: I67d27099b546bc913807f516ec807f36bd74f9a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523491
Commit-Queue: Chris Sharp <csharp@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826299}

chrome/browser/sync/profile_sync_service_factory.cc

@@ -288,7 +288,8 @@ bool ProfileSyncServiceFactory::IsSyncAllowed(Profile* profile) {
   // No ProfileSyncService created yet - we don't want to create one, so just
   // infer the accessible state by looking at prefs/command line flags.
   syncer::SyncPrefs prefs(profile->GetPrefs());
-  return switches::IsSyncAllowedByFlag() && !prefs.IsManaged();
+  return switches::IsSyncAllowedByFlag() &&
+         (!prefs.IsManaged() || prefs.IsLocalSyncEnabled());
 }
 
 // static

components/policy/resources/policy_templates.json

@@ -3456,9 +3456,7 @@
 
       If this policy is left not set Google Sync will be available for the user to choose whether to use it or not.
 
-      To fully disable Google Sync, it is recommended that you disable the Google Sync service in the Google Admin console.
-
-      This policy should not be enabled when <ph name="ROAMING_PROFILE_SUPPORT_ENABLED_POLICY_NAME">RoamingProfileSupportEnabled</ph> policy is set to enabled as that feature shares the same client side functionality. The Google-hosted synchronization is disabled in this case completely.''',
+      To fully disable Google Sync, it is recommended that you disable the Google Sync service in the Google Admin console.''',
       'arc_support': 'Disabling Google Sync will cause Android Backup and Restore to not function properly.',
     },
     {
@@ -3498,9 +3496,7 @@
       'tags': ['local-data-access'],
       'desc': '''If you enable this setting, the settings stored in <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> profiles like bookmarks, autofill data, passwords, etc. will also be written to a file stored in the Roaming user profile folder or a location specified by the Administrator through the <ph name="ROAMING_PROFILE_LOCATION_POLICY_NAME">RoamingProfileLocation</ph> policy. Enabling this policy disables cloud sync.
 
-      If this policy is disabled or left not set only the regular local profiles will be used.
-
-      The <ph name="SYNC_DISABLED_POLICY_NAME">SyncDisabled</ph> policy disables all data synchronization, overriding <ph name="ROAMING_PROFILE_SUPPORT_ENABLED_POLICY_NAME">RoamingProfileSupportEnabled</ph>.''',
+      If this policy is disabled or left not set only the regular local profiles will be used.''',
       'label': '''Enable the creation of roaming copies for <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> profile data.''',
     },
     {

components/sync/driver/profile_sync_service.cc

@@ -727,18 +727,20 @@ SyncService::DisableReasonSet ProfileSyncService::GetDisableReasons() const {
   if (!user_settings_->IsSyncAllowedByPlatform()) {
     result.Put(DISABLE_REASON_PLATFORM_OVERRIDE);
   }
-  if (sync_prefs_.IsManaged() || sync_disabled_by_admin_) {
-    result.Put(DISABLE_REASON_ENTERPRISE_POLICY);
-  }
-  // Local sync doesn't require sign-in.
-  if (!IsSignedIn() && !IsLocalSyncEnabled()) {
-    result.Put(DISABLE_REASON_NOT_SIGNED_IN);
-  }
-  // When local sync is on sync should be considered requsted or otherwise it
-  // will not resume after the policy or the flag has been removed.
-  if (!user_settings_->IsSyncRequested() && !IsLocalSyncEnabled()) {
-    result.Put(DISABLE_REASON_USER_CHOICE);
+
+  // If local sync is enabled, most disable reasons don't apply.
+  if (!IsLocalSyncEnabled()) {
+    if (sync_prefs_.IsManaged() || sync_disabled_by_admin_) {
+      result.Put(DISABLE_REASON_ENTERPRISE_POLICY);
+    }
+    if (!IsSignedIn()) {
+      result.Put(DISABLE_REASON_NOT_SIGNED_IN);
+    }
+    if (!user_settings_->IsSyncRequested()) {
+      result.Put(DISABLE_REASON_USER_CHOICE);
+    }
   }
+
   if (unrecoverable_error_reason_) {
     result.Put(DISABLE_REASON_UNRECOVERABLE_ERROR);
   }
@@ -1495,6 +1497,13 @@ void ProfileSyncService::GetEntityCountsForDebugging(
 
 void ProfileSyncService::OnSyncManagedPrefChange(bool is_sync_managed) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // Local sync is not controlled by the "sync managed" policy, so these pref
+  // changes make no difference to the service state.
+  if (IsLocalSyncEnabled()) {
+    return;
+  }
+
   if (is_sync_managed) {
     StopImpl(CLEAR_DATA);
   } else {

components/sync/driver/profile_sync_service_unittest.cc

@@ -1281,8 +1281,8 @@ TEST_F(ProfileSyncServiceTest, DisableSyncOnClient) {
   EXPECT_FALSE(service()->IsSyncFeatureActive());
 }
 
-// Verify a that local sync mode resumes after the policy is lifted.
-TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) {
+// Verify a that local sync mode isn't impacted by sync being disabled.
+TEST_F(ProfileSyncServiceTest, LocalBackendUnimpactedByPolicy) {
   prefs()->SetManagedPref(prefs::kSyncManaged,
                           std::make_unique<base::Value>(false));
   CreateServiceWithLocalSyncBackend();
@@ -1294,10 +1294,8 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) {
   prefs()->SetManagedPref(prefs::kSyncManaged,
                           std::make_unique<base::Value>(true));
 
-  EXPECT_EQ(SyncService::DisableReasonSet(
-                SyncService::DISABLE_REASON_ENTERPRISE_POLICY),
-            service()->GetDisableReasons());
-  EXPECT_EQ(SyncService::TransportState::DISABLED,
+  EXPECT_EQ(SyncService::DisableReasonSet(), service()->GetDisableReasons());
+  EXPECT_EQ(SyncService::TransportState::ACTIVE,
             service()->GetTransportState());
 
   // Note: If standalone transport is enabled, then setting kSyncManaged to