diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java index a92b755331c..ca3786a443b 100644 --- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java +++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java @@ -115,6 +115,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl private @Nullable Delegate mDelegate = null; private @Nullable String mFlagOverrideForTest = null; private @Nullable PreferenceScreen mPreferenceScreen = null; + private @Nullable PreferenceGroup mPreferenceGroup = null; private Optional mSimulateHiddenForTests = Optional.empty(); private boolean mIsWorkProfile = false; @@ -161,12 +162,6 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl return UNSUPPORTED_ON_DEVICE; } - // If there is no top provider or any providers in the list then - // we should hide this pref. - if (isHiddenDueToNoProviderSet()) { - return CONDITIONALLY_UNAVAILABLE; - } - if (!hasNonPrimaryServices()) { return CONDITIONALLY_UNAVAILABLE; } @@ -355,24 +350,11 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl } // Get the list of new providers and components. - List newProviders = + setAvailableServices( mCredentialManager.getCredentialProviderServices( getUser(), - CredentialManager.PROVIDER_FILTER_USER_PROVIDERS_INCLUDING_HIDDEN); - Set newComponents = buildComponentNameSet(newProviders, false); - Set newPrimaryComponents = buildComponentNameSet(newProviders, true); - - // Get the list of old components - Set oldComponents = buildComponentNameSet(mServices, false); - Set oldPrimaryComponents = buildComponentNameSet(mServices, true); - - // If the sets are equal then don't update the UI. - if (oldComponents.equals(newComponents) - && oldPrimaryComponents.equals(newPrimaryComponents)) { - return; - } - - setAvailableServices(newProviders, null); + CredentialManager.PROVIDER_FILTER_USER_PROVIDERS_INCLUDING_HIDDEN), + null); if (mPreferenceScreen != null) { displayPreference(mPreferenceScreen); @@ -396,11 +378,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl } @VisibleForTesting - public boolean isHiddenDueToNoProviderSet() { - return isHiddenDueToNoProviderSet(getProviders()); - } - - private boolean isHiddenDueToNoProviderSet( + public boolean isHiddenDueToNoProviderSet( Pair, CombinedProviderInfo> providerPair) { if (mSimulateHiddenForTests.isPresent()) { return mSimulateHiddenForTests.get(); @@ -444,17 +422,67 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl @Override public void displayPreference(PreferenceScreen screen) { - super.displayPreference(screen); + final String prefKey = getPreferenceKey(); + if (TextUtils.isEmpty(prefKey)) { + Log.w(TAG, "Skipping displayPreference because key is empty"); + return; + } - // Since the UI is being cleared, clear any refs. + // Store this reference for later. + if (mPreferenceScreen == null) { + mPreferenceScreen = screen; + mPreferenceGroup = screen.findPreference(prefKey); + } + + final Pair, CombinedProviderInfo> providerPair = getProviders(); + + maybeUpdateListOfPrefs(providerPair); + maybeUpdatePreferenceVisibility(providerPair); + } + + private void maybeUpdateListOfPrefs( + Pair, CombinedProviderInfo> providerPair) { + if (mPreferenceScreen == null || mPreferenceGroup == null) { + return; + } + + // Build the new list of prefs. + Map newPrefs = + buildPreferenceList(mPreferenceScreen.getContext(), providerPair); + + // Determine if we need to update the prefs. + Set existingPrefPackageNames = mPrefs.keySet(); + if (existingPrefPackageNames.equals(newPrefs.keySet())) { + return; + } + + // Since the UI is being cleared, clear any refs and prefs. mPrefs.clear(); + mPreferenceGroup.removeAll(); - mPreferenceScreen = screen; - PreferenceGroup group = screen.findPreference(getPreferenceKey()); - group.removeAll(); + // Populate the preference list with new data. + mPrefs.putAll(newPrefs); + for (CombiPreference pref : newPrefs.values()) { + mPreferenceGroup.addPreference(pref); + } + } - Context context = screen.getContext(); - mPrefs.putAll(buildPreferenceList(context, group)); + private void maybeUpdatePreferenceVisibility( + Pair, CombinedProviderInfo> providerPair) { + if (mPreferenceScreen == null || mPreferenceGroup == null) { + return; + } + + final boolean isAvailable = + (getAvailabilityStatus() == AVAILABLE) && !isHiddenDueToNoProviderSet(providerPair); + + if (isAvailable) { + mPreferenceScreen.addPreference(mPreferenceGroup); + mPreferenceGroup.setVisible(true); + } else { + mPreferenceScreen.removePreference(mPreferenceGroup); + mPreferenceGroup.setVisible(false); + } } /** @@ -511,9 +539,9 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl /** Aggregates the list of services and builds a list of UI prefs to show. */ @VisibleForTesting public @NonNull Map buildPreferenceList( - @NonNull Context context, @NonNull PreferenceGroup group) { - // Get the providers and extract the values. - Pair, CombinedProviderInfo> providerPair = getProviders(); + @NonNull Context context, + @NonNull Pair, CombinedProviderInfo> providerPair) { + // Extract the values. CombinedProviderInfo topProvider = providerPair.second; List providers = providerPair.first; @@ -554,7 +582,6 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl combinedInfo.getSettingsActivity(), combinedInfo.getDeviceAdminRestrictions(context, getUser())); output.put(packageName, pref); - group.addPreference(pref); } // Set the visibility if we have services. @@ -1023,11 +1050,12 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl public void onClick(View buttonView) { // Forward the event. 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); + 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); } } } @@ -1083,8 +1111,10 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl if (mSwitch != null && !TextUtils.isEmpty(appName)) { mSwitch.setContentDescription( - getContext().getString( - R.string.credman_on_off_switch_content_description, appName)); + getContext() + .getString( + R.string.credman_on_off_switch_content_description, + appName)); } } 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 c0023ee8afd..3cd15330a2f 100644 --- a/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java +++ b/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java @@ -17,7 +17,6 @@ package com.android.settings.applications.credentials; import static com.android.settings.core.BasePreferenceController.AVAILABLE; -import static com.android.settings.core.BasePreferenceController.CONDITIONALLY_UNAVAILABLE; import static com.android.settings.core.BasePreferenceController.UNSUPPORTED_ON_DEVICE; import static com.google.common.truth.Truth.assertThat; @@ -36,7 +35,9 @@ import android.graphics.drawable.Drawable; import android.net.Uri; import android.os.Looper; import android.provider.Settings; +import android.util.Pair; +import androidx.annotation.Nullable; import androidx.preference.Preference; import androidx.preference.PreferenceCategory; import androidx.preference.PreferenceManager; @@ -124,19 +125,37 @@ public class CredentialManagerPreferenceControllerTest { controller.setSimulateConnectedForTests(true); assertThat(controller.isConnected()).isTrue(); controller.setSimulateHiddenForTests(Optional.of(false)); - assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); + assertThat(controller.isHiddenDueToNoProviderSet(createPair())).isFalse(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); } @Test - public void getAvailabilityStatus_isHidden_returnsConditionallyUnavailable() { + public void isHiddenDueToNoProviderSet_hiddenDueToEmptyPair() { CredentialManagerPreferenceController controller = createControllerWithServices(Lists.newArrayList(createCredentialProviderInfo())); - controller.setSimulateConnectedForTests(true); - assertThat(controller.isConnected()).isTrue(); - controller.setSimulateHiddenForTests(Optional.of(true)); - assertThat(controller.isHiddenDueToNoProviderSet()).isTrue(); - assertThat(controller.getAvailabilityStatus()).isEqualTo(CONDITIONALLY_UNAVAILABLE); + assertThat(controller.isHiddenDueToNoProviderSet(createPair())).isTrue(); + } + + @Test + public void isHiddenDueToNoProviderSet_hiddenDueToNoPrimaryProvider() { + CredentialManagerPreferenceController controller = + createControllerWithServices(Lists.newArrayList(createCredentialProviderInfo())); + + Pair, CombinedProviderInfo> testPair = + new Pair<>(Lists.newArrayList(createCombinedProviderInfo()), null); + assertThat(controller.isHiddenDueToNoProviderSet(testPair)).isTrue(); + } + + @Test + public void isHiddenDueToNoProviderSet_validDataSoNotHidden() { + CredentialManagerPreferenceController controller = + createControllerWithServices(Lists.newArrayList(createCredentialProviderInfo())); + + Pair, CombinedProviderInfo> testPair = + new Pair<>( + Lists.newArrayList(createCombinedProviderInfo()), + createCombinedProviderInfo()); + assertThat(controller.isHiddenDueToNoProviderSet(testPair)).isFalse(); } @Test @@ -146,7 +165,7 @@ public class CredentialManagerPreferenceControllerTest { controller.setSimulateConnectedForTests(true); controller.setSimulateHiddenForTests(Optional.of(false)); - assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); + assertThat(controller.isHiddenDueToNoProviderSet(createPair())).isFalse(); assertThat(controller.isConnected()).isTrue(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); @@ -170,7 +189,7 @@ public class CredentialManagerPreferenceControllerTest { controller.setSimulateConnectedForTests(true); assertThat(controller.isConnected()).isTrue(); controller.setSimulateHiddenForTests(Optional.of(false)); - assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); + assertThat(controller.isHiddenDueToNoProviderSet(createPair())).isFalse(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); // Test the data is correct. @@ -214,7 +233,7 @@ public class CredentialManagerPreferenceControllerTest { controller.setSimulateConnectedForTests(true); assertThat(controller.isConnected()).isTrue(); controller.setSimulateHiddenForTests(Optional.of(false)); - assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); + assertThat(controller.isHiddenDueToNoProviderSet(createPair())).isFalse(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); // Ensure that we stay under 5 providers (one is reserved for primary). @@ -283,7 +302,7 @@ public class CredentialManagerPreferenceControllerTest { controller.setSimulateConnectedForTests(true); assertThat(controller.isConnected()).isTrue(); controller.setSimulateHiddenForTests(Optional.of(false)); - assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); + assertThat(controller.isHiddenDueToNoProviderSet(createPair())).isFalse(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); // Test the data is correct. @@ -336,17 +355,26 @@ public class CredentialManagerPreferenceControllerTest { createControllerWithServices( Lists.newArrayList(serviceA1, serviceB1, serviceC1, serviceC2, serviceC3)); controller.setSimulateConnectedForTests(true); - controller.setSimulateHiddenForTests(Optional.of(false)); - assertThat(controller.isHiddenDueToNoProviderSet()).isFalse(); assertThat(controller.isConnected()).isTrue(); assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE); - controller.displayPreference(mScreen); - assertThat(mCredentialsPreferenceCategory.getPreferenceCount()).isEqualTo(3); + CombinedProviderInfo combinedProviderA = + new CombinedProviderInfo(Lists.newArrayList(serviceA1), null, false, false); + CombinedProviderInfo combinedProviderB = + new CombinedProviderInfo(Lists.newArrayList(serviceB1), null, false, false); + CombinedProviderInfo combinedProviderC = + new CombinedProviderInfo( + Lists.newArrayList(serviceC1, serviceC2, serviceC3), null, false, false); + Pair, CombinedProviderInfo> providerPair = + createPair( + Lists.newArrayList(combinedProviderA, combinedProviderB, combinedProviderC), + createCombinedProviderInfo()); + + assertThat(controller.isHiddenDueToNoProviderSet(providerPair)).isFalse(); Map prefs = - controller.buildPreferenceList(mContext, mCredentialsPreferenceCategory); + controller.buildPreferenceList(mContext, providerPair); assertThat(prefs.keySet()) .containsExactly(TEST_PACKAGE_NAME_A, TEST_PACKAGE_NAME_B, TEST_PACKAGE_NAME_C); assertThat(prefs.size()).isEqualTo(3); @@ -531,8 +559,7 @@ public class CredentialManagerPreferenceControllerTest { @Test public void hasNonPrimaryServices_allServicesArePrimary() { CredentialManagerPreferenceController controller = - createControllerWithServices( - Lists.newArrayList(createCredentialProviderPrimary())); + createControllerWithServices(Lists.newArrayList(createCredentialProviderPrimary())); assertThat(controller.hasNonPrimaryServices()).isFalse(); } @@ -540,8 +567,8 @@ public class CredentialManagerPreferenceControllerTest { public void hasNonPrimaryServices_mixtureOfServices() { CredentialManagerPreferenceController controller = createControllerWithServices( - Lists.newArrayList(createCredentialProviderInfo(), - createCredentialProviderPrimary())); + Lists.newArrayList( + createCredentialProviderInfo(), createCredentialProviderPrimary())); assertThat(controller.hasNonPrimaryServices()).isTrue(); } @@ -599,11 +626,25 @@ public class CredentialManagerPreferenceControllerTest { private CredentialProviderInfo createCredentialProviderPrimary() { return createCredentialProviderInfoBuilder( - "com.android.primary", "CredManProvider", "Service Label", "App Name") + "com.android.primary", "CredManProvider", "Service Label", "App Name") .setPrimary(true) .build(); } + private Pair, CombinedProviderInfo> createPair() { + return createPair(Lists.newArrayList(), null); + } + + private Pair, CombinedProviderInfo> createPair( + List providers, @Nullable CombinedProviderInfo primaryProvider) { + return new Pair<>(providers, primaryProvider); + } + + private CombinedProviderInfo createCombinedProviderInfo() { + return new CombinedProviderInfo( + Lists.newArrayList(createCredentialProviderInfo()), null, false, false); + } + private CredentialProviderInfo createCredentialProviderInfoWithSubtitle( String packageName, String className, CharSequence label, CharSequence subtitle) { ServiceInfo si = new ServiceInfo();