[Android] Introduce in-memory cache for Android sync settings.

Checking Android sync settings appears to be cheap but masks potential
slow calls and jank on the ui-thread due to many parties querying sync
state. One example for slowness is due to this issuing IPCs to a
separate service which may need to be re-started and load state from
disk.

Testing on a Galaxy Nexus shows ~50-70ms aggregrate time spent querying
Android sync state reducing to <5ms for loading an initial page. There
are additional checks throughout the lifetime of the app that now become
in-memory lookups.

Regarding the implementation:
- Master Sync Automatically is account agnostic and can be cached once
  at startup, and only updated when a notification is received that
  settings have changed.
- The remaining settings are per-account. On access of any of these
  settings, we grab the latest values for the requested account, and
  also update when a notification is received. Updates are infrequent
 and only when the signed in account changes.

BUG=238506
NOTRY=true

Review URL: https://chromiumcodereview.appspot.com/16092014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209249 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 403 insertions, 97 deletions

diff --git a/sync/android/java/src/org/chromium/sync/notifier/SyncStatusHelper.java b/sync/android/java/src/org/chromium/sync/notifier/SyncStatusHelper.java
index 0761b80..4b353c4 100644
--- a/sync/android/java/src/org/chromium/sync/notifier/SyncStatusHelper.java
+++ b/sync/android/java/src/org/chromium/sync/notifier/SyncStatusHelper.java
@@ -13,30 +13,111 @@ import android.os.StrictMode;
 
 import com.google.common.annotations.VisibleForTesting;
 
+import org.chromium.base.ObserverList;
 import org.chromium.sync.signin.AccountManagerHelper;
 import org.chromium.sync.signin.ChromeSigninController;
 
+import javax.annotation.concurrent.NotThreadSafe;
+import javax.annotation.concurrent.ThreadSafe;
+
 /**
- * A helper class to handle the current status of sync for Chrome in Android-land.
+ * A helper class to handle the current status of sync for Chrome in Android settings.
  *
- * It also has a helper class to be used by observers whenever sync settings change.
+ * It also provides an observer to be used whenever Android sync settings change.
  *
  * To retrieve an instance of this class, call SyncStatusHelper.get(someContext).
  */
