[Android] Add account list cache to AccountManagerFacade

Getting account list can take a while, especially in edge cases like
after Google Play Services update. This CL addresses this by adding
a cache for account list. Accessing the cache is cheap, so
getGoogleAccounts() and similar methods can now be safely invoked from
UI thread. The only edge case for getGoogleAccounts() is a time period
before the cache is populated after Chrome startup. In this case
getGoogleAccounts() will wait until the cache is populated. If this
waiting occurs on UI thread, it will record waiting time in
Signin.AndroidPopulateAccountCacheWaitingTime histogram.

Bug: 698258, 759798
Change-Id: I1e9b481cb4ccf4c24ff363fb7f4214d9d8c571f7
Reviewed-on: https://chromium-review.googlesource.com/675684Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506707}

chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkPersonalizedSigninPromoTest.java

@@ -67,8 +67,8 @@ public class BookmarkPersonalizedSigninPromoTest {
 
     @Before
     public void setUp() throws Exception {
-        mActivityTestRule.startMainActivityFromLauncher();
         AccountManagerFacade.overrideAccountManagerFacadeForTests(mAccountManagerDelegate);
+        mActivityTestRule.startMainActivityFromLauncher();
     }
 
     @Test
@@ -163,7 +163,7 @@ public class BookmarkPersonalizedSigninPromoTest {
     private void addTestAccount() {
         Account account = AccountManagerFacade.createAccountFromName(TEST_ACCOUNT_NAME);
         AccountHolder.Builder accountHolder = AccountHolder.builder(account).alwaysAccept(true);
-        mAccountManagerDelegate.addAccountHolderExplicitly(accountHolder.build());
+        mAccountManagerDelegate.addAccountHolderBlocking(accountHolder.build());
         ProfileDataSource.ProfileData profileData =
                 new ProfileDataSource.ProfileData(TEST_ACCOUNT_NAME, null, TEST_FULL_NAME, null);
         ThreadUtils.runOnUiThreadBlocking(

chrome/android/javatests/src/org/chromium/chrome/browser/preferences/PasswordViewingTypeTest.java

@@ -43,7 +43,6 @@ public class PasswordViewingTypeTest {
 
     private MainPreferences mMainPreferences;
     private ChromeBasePreference mPasswordsPref;
-    private static final String DEFAULT_ACCOUNT = "test@gmail.com";
     private Context mContext;
     private MockSyncContentResolverDelegate mSyncContentResolverDelegate;
     private String mAuthority;
@@ -52,6 +51,7 @@ public class PasswordViewingTypeTest {
 
     @Before
     public void setUp() throws Exception {
+        setupTestAccount();
         mSyncContentResolverDelegate = new MockSyncContentResolverDelegate();
         mContext = InstrumentationRegistry.getInstrumentation().getTargetContext();
         mMainPreferences = (MainPreferences) startMainPreferences(
@@ -59,7 +59,6 @@ public class PasswordViewingTypeTest {
                                    .getFragmentForTest();
         mPasswordsPref = (ChromeBasePreference) mMainPreferences.findPreference(
                 MainPreferences.PREF_SAVED_PASSWORDS);
-        setupTestAccount();
         AndroidSyncSettings.overrideForTests(mContext, mSyncContentResolverDelegate, null);
         mAuthority = AndroidSyncSettings.getContractAuthority(mContext);
         AndroidSyncSettings.updateAccount(mContext, mAccount);
@@ -73,7 +72,7 @@ public class PasswordViewingTypeTest {
         mAccount = AccountManagerFacade.createAccountFromName("account@example.com");
         AccountHolder.Builder accountHolder =
                 AccountHolder.builder(mAccount).password("password").alwaysAccept(true);
-        mAccountManager.addAccountHolderExplicitly(accountHolder.build());
+        mAccountManager.addAccountHolderBlocking(accountHolder.build());
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java

@@ -259,12 +259,11 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsOneAccountsRegisteredAndNoSignedInUser() throws Throwable {
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add account.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-
                 // Run test.
                 mOAuth2TokenService.validateAccounts(false);
 
@@ -279,12 +278,11 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsOneAccountsRegisteredSignedIn() throws Throwable {
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add account.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 
@@ -314,12 +312,11 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsSingleAccountWithoutChanges() throws Throwable {
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add account.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 
@@ -341,12 +338,11 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsSingleAccountThenAddOne() throws Throwable {
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add account.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 
@@ -355,10 +351,15 @@ public class OAuth2TokenServiceIntegrationTest {
                 Assert.assertEquals(1, mObserver.getAvailableCallCount());
                 Assert.assertEquals(0, mObserver.getRevokedCallCount());
                 Assert.assertEquals(0, mObserver.getLoadedCallCount());
+            }
+        });
 
-                // Add another account.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+        // Add another account.
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
 
+        mUiThreadTestRule.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
                 // Seed AccountTrackerService again since accounts changed after last validation.
                 seedAccountTrackerService(mContext);
 
@@ -374,21 +375,27 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsTwoAccountsThenRemoveOne() throws Throwable {
+        // Add accounts.
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add accounts.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 
                 // Run one validation.
                 mOAuth2TokenService.validateAccounts(false);
                 Assert.assertEquals(2, mObserver.getAvailableCallCount());
+            }
+        });
+
+        mAccountManager.removeAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
 
-                mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+        mUiThreadTestRule.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
                 mOAuth2TokenService.validateAccounts(false);
 
                 Assert.assertEquals(2, mObserver.getAvailableCallCount());
@@ -401,23 +408,28 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsTwoAccountsThenRemoveAll() throws Throwable {
+        // Add accounts.
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add accounts.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 
                 mOAuth2TokenService.validateAccounts(false);
                 Assert.assertEquals(2, mObserver.getAvailableCallCount());
+            }
+        });
 
-                // Remove all.
-                mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-                mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+        // Remove all.
+        mAccountManager.removeAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+        mAccountManager.removeAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
 
+        mUiThreadTestRule.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
                 // Re-validate and run checks.
                 mOAuth2TokenService.validateAccounts(false);
                 Assert.assertEquals(2, mObserver.getRevokedCallCount());
@@ -430,13 +442,13 @@ public class OAuth2TokenServiceIntegrationTest {
     @MediumTest
     @RetryOnFailure
     public void testValidateAccountsTwoAccountsThenRemoveAllSignOut() throws Throwable {
+        // Add accounts.
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add accounts.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 
@@ -445,9 +457,15 @@ public class OAuth2TokenServiceIntegrationTest {
 
                 // Remove all.
                 mChromeSigninController.setSignedInAccountName(null);
-                mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-                mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+            }
+        });
 
+        mAccountManager.removeAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+        mAccountManager.removeAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
+
+        mUiThreadTestRule.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
                 // Re-validate and run checks.
                 mOAuth2TokenService.validateAccounts(false);
                 Assert.assertEquals(2, mObserver.getRevokedCallCount());
@@ -459,13 +477,13 @@ public class OAuth2TokenServiceIntegrationTest {
     @Test
     @MediumTest
     public void testValidateAccountsTwoAccountsRegisteredAndOneSignedIn() throws Throwable {
+        // Add accounts.
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_1);
+        mAccountManager.addAccountHolderBlocking(TEST_ACCOUNT_HOLDER_2);
+
         mUiThreadTestRule.runOnUiThread(new Runnable() {
             @Override
             public void run() {
-                // Add accounts.
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
-                mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
-
                 // Mark user as signed in.
                 mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
 

chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java

@@ -63,7 +63,7 @@ public class OAuth2TokenServiceTest {
     public void testGetAccountsOneAccountRegistered() {
         Account account1 = AccountManagerFacade.createAccountFromName("foo@gmail.com");
         AccountHolder accountHolder1 = AccountHolder.builder(account1).build();
-        mAccountManager.addAccountHolderExplicitly(accountHolder1);
+        mAccountManager.addAccountHolderBlocking(accountHolder1);
 
         String[] sysAccounts = OAuth2TokenService.getSystemAccountNames();
         Assert.assertEquals("There should be one registered account", 1, sysAccounts.length);
@@ -80,10 +80,10 @@ public class OAuth2TokenServiceTest {
     public void testGetAccountsTwoAccountsRegistered() {
         Account account1 = AccountManagerFacade.createAccountFromName("foo@gmail.com");
         AccountHolder accountHolder1 = AccountHolder.builder(account1).build();
-        mAccountManager.addAccountHolderExplicitly(accountHolder1);
+        mAccountManager.addAccountHolderBlocking(accountHolder1);
         Account account2 = AccountManagerFacade.createAccountFromName("bar@gmail.com");
         AccountHolder accountHolder2 = AccountHolder.builder(account2).build();
-        mAccountManager.addAccountHolderExplicitly(accountHolder2);
+        mAccountManager.addAccountHolderBlocking(accountHolder2);
 
         String[] sysAccounts = OAuth2TokenService.getSystemAccountNames();
         Assert.assertEquals("There should be one registered account", 2, sysAccounts.length);
@@ -126,7 +126,7 @@ public class OAuth2TokenServiceTest {
                                               .hasBeenAccepted(oauth2Scope, true)
                                               .authToken(oauth2Scope, expectedToken)
                                               .build();
-        mAccountManager.addAccountHolderExplicitly(accountHolder);
+        mAccountManager.addAccountHolderBlocking(accountHolder);
 
         String accessToken = OAuth2TokenService.getOAuth2AccessTokenWithTimeout(
                 mContext, account, scope, 5, TimeUnit.SECONDS);

chrome/android/javatests/src/org/chromium/chrome/browser/signin/SigninHelperTest.java

@@ -149,7 +149,7 @@ public class SigninHelperTest {
         mEventChecker.insertRenameEvent("D", "A"); // Looped.
         Account account = AccountManagerFacade.createAccountFromName("D");
         AccountHolder accountHolder = AccountHolder.builder(account).build();
-        mAccountManager.addAccountHolderExplicitly(accountHolder);
+        mAccountManager.addAccountHolderBlocking(accountHolder);
         SigninHelper.updateAccountRenameData(mContext, mEventChecker);
         Assert.assertEquals("D", getNewSignedInAccountName());
     }

chrome/android/javatests/src/org/chromium/chrome/browser/util/FeatureUtilitiesTest.java

@@ -142,7 +142,7 @@ public class FeatureUtilitiesTest {
     }
 
     private void addTestAccount() {
-        mAccountManager.addAccountHolderExplicitly(
+        mAccountManager.addAccountHolderBlocking(
                 AccountHolder.builder(mTestAccount).alwaysAccept(true).build());
     }
 

chrome/test/android/javatests/src/org/chromium/chrome/test/util/ChromeSigninUtils.java

@@ -15,6 +15,7 @@ import android.content.Context;
 import android.os.Bundle;
 import android.text.TextUtils;
 
+import org.chromium.components.signin.AccountManagerFacade;
 import org.chromium.components.signin.ChromeSigninController;
 import org.chromium.components.signin.test.util.AccountHolder;
 import org.chromium.components.signin.test.util.FakeAccountManagerDelegate;
@@ -48,6 +49,7 @@ public class ChromeSigninUtils {
         mAccountManager = AccountManager.get(mTargetContext);
         mFakeAccountManagerDelegate = new FakeAccountManagerDelegate(
                 FakeAccountManagerDelegate.DISABLE_PROFILE_DATA_SOURCE);
+        AccountManagerFacade.overrideAccountManagerFacadeForTests(mFakeAccountManagerDelegate);
     }
 
     /**
@@ -76,15 +78,15 @@ public class ChromeSigninUtils {
 
         Account account = new Account(username, GOOGLE_ACCOUNT_TYPE);
         AccountHolder accountHolder = AccountHolder.builder(account).password(password).build();
-        mFakeAccountManagerDelegate.addAccountHolderExplicitly(accountHolder);
+        mFakeAccountManagerDelegate.addAccountHolderBlocking(accountHolder);
     }
 
     /**
      * Removes all fake accounts from the OS.
      */
-    public void removeAllFakeAccountsFromOs() {
+    public void removeAllFakeAccountsFromOs() throws Exception {
         for (Account acct : mFakeAccountManagerDelegate.getAccountsSyncNoThrow()) {
-            mFakeAccountManagerDelegate.removeAccountHolderExplicitly(
+            mFakeAccountManagerDelegate.removeAccountHolderBlocking(
                     AccountHolder.builder(acct).build());
         }
     }

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/signin/SigninTestUtil.java

@@ -64,9 +64,11 @@ public final class SigninTestUtil {
      * Tears down the test authentication environment.
      */
     public static void tearDownAuthForTest() {
-        for (AccountHolder accountHolder : sAddedAccounts) {
-            sAccountManager.removeAccountHolderExplicitly(accountHolder);
-        }
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            for (AccountHolder accountHolder : sAddedAccounts) {
+                sAccountManager.removeAccountHolderExplicitly(accountHolder);
+            }
+        });
         sAddedAccounts.clear();
         sContext = null;
     }
@@ -90,13 +92,12 @@ public final class SigninTestUtil {
      * Add an account with a given name.
      */
     public static Account addTestAccount(String name) {
-        Account account = createTestAccount(name);
-        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
-            @Override
-            public void run() {
-                AccountTrackerService.get().invalidateAccountSeedStatus(true);
-            }
-        });
+        Account account = AccountManagerFacade.createAccountFromName(name);
+        AccountHolder accountHolder = AccountHolder.builder(account).alwaysAccept(true).build();
+        sAccountManager.addAccountHolderBlocking(accountHolder);
+        sAddedAccounts.add(accountHolder);
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> AccountTrackerService.get().invalidateAccountSeedStatus(true));
         return account;
     }
 
@@ -104,26 +105,14 @@ public final class SigninTestUtil {
      * Add and sign in an account with the default name.
      */
     public static Account addAndSignInTestAccount() {
-        Account account = createTestAccount(DEFAULT_ACCOUNT);
-        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
-            @Override
-            public void run() {
-                ChromeSigninController.get().setSignedInAccountName(DEFAULT_ACCOUNT);
-                AccountTrackerService.get().invalidateAccountSeedStatus(true);
-            }
+        Account account = addTestAccount();
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            ChromeSigninController.get().setSignedInAccountName(DEFAULT_ACCOUNT);
+            AccountTrackerService.get().invalidateAccountSeedStatus(true);
         });
         return account;
     }
 
-    private static Account createTestAccount(String accountName) {
-        assert sContext != null;
-        Account account = AccountManagerFacade.createAccountFromName(accountName);
-        AccountHolder accountHolder = AccountHolder.builder(account).alwaysAccept(true).build();
-        sAccountManager.addAccountHolderExplicitly(accountHolder);
-        sAddedAccounts.add(accountHolder);
-        return account;
-    }
-
     private static void overrideAccountIdProvider() {
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
             @Override

components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java

@@ -40,8 +40,7 @@ public interface AccountManagerDelegate {
     void removeObserver(AccountsChangeObserver observer);
 
     /**
-     * Get all the accounts.
-     * This method shouldn't be called on the UI thread (violated due to crbug.com/517697).
+     * Get all the accounts synchronously.
      */
     @WorkerThread
     Account[] getAccountsSync() throws AccountManagerDelegateException;

components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerFacade.java

@@ -8,20 +8,25 @@ import android.accounts.Account;
 import android.accounts.AuthenticatorDescription;
 import android.app.Activity;
 import android.os.AsyncTask;
+import android.os.SystemClock;
 import android.support.annotation.AnyThread;
 import android.support.annotation.MainThread;
 import android.support.annotation.Nullable;
-import android.support.annotation.WorkerThread;
 
 import org.chromium.base.Callback;
 import org.chromium.base.Log;
+import org.chromium.base.ObserverList;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.SuppressFBWarnings;
+import org.chromium.base.metrics.CachedMetrics;
 import org.chromium.net.NetworkChangeNotifier;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -47,8 +52,6 @@ public class AccountManagerFacade {
     @VisibleForTesting
     public static final String FEATURE_IS_CHILD_ACCOUNT_KEY = "service_uca";
 
-    /** Synchronizes accesses to sInstance and sTestingInstance. */
-    private static final Object sLock = new Object();
     private static AccountManagerFacade sInstance;
     private static AccountManagerFacade sTestingInstance;
 
@@ -56,6 +59,13 @@ public class AccountManagerFacade {
             new AtomicReference<>();
 
     private final AccountManagerDelegate mDelegate;
+    private final ObserverList<AccountsChangeObserver> mObservers = new ObserverList<>();
+    private final AtomicReference<AccountManagerResult<Account[]>> mMaybeAccounts =
+            new AtomicReference<>();
+    private final AsyncTask<Void, Void, AccountManagerResult<Account[]>> mPopulateAccountCacheTask;
+    private final CachedMetrics.TimesHistogramSample mPopulateAccountCacheWaitingTimeHistogram =
+            new CachedMetrics.TimesHistogramSample(
+                    "Signin.AndroidPopulateAccountCacheWaitingTime", TimeUnit.MILLISECONDS);
 
     /**
      * A simple callback for getAuthToken.
@@ -81,8 +91,12 @@ public class AccountManagerFacade {
      * @param delegate the AccountManagerDelegate to use as a backend
      */
     private AccountManagerFacade(AccountManagerDelegate delegate) {
+        ThreadUtils.assertOnUiThread();
         mDelegate = delegate;
         mDelegate.registerObservers();
+        mDelegate.addObserver(this::updateAccounts);
+
+        mPopulateAccountCacheTask = updateAccounts();
     }
 
     /**
@@ -91,16 +105,16 @@ public class AccountManagerFacade {
      *
      * @param delegate the AccountManagerDelegate to use
      */
-    @AnyThread
+    @MainThread
+    @SuppressFBWarnings("LI_LAZY_INIT_UPDATE_STATIC")
     public static void initializeAccountManagerFacade(AccountManagerDelegate delegate) {
-        synchronized (sLock) {
-            if (sInstance != null) {
-                throw new IllegalStateException("AccountManagerFacade is already initialized!");
-            }
-            sInstance = new AccountManagerFacade(delegate);
-            if (sTestingInstance != null) return;
-            sAtomicInstance.set(sInstance);
+        ThreadUtils.assertOnUiThread();
+        if (sInstance != null) {
+            throw new IllegalStateException("AccountManagerFacade is already initialized!");
         }
+        sInstance = new AccountManagerFacade(delegate);
+        if (sTestingInstance != null) return;
+        sAtomicInstance.set(sInstance);
     }
 
     /**
@@ -112,10 +126,10 @@ public class AccountManagerFacade {
     @VisibleForTesting
     @AnyThread
     public static void overrideAccountManagerFacadeForTests(AccountManagerDelegate delegate) {
-        synchronized (sLock) {
+        ThreadUtils.runOnUiThreadBlocking(() -> {
             sTestingInstance = new AccountManagerFacade(delegate);
             sAtomicInstance.set(sTestingInstance);
-        }
+        });
     }
 
     /**
@@ -125,10 +139,10 @@ public class AccountManagerFacade {
     @VisibleForTesting
     @AnyThread
     public static void resetAccountManagerFacadeForTests() {
-        synchronized (sLock) {
+        ThreadUtils.runOnUiThreadBlocking(() -> {
             sTestingInstance = null;
             sAtomicInstance.set(sInstance);
-        }
+        });
     }
 
     /**
@@ -151,7 +165,8 @@ public class AccountManagerFacade {
     @MainThread
     public void addObserver(AccountsChangeObserver observer) {
         ThreadUtils.assertOnUiThread();
-        mDelegate.addObserver(observer);
+        boolean success = mObservers.addObserver(observer);
+        assert success : "Observer already added!";
     }
 
     /**
@@ -161,7 +176,8 @@ public class AccountManagerFacade {
     @MainThread
     public void removeObserver(AccountsChangeObserver observer) {
         ThreadUtils.assertOnUiThread();
-        mDelegate.removeObserver(observer);
+        boolean success = mObservers.removeObserver(observer);
+        assert success : "Can't find observer";
     }
 
     /**
@@ -178,7 +194,7 @@ public class AccountManagerFacade {
      * @throws AccountManagerDelegateException if Google Play Services are out of date,
      *         Chrome lacks necessary permissions, etc.
      */
-    @WorkerThread
+    @AnyThread
     public List<String> getGoogleAccountNames() throws AccountManagerDelegateException {
         List<String> accountNames = new ArrayList<>();
         for (Account account : getGoogleAccounts()) {
@@ -191,7 +207,7 @@ public class AccountManagerFacade {
      * Retrieves a list of the Google account names on the device.
      * Returns an empty list if Google Play Services aren't available or out of date.
      */
-    @WorkerThread
+    @AnyThread
     public List<String> tryGetGoogleAccountNames() {
         List<String> accountNames = new ArrayList<>();
         for (Account account : tryGetGoogleAccounts()) {
@@ -241,9 +257,24 @@ public class AccountManagerFacade {
      * @throws AccountManagerDelegateException if Google Play Services are out of date,
      *         Chrome lacks necessary permissions, etc.
      */
-    @WorkerThread
+    @AnyThread
     public Account[] getGoogleAccounts() throws AccountManagerDelegateException {
-        return mDelegate.getAccountsSync();
+        AccountManagerResult<Account[]> maybeAccounts = mMaybeAccounts.get();
+        if (maybeAccounts == null) {
+            try {
+                // First call to update hasn't finished executing yet, get() will wait for it
+                long now = SystemClock.elapsedRealtime();
+                maybeAccounts = mPopulateAccountCacheTask.get();
+                if (ThreadUtils.runningOnUiThread()) {
+                    mPopulateAccountCacheWaitingTimeHistogram.record(
+                            SystemClock.elapsedRealtime() - now);
+                }
+            } catch (InterruptedException | ExecutionException e) {
+                Log.w(TAG, "Update accounts task failed", e);
+                return new Account[0];
+            }
+        }
+        return maybeAccounts.get();
     }
 
     /**
@@ -273,7 +304,7 @@ public class AccountManagerFacade {
      * Retrieves all Google accounts on the device.
      * Returns an empty array if an error occurs while getting account list.
      */
-    @WorkerThread
+    @AnyThread
     public Account[] tryGetGoogleAccounts() {
         try {
             return getGoogleAccounts();
@@ -305,7 +336,7 @@ public class AccountManagerFacade {
      * Determine whether there are any Google accounts on the device.
      * Returns false if an error occurs while getting account list.
      */
-    @WorkerThread
+    @AnyThread
     public boolean hasGoogleAccounts() {
         return tryGetGoogleAccounts().length > 0;
     }
@@ -335,7 +366,7 @@ public class AccountManagerFacade {
      * Returns the account if it exists; null if account doesn't exists or an error occurs
      * while getting account list.
      */
-    @WorkerThread
+    @AnyThread
     public Account getAccountFromName(String accountName) {
         String canonicalName = canonicalizeName(accountName);
         Account[] accounts = tryGetGoogleAccounts();
@@ -369,7 +400,7 @@ public class AccountManagerFacade {
      * Returns whether an account exists with the given name.
      * Returns false if an error occurs while getting account list.
      */
-    @WorkerThread
+    @AnyThread
     public boolean hasAccountForName(String accountName) {
         return getAccountFromName(accountName) != null;
     }
@@ -502,6 +533,35 @@ public class AccountManagerFacade {
         return mDelegate.getProfileDataSource();
     }
 
+    private AsyncTask<Void, Void, AccountManagerResult<Account[]>> updateAccounts() {
+        ThreadUtils.assertOnUiThread();
+        AsyncTask<Void, Void, AccountManagerResult<Account[]>> updateAccountsTask =
+                new AsyncTask<Void, Void, AccountManagerResult<Account[]>>() {
+                    @Override
+                    public AccountManagerResult<Account[]> doInBackground(Void... params) {
+                        try {
+                            return new AccountManagerResult<>(mDelegate.getAccountsSync());
+                        } catch (AccountManagerDelegateException ex) {
+                            return new AccountManagerResult<>(ex);
+                        }
+                    }
+
+                    @Override
+                    public void onPostExecute(AccountManagerResult<Account[]> accounts) {
+                        mMaybeAccounts.set(accounts);
+                        fireOnAccountsChangedNotification();
+                    }
+                };
+        updateAccountsTask.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR);
+        return updateAccountsTask;
+    }
+
+    private void fireOnAccountsChangedNotification() {
+        for (AccountsChangeObserver observer : mObservers) {
+            observer.onAccountsChanged();
+        }
+    }
+
     private interface AuthTask<T> {
         T run() throws AuthException;
         void onSuccess(T result);

components/signin/core/browser/android/javatests/src/org/chromium/components/signin/test/AccountManagerFacadeTest.java

@@ -5,7 +5,6 @@
 package org.chromium.components.signin.test;
 
 import android.accounts.Account;
-import android.support.test.annotation.UiThreadTest;
 import android.support.test.filters.SmallTest;
 import android.support.test.rule.UiThreadTestRule;
 
@@ -52,8 +51,8 @@ public class AccountManagerFacadeTest {
 
     @Test
     @SmallTest
-    public void testCanonicalAccount() throws InterruptedException {
-        addTestAccount("test@gmail.com", "password");
+    public void testCanonicalAccount() {
+        addTestAccount("test@gmail.com");
 
         Assert.assertTrue(hasAccountForName("test@gmail.com"));
         Assert.assertTrue(hasAccountForName("Test@gmail.com"));
@@ -64,8 +63,8 @@ public class AccountManagerFacadeTest {
     // of stack trace or error message in that bug BEFORE disabling the test.
     @Test
     @SmallTest
-    public void testNonCanonicalAccount() throws InterruptedException {
-        addTestAccount("test.me@gmail.com", "password");
+    public void testNonCanonicalAccount() {
+        addTestAccount("test.me@gmail.com");
 
         Assert.assertTrue(hasAccountForName("test.me@gmail.com"));
         Assert.assertTrue(hasAccountForName("testme@gmail.com"));
@@ -75,33 +74,34 @@ public class AccountManagerFacadeTest {
 
     @Test
     @SmallTest
-    @UiThreadTest
-    public void testProfileDataSource() throws InterruptedException {
+    public void testProfileDataSource() throws Throwable {
         String accountName = "test@gmail.com";
-        addTestAccount(accountName, "password");
-        ProfileDataSource.ProfileData profileData = new ProfileDataSource.ProfileData(
-                accountName, null, "Test Full Name", "Test Given Name");
-
-        ProfileDataSource profileDataSource = mDelegate.getProfileDataSource();
-        Assert.assertNotNull(profileDataSource);
-        mDelegate.setProfileData(accountName, profileData);
-        Assert.assertArrayEquals(profileDataSource.getProfileDataMap().values().toArray(),
-                new ProfileDataSource.ProfileData[] {profileData});
-
-        mDelegate.setProfileData(accountName, null);
-        Assert.assertArrayEquals(profileDataSource.getProfileDataMap().values().toArray(),
-                new ProfileDataSource.ProfileData[0]);
+        addTestAccount(accountName);
+
+        mRule.runOnUiThread(() -> {
+            ProfileDataSource.ProfileData profileData = new ProfileDataSource.ProfileData(
+                    accountName, null, "Test Full Name", "Test Given Name");
+
+            ProfileDataSource profileDataSource = mDelegate.getProfileDataSource();
+            Assert.assertNotNull(profileDataSource);
+            mDelegate.setProfileData(accountName, profileData);
+            Assert.assertArrayEquals(profileDataSource.getProfileDataMap().values().toArray(),
+                    new ProfileDataSource.ProfileData[] {profileData});
+
+            mDelegate.setProfileData(accountName, null);
+            Assert.assertArrayEquals(profileDataSource.getProfileDataMap().values().toArray(),
+                    new ProfileDataSource.ProfileData[0]);
+        });
     }
 
-    private Account addTestAccount(String accountName, String password) {
+    private Account addTestAccount(String accountName) {
         Account account = AccountManagerFacade.createAccountFromName(accountName);
-        AccountHolder.Builder accountHolder =
-                AccountHolder.builder(account).password(password).alwaysAccept(true);
-        mDelegate.addAccountHolderExplicitly(accountHolder.build());
+        AccountHolder holder = AccountHolder.builder(account).alwaysAccept(true).build();
+        mDelegate.addAccountHolderBlocking(holder);
         return account;
     }
 
-    private boolean hasAccountForName(final String accountName) throws InterruptedException {
+    private boolean hasAccountForName(final String accountName) {
         final SimpleFuture<Boolean> result = new SimpleFuture<Boolean>();
         ThreadUtils.postOnUiThread(new Runnable() {
             @Override
@@ -109,6 +109,10 @@ public class AccountManagerFacadeTest {
                 mHelper.hasAccountForName(accountName, result.createCallback());
             }
         });
-        return result.get();
+        try {
+            return result.get();
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Exception occurred while waiting for future", e);
+        }
     }
 }

components/signin/core/browser/android/javatests/src/org/chromium/components/signin/test/util/FakeAccountManagerDelegate.java

@@ -33,6 +33,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
 
 /**
  * The FakeAccountManagerDelegate is intended for testing components that use AccountManagerFacade.
@@ -43,7 +44,7 @@ import java.util.UUID;
  * Currently, this implementation supports adding and removing accounts, handling credentials
  * (including confirming them), and handling of dummy auth tokens.
  *
- * If you want to auto-approve a given authtokentype, use addAccountHolderExplicitly(...) with
+ * If you want to auto-approve a given authtokentype, use {@link #addAccountHolderBlocking} with
  * an AccountHolder you have built with hasBeenAccepted("yourAuthTokenType", true).
  *
  * If you want to auto-approve all auth token types for a given account, use the {@link
@@ -171,35 +172,119 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate {
 
     public Account[] getAccountsSyncNoThrow() {
         ArrayList<Account> result = new ArrayList<>();
-        for (AccountHolder ah : mAccounts) {
-            result.add(ah.getAccount());
+        synchronized (mAccounts) {
+            for (AccountHolder ah : mAccounts) {
+                result.add(ah.getAccount());
+            }
         }
         return result.toArray(new Account[0]);
     }
 
     /**
-     * Add an AccountHolder directly.
+     * Add an AccountHolder.
+     *
+     * WARNING: this method will not wait for the cache in AccountManagerFacade to be updated. Tests
+     * that get accounts from AccountManagerFacade should use {@link #addAccountHolderBlocking}.
      *
      * @param accountHolder the account holder to add
      */
     public void addAccountHolderExplicitly(AccountHolder accountHolder) {
-        boolean added = mAccounts.add(accountHolder);
-        Assert.assertTrue("Account was already added", added);
-        for (AccountsChangeObserver observer : mObservers) {
-            observer.onAccountsChanged();
-        }
+        // TODO(https://crbug.com/698258): replace with assertOnUiThread after fixing internal tests
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            synchronized (mAccounts) {
+                boolean added = mAccounts.add(accountHolder);
+                Assert.assertTrue("Account already added", added);
+                for (AccountsChangeObserver observer : mObservers) {
+                    observer.onAccountsChanged();
+                }
+            }
+        });
     }
 
     /**
-     * Remove an AccountHolder directly.
+     * Remove an AccountHolder.
+     *
+     * WARNING: this method will not wait for the cache in AccountManagerFacade to be updated. Tests
+     * that get accounts from AccountManagerFacade should use {@link #removeAccountHolderBlocking}.
      *
      * @param accountHolder the account holder to remove
      */
     public void removeAccountHolderExplicitly(AccountHolder accountHolder) {
-        boolean removed = mAccounts.remove(accountHolder);
-        Assert.assertTrue("Account was already added", removed);
-        for (AccountsChangeObserver observer : mObservers) {
-            observer.onAccountsChanged();
+        // TODO(https://crbug.com/698258): replace with assertOnUiThread after fixing internal tests
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            synchronized (mAccounts) {
+                boolean removed = mAccounts.remove(accountHolder);
+                Assert.assertTrue("Can't find account", removed);
+                for (AccountsChangeObserver observer : mObservers) {
+                    observer.onAccountsChanged();
+                }
+            }
+        });
+    }
+
+    /**
+     * Add an AccountHolder and waits for AccountManagerFacade to update its cache. Requires
+     * AccountManagerFacade to be initialized with this delegate.
+     *
+     * @param accountHolder the account holder to add
+     */
+    public void addAccountHolderBlocking(AccountHolder accountHolder) {
+        ThreadUtils.assertOnBackgroundThread();
+
+        CountDownLatch cacheUpdated = new CountDownLatch(1);
+        AccountsChangeObserver observer = () -> {
+            // Observers are invoked asynchronously, so this call may be unrelated to accountHolder,
+            // hereby this check that account is in AccountManagerFacade cache.
+            if (AccountManagerFacade.get().hasAccountForName(accountHolder.getAccount().name)) {
+                cacheUpdated.countDown();
+            }
+        };
+
+        try {
+            ThreadUtils.runOnUiThreadBlocking(() -> {
+                AccountManagerFacade.get().addObserver(observer);
+                addAccountHolderExplicitly(accountHolder);
+            });
+
+            cacheUpdated.await();
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Exception occurred while waiting for future", e);
+        } finally {
+            ThreadUtils.runOnUiThreadBlocking(
+                    () -> AccountManagerFacade.get().removeObserver(observer));
+        }
+    }
+
+    /**
+     * Removes an AccountHolder and waits for AccountManagerFacade to update its cache. Requires
+     * AccountManagerFacade to be initialized with this delegate.
+     *
+     * @param accountHolder the account holder to remove
+     */
+    public void removeAccountHolderBlocking(AccountHolder accountHolder) {
+        ThreadUtils.assertOnBackgroundThread();
+
+        CountDownLatch cacheUpdated = new CountDownLatch(1);
+        AccountsChangeObserver observer = () -> {
+            // Observers are invoked asynchronously, so this call may be unrelated to accountHolder,
+            // hereby this check that account isn't in AccountManagerFacade cache.
+            if (!AccountManagerFacade.get().hasAccountForName(accountHolder.getAccount().name)) {
+                cacheUpdated.countDown();
+            }
+        };
+
+        try {
+            ThreadUtils.runOnUiThreadBlocking(() -> {
+                AccountManagerFacade.get().addObserver(observer);
+                removeAccountHolderExplicitly(accountHolder);
+            });
+
+            cacheUpdated.await();
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Exception occurred while waiting for future", e);
+        } finally {
+            ThreadUtils.runOnUiThreadBlocking(
+                    () -> AccountManagerFacade.get().removeObserver(observer));
         }
     }
 
@@ -228,9 +313,11 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate {
         if (authToken == null) {
             throw new IllegalArgumentException("AuthToken can not be null");
         }
-        for (AccountHolder ah : mAccounts) {
-            if (ah.removeAuthToken(authToken)) {
-                break;
+        synchronized (mAccounts) {
+            for (AccountHolder ah : mAccounts) {
+                if (ah.removeAuthToken(authToken)) {
+                    break;
+                }
             }
         }
     }
@@ -265,21 +352,18 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate {
             return;
         }
 
-        ThreadUtils.postOnUiThread(new Runnable() {
-            @Override
-            public void run() {
-                callback.onResult(true);
-            }
-        });
+        ThreadUtils.postOnUiThread(() -> callback.onResult(true));
     }
 
     private AccountHolder getAccountHolder(Account account) {
         if (account == null) {
             throw new IllegalArgumentException("Account can not be null");
         }
-        for (AccountHolder accountHolder : mAccounts) {
-            if (account.equals(accountHolder.getAccount())) {
-                return accountHolder;
+        synchronized (mAccounts) {
+            for (AccountHolder accountHolder : mAccounts) {
+                if (account.equals(accountHolder.getAccount())) {
+                    return accountHolder;
+                }
             }
         }
         throw new IllegalArgumentException("Can not find AccountHolder for account " + account);

components/sync/android/javatests/src/org/chromium/components/sync/AndroidSyncSettingsTest.java

@@ -146,27 +146,20 @@ public class AndroidSyncSettingsTest {
         mAccountManager = new FakeAccountManagerDelegate(
                 FakeAccountManagerDelegate.DISABLE_PROFILE_DATA_SOURCE);
         AccountManagerFacade.overrideAccountManagerFacadeForTests(mAccountManager);
-        mAccount = setupTestAccount("account@example.com");
-        mAlternateAccount = setupTestAccount("alternate@example.com");
+        mAccount = addTestAccount("account@example.com");
+        mAlternateAccount = addTestAccount("alternate@example.com");
     }
 
-    private Account setupTestAccount(String accountName) {
-        Account account = AccountManagerFacade.createAccountFromName(accountName);
-        AccountHolder.Builder accountHolder =
-                AccountHolder.builder(account).password("password").alwaysAccept(true);
-        mAccountManager.addAccountHolderExplicitly(accountHolder.build());
+    private Account addTestAccount(String name) {
+        Account account = AccountManagerFacade.createAccountFromName(name);
+        AccountHolder holder = AccountHolder.builder(account).alwaysAccept(true).build();
+        mAccountManager.addAccountHolderBlocking(holder);
         return account;
     }
 
     @After
     public void tearDown() throws Exception {
         if (mNumberOfCallsToWait > 0) mCallbackHelper.waitForCallback(0, mNumberOfCallsToWait);
-        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
-            @Override
-            public void run() {
-                AccountManagerFacade.resetAccountManagerFacadeForTests();
-            }
-        });
     }
 
     private void enableChromeSyncOnUiThread() {

tools/metrics/histograms/histograms.xml

@@ -76049,6 +76049,13 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Signin.AndroidPopulateAccountCacheWaitingTime" units="ms">
+  <owner>bsazonov@chromium.org</owner>
+  <summary>
+    The time UI thread spent waiting for the list of accounts to be populated.
+  </summary>
+</histogram>
+
 <histogram name="Signin.AndroidSigninPromoAction"
     enum="AndroidSigninPromoAction">
   <obsolete>