Reimplement AccountManagerHelper singleton instance creation

Current implementation of AccountManagerHelper can instantiate wrong delegate
type if get() is called in too early. This CL fixes it. AtomicReference is used
to avoid relying on external synchronization, because get() can be called
from different threads. This CL also adds
AccountManagerHelper.resetAccountManagerHelperForTests method that makes sure
there's no interference between tests.

BUG=698258

Review-Url: https://codereview.chromium.org/2747293005
Cr-Commit-Position: refs/heads/master@{#463963}

chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java

@@ -561,5 +561,6 @@ public void run() {
                 SigninManager.get(getActivity()).removeSignInStateObserver(mTestObserver);
             }
         });
+        SigninTestUtil.tearDownAuthForTest();
     }
 }

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

@@ -12,6 +12,7 @@
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
@@ -76,6 +77,11 @@ private void setupTestAccount(Context context) {
         mAccountManager.addAccountHolderExplicitly(accountHolder.build());
     }
 
+    @After
+    public void tearDown() {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+    }
+
     /**
      * Launches the main preferences.
      */

chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java

@@ -59,6 +59,7 @@ protected void setUp() throws Exception {
     @Override
     protected void tearDown() throws Exception {
         mTestServer.stopAndDestroyServer();
+        SigninTestUtil.tearDownAuthForTest();
         super.tearDown();
     }
 

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

@@ -65,15 +65,17 @@ public void setUp() throws Exception {
         mapAccountNamesToIds();
         ApplicationData.clearAppData(
                 InstrumentationRegistry.getInstrumentation().getTargetContext());
-        mActivityTestRule.loadNativeLibraryAndInitBrowserProcess();
 
-        // Set up AccountManager.
+        // loadNativeLibraryAndInitBrowserProcess will access AccountManagerHelper, so it should
+        // be initialized beforehand.
         mContext = new AdvancedMockContext(
                 InstrumentationRegistry.getInstrumentation().getTargetContext());
         mAccountManager = new MockAccountManager(
                 mContext, InstrumentationRegistry.getInstrumentation().getContext());
         AccountManagerHelper.overrideAccountManagerHelperForTests(mContext, mAccountManager);
 
+        mActivityTestRule.loadNativeLibraryAndInitBrowserProcess();
+
         // Make sure there is no account signed in yet.
         mChromeSigninController = ChromeSigninController.get();
         mChromeSigninController.setSignedInAccountName(null);
@@ -98,6 +100,7 @@ public void run() {
                 mOAuth2TokenService.validateAccounts(false);
             }
         });
+        AccountManagerHelper.resetAccountManagerHelperForTests();
     }
 
     private void mapAccountNamesToIds() {

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

@@ -8,6 +8,7 @@
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -40,6 +41,11 @@ public void setUp() throws Exception {
         AccountManagerHelper.overrideAccountManagerHelperForTests(mContext, mAccountManager);
     }
 
+    @After
+    public void tearDown() {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+    }
+
     /*
      *  @SmallTest
      *  @Feature({"Sync"})

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

@@ -8,6 +8,7 @@
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -44,6 +45,11 @@ public void setUp() {
         AccountManagerHelper.overrideAccountManagerHelperForTests(mContext, mAccountManager);
     }
 
+    @After
+    public void tearDown() {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+    }
+
     @Test
     @SmallTest
     @RetryOnFailure

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

@@ -260,6 +260,9 @@ public void run() {
                 mBookmarks.destroy();
             }
         });
+
+        SigninTestUtil.tearDownAuthForTest();
+
         super.tearDown();
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProviderTest.java

@@ -56,6 +56,7 @@ public void setUp() throws Exception {
     @Override
     public void tearDown() throws Exception {
         SigninTestUtil.resetSigninState();
+        SigninTestUtil.tearDownAuthForTest();
         super.tearDown();
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseActivityTest.java

@@ -53,6 +53,7 @@ public void tearDown() throws Exception {
         // Clear ProfileSyncService in case it was mocked.
         ProfileSyncService.overrideForTests(null);
         SigninTestUtil.resetSigninState();
+        SigninTestUtil.tearDownAuthForTest();
     }
 
     /**

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

@@ -15,6 +15,7 @@
 import android.test.mock.MockContext;
 import android.test.mock.MockPackageManager;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -60,6 +61,11 @@ public void setUp() {
                 InstrumentationRegistry.getInstrumentation().getTargetContext());
     }
 
+    @After
+    public void tearDown() {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+    }
+
     private static class IntentTestPackageManager extends MockPackageManager {
 
         private final String mAction;

chrome/android/junit/src/org/chromium/chrome/browser/invalidation/InvalidationControllerTest.java

@@ -12,6 +12,7 @@
 import android.content.pm.PackageInfo;
 import android.os.Bundle;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -30,7 +31,9 @@
 import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.sync.ProfileSyncService;
+import org.chromium.components.signin.AccountManagerHelper;
 import org.chromium.components.signin.ChromeSigninController;
+import org.chromium.components.signin.SystemAccountManagerDelegate;
 import org.chromium.components.sync.AndroidSyncSettings;
 import org.chromium.components.sync.ModelType;
 import org.chromium.components.sync.ModelTypeHelper;
@@ -111,6 +114,9 @@ public void setUp() throws Exception {
 
         ContextUtils.initApplicationContextForTests(mContext.getApplicationContext());
 
+        AccountManagerHelper.overrideAccountManagerHelperForTests(
+                mContext, new SystemAccountManagerDelegate());
+
         ModelTypeHelper.setTestDelegate(new ModelTypeHelper.TestDelegate() {
             @Override
             public String toNotificationType(int modelType) {
@@ -139,6 +145,11 @@ public String toNotificationType(int modelType) {
         AndroidSyncSettings.enableChromeSync(mContext);
     }
 
+    @After
+    public void tearDown() {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+    }
+
     /**
      * Verify the intent sent by InvalidationController#stop().
      */

chrome/android/junit/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProviderUnitTest.java

@@ -237,6 +237,8 @@ public void testShouldProceed_notSignedIn() throws ProcessInitException {
                 .nativeShouldProceed(eq(5678L),
                         any(SupervisedUserContentProvider.SupervisedUserQueryReply.class),
                         eq("url"));
+
+        AccountManagerHelper.resetAccountManagerHelperForTests();
     }
 
     @Test
@@ -266,6 +268,8 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
 
         assertThat(result.shouldProceed(), is(false));
         assertThat(result.getErrorInt(0), is(5));
+
+        AccountManagerHelper.resetAccountManagerHelperForTests();
     }
 
     @Test

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