+@ThreadSafe
 public class SyncStatusHelper {
-    // TODO(dsmyers): remove the downstream version of this constant.
+
+    /**
+     * In-memory holder of the sync configurations for a given account. On each
+     * access, updates the cache if the account has changed. This lazy-updating
+     * model is appropriate as the account changes rarely but may not be known
+     * when initially constructed. So long as we keep a single account, no
+     * expensive calls to Android are made.
+     */
+    @NotThreadSafe
+    private class CachedAccountSyncSettings {
+        private Account mAccount;
+        private boolean mSyncAutomatically;
+        private int mIsSyncable;
+
+        private void ensureSettingsAreForAccount(Account account) {
+            assert account != null;
+            if (account.equals(mAccount)) return;
+            updateSyncSettingsForAccount(account);
+        }
+
+        public void updateSyncSettingsForAccount(Account account) {
+            if (account == null) return;
+            mAccount = account;
+
+            StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
+            mSyncAutomatically = mSyncContentResolverWrapper.getSyncAutomatically(
+                    account, mContractAuthority);
+            mIsSyncable = mSyncContentResolverWrapper.getIsSyncable(account, mContractAuthority);
+            StrictMode.setThreadPolicy(oldPolicy);
+        }
+
+        public boolean getSyncAutomatically(Account account) {
+            ensureSettingsAreForAccount(account);
+            return mSyncAutomatically;
+        }
+
+        public void setIsSyncable(Account account) {
+            ensureSettingsAreForAccount(account);
+            if (mIsSyncable == 0) return;
+
+            mIsSyncable = 1;
+            StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
+            mSyncContentResolverWrapper.setIsSyncable(account, mContractAuthority, 1);
+            StrictMode.setThreadPolicy(oldPolicy);
+        }
+
+        public void setSyncAutomatically(Account account, boolean value) {
+            ensureSettingsAreForAccount(account);
+            if (mSyncAutomatically == value) return;
+
+            mSyncAutomatically = value;
+            StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
+            mSyncContentResolverWrapper.setSyncAutomatically(account, mContractAuthority, value);
+            StrictMode.setThreadPolicy(oldPolicy);
+        }
+    }
+
     public static final String AUTH_TOKEN_TYPE_SYNC = "chromiumsync";
 
     public static final String TAG = SyncStatusHelper.class.getSimpleName();
 
-    private static final Object LOCK = new Object();
+    /**
+     * Lock for ensuring singleton instantiation across threads.
+     */
+    private static final Object INSTANCE_LOCK = new Object();
 
     private static SyncStatusHelper sSyncStatusHelper;
 
+    private final String mContractAuthority;
+
     private final Context mApplicationContext;
 
     private final SyncContentResolverDelegate mSyncContentResolverWrapper;
 
+    private boolean mCachedMasterSyncAutomatically;
+
+    // Instantiation of SyncStatusHelper is guarded by a lock so volatile is unneeded.
+    private final CachedAccountSyncSettings mCachedSettings = new CachedAccountSyncSettings();
+
+    private final ObserverList<SyncSettingsChangedObserver> mObservers =
+            new ObserverList<SyncSettingsChangedObserver>();
+
+    /**
+     * Provides notifications when Android sync settings have changed.
+     */
+    public interface SyncSettingsChangedObserver {
+        public void syncSettingsChanged();
+    }
+
     /**
      * @param context the context
      * @param syncContentResolverWrapper an implementation of SyncContentResolverWrapper
@@ -45,6 +126,23 @@ public class SyncStatusHelper {
             SyncContentResolverDelegate syncContentResolverWrapper) {
         mApplicationContext = context.getApplicationContext();
         mSyncContentResolverWrapper = syncContentResolverWrapper;
+        mContractAuthority =
+                InvalidationController.get(mApplicationContext).getContractAuthority();
+
+        updateMasterSyncAutomaticallySetting();
+
+        mSyncContentResolverWrapper.addStatusChangeListener(
+                ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS,
+                new AndroidSyncSettingsChangedObserver());
+    }
+
+    private void updateMasterSyncAutomaticallySetting() {
+        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
+        synchronized (mCachedSettings) {
+            mCachedMasterSyncAutomatically = mSyncContentResolverWrapper
+                    .getMasterSyncAutomatically();
+        }
+        StrictMode.setThreadPolicy(oldPolicy);
     }
 
     /**
@@ -54,11 +152,11 @@ public class SyncStatusHelper {
      * instance of the SyncStatusHelper by calling overrideSyncStatusHelperForTests(...) with
      * your SyncContentResolverWrapper.
      *
-     * @param context the ApplicationContext is retreived from the context used as an argument.
+     * @param context the ApplicationContext is retrieved from the context used as an argument.
      * @return a singleton instance of the SyncStatusHelper
      */
     public static SyncStatusHelper get(Context context) {
-        synchronized (LOCK) {
+        synchronized (INSTANCE_LOCK) {
             if (sSyncStatusHelper == null) {
                 sSyncStatusHelper = new SyncStatusHelper(context,
                         new SystemSyncContentResolverDelegate());
@@ -77,7 +175,7 @@ public class SyncStatusHelper {
     @VisibleForTesting
     public static void overrideSyncStatusHelperForTests(Context context,
             SyncContentResolverDelegate syncContentResolverWrapper) {
-        synchronized (LOCK) {
+        synchronized (INSTANCE_LOCK) {
             if (sSyncStatusHelper != null) {
                 throw new IllegalStateException("SyncStatusHelper already exists");
             }
@@ -89,16 +187,15 @@ public class SyncStatusHelper {
      * Wrapper method for the ContentResolver.addStatusChangeListener(...) when we are only
      * interested in the settings type.
      */
-    public Object registerContentResolverObserver(SyncStatusObserver observer) {
-        return mSyncContentResolverWrapper.addStatusChangeListener(
-                ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS, observer);
+    public void registerContentResolverObserver(SyncSettingsChangedObserver observer) {
+        mObservers.addObserver(observer);
     }
 
     /**
      * Wrapper method for the ContentResolver.removeStatusChangeListener(...).
      */
-    public void unregisterContentResolverObserver(Object observerHandle) {
-        mSyncContentResolverWrapper.removeStatusChangeListener(observerHandle);
+    public void unregisterContentResolverObserver(SyncSettingsChangedObserver observer) {
+        mObservers.removeObserver(observer);
     }
 
     /**
@@ -110,14 +207,10 @@ public class SyncStatusHelper {
      * @return true if sync is on, false otherwise
      */
     public boolean isSyncEnabled(Account account) {
-        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
-        String contractAuthority =
-                InvalidationController.get(mApplicationContext).getContractAuthority();
-        boolean enabled = account != null &&
-                mSyncContentResolverWrapper.getMasterSyncAutomatically() &&
-                mSyncContentResolverWrapper.getSyncAutomatically(account, contractAuthority);
-        StrictMode.setThreadPolicy(oldPolicy);
-        return enabled;
+        if (account == null) return false;
+        synchronized (mCachedSettings) {
+            return mCachedMasterSyncAutomatically && mCachedSettings.getSyncAutomatically(account);
+        }
     }
 
     /**
@@ -142,13 +235,10 @@ public class SyncStatusHelper {
      * @return true if sync is on, false otherwise
      */
     public boolean isSyncEnabledForChrome(Account account) {
-        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
-        String contractAuthority =
-                InvalidationController.get(mApplicationContext).getContractAuthority();
-        boolean enabled = account != null &&
-                mSyncContentResolverWrapper.getSyncAutomatically(account, contractAuthority);
-        StrictMode.setThreadPolicy(oldPolicy);
-        return enabled;
+        if (account == null) return false;
+        synchronized (mCachedSettings) {
+            return mCachedSettings.getSyncAutomatically(account);
+        }
     }
 
     /**
@@ -157,10 +247,9 @@ public class SyncStatusHelper {
      * @return true if the global master sync is on, false otherwise
      */
     public boolean isMasterSyncAutomaticallyEnabled() {
-        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
-        boolean enabled = mSyncContentResolverWrapper.getMasterSyncAutomatically();
-        StrictMode.setThreadPolicy(oldPolicy);
-        return enabled;
+        synchronized (mCachedSettings) {
+            return mCachedMasterSyncAutomatically;
+        }
     }
 
     /**
@@ -169,14 +258,11 @@ public class SyncStatusHelper {
      * @param account the account to enable sync on
      */
     public void enableAndroidSync(Account account) {
-        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
         makeSyncable(account);
-        String contractAuthority =
-                InvalidationController.get(mApplicationContext).getContractAuthority();
-        if (!mSyncContentResolverWrapper.getSyncAutomatically(account, contractAuthority)) {
-            mSyncContentResolverWrapper.setSyncAutomatically(account, contractAuthority, true);
+
+        synchronized (mCachedSettings) {
+            mCachedSettings.setSyncAutomatically(account, true);
         }
-        StrictMode.setThreadPolicy(oldPolicy);
     }
 
     /**
@@ -185,13 +271,9 @@ public class SyncStatusHelper {
      * @param account the account to disable Chrome sync on
      */
     public void disableAndroidSync(Account account) {
-        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
-        String contractAuthority =
-                InvalidationController.get(mApplicationContext).getContractAuthority();
-        if (mSyncContentResolverWrapper.getSyncAutomatically(account, contractAuthority)) {
-            mSyncContentResolverWrapper.setSyncAutomatically(account, contractAuthority, false);
+        synchronized (mCachedSettings) {
+            mCachedSettings.setSyncAutomatically(account, false);
         }
-        StrictMode.setThreadPolicy(oldPolicy);
     }
 
     /**
@@ -201,31 +283,25 @@ public class SyncStatusHelper {
      * @param account the account to enable Chrome sync on
      */
     private void makeSyncable(Account account) {
-        String contractAuthority =
-                InvalidationController.get(mApplicationContext).getContractAuthority();
-        if (hasFinishedFirstSync(account)) {
-            mSyncContentResolverWrapper.setIsSyncable(account, contractAuthority, 1);
+        synchronized (mCachedSettings) {
+            mCachedSettings.setIsSyncable(account);
         }
-        // Disable the syncability of Chrome for all other accounts
+
+        StrictMode.ThreadPolicy oldPolicy = temporarilyAllowDiskWritesAndDiskReads();
+        // Disable the syncability of Chrome for all other accounts. Don't use
+        // our cache as we're touching many accounts that aren't signed in, so this saves
+        // extra calls to Android sync configuration.
         Account[] googleAccounts = AccountManagerHelper.get(mApplicationContext).
                 getGoogleAccounts();
         for (Account accountToSetNotSyncable : googleAccounts) {
             if (!accountToSetNotSyncable.equals(account) &&
                     mSyncContentResolverWrapper.getIsSyncable(
-                            accountToSetNotSyncable, contractAuthority) > 0) {
+                            accountToSetNotSyncable, mContractAuthority) > 0) {
                 mSyncContentResolverWrapper.setIsSyncable(accountToSetNotSyncable,
-                        contractAuthority, 0);
+                        mContractAuthority, 0);
             }
         }
-    }
-
-    /**
-     * Returns whether the given account has ever been synced.
-     */
-    boolean hasFinishedFirstSync(Account account) {
-        String contractAuthority =
-                InvalidationController.get(mApplicationContext).getContractAuthority();
-        return mSyncContentResolverWrapper.getIsSyncable(account, contractAuthority) <= 0;
+        StrictMode.setThreadPolicy(oldPolicy);
     }
 
     /**
@@ -233,16 +309,23 @@ public class SyncStatusHelper {
      *
      * To register the observer, call SyncStatusHelper.registerObserver(...).
      */
-    public static abstract class SyncSettingsChangedObserver implements SyncStatusObserver {
-
+    private class AndroidSyncSettingsChangedObserver implements SyncStatusObserver {
         @Override
         public void onStatusChanged(int which) {
             if (ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS == which) {
-                syncSettingsChanged();
+                // Sync settings have changed; update our in-memory caches
+                updateMasterSyncAutomaticallySetting();
+                synchronized (mCachedSettings) {
+                    mCachedSettings.updateSyncSettingsForAccount(
+                            ChromeSigninController.get(mApplicationContext).getSignedInUser());
+                }
+
+                // Notify anyone else that's interested
+                for (SyncSettingsChangedObserver observer: mObservers) {
+                    observer.syncSettingsChanged();
+                }
             }
         }
-
-        protected abstract void syncSettingsChanged();
     }
 
     /**
diff --git a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationControllerTest.java b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationControllerTest.java
index 76ddd49..44f9eb4 100644
--- a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationControllerTest.java
+++ b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationControllerTest.java
@@ -43,6 +43,11 @@ public class InvalidationControllerTest extends InstrumentationTestCase {
     protected void setUp() throws Exception {
         mContext = new IntentSavingContext(getInstrumentation().getTargetContext());
         mController = InvalidationController.get(mContext);
+        // We don't want to use the system content resolver, so we override it.
+        MockSyncContentResolverDelegate delegate = new MockSyncContentResolverDelegate();
+        // Android master sync can safely always be on.
+        delegate.setMasterSyncAutomatically(true);
+        SyncStatusHelper.overrideSyncStatusHelperForTests(mContext, delegate);
     }
 
     @SmallTest
@@ -112,11 +117,6 @@ public class InvalidationControllerTest extends InstrumentationTestCase {
     }
 
     private void setupSync(boolean syncEnabled) {
-        MockSyncContentResolverDelegate contentResolver = new MockSyncContentResolverDelegate();
-        // Android master sync can safely always be on.
-        contentResolver.setMasterSyncAutomatically(true);
-        // We don't want to use the system content resolver, so we override it.
-        SyncStatusHelper.overrideSyncStatusHelperForTests(mContext, contentResolver);
         Account account = AccountManagerHelper.createAccountFromName("test@gmail.com");
         ChromeSigninController chromeSigninController = ChromeSigninController.get(mContext);
         chromeSigninController.setSignedInAccountName(account.name);
diff --git a/sync/android/javatests/src/org/chromium/sync/notifier/signin/SyncStatusHelperTest.java b/sync/android/javatests/src/org/chromium/sync/notifier/signin/SyncStatusHelperTest.java
index 69f3528..a6d3dc7 100644
--- a/sync/android/javatests/src/org/chromium/sync/notifier/signin/SyncStatusHelperTest.java
+++ b/sync/android/javatests/src/org/chromium/sync/notifier/signin/SyncStatusHelperTest.java
@@ -4,41 +4,234 @@
 
 package org.chromium.sync.notifier.signin;
 
-import android.content.ContentResolver;
+import android.accounts.Account;
 import android.test.InstrumentationTestCase;
 import android.test.suitebuilder.annotation.SmallTest;
 
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.Feature;
+import org.chromium.sync.notifier.InvalidationController;
 import org.chromium.sync.notifier.SyncStatusHelper;
+import org.chromium.sync.signin.ChromeSigninController;
+import org.chromium.sync.test.util.MockSyncContentResolverDelegate;
 
 public class SyncStatusHelperTest extends InstrumentationTestCase {
 
+    private static class CountingMockSyncContentResolverDelegate
+            extends MockSyncContentResolverDelegate {
+        private int mGetMasterSyncAutomaticallyCalls;
+        private int mGetSyncAutomaticallyCalls;
+        private int mGetIsSyncableCalls;
+
+        @Override
+        public boolean getMasterSyncAutomatically() {
+            mGetMasterSyncAutomaticallyCalls++;
+            return super.getMasterSyncAutomatically();
+        }
+
+        @Override
+        public boolean getSyncAutomatically(Account account, String authority) {
+            mGetSyncAutomaticallyCalls++;
+            return super.getSyncAutomatically(account, authority);
+        }
+
+        @Override
+        public int getIsSyncable(Account account, String authority) {
+            mGetIsSyncableCalls++;
+            return super.getIsSyncable(account, authority);
+        }
+    }
+
+    private SyncStatusHelper mHelper;
+
+    private CountingMockSyncContentResolverDelegate mSyncContentResolverDelegate;
+
+    private String mAuthority;
+
+    private Account mTestAccount;
+
+    private Account mAlternateTestAccount;
+
+    @Override
+    protected void setUp() throws Exception {
+        mSyncContentResolverDelegate = new CountingMockSyncContentResolverDelegate();
+        SyncStatusHelper.overrideSyncStatusHelperForTests(
+                getInstrumentation().getTargetContext(), mSyncContentResolverDelegate);
+        mHelper = SyncStatusHelper.get(getInstrumentation().getTargetContext());
+        // Need to set the signed in account name to ensure that sync settings notifications
+        // update the right account.
+        ChromeSigninController.get(
+                getInstrumentation().getTargetContext()).setSignedInAccountName(
+                        "account@example.com");
+        mAuthority = InvalidationController.get(
+                getInstrumentation().getTargetContext()).getContractAuthority();
+        mTestAccount = new Account("account@example.com", "com.google");
+        mAlternateTestAccount = new Account("alternateAccount@example.com", "com.google");
+        super.setUp();
+    }
+
+    @SmallTest
+    @Feature({"Sync"})
+    public void testToggleMasterSyncAutomaticallyFromSettings() throws InterruptedException {
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("master sync should be set", mHelper.isMasterSyncAutomaticallyEnabled());
+
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(false);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertFalse("master sync should be unset", mHelper.isMasterSyncAutomaticallyEnabled());
+    }
+
     @SmallTest
     @Feature({"Sync"})
-    public void testStatusChangeForSettingsShouldCauseCall() {
-        TestSyncSettingsChangedObserver observer = new TestSyncSettingsChangedObserver();
-        observer.onStatusChanged(ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
-        assertTrue("Should have called the observer", observer.gotCalled);
+    public void testToggleAccountSyncFromSettings() throws InterruptedException {
+        // Turn on syncability.
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+
+        // First sync
+        mSyncContentResolverDelegate.setIsSyncable(mTestAccount, mAuthority, 1);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        mSyncContentResolverDelegate.setSyncAutomatically(mTestAccount, mAuthority, true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("sync should be set", mHelper.isSyncEnabled(mTestAccount));
+        assertTrue("sync should be set for chrome app",
+                mHelper.isSyncEnabledForChrome(mTestAccount));
+
+        // Disable sync automatically for the app
+        mSyncContentResolverDelegate.setSyncAutomatically(mTestAccount, mAuthority, false);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertFalse("sync should be unset", mHelper.isSyncEnabled(mTestAccount));
+        assertFalse("sync should be unset for chrome app",
+                mHelper.isSyncEnabledForChrome(mTestAccount));
+
+        // Re-enable sync
+        mSyncContentResolverDelegate.setSyncAutomatically(mTestAccount, mAuthority, true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("sync should be re-enabled", mHelper.isSyncEnabled(mTestAccount));
+        assertTrue("sync should be unset for chrome app",
+                mHelper.isSyncEnabledForChrome(mTestAccount));
+
+        // Disabled from master sync
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(false);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertFalse("sync should be disabled due to master sync",
+                mHelper.isSyncEnabled(mTestAccount));
+        assertTrue("sync should be set for chrome app",
+                mHelper.isSyncEnabledForChrome(mTestAccount));
     }
 
     @SmallTest
     @Feature({"Sync"})
-    public void testStatusChangeForAnythingElseShouldNoCauseCall() {
-        TestSyncSettingsChangedObserver observer = new TestSyncSettingsChangedObserver();
-        observer.onStatusChanged(ContentResolver.SYNC_OBSERVER_TYPE_ACTIVE);
-        observer.onStatusChanged(ContentResolver.SYNC_OBSERVER_TYPE_PENDING);
-        assertFalse("Should not have called the observer", observer.gotCalled);
+    public void testToggleAccountSyncFromApplication() throws InterruptedException {
+        // Turn on syncability.
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mHelper.enableAndroidSync(mTestAccount);
+            }
+        });
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("account should be synced", mHelper.isSyncEnabled(mTestAccount));
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mHelper.disableAndroidSync(mTestAccount);
+            }
+        });
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertFalse("account should not be synced", mHelper.isSyncEnabled(mTestAccount));
     }
 
-    private static class TestSyncSettingsChangedObserver
-            extends SyncStatusHelper.SyncSettingsChangedObserver {
+    @SmallTest
+    @Feature({"Sync"})
+    public void testToggleSyncabilityForMultipleAccounts() throws InterruptedException {
+        // Turn on syncability.
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
 
-        public boolean gotCalled;
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mHelper.enableAndroidSync(mTestAccount);
+            }
+        });
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("account should be synced", mHelper.isSyncEnabled(mTestAccount));
 
-        @Override
-        protected void syncSettingsChanged() {
-            gotCalled = true;
-        }
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mHelper.enableAndroidSync(mAlternateTestAccount);
+            }
+        });
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("alternate account should be synced",
+                mHelper.isSyncEnabled(mAlternateTestAccount));
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mHelper.disableAndroidSync(mAlternateTestAccount);
+            }
+        });
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertFalse("alternate account should not be synced",
+                mHelper.isSyncEnabled(mAlternateTestAccount));
+        assertTrue("account should still be synced", mHelper.isSyncEnabled(mTestAccount));
+
+        // Ensure we don't erroneously re-use cached data.
+        assertFalse("null account should not be synced", mHelper.isSyncEnabled(null));
     }
 
+    @SmallTest
+    @Feature({"Sync"})
+    public void testSyncSettingsCaching() throws InterruptedException {
+        // Turn on syncability.
+        mSyncContentResolverDelegate.setMasterSyncAutomatically(true);
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mHelper.enableAndroidSync(mTestAccount);
+            }
+        });
+        mSyncContentResolverDelegate.waitForLastNotificationCompleted();
+        assertTrue("account should be synced", mHelper.isSyncEnabled(mTestAccount));
+
+        int masterSyncAutomaticallyCalls =
+                mSyncContentResolverDelegate.mGetMasterSyncAutomaticallyCalls;
+        int isSyncableCalls = mSyncContentResolverDelegate.mGetIsSyncableCalls;
+        int getSyncAutomaticallyAcalls = mSyncContentResolverDelegate.mGetSyncAutomaticallyCalls;
+
+        // Do a bunch of reads.
+        mHelper.isMasterSyncAutomaticallyEnabled();
+        mHelper.isSyncEnabled();
+        mHelper.isSyncEnabled(mTestAccount);
+        mHelper.isSyncEnabledForChrome(mTestAccount);
+
+        // Ensure values were read from cache.
+        assertEquals(masterSyncAutomaticallyCalls,
+                mSyncContentResolverDelegate.mGetMasterSyncAutomaticallyCalls);
+        assertEquals(isSyncableCalls, mSyncContentResolverDelegate.mGetIsSyncableCalls);
+        assertEquals(getSyncAutomaticallyAcalls,
+                mSyncContentResolverDelegate.mGetSyncAutomaticallyCalls);
+
+        // Do a bunch of reads for alternate account.
+        mHelper.isMasterSyncAutomaticallyEnabled();
+        mHelper.isSyncEnabled(mAlternateTestAccount);
+        mHelper.isSyncEnabledForChrome(mAlternateTestAccount);
+
+        // Ensure master sync was cached but others are fetched once.
+        assertEquals(masterSyncAutomaticallyCalls,
+                mSyncContentResolverDelegate.mGetMasterSyncAutomaticallyCalls);
+        assertEquals(isSyncableCalls + 1, mSyncContentResolverDelegate.mGetIsSyncableCalls);
+        assertEquals(getSyncAutomaticallyAcalls + 1,
+                mSyncContentResolverDelegate.mGetSyncAutomaticallyCalls);
+    }
 }
diff --git a/sync/test/android/javatests/src/org/chromium/sync/test/util/MockSyncContentResolverDelegate.java b/sync/test/android/javatests/src/org/chromium/sync/test/util/MockSyncContentResolverDelegate.java
index f253d8c..e201a63 100644
--- a/sync/test/android/javatests/src/org/chromium/sync/test/util/MockSyncContentResolverDelegate.java
+++ b/sync/test/android/javatests/src/org/chromium/sync/test/util/MockSyncContentResolverDelegate.java
@@ -10,12 +10,17 @@ import android.content.ContentResolver;
 import android.content.SyncStatusObserver;
 import android.os.AsyncTask;
 
+import junit.framework.Assert;
+
+import org.chromium.base.ThreadUtils;
 import org.chromium.sync.notifier.SyncContentResolverDelegate;
 
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
 
 
 /**
@@ -32,6 +37,8 @@ public class MockSyncContentResolverDelegate implements SyncContentResolverDeleg
 
     private boolean mMasterSyncAutomatically;
 
+    private Semaphore mPendingObserverCount;
+
     public MockSyncContentResolverDelegate() {
         mSyncAutomaticallyMap = new HashMap<String, Boolean>();
         mObservers = new HashSet<AsyncSyncStatusObserver>();
@@ -125,12 +132,25 @@ public class MockSyncContentResolverDelegate implements SyncContentResolverDeleg
 
     private void notifyObservers() {
         synchronized (mObservers) {
+            mPendingObserverCount = new Semaphore(1 - mObservers.size());
             for (AsyncSyncStatusObserver observer : mObservers) {
-                observer.notifyObserverAsync();
+                observer.notifyObserverAsync(mPendingObserverCount);
             }
         }
     }
 
+    /**
+     * Blocks until the last notification has been issued to all registered observers.
+     * Note that if an observer is removed while a notification is being handled this can
+     * fail to return correctly.
+     *
+     * @throws InterruptedException
+     */
+    public void waitForLastNotificationCompleted() throws InterruptedException {
+        Assert.assertTrue("Timed out waiting for notifications to complete.",
+                mPendingObserverCount.tryAcquire(5, TimeUnit.SECONDS));
+    }
+
     private static class AsyncSyncStatusObserver {
 
         private final SyncStatusObserver mSyncStatusObserver;
@@ -139,16 +159,26 @@ public class MockSyncContentResolverDelegate implements SyncContentResolverDeleg
             mSyncStatusObserver = syncStatusObserver;
         }
 
-        private void notifyObserverAsync() {
-            new AsyncTask<Void, Void, Void>() {
-
-                @Override
-                protected Void doInBackground(Void... params) {
-                    mSyncStatusObserver.onStatusChanged(
-                            ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
-                    return null;
-                }
-            }.execute();
+        private void notifyObserverAsync(final Semaphore pendingObserverCount) {
+            if (ThreadUtils.runningOnUiThread()) {
+                new AsyncTask<Void, Void, Void>() {
+                    @Override
+                    protected Void doInBackground(Void... params) {
+                        mSyncStatusObserver.onStatusChanged(
+                                ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
+                        return null;
+                    }
+
+                    @Override
+                    protected void onPostExecute(Void result) {
+                        pendingObserverCount.release();
+                    }
+                }.execute();
+            } else {
+                mSyncStatusObserver.onStatusChanged(
+                        ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS);
+                pendingObserverCount.release();
+            }
         }
     }
 }