Decouple ChromeSync from Android Master Sync

This CL introduces a feature that when enabled causes ChromeSync not to
respect the state of the master sync toggle on Android [1]. Users who
previously had ChromeSync disabled just because of this toggle will be
migrated into a state with sync enabled.

[1] https://screenshot.googleplex.com/4BWPEDJ28F0

TBR=treib@chromium.org

Bug: 1105795
Change-Id: I3e93b1f7eba1f1188138eb805818552157af2456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2299353
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789515}

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java

@@ -211,7 +211,7 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs
             return sPromoStateForTests;
         }
 
-        if (!AndroidSyncSettings.get().isMasterSyncEnabled()) {
+        if (!AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync()) {
             return PromoState.PROMO_NONE;
         }
 

chrome/android/java/src/org/chromium/chrome/browser/signin/SyncPromoView.java

@@ -95,7 +95,7 @@ public class SyncPromoView extends LinearLayout implements AndroidSyncSettingsOb
 
     private void update() {
         ViewState viewState;
-        if (!AndroidSyncSettings.get().isMasterSyncEnabled()) {
+        if (!AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync()) {
             viewState = getStateForEnableAndroidSync();
         } else if (!AndroidSyncSettings.get().isChromeSyncEnabled()) {
             viewState = getStateForEnableChromeSync();

chrome/android/java/src/org/chromium/chrome/browser/sync/AndroidSyncSettings.java

@@ -20,6 +20,7 @@ import org.chromium.base.ContextUtils;
 import org.chromium.base.ObserverList;
 import org.chromium.base.StrictModeContext;
 import org.chromium.base.ThreadUtils;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.ChromeSigninController;
 import org.chromium.components.sync.SyncContentResolverDelegate;
@@ -120,11 +121,11 @@ public class AndroidSyncSettings {
      * @return true if sync is on, false otherwise
      */
     public boolean isSyncEnabled() {
-        return mMasterSyncEnabled && mChromeSyncEnabled;
+        return mChromeSyncEnabled && doesMasterSyncSettingAllowChromeSync();
     }
 
     /**
-     * Checks whether sync is currently enabled from Chrome for a given account.
+     * Checks whether sync is currently enabled for Chrome for a given account.
      *
      * It checks only Chrome sync setting for the given account,
      * and ignores the master sync setting.
@@ -137,10 +138,13 @@ public class AndroidSyncSettings {
     }
 
     /**
-     * Checks whether the master sync flag for Android is currently enabled.
+     * Checks whether the master sync flag for Android allows syncing Chrome
+     * data.
      */
-    public boolean isMasterSyncEnabled() {
-        return mMasterSyncEnabled;
+    public boolean doesMasterSyncSettingAllowChromeSync() {
+        return mMasterSyncEnabled
+                || ChromeFeatureList.isEnabled(
+                        ChromeFeatureList.DECOUPLE_SYNC_FROM_ANDROID_MASTER_SYNC);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java

@@ -114,7 +114,7 @@ public class SyncController implements ProfileSyncService.SyncStateChangedListen
         // TODO(crbug.com/921025): Don't mix these two concepts.
 
         mProfileSyncService.setSyncAllowedByPlatform(
-                AndroidSyncSettings.get().isMasterSyncEnabled());
+                AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync());
 
         boolean isSyncEnabled = AndroidSyncSettings.get().isSyncEnabled();
         if (isSyncEnabled == mProfileSyncService.isSyncRequested()) return;
@@ -127,7 +127,7 @@ public class SyncController implements ProfileSyncService.SyncStateChangedListen
                 // prefs (here and in the Sync customization UI).
                 AndroidSyncSettings.get().enableChromeSync();
             } else {
-                if (AndroidSyncSettings.get().isMasterSyncEnabled()) {
+                if (AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync()) {
                     RecordHistogram.recordEnumeratedHistogram("Sync.StopSource",
                             StopSource.ANDROID_CHROME_SYNC, StopSource.STOP_SOURCE_LIMIT);
                 } else {

chrome/android/java/src/org/chromium/chrome/browser/sync/settings/SyncSettingsUtils.java

@@ -72,7 +72,7 @@ public class SyncSettingsUtils {
      */
     @SyncError
     public static int getSyncError() {
-        if (!AndroidSyncSettings.get().isMasterSyncEnabled()) {
+        if (!AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync()) {
             return SyncError.ANDROID_SYNC_DISABLED;
         }
 
@@ -159,7 +159,7 @@ public class SyncSettingsUtils {
         ProfileSyncService profileSyncService = ProfileSyncService.get();
         Resources res = context.getResources();
 
-        if (!AndroidSyncSettings.get().isMasterSyncEnabled()) {
+        if (!AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync()) {
             return res.getString(R.string.sync_android_master_sync_disabled);
         }
 

chrome/android/javatests/src/org/chromium/chrome/browser/sync/AndroidSyncSettingsTest.java

@@ -13,7 +13,9 @@ import androidx.test.filters.SmallTest;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.Callback;
@@ -21,7 +23,11 @@ import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.sync.AndroidSyncSettings.AndroidSyncSettingsObserver;
+import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
+import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
+import org.chromium.chrome.test.util.browser.Features.JUnitProcessor;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.AccountUtils;
 import org.chromium.components.signin.ChromeSigninController;
@@ -36,6 +42,7 @@ import java.util.concurrent.atomic.AtomicInteger;
  * Tests for AndroidSyncSettings.
  */
 @RunWith(BaseJUnit4ClassRunner.class)
+@DisableFeatures(ChromeFeatureList.DECOUPLE_SYNC_FROM_ANDROID_MASTER_SYNC)
 public class AndroidSyncSettingsTest {
     private static class CountingMockSyncContentResolverDelegate
             extends MockSyncContentResolverDelegate {
@@ -100,6 +107,9 @@ public class AndroidSyncSettingsTest {
         }
     }
 
+    @Rule
+    public TestRule mProcessorRule = new JUnitProcessor();
+
     private AndroidSyncSettings mAndroidSyncSettings;
     private CountingMockSyncContentResolverDelegate mSyncContentResolverDelegate;
     private String mAuthority;
@@ -207,12 +217,13 @@ public class AndroidSyncSettingsTest {
     public void testToggleMasterSyncFromSettings() throws InterruptedException {
         mSyncContentResolverDelegate.setMasterSyncAutomatically(true);
         mSyncContentResolverDelegate.waitForLastNotificationCompleted();
-        Assert.assertTrue("master sync should be set", mAndroidSyncSettings.isMasterSyncEnabled());
+        Assert.assertTrue("master sync should be set",
+                mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync());
 
         mSyncContentResolverDelegate.setMasterSyncAutomatically(false);
         mSyncContentResolverDelegate.waitForLastNotificationCompleted();
-        Assert.assertFalse(
-                "master sync should be unset", mAndroidSyncSettings.isMasterSyncEnabled());
+        Assert.assertFalse("master sync should be unset",
+                mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync());
     }
 
     @Test
@@ -251,8 +262,8 @@ public class AndroidSyncSettingsTest {
         mSyncContentResolverDelegate.waitForLastNotificationCompleted();
         Assert.assertFalse(
                 "sync should be disabled due to master sync", mAndroidSyncSettings.isSyncEnabled());
-        Assert.assertFalse(
-                "master sync should be disabled", mAndroidSyncSettings.isMasterSyncEnabled());
+        Assert.assertFalse("master sync should be disabled",
+                mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync());
         Assert.assertTrue(
                 "sync should be set for chrome app", mAndroidSyncSettings.isChromeSyncEnabled());
     }
@@ -324,7 +335,7 @@ public class AndroidSyncSettingsTest {
                 mSyncContentResolverDelegate.mGetSyncAutomaticallyCalls.get();
 
         // Do a bunch of reads.
-        mAndroidSyncSettings.isMasterSyncEnabled();
+        mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync();
         mAndroidSyncSettings.isSyncEnabled();
         mAndroidSyncSettings.isChromeSyncEnabled();
 
@@ -338,7 +349,7 @@ public class AndroidSyncSettingsTest {
 
         // Do a bunch of reads for alternate account.
         updateAccount(mAlternateAccount);
-        mAndroidSyncSettings.isMasterSyncEnabled();
+        mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync();
         mAndroidSyncSettings.isSyncEnabled();
         mAndroidSyncSettings.isChromeSyncEnabled();
 
@@ -441,4 +452,23 @@ public class AndroidSyncSettingsTest {
         // Should still be enabled.
         Assert.assertTrue(mSyncContentResolverDelegate.getSyncAutomatically(mAccount, mAuthority));
     }
+
+    @Test
+    @SmallTest
+    @Feature({"Sync"})
+    @EnableFeatures(ChromeFeatureList.DECOUPLE_SYNC_FROM_ANDROID_MASTER_SYNC)
+    public void testSyncStateDoesNotDependOnMasterSync() throws InterruptedException {
+        mSyncContentResolverDelegate.setSyncAutomatically(mAccount, mAuthority, true);
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(false);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        Assert.assertTrue(mAndroidSyncSettings.isChromeSyncEnabled());
+        Assert.assertTrue(mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync());
+        Assert.assertTrue(mAndroidSyncSettings.isSyncEnabled());
+
+        mSyncContentResolverDelegate.setSyncAutomatically(mAccount, mAuthority, false);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        Assert.assertFalse(mAndroidSyncSettings.isChromeSyncEnabled());
+        Assert.assertTrue(mAndroidSyncSettings.doesMasterSyncSettingAllowChromeSync());
+        Assert.assertFalse(mAndroidSyncSettings.isSyncEnabled());
+    }
 }

chrome/browser/flags/android/chrome_feature_list.cc

@@ -255,6 +255,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &safe_browsing::kSafeBrowsingSecuritySectionUIAndroid,
     &security_state::features::kMarkHttpAsFeature,
     &signin::kMobileIdentityConsistency,
+    &switches::kDecoupleSyncFromAndroidMasterSync,
     &switches::kSyncUseSessionsUnregisterDelay,
     &subresource_filter::kSafeBrowsingSubresourceFilter,
 };

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java

@@ -271,6 +271,8 @@ public abstract class ChromeFeatureList {
             "CookiesWithoutSameSiteMustBeSecure";
     public static final String DARKEN_WEBSITES_CHECKBOX_IN_THEMES_SETTING =
             "DarkenWebsitesCheckboxInThemesSetting";
+    public static final String DECOUPLE_SYNC_FROM_ANDROID_MASTER_SYNC =
+            "DecoupleSyncFromAndroidMasterSync";
     public static final String DIRECT_ACTIONS = "DirectActions";
     public static final String DNS_OVER_HTTPS = "DnsOverHttps";
     public static final String DOWNLOAD_FILE_PROVIDER = "DownloadFileProvider";

components/sync/driver/sync_driver_switches.cc

@@ -64,4 +64,8 @@ const base::Feature kSyncDeviceInfoInTransportMode{
 const base::Feature kProfileSyncServiceUsesThreadPool{
     "ProfileSyncServiceUsesThreadPool", base::FEATURE_ENABLED_BY_DEFAULT};
 
+// Stops honoring the Android master sync toggle.
+const base::Feature kDecoupleSyncFromAndroidMasterSync{
+    "DecoupleSyncFromAndroidMasterSync", base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace switches

components/sync/driver/sync_driver_switches.h

@@ -32,6 +32,7 @@ extern const base::Feature
 extern const base::Feature kSyncWifiConfigurations;
 extern const base::Feature kSyncDeviceInfoInTransportMode;
 extern const base::Feature kProfileSyncServiceUsesThreadPool;
+extern const base::Feature kDecoupleSyncFromAndroidMasterSync;
 
 }  // namespace switches