From 633250bd788f25a863469845a9eaca1aba45a48d Mon Sep 17 00:00:00 2001 From: Doris Ling Date: Tue, 2 May 2017 11:28:14 -0700 Subject: [PATCH] Also update the account preferences in displayPreference(). - start creating the account preferences in displayPreference() to make the data available earlier. - when trying to update the UI when the page is resumed, instead of clearing the whole list and add the latest accounts, compare the differences between the currently shown accounts and the latest accounts and only update the account preferences that has been changed. Change-Id: I249eb2fe67f4adaab02bf77680c62d07756dc94a Fix: 34035653 Test: make RunSettingsRoboTests --- AndroidManifest.xml | 2 - .../accounts/AccountPreferenceController.java | 67 ++++-- .../AccountPreferenceControllerTest.java | 219 +++++++++++++++--- 3 files changed, 242 insertions(+), 46 deletions(-) diff --git a/AndroidManifest.xml b/AndroidManifest.xml index 45298913c2b..9e2338097ef 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -2373,8 +2373,6 @@ - accountPreferences = new ArrayMap<>(); } public AccountPreferenceController(Context context, SettingsPreferenceFragment parent, @@ -149,6 +158,12 @@ public class AccountPreferenceController extends PreferenceController return null; } + @Override + public void displayPreference(PreferenceScreen screen) { + super.displayPreference(screen); + updateUi(); + } + @Override public void updateRawDataToIndex(List rawData) { if (!isAvailable()) { @@ -189,7 +204,6 @@ public class AccountPreferenceController extends PreferenceController @Override public void onResume() { - cleanUpPreferences(); updateUi(); mManagedProfileBroadcastReceiver.register(mContext); listenToAccountUpdates(); @@ -253,6 +267,9 @@ public class AccountPreferenceController extends PreferenceController return; } + for (int i = 0, size = mProfiles.size(); i < size; i++) { + mProfiles.valueAt(i).pendingRemoval = true; + } if (mUm.isLinkedUser()) { // Restricted user or similar UserInfo userInfo = mUm.getUserInfo(UserHandle.myUserId()); @@ -264,6 +281,7 @@ public class AccountPreferenceController extends PreferenceController updateProfileUi(profiles.get(i)); } } + cleanUpPreferences(); // Add all preferences, starting with one for the primary profile. // Note that we're relying on the ordering given by the SparseArray keys, and on the @@ -278,6 +296,11 @@ public class AccountPreferenceController extends PreferenceController if (mParent.getPreferenceManager() == null) { return; } + final ProfileData data = mProfiles.get(userInfo.id); + if (data != null) { + data.pendingRemoval = false; + return; + } final Context context = mContext; final ProfileData profileData = new ProfileData(); profileData.userInfo = userInfo; @@ -366,12 +389,14 @@ public class AccountPreferenceController extends PreferenceController if (screen == null) { return; } - for (int i = 0; i < mProfiles.size(); i++) { - final PreferenceGroup preferenceGroup = mProfiles.valueAt(i).preferenceGroup; - screen.removePreference(preferenceGroup); + final int count = mProfiles.size(); + for (int i = count-1; i >= 0; i--) { + final ProfileData data = mProfiles.valueAt(i); + if (data.pendingRemoval) { + screen.removePreference(data.preferenceGroup); + mProfiles.removeAt(i); + } } - mProfiles.clear(); - mAccountProfileOrder = ORDER_ACCOUNT_PROFILES; } private void listenToAccountUpdates() { @@ -400,18 +425,31 @@ public class AccountPreferenceController extends PreferenceController // This could happen if activity is finishing return; } - profileData.preferenceGroup.removeAll(); if (profileData.userInfo.isEnabled()) { + final ArrayMap preferenceToRemove = + new ArrayMap<>(profileData.accountPreferences); final ArrayList preferences = getAccountTypePreferences( - profileData.authenticatorHelper, profileData.userInfo.getUserHandle()); + profileData.authenticatorHelper, profileData.userInfo.getUserHandle(), + preferenceToRemove); final int count = preferences.size(); for (int i = 0; i < count; i++) { - profileData.preferenceGroup.addPreference(preferences.get(i)); + final AccountTypePreference preference = preferences.get(i); + preference.setOrder(i); + if (!profileData.accountPreferences.containsValue(preference)) { + profileData.preferenceGroup.addPreference(preferences.get(i)); + profileData.accountPreferences.put(preference.getTitle(), preference); + } } if (profileData.addAccountPreference != null) { profileData.preferenceGroup.addPreference(profileData.addAccountPreference); } + for (CharSequence name : preferenceToRemove.keySet()) { + profileData.preferenceGroup.removePreference( + profileData.accountPreferences.get(name)); + profileData.accountPreferences.remove(name); + } } else { + profileData.preferenceGroup.removeAll(); // Put a label instead of the accounts list if (mProfileNotAvailablePreference == null) { mProfileNotAvailablePreference = @@ -433,7 +471,8 @@ public class AccountPreferenceController extends PreferenceController } private ArrayList getAccountTypePreferences(AuthenticatorHelper helper, - UserHandle userHandle) { + UserHandle userHandle, + ArrayMap preferenceToRemove) { final String[] accountTypes = helper.getEnabledAccountTypes(); final ArrayList accountTypePreferences = new ArrayList<>(accountTypes.length); @@ -453,13 +492,16 @@ public class AccountPreferenceController extends PreferenceController final Account[] accounts = AccountManager.get(mContext) .getAccountsByTypeAsUser(accountType, userHandle); - final boolean skipToAccount = accounts.length == 1 - && !helper.hasAccountPreferences(accountType); final Drawable icon = helper.getDrawableForType(mContext, accountType); final Context prefContext = mParent.getPreferenceManager().getContext(); // Add a preference row for each individual account for (Account account : accounts) { + final AccountTypePreference preference = preferenceToRemove.remove(account.name); + if (preference != null) { + accountTypePreferences.add(preference); + continue; + } final ArrayList auths = helper.getAuthoritiesForAccountType(account.type); if (!AccountRestrictionHelper.showAccount(mAuthorities, auths)) { @@ -531,7 +573,6 @@ public class AccountPreferenceController extends PreferenceController || action.equals(Intent.ACTION_MANAGED_PROFILE_ADDED)) { // Clean old state stopListeningToAccountUpdates(); - cleanUpPreferences(); // Build new state updateUi(); listenToAccountUpdates(); diff --git a/tests/robotests/src/com/android/settings/accounts/AccountPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/accounts/AccountPreferenceControllerTest.java index d2ded86c964..8453da5c58c 100644 --- a/tests/robotests/src/com/android/settings/accounts/AccountPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/accounts/AccountPreferenceControllerTest.java @@ -27,6 +27,7 @@ import android.support.v7.preference.PreferenceGroup; import android.support.v7.preference.PreferenceManager; import android.support.v7.preference.PreferenceScreen; +import android.text.TextUtils; import com.android.settings.AccessiblePreferenceCategory; import com.android.settings.R; import com.android.settings.SettingsPreferenceFragment; @@ -39,6 +40,7 @@ import com.android.settings.testutils.shadow.ShadowContentResolver; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.annotation.Config; @@ -51,20 +53,22 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Answers.RETURNS_DEEP_STUBS; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @RunWith(SettingsRobolectricTestRunner.class) -@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) +@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION, + shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) public class AccountPreferenceControllerTest { @Mock(answer = RETURNS_DEEP_STUBS) private PreferenceScreen mScreen; - @Mock(answer = RETURNS_DEEP_STUBS) private UserManager mUserManager; @Mock(answer = RETURNS_DEEP_STUBS) @@ -145,43 +149,56 @@ public class AccountPreferenceControllerTest { @Test @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) - public void onResume_oneProfiles_shouldRemoveOneAccountCategory() { - final List infos = new ArrayList<>(); - infos.add(new UserInfo(1, "user 1", UserInfo.FLAG_MANAGED_PROFILE)); - when(mUserManager.isManagedProfile()).thenReturn(false); - when(mUserManager.isLinkedUser()).thenReturn(false); - when(mUserManager.getProfiles(anyInt())).thenReturn(infos); - AccessiblePreferenceCategory preferenceGroup = mock(AccessiblePreferenceCategory.class); - when(mAccountHelper.createAccessiblePreferenceCategory(any(Context.class))).thenReturn( - preferenceGroup); - - // First time resume will build the UI, 2nd time will refresh the UI - mController.onResume(); - mController.onResume(); - - verify(mScreen, times(1)).removePreference(any(PreferenceGroup.class)); - verify(mScreen).removePreference(preferenceGroup); - } - - @Test - @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) - public void onResume_twoProfiles_shouldRemoveTwoAccountCategory() { + public void onResume_noProfileChange_shouldNotAddOrRemoveAccountCategory() { final List infos = new ArrayList<>(); infos.add(new UserInfo(1, "user 1", 0)); infos.add(new UserInfo(2, "user 2", UserInfo.FLAG_MANAGED_PROFILE)); when(mUserManager.isManagedProfile()).thenReturn(false); when(mUserManager.isLinkedUser()).thenReturn(false); when(mUserManager.getProfiles(anyInt())).thenReturn(infos); - AccessiblePreferenceCategory preferenceGroup = mock(AccessiblePreferenceCategory.class); - when(mAccountHelper.createAccessiblePreferenceCategory(any(Context.class))).thenReturn( - preferenceGroup); - - // First time resume will build the UI, 2nd time will refresh the UI - mController.onResume(); + // First time resume will build the UI mController.onResume(); + reset(mScreen); - verify(mScreen, times(2)).removePreference(any(PreferenceGroup.class)); - verify(mScreen, times(2)).removePreference(preferenceGroup); + mController.onResume(); + verify(mScreen, never()).addPreference(any(PreferenceGroup.class)); + verify(mScreen, never()).removePreference(any(PreferenceGroup.class)); + } + + @Test + @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) + public void onResume_oneNewProfile_shouldAddOneAccountCategory() { + final List 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); + // First time resume will build the UI + mController.onResume(); + // add a new profile + infos.add(new UserInfo(2, "user 2", UserInfo.FLAG_MANAGED_PROFILE)); + reset(mScreen); + + mController.onResume(); + verify(mScreen, times(1)).addPreference(any(PreferenceGroup.class)); + } + + @Test + @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) + public void onResume_oneProfileRemoved_shouldRemoveOneAccountCategory() { + final List infos = new ArrayList<>(); + infos.add(new UserInfo(1, "user 1", 0)); + infos.add(new UserInfo(2, "user 2", UserInfo.FLAG_MANAGED_PROFILE)); + when(mUserManager.isManagedProfile()).thenReturn(false); + when(mUserManager.isLinkedUser()).thenReturn(false); + when(mUserManager.getProfiles(anyInt())).thenReturn(infos); + // First time resume will build the UI + mController.onResume(); + // remove a profile + infos.remove(1); + + mController.onResume(); + verify(mScreen, times(1)).removePreference(any(PreferenceGroup.class)); } @Test @@ -348,4 +365,144 @@ public class AccountPreferenceControllerTest { verify(preferenceGroup, times(3)).addPreference(any(Preference.class)); } + @Test + @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) + public void onResume_noAccountChange_shouldNotAddAccountPreference() { + final List 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); + Account[] accounts = {new Account("Acct1", "com.acct1")}; + when(mAccountManager.getAccountsAsUser(anyInt())).thenReturn(accounts); + + Account[] accountType1 = new Account[2]; + accountType1[0] = new Account("Acct11", "com.acct1"); + accountType1[1] = new Account("Acct12", "com.acct1"); + when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct1"), any(UserHandle.class))) + .thenReturn(accountType1); + + AuthenticatorDescription[] authDescs = { + new AuthenticatorDescription("com.acct1", "com.android.settings", + R.string.account_settings_title, 0, 0, 0, false) + }; + 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(); + + mController.onResume(); + + // each account should be added only once + verify(preferenceGroup).addPreference(argThat(new PreferenceMatcher("Acct11"))); + verify(preferenceGroup).addPreference(argThat(new PreferenceMatcher("Acct12"))); + } + + @Test + public void onResume_oneNewAccount_shouldAddOneAccountPreference() { + final List 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); + Account[] accounts = {new Account("Acct1", "com.acct1")}; + when(mAccountManager.getAccountsAsUser(anyInt())).thenReturn(accounts); + + Account[] accountType1 = new Account[2]; + accountType1[0] = new Account("Acct11", "com.acct1"); + accountType1[1] = new Account("Acct12", "com.acct1"); + when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct1"), any(UserHandle.class))) + .thenReturn(accountType1); + + AuthenticatorDescription[] authDescs = { + new AuthenticatorDescription("com.acct1", "com.android.settings", + R.string.account_settings_title, 0, 0, 0, false) + }; + 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(); + + // add a new account + accountType1 = new Account[3]; + accountType1[0] = new Account("Acct11", "com.acct1"); + accountType1[1] = new Account("Acct12", "com.acct1"); + accountType1[2] = new Account("Acct13", "com.acct1"); + when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct1"), any(UserHandle.class))) + .thenReturn(accountType1); + + mController.onResume(); + + // each account should be added only once + verify(preferenceGroup, times(1)).addPreference(argThat(new PreferenceMatcher("Acct11"))); + verify(preferenceGroup, times(1)).addPreference(argThat(new PreferenceMatcher("Acct12"))); + verify(preferenceGroup, times(1)).addPreference(argThat(new PreferenceMatcher("Acct13"))); + } + + @Test + @Config(shadows = {ShadowAccountManager.class, ShadowContentResolver.class}) + public void onResume_oneAccountRemoved_shouldRemoveOneAccountPreference() { + final List 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); + Account[] accounts = {new Account("Acct1", "com.acct1")}; + when(mAccountManager.getAccountsAsUser(anyInt())).thenReturn(accounts); + + Account[] accountType1 = new Account[2]; + accountType1[0] = new Account("Acct11", "com.acct1"); + accountType1[1] = new Account("Acct12", "com.acct1"); + when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct1"), any(UserHandle.class))) + .thenReturn(accountType1); + + AuthenticatorDescription[] authDescs = { + new AuthenticatorDescription("com.acct1", "com.android.settings", + R.string.account_settings_title, 0, 0, 0, false) + }; + 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(); + + // remove an account + accountType1 = new Account[1]; + accountType1[0] = new Account("Acct11", "com.acct1"); + when(mAccountManager.getAccountsByTypeAsUser(eq("com.acct1"), any(UserHandle.class))) + .thenReturn(accountType1); + + mController.onResume(); + + verify(preferenceGroup, times(1)).addPreference(argThat(new PreferenceMatcher("Acct11"))); + verify(preferenceGroup, times(1)).addPreference(argThat(new PreferenceMatcher("Acct12"))); + verify(preferenceGroup, times(1)).removePreference( + argThat(new PreferenceMatcher("Acct12"))); + } + + private static class PreferenceMatcher extends ArgumentMatcher { + + private final String mExpectedTitle; + + public PreferenceMatcher(String title) { + mExpectedTitle = title; + } + + @Override + public boolean matches(Object arg) { + final Preference preference = (Preference) arg; + return TextUtils.equals(mExpectedTitle, preference.getTitle()); + } + } + }