From 29011256745c44e840fc7a0eabd0b502403f6e27 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Mon, 8 May 2023 18:37:16 +0800 Subject: [PATCH] Fix flicker in SIM card page Before fix, the DefaultSubscriptionController will set an empty summary before data is ready. Not set the summary before data is ready to fix. Also fix the ignored tests. Fix: 277301125 Test: Manual on Mobile Settings Test: Unit test Change-Id: I07d964b9f9ce21f8f168cb775eea188e48172c5d --- .../DefaultSubscriptionController.java | 5 +- .../DefaultSubscriptionControllerTest.java | 131 ++++++++++-------- 2 files changed, 78 insertions(+), 58 deletions(-) diff --git a/src/com/android/settings/network/telephony/DefaultSubscriptionController.java b/src/com/android/settings/network/telephony/DefaultSubscriptionController.java index 78947aa425b..206b3a9cc08 100644 --- a/src/com/android/settings/network/telephony/DefaultSubscriptionController.java +++ b/src/com/android/settings/network/telephony/DefaultSubscriptionController.java @@ -112,6 +112,8 @@ public abstract class DefaultSubscriptionController extends TelephonyBasePrefere public void displayPreference(PreferenceScreen screen) { super.displayPreference(screen); mPreference = screen.findPreference(getPreferenceKey()); + // Set a summary placeholder to reduce flicker. + mPreference.setSummaryProvider(pref -> mContext.getString(R.string.summary_placeholder)); updateEntries(); } @@ -120,7 +122,7 @@ public abstract class DefaultSubscriptionController extends TelephonyBasePrefere // Currently, cannot use ListPreference.setSummary() when the summary contains user // generated string, because ListPreference.getSummary() is using String.format() to format // the summary when the summary is set by ListPreference.setSummary(). - if (preference != null) { + if (preference != null && !mSubInfoEntityList.isEmpty()) { preference.setSummaryProvider(pref -> getSummary()); } } @@ -146,6 +148,7 @@ public abstract class DefaultSubscriptionController extends TelephonyBasePrefere final ArrayList displayNames = new ArrayList<>(); final ArrayList subscriptionIds = new ArrayList<>(); List list = getSubscriptionInfoList(); + if (list.isEmpty()) return; if (list.size() == 1) { mPreference.setEnabled(false); diff --git a/tests/unit/src/com/android/settings/network/telephony/DefaultSubscriptionControllerTest.java b/tests/unit/src/com/android/settings/network/telephony/DefaultSubscriptionControllerTest.java index b2ad3d793bc..ef5d4a72718 100644 --- a/tests/unit/src/com/android/settings/network/telephony/DefaultSubscriptionControllerTest.java +++ b/tests/unit/src/com/android/settings/network/telephony/DefaultSubscriptionControllerTest.java @@ -47,7 +47,6 @@ import com.android.settingslib.mobile.dataservice.SubscriptionInfoEntity; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -128,14 +127,12 @@ public class DefaultSubscriptionControllerTest { SubscriptionUtil.setActiveSubscriptionsForTesting(null); } - private SubscriptionInfoEntity setupSubscriptionInfoEntity(String subId, int slotId, - int carrierId, String displayName, String mcc, String mnc, String countryIso, - int cardId, boolean isValid, boolean isActive, boolean isAvailable) { - return new SubscriptionInfoEntity(subId, slotId, carrierId, - displayName, displayName, 0, mcc, mnc, countryIso, false, cardId, - TelephonyManager.DEFAULT_PORT_INDEX, false, null, - SubscriptionManager.SUBSCRIPTION_TYPE_LOCAL_SIM, displayName, false, - "1234567890", true, false, isValid, true, isActive, isAvailable, false); + private SubscriptionInfoEntity setupSubscriptionInfoEntity( + String subId, String displayName, String mcc, String mnc, String countryIso) { + return new SubscriptionInfoEntity(subId, 1, 1, displayName, displayName, 0, mcc, mnc, + countryIso, false, 1, TelephonyManager.DEFAULT_PORT_INDEX, false, null, + SubscriptionManager.SUBSCRIPTION_TYPE_LOCAL_SIM, displayName, false, "1234567890", + true, false, true, true, true, true, false); } @Test @@ -146,16 +143,45 @@ public class DefaultSubscriptionControllerTest { assertThat(mController.getAvailabilityStatus()).isEqualTo(AVAILABLE); } + @Test + public void getSummary_singleSub() { + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubscriptionInfoEntityList.add(mSubInfo1); + mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); + mController.displayPreference(mScreen); + + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); + + assertThat(mListPreference.getSummary().toString()).isEqualTo(SUB_ID_1); + } + + @Test + public void getSummary_twoSubs() { + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); + mSubscriptionInfoEntityList.add(mSubInfo1); + mSubscriptionInfoEntityList.add(mSubInfo2); + mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); + mController.displayPreference(mScreen); + + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); + + assertThat(mListPreference.getSummary().toString()).isEqualTo(SUB_ID_1); + } + @Test public void onPreferenceChange_prefChangedToSub2_callbackCalledCorrectly() { - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); mSubscriptionInfoEntityList.add(mSubInfo1); mSubscriptionInfoEntityList.add(mSubInfo2); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); mController.displayPreference(mScreen); mListPreference.setValue("222"); @@ -165,14 +191,14 @@ public class DefaultSubscriptionControllerTest { @Test public void onPreferenceChange_prefChangedToAlwaysAsk_callbackCalledCorrectly() { - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); mSubscriptionInfoEntityList.add(mSubInfo1); mSubscriptionInfoEntityList.add(mSubInfo2); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); mController.displayPreference(mScreen); mListPreference.setValue(Integer.toString(SubscriptionManager.INVALID_SUBSCRIPTION_ID)); @@ -185,14 +211,14 @@ public class DefaultSubscriptionControllerTest { @Test public void onPreferenceChange_prefBecomesAvailable_onPreferenceChangeCallbackNotNull() { // Start with only one sub active, so the pref is not available - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); mSubscriptionInfoEntityList.add(mSubInfo1); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); mController.displayPreference(mScreen); assertThat(mController.isAvailable()).isTrue(); @@ -206,13 +232,12 @@ public class DefaultSubscriptionControllerTest { assertThat(mController.getDefaultSubscriptionId()).isEqualTo(2); } - @Ignore @Test public void onSubscriptionsChanged_twoSubscriptionsDefaultChanges_selectedEntryGetsUpdated() { - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); mSubscriptionInfoEntityList.add(mSubInfo1); mSubscriptionInfoEntityList.add(mSubInfo2); @@ -225,19 +250,19 @@ public class DefaultSubscriptionControllerTest { mController.setDefaultSubscription(Integer.parseInt(mSubInfo2.subId)); mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); assertThat(mListPreference.getEntry()).isEqualTo(DISPLAY_NAME_2); - assertThat(mListPreference.getValue()).isEqualTo(mSubInfo2); + assertThat(mListPreference.getValue()).isEqualTo(mSubInfo2.subId); } @Test public void onSubscriptionsChanged_goFromTwoSubscriptionsToOne_prefDisappears() { - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); mSubscriptionInfoEntityList.add(mSubInfo1); mSubscriptionInfoEntityList.add(mSubInfo2); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); mController.displayPreference(mScreen); mController.displayPreference(mScreen); @@ -246,7 +271,6 @@ public class DefaultSubscriptionControllerTest { assertThat(mListPreference.isEnabled()).isTrue(); mSubscriptionInfoEntityList.remove(mSubInfo2); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); assertThat(mController.isAvailable()).isTrue(); @@ -257,20 +281,19 @@ public class DefaultSubscriptionControllerTest { @Test @UiThreadTest public void onSubscriptionsChanged_goFromOneSubscriptionToTwo_prefAppears() { - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); mSubscriptionInfoEntityList.add(mSubInfo1); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); + mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); mController.displayPreference(mScreen); assertThat(mController.isAvailable()).isTrue(); assertThat(mListPreference.isVisible()).isTrue(); assertThat(mListPreference.isEnabled()).isFalse(); mSubscriptionInfoEntityList.add(mSubInfo2); - mController.setSubscriptionInfoList(mSubscriptionInfoEntityList); mController.onActiveSubInfoChanged(mSubscriptionInfoEntityList); assertThat(mController.isAvailable()).isTrue(); @@ -278,15 +301,14 @@ public class DefaultSubscriptionControllerTest { assertThat(mListPreference.isEnabled()).isTrue(); } - @Ignore @Test public void onSubscriptionsChanged_goFromTwoToThreeSubscriptions_listGetsUpdated() { - mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, 1, 1, DISPLAY_NAME_1, SUB_MCC_1, - SUB_MNC_1, SUB_COUNTRY_ISO_1, 1, true, true, true); - mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, 1, 1, DISPLAY_NAME_2, SUB_MCC_2, - SUB_MNC_2, SUB_COUNTRY_ISO_2, 1, true, true, true); - mSubInfo3 = setupSubscriptionInfoEntity(SUB_ID_3, 1, 1, DISPLAY_NAME_3, SUB_MCC_3, - SUB_MNC_3, SUB_COUNTRY_ISO_3, 1, true, true, true); + mSubInfo1 = setupSubscriptionInfoEntity(SUB_ID_1, DISPLAY_NAME_1, SUB_MCC_1, SUB_MNC_1, + SUB_COUNTRY_ISO_1); + mSubInfo2 = setupSubscriptionInfoEntity(SUB_ID_2, DISPLAY_NAME_2, SUB_MCC_2, SUB_MNC_2, + SUB_COUNTRY_ISO_2); + mSubInfo3 = setupSubscriptionInfoEntity(SUB_ID_3, DISPLAY_NAME_3, SUB_MCC_3, SUB_MNC_3, + SUB_COUNTRY_ISO_3); mController.setDefaultSubscription(Integer.parseInt(mSubInfo1.subId)); mSubscriptionInfoEntityList.add(mSubInfo1); mSubscriptionInfoEntityList.add(mSubInfo2); @@ -322,9 +344,8 @@ public class DefaultSubscriptionControllerTest { return sub; } - private class TestDefaultSubscriptionController extends DefaultSubscriptionController { + private static class TestDefaultSubscriptionController extends DefaultSubscriptionController { int mSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID; - private List mSubscriptionInfoEntity; TestDefaultSubscriptionController(Context context, String preferenceKey, Lifecycle lifecycle, LifecycleOwner lifecycleOwner) { @@ -342,12 +363,8 @@ public class DefaultSubscriptionControllerTest { } @Override - protected List getSubscriptionInfoList() { - return mSubscriptionInfoEntity; - } - - public void setSubscriptionInfoList(List list) { - mSubscriptionInfoEntity = list; + public CharSequence getSummary() { + return String.valueOf(mSubId); } } }