@@ -128,6 +128,7 @@ public void run() {
             }
         });
         SigninTestUtil.resetSigninState();
+        SigninTestUtil.tearDownAuthForTest();
 
         super.tearDown();
     }

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

@@ -52,6 +52,14 @@ public void run() {
         resetSigninState();
     }
 
+    /**
+     * Tears down the test authentication environment.
+     */
+    public static void tearDownAuthForTest() {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+        sContext = null;
+    }
+
     /**
      * Returns the currently signed in account.
      */

components/signin/core/browser/account_tracker_service_unittest.cc

@@ -15,6 +15,7 @@
 #include "components/signin/core/browser/account_fetcher_service.h"
 #include "components/signin/core/browser/account_info.h"
 #include "components/signin/core/browser/account_tracker_service.h"
+#include "components/signin/core/browser/child_account_info_fetcher.h"
 #include "components/signin/core/browser/fake_account_fetcher_service.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/signin/core/common/signin_pref_names.h"
@@ -255,6 +256,8 @@ class AccountTrackerServiceTest : public testing::Test {
   ~AccountTrackerServiceTest() override {}
 
   void SetUp() override {
+    ChildAccountInfoFetcher::InitializeForTests();
+
     fake_oauth2_token_service_.reset(new FakeOAuth2TokenService());
 
     pref_service_.registry()->RegisterListPref(

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

@@ -21,12 +21,14 @@
 import java.util.Locale;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Pattern;
 
 /**
  * AccountManagerHelper wraps our access of AccountManager in Android.
  *
- * Use the AccountManagerHelper.get(someContext) to instantiate it
+ * Use the {@link #initializeAccountManagerHelper} to instantiate it.
+ * After initialization, instance get be acquired by calling {@link #get}.
  */
 public class AccountManagerHelper {
     private static final String TAG = "Sync_Signin";
@@ -46,9 +48,7 @@ public class AccountManagerHelper {
     @VisibleForTesting
     public static final String FEATURE_IS_CHILD_ACCOUNT_KEY = "service_uca";
 
-    private static final Object sLock = new Object();
-
-    private static AccountManagerHelper sAccountManagerHelper;
+    private static final AtomicReference<AccountManagerHelper> sInstance = new AtomicReference<>();
 
     private final AccountManagerDelegate mAccountManager;
 
@@ -87,26 +87,21 @@ private AccountManagerHelper(AccountManagerDelegate accountManager) {
      * @param delegate the custom AccountManagerDelegate to use.
      */
     public static void initializeAccountManagerHelper(AccountManagerDelegate delegate) {
-        synchronized (sLock) {
-            assert sAccountManagerHelper == null;
-            sAccountManagerHelper = new AccountManagerHelper(delegate);
+        if (!sInstance.compareAndSet(null, new AccountManagerHelper(delegate))) {
+            throw new IllegalStateException("AccountManagerHelper is already initialized!");
         }
     }
 
     /**
-     * A getter method for AccountManagerHelper singleton which also initializes it if not wasn't
-     * already initialized.
+     * Singleton instance getter. Singleton must be initialized before calling this
+     * (by initializeAccountManagerHelper or overrideAccountManagerHelperForTests).
      *
-     * @return a singleton instance of the AccountManagerHelper
+     * @return a singleton instance
      */
     public static AccountManagerHelper get() {
-        synchronized (sLock) {
-            if (sAccountManagerHelper == null) {
-                sAccountManagerHelper =
-                        new AccountManagerHelper(new SystemAccountManagerDelegate());
-            }
-        }
-        return sAccountManagerHelper;
+        AccountManagerHelper instance = sInstance.get();
+        assert instance != null : "AccountManagerHelper is not initialized!";
+        return instance;
     }
 
     /**
@@ -120,9 +115,16 @@ public static AccountManagerHelper get() {
     @VisibleForTesting
     public static void overrideAccountManagerHelperForTests(
             Context context, AccountManagerDelegate delegate) {
-        synchronized (sLock) {
-            sAccountManagerHelper = new AccountManagerHelper(delegate);
-        }
+        sInstance.set(new AccountManagerHelper(delegate));
+    }
+
+    /**
+     * Resets custom AccountManagerHelper set with {@link #overrideAccountManagerHelperForTests}.
+     * Only for use in Tests.
+     */
+    @VisibleForTesting
+    public static void resetAccountManagerHelperForTests() {
+        sInstance.set(null);
     }
 
     /**

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

@@ -89,6 +89,13 @@ private void setIsChildAccount(boolean isChildAccount) {
         nativeSetIsChildAccount(mNativeAccountFetcherService, mAccountId, isChildAccount);
     }
 
+    @CalledByNative
+    private static void initializeForTests() {
+        Context context = ContextUtils.getApplicationContext();
+        AccountManagerDelegate delegate = new SystemAccountManagerDelegate();
+        AccountManagerHelper.overrideAccountManagerHelperForTests(context, delegate);
+    }
+
     private static native void nativeSetIsChildAccount(
             long accountFetcherServicePtr, String accountId, boolean isChildAccount);
 }

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

@@ -31,6 +31,12 @@ protected void setUp() throws Exception {
         mHelper = AccountManagerHelper.get();
     }
 
+    @Override
+    protected void tearDown() throws Exception {
+        AccountManagerHelper.resetAccountManagerHelperForTests();
+        super.tearDown();
+    }
+
     @SmallTest
     public void testCanonicalAccount() throws InterruptedException {
         addTestAccount("test@gmail.com", "password");

components/signin/core/browser/child_account_info_fetcher.cc

@@ -31,3 +31,9 @@ std::unique_ptr<ChildAccountInfoFetcher> ChildAccountInfoFetcher::CreateFrom(
 
 ChildAccountInfoFetcher::~ChildAccountInfoFetcher() {
 }
+
+void ChildAccountInfoFetcher::InitializeForTests() {
+#if defined(OS_ANDROID)
+  ChildAccountInfoFetcherAndroid::InitializeForTests();
+#endif
+}

components/signin/core/browser/child_account_info_fetcher.h

@@ -34,6 +34,8 @@ class ChildAccountInfoFetcher {
       net::URLRequestContextGetter* request_context_getter,
       invalidation::InvalidationService* invalidation_service);
   virtual ~ChildAccountInfoFetcher();
+
+  static void InitializeForTests();
 };
 
 #endif  // COMPONENTS_SIGNIN_CORE_BROWSER_CHILD_ACCOUNT_INFO_FETCHER_H_

components/signin/core/browser/child_account_info_fetcher_android.cc

@@ -29,6 +29,11 @@ std::unique_ptr<ChildAccountInfoFetcher> ChildAccountInfoFetcherAndroid::Create(
       new ChildAccountInfoFetcherAndroid(service, account_id, account_name));
 }
 
+void ChildAccountInfoFetcherAndroid::InitializeForTests() {
+  Java_ChildAccountInfoFetcher_initializeForTests(
+      base::android::AttachCurrentThread());
+}
+
 ChildAccountInfoFetcherAndroid::ChildAccountInfoFetcherAndroid(
     AccountFetcherService* service,
     const std::string& account_id,

components/signin/core/browser/child_account_info_fetcher_android.h

@@ -20,6 +20,8 @@ class ChildAccountInfoFetcherAndroid : public ChildAccountInfoFetcher {
       AccountFetcherService* service,
       const std::string& account_id);
 
+  static void InitializeForTests();
+
   // Register JNI methods.
   static bool Register(JNIEnv* env);