PawletOS/app_Settings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix bug where maximum provider limit was not being reflected

Browse Source 
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
This commit is contained in:
Becca Hughes
2024-02-09 21:08:18 +00:00
parent 8cff839cd7
commit f368299931
2 changed files with 49 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
View File

@@ -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();

31
tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java
View File

@@ -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<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 @@ public class CredentialManagerPreferenceControllerTest {
// 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 @@ 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);