From b2f1263b254e2ebc92d414b6973799259a106b58 Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Wed, 29 Mar 2017 12:16:55 -0700 Subject: [PATCH] Don't show empty account list in account chooser. Use accounts order from getAccounts() method instead of getAccountsAndVisibilityForPackage(), which is unpredicteble (not linked HashMap) Test: manual Bug: 34684498 Change-Id: Idbd4dc1a4f7d5b5b8a1329b27f01a0793a64245d --- .../ChooseTypeAndAccountActivity.java | 105 ++++++++++-------- 1 file changed, 61 insertions(+), 44 deletions(-) diff --git a/core/java/android/accounts/ChooseTypeAndAccountActivity.java b/core/java/android/accounts/ChooseTypeAndAccountActivity.java index 35011b5f3ec3e..844258515561c 100644 --- a/core/java/android/accounts/ChooseTypeAndAccountActivity.java +++ b/core/java/android/accounts/ChooseTypeAndAccountActivity.java @@ -40,8 +40,8 @@ import com.android.internal.R; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -98,11 +98,10 @@ public class ChooseTypeAndAccountActivity extends Activity "alwaysPromptForAccount"; /** - * If set then this string willb e used as the description rather than + * If set then this string will be used as the description rather than * the default. */ - public static final String EXTRA_DESCRIPTION_TEXT_OVERRIDE = - "descriptionTextOverride"; + public static final String EXTRA_DESCRIPTION_TEXT_OVERRIDE = "descriptionTextOverride"; public static final int REQUEST_NULL = 0; public static final int REQUEST_CHOOSE_TYPE = 1; @@ -112,7 +111,8 @@ public class ChooseTypeAndAccountActivity extends Activity private static final String KEY_INSTANCE_STATE_EXISTING_ACCOUNTS = "existingAccounts"; private static final String KEY_INSTANCE_STATE_SELECTED_ACCOUNT_NAME = "selectedAccountName"; private static final String KEY_INSTANCE_STATE_SELECTED_ADD_ACCOUNT = "selectedAddAccount"; - private static final String KEY_INSTANCE_STATE_ACCOUNT_LIST = "accountAndVisibilityList"; + private static final String KEY_INSTANCE_STATE_ACCOUNTS_LIST = "accountsList"; + private static final String KEY_INSTANCE_STATE_VISIBILITY_LIST = "visibilityList"; private static final int SELECTED_ITEM_NONE = -1; @@ -122,7 +122,7 @@ public class ChooseTypeAndAccountActivity extends Activity private boolean mSelectedAddNewAccount = false; private String mDescriptionOverride; - private Map mAccounts; + private LinkedHashMap mAccounts; // TODO Redesign flow to show NOT_VISIBLE accounts // and display a warning if they are selected. // Currently NOT_VISBILE accounts are not shown at all. @@ -164,6 +164,10 @@ public class ChooseTypeAndAccountActivity extends Activity // save some items we use frequently final Intent intent = getIntent(); + mSetOfAllowableAccounts = getAllowableAccountSet(intent); + mSetOfRelevantAccountTypes = getReleventAccountTypes(intent); + mDescriptionOverride = intent.getStringExtra(EXTRA_DESCRIPTION_TEXT_OVERRIDE); + if (savedInstanceState != null) { mPendingRequest = savedInstanceState.getInt(KEY_INSTANCE_STATE_PENDING_REQUEST); mExistingAccounts = @@ -174,8 +178,15 @@ public class ChooseTypeAndAccountActivity extends Activity savedInstanceState.getString(KEY_INSTANCE_STATE_SELECTED_ACCOUNT_NAME); mSelectedAddNewAccount = savedInstanceState.getBoolean(KEY_INSTANCE_STATE_SELECTED_ADD_ACCOUNT, false); - mAccounts = (Map) savedInstanceState - .getSerializable(KEY_INSTANCE_STATE_ACCOUNT_LIST); + // restore mAccounts + Parcelable[] accounts = + savedInstanceState.getParcelableArray(KEY_INSTANCE_STATE_ACCOUNTS_LIST); + ArrayList visibility = + savedInstanceState.getIntegerArrayList(KEY_INSTANCE_STATE_VISIBILITY_LIST); + mAccounts = new LinkedHashMap<>(); + for (int i = 0; i < accounts.length; i++) { + mAccounts.put((Account) accounts[i], visibility.get(i)); + } } else { mPendingRequest = REQUEST_NULL; mExistingAccounts = null; @@ -185,20 +196,21 @@ public class ChooseTypeAndAccountActivity extends Activity if (selectedAccount != null) { mSelectedAccountName = selectedAccount.name; } + mAccounts = getAcceptableAccountChoices(AccountManager.get(this)); } if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "selected account name is " + mSelectedAccountName); } + mPossiblyVisibleAccounts = new ArrayList<>(mAccounts.size()); + for (Map.Entry entry : mAccounts.entrySet()) { + if (AccountManager.VISIBILITY_NOT_VISIBLE != entry.getValue()) { + mPossiblyVisibleAccounts.add(entry.getKey()); + } + } - mSetOfAllowableAccounts = getAllowableAccountSet(intent); - mSetOfRelevantAccountTypes = getReleventAccountTypes(intent); - mDescriptionOverride = intent.getStringExtra(EXTRA_DESCRIPTION_TEXT_OVERRIDE); - - mAccounts = getAcceptableAccountChoices(AccountManager.get(this)); - if (mAccounts.isEmpty() - && mDisallowAddAccounts) { + if (mPossiblyVisibleAccounts.isEmpty() && mDisallowAddAccounts) { requestWindowFeature(Window.FEATURE_NO_TITLE); setContentView(R.layout.app_not_authorized); mDontShowPicker = true; @@ -216,7 +228,7 @@ public class ChooseTypeAndAccountActivity extends Activity if (mPendingRequest == REQUEST_NULL) { // If there are no relevant accounts and only one relevant account type go directly to // add account. Otherwise let the user choose. - if (mAccounts.isEmpty()) { + if (mPossiblyVisibleAccounts.isEmpty()) { setNonLabelThemeAndCallSuperCreate(savedInstanceState); if (mSetOfRelevantAccountTypes.size() == 1) { runAddAccountForAuthenticator(mSetOfRelevantAccountTypes.iterator().next()); @@ -226,12 +238,6 @@ public class ChooseTypeAndAccountActivity extends Activity } } - mPossiblyVisibleAccounts = new ArrayList<>(mAccounts.size()); - for (Map.Entry entry : mAccounts.entrySet()) { - if (AccountManager.VISIBILITY_NOT_VISIBLE != entry.getValue()) { - mPossiblyVisibleAccounts.add(entry.getKey()); - } - } String[] listItems = getListOfDisplayableOptions(mPossiblyVisibleAccounts); mSelectedItemIndex = getItemIndexToSelect(mPossiblyVisibleAccounts, mSelectedAccountName, mSelectedAddNewAccount); @@ -270,10 +276,16 @@ public class ChooseTypeAndAccountActivity extends Activity mPossiblyVisibleAccounts.get(mSelectedItemIndex).name); } } - // should be HashMap by default. - HashMap accountsHashMap = (mAccounts instanceof HashMap) - ? (HashMap) mAccounts : new HashMap(mAccounts); - outState.putSerializable(KEY_INSTANCE_STATE_ACCOUNT_LIST, accountsHashMap); + // save mAccounts + Parcelable[] accounts = new Parcelable[mAccounts.size()]; + ArrayList visibility = new ArrayList<>(mAccounts.size()); + int i = 0; + for (Map.Entry e : mAccounts.entrySet()) { + accounts[i++] = e.getKey(); + visibility.add(e.getValue()); + } + outState.putParcelableArray(KEY_INSTANCE_STATE_ACCOUNTS_LIST, accounts); + outState.putIntegerArrayList(KEY_INSTANCE_STATE_VISIBILITY_LIST, visibility); } public void onCancelButtonClicked(View view) { @@ -308,7 +320,7 @@ public class ChooseTypeAndAccountActivity extends Activity if (resultCode == RESULT_CANCELED) { // if canceling out of addAccount and the original state caused us to skip this, // finish this activity - if (mAccounts.isEmpty()) { + if (mPossiblyVisibleAccounts.isEmpty()) { setResult(Activity.RESULT_CANCELED); finish(); } @@ -428,18 +440,20 @@ public class ChooseTypeAndAccountActivity extends Activity private void setResultAndFinish(final String accountName, final String accountType) { // Mark account as visible since user chose it. Account account = new Account(accountName, accountType); - Integer oldVisibility = mAccounts.get(account); - // oldVisibility is null if new account was added - if (oldVisibility == null) { - Map accountsAndVisibility = AccountManager.get(this) - .getAccountsAndVisibilityForPackage(mCallingPackage, null /* type */); - oldVisibility = accountsAndVisibility.get(account); - } + Integer oldVisibility = + AccountManager.get(this).getAccountVisibility(account, mCallingPackage); if (oldVisibility != null && oldVisibility == AccountManager.VISIBILITY_USER_MANAGED_NOT_VISIBLE) { AccountManager.get(this).setAccountVisibility(account, mCallingPackage, AccountManager.VISIBILITY_USER_MANAGED_VISIBLE); } + + if (oldVisibility != null && oldVisibility == AccountManager.VISIBILITY_NOT_VISIBLE) { + // Added account is not visible to caller. + setResult(Activity.RESULT_CANCELED); + finish(); + return; + } Bundle bundle = new Bundle(); bundle.putString(AccountManager.KEY_ACCOUNT_NAME, accountName); bundle.putString(AccountManager.KEY_ACCOUNT_TYPE, accountType); @@ -448,6 +462,7 @@ public class ChooseTypeAndAccountActivity extends Activity Log.v(TAG, "ChooseTypeAndAccountActivity.setResultAndFinish: selected account " + accountName + ", " + accountType); } + finish(); } @@ -509,22 +524,24 @@ public class ChooseTypeAndAccountActivity extends Activity * that don't match the allowable types, if provided, or that don't match the allowable * accounts, if provided. */ - private Map getAcceptableAccountChoices(AccountManager accountManager) { - Map accountsAndVisibility = - accountManager.getAccountsAndVisibilityForPackage(mCallingPackage, null /* type */); - - Map accountsToPopulate = - new HashMap(accountsAndVisibility.size()); - for (Map.Entry entry : accountsAndVisibility.entrySet()) { + private LinkedHashMap getAcceptableAccountChoices(AccountManager accountManager) { + Map accountsAndVisibilityForCaller = + accountManager.getAccountsAndVisibilityForPackage(mCallingPackage, null); + Account[] allAccounts = accountManager.getAccounts(); + LinkedHashMap accountsToPopulate = + new LinkedHashMap<>(accountsAndVisibilityForCaller.size()); + for (Account account : allAccounts) { if (mSetOfAllowableAccounts != null - && !mSetOfAllowableAccounts.contains(entry.getKey())) { + && !mSetOfAllowableAccounts.contains(account)) { continue; } if (mSetOfRelevantAccountTypes != null - && !mSetOfRelevantAccountTypes.contains(entry.getKey().type)) { + && !mSetOfRelevantAccountTypes.contains(account.type)) { continue; } - accountsToPopulate.put(entry.getKey(), entry.getValue()); + if (accountsAndVisibilityForCaller.get(account) != null) { + accountsToPopulate.put(account, accountsAndVisibilityForCaller.get(account)); + } } return accountsToPopulate; }