Fix settings flakiness bug

The flakiness was caused by mVisibility
being out of sync and resulting in the
preference being hidden / shown incorrectly.

Test: manual & unit tests
Bug: 322072349
Change-Id: I588da2100b8f2cea38f2bdb63af4e8d19d0efd0b

src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java

@@ -46,6 +46,7 @@ import android.provider.Settings;
 import android.service.autofill.AutofillServiceInfo;
 import android.text.TextUtils;
 import android.util.Log;
+import android.util.Pair;
 import android.view.View;
 import android.widget.CompoundButton;
 
@@ -77,6 +78,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Executor;
 
@@ -114,7 +116,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl
     private @Nullable String mFlagOverrideForTest = null;
     private @Nullable PreferenceScreen mPreferenceScreen = null;
 
-    private boolean mVisibility = false;
+    private Optional<Boolean> mSimulateHiddenForTests = Optional.empty();
     private boolean mIsWorkProfile = false;
     private boolean mSimulateConnectedForTests = false;
 
@@ -159,7 +161,9 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl
             return UNSUPPORTED_ON_DEVICE;
         }
 
-        if (!mVisibility) {
+        // If there is no top provider or any providers in the list then
+        // we should hide this pref.
+        if (isHiddenDueToNoProviderSet()) {
             return CONDITIONALLY_UNAVAILABLE;
         }
 
@@ -378,20 +382,29 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl
     }
 
     @VisibleForTesting
-    public void setVisibility(boolean newVisibility) {
+    public void forceDelegateRefresh() {
-        if (newVisibility == mVisibility) {
-            return;
-        }
-
-        mVisibility = newVisibility;
         if (mDelegate != null) {
             mDelegate.forceDelegateRefresh();
         }
     }
 
     @VisibleForTesting
-    public boolean getVisibility() {
+    public void setSimulateHiddenForTests(Optional<Boolean> simulateHiddenForTests) {
-        return mVisibility;
+        mSimulateHiddenForTests = simulateHiddenForTests;
+    }
+
+    @VisibleForTesting
+    public boolean isHiddenDueToNoProviderSet() {
+        return isHiddenDueToNoProviderSet(getProviders());
+    }
+
+    private boolean isHiddenDueToNoProviderSet(
+            Pair<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair) {
+        if (mSimulateHiddenForTests.isPresent()) {
+            return mSimulateHiddenForTests.get();
+        }
+
+        return (providerPair.first.size() == 0 || providerPair.second == null);
     }
 
     @VisibleForTesting
@@ -459,10 +472,11 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl
         return preference;
     }
 
-    /** Aggregates the list of services and builds a list of UI prefs to show. */
+    /**
-    @VisibleForTesting
+     * Returns a pair that contains a list of the providers in the first position and the top
-    public Map<String, CombiPreference> buildPreferenceList(
+     * provider in the second position.
-            Context context, PreferenceGroup group) {
+     */
+    private Pair<List<CombinedProviderInfo>, CombinedProviderInfo> getProviders() {
         // Get the selected autofill provider. If it is the placeholder then replace it with an
         // empty string.
         String selectedAutofillProvider =
@@ -475,15 +489,25 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl
         // Get the list of combined providers.
         List<CombinedProviderInfo> providers =
                 CombinedProviderInfo.buildMergedList(
-                        AutofillServiceInfo.getAvailableServices(context, getUser()),
+                        AutofillServiceInfo.getAvailableServices(mContext, getUser()),
                         mServices,
                         selectedAutofillProvider);
+        return new Pair<>(providers, CombinedProviderInfo.getTopProvider(providers));
+    }
 
-        // Get the provider that is displayed at the top. If there is none then hide
+    /** Aggregates the list of services and builds a list of UI prefs to show. */
-        // everything.
+    @VisibleForTesting
-        CombinedProviderInfo topProvider = CombinedProviderInfo.getTopProvider(providers);
+    public @NonNull Map<String, CombiPreference> buildPreferenceList(
-        if (topProvider == null) {
+            @NonNull Context context, @NonNull PreferenceGroup group) {
-            setVisibility(false);
+        // Get the providers and extract the values.
+        Pair<List<CombinedProviderInfo>, CombinedProviderInfo> providerPair = getProviders();
+        CombinedProviderInfo topProvider = providerPair.second;
+        List<CombinedProviderInfo> providers = providerPair.first;
+
+        // If the provider is set to "none" or there are no providers then we should not
+        // return any providers.
+        if (isHiddenDueToNoProviderSet(providerPair)) {
+            forceDelegateRefresh();
             return new HashMap<>();
         }
 
@@ -520,7 +544,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl
         }
 
         // Set the visibility if we have services.
-        setVisibility(!output.isEmpty());
+        forceDelegateRefresh();
 
         return output;
     }

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

@@ -17,6 +17,7 @@
 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;
@@ -55,6 +56,7 @@ import org.junit.runner.RunWith;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 @RunWith(AndroidJUnit4.class)
@@ -121,17 +123,34 @@ public class CredentialManagerPreferenceControllerTest {
                 createControllerWithServices(Lists.newArrayList(createCredentialProviderInfo()));
         controller.setSimulateConnectedForTests(true);
         assertThat(controller.isConnected()).isTrue();
-        controller.setVisibility(true);
+        controller.setSimulateHiddenForTests(Optional.of(false));
-        assertThat(controller.getVisibility()).isTrue();
+        assertThat(controller.isHiddenDueToNoProviderSet()).isFalse();
         assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE);
     }
 
+    @Test
+    public void getAvailabilityStatus_isHidden_returnsConditionallyUnavailable() {
+        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);
+    }
+
     @Test
     public void displayPreference_withServices_preferencesAdded() {
         CredentialManagerPreferenceController controller =
                 createControllerWithServices(Lists.newArrayList(createCredentialProviderInfo()));
+        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(controller.isConnected()).isFalse();
         assertThat(mCredentialsPreferenceCategory.getPreferenceCount()).isEqualTo(1);
 
         Preference pref = mCredentialsPreferenceCategory.getPreference(0);
@@ -150,8 +169,8 @@ public class CredentialManagerPreferenceControllerTest {
                 createControllerWithServices(Lists.newArrayList(providerInfo1, providerInfo2));
         controller.setSimulateConnectedForTests(true);
         assertThat(controller.isConnected()).isTrue();
-        controller.setVisibility(true);
+        controller.setSimulateHiddenForTests(Optional.of(false));
-        assertThat(controller.getVisibility()).isTrue();
+        assertThat(controller.isHiddenDueToNoProviderSet()).isFalse();
         assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE);
 
         // Test the data is correct.
@@ -194,8 +213,8 @@ public class CredentialManagerPreferenceControllerTest {
                                 createCredentialProviderInfo("com.android.provider6", "ClassA")));
         controller.setSimulateConnectedForTests(true);
         assertThat(controller.isConnected()).isTrue();
-        controller.setVisibility(true);
+        controller.setSimulateHiddenForTests(Optional.of(false));
-        assertThat(controller.getVisibility()).isTrue();
+        assertThat(controller.isHiddenDueToNoProviderSet()).isFalse();
         assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE);
 
         // Ensure that we stay under 5 providers.
@@ -263,8 +282,8 @@ public class CredentialManagerPreferenceControllerTest {
                 createControllerWithServices(Lists.newArrayList(providerInfo1, providerInfo2));
         controller.setSimulateConnectedForTests(true);
         assertThat(controller.isConnected()).isTrue();
-        controller.setVisibility(true);
+        controller.setSimulateHiddenForTests(Optional.of(false));
-        assertThat(controller.getVisibility()).isTrue();
+        assertThat(controller.isHiddenDueToNoProviderSet()).isFalse();
         assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE);
 
         // Test the data is correct.
@@ -316,9 +335,14 @@ public class CredentialManagerPreferenceControllerTest {
         CredentialManagerPreferenceController controller =
                 createControllerWithServices(
                         Lists.newArrayList(serviceA1, serviceB1, serviceC1, serviceC2, serviceC3));
-        controller.displayPreference(mScreen);
+        controller.setSimulateConnectedForTests(true);
+        controller.setSimulateHiddenForTests(Optional.of(false));
 
-        assertThat(controller.isConnected()).isFalse();
+        assertThat(controller.isHiddenDueToNoProviderSet()).isFalse();
+        assertThat(controller.isConnected()).isTrue();
+        assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE);
+
+        controller.displayPreference(mScreen);
         assertThat(mCredentialsPreferenceCategory.getPreferenceCount()).isEqualTo(3);
 
         Map<String, CredentialManagerPreferenceController.CombiPreference> prefs =