Skip to content

Commit

Permalink
[Signin][Android] Populate profile data cache without GmsProfileDataS…
Browse files Browse the repository at this point in the history 
…ource

This CL populates the profile data cache without GmsProfileDataSource.
This step is necessary for us to remove GmsProfileDataSource completely
in the future.

Bug: 1181226
Change-Id: Ie24d3d50843331c638015460eeac767bfd85162c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2713105
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858506}
  • Loading branch information
Alice Wang authored and Chromium LUCI CQ committed Mar 1, 2021
1 parent a1ff7c3 commit ab04436
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 14 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/signin/services/android/BUILD.gn
View file
Expand Up @@ -31,6 +31,7 @@ android_library("java") {
"$google_play_services_package:google_play_services_auth_base_java",
"//base:base_java",
"//base:jni_java",
"//chrome/browser/flags:java",
"//chrome/browser/preferences:java",
"//chrome/browser/profiles/android:java",
"//components/browser_ui/util/android:java",
Expand Down Expand Up @@ -60,6 +61,7 @@ android_library("javatests") {
deps = [
":java",
"//base:base_java_test_support",
"//chrome/browser/flags:java",
"//chrome/browser/profiles/android:java",
"//chrome/test/android:chrome_java_test_support",
"//components/signin/public/android:java",
Expand Down
45 changes: 32 additions & 13 deletions ...rvices/android/java/src/org/chromium/chrome/browser/signin/services/ProfileDataCache.java
View file
Expand Up @@ -26,8 +26,10 @@
import androidx.annotation.VisibleForTesting;
import androidx.appcompat.content.res.AppCompatResources;

import org.chromium.base.Callback;
import org.chromium.base.ObserverList;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.components.browser_ui.util.AvatarGenerator;
import org.chromium.components.signin.AccountManagerFacadeProvider;
Expand Down Expand Up @@ -108,7 +110,7 @@ int getBorderSize() {
private final Drawable mPlaceholderImage;
private final ObserverList<Observer> mObservers = new ObserverList<>();
private final Map<String, DisplayableProfileData> mCachedProfileData = new HashMap<>();
private @Nullable final ProfileDataSource mProfileDataSource;
private @Nullable ProfileDataSource mProfileDataSource;
private final IdentityManager mIdentityManager;

@VisibleForTesting
Expand Down Expand Up @@ -178,6 +180,18 @@ public void update(List<String> accountEmails) {
}
}

/**
* Disables the Gms profile data source in this class. The {@link AccountInfoService} will
* query the native code for profile data in this case.
* This method is only used to deprecate the Gms profile data source.
* TODO(crbug/1154606): Remove this method after retiring the GmsProfileDataSource.
*/
public void disableGmsProfileDataSource() {
assert ChromeFeatureList.isEnabled(ChromeFeatureList.DEPRECATE_MENAGERIE_API)
: "This method should only be called with DEPRECATE_MENAGERIE_API enabled";
mProfileDataSource = null;
}

/**
* @return The {@link DisplayableProfileData} containing the profile data corresponding to the
* given account or a {@link DisplayableProfileData} with a placeholder image and null
Expand All @@ -199,9 +213,9 @@ public void addObserver(Observer observer) {
if (mObservers.isEmpty()) {
if (mProfileDataSource != null) {
mProfileDataSource.addObserver(this);
updateCacheFromProfileDataSource();
}
mIdentityManager.addObserver(this);
populateCache();
}
mObservers.addObserver(observer);
}
Expand All @@ -220,18 +234,23 @@ public void removeObserver(Observer observer) {
}
}

private void updateCacheFromProfileDataSource() {
AccountManagerFacadeProvider.getInstance().tryGetGoogleAccounts(this::updateAccounts);
}

private void updateAccounts(final List<Account> accounts) {
for (Account account : accounts) {
ProfileData profileData = mProfileDataSource.getProfileDataForAccount(account.name);
if (profileData != null) {
updateCachedProfileDataAndNotifyObservers(
createDisplayableProfileData(profileData));
private void populateCache() {
final Callback<String> fetchDataForAccount = accountEmail -> {
if (mProfileDataSource != null) {
ProfileData profileData = mProfileDataSource.getProfileDataForAccount(accountEmail);
if (profileData != null) {
onProfileDataUpdated(profileData);
}
} else {
AccountInfoService.get().startFetchingAccountInfoFor(
accountEmail, this::onExtendedAccountInfoUpdated);
}
}
};
AccountManagerFacadeProvider.getInstance().tryGetGoogleAccounts(accounts -> {
for (Account account : accounts) {
fetchDataForAccount.onResult(account.name);
}
});
}

private DisplayableProfileData createDisplayableProfileData(
Expand Down
47 changes: 46 additions & 1 deletion ...roid/java/src/org/chromium/chrome/browser/signin/services/ProfileDataCacheRenderTest.java
View file
Expand Up @@ -25,6 +25,7 @@
import androidx.test.filters.MediumTest;

import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -38,10 +39,15 @@
import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate;
import org.chromium.chrome.test.util.ChromeRenderTestRule;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
import org.chromium.components.signin.AccountTrackerService;
import org.chromium.components.signin.ProfileDataSource;
import org.chromium.components.signin.base.AccountInfo;
import org.chromium.components.signin.base.CoreAccountId;
Expand All @@ -62,6 +68,7 @@
*/
@RunWith(ParameterizedRunner.class)
@UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@DisableFeatures({ChromeFeatureList.DEPRECATE_MENAGERIE_API})
@Batch(ProfileDataCacheRenderTest.PROFILE_DATA_BATCH_NAME)
public class ProfileDataCacheRenderTest extends DummyUiActivityTestCase {
public static final String PROFILE_DATA_BATCH_NAME = "profile_data";
Expand All @@ -79,9 +86,12 @@ public ProfileDataCacheRenderTest(int imageSize) {
}

@Rule
public ChromeRenderTestRule mRenderTestRule =
public final ChromeRenderTestRule mRenderTestRule =
ChromeRenderTestRule.Builder.withPublicCorpus().build();

@Rule
public final Features.JUnitProcessor mProcessor = new Features.JUnitProcessor();

@Rule
public final AccountManagerTestRule mAccountManagerTestRule =
new AccountManagerTestRule(new FakeProfileDataSource());
Expand All @@ -95,9 +105,15 @@ public ProfileDataCacheRenderTest(int imageSize) {
@Mock
private IdentityServicesProvider mIdentityServicesProviderMock;

@Mock
private AccountTrackerService mAccountTrackerServiceMock;

@Mock
private IdentityManager.Natives mIdentityManagerNativeMock;

@Mock
private ProfileDataCache.Observer mObserverMock;

private final IdentityManager mIdentityManager =
new IdentityManager(0 /* nativeIdentityManager */, null /* OAuth2TokenService */);

Expand All @@ -117,6 +133,8 @@ public void setUp() {
Profile.setLastUsedProfileForTesting(mProfileMock);
when(mIdentityServicesProviderMock.getIdentityManager(mProfileMock))
.thenReturn(mIdentityManager);
when(mIdentityServicesProviderMock.getAccountTrackerService(mProfileMock))
.thenReturn(mAccountTrackerServiceMock);
IdentityServicesProvider.setInstanceForTests(mIdentityServicesProviderMock);

TestThreadUtils.runOnUiThreadBlocking(() -> {
Expand Down Expand Up @@ -174,6 +192,33 @@ public void testProfileDataPopulatedFromIdentityManagerObserver() throws IOExcep
mRenderTestRule.render(mImageView, "profile_data_cache_avatar" + mImageSize);
}

@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.DEPRECATE_MENAGERIE_API})
@Feature("RenderTest")
public void testProfileDataPopulatedWithoutGmsProfileDataSource() throws IOException {
when(mAccountTrackerServiceMock.checkAndSeedSystemAccounts()).thenReturn(true);
when(mIdentityManagerNativeMock
.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
anyLong(), eq(ACCOUNT_EMAIL)))
.thenReturn(mAccountInfoWithAvatar);
mAccountManagerTestRule.addAccount(ACCOUNT_EMAIL);

mProfileDataCache = new ProfileDataCache(getActivity(), mImageSize, /*badgeConfig=*/null);
mProfileDataCache.disableGmsProfileDataSource();
// ProfileDataCache only populates the cache when an observer is added.
TestThreadUtils.runOnUiThreadBlocking(
() -> { mProfileDataCache.addObserver(mObserverMock); });

final DisplayableProfileData profileData =
mProfileDataCache.getProfileDataOrDefault(mAccountInfoWithAvatar.getEmail());
Assert.assertEquals(mAccountInfoWithAvatar.getFullName(), profileData.getFullName());
Assert.assertEquals(mAccountInfoWithAvatar.getGivenName(), profileData.getGivenName());
TestThreadUtils.runOnUiThreadBlocking(
() -> { checkImageIsScaled(mAccountInfoWithAvatar.getEmail()); });
mRenderTestRule.render(mImageView, "profile_data_cache_avatar" + mImageSize);
}

@Test
@MediumTest
@Feature("RenderTest")
Expand Down

0 comments on commit ab04436

Please sign in to comment.