Commit d96d58ed authored by Pâris MEULEMAN's avatar Pâris MEULEMAN Committed by Commit Bot

Remove static usage of AccountManagerFacade in OAuth2TokenService

Remove most of the static functions relying on AccountManagerFacade in
OAuth2TokenService, instead provide AccountManagerFacade during
construction. OAuth2TokenService.GetNewAccessToken wasn't updated
as its dependency, |InvalidationService| does not have access to
OAuth2TokenService instances.

Bug: 934688
Change-Id: I098923209ecdc66970ec8f6a8d73343325e1bf98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763426
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Auto-Submit: Pâris Meuleman <pmeuleman@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701914}
parent 51d6c04e
...@@ -16,6 +16,7 @@ import org.chromium.base.ContextUtils; ...@@ -16,6 +16,7 @@ import org.chromium.base.ContextUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.signin.IdentityServicesProvider;
import org.chromium.components.signin.AccountManagerFacade; import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.OAuth2TokenService; import org.chromium.components.signin.OAuth2TokenService;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
...@@ -268,7 +269,7 @@ class AutofillAssistantClient { ...@@ -268,7 +269,7 @@ class AutofillAssistantClient {
return; return;
} }
OAuth2TokenService.getAccessToken( IdentityServicesProvider.getOAuth2TokenService().getAccessToken(
mAccount, AUTH_TOKEN_TYPE, new OAuth2TokenService.GetAccessTokenCallback() { mAccount, AUTH_TOKEN_TYPE, new OAuth2TokenService.GetAccessTokenCallback() {
@Override @Override
public void onGetTokenSuccess(String token) { public void onGetTokenSuccess(String token) {
...@@ -294,7 +295,7 @@ class AutofillAssistantClient { ...@@ -294,7 +295,7 @@ class AutofillAssistantClient {
return; return;
} }
OAuth2TokenService.invalidateAccessToken(accessToken); IdentityServicesProvider.getOAuth2TokenService().invalidateAccessToken(accessToken);
} }
/** Returns the e-mail address that corresponds to the access token or an empty string. */ /** Returns the e-mail address that corresponds to the access token or an empty string. */
...@@ -316,8 +317,9 @@ class AutofillAssistantClient { ...@@ -316,8 +317,9 @@ class AutofillAssistantClient {
// According to API, location for CDMA networks is unreliable // According to API, location for CDMA networks is unreliable
if (telephonyManager != null if (telephonyManager != null
&& telephonyManager.getPhoneType() != TelephonyManager.PHONE_TYPE_CDMA) && telephonyManager.getPhoneType() != TelephonyManager.PHONE_TYPE_CDMA) {
return telephonyManager.getNetworkCountryIso(); return telephonyManager.getNetworkCountryIso();
}
return null; return null;
} }
......
...@@ -22,6 +22,7 @@ import org.chromium.base.ThreadUtils; ...@@ -22,6 +22,7 @@ import org.chromium.base.ThreadUtils;
import org.chromium.base.task.PostTask; import org.chromium.base.task.PostTask;
import org.chromium.base.task.TaskTraits; import org.chromium.base.task.TaskTraits;
import org.chromium.chrome.browser.init.ProcessInitializationHandler; import org.chromium.chrome.browser.init.ProcessInitializationHandler;
import org.chromium.chrome.browser.signin.IdentityServicesProvider;
import org.chromium.components.signin.ChromeSigninController; import org.chromium.components.signin.ChromeSigninController;
import org.chromium.components.signin.OAuth2TokenService; import org.chromium.components.signin.OAuth2TokenService;
import org.chromium.components.sync.SyncConstants; import org.chromium.components.sync.SyncConstants;
...@@ -67,7 +68,8 @@ public class InvalidationGcmUpstreamSender extends GcmUpstreamSenderService { ...@@ -67,7 +68,8 @@ public class InvalidationGcmUpstreamSender extends GcmUpstreamSenderService {
} }
// Attempt to retrieve a token for the user. // Attempt to retrieve a token for the user.
OAuth2TokenService.getAccessToken(account, SyncConstants.CHROME_SYNC_OAUTH2_SCOPE, IdentityServicesProvider.getOAuth2TokenService().getAccessToken(account,
SyncConstants.CHROME_SYNC_OAUTH2_SCOPE,
new OAuth2TokenService.GetAccessTokenCallback() { new OAuth2TokenService.GetAccessTokenCallback() {
@Override @Override
public void onGetTokenSuccess(final String token) { public void onGetTokenSuccess(final String token) {
......
...@@ -37,6 +37,7 @@ import org.chromium.chrome.browser.tab.Tab; ...@@ -37,6 +37,7 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeActivityTestRule; import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.TabTitleObserver; import org.chromium.chrome.test.util.browser.TabTitleObserver;
import org.chromium.chrome.test.util.browser.signin.SigninTestUtil;
import org.chromium.components.background_task_scheduler.BackgroundTaskScheduler; import org.chromium.components.background_task_scheduler.BackgroundTaskScheduler;
import org.chromium.components.background_task_scheduler.BackgroundTaskSchedulerFactory; import org.chromium.components.background_task_scheduler.BackgroundTaskSchedulerFactory;
import org.chromium.components.background_task_scheduler.TaskInfo; import org.chromium.components.background_task_scheduler.TaskInfo;
...@@ -81,6 +82,10 @@ public final class BackgroundSyncTest { ...@@ -81,6 +82,10 @@ public final class BackgroundSyncTest {
.when(mTaskScheduler) .when(mTaskScheduler)
.schedule(eq(ContextUtils.getApplicationContext()), any(TaskInfo.class)); .schedule(eq(ContextUtils.getApplicationContext()), any(TaskInfo.class));
// loadNativeLibraryAndInitBrowserProcess will access AccountManagerFacade, so it should
// be initialized beforehand.
SigninTestUtil.setUpAuthForTest();
// This is necessary because our test devices don't have Google Play Services up to date, // This is necessary because our test devices don't have Google Play Services up to date,
// and BackgroundSync requires that. Remove this once https://crbug.com/514449 has been // and BackgroundSync requires that. Remove this once https://crbug.com/514449 has been
// fixed. // fixed.
...@@ -102,6 +107,7 @@ public final class BackgroundSyncTest { ...@@ -102,6 +107,7 @@ public final class BackgroundSyncTest {
@After @After
public void tearDown() { public void tearDown() {
if (mTestServer != null) mTestServer.stopAndDestroyServer(); if (mTestServer != null) mTestServer.stopAndDestroyServer();
SigninTestUtil.tearDownAuthForTest();
} }
@Test @Test
......
...@@ -7,15 +7,14 @@ package org.chromium.chrome.browser.payments; ...@@ -7,15 +7,14 @@ package org.chromium.chrome.browser.payments;
import android.support.test.filters.MediumTest; import android.support.test.filters.MediumTest;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.chromium.base.LocaleUtils; import org.chromium.base.LocaleUtils;
import org.chromium.base.test.BaseJUnit4ClassRunner; import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeBrowserTestRule;
import org.chromium.components.payments.CurrencyFormatter; import org.chromium.components.payments.CurrencyFormatter;
import org.chromium.content_public.browser.test.NativeLibraryTestRule;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
...@@ -26,7 +25,7 @@ import java.util.List; ...@@ -26,7 +25,7 @@ import java.util.List;
@RunWith(BaseJUnit4ClassRunner.class) @RunWith(BaseJUnit4ClassRunner.class)
public class CurrencyFormatterTest { public class CurrencyFormatterTest {
@Rule @Rule
public NativeLibraryTestRule mActivityTestRule = new NativeLibraryTestRule(); public ChromeBrowserTestRule mActivityTestRule = new ChromeBrowserTestRule();
/** /**
* Unicode non-breaking space. * Unicode non-breaking space.
...@@ -34,11 +33,6 @@ public class CurrencyFormatterTest { ...@@ -34,11 +33,6 @@ public class CurrencyFormatterTest {
private static final String NBSP = "\u00A0"; private static final String NBSP = "\u00A0";
private static final String NarrowNBSP = "\u202F"; private static final String NarrowNBSP = "\u202F";
@Before
public void setUp() {
mActivityTestRule.loadNativeLibraryAndInitBrowserProcess();
}
private static String longStringOfLength(int len) { private static String longStringOfLength(int len) {
StringBuilder currency = new StringBuilder(); StringBuilder currency = new StringBuilder();
for (int i = 0; i < len; i++) { for (int i = 0; i < len; i++) {
......
...@@ -34,6 +34,7 @@ public class OAuth2TokenServiceTest { ...@@ -34,6 +34,7 @@ public class OAuth2TokenServiceTest {
@Rule @Rule
public AccountManagerTestRule mAccountManagerTestRule = new AccountManagerTestRule(); public AccountManagerTestRule mAccountManagerTestRule = new AccountManagerTestRule();
private OAuth2TokenService mOAuth2TokenService;
/** /**
* Class handling GetAccessToken callbacks and providing a blocking {@link * Class handling GetAccessToken callbacks and providing a blocking {@link
...@@ -72,6 +73,8 @@ public class OAuth2TokenServiceTest { ...@@ -72,6 +73,8 @@ public class OAuth2TokenServiceTest {
@Before @Before
public void setUp() { public void setUp() {
mContext = new AdvancedMockContext(InstrumentationRegistry.getTargetContext()); mContext = new AdvancedMockContext(InstrumentationRegistry.getTargetContext());
mOAuth2TokenService = new OAuth2TokenService(0 /*nativeOAuth2TokenServiceDelegate*/,
null /* AccountTrackerService */, AccountManagerFacade.get());
} }
@After @After
...@@ -95,7 +98,7 @@ public class OAuth2TokenServiceTest { ...@@ -95,7 +98,7 @@ public class OAuth2TokenServiceTest {
AccountHolder accountHolder1 = AccountHolder.builder(account1).build(); AccountHolder accountHolder1 = AccountHolder.builder(account1).build();
mAccountManagerTestRule.addAccount(accountHolder1); mAccountManagerTestRule.addAccount(accountHolder1);
String[] sysAccounts = OAuth2TokenService.getSystemAccountNames(); String[] sysAccounts = mOAuth2TokenService.getSystemAccountNames();
Assert.assertEquals("There should be one registered account", 1, sysAccounts.length); Assert.assertEquals("There should be one registered account", 1, sysAccounts.length);
Assert.assertEquals("The account should be " + account1, account1.name, sysAccounts[0]); Assert.assertEquals("The account should be " + account1, account1.name, sysAccounts[0]);
...@@ -114,7 +117,7 @@ public class OAuth2TokenServiceTest { ...@@ -114,7 +117,7 @@ public class OAuth2TokenServiceTest {
AccountHolder accountHolder2 = AccountHolder.builder(account2).build(); AccountHolder accountHolder2 = AccountHolder.builder(account2).build();
mAccountManagerTestRule.addAccount(accountHolder2); mAccountManagerTestRule.addAccount(accountHolder2);
String[] sysAccounts = OAuth2TokenService.getSystemAccountNames(); String[] sysAccounts = mOAuth2TokenService.getSystemAccountNames();
Assert.assertEquals("There should be one registered account", 2, sysAccounts.length); Assert.assertEquals("There should be one registered account", 2, sysAccounts.length);
Assert.assertTrue("The list should contain " + account1, Assert.assertTrue("The list should contain " + account1,
Arrays.asList(sysAccounts).contains(account1.name)); Arrays.asList(sysAccounts).contains(account1.name));
...@@ -155,7 +158,7 @@ public class OAuth2TokenServiceTest { ...@@ -155,7 +158,7 @@ public class OAuth2TokenServiceTest {
mAccountManagerTestRule.addAccount(accountHolder); mAccountManagerTestRule.addAccount(accountHolder);
GetAccessTokenCallbackForTest callback = new GetAccessTokenCallbackForTest(); GetAccessTokenCallbackForTest callback = new GetAccessTokenCallbackForTest();
TestThreadUtils.runOnUiThreadBlocking( TestThreadUtils.runOnUiThreadBlocking(
() -> { OAuth2TokenService.getAccessToken(account, scope, callback); }); () -> { mOAuth2TokenService.getAccessToken(account, scope, callback); });
Assert.assertEquals(expectedToken, callback.getToken()); Assert.assertEquals(expectedToken, callback.getToken());
} }
} }
...@@ -6,6 +6,7 @@ import("//build/config/android/rules.gni") ...@@ -6,6 +6,7 @@ import("//build/config/android/rules.gni")
generate_jni("jni_headers") { generate_jni("jni_headers") {
sources = [ sources = [
"java/src/org/chromium/components/signin/AccountManagerFacade.java",
"java/src/org/chromium/components/signin/AccountTrackerService.java", "java/src/org/chromium/components/signin/AccountTrackerService.java",
"java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java", "java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java",
"java/src/org/chromium/components/signin/ConsistencyCookieManager.java", "java/src/org/chromium/components/signin/ConsistencyCookieManager.java",
......
...@@ -29,6 +29,7 @@ import org.chromium.base.Log; ...@@ -29,6 +29,7 @@ import org.chromium.base.Log;
import org.chromium.base.ObserverList; import org.chromium.base.ObserverList;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.metrics.CachedMetrics; import org.chromium.base.metrics.CachedMetrics;
import org.chromium.base.task.AsyncTask; import org.chromium.base.task.AsyncTask;
import org.chromium.components.signin.util.PatternMatcher; import org.chromium.components.signin.util.PatternMatcher;
...@@ -165,6 +166,7 @@ public class AccountManagerFacade { ...@@ -165,6 +166,7 @@ public class AccountManagerFacade {
* @return a singleton instance * @return a singleton instance
*/ */
@AnyThread @AnyThread
@CalledByNative
public static AccountManagerFacade get() { public static AccountManagerFacade get() {
AccountManagerFacade instance = sAtomicInstance.get(); AccountManagerFacade instance = sAtomicInstance.get();
assert instance != null : "AccountManagerFacade is not initialized!"; assert instance != null : "AccountManagerFacade is not initialized!";
......
...@@ -65,32 +65,40 @@ public final class OAuth2TokenService ...@@ -65,32 +65,40 @@ public final class OAuth2TokenService
private final long mNativeOAuth2TokenServiceDelegate; private final long mNativeOAuth2TokenServiceDelegate;
private final AccountTrackerService mAccountTrackerService; private final AccountTrackerService mAccountTrackerService;
private final AccountManagerFacade mAccountManagerFacade;
private boolean mPendingUpdate; private boolean mPendingUpdate;
private OAuth2TokenService( @VisibleForTesting
long nativeOAuth2TokenServiceDelegate, AccountTrackerService accountTrackerService) { public OAuth2TokenService(long nativeOAuth2TokenServiceDelegate,
AccountTrackerService accountTrackerService,
AccountManagerFacade accountManagerFacade) {
mNativeOAuth2TokenServiceDelegate = nativeOAuth2TokenServiceDelegate; mNativeOAuth2TokenServiceDelegate = nativeOAuth2TokenServiceDelegate;
mAccountTrackerService = accountTrackerService; mAccountTrackerService = accountTrackerService;
mAccountManagerFacade = accountManagerFacade;
mAccountTrackerService.addSystemAccountsSeededListener(this); // AccountTrackerService might be null in tests.
if (mAccountTrackerService != null) {
mAccountTrackerService.addSystemAccountsSeededListener(this);
}
} }
@CalledByNative @CalledByNative
private static OAuth2TokenService create( private static OAuth2TokenService create(long nativeOAuth2TokenServiceDelegate,
long nativeOAuth2TokenServiceDelegate, AccountTrackerService accountTrackerService) { AccountTrackerService accountTrackerService,
ThreadUtils.assertOnUiThread(); AccountManagerFacade accountManagerFacade) {
return new OAuth2TokenService(nativeOAuth2TokenServiceDelegate, accountTrackerService); assert nativeOAuth2TokenServiceDelegate != 0;
return new OAuth2TokenService(
nativeOAuth2TokenServiceDelegate, accountTrackerService, accountManagerFacade);
} }
private static Account getAccountOrNullFromUsername(String username) { private Account getAccountOrNullFromUsername(String username) {
if (username == null) { if (username == null) {
Log.e(TAG, "Username is null"); Log.e(TAG, "Username is null");
return null; return null;
} }
AccountManagerFacade accountManagerFacade = AccountManagerFacade.get(); Account account = mAccountManagerFacade.getAccountFromName(username);
Account account = accountManagerFacade.getAccountFromName(username);
if (account == null) { if (account == null) {
Log.e(TAG, "Account not found for provided username."); Log.e(TAG, "Account not found for provided username.");
return null; return null;
...@@ -103,11 +111,11 @@ public final class OAuth2TokenService ...@@ -103,11 +111,11 @@ public final class OAuth2TokenService
*/ */
@VisibleForTesting @VisibleForTesting
@CalledByNative @CalledByNative
public static String[] getSystemAccountNames() { public String[] getSystemAccountNames() {
// TODO(https://crbug.com/768366): Remove this after adding cache to account manager facade. // TODO(https://crbug.com/768366): Remove this after adding cache to account manager facade.
// This function is called by native code on UI thread. // This function is called by native code on UI thread.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) { try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
List<String> accountNames = AccountManagerFacade.get().tryGetGoogleAccountNames(); List<String> accountNames = mAccountManagerFacade.tryGetGoogleAccountNames();
return accountNames.toArray(new String[accountNames.size()]); return accountNames.toArray(new String[accountNames.size()]);
} }
} }
...@@ -118,6 +126,7 @@ public final class OAuth2TokenService ...@@ -118,6 +126,7 @@ public final class OAuth2TokenService
* from the OS. updateAccountList should be called to keep these two * from the OS. updateAccountList should be called to keep these two
* in sync. * in sync.
*/ */
@VisibleForTesting
@CalledByNative @CalledByNative
public static String[] getAccounts() { public static String[] getAccounts() {
return getStoredAccounts(); return getStoredAccounts();
...@@ -131,7 +140,7 @@ public final class OAuth2TokenService ...@@ -131,7 +140,7 @@ public final class OAuth2TokenService
*/ */
@MainThread @MainThread
@CalledByNative @CalledByNative
private static void getAccessTokenFromNative( private void getAccessTokenFromNative(
String username, String scope, final long nativeCallback) { String username, String scope, final long nativeCallback) {
Account account = getAccountOrNullFromUsername(username); Account account = getAccountOrNullFromUsername(username);
if (account == null) { if (account == null) {
...@@ -163,12 +172,11 @@ public final class OAuth2TokenService ...@@ -163,12 +172,11 @@ public final class OAuth2TokenService
* @param callback called on successful and unsuccessful fetching of auth token. * @param callback called on successful and unsuccessful fetching of auth token.
*/ */
@MainThread @MainThread
public static void getAccessToken( public void getAccessToken(Account account, String scope, GetAccessTokenCallback callback) {
Account account, String scope, GetAccessTokenCallback callback) {
ConnectionRetry.runAuthTask(new AuthTask<String>() { ConnectionRetry.runAuthTask(new AuthTask<String>() {
@Override @Override
public String run() throws AuthException { public String run() throws AuthException {
return AccountManagerFacade.get().getAccessToken(account, scope); return mAccountManagerFacade.getAccessToken(account, scope);
} }
@Override @Override
public void onSuccess(String token) { public void onSuccess(String token) {
...@@ -187,14 +195,14 @@ public final class OAuth2TokenService ...@@ -187,14 +195,14 @@ public final class OAuth2TokenService
*/ */
@MainThread @MainThread
@CalledByNative @CalledByNative
public static void invalidateAccessToken(String accessToken) { public void invalidateAccessToken(String accessToken) {
if (TextUtils.isEmpty(accessToken)) { if (TextUtils.isEmpty(accessToken)) {
return; return;
} }
ConnectionRetry.runAuthTask(new AuthTask<Boolean>() { ConnectionRetry.runAuthTask(new AuthTask<Boolean>() {
@Override @Override
public Boolean run() throws AuthException { public Boolean run() throws AuthException {
AccountManagerFacade.get().invalidateAccessToken(accessToken); mAccountManagerFacade.invalidateAccessToken(accessToken);
return true; return true;
} }
@Override @Override
...@@ -238,8 +246,8 @@ public final class OAuth2TokenService ...@@ -238,8 +246,8 @@ public final class OAuth2TokenService
* Called by native to check whether the account has an OAuth2 refresh token. * Called by native to check whether the account has an OAuth2 refresh token.
*/ */
@CalledByNative @CalledByNative
public static boolean hasOAuth2RefreshToken(String accountName) { private boolean hasOAuth2RefreshToken(String accountName) {
if (!AccountManagerFacade.get().isCachePopulated()) { if (!mAccountManagerFacade.isCachePopulated()) {
return false; return false;
} }
...@@ -304,12 +312,12 @@ public final class OAuth2TokenService ...@@ -304,12 +312,12 @@ public final class OAuth2TokenService
return accounts == null ? new String[] {} : accounts.toArray(new String[0]); return accounts == null ? new String[] {} : accounts.toArray(new String[0]);
} }
@CalledByNative
/** /**
* Called by native to save the account IDs that have associated OAuth2 refresh tokens. * Called by native to save the account IDs that have associated OAuth2 refresh tokens.
* This is called during updateAccountList to sync with getSystemAccountNames. * This is called during updateAccountList to sync with getSystemAccountNames.
* @param accounts IDs to save. * @param accounts IDs to save.
*/ */
@CalledByNative
private static void setAccounts(String[] accounts) { private static void setAccounts(String[] accounts) {
Set<String> set = new HashSet<>(Arrays.asList(accounts)); Set<String> set = new HashSet<>(Arrays.asList(accounts));
ContextUtils.getAppSharedPreferences() ContextUtils.getAppSharedPreferences()
......
# Copyright 2019 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# TODO(crbug.com/986435) Remove this target when IdentityServicesProvider
# will provide AccountManagerFacade.
source_set("base") {
if (is_android) {
sources = [
"account_manager_facade_android.cc",
"account_manager_facade_android.h",
]
deps = [
"//base",
"//components/signin/core/browser/android:jni_headers",
]
}
}
include_rules = [
]
specific_include_rules = {
"account_manager_facade_android.cc": [
"+components/signin/core/browser/android/jni_headers/AccountManagerFacade_jni.h",
],
}
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/signin/internal/base/account_manager_facade_android.h"
#include "components/signin/core/browser/android/jni_headers/AccountManagerFacade_jni.h"
base::android::ScopedJavaLocalRef<jobject>
AccountManagerFacadeAndroid::GetJavaObject() {
return Java_AccountManagerFacade_get(base::android::AttachCurrentThread());
}
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_SIGNIN_INTERNAL_BASE_ACCOUNT_MANAGER_FACADE_ANDROID_H_
#define COMPONENTS_SIGNIN_INTERNAL_BASE_ACCOUNT_MANAGER_FACADE_ANDROID_H_
#include "base/android/scoped_java_ref.h"
// Simple accessor to java's AccountManagerFacade
class AccountManagerFacadeAndroid {
public:
// Returns a reference to the corresponding Java AccountManagerFacade object.
// TODO(crbug.com/986435) Remove direct access to AccountManagerFacade.get
// from C++ when IdentityServicesProvider will handle its instance management.
static base::android::ScopedJavaLocalRef<jobject> GetJavaObject();
};
#endif // COMPONENTS_SIGNIN_INTERNAL_BASE_ACCOUNT_MANAGER_FACADE_ANDROID_H_
...@@ -70,7 +70,10 @@ source_set("identity_manager") { ...@@ -70,7 +70,10 @@ source_set("identity_manager") {
] ]
if (is_android) { if (is_android) {
deps += [ "//components/signin/core/browser/android:jni_headers" ] deps += [
"//components/signin/core/browser/android:jni_headers",
"//components/signin/internal/base",
]
} else { } else {
sources += [ sources += [
"mutable_profile_oauth2_token_service_delegate.cc", "mutable_profile_oauth2_token_service_delegate.cc",
......
include_rules = [ include_rules = [
"+chromeos/constants/chromeos_features.h", "+chromeos/constants/chromeos_features.h",
"+components/signin/internal/base",
"+components/signin/public/base", "+components/signin/public/base",
"+components/signin/public/identity_manager", "+components/signin/public/identity_manager",
"+components/signin/public/webdata", "+components/signin/public/webdata",
......
...@@ -41,8 +41,10 @@ typedef base::Callback< ...@@ -41,8 +41,10 @@ typedef base::Callback<
class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher { class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher {
public: public:
AndroidAccessTokenFetcher(OAuth2AccessTokenConsumer* consumer, AndroidAccessTokenFetcher(
const std::string& account_id); OAuth2TokenServiceDelegateAndroid* oauth2_token_service_delegate,
OAuth2AccessTokenConsumer* consumer,
const std::string& account_id);
~AndroidAccessTokenFetcher() override; ~AndroidAccessTokenFetcher() override;
// Overrides from OAuth2AccessTokenFetcher: // Overrides from OAuth2AccessTokenFetcher:
...@@ -59,6 +61,7 @@ class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher { ...@@ -59,6 +61,7 @@ class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher {
private: private:
std::string CombineScopes(const std::vector<std::string>& scopes); std::string CombineScopes(const std::vector<std::string>& scopes);
OAuth2TokenServiceDelegateAndroid* oauth2_token_service_delegate_;
std::string account_id_; std::string account_id_;
bool request_was_cancelled_; bool request_was_cancelled_;
base::WeakPtrFactory<AndroidAccessTokenFetcher> weak_factory_; base::WeakPtrFactory<AndroidAccessTokenFetcher> weak_factory_;
...@@ -67,9 +70,11 @@ class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher { ...@@ -67,9 +70,11 @@ class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher {
}; };
AndroidAccessTokenFetcher::AndroidAccessTokenFetcher( AndroidAccessTokenFetcher::AndroidAccessTokenFetcher(
OAuth2TokenServiceDelegateAndroid* oauth2_token_service_delegate,
OAuth2AccessTokenConsumer* consumer, OAuth2AccessTokenConsumer* consumer,
const std::string& account_id) const std::string& account_id)
: OAuth2AccessTokenFetcher(consumer), : OAuth2AccessTokenFetcher(consumer),
oauth2_token_service_delegate_(oauth2_token_service_delegate),
account_id_(account_id), account_id_(account_id),
request_was_cancelled_(false), request_was_cancelled_(false),
weak_factory_(this) {} weak_factory_(this) {}
...@@ -91,7 +96,7 @@ void AndroidAccessTokenFetcher::Start(const std::string& client_id, ...@@ -91,7 +96,7 @@ void AndroidAccessTokenFetcher::Start(const std::string& client_id,
// Call into Java to get a new token. // Call into Java to get a new token.
Java_OAuth2TokenService_getAccessTokenFromNative( Java_OAuth2TokenService_getAccessTokenFromNative(
env, j_username, j_scope, env, oauth2_token_service_delegate_->GetJavaObject(), j_username, j_scope,
reinterpret_cast<intptr_t>(heap_callback.release())); reinterpret_cast<intptr_t>(heap_callback.release()));
} }
...@@ -132,19 +137,24 @@ std::string AndroidAccessTokenFetcher::CombineScopes( ...@@ -132,19 +137,24 @@ std::string AndroidAccessTokenFetcher::CombineScopes(
} // namespace } // namespace
// TODO(crbug.com/1009957) Remove disable_interation_with_system_accounts_
// from OAuth2TokenServiceDelegateAndroid
bool OAuth2TokenServiceDelegateAndroid:: bool OAuth2TokenServiceDelegateAndroid::
disable_interaction_with_system_accounts_ = false; disable_interaction_with_system_accounts_ = false;
OAuth2TokenServiceDelegateAndroid::OAuth2TokenServiceDelegateAndroid( OAuth2TokenServiceDelegateAndroid::OAuth2TokenServiceDelegateAndroid(
AccountTrackerService* account_tracker_service) AccountTrackerService* account_tracker_service,
const base::android::JavaRef<jobject>& account_manager_facade)
: account_tracker_service_(account_tracker_service), : account_tracker_service_(account_tracker_service),
fire_refresh_token_loaded_(RT_LOAD_NOT_START) { fire_refresh_token_loaded_(RT_LOAD_NOT_START) {
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::ctor"; DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::ctor";
DCHECK(account_tracker_service_); DCHECK(account_tracker_service_);
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
base::android::ScopedJavaLocalRef<jobject> local_java_ref = base::android::ScopedJavaLocalRef<jobject> local_java_ref =
Java_OAuth2TokenService_create(env, reinterpret_cast<intptr_t>(this), Java_OAuth2TokenService_create(env, reinterpret_cast<intptr_t>(this),
account_tracker_service_->GetJavaObject()); account_tracker_service_->GetJavaObject(),
account_manager_facade);
java_ref_.Reset(env, local_java_ref.obj()); java_ref_.Reset(env, local_java_ref.obj());
if (account_tracker_service_->GetMigrationState() == if (account_tracker_service_->GetMigrationState() ==
...@@ -173,6 +183,10 @@ ScopedJavaLocalRef<jobject> OAuth2TokenServiceDelegateAndroid::GetJavaObject() { ...@@ -173,6 +183,10 @@ ScopedJavaLocalRef<jobject> OAuth2TokenServiceDelegateAndroid::GetJavaObject() {
bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable( bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable(
const CoreAccountId& account_id) const { const CoreAccountId& account_id) const {
DCHECK(!disable_interaction_with_system_accounts_)
<< __FUNCTION__
<< " needs to interact with system accounts and cannot be used with "
"disable_interaction_with_system_accounts_";
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable" DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable"
<< " account= " << account_id; << " account= " << account_id;
std::string account_name = MapAccountIdToAccountName(account_id); std::string account_name = MapAccountIdToAccountName(account_id);
...@@ -187,7 +201,8 @@ bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable( ...@@ -187,7 +201,8 @@ bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable(
ScopedJavaLocalRef<jstring> j_account_id = ScopedJavaLocalRef<jstring> j_account_id =
ConvertUTF8ToJavaString(env, account_name); ConvertUTF8ToJavaString(env, account_name);
jboolean refresh_token_is_available = jboolean refresh_token_is_available =
Java_OAuth2TokenService_hasOAuth2RefreshToken(env, j_account_id); Java_OAuth2TokenService_hasOAuth2RefreshToken(env, java_ref_,
j_account_id);
return refresh_token_is_available == JNI_TRUE; return refresh_token_is_available == JNI_TRUE;
} }
...@@ -226,6 +241,7 @@ std::vector<CoreAccountId> OAuth2TokenServiceDelegateAndroid::GetAccounts() ...@@ -226,6 +241,7 @@ std::vector<CoreAccountId> OAuth2TokenServiceDelegateAndroid::GetAccounts()
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobjectArray> j_accounts = ScopedJavaLocalRef<jobjectArray> j_accounts =
Java_OAuth2TokenService_getAccounts(env); Java_OAuth2TokenService_getAccounts(env);
;
// TODO(fgorski): We may decide to filter out some of the accounts. // TODO(fgorski): We may decide to filter out some of the accounts.
base::android::AppendJavaStringArrayToStringVector(env, j_accounts, base::android::AppendJavaStringArrayToStringVector(env, j_accounts,
&accounts); &accounts);
...@@ -234,10 +250,14 @@ std::vector<CoreAccountId> OAuth2TokenServiceDelegateAndroid::GetAccounts() ...@@ -234,10 +250,14 @@ std::vector<CoreAccountId> OAuth2TokenServiceDelegateAndroid::GetAccounts()
std::vector<std::string> std::vector<std::string>
OAuth2TokenServiceDelegateAndroid::GetSystemAccountNames() { OAuth2TokenServiceDelegateAndroid::GetSystemAccountNames() {
DCHECK(!disable_interaction_with_system_accounts_)
<< __FUNCTION__
<< " needs to interact with system accounts and cannot be used with "
"disable_interaction_with_system_accounts_";
std::vector<std::string> account_names; std::vector<std::string> account_names;
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobjectArray> j_accounts = ScopedJavaLocalRef<jobjectArray> j_accounts =
Java_OAuth2TokenService_getSystemAccountNames(env); Java_OAuth2TokenService_getSystemAccountNames(env, java_ref_);
base::android::AppendJavaStringArrayToStringVector(env, j_accounts, base::android::AppendJavaStringArrayToStringVector(env, j_accounts,
&account_names); &account_names);
return account_names; return account_names;
...@@ -284,7 +304,8 @@ OAuth2TokenServiceDelegateAndroid::CreateAccessTokenFetcher( ...@@ -284,7 +304,8 @@ OAuth2TokenServiceDelegateAndroid::CreateAccessTokenFetcher(
std::string account_name = MapAccountIdToAccountName(account_id); std::string account_name = MapAccountIdToAccountName(account_id);
DCHECK(!account_name.empty()) DCHECK(!account_name.empty())
<< "Cannot find account name for account id " << account_id; << "Cannot find account name for account id " << account_id;
return std::make_unique<AndroidAccessTokenFetcher>(consumer, account_name); return std::make_unique<AndroidAccessTokenFetcher>(this, consumer,
account_name);
} }
void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated( void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated(
...@@ -292,11 +313,15 @@ void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated( ...@@ -292,11 +313,15 @@ void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated(
const std::string& client_id, const std::string& client_id,
const OAuth2AccessTokenManager::ScopeSet& scopes, const OAuth2AccessTokenManager::ScopeSet& scopes,
const std::string& access_token) { const std::string& access_token) {
DCHECK(!disable_interaction_with_system_accounts_)
<< __FUNCTION__
<< " needs to interact with system accounts and cannot be used with "
"disable_interaction_with_system_accounts_";
ValidateAccountId(account_id); ValidateAccountId(account_id);
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jstring> j_access_token = ScopedJavaLocalRef<jstring> j_access_token =
ConvertUTF8ToJavaString(env, access_token); ConvertUTF8ToJavaString(env, access_token);
Java_OAuth2TokenService_invalidateAccessToken(env, j_access_token); Java_OAuth2TokenService_invalidateAccessToken(env, java_ref_, j_access_token);
} }
void OAuth2TokenServiceDelegateAndroid::UpdateAccountList( void OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
......
...@@ -31,7 +31,8 @@ class OAuth2TokenServiceDelegateAndroid ...@@ -31,7 +31,8 @@ class OAuth2TokenServiceDelegateAndroid
: public ProfileOAuth2TokenServiceDelegate { : public ProfileOAuth2TokenServiceDelegate {
public: public:
OAuth2TokenServiceDelegateAndroid( OAuth2TokenServiceDelegateAndroid(
AccountTrackerService* account_tracker_service); AccountTrackerService* account_tracker_service,
const base::android::JavaRef<jobject>& account_manager_facade);
~OAuth2TokenServiceDelegateAndroid() override; ~OAuth2TokenServiceDelegateAndroid() override;
// Creates a new instance of the OAuth2TokenServiceDelegateAndroid. // Creates a new instance of the OAuth2TokenServiceDelegateAndroid.
...@@ -41,12 +42,18 @@ class OAuth2TokenServiceDelegateAndroid ...@@ -41,12 +42,18 @@ class OAuth2TokenServiceDelegateAndroid
base::android::ScopedJavaLocalRef<jobject> GetJavaObject() override; base::android::ScopedJavaLocalRef<jobject> GetJavaObject() override;
// Called by the TestingProfile class to disable account validation in // Called by the TestingProfile class to disable account validation in
// tests. This prevents the token service from trying to look up system // tests. This prevents the token service from building the java objects
// accounts which requires special permission. // which require prior initialization (AccountManagerFacade)
// TODO(crbug.com/1009957) Remove disable_interation_with_system_accounts_
// from OAuth2TokenServiceDelegateAndroid
static void set_disable_interaction_with_system_accounts() { static void set_disable_interaction_with_system_accounts() {
disable_interaction_with_system_accounts_ = true; disable_interaction_with_system_accounts_ = true;
} }
static bool get_disable_interaction_with_system_accounts() {
return disable_interaction_with_system_accounts_;
}
// ProfileOAuth2TokenServiceDelegate overrides: // ProfileOAuth2TokenServiceDelegate overrides:
bool RefreshTokenIsAvailable(const CoreAccountId& account_id) const override; bool RefreshTokenIsAvailable(const CoreAccountId& account_id) const override;
GoogleServiceAuthError GetAuthError( GoogleServiceAuthError GetAuthError(
...@@ -132,6 +139,10 @@ class OAuth2TokenServiceDelegateAndroid ...@@ -132,6 +139,10 @@ class OAuth2TokenServiceDelegateAndroid
RefreshTokenLoadStatus fire_refresh_token_loaded_; RefreshTokenLoadStatus fire_refresh_token_loaded_;
base::Time last_update_accounts_time_; base::Time last_update_accounts_time_;
// For testing, disables the creation of the java counterpart, see
// set_disable_interaction_with_system_accounts().
// TODO(crbug.com/1009957) Remove disable_interation_with_system_accounts_
// from OAuth2TokenServiceDelegateAndroid
static bool disable_interaction_with_system_accounts_; static bool disable_interaction_with_system_accounts_;
DISALLOW_COPY_AND_ASSIGN(OAuth2TokenServiceDelegateAndroid); DISALLOW_COPY_AND_ASSIGN(OAuth2TokenServiceDelegateAndroid);
......
...@@ -23,7 +23,7 @@ class OAuth2TokenServiceDelegateAndroidForTest ...@@ -23,7 +23,7 @@ class OAuth2TokenServiceDelegateAndroidForTest
public: public:
OAuth2TokenServiceDelegateAndroidForTest( OAuth2TokenServiceDelegateAndroidForTest(
AccountTrackerService* account_tracker_service) AccountTrackerService* account_tracker_service)
: OAuth2TokenServiceDelegateAndroid(account_tracker_service) {} : OAuth2TokenServiceDelegateAndroid(account_tracker_service, nullptr) {}
MOCK_METHOD1(SetAccounts, void(const std::vector<CoreAccountId>&)); MOCK_METHOD1(SetAccounts, void(const std::vector<CoreAccountId>&));
}; };
...@@ -45,8 +45,6 @@ class OAuth2TokenServiceDelegateAndroidTest : public testing::Test { ...@@ -45,8 +45,6 @@ class OAuth2TokenServiceDelegateAndroidTest : public testing::Test {
testing::Test::SetUp(); testing::Test::SetUp();
AccountTrackerService::RegisterPrefs(pref_service_.registry()); AccountTrackerService::RegisterPrefs(pref_service_.registry());
account_tracker_service_.Initialize(&pref_service_, base::FilePath()); account_tracker_service_.Initialize(&pref_service_, base::FilePath());
OAuth2TokenServiceDelegateAndroid::
set_disable_interaction_with_system_accounts();
delegate_ = std::make_unique<OAuth2TokenServiceDelegateAndroidForTest>( delegate_ = std::make_unique<OAuth2TokenServiceDelegateAndroidForTest>(
&account_tracker_service_); &account_tracker_service_);
delegate_->AddObserver(&observer_); delegate_->AddObserver(&observer_);
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "components/signin/public/base/signin_client.h" #include "components/signin/public/base/signin_client.h"
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "components/signin/internal/base/account_manager_facade_android.h"
#include "components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h" #include "components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h"
#else #else
#include "components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h" #include "components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h"
...@@ -35,10 +36,17 @@ ...@@ -35,10 +36,17 @@
namespace { namespace {
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// TODO(crbug.com/986435) Provide AccountManagerFacade as a parameter once
// IdentityServicesProvider owns its instance management.
std::unique_ptr<OAuth2TokenServiceDelegateAndroid> CreateAndroidOAuthDelegate( std::unique_ptr<OAuth2TokenServiceDelegateAndroid> CreateAndroidOAuthDelegate(
AccountTrackerService* account_tracker_service) { AccountTrackerService* account_tracker_service) {
auto account_manager_facade =
OAuth2TokenServiceDelegateAndroid::
get_disable_interaction_with_system_accounts()
? nullptr
: AccountManagerFacadeAndroid::GetJavaObject();
return std::make_unique<OAuth2TokenServiceDelegateAndroid>( return std::make_unique<OAuth2TokenServiceDelegateAndroid>(
account_tracker_service); account_tracker_service, account_manager_facade);
} }
#elif defined(OS_IOS) #elif defined(OS_IOS)
std::unique_ptr<ProfileOAuth2TokenServiceIOSDelegate> CreateIOSOAuthDelegate( std::unique_ptr<ProfileOAuth2TokenServiceIOSDelegate> CreateIOSOAuthDelegate(
......
...@@ -164,7 +164,8 @@ void UpdatePersistentErrorOfRefreshTokenForAccount( ...@@ -164,7 +164,8 @@ void UpdatePersistentErrorOfRefreshTokenForAccount(
void DisableAccessTokenFetchRetries(IdentityManager* identity_manager); void DisableAccessTokenFetchRetries(IdentityManager* identity_manager);
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// Disables interaction with system accounts, which requires special permission. // Disables interaction with system accounts, which requires special
// initialization of the java subsystems (AccountManagerFacade).
void DisableInteractionWithSystemAccounts(); void DisableInteractionWithSystemAccounts();
#endif #endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment