diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java index d98bc51c794..be0658e8c64 100644 --- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java +++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java @@ -568,7 +568,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl */ @VisibleForTesting public boolean togglePackageNameEnabled(String packageName) { - if (mEnabledPackageNames.size() >= MAX_SELECTABLE_PROVIDERS) { + if (hasProviderLimitBeenReached()) { return false; } else { mEnabledPackageNames.add(packageName); @@ -623,6 +623,19 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl return mIconResizer.createIconThumbnail(providerIcon); } + private boolean hasProviderLimitBeenReached() { + return hasProviderLimitBeenReached(mEnabledPackageNames.size()); + } + + @VisibleForTesting + public static boolean hasProviderLimitBeenReached(int enabledAdditionalProviderCount) { + // If the number of package names has reached the maximum limit then + // we should stop any new packages from being added. We will also + // reserve one place for the primary provider so if the max limit is + // five providers this will be four additional plus the primary. + return (enabledAdditionalProviderCount + 1) >= MAX_SELECTABLE_PROVIDERS; + } + private CombiPreference addProviderPreference( @NonNull Context prefContext, @NonNull CharSequence title, @@ -648,19 +661,18 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl pref.setPreferenceListener( new CombiPreference.OnCombiPreferenceClickListener() { @Override - public void onCheckChanged(CombiPreference p, boolean isChecked) { + public boolean onCheckChanged(CombiPreference p, boolean isChecked) { if (isChecked) { - if (mEnabledPackageNames.size() >= MAX_SELECTABLE_PROVIDERS) { + if (hasProviderLimitBeenReached()) { // Show the error if too many enabled. - pref.setChecked(false); final DialogFragment fragment = newErrorDialogFragment(); if (fragment == null || mFragmentManager == null) { - return; + return false; } fragment.show(mFragmentManager, ErrorDialogFragment.TAG); - return; + return false; } togglePackageNameEnabled(packageName); @@ -672,6 +684,8 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl } else { togglePackageNameDisabled(packageName); } + + return true; } @Override @@ -989,8 +1003,13 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl @Override public void onClick(View buttonView) { // Forward the event. - if (mSwitch != null) { - mOnClickListener.onCheckChanged(CombiPreference.this, mSwitch.isChecked()); + if (mSwitch != null && mOnClickListener != null) { + if (!mOnClickListener.onCheckChanged(CombiPreference.this, mSwitch.isChecked())) { + // The update was not successful since there were too + // many enabled providers to manually reset any state. + mChecked = false; + mSwitch.setChecked(false); + } } } } @@ -1004,7 +1023,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl public interface OnCombiPreferenceClickListener { /** Called when the check is updated */ - void onCheckChanged(CombiPreference p, boolean isChecked); + boolean onCheckChanged(CombiPreference p, boolean isChecked); /** Called when the left side is clicked. */ void onLeftSideClicked(); diff --git a/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java b/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java index b5aeac75ff7..0a04a739b53 100644 --- a/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java +++ b/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java @@ -217,33 +217,33 @@ public class CredentialManagerPreferenceControllerTest { assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); - // Ensure that we stay under 5 providers. + // Ensure that we stay under 5 providers (one is reserved for primary). assertThat(controller.togglePackageNameEnabled("com.android.provider1")).isTrue(); assertThat(controller.togglePackageNameEnabled("com.android.provider2")).isTrue(); assertThat(controller.togglePackageNameEnabled("com.android.provider3")).isTrue(); assertThat(controller.togglePackageNameEnabled("com.android.provider4")).isTrue(); - assertThat(controller.togglePackageNameEnabled("com.android.provider5")).isTrue(); + assertThat(controller.togglePackageNameEnabled("com.android.provider5")).isFalse(); assertThat(controller.togglePackageNameEnabled("com.android.provider6")).isFalse(); // Check that they are all actually registered. Set enabledProviders = controller.getEnabledProviders(); - assertThat(enabledProviders.size()).isEqualTo(5); + assertThat(enabledProviders.size()).isEqualTo(4); assertThat(enabledProviders.contains("com.android.provider1")).isTrue(); assertThat(enabledProviders.contains("com.android.provider2")).isTrue(); assertThat(enabledProviders.contains("com.android.provider3")).isTrue(); assertThat(enabledProviders.contains("com.android.provider4")).isTrue(); - assertThat(enabledProviders.contains("com.android.provider5")).isTrue(); + assertThat(enabledProviders.contains("com.android.provider5")).isFalse(); assertThat(enabledProviders.contains("com.android.provider6")).isFalse(); // Check that the settings string has the right component names. List enabledServices = controller.getEnabledSettings(); - assertThat(enabledServices.size()).isEqualTo(6); + assertThat(enabledServices.size()).isEqualTo(5); assertThat(enabledServices.contains("com.android.provider1/ClassA")).isTrue(); assertThat(enabledServices.contains("com.android.provider1/ClassB")).isTrue(); assertThat(enabledServices.contains("com.android.provider2/ClassA")).isTrue(); assertThat(enabledServices.contains("com.android.provider3/ClassA")).isTrue(); assertThat(enabledServices.contains("com.android.provider4/ClassA")).isTrue(); - assertThat(enabledServices.contains("com.android.provider5/ClassA")).isTrue(); + assertThat(enabledServices.contains("com.android.provider5/ClassA")).isFalse(); assertThat(enabledServices.contains("com.android.provider6/ClassA")).isFalse(); // Toggle the provider disabled. @@ -251,22 +251,22 @@ public class CredentialManagerPreferenceControllerTest { // Check that the provider was removed from the list of providers. Set currentlyEnabledProviders = controller.getEnabledProviders(); - assertThat(currentlyEnabledProviders.size()).isEqualTo(4); + assertThat(currentlyEnabledProviders.size()).isEqualTo(3); assertThat(currentlyEnabledProviders.contains("com.android.provider1")).isTrue(); assertThat(currentlyEnabledProviders.contains("com.android.provider2")).isFalse(); assertThat(currentlyEnabledProviders.contains("com.android.provider3")).isTrue(); assertThat(currentlyEnabledProviders.contains("com.android.provider4")).isTrue(); - assertThat(currentlyEnabledProviders.contains("com.android.provider5")).isTrue(); + assertThat(currentlyEnabledProviders.contains("com.android.provider5")).isFalse(); assertThat(currentlyEnabledProviders.contains("com.android.provider6")).isFalse(); // Check that the provider was removed from the list of services stored in the setting. List currentlyEnabledServices = controller.getEnabledSettings(); - assertThat(currentlyEnabledServices.size()).isEqualTo(5); + assertThat(currentlyEnabledServices.size()).isEqualTo(4); assertThat(currentlyEnabledServices.contains("com.android.provider1/ClassA")).isTrue(); assertThat(currentlyEnabledServices.contains("com.android.provider1/ClassB")).isTrue(); assertThat(currentlyEnabledServices.contains("com.android.provider3/ClassA")).isTrue(); assertThat(currentlyEnabledServices.contains("com.android.provider4/ClassA")).isTrue(); - assertThat(currentlyEnabledServices.contains("com.android.provider5/ClassA")).isTrue(); + assertThat(currentlyEnabledServices.contains("com.android.provider5/ClassA")).isFalse(); assertThat(currentlyEnabledServices.contains("com.android.provider6/ClassA")).isFalse(); } @@ -528,6 +528,17 @@ public class CredentialManagerPreferenceControllerTest { assertThat(thumbnail.getIntrinsicWidth()).isEqualTo(getIconSize()); } + @Test + public void testProviderLimitReached() { + // The limit is 5 with one slot reserved for primary. + assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(0)).isFalse(); + assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(1)).isFalse(); + assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(2)).isFalse(); + assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(3)).isFalse(); + assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(4)).isTrue(); + assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(5)).isTrue(); + } + private int getIconSize() { final Resources resources = mContext.getResources(); return (int) resources.getDimension(android.R.dimen.app_icon_size);