Merge "Fix bug where maximum provider limit was not being reflected" into main

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) {
+                if (mSwitch != null && mOnClickListener != null) {
-                    mOnClickListener.onCheckChanged(CombiPreference.this, mSwitch.isChecked());
+                    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();

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