Skip to content

GitLab

T
tangled
Commit 5ff24a2e authored by Tanmoy Mollik's avatar Tanmoy Mollik Committed by Chromium LUCI CQ
Browse files
Options

[Android] Refactor IdentityManager.Observer API override in SigninManagerImpl 

Replace IdentityManager.Observer.(onPrimaryAccountSet/Cleared) call with
onPrimaryAccountChanged call.
Thse cl also replaces these calls in SigninManagerTest

Bug: 1158855
Change-Id: I7ffac4a907c34fb22b0ff07ffc27e09a4465a563
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2606347Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarAlice Wang <aliceywang@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841043}
parent 5f2acf23
Hide whitespace changes
Inline Side-by-side
Showing with 53 additions and 35 deletions
chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManagerImpl.java
View file @ 5ff24a2e
......@@ -33,6 +33,7 @@ import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.identitymanager.ConsentLevel;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.identitymanager.IdentityMutator;
import org.chromium.components.signin.identitymanager.PrimaryAccountChangeEvent;
import org.chromium.components.signin.metrics.SigninAccessPoint;
import org.chromium.components.signin.metrics.SigninReason;
import org.chromium.components.signin.metrics.SignoutDelete;
......@@ -471,14 +472,43 @@ class SigninManagerImpl implements AccountTrackerService.OnSystemAccountsSeededL
}
/**
* Implements {@link IdentityManager.Observer}: take action when primary account is set.
* Simply verify that the request is ongoing (mSignInState != null), as only SigninManager
* should update IdentityManager. This is triggered by the call to
* IdentityMutator.setPrimaryAccount
* Implements {@link IdentityManager.Observer}
*/
@Override
public void onPrimaryAccountSet(CoreAccountInfo account) {
assert mSignInState != null;
public void onPrimaryAccountChanged(PrimaryAccountChangeEvent eventDetails) {
switch (eventDetails.getEventTypeFor(ConsentLevel.NOT_REQUIRED)) {
case PrimaryAccountChangeEvent.Type.SET:
// Simply verify that the request is ongoing (mSignInState != null), as only
// SigninManager should update IdentityManager. This is triggered by the call to
// IdentityMutator.setPrimaryAccount
assert mSignInState != null;
break;
case PrimaryAccountChangeEvent.Type.CLEARED:
if (mSignOutState == null) {
// mSignOutState can only be null when the sign out is triggered by
// native (since otherwise SigninManager.signOut would have created
// it). As sign out from native can only happen from policy code,
// the account is managed and the user data must be wiped.
mSignOutState = new SignOutState(null, true);
}
Log.d(TAG,
"On native signout, wipe user data: " + mSignOutState.mShouldWipeUserData);
// TODO(https://crbug.com/1091858): Remove this after migrating the legacy code that
// uses the sync account before the native is
// loaded.
SigninPreferencesManager.getInstance().setLegacySyncAccountEmail(null);
if (mSignOutState.mSignOutCallback != null) {
mSignOutState.mSignOutCallback.preWipeData();
}
disableSyncAndWipeData(mSignOutState.mShouldWipeUserData, this::finishSignOut);
mAccountTrackerService.invalidateAccountSeedStatus(true);
break;
case PrimaryAccountChangeEvent.Type.NONE:
break;
}
}
/**
......@@ -538,7 +568,7 @@ class SigninManagerImpl implements AccountTrackerService.OnSystemAccountsSeededL
Log.d(TAG, "Signing out, management domain: " + managementDomain);
// User data will be wiped in disableSyncAndWipeData(), called from
// onPrimaryAccountcleared().
// onPrimaryAccountChanged().
mIdentityMutator.clearPrimaryAccount(signoutSource,
// Always use IGNORE_METRIC for the profile deletion argument. Chrome
// Android has just a single-profile which is never deleted upon
......@@ -585,27 +615,6 @@ class SigninManagerImpl implements AccountTrackerService.OnSystemAccountsSeededL
notifySignInAllowedChanged();
}
@Override
public void onPrimaryAccountCleared(CoreAccountInfo account) {
if (mSignOutState == null) {
// mSignOutState can only be null when the sign out is triggered by
// native (since otherwise SigninManager.signOut would have created
// it). As sign out from native can only happen from policy code,
// the account is managed and the user data must be wiped.
mSignOutState = new SignOutState(null, true);
}
Log.d(TAG, "On native signout, wipe user data: " + mSignOutState.mShouldWipeUserData);
// TODO(https://crbug.com/1091858): Remove this after migrating the legacy code that uses
// the sync account before the native is loaded.
SigninPreferencesManager.getInstance().setLegacySyncAccountEmail(null);
if (mSignOutState.mSignOutCallback != null) mSignOutState.mSignOutCallback.preWipeData();
disableSyncAndWipeData(mSignOutState.mShouldWipeUserData, this::finishSignOut);
mAccountTrackerService.invalidateAccountSeedStatus(true);
}
@VisibleForTesting
void finishSignOut() {
// Should be set at start of sign-out flow.
......
chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninManagerTest.java
View file @ 5ff24a2e
......@@ -42,6 +42,7 @@ import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.identitymanager.ConsentLevel;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.identitymanager.IdentityMutator;
import org.chromium.components.signin.identitymanager.PrimaryAccountChangeEvent;
import org.chromium.components.signin.metrics.SigninAccessPoint;
import org.chromium.components.signin.metrics.SignoutDelete;
import org.chromium.components.signin.metrics.SignoutReason;
......@@ -167,7 +168,8 @@ public class SigninManagerTest {
verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
// Simulate native callback to trigger clearing of account data.
mIdentityManager.onPrimaryAccountCleared(ACCOUNT_INFO);
mIdentityManager.onPrimaryAccountChanged(new PrimaryAccountChangeEvent(
PrimaryAccountChangeEvent.Type.CLEARED, PrimaryAccountChangeEvent.Type.NONE));
// Sign-out should only clear the profile when the user is managed.
verify(mNativeMock, times(1)).wipeProfileData(anyLong(), any());
......@@ -188,7 +190,8 @@ public class SigninManagerTest {
verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
// Simulate native callback to trigger clearing of account data.
mIdentityManager.onPrimaryAccountCleared(ACCOUNT_INFO);
mIdentityManager.onPrimaryAccountChanged(new PrimaryAccountChangeEvent(
PrimaryAccountChangeEvent.Type.CLEARED, PrimaryAccountChangeEvent.Type.NONE));
// Sign-out should only clear the service worker cache when the user is not managed.
verify(mNativeMock, never()).wipeProfileData(anyLong(), any());
......@@ -209,7 +212,8 @@ public class SigninManagerTest {
verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
// Simulate native callback to trigger clearing of account data.
mIdentityManager.onPrimaryAccountCleared(ACCOUNT_INFO);
mIdentityManager.onPrimaryAccountChanged(new PrimaryAccountChangeEvent(
PrimaryAccountChangeEvent.Type.CLEARED, PrimaryAccountChangeEvent.Type.NONE));
// Sign-out should only clear the service worker cache when the user is not managed.
verify(mNativeMock, times(1)).wipeProfileData(anyLong(), any());
......@@ -220,7 +224,8 @@ public class SigninManagerTest {
public void signOutFromNative() {
createSigninManager();
// Simulate native initiating the sign-out.
mIdentityManager.onPrimaryAccountCleared(ACCOUNT_INFO);
mIdentityManager.onPrimaryAccountChanged(new PrimaryAccountChangeEvent(
PrimaryAccountChangeEvent.Type.CLEARED, PrimaryAccountChangeEvent.Type.NONE));
// Sign-out should only clear the profile when the user is managed.
verify(mNativeMock, times(1)).wipeProfileData(anyLong(), any());
......@@ -308,7 +313,8 @@ public class SigninManagerTest {
@Test
public void callbackNotifiedOnSignout() {
doAnswer(invocation -> {
mIdentityManager.onPrimaryAccountCleared(ACCOUNT_INFO);
mIdentityManager.onPrimaryAccountChanged(new PrimaryAccountChangeEvent(
PrimaryAccountChangeEvent.Type.CLEARED, PrimaryAccountChangeEvent.Type.NONE));
return null;
})
.when(mIdentityMutator)
......
components/signin/public/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java
View file @ 5ff24a2e
......@@ -114,7 +114,8 @@ public class IdentityManager {
* or sync consent granted/revoked in C++.
*/
@CalledByNative
private void onPrimaryAccountChanged(PrimaryAccountChangeEvent eventDetails) {
@VisibleForTesting
public void onPrimaryAccountChanged(PrimaryAccountChangeEvent eventDetails) {
for (Observer observer : mObservers) {
observer.onPrimaryAccountChanged(eventDetails);
}
......
components/signin/public/android/java/src/org/chromium/components/signin/identitymanager/PrimaryAccountChangeEvent.java
View file @ 5ff24a2e
......@@ -5,6 +5,7 @@
package org.chromium.components.signin.identitymanager;
import androidx.annotation.IntDef;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative;
......@@ -36,7 +37,8 @@ public class PrimaryAccountChangeEvent {
private final @Type int mEventTypeForConsentLevelNotRequired;
@CalledByNative
private PrimaryAccountChangeEvent(
@VisibleForTesting
public PrimaryAccountChangeEvent(
@Type int eventTypeForConsentLevelNotRequired, @Type int eventTypeForConsentLevelSync) {
mEventTypeForConsentLevelNotRequired = eventTypeForConsentLevelNotRequired;
mEventTypeForConsentLevelSync = eventTypeForConsentLevelSync;
......
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