Fix bug where accounts are duplicated in user&account list.
This happens when user has different accounts with same account name. For example: user@domain.com for type1 and same email for type2, etc. When we refresh the account list during onResume(), the same account name incorrectly causes the logic to think they are the same account and updates the account list by mistake. - Introduced a util method to generate unique key from Account object. And set it as preference key. - when updating preference on screen, use key to find preference instead of the preference instance itself. Change-Id: I0aa692cb965b7037155a746389a919cd155843da Fix: 34035653 Test: make RunSettingsRoboTests
This commit is contained in:
@@ -123,9 +123,9 @@ public class AccountPreferenceController extends PreferenceController
|
|||||||
*/
|
*/
|
||||||
public boolean pendingRemoval;
|
public boolean pendingRemoval;
|
||||||
/**
|
/**
|
||||||
* The map from account name to account preference
|
* The map from account key to account preference
|
||||||
*/
|
*/
|
||||||
public ArrayMap<CharSequence, AccountTypePreference> accountPreferences = new ArrayMap<>();
|
public ArrayMap<String, AccountTypePreference> accountPreferences = new ArrayMap<>();
|
||||||
}
|
}
|
||||||
|
|
||||||
public AccountPreferenceController(Context context, SettingsPreferenceFragment parent,
|
public AccountPreferenceController(Context context, SettingsPreferenceFragment parent,
|
||||||
@@ -426,7 +426,7 @@ public class AccountPreferenceController extends PreferenceController
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (profileData.userInfo.isEnabled()) {
|
if (profileData.userInfo.isEnabled()) {
|
||||||
final ArrayMap<CharSequence, AccountTypePreference> preferenceToRemove =
|
final ArrayMap<String, AccountTypePreference> preferenceToRemove =
|
||||||
new ArrayMap<>(profileData.accountPreferences);
|
new ArrayMap<>(profileData.accountPreferences);
|
||||||
final ArrayList<AccountTypePreference> preferences = getAccountTypePreferences(
|
final ArrayList<AccountTypePreference> preferences = getAccountTypePreferences(
|
||||||
profileData.authenticatorHelper, profileData.userInfo.getUserHandle(),
|
profileData.authenticatorHelper, profileData.userInfo.getUserHandle(),
|
||||||
@@ -435,18 +435,19 @@ public class AccountPreferenceController extends PreferenceController
|
|||||||
for (int i = 0; i < count; i++) {
|
for (int i = 0; i < count; i++) {
|
||||||
final AccountTypePreference preference = preferences.get(i);
|
final AccountTypePreference preference = preferences.get(i);
|
||||||
preference.setOrder(i);
|
preference.setOrder(i);
|
||||||
if (!profileData.accountPreferences.containsValue(preference)) {
|
final String key = preference.getKey();
|
||||||
profileData.preferenceGroup.addPreference(preferences.get(i));
|
if (!profileData.accountPreferences.containsKey(key)) {
|
||||||
profileData.accountPreferences.put(preference.getTitle(), preference);
|
profileData.preferenceGroup.addPreference(preference);
|
||||||
|
profileData.accountPreferences.put(key, preference);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (profileData.addAccountPreference != null) {
|
if (profileData.addAccountPreference != null) {
|
||||||
profileData.preferenceGroup.addPreference(profileData.addAccountPreference);
|
profileData.preferenceGroup.addPreference(profileData.addAccountPreference);
|
||||||
}
|
}
|
||||||
for (CharSequence name : preferenceToRemove.keySet()) {
|
for (String key : preferenceToRemove.keySet()) {
|
||||||
profileData.preferenceGroup.removePreference(
|
profileData.preferenceGroup.removePreference(
|
||||||
profileData.accountPreferences.get(name));
|
profileData.accountPreferences.get(key));
|
||||||
profileData.accountPreferences.remove(name);
|
profileData.accountPreferences.remove(key);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
profileData.preferenceGroup.removeAll();
|
profileData.preferenceGroup.removeAll();
|
||||||
@@ -471,8 +472,7 @@ public class AccountPreferenceController extends PreferenceController
|
|||||||
}
|
}
|
||||||
|
|
||||||
private ArrayList<AccountTypePreference> getAccountTypePreferences(AuthenticatorHelper helper,
|
private ArrayList<AccountTypePreference> getAccountTypePreferences(AuthenticatorHelper helper,
|
||||||
UserHandle userHandle,
|
UserHandle userHandle, ArrayMap<String, AccountTypePreference> preferenceToRemove) {
|
||||||
ArrayMap<CharSequence, AccountTypePreference> preferenceToRemove) {
|
|
||||||
final String[] accountTypes = helper.getEnabledAccountTypes();
|
final String[] accountTypes = helper.getEnabledAccountTypes();
|
||||||
final ArrayList<AccountTypePreference> accountTypePreferences =
|
final ArrayList<AccountTypePreference> accountTypePreferences =
|
||||||
new ArrayList<>(accountTypes.length);
|
new ArrayList<>(accountTypes.length);
|
||||||
@@ -497,7 +497,8 @@ public class AccountPreferenceController extends PreferenceController
|
|||||||
|
|
||||||
// Add a preference row for each individual account
|
// Add a preference row for each individual account
|
||||||
for (Account account : accounts) {
|
for (Account account : accounts) {
|
||||||
final AccountTypePreference preference = preferenceToRemove.remove(account.name);
|
final AccountTypePreference preference =
|
||||||
|
preferenceToRemove.remove(AccountTypePreference.buildKey(account));
|
||||||
if (preference != null) {
|
if (preference != null) {
|
||||||
accountTypePreferences.add(preference);
|
accountTypePreferences.add(preference);
|
||||||
continue;
|
continue;
|
||||||
@@ -521,7 +522,7 @@ public class AccountPreferenceController extends PreferenceController
|
|||||||
fragmentArguments.putParcelable(EXTRA_USER, userHandle);
|
fragmentArguments.putParcelable(EXTRA_USER, userHandle);
|
||||||
accountTypePreferences.add(new AccountTypePreference(
|
accountTypePreferences.add(new AccountTypePreference(
|
||||||
prefContext, mMetricsFeatureProvider.getMetricsCategory(mParent),
|
prefContext, mMetricsFeatureProvider.getMetricsCategory(mParent),
|
||||||
account.name, titleResPackageName, titleResId, label,
|
account, titleResPackageName, titleResId, label,
|
||||||
AccountDetailDashboardFragment.class.getName(), fragmentArguments, icon));
|
AccountDetailDashboardFragment.class.getName(), fragmentArguments, icon));
|
||||||
}
|
}
|
||||||
helper.preloadDrawableForType(mContext, accountType);
|
helper.preloadDrawableForType(mContext, accountType);
|
||||||
|
@@ -16,6 +16,7 @@
|
|||||||
|
|
||||||
package com.android.settings.accounts;
|
package com.android.settings.accounts;
|
||||||
|
|
||||||
|
import android.accounts.Account;
|
||||||
import android.content.Context;
|
import android.content.Context;
|
||||||
import android.graphics.drawable.Drawable;
|
import android.graphics.drawable.Drawable;
|
||||||
import android.os.Bundle;
|
import android.os.Bundle;
|
||||||
@@ -67,18 +68,11 @@ public class AccountTypePreference extends Preference implements OnPreferenceCli
|
|||||||
|
|
||||||
private final int mMetricsCategory;
|
private final int mMetricsCategory;
|
||||||
|
|
||||||
public AccountTypePreference(Context context, int metricsCategory, CharSequence title,
|
public AccountTypePreference(Context context, int metricsCategory, Account account,
|
||||||
String titleResPackageName, int titleResId, String fragment, Bundle fragmentArguments,
|
|
||||||
Drawable icon) {
|
|
||||||
this(context, metricsCategory, title, titleResPackageName, titleResId, null, fragment,
|
|
||||||
fragmentArguments, icon);
|
|
||||||
}
|
|
||||||
|
|
||||||
public AccountTypePreference(Context context, int metricsCategory, CharSequence title,
|
|
||||||
String titleResPackageName, int titleResId, CharSequence summary, String fragment,
|
String titleResPackageName, int titleResId, CharSequence summary, String fragment,
|
||||||
Bundle fragmentArguments, Drawable icon) {
|
Bundle fragmentArguments, Drawable icon) {
|
||||||
super(context);
|
super(context);
|
||||||
mTitle = title;
|
mTitle = account.name;
|
||||||
mTitleResPackageName = titleResPackageName;
|
mTitleResPackageName = titleResPackageName;
|
||||||
mTitleResId = titleResId;
|
mTitleResId = titleResId;
|
||||||
mSummary = summary;
|
mSummary = summary;
|
||||||
@@ -87,7 +81,8 @@ public class AccountTypePreference extends Preference implements OnPreferenceCli
|
|||||||
mMetricsCategory = metricsCategory;
|
mMetricsCategory = metricsCategory;
|
||||||
setWidgetLayoutResource(R.layout.account_type_preference);
|
setWidgetLayoutResource(R.layout.account_type_preference);
|
||||||
|
|
||||||
setTitle(title);
|
setKey(buildKey(account));
|
||||||
|
setTitle(mTitle);
|
||||||
setSummary(summary);
|
setSummary(summary);
|
||||||
setIcon(icon);
|
setIcon(icon);
|
||||||
|
|
||||||
@@ -115,6 +110,13 @@ public class AccountTypePreference extends Preference implements OnPreferenceCli
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build a unique preference key based on account.
|
||||||
|
*/
|
||||||
|
public static String buildKey(Account account) {
|
||||||
|
return String.valueOf(account.hashCode());
|
||||||
|
}
|
||||||
|
|
||||||
public CharSequence getTitle() {
|
public CharSequence getTitle() {
|
||||||
return mTitle;
|
return mTitle;
|
||||||
}
|
}
|
||||||
@@ -122,5 +124,4 @@ public class AccountTypePreference extends Preference implements OnPreferenceCli
|
|||||||
public CharSequence getSummary() {
|
public CharSequence getSummary() {
|
||||||
return mSummary;
|
return mSummary;
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
@@ -26,8 +26,8 @@ import android.support.v7.preference.Preference;
|
|||||||
import android.support.v7.preference.PreferenceGroup;
|
import android.support.v7.preference.PreferenceGroup;
|
||||||
import android.support.v7.preference.PreferenceManager;
|
import android.support.v7.preference.PreferenceManager;
|
||||||
import android.support.v7.preference.PreferenceScreen;
|
import android.support.v7.preference.PreferenceScreen;
|
||||||
|
|
||||||
import android.text.TextUtils;
|
import android.text.TextUtils;
|
||||||
|
|
||||||
import com.android.settings.AccessiblePreferenceCategory;
|
import com.android.settings.AccessiblePreferenceCategory;
|
||||||
import com.android.settings.R;
|
import com.android.settings.R;
|
||||||
import com.android.settings.SettingsPreferenceFragment;
|
import com.android.settings.SettingsPreferenceFragment;
|
||||||
@@ -92,7 +92,7 @@ public class AccountPreferenceControllerTest {
|
|||||||
when(mFragment.getPreferenceScreen()).thenReturn(mScreen);
|
when(mFragment.getPreferenceScreen()).thenReturn(mScreen);
|
||||||
when(mFragment.getPreferenceManager().getContext()).thenReturn(mContext);
|
when(mFragment.getPreferenceManager().getContext()).thenReturn(mContext);
|
||||||
when(mAccountManager.getAuthenticatorTypesAsUser(anyInt())).thenReturn(
|
when(mAccountManager.getAuthenticatorTypesAsUser(anyInt())).thenReturn(
|
||||||
new AuthenticatorDescription[0]);
|
new AuthenticatorDescription[0]);
|
||||||
when(mAccountManager.getAccountsAsUser(anyInt())).thenReturn(new Account[0]);
|
when(mAccountManager.getAccountsAsUser(anyInt())).thenReturn(new Account[0]);
|
||||||
mController = new AccountPreferenceController(mContext, mFragment, null, mAccountHelper);
|
mController = new AccountPreferenceController(mContext, mFragment, null, mAccountHelper);
|
||||||
}
|
}
|
||||||
@@ -365,6 +365,51 @@ public class AccountPreferenceControllerTest {
|
|||||||
verify(preferenceGroup, times(3)).addPreference(any(Preference.class));
|
verify(preferenceGroup, times(3)).addPreference(any(Preference.class));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class})
|
||||||
|
public void onResume_twoAccountsOfSameName_shouldAddFivePreferences() {
|
||||||
|
final List<UserInfo> infos = new ArrayList<>();
|
||||||
|
infos.add(new UserInfo(1, "user 1", 0));
|
||||||
|
when(mUserManager.isManagedProfile()).thenReturn(false);
|
||||||
|
when(mUserManager.isLinkedUser()).thenReturn(false);
|
||||||
|
when(mUserManager.getProfiles(anyInt())).thenReturn(infos);
|
||||||
|
|
||||||
|
final Account[] accountType1 = new Account[2];
|
||||||
|
accountType1[0] = new Account("Account1", "com.acct1");
|
||||||
|
accountType1[1] = new Account("Account2", "com.acct1");
|
||||||
|
final Account[] accountType2 = new Account[2];
|
||||||
|
accountType2[0] = new Account("Account1", "com.acct2");
|
||||||
|
accountType2[1] = new Account("Account2", "com.acct2");
|
||||||
|
final Account[] allAccounts = new Account[4];
|
||||||
|
allAccounts[0] = accountType1[0];
|
||||||
|
allAccounts[1] = accountType1[1];
|
||||||
|
allAccounts[2] = accountType2[0];
|
||||||
|
allAccounts[3] = accountType2[1];
|
||||||
|
final AuthenticatorDescription[] authDescs = {
|
||||||
|
new AuthenticatorDescription("com.acct1", "com.android.settings",
|
||||||
|
R.string.account_settings_title, 0, 0, 0, false),
|
||||||
|
new AuthenticatorDescription("com.acct2", "com.android.settings",
|
||||||
|
R.string.account_settings_title, 0, 0, 0, false)
|
||||||
|
};
|
||||||
|
|
||||||
|
when(mAccountManager.getAccountsAsUser(anyInt())).thenReturn(allAccounts);
|
||||||
|
when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct1"), any(UserHandle.class)))
|
||||||
|
.thenReturn(accountType1);
|
||||||
|
when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct2"), any(UserHandle.class)))
|
||||||
|
.thenReturn(accountType2);
|
||||||
|
when(mAccountManager.getAuthenticatorTypesAsUser(anyInt())).thenReturn(authDescs);
|
||||||
|
|
||||||
|
AccessiblePreferenceCategory preferenceGroup = mock(AccessiblePreferenceCategory.class);
|
||||||
|
when(preferenceGroup.getPreferenceManager()).thenReturn(mock(PreferenceManager.class));
|
||||||
|
when(mAccountHelper.createAccessiblePreferenceCategory(any(Context.class))).thenReturn(
|
||||||
|
preferenceGroup);
|
||||||
|
|
||||||
|
mController.onResume();
|
||||||
|
|
||||||
|
// should add 4 individual account and the Add account preference
|
||||||
|
verify(preferenceGroup, times(5)).addPreference(any(Preference.class));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class})
|
@Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class})
|
||||||
public void onResume_noAccountChange_shouldNotAddAccountPreference() {
|
public void onResume_noAccountChange_shouldNotAddAccountPreference() {
|
||||||
|
Reference in New Issue
Block a user