[Sync] Test SCF control states and a regression

Four new tests for SyncCustomizationFragment:

- SCF state is correct starting from sync on and transitioning to off.
- SCF state is correct starting from sync off and transitioning to on.
- SCF's data type controls respond correctly to the sync everything switch.
- SCF opening and closing does not start sync.

In order to prevent flakiness, this CL makes some additional changes.

- Remove MockAccountManager.postAsyncAccountChangedEvent().
  Justification: we have no control about when Android performs
  the callback from it. The callback coming in at weird points
  during other test cases was making tests flaky. Testing mechanisms
  should be predictable.
- SCF now caches the backend initialized state in order to filter
  syncStateChanged() calls down to only changes to that state.

BUG=480604

Review URL: https://codereview.chromium.org/1118833002

Cr-Commit-Position: refs/heads/master@{#329740}

chrome/android/java/src/org/chromium/chrome/browser/sync/ui/SyncCustomizationFragment.java

@@ -79,16 +79,18 @@ public class SyncCustomizationFragment extends PreferenceFragment implements
     public static final String PREFERENCE_SYNC_RECENT_TABS = "sync_recent_tabs";
     @VisibleForTesting
     public static final String PREFERENCE_ENCRYPTION = "encryption";
+    @VisibleForTesting
+    public static final String PREF_SYNC_SWITCH = "sync_switch";
+    @VisibleForTesting
+    public static final String PREFERENCE_SYNC_MANAGE_DATA = "sync_manage_data";
 
     public static final String ARGUMENT_ACCOUNT = "account";
 
     private static final int ERROR_COLOR = Color.RED;
-    @VisibleForTesting
-    public static final String PREF_SYNC_SWITCH = "sync_switch";
-    private static final String PREFERENCE_SYNC_MANAGE_DATA = "sync_manage_data";
 
     private ChromeSwitchPreference mSyncSwitchPreference;
     private AndroidSyncSettings mAndroidSyncSettings;
+    private boolean mIsSyncInitialized;
 
     @VisibleForTesting
     public static final String[] PREFS_TO_SAVE = {
@@ -120,6 +122,7 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container,
                              Bundle savedInstanceState) {
         mAndroidSyncSettings = AndroidSyncSettings.get(getActivity());
         mProfileSyncService = ProfileSyncService.get(getActivity());
+        mIsSyncInitialized = mProfileSyncService.isSyncInitialized();
 
         getActivity().setTitle(R.string.sign_in_sync);
 
@@ -223,6 +226,7 @@ public ChromeSwitchPreference getSyncSwitchPreference() {
     @Override
     public void onResume() {
         super.onResume();
+        mIsSyncInitialized = mProfileSyncService.isSyncInitialized();
         // This prevents sync from actually syncing until the dialog is closed.
         mProfileSyncService.setSetupInProgress(true);
         mProfileSyncService.addSyncStateChangedListener(this);
@@ -570,8 +574,12 @@ private void updateDataTypeState() {
      */
     @Override
     public void syncStateChanged() {
-        // Update all because Password syncability is also affected by the backend.
-        updateSyncStateFromSwitch();
+        boolean wasSyncInitialized = mIsSyncInitialized;
+        mIsSyncInitialized = mProfileSyncService.isSyncInitialized();
+        if (mIsSyncInitialized != wasSyncInitialized) {
+            // Update all because Password syncability is also affected by the backend.
+            updateSyncStateFromSwitch();
+        }
     }
 
     /**

chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncCustomizationFragmentTest.java

@@ -6,7 +6,10 @@
 
 import android.app.Activity;
 import android.app.FragmentTransaction;
+import android.preference.CheckBoxPreference;
+import android.preference.Preference;
 import android.preference.SwitchPreference;
+import android.preference.TwoStatePreference;
 import android.test.suitebuilder.annotation.SmallTest;
 
 import org.chromium.base.ThreadUtils;
@@ -36,23 +39,81 @@ protected void setUp() throws Exception {
     @Feature({"Sync"})
     public void testSyncSwitch() throws Exception {
         setupTestAccountAndSignInToSync(CLIENT_ID);
-
-        // Make sure sync is actually enabled.
-        mAndroidSyncSettings.enableChromeSync();
-        getInstrumentation().waitForIdleSync();
+        startSync();
         SyncCustomizationFragment fragment = startSyncCustomizationFragment();
         final SwitchPreference syncSwitch = getSyncSwitch(fragment);
 
         assertTrue(syncSwitch.isChecked());
         assertTrue(mAndroidSyncSettings.isChromeSyncEnabled());
-        toggleSwitch(syncSwitch);
+        togglePreference(syncSwitch);
         assertFalse(syncSwitch.isChecked());
         assertFalse(mAndroidSyncSettings.isChromeSyncEnabled());
-        toggleSwitch(syncSwitch);
+        togglePreference(syncSwitch);
         assertTrue(syncSwitch.isChecked());
         assertTrue(mAndroidSyncSettings.isChromeSyncEnabled());
     }
 
+    /**
+     * This is a regression test for http://crbug.com/454939.
+     */
+    @SmallTest
+    @Feature({"Sync"})
+    public void testOpeningSettingsDoesntEnableSync() throws Exception {
+        setupTestAccountAndSignInToSync(CLIENT_ID);
+        stopSync();
+        SyncCustomizationFragment fragment = startSyncCustomizationFragment();
+        closeFragment(fragment);
+        assertFalse(mAndroidSyncSettings.isChromeSyncEnabled());
+    }
+
+    @SmallTest
+    @Feature({"Sync"})
+    public void testDefaultControlStatesWithSyncOffThenOn() throws Exception {
+        setupTestAccountAndSignInToSync(CLIENT_ID);
+        stopSync();
+        SyncCustomizationFragment fragment = startSyncCustomizationFragment();
+        assertDefaultSyncOffState(fragment);
+        togglePreference(getSyncSwitch(fragment));
+        waitForSyncInitialized();
+        assertDefaultSyncOnState(fragment);
+    }
+
+    @SmallTest
+    @Feature({"Sync"})
+    public void testDefaultControlStatesWithSyncOnThenOff() throws Exception {
+        setupTestAccountAndSignInToSync(CLIENT_ID);
+        startSync();
+        SyncCustomizationFragment fragment = startSyncCustomizationFragment();
+        assertDefaultSyncOnState(fragment);
+        togglePreference(getSyncSwitch(fragment));
+        assertDefaultSyncOffState(fragment);
+    }
+
+    @SmallTest
+    @Feature({"Sync"})
+    public void testSyncEverythingAndDataTypes() throws Exception {
+        setupTestAccountAndSignInToSync(CLIENT_ID);
+        startSync();
+        SyncCustomizationFragment fragment = startSyncCustomizationFragment();
+        SwitchPreference syncEverything = getSyncEverything(fragment);
+        CheckBoxPreference[] dataTypes = getDataTypes(fragment);
+
+        assertDefaultSyncOnState(fragment);
+        togglePreference(syncEverything);
+        for (CheckBoxPreference dataType : dataTypes) {
+            assertTrue(dataType.isChecked());
+            assertTrue(dataType.isEnabled());
+        }
+
+        // If all data types are unchecked, sync should turn off.
+        for (CheckBoxPreference dataType : dataTypes) {
+            togglePreference(dataType);
+        }
+        getInstrumentation().waitForIdleSync();
+        assertDefaultSyncOffState(fragment);
+        assertFalse(mAndroidSyncSettings.isChromeSyncEnabled());
+    }
+
     private SyncCustomizationFragment startSyncCustomizationFragment() {
         FragmentTransaction transaction = mActivity.getFragmentManager().beginTransaction();
         transaction.add(R.id.content_container, new SyncCustomizationFragment(), TAG);
@@ -61,6 +122,13 @@ private SyncCustomizationFragment startSyncCustomizationFragment() {
         return (SyncCustomizationFragment) mActivity.getFragmentManager().findFragmentByTag(TAG);
     }
 
+    private void closeFragment(SyncCustomizationFragment fragment) {
+        FragmentTransaction transaction = mActivity.getFragmentManager().beginTransaction();
+        transaction.remove(fragment);
+        transaction.commit();
+        getInstrumentation().waitForIdleSync();
+    }
+
     private SwitchPreference getSyncSwitch(SyncCustomizationFragment fragment) {
         return (SwitchPreference) fragment.findPreference(
                 SyncCustomizationFragment.PREF_SYNC_SWITCH);
@@ -71,7 +139,66 @@ private SwitchPreference getSyncEverything(SyncCustomizationFragment fragment) {
                 SyncCustomizationFragment.PREFERENCE_SYNC_EVERYTHING);
     }
 
-    private void toggleSwitch(final SwitchPreference pref) {
+    private CheckBoxPreference[] getDataTypes(SyncCustomizationFragment fragment) {
+        return new CheckBoxPreference[] {
+            (CheckBoxPreference) fragment.findPreference(
+                    SyncCustomizationFragment.PREFERENCE_SYNC_AUTOFILL),
+            (CheckBoxPreference) fragment.findPreference(
+                    SyncCustomizationFragment.PREFERENCE_SYNC_BOOKMARKS),
+            (CheckBoxPreference) fragment.findPreference(
+                    SyncCustomizationFragment.PREFERENCE_SYNC_OMNIBOX),
+            (CheckBoxPreference) fragment.findPreference(
+                    SyncCustomizationFragment.PREFERENCE_SYNC_PASSWORDS),
+            (CheckBoxPreference) fragment.findPreference(
+                    SyncCustomizationFragment.PREFERENCE_SYNC_RECENT_TABS)
+        };
+    }
+
+    private Preference getEncryption(SyncCustomizationFragment fragment) {
+        return (Preference) fragment.findPreference(
+                SyncCustomizationFragment.PREFERENCE_ENCRYPTION);
+    }
+
+    private Preference getManageData(SyncCustomizationFragment fragment) {
+        return (Preference) fragment.findPreference(
+                SyncCustomizationFragment.PREFERENCE_SYNC_MANAGE_DATA);
+    }
+
+    private void assertDefaultSyncOnState(SyncCustomizationFragment fragment) {
+        assertTrue("The sync switch should be on.", getSyncSwitch(fragment).isChecked());
+        assertTrue("The sync switch should be enabled.", getSyncSwitch(fragment).isEnabled());
+        SwitchPreference syncEverything = getSyncEverything(fragment);
+        assertTrue("The sync everything switch should be on.", syncEverything.isChecked());
+        assertTrue("The sync everything switch should be enabled.", syncEverything.isEnabled());
+        for (CheckBoxPreference dataType : getDataTypes(fragment)) {
+            String key = dataType.getKey();
+            assertTrue("Data type " + key + " should be checked.", dataType.isChecked());
+            assertFalse("Data type " + key + " should be disabled.", dataType.isEnabled());
+        }
+        assertTrue("The encryption button should be enabled.",
+                getEncryption(fragment).isEnabled());
+        assertTrue("The manage sync data button should be always enabled.",
+                getManageData(fragment).isEnabled());
+    }
+
+    private void assertDefaultSyncOffState(SyncCustomizationFragment fragment) {
+        assertFalse("The sync switch should be off.", getSyncSwitch(fragment).isChecked());
+        assertTrue("The sync switch should be enabled.", getSyncSwitch(fragment).isEnabled());
+        SwitchPreference syncEverything = getSyncEverything(fragment);
+        assertTrue("The sync everything switch should be on.", syncEverything.isChecked());
+        assertFalse("The sync everything switch should be disabled.", syncEverything.isEnabled());
+        for (CheckBoxPreference dataType : getDataTypes(fragment)) {
+            String key = dataType.getKey();
+            assertTrue("Data type " + key + " should be checked.", dataType.isChecked());
+            assertFalse("Data type " + key + " should be disabled.", dataType.isEnabled());
+        }
+        assertFalse("The encryption button should be disabled.",
+                getEncryption(fragment).isEnabled());
+        assertTrue("The manage sync data button should be always enabled.",
+                getManageData(fragment).isEnabled());
+    }
+
+    private void togglePreference(final TwoStatePreference pref) {
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
             @Override
             public void run() {

chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTestBase.java

@@ -15,12 +15,16 @@
 import org.chromium.chrome.browser.signin.AccountIdProvider;
 import org.chromium.chrome.shell.ChromeShellTestBase;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
+import org.chromium.content.browser.test.util.Criteria;
+import org.chromium.content.browser.test.util.CriteriaHelper;
 import org.chromium.sync.AndroidSyncSettings;
 import org.chromium.sync.signin.AccountManagerHelper;
 import org.chromium.sync.signin.ChromeSigninController;
 import org.chromium.sync.test.util.MockAccountManager;
 import org.chromium.sync.test.util.MockSyncContentResolverDelegate;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 /**
  * Base class for common functionality between sync tests.
  */
@@ -33,6 +37,7 @@ public class SyncTestBase extends ChromeShellTestBase {
     protected MockAccountManager mAccountManager;
     protected SyncController mSyncController;
     protected FakeServerHelper mFakeServerHelper;
+    protected ProfileSyncService mProfileSyncService;
 
     @Override
     protected void setUp() throws Exception {
@@ -42,8 +47,8 @@ protected void setUp() throws Exception {
         mContext = new SyncTestUtil.SyncTestContext(targetContext);
 
         mapAccountNamesToIds();
-        setUpMockAccountManager();
         setUpMockAndroidSyncSettings();
+        setUpMockAccountManager();
 
         // Initializes ChromeSigninController to use our test context.
         ChromeSigninController.get(mContext);
@@ -53,10 +58,18 @@ protected void setUp() throws Exception {
             @Override
             public void run() {
                 mSyncController = SyncController.get(mContext);
+                // Ensure SyncController is registered with the new AndroidSyncSettings.
+                AndroidSyncSettings.get(mContext).registerObserver(mSyncController);
                 mFakeServerHelper = FakeServerHelper.get();
             }
         });
         FakeServerHelper.useFakeServer(targetContext);
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mProfileSyncService = ProfileSyncService.get(mContext);
+            }
+        });
     }
 
     @Override
@@ -131,4 +144,40 @@ public void run() {
         assertTrue("Sync everything should be enabled",
                 SyncTestUtil.isSyncEverythingEnabled(mContext));
     }
+
+    protected void startSync() throws InterruptedException {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                SyncController.get(mContext).start();
+            }
+        });
+        waitForSyncInitialized();
+    }
+
+    protected void stopSync() {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                SyncController.get(mContext).stop();
+            }
+        });
+        getInstrumentation().waitForIdleSync();
+    }
+
+    protected void waitForSyncInitialized() throws InterruptedException {
+        assertTrue(CriteriaHelper.pollForCriteria(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                final AtomicBoolean isInitialized = new AtomicBoolean(false);
+                ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+                    @Override
+                    public void run() {
+                        isInitialized.set(mProfileSyncService.isSyncInitialized());
+                    }
+                });
+                return isInitialized.get();
+            }
+        }));
+    }
 }

sync/test/android/javatests/src/org/chromium/sync/test/util/MockAccountManager.java

@@ -143,18 +143,46 @@ public Account[] getAccountsByType(@Nullable String type) {
     public boolean addAccountExplicitly(Account account, String password, Bundle userdata) {
         AccountHolder accountHolder =
                 AccountHolder.create().account(account).password(password).build();
-        return addAccountHolderExplicitly(accountHolder);
+        return addAccountHolderExplicitly(accountHolder, false);
     }
 
     public boolean addAccountHolderExplicitly(AccountHolder accountHolder) {
+        return addAccountHolderExplicitly(accountHolder, false);
+    }
+
+    /**
+     * Add an AccountHolder directly.
+     *
+     * @param accountHolder the account holder to add
+     * @param broadcastEvent whether to broadcast an AccountChangedEvent
+     * @return whether the account holder was added successfully
+     */
+    public boolean addAccountHolderExplicitly(AccountHolder accountHolder,
+            boolean broadcastEvent) {
         boolean result = mAccounts.add(accountHolder);
-        postAsyncAccountChangedEvent();
+        if (broadcastEvent) {
+            postAsyncAccountChangedEvent();
+        }
         return result;
     }
 
     public boolean removeAccountHolderExplicitly(AccountHolder accountHolder) {
+        return removeAccountHolderExplicitly(accountHolder, false);
+    }
+
+    /**
+     * Remove an AccountHolder directly.
+     *
+     * @param accountHolder the account holder to remove
+     * @param broadcastEvent whether to broadcast an AccountChangedEvent
+     * @return whether the account holder was removed successfully
+     */
+    public boolean removeAccountHolderExplicitly(AccountHolder accountHolder,
+            boolean broadcastEvent) {
         boolean result = mAccounts.remove(accountHolder);
-        postAsyncAccountChangedEvent();
+        if (broadcastEvent) {
+            postAsyncAccountChangedEvent();
+        }
         return result;
     }