Fix bug where maximum provider limit was not being reflected
The limit of 5 providers had two issues, one it did not
reserve one slot for primary which meant that you could
enable 6 and also even though it wouldn't save and would
show an error when you hit that limit the toggle was not
reset which made it look like it was enabled.
Test: on device + unit
Bug: 324426504
Change-Id: Ibec4efd6394835729869194181161fe8ae743e76
diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
index d98bc51..be0658e 100644
--- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
+++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
@@ -568,7 +568,7 @@
*/
@VisibleForTesting
public boolean togglePackageNameEnabled(String packageName) {
- if (mEnabledPackageNames.size() >= MAX_SELECTABLE_PROVIDERS) {
+ if (hasProviderLimitBeenReached()) {
return false;
} else {
mEnabledPackageNames.add(packageName);
@@ -623,6 +623,19 @@
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 @@
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 @@
} else {
togglePackageNameDisabled(packageName);
}
+
+ return true;
}
@Override
@@ -989,8 +1003,13 @@
@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 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 b5aeac7..0a04a73 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 @@
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<String> 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<String> 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 @@
// Check that the provider was removed from the list of providers.
Set<String> 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<String> 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 @@
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);