Commit 46031628 authored by Haiyang Pan's avatar Haiyang Pan Committed by Commit Bot

Revert "[Android][Signin] Add observer to listen to monogram update"

This reverts commit a5707a7d.

Reason for revert: Likely the cause of test failure in AccountPickerDialogFragmentTest on android builders:
* https://ci.chromium.org/p/chromium/builders/ci/android-arm64-proguard-rel/2755
* https://ci.chromium.org/p/chromium/builders/ci/android-pie-x86-rel/2363

The error said:
Attempt to invoke virtual method 'boolean org.chromium.base.ObserverList.c(java.lang.Object)' on a null object reference
	at org.chromium.components.signin.identitymanager.IdentityManager.addObserver(IdentityManager.java:95)
	at org.chromium.chrome.browser.signin.ProfileDataCache.addObserver(ProfileDataCache.java:211)

Original change's description:
> [Android][Signin] Add observer to listen to monogram update
>
> This CL adds an observer to update ProfileDataCache when user's profile
> photo(include monogram) from IdentityManager is updated.
>
> Bug: 1127886
> Change-Id: I0c70f72708e50dceb63eb122055315f7818cba06
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430984
> Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
> Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> Commit-Queue: Alice Wang <aliceywang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#811237}

TBR=msarda@chromium.org,bsazonov@chromium.org,aliceywang@chromium.org

Change-Id: I91562cb8017c2a74afd391fede5d9b206e7e6b5d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1127886
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435151Reviewed-by: default avatarHaiyang Pan <hypan@google.com>
Commit-Queue: Haiyang Pan <hypan@google.com>
Cr-Commit-Position: refs/heads/master@{#811317}
parent ca6aa8a7
...@@ -32,8 +32,6 @@ import org.chromium.components.browser_ui.util.AvatarGenerator; ...@@ -32,8 +32,6 @@ import org.chromium.components.browser_ui.util.AvatarGenerator;
import org.chromium.components.signin.AccountManagerFacadeProvider; import org.chromium.components.signin.AccountManagerFacadeProvider;
import org.chromium.components.signin.ProfileDataSource; import org.chromium.components.signin.ProfileDataSource;
import org.chromium.components.signin.base.AccountInfo; import org.chromium.components.signin.base.AccountInfo;
import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.identitymanager.IdentityManager;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
...@@ -46,8 +44,7 @@ import java.util.Map; ...@@ -46,8 +44,7 @@ import java.util.Map;
* should be provided by calling {@link #update(List)} * should be provided by calling {@link #update(List)}
*/ */
@MainThread @MainThread
public class ProfileDataCache implements ProfileDownloader.Observer, ProfileDataSource.Observer, public class ProfileDataCache implements ProfileDownloader.Observer, ProfileDataSource.Observer {
IdentityManager.Observer {
/** /**
* Observer to get notifications about changes in profile data. * Observer to get notifications about changes in profile data.
*/ */
...@@ -115,7 +112,6 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -115,7 +112,6 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
private final ObserverList<Observer> mObservers = new ObserverList<>(); private final ObserverList<Observer> mObservers = new ObserverList<>();
private final Map<String, DisplayableProfileData> mCachedProfileData = new HashMap<>(); private final Map<String, DisplayableProfileData> mCachedProfileData = new HashMap<>();
private @Nullable final ProfileDataSource mProfileDataSource; private @Nullable final ProfileDataSource mProfileDataSource;
private final IdentityManager mIdentityManager;
public ProfileDataCache(Context context, @Px int imageSize) { public ProfileDataCache(Context context, @Px int imageSize) {
this(context, imageSize, null); this(context, imageSize, null);
...@@ -134,8 +130,6 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -134,8 +130,6 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
mBadgeConfig = badgeConfig; mBadgeConfig = badgeConfig;
mPlaceholderImage = getScaledPlaceholderImage(context, imageSize); mPlaceholderImage = getScaledPlaceholderImage(context, imageSize);
mProfileDataSource = profileDataSource; mProfileDataSource = profileDataSource;
mIdentityManager = IdentityServicesProvider.get().getIdentityManager(
Profile.getLastUsedRegularProfile());
} }
/** /**
...@@ -208,7 +202,6 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -208,7 +202,6 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
} else { } else {
ProfileDownloader.addObserver(this); ProfileDownloader.addObserver(this);
} }
mIdentityManager.addObserver(this);
} }
mObservers.addObserver(observer); mObservers.addObserver(observer);
} }
...@@ -225,14 +218,13 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -225,14 +218,13 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
} else { } else {
ProfileDownloader.removeObserver(this); ProfileDownloader.removeObserver(this);
} }
mIdentityManager.removeObserver(this);
} }
} }
private void updateCacheFromProfileDataSource() { private void updateCacheFromProfileDataSource() {
for (ProfileDataSource.ProfileData profileData : for (ProfileDataSource.ProfileData profileData :
mProfileDataSource.getProfileDataMap().values()) { mProfileDataSource.getProfileDataMap().values()) {
updateCachedProfileDataAndNotifyObservers(createDisplayableProfileData(profileData)); updateCachedProfileData(createDisplayableProfileData(profileData));
} }
} }
...@@ -247,7 +239,7 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -247,7 +239,7 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
public void onProfileDownloaded(String accountId, String fullName, String givenName, public void onProfileDownloaded(String accountId, String fullName, String givenName,
Bitmap bitmap) { Bitmap bitmap) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
updateCachedProfileDataAndNotifyObservers(new DisplayableProfileData( updateCachedProfileData(new DisplayableProfileData(
accountId, prepareAvatar(bitmap, accountId), fullName, givenName)); accountId, prepareAvatar(bitmap, accountId), fullName, givenName));
} }
...@@ -260,38 +252,11 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -260,38 +252,11 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
mCachedProfileData.remove(accountId); mCachedProfileData.remove(accountId);
notifyObservers(accountId); notifyObservers(accountId);
} else { } else {
updateCachedProfileDataAndNotifyObservers(createDisplayableProfileData(profileData)); updateCachedProfileData(createDisplayableProfileData(profileData));
} }
}
/**
* Implements {@link IdentityManager.Observer}.
*/
@Override
public void onExtendedAccountInfoUpdated(AccountInfo accountInfo) {
final String accountEmail = accountInfo.getEmail();
DisplayableProfileData profileData = mCachedProfileData.get(accountEmail);
// if profileData is null, we will fetch monogram when generating
// the cache so that different sources will be handled in order.
if (profileData != null && profileData.getImage() == mPlaceholderImage) {
updateCachedProfileDataAndNotifyObservers(new DisplayableProfileData(accountEmail,
prepareAvatar(accountInfo.getAccountImage(), accountEmail),
profileData.getFullName(), profileData.getGivenName()));
}
} }
/**
* Implements {@link IdentityManager.Observer}.
*/
@Override
public void onPrimaryAccountSet(CoreAccountInfo account) {}
/**
* Implements {@link IdentityManager.Observer}.
*/
@Override
public void onPrimaryAccountCleared(CoreAccountInfo account) {}
/** /**
* Returns a profile data cache object without a badge.The badge is put with respect to * Returns a profile data cache object without a badge.The badge is put with respect to
* R.dimen.user_picture_size. So this method only works with the user avatar of this size. * R.dimen.user_picture_size. So this method only works with the user avatar of this size.
...@@ -350,7 +315,7 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -350,7 +315,7 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
return overlayBadgeOnUserPicture(croppedAvatar); return overlayBadgeOnUserPicture(croppedAvatar);
} }
private void updateCachedProfileDataAndNotifyObservers(DisplayableProfileData profileData) { private void updateCachedProfileData(DisplayableProfileData profileData) {
mCachedProfileData.put(profileData.getAccountName(), profileData); mCachedProfileData.put(profileData.getAccountName(), profileData);
notifyObservers(profileData.getAccountName()); notifyObservers(profileData.getAccountName());
} }
...@@ -414,10 +379,12 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData ...@@ -414,10 +379,12 @@ public class ProfileDataCache implements ProfileDownloader.Observer, ProfileData
* TODO(https://crbug.com/1130545): We should refactor the different sources for getting * TODO(https://crbug.com/1130545): We should refactor the different sources for getting
* the profile image. * the profile image.
*/ */
private @Nullable Bitmap getAccountImageFromIdentityManager(String accountEmail) { private static @Nullable Bitmap getAccountImageFromIdentityManager(String accountEmail) {
AccountInfo accountInfo = AccountInfo accountInfo =
mIdentityManager.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( IdentityServicesProvider.get()
accountEmail); .getIdentityManager(Profile.getLastUsedRegularProfile())
.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
accountEmail);
return accountInfo != null ? accountInfo.getAccountImage() : null; return accountInfo != null ? accountInfo.getAccountImage() : null;
} }
} }
...@@ -6,8 +6,6 @@ package org.chromium.chrome.browser.signin; ...@@ -6,8 +6,6 @@ package org.chromium.chrome.browser.signin;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotEquals;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
import android.app.Activity; import android.app.Activity;
import android.graphics.Bitmap; import android.graphics.Bitmap;
...@@ -22,11 +20,9 @@ import android.widget.ImageView; ...@@ -22,11 +20,9 @@ import android.widget.ImageView;
import androidx.annotation.Px; import androidx.annotation.Px;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
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.mockito.Mock;
import org.chromium.base.test.params.ParameterAnnotations.ClassParameter; import org.chromium.base.test.params.ParameterAnnotations.ClassParameter;
import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate; import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate;
...@@ -34,11 +30,9 @@ import org.chromium.base.test.params.ParameterSet; ...@@ -34,11 +30,9 @@ import org.chromium.base.test.params.ParameterSet;
import org.chromium.base.test.params.ParameterizedRunner; import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.util.Batch; import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate; import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate;
import org.chromium.chrome.test.util.ChromeRenderTestRule; import org.chromium.chrome.test.util.ChromeRenderTestRule;
import org.chromium.components.signin.ProfileDataSource; import org.chromium.components.signin.ProfileDataSource;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.test.util.FakeProfileDataSource; import org.chromium.components.signin.test.util.FakeProfileDataSource;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.DummyUiActivityTestCase; import org.chromium.ui.test.util.DummyUiActivityTestCase;
...@@ -73,27 +67,14 @@ public class ProfileDataCacheRenderTest extends DummyUiActivityTestCase { ...@@ -73,27 +67,14 @@ public class ProfileDataCacheRenderTest extends DummyUiActivityTestCase {
public ChromeRenderTestRule mRenderTestRule = public ChromeRenderTestRule mRenderTestRule =
ChromeRenderTestRule.Builder.withPublicCorpus().build(); ChromeRenderTestRule.Builder.withPublicCorpus().build();
@Mock
private Profile mProfileMock;
@Mock
private IdentityServicesProvider mIdentityServicesProviderMock;
private final IdentityManager mIdentityManager =
new IdentityManager(0 /* nativeIdentityManager */, null /* OAuth2TokenService */);
private FrameLayout mContentView; private FrameLayout mContentView;
private ImageView mImageView; private ImageView mImageView;
private FakeProfileDataSource mProfileDataSource; private FakeProfileDataSource mProfileDataSource;
private ProfileDataCache mProfileDataCache; private ProfileDataCache mProfileDataCache;
@Before @Override
public void setUp() { public void setUpTest() throws Exception {
initMocks(this); super.setUpTest();
Profile.setLastUsedProfileForTesting(mProfileMock);
when(mIdentityServicesProviderMock.getIdentityManager(mProfileMock))
.thenReturn(mIdentityManager);
IdentityServicesProvider.setInstanceForTests(mIdentityServicesProviderMock);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Activity activity = getActivity(); Activity activity = getActivity();
mContentView = new FrameLayout(activity); mContentView = new FrameLayout(activity);
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.signin; package org.chromium.chrome.browser.signin;
import static org.mockito.Mockito.mockingDetails; import static org.mockito.Mockito.mockingDetails;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks; import static org.mockito.MockitoAnnotations.initMocks;
import android.app.Activity; import android.app.Activity;
...@@ -21,7 +20,6 @@ import androidx.annotation.DrawableRes; ...@@ -21,7 +20,6 @@ import androidx.annotation.DrawableRes;
import androidx.test.filters.MediumTest; import androidx.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;
...@@ -30,11 +28,9 @@ import org.mockito.Mock; ...@@ -30,11 +28,9 @@ import org.mockito.Mock;
import org.chromium.base.test.util.Batch; import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.ChromeRenderTestRule; import org.chromium.chrome.test.util.ChromeRenderTestRule;
import org.chromium.components.signin.ProfileDataSource; import org.chromium.components.signin.ProfileDataSource;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.test.util.FakeProfileDataSource; import org.chromium.components.signin.test.util.FakeProfileDataSource;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.DummyUiActivityTestCase; import org.chromium.ui.test.util.DummyUiActivityTestCase;
...@@ -53,32 +49,20 @@ public class ProfileDataCacheWithBadgeRenderTest extends DummyUiActivityTestCase ...@@ -53,32 +49,20 @@ public class ProfileDataCacheWithBadgeRenderTest extends DummyUiActivityTestCase
public ChromeRenderTestRule mRenderTestRule = public ChromeRenderTestRule mRenderTestRule =
ChromeRenderTestRule.Builder.withPublicCorpus().build(); ChromeRenderTestRule.Builder.withPublicCorpus().build();
@Mock
private Profile mProfileMock;
@Mock
private IdentityServicesProvider mIdentityServicesProviderMock;
@Mock @Mock
private ProfileDataCache.Observer mObserver; private ProfileDataCache.Observer mObserver;
private static final String TEST_ACCOUNT_NAME = "test@example.com"; private static final String TEST_ACCOUNT_NAME = "test@example.com";
private final IdentityManager mIdentityManager =
new IdentityManager(0 /* nativeIdentityManager */, null /* OAuth2TokenService */);
private FrameLayout mContentView; private FrameLayout mContentView;
private ImageView mImageView; private ImageView mImageView;
private FakeProfileDataSource mProfileDataSource; private FakeProfileDataSource mProfileDataSource;
private ProfileDataCache mProfileDataCache; private ProfileDataCache mProfileDataCache;
@Before @Override
public void setUp() { public void setUpTest() throws Exception {
super.setUpTest();
initMocks(this); initMocks(this);
Profile.setLastUsedProfileForTesting(mProfileMock);
when(mIdentityServicesProviderMock.getIdentityManager(mProfileMock))
.thenReturn(mIdentityManager);
IdentityServicesProvider.setInstanceForTests(mIdentityServicesProviderMock);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Activity activity = getActivity(); Activity activity = getActivity();
mContentView = new FrameLayout(activity); mContentView = new FrameLayout(activity);
......
...@@ -46,11 +46,6 @@ public class IdentityManager { ...@@ -46,11 +46,6 @@ public class IdentityManager {
* the settings. * the settings.
*/ */
default void onAccountsCookieDeletedByUserAction() {} default void onAccountsCookieDeletedByUserAction() {}
/**
* Called after an account is updated.
*/
default void onExtendedAccountInfoUpdated(AccountInfo accountInfo) {}
} }
/** /**
* A simple callback for getAccessToken. * A simple callback for getAccessToken.
...@@ -131,16 +126,6 @@ public class IdentityManager { ...@@ -131,16 +126,6 @@ public class IdentityManager {
} }
} }
/**
* Called after an account is updated.
*/
@CalledByNative
private void onExtendedAccountInfoUpdated(AccountInfo accountInfo) {
for (Observer observer : mObservers) {
observer.onExtendedAccountInfoUpdated(accountInfo);
}
}
/** /**
* Returns whether the user's primary account is available. * Returns whether the user's primary account is available.
*/ */
......
...@@ -636,13 +636,6 @@ void IdentityManager::OnAccountUpdated(const AccountInfo& info) { ...@@ -636,13 +636,6 @@ void IdentityManager::OnAccountUpdated(const AccountInfo& info) {
for (auto& observer : observer_list_) { for (auto& observer : observer_list_) {
observer.OnExtendedAccountInfoUpdated(info); observer.OnExtendedAccountInfoUpdated(info);
} }
#if defined(OS_ANDROID)
if (java_identity_manager_) {
JNIEnv* env = base::android::AttachCurrentThread();
Java_IdentityManager_onExtendedAccountInfoUpdated(
env, java_identity_manager_, ConvertToJavaAccountInfo(env, info));
}
#endif
} }
void IdentityManager::OnAccountRemoved(const AccountInfo& info) { void IdentityManager::OnAccountRemoved(const AccountInfo& info) {
......
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