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

Fix bug where empty section is sometimes shown

Browse Source 
There is a bug where the empty section is
sometimes shown for additional providers. This
is because the logic for calculating the
visibility gets the providers and then the
logic for displaying the list gets the providers
and can result in getting out of sync. This aligns
both sets of logic to use the same data.

Test: ondevice & unit
Bug: 330163369
Change-Id: Icf71c78d67d55c929476f8e699d5893e60a49776
This commit is contained in:
Becca Hughes
2024-04-17 12:49:07 -07:00
parent 9640114704
commit 1c3fb49913
2 changed files with 138 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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<Boolean> 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<CredentialProviderInfo> newProviders =
setAvailableServices(
mCredentialManager.getCredentialProviderServices(
getUser(),
CredentialManager.PROVIDER_FILTER_USER_PROVIDERS_INCLUDING_HIDDEN);
Set<ComponentName> newComponents = buildComponentNameSet(newProviders, false);
Set<ComponentName> newPrimaryComponents = buildComponentNameSet(newProviders, true);
// Get the list of old components
Set<ComponentName> oldComponents = buildComponentNameSet(mServices, false);
Set<ComponentName> 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<List<CombinedProviderInfo>, 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<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair = getProviders();
maybeUpdateListOfPrefs(providerPair);
maybeUpdatePreferenceVisibility(providerPair);
}
private void maybeUpdateListOfPrefs(
Pair<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair) {
if (mPreferenceScreen == null || mPreferenceGroup == null) {
return;
}
// Build the new list of prefs.
Map<String, CombiPreference> newPrefs =
buildPreferenceList(mPreferenceScreen.getContext(), providerPair);
// Determine if we need to update the prefs.
Set<String> 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<List<CombinedProviderInfo>, 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<String, CombiPreference> buildPreferenceList(
@NonNull Context context, @NonNull PreferenceGroup group) {
// Get the providers and extract the values.
Pair<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair = getProviders();
@NonNull Context context,
@NonNull Pair<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair) {
// Extract the values.
CombinedProviderInfo topProvider = providerPair.second;
List<CombinedProviderInfo> 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));
}
}

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

@@ -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<List<CombinedProviderInfo>, 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<List<CombinedProviderInfo>, 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<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair =
createPair(
Lists.newArrayList(combinedProviderA, combinedProviderB, combinedProviderC),
createCombinedProviderInfo());
assertThat(controller.isHiddenDueToNoProviderSet(providerPair)).isFalse();
Map<String, CredentialManagerPreferenceController.CombiPreference> 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<List<CombinedProviderInfo>, CombinedProviderInfo> createPair() {
return createPair(Lists.newArrayList(), null);
}
private Pair<List<CombinedProviderInfo>, CombinedProviderInfo> createPair(
List<CombinedProviderInfo> 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();