From d2a7f9ae7946a9c58a711651ddd99d332cc0c1e6 Mon Sep 17 00:00:00 2001 From: Zhen Zhang Date: Thu, 30 Jan 2020 15:12:50 -0800 Subject: [PATCH] Use feature flag to show/hide AllInOneTetherSettings This partially reverts commit 0ccc849de7b0ce4c208434a60fb231f749044191 which added a config value. Instead, we will use feature flag to switch between the fragments. This CL also adds a postfix to keys in all_tether_prefs to de-duplicate with keys in tether_prefs and wifi_tether_settings. Bug: 148182953 Change-Id: I92832c786473990065a965409072e4117a7e75a8 Fix: 148618984 Test: make RunSettingsRoboTests Test: make RunSettingsRoboTests ROBOTEST_FILTER=CodeInspectionTest --- res/values/config.xml | 3 -- res/xml/all_tether_prefs.xml | 20 ++++++----- .../settings/AllInOneTetherSettings.java | 35 +++++++++++-------- src/com/android/settings/TetherSettings.java | 5 +-- .../network/TetherPreferenceController.java | 7 ++-- .../WifiTetherApBandPreferenceController.java | 7 +++- ...ifiTetherPasswordPreferenceController.java | 7 +++- .../WifiTetherSSIDPreferenceController.java | 7 +++- ...ifiTetherSecurityPreferenceController.java | 7 +++- .../wifi/tether/WifiTetherSettings.java | 5 +-- .../settings/AllInOneTetherSettingsTest.java | 28 +++++++++------ 11 files changed, 84 insertions(+), 47 deletions(-) diff --git a/res/values/config.xml b/res/values/config.xml index 86be2cb91a4..fc2ca1f89e1 100755 --- a/res/values/config.xml +++ b/res/values/config.xml @@ -434,7 +434,4 @@ - - - false diff --git a/res/xml/all_tether_prefs.xml b/res/xml/all_tether_prefs.xml index c541fe457ae..07e5d64b4bf 100644 --- a/res/xml/all_tether_prefs.xml +++ b/res/xml/all_tether_prefs.xml @@ -18,48 +18,50 @@ + android:key="all_tether_prefs_screen" + android:title="@string/tether_settings_title_all" + settings:searchable="false"> diff --git a/src/com/android/settings/AllInOneTetherSettings.java b/src/com/android/settings/AllInOneTetherSettings.java index 1c7ea1d5ce1..098e6fbee64 100644 --- a/src/com/android/settings/AllInOneTetherSettings.java +++ b/src/com/android/settings/AllInOneTetherSettings.java @@ -36,12 +36,14 @@ import android.net.wifi.WifiManager; import android.os.Bundle; import android.os.UserManager; import android.text.TextUtils; +import android.util.FeatureFlagUtils; import android.util.Log; import androidx.annotation.VisibleForTesting; import androidx.preference.Preference; import androidx.preference.PreferenceGroup; +import com.android.settings.core.FeatureFlags; import com.android.settings.dashboard.RestrictedDashboardFragment; import com.android.settings.datausage.DataSaverBackend; import com.android.settings.network.TetherEnabler; @@ -72,19 +74,24 @@ public final class AllInOneTetherSettings extends RestrictedDashboardFragment WifiTetherBasePreferenceController.OnTetherConfigUpdateListener, SharedPreferences.OnSharedPreferenceChangeListener { - @VisibleForTesting - static final String KEY_TETHER_PREFS_SCREEN = "tether_prefs_screen"; - @VisibleForTesting - static final String KEY_WIFI_TETHER_NETWORK_NAME = "wifi_tether_network_name"; - @VisibleForTesting - static final String KEY_WIFI_TETHER_NETWORK_PASSWORD = "wifi_tether_network_password"; - @VisibleForTesting - static final String KEY_WIFI_TETHER_AUTO_OFF = "wifi_tether_auto_turn_off"; - @VisibleForTesting - static final String KEY_WIFI_TETHER_NETWORK_AP_BAND = "wifi_tether_network_ap_band"; + // TODO(b/148622133): Should clean up the postfix once this fragment replaced TetherSettings. + public static final String DEDUP_POSTFIX = "_2"; + @VisibleForTesting + static final String KEY_WIFI_TETHER_NETWORK_NAME = "wifi_tether_network_name" + DEDUP_POSTFIX; + @VisibleForTesting + static final String KEY_WIFI_TETHER_NETWORK_PASSWORD = + "wifi_tether_network_password" + DEDUP_POSTFIX; + @VisibleForTesting + static final String KEY_WIFI_TETHER_AUTO_OFF = "wifi_tether_auto_turn_off" + DEDUP_POSTFIX; + @VisibleForTesting + static final String KEY_WIFI_TETHER_NETWORK_AP_BAND = + "wifi_tether_network_ap_band" + DEDUP_POSTFIX; + @VisibleForTesting + static final String KEY_WIFI_TETHER_SECURITY = "wifi_tether_security" + DEDUP_POSTFIX; + + private static final String KEY_DATA_SAVER_FOOTER = "disabled_on_data_saver" + DEDUP_POSTFIX; private static final String KEY_WIFI_TETHER_GROUP = "wifi_tether_settings_group"; - private static final String KEY_DATA_SAVER_FOOTER = "disabled_on_data_saver"; private static final int EXPANDED_CHILD_COUNT_WITH_SECURITY_NON = 2; private static final int EXPANDED_CHILD_COUNT_DEFAULT = 3; private static final String TAG = "AllInOneTetherSettings"; @@ -394,20 +401,18 @@ public final class AllInOneTetherSettings extends RestrictedDashboardFragment final List keys = super.getNonIndexableKeys(context); if (!TetherUtil.isTetherAvailable(context)) { - keys.add(KEY_TETHER_PREFS_SCREEN); keys.add(KEY_WIFI_TETHER_NETWORK_NAME); keys.add(KEY_WIFI_TETHER_NETWORK_PASSWORD); keys.add(KEY_WIFI_TETHER_AUTO_OFF); keys.add(KEY_WIFI_TETHER_NETWORK_AP_BAND); + keys.add(KEY_WIFI_TETHER_SECURITY); } - return keys; } @Override protected boolean isPageSearchEnabled(Context context) { - return context.getResources().getBoolean( - R.bool.config_show_all_in_one_tether_settings); + return FeatureFlagUtils.isEnabled(context, FeatureFlags.TETHER_ALL_IN_ONE); } @Override diff --git a/src/com/android/settings/TetherSettings.java b/src/com/android/settings/TetherSettings.java index da86cbf8b78..a29ec9574bc 100644 --- a/src/com/android/settings/TetherSettings.java +++ b/src/com/android/settings/TetherSettings.java @@ -36,11 +36,13 @@ import android.os.Environment; import android.os.Handler; import android.os.UserManager; import android.provider.SearchIndexableResource; +import android.util.FeatureFlagUtils; import androidx.annotation.VisibleForTesting; import androidx.preference.Preference; import androidx.preference.SwitchPreference; +import com.android.settings.core.FeatureFlags; import com.android.settings.datausage.DataSaverBackend; import com.android.settings.search.BaseSearchIndexProvider; import com.android.settings.wifi.tether.WifiTetherPreferenceController; @@ -452,8 +454,7 @@ public class TetherSettings extends RestrictedSettingsFragment @Override protected boolean isPageSearchEnabled(Context context) { - return !context.getResources().getBoolean( - R.bool.config_show_all_in_one_tether_settings); + return !FeatureFlagUtils.isEnabled(context, FeatureFlags.TETHER_ALL_IN_ONE); } @Override diff --git a/src/com/android/settings/network/TetherPreferenceController.java b/src/com/android/settings/network/TetherPreferenceController.java index 5e526924aac..8fc05aabb8e 100644 --- a/src/com/android/settings/network/TetherPreferenceController.java +++ b/src/com/android/settings/network/TetherPreferenceController.java @@ -33,13 +33,16 @@ import android.os.Bundle; import android.os.Handler; import android.os.UserHandle; import android.provider.Settings; +import android.util.FeatureFlagUtils; import androidx.annotation.VisibleForTesting; import androidx.preference.Preference; import androidx.preference.PreferenceScreen; +import com.android.settings.AllInOneTetherSettings; import com.android.settings.R; import com.android.settings.TetherSettings; +import com.android.settings.core.FeatureFlags; import com.android.settings.core.PreferenceControllerMixin; import com.android.settingslib.TetherUtil; import com.android.settingslib.core.AbstractPreferenceController; @@ -110,8 +113,8 @@ public class TetherPreferenceController extends AbstractPreferenceController imp // Grey out if provisioning is not available. mPreference.setEnabled(!TetherSettings.isProvisioningNeededButUnavailable(mContext)); - if (mContext.getResources().getBoolean(R.bool.config_show_all_in_one_tether_settings)) { - mPreference.setFragment("com.android.settings.AllInOneTetherSettings"); + if (FeatureFlagUtils.isEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE)) { + mPreference.setFragment(AllInOneTetherSettings.class.getName()); } } } diff --git a/src/com/android/settings/wifi/tether/WifiTetherApBandPreferenceController.java b/src/com/android/settings/wifi/tether/WifiTetherApBandPreferenceController.java index b9b0d64d5c6..add40b0d9c7 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherApBandPreferenceController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherApBandPreferenceController.java @@ -16,9 +16,12 @@ package com.android.settings.wifi.tether; +import static com.android.settings.AllInOneTetherSettings.DEDUP_POSTFIX; + import android.content.Context; import android.content.res.Resources; import android.net.wifi.SoftApConfiguration; +import android.util.FeatureFlagUtils; import android.util.Log; import androidx.annotation.VisibleForTesting; @@ -26,6 +29,7 @@ import androidx.preference.ListPreference; import androidx.preference.Preference; import com.android.settings.R; +import com.android.settings.core.FeatureFlags; public class WifiTetherApBandPreferenceController extends WifiTetherBasePreferenceController { @@ -87,7 +91,8 @@ public class WifiTetherApBandPreferenceController extends WifiTetherBasePreferen @Override public String getPreferenceKey() { - return PREF_KEY; + return FeatureFlagUtils.isEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE) + ? PREF_KEY + DEDUP_POSTFIX : PREF_KEY; } @Override diff --git a/src/com/android/settings/wifi/tether/WifiTetherPasswordPreferenceController.java b/src/com/android/settings/wifi/tether/WifiTetherPasswordPreferenceController.java index 1563e616718..03ea121faac 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherPasswordPreferenceController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherPasswordPreferenceController.java @@ -16,14 +16,18 @@ package com.android.settings.wifi.tether; +import static com.android.settings.AllInOneTetherSettings.DEDUP_POSTFIX; + import android.content.Context; import android.net.wifi.SoftApConfiguration; import android.text.TextUtils; +import android.util.FeatureFlagUtils; import androidx.preference.EditTextPreference; import androidx.preference.Preference; import com.android.settings.R; +import com.android.settings.core.FeatureFlags; import com.android.settings.widget.ValidatedEditTextPreference; import com.android.settings.wifi.WifiUtils; @@ -43,7 +47,8 @@ public class WifiTetherPasswordPreferenceController extends WifiTetherBasePrefer @Override public String getPreferenceKey() { - return PREF_KEY; + return FeatureFlagUtils.isEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE) + ? PREF_KEY + DEDUP_POSTFIX : PREF_KEY; } @Override diff --git a/src/com/android/settings/wifi/tether/WifiTetherSSIDPreferenceController.java b/src/com/android/settings/wifi/tether/WifiTetherSSIDPreferenceController.java index 0283b0f0398..a4289a682e7 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherSSIDPreferenceController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherSSIDPreferenceController.java @@ -16,16 +16,20 @@ package com.android.settings.wifi.tether; +import static com.android.settings.AllInOneTetherSettings.DEDUP_POSTFIX; + import android.app.settings.SettingsEnums; import android.content.Context; import android.content.Intent; import android.net.wifi.SoftApConfiguration; +import android.util.FeatureFlagUtils; import android.util.Log; import androidx.annotation.VisibleForTesting; import androidx.preference.EditTextPreference; import androidx.preference.Preference; +import com.android.settings.core.FeatureFlags; import com.android.settings.overlay.FeatureFactory; import com.android.settings.widget.ValidatedEditTextPreference; import com.android.settings.wifi.dpp.WifiDppUtils; @@ -54,7 +58,8 @@ public class WifiTetherSSIDPreferenceController extends WifiTetherBasePreference @Override public String getPreferenceKey() { - return PREF_KEY; + return FeatureFlagUtils.isEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE) + ? PREF_KEY + DEDUP_POSTFIX : PREF_KEY; } @Override diff --git a/src/com/android/settings/wifi/tether/WifiTetherSecurityPreferenceController.java b/src/com/android/settings/wifi/tether/WifiTetherSecurityPreferenceController.java index 9b9617ac447..56b50314b6d 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherSecurityPreferenceController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherSecurityPreferenceController.java @@ -1,12 +1,16 @@ package com.android.settings.wifi.tether; +import static com.android.settings.AllInOneTetherSettings.DEDUP_POSTFIX; + import android.content.Context; import android.net.wifi.SoftApConfiguration; +import android.util.FeatureFlagUtils; import androidx.preference.ListPreference; import androidx.preference.Preference; import com.android.settings.R; +import com.android.settings.core.FeatureFlags; public class WifiTetherSecurityPreferenceController extends WifiTetherBasePreferenceController { @@ -23,7 +27,8 @@ public class WifiTetherSecurityPreferenceController extends WifiTetherBasePrefer @Override public String getPreferenceKey() { - return PREF_KEY; + return FeatureFlagUtils.isEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE) + ? PREF_KEY + DEDUP_POSTFIX : PREF_KEY; } @Override diff --git a/src/com/android/settings/wifi/tether/WifiTetherSettings.java b/src/com/android/settings/wifi/tether/WifiTetherSettings.java index 46655151873..66d017ae5ba 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherSettings.java +++ b/src/com/android/settings/wifi/tether/WifiTetherSettings.java @@ -28,6 +28,7 @@ import android.net.wifi.SoftApConfiguration; import android.net.wifi.WifiManager; import android.os.Bundle; import android.os.UserManager; +import android.util.FeatureFlagUtils; import android.util.Log; import androidx.annotation.VisibleForTesting; @@ -35,6 +36,7 @@ import androidx.preference.PreferenceGroup; import com.android.settings.R; import com.android.settings.SettingsActivity; +import com.android.settings.core.FeatureFlags; import com.android.settings.dashboard.RestrictedDashboardFragment; import com.android.settings.search.BaseSearchIndexProvider; import com.android.settings.widget.SwitchBar; @@ -259,8 +261,7 @@ public class WifiTetherSettings extends RestrictedDashboardFragment @Override protected boolean isPageSearchEnabled(Context context) { - return !context.getResources().getBoolean( - R.bool.config_show_all_in_one_tether_settings); + return !FeatureFlagUtils.isEnabled(context, FeatureFlags.TETHER_ALL_IN_ONE); } @Override diff --git a/tests/robotests/src/com/android/settings/AllInOneTetherSettingsTest.java b/tests/robotests/src/com/android/settings/AllInOneTetherSettingsTest.java index 37a3474b4f4..61db0e501af 100644 --- a/tests/robotests/src/com/android/settings/AllInOneTetherSettingsTest.java +++ b/tests/robotests/src/com/android/settings/AllInOneTetherSettingsTest.java @@ -26,7 +26,9 @@ import android.content.Context; import android.net.ConnectivityManager; import android.os.UserHandle; import android.os.UserManager; +import android.util.FeatureFlagUtils; +import com.android.settings.core.FeatureFlags; import com.android.settings.testutils.shadow.ShadowWifiManager; import com.android.settings.wifi.tether.WifiTetherAutoOffPreferenceController; @@ -69,21 +71,27 @@ public class AllInOneTetherSettingsTest { } @Test - public void getNonIndexableKeys_tetherAvailable_keysReturned() { + public void getNonIndexableKeys_tetherAvailable_keysNotReturned() { // To let TetherUtil.isTetherAvailable return true, select one of the combinations setupIsTetherAvailable(true); final List niks = AllInOneTetherSettings.SEARCH_INDEX_DATA_PROVIDER.getNonIndexableKeys(mContext); - // TODO(b/147675042) Should revert these assertions, after switching to new UI. - // Because of the config check, we are not indexing these keys. Once we enable the config - // these assert would also need to change. - assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_NAME); - assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_PASSWORD); - assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_AUTO_OFF); - assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_AP_BAND); - assertThat(niks).contains(AllInOneTetherSettings.KEY_TETHER_PREFS_SCREEN); + if (FeatureFlagUtils.isEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE)) { + assertThat(niks).doesNotContain(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_NAME); + assertThat(niks).doesNotContain( + AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_PASSWORD); + assertThat(niks).doesNotContain(AllInOneTetherSettings.KEY_WIFI_TETHER_AUTO_OFF); + assertThat(niks).doesNotContain(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_AP_BAND); + assertThat(niks).doesNotContain(AllInOneTetherSettings.KEY_WIFI_TETHER_SECURITY); + } else { + assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_NAME); + assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_PASSWORD); + assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_AUTO_OFF); + assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_AP_BAND); + assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_SECURITY); + } } @Test @@ -98,7 +106,7 @@ public class AllInOneTetherSettingsTest { assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_PASSWORD); assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_AUTO_OFF); assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_NETWORK_AP_BAND); - assertThat(niks).contains(AllInOneTetherSettings.KEY_TETHER_PREFS_SCREEN); + assertThat(niks).contains(AllInOneTetherSettings.KEY_WIFI_TETHER_SECURITY); } @Test