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

[Signin][Android] Move account seeding to SigninManager.java

This moves the initial account seeding and reload from
oauth2_token_service_delegate_android.*'s construction to
SigninManager.java's. This also removes direct calls to
OAuth2TokenService.java updateAccountList, using IdentityMutator
instead.

Bug: 934688
Change-Id: I02dd5932740f23cce2187ea367ca65282f91cf4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1751188
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Auto-Submit: Pâris Meuleman <pmeuleman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709547}
parent 7900d1c6
......@@ -424,7 +424,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/share/ShareMenuActionHandlerIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/share/ShareMenuActionHandlerTest.java",
"javatests/src/org/chromium/chrome/browser/share/ShareUrlTest.java",
"javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/signin/IdentityManagerIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java",
"javatests/src/org/chromium/chrome/browser/signin/ProfileDataCacheRenderTest.java",
"javatests/src/org/chromium/chrome/browser/signin/SigninHelperTest.java",
......
......@@ -185,7 +185,7 @@ public class SigninHelper {
if (accountsChanged) {
// Account details have changed so inform the token service that credentials
// should now be available.
mOAuth2TokenService.updateAccountList();
mSigninManager.reloadAllAccountsFromSystem();
}
}
......
......@@ -232,6 +232,8 @@ public class SigninManager
mAccountTrackerService.addSystemAccountsSeededListener(this);
mIdentityManager.addObserver(this);
reloadAllAccountsFromSystem();
}
/**
......@@ -478,9 +480,8 @@ public class SigninManager
mSignInState.mCallback.onSignInComplete();
}
// Trigger token requests via native.
mIdentityMutator.reloadAllAccountsFromSystemWithPrimaryAccount(
mSignInState.mCoreAccountInfo.getId());
// Trigger token requests via identity mutator.
reloadAllAccountsFromSystem();
RecordUserAction.record("Signin_Signin_Succeed");
logSigninCompleteAccessPoint();
......@@ -587,6 +588,14 @@ public class SigninManager
return SigninManagerJni.get().getManagementDomain(mNativeSigninManagerAndroid);
}
/**
* Reloads accounts from system within IdentityManager.
*/
void reloadAllAccountsFromSystem() {
mIdentityMutator.reloadAllAccountsFromSystemWithPrimaryAccount(
mIdentityManager.getPrimaryAccountId());
}
/**
* Aborts the current sign in.
*
......@@ -704,6 +713,11 @@ public class SigninManager
}
}
@VisibleForTesting
IdentityMutator getIdentityMutator() {
return mIdentityMutator;
}
// Native methods.
@NativeMethods
interface Natives {
......
......@@ -78,6 +78,7 @@ public class SigninManagerTest {
AndroidSyncSettings androidSyncSettings = mock(AndroidSyncSettings.class);
doReturn(null).when(mIdentityManager).getPrimaryAccountId();
mSigninManager = new SigninManager(0 /* nativeSigninManagerAndroid */,
mAccountTrackerService, mIdentityManager, mIdentityMutator, androidSyncSettings);
......@@ -224,6 +225,7 @@ public class SigninManagerTest {
.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(any());
doReturn(false).when(mIdentityManager).hasPrimaryAccount();
doReturn(true).when(mIdentityMutator).setPrimaryAccount(any());
doReturn(account.getId()).when(mIdentityManager).getPrimaryAccountId();
doNothing().when(mIdentityMutator).reloadAllAccountsFromSystemWithPrimaryAccount(any());
mSigninManager.onFirstRunCheckDone(); // Allow sign-in.
......
......@@ -68,6 +68,9 @@ public final class OAuth2TokenService
private final AccountManagerFacade mAccountManagerFacade;
private boolean mPendingUpdate;
// TODO(crbug.com/934688) Once OAuth2TokenService.java is internalized, use CoreAccountId
// instead of String.
private String mPendingUpdateAccountId;
@VisibleForTesting
public OAuth2TokenService(long nativeOAuth2TokenServiceDelegate,
......@@ -285,44 +288,28 @@ public final class OAuth2TokenService
@Override
public void onSystemAccountsSeedingComplete() {
if (mPendingUpdate) {
updateAccountListInternal();
reloadAllAccountsWithPrimaryAccountAfterSeeding(mPendingUpdateAccountId);
mPendingUpdate = false;
mPendingUpdateAccountId = null;
}
}
@CalledByNative
public void updateAccountList() {
private void seedAndReloadAccountsWithPrimaryAccount(@Nullable String accountId) {
ThreadUtils.assertOnUiThread();
if (!mAccountTrackerService.checkAndSeedSystemAccounts()) {
assert !mPendingUpdate && mPendingUpdateAccountId == null;
mPendingUpdate = true;
mPendingUpdateAccountId = accountId;
return;
}
updateAccountListInternal();
reloadAllAccountsWithPrimaryAccountAfterSeeding(accountId);
}
private void updateAccountListInternal() {
String currentlySignedInAccount = ChromeSigninController.get().getSignedInAccountName();
if (currentlySignedInAccount != null
&& isSignedInAccountChanged(currentlySignedInAccount)) {
// Set currentlySignedInAccount to null for validation if signed-in account was changed
// (renamed or removed from the device), this will cause all credentials in token
// service be revoked.
// Could only get here during Chrome cold startup.
// After chrome started, SigninHelper and AccountsChangedReceiver will handle account
// change (re-signin or sign out signed-in account).
currentlySignedInAccount = null;
}
OAuth2TokenServiceJni.get().updateAccountList(mNativeOAuth2TokenServiceDelegate,
OAuth2TokenService.this, currentlySignedInAccount);
}
private boolean isSignedInAccountChanged(String signedInAccountName) {
String[] accountNames = getSystemAccountNames();
for (String accountName : accountNames) {
if (accountName.equals(signedInAccountName)) return false;
}
return true;
private void reloadAllAccountsWithPrimaryAccountAfterSeeding(@Nullable String accountId) {
OAuth2TokenServiceJni.get().reloadAllAccountsWithPrimaryAccountAfterSeeding(
mNativeOAuth2TokenServiceDelegate, accountId);
}
private static String[] getStoredAccounts() {
......@@ -427,7 +414,7 @@ public final class OAuth2TokenService
interface Natives {
void onOAuth2TokenFetched(
String authToken, boolean isTransientError, long nativeCallback);
void updateAccountList(long nativeOAuth2TokenServiceDelegateAndroid,
OAuth2TokenService caller, String currentlySignedInAccount);
void reloadAllAccountsWithPrimaryAccountAfterSeeding(
long nativeOAuth2TokenServiceDelegateAndroid, @Nullable String accountId);
}
}
......@@ -20,7 +20,7 @@ DeviceAccountsSynchronizerImpl::~DeviceAccountsSynchronizerImpl() = default;
#if defined(OS_ANDROID)
void DeviceAccountsSynchronizerImpl::
ReloadAllAccountsFromSystemWithPrimaryAccount(
const CoreAccountId& primary_account_id) {
const base::Optional<CoreAccountId>& primary_account_id) {
token_service_delegate_->ReloadAllAccountsFromSystemWithPrimaryAccount(
primary_account_id);
}
......
......@@ -22,7 +22,7 @@ class DeviceAccountsSynchronizerImpl : public DeviceAccountsSynchronizer {
// DeviceAccountsSynchronizer implementation.
#if defined(OS_ANDROID)
void ReloadAllAccountsFromSystemWithPrimaryAccount(
const CoreAccountId& primary_account_id) override;
const base::Optional<CoreAccountId>& primary_account_id) override;
#endif
#if defined(OS_IOS)
......
......@@ -169,10 +169,6 @@ OAuth2TokenServiceDelegateAndroid::OAuth2TokenServiceDelegateAndroid(
}
SetAccounts(accounts_id);
}
if (!disable_interaction_with_system_accounts_) {
Java_OAuth2TokenService_updateAccountList(AttachCurrentThread(), java_ref_);
}
}
OAuth2TokenServiceDelegateAndroid::~OAuth2TokenServiceDelegateAndroid() {}
......@@ -324,32 +320,43 @@ void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated(
Java_OAuth2TokenService_invalidateAccessToken(env, java_ref_, j_access_token);
}
void OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
JNIEnv* env,
const JavaParamRef<jobject>& obj,
const JavaParamRef<jstring>& j_current_acc) {
std::string signed_in_account_name;
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList from java";
if (j_current_acc)
signed_in_account_name = ConvertJavaStringToUTF8(env, j_current_acc);
if (!signed_in_account_name.empty())
signed_in_account_name = gaia::CanonicalizeEmail(signed_in_account_name);
void OAuth2TokenServiceDelegateAndroid::
ReloadAllAccountsFromSystemWithPrimaryAccount(
const base::Optional<CoreAccountId>& primary_account_id) {
JNIEnv* env = AttachCurrentThread();
// Clear any auth errors so that client can retry to get access tokens.
errors_.clear();
ScopedJavaLocalRef<jstring> j_account_id =
primary_account_id.has_value()
? ConvertUTF8ToJavaString(env, primary_account_id->id)
: nullptr;
Java_OAuth2TokenService_seedAndReloadAccountsWithPrimaryAccount(
env, java_ref_, j_account_id);
}
UpdateAccountList(MapAccountNameToAccountId(signed_in_account_name),
GetValidAccounts(), GetSystemAccounts());
void OAuth2TokenServiceDelegateAndroid::
ReloadAllAccountsWithPrimaryAccountAfterSeeding(
JNIEnv* env,
const base::android::JavaParamRef<jstring>& account_id) {
base::Optional<CoreAccountId> core_account_id;
if (account_id) {
core_account_id = CoreAccountId();
core_account_id->id = ConvertJavaStringToUTF8(env, account_id);
}
UpdateAccountList(core_account_id, GetValidAccounts(), GetSystemAccounts());
}
void OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
const CoreAccountId& signed_in_account_id,
const base::Optional<CoreAccountId>& signed_in_account_id,
const std::vector<CoreAccountId>& prev_ids,
const std::vector<CoreAccountId>& curr_ids) {
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList:"
<< " sigined_in_account_id=" << signed_in_account_id
<< " sigined_in_account_id="
<< (signed_in_account_id.has_value() ? signed_in_account_id->id
: std::string())
<< " prev_ids=" << prev_ids.size()
<< " curr_ids=" << curr_ids.size();
// Clear any auth errors so that client can retry to get access tokens.
errors_.clear();
std::vector<CoreAccountId> refreshed_ids;
std::vector<CoreAccountId> revoked_ids;
......@@ -385,7 +392,7 @@ void OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
// in.
if (account_tracker_service_->GetMigrationState() ==
AccountTrackerService::MIGRATION_IN_PROGRESS &&
signed_in_account_id.empty()) {
!signed_in_account_id.has_value()) {
account_tracker_service_->SetMigrationDone();
}
......@@ -397,17 +404,18 @@ void OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
}
bool OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
const CoreAccountId& signed_in_id,
const base::Optional<CoreAccountId>& signed_in_id,
const std::vector<CoreAccountId>& prev_ids,
const std::vector<CoreAccountId>& curr_ids,
std::vector<CoreAccountId>* refreshed_ids,
std::vector<CoreAccountId>* revoked_ids) {
bool keep_accounts = base::FeatureList::IsEnabled(signin::kMiceFeature) ||
base::Contains(curr_ids, signed_in_id);
bool keep_accounts =
base::FeatureList::IsEnabled(signin::kMiceFeature) ||
(signed_in_id.has_value() && base::Contains(curr_ids, *signed_in_id));
if (keep_accounts) {
// Revoke token for ids that have been removed from the device.
for (const CoreAccountId& prev_id : prev_ids) {
if (prev_id == signed_in_id)
if (signed_in_id.has_value() && prev_id == *signed_in_id)
continue;
if (!base::Contains(curr_ids, prev_id)) {
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList:"
......@@ -416,28 +424,28 @@ bool OAuth2TokenServiceDelegateAndroid::UpdateAccountList(
}
}
if (!signed_in_id.empty()) {
if (signed_in_id.has_value()) {
// Always fire the primary signed in account first.
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList:"
<< "refreshed=" << signed_in_id;
refreshed_ids->push_back(signed_in_id);
<< "refreshed=" << *signed_in_id;
refreshed_ids->push_back(*signed_in_id);
}
for (const CoreAccountId& curr_id : curr_ids) {
if (curr_id == signed_in_id)
if (signed_in_id.has_value() && curr_id == *signed_in_id)
continue;
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList:"
<< "refreshed=" << curr_id;
refreshed_ids->push_back(curr_id);
}
} else {
// Revoke all ids.
if (base::Contains(prev_ids, signed_in_id)) {
// Revoke all ids with signed in account first.
if (signed_in_id.has_value() && base::Contains(prev_ids, *signed_in_id)) {
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList:"
<< "revoked=" << signed_in_id;
revoked_ids->push_back(signed_in_id);
<< "revoked=" << *signed_in_id;
revoked_ids->push_back(*signed_in_id);
}
for (const CoreAccountId& prev_id : prev_ids) {
if (prev_id == signed_in_id)
if (signed_in_id.has_value() && prev_id == *signed_in_id)
continue;
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAccountList:"
<< "revoked=" << prev_id;
......@@ -488,15 +496,6 @@ void OAuth2TokenServiceDelegateAndroid::LoadCredentials(
}
}
void OAuth2TokenServiceDelegateAndroid::
ReloadAllAccountsFromSystemWithPrimaryAccount(
const CoreAccountId& primary_account_id) {
// UpdateAccountList() effectively synchronizes the accounts in the Token
// Service with those present at the system level.
UpdateAccountList(primary_account_id, GetValidAccounts(),
GetSystemAccounts());
}
std::string OAuth2TokenServiceDelegateAndroid::MapAccountIdToAccountName(
const CoreAccountId& account_id) const {
return account_tracker_service_->GetAccountInfo(account_id).email;
......
......@@ -62,19 +62,6 @@ class OAuth2TokenServiceDelegateAndroid
const GoogleServiceAuthError& error) override;
std::vector<CoreAccountId> GetAccounts() const override;
void UpdateAccountList(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& current_account);
// Takes a the signed in sync account as well as all the other
// android account ids and check the token status of each.
// NOTE: TokenAvailable notifications will be sent for all accounts, even if
// they were already known. See https://crbug.com/939470 for details.
void UpdateAccountList(const CoreAccountId& signed_in_account_id,
const std::vector<CoreAccountId>& prev_ids,
const std::vector<CoreAccountId>& curr_ids);
// Overridden from ProfileOAuth2TokenService to complete signout of all
// POA2TService aware accounts.
void RevokeAllCredentials() override;
......@@ -82,7 +69,23 @@ class OAuth2TokenServiceDelegateAndroid
void LoadCredentials(const CoreAccountId& primary_account_id) override;
void ReloadAllAccountsFromSystemWithPrimaryAccount(
const CoreAccountId& primary_account_id) override;
const base::Optional<CoreAccountId>& primary_account_id) override;
// Resumes the reload of accounts once the account seeding is complete.
// TODO(crbug.com/934688) Once OAuth2TokenService.java is internalized, use
// CoreAccountId instead of String.
void ReloadAllAccountsWithPrimaryAccountAfterSeeding(
JNIEnv* env,
const base::android::JavaParamRef<jstring>& account_id);
// Takes a the signed in sync account as well as all the other
// android account ids and check the token status of each.
// NOTE: TokenAvailable notifications will be sent for all accounts, even if
// they were already known. See https://crbug.com/939470 for details.
void UpdateAccountList(
const base::Optional<CoreAccountId>& signed_in_account_id,
const std::vector<CoreAccountId>& prev_ids,
const std::vector<CoreAccountId>& curr_ids);
protected:
std::unique_ptr<OAuth2AccessTokenFetcher> CreateAccessTokenFetcher(
......@@ -115,7 +118,7 @@ class OAuth2TokenServiceDelegateAndroid
// Return whether accounts are valid and we have access to all the tokens in
// |curr_ids|.
bool UpdateAccountList(const CoreAccountId& signed_in_id,
bool UpdateAccountList(const base::Optional<CoreAccountId>& signed_in_id,
const std::vector<CoreAccountId>& prev_ids,
const std::vector<CoreAccountId>& curr_ids,
std::vector<CoreAccountId>* refreshed_ids,
......
......@@ -143,7 +143,7 @@ class ProfileOAuth2TokenServiceDelegate {
// Triggers platform specific implementation for Android to reload accounts
// from system.
virtual void ReloadAllAccountsFromSystemWithPrimaryAccount(
const CoreAccountId& primary_account_id) {}
const base::Optional<CoreAccountId>& primary_account_id) {}
#endif
// -----------------------------------------------------------------------
......
......@@ -104,6 +104,23 @@ public class IdentityManager {
return IdentityManagerJni.get().hasPrimaryAccount(mNativeIdentityManager);
}
/**
* Provides access to the core information of the user's primary account.
* Returns null if no such info is available, either because there
* is no primary account yet or because the user signed out.
*/
public @Nullable CoreAccountInfo getPrimaryAccountInfo() {
return IdentityManagerJni.get().getPrimaryAccountInfo(mNativeIdentityManager);
}
/**
* Provides access to the account ID of the user's primary account. Returns null if no such info
* is available.
*/
public @Nullable CoreAccountId getPrimaryAccountId() {
return IdentityManagerJni.get().getPrimaryAccountId(mNativeIdentityManager);
}
/**
* Looks up and returns information for account with given |email_address|. If the account
* cannot be found, return a null value.
......@@ -117,6 +134,8 @@ public class IdentityManager {
@NativeMethods
interface Natives {
public @Nullable CoreAccountInfo getPrimaryAccountInfo(long nativeIdentityManager);
public @Nullable CoreAccountId getPrimaryAccountId(long nativeIdentityManager);
public boolean hasPrimaryAccount(long nativeIdentityManager);
public @Nullable CoreAccountInfo
findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
......
......@@ -4,6 +4,8 @@
package org.chromium.components.signin.identitymanager;
import androidx.annotation.Nullable;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.components.signin.metrics.SignoutDelete;
......@@ -59,7 +61,7 @@ public class IdentityMutator {
* Reloads the accounts in the token service from the system accounts. This API calls
* ProfileOAuth2TokenServiceDelegate::ReloadAllAccountsFromSystemWithPrimaryAccount.
*/
public void reloadAllAccountsFromSystemWithPrimaryAccount(CoreAccountId accountId) {
public void reloadAllAccountsFromSystemWithPrimaryAccount(@Nullable CoreAccountId accountId) {
IdentityMutatorJni.get().reloadAllAccountsFromSystemWithPrimaryAccount(
mNativeIdentityMutator, accountId);
}
......@@ -71,6 +73,6 @@ public class IdentityMutator {
@ClearAccountsAction int action, @SignoutReason int sourceMetric,
@SignoutDelete int deleteMetric);
public void reloadAllAccountsFromSystemWithPrimaryAccount(
long nativeJniIdentityMutator, CoreAccountId accountId);
long nativeJniIdentityMutator, @Nullable CoreAccountId accountId);
}
}
......@@ -5,6 +5,7 @@
#ifndef COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_DEVICE_ACCOUNTS_SYNCHRONIZER_H_
#define COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_DEVICE_ACCOUNTS_SYNCHRONIZER_H_
#include "base/optional.h"
#include "build/build_config.h"
#include "google_apis/gaia/core_account_id.h"
......@@ -22,7 +23,7 @@ class DeviceAccountsSynchronizer {
// accounts will be visible in IdentityManager::GetAccountsWithRefreshTokens()
// with any persistent errors cleared after this method is called.
virtual void ReloadAllAccountsFromSystemWithPrimaryAccount(
const CoreAccountId& primary_account_id) = 0;
const base::Optional<CoreAccountId>& primary_account_id) = 0;
#endif
#if defined(OS_IOS)
......
......@@ -406,6 +406,20 @@ bool IdentityManager::HasPrimaryAccount(JNIEnv* env) const {
return HasPrimaryAccount();
}
base::android::ScopedJavaLocalRef<jobject>
IdentityManager::GetPrimaryAccountInfo(JNIEnv* env) const {
if (HasPrimaryAccount())
return ConvertToJavaCoreAccountInfo(env, GetPrimaryAccountInfo());
return nullptr;
}
base::android::ScopedJavaLocalRef<jobject> IdentityManager::GetPrimaryAccountId(
JNIEnv* env) const {
if (HasPrimaryAccount())
return ConvertToJavaCoreAccountId(env, GetPrimaryAccountId());
return nullptr;
}
base::android::ScopedJavaLocalRef<jobject> IdentityManager::
FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
JNIEnv* env,
......
......@@ -454,6 +454,12 @@ class IdentityManager : public KeyedService,
// Overloads for calls from java:
bool HasPrimaryAccount(JNIEnv* env) const;
base::android::ScopedJavaLocalRef<jobject> GetPrimaryAccountInfo(
JNIEnv* env) const;
base::android::ScopedJavaLocalRef<jobject> GetPrimaryAccountId(
JNIEnv* env) const;
base::android::ScopedJavaLocalRef<jobject>
FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
JNIEnv* env,
......
......@@ -46,12 +46,18 @@ bool JniIdentityMutator::ClearPrimaryAccount(JNIEnv* env,
void JniIdentityMutator::ReloadAllAccountsFromSystemWithPrimaryAccount(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& primary_account_id) {
const base::android::JavaParamRef<jobject>& j_primary_account_id) {
DeviceAccountsSynchronizer* device_accounts_synchronizer =
identity_mutator_->GetDeviceAccountsSynchronizer();
DCHECK(device_accounts_synchronizer);
base::Optional<CoreAccountId> primary_account_id;
if (j_primary_account_id) {
primary_account_id = CoreAccountId();
primary_account_id->id =
ConvertFromJavaCoreAccountId(env, j_primary_account_id);
}
device_accounts_synchronizer->ReloadAllAccountsFromSystemWithPrimaryAccount(
ConvertFromJavaCoreAccountId(env, primary_account_id));
primary_account_id);
}
#endif // defined(OS_ANDROID)
......
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