From dbaea5af63faaec8ffd34cbb8477ae86c76b9102 Mon Sep 17 00:00:00 2001 From: jackqdyulei Date: Wed, 31 Jan 2018 14:26:42 -0800 Subject: [PATCH] Clean up WifiTetherPreferenceController In previous code there are two main issues: 1. It listens to update from both WIFI_AP_STATE_CHANGED_ACTION and ACTION_TETHER_STATE_CHANGED. It is unnecessary because they provides same info(whether wifi hotspot is enabled, enabling...) 2. New API softApCallback already covers the WIFI_AP_STATE_CHANGED_ACTION, so we don't need this broadcast anymore. This cl fixes those two issues by cleaning up BroadcastReceiver and update the tests. Bug: 72702183 Test: RunSettingsRoboTests Change-Id: I21c2818e0f0185172f34447a1716dc47ee065e23 --- .../WifiTetherPreferenceController.java | 63 ++--------- .../tether/WifiTetherSwitchBarController.java | 11 +- .../WifiTetherPreferenceControllerTest.java | 104 ++++++------------ 3 files changed, 51 insertions(+), 127 deletions(-) diff --git a/src/com/android/settings/wifi/tether/WifiTetherPreferenceController.java b/src/com/android/settings/wifi/tether/WifiTetherPreferenceController.java index 11f1f5977d6..826ed426019 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherPreferenceController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherPreferenceController.java @@ -39,13 +39,12 @@ import com.android.settingslib.core.lifecycle.LifecycleObserver; import com.android.settingslib.core.lifecycle.events.OnStart; import com.android.settingslib.core.lifecycle.events.OnStop; -import java.util.List; - public class WifiTetherPreferenceController extends AbstractPreferenceController implements PreferenceControllerMixin, LifecycleObserver, OnStart, OnStop { - public static final IntentFilter WIFI_TETHER_INTENT_FILTER; private static final String WIFI_TETHER_SETTINGS = "wifi_tether"; + private static final IntentFilter AIRPLANE_INTENT_FILTER = new IntentFilter( + Intent.ACTION_AIRPLANE_MODE_CHANGED); private final ConnectivityManager mConnectivityManager; private final String[] mWifiRegexs; @@ -58,12 +57,6 @@ public class WifiTetherPreferenceController extends AbstractPreferenceController @VisibleForTesting WifiTetherSoftApManager mWifiTetherSoftApManager; - static { - WIFI_TETHER_INTENT_FILTER = new IntentFilter(WifiManager.WIFI_AP_STATE_CHANGED_ACTION); - WIFI_TETHER_INTENT_FILTER.addAction(ConnectivityManager.ACTION_TETHER_STATE_CHANGED); - WIFI_TETHER_INTENT_FILTER.addAction(Intent.ACTION_AIRPLANE_MODE_CHANGED); - } - public WifiTetherPreferenceController(Context context, Lifecycle lifecycle) { this(context, lifecycle, true /* initSoftApManager */); } @@ -113,7 +106,7 @@ public class WifiTetherPreferenceController extends AbstractPreferenceController @Override public void onStart() { if (mPreference != null) { - mContext.registerReceiver(mReceiver, WIFI_TETHER_INTENT_FILTER); + mContext.registerReceiver(mReceiver, AIRPLANE_INTENT_FILTER); clearSummaryForAirplaneMode(); if (mWifiTetherSoftApManager != null) { mWifiTetherSoftApManager.registerSoftApCallback(); @@ -140,6 +133,7 @@ public class WifiTetherPreferenceController extends AbstractPreferenceController @Override public void onStateChanged(int state, int failureReason) { mSoftApState = state; + handleWifiApStateChanged(state, failureReason); } @Override @@ -162,34 +156,21 @@ public class WifiTetherPreferenceController extends AbstractPreferenceController @Override public void onReceive(Context context, Intent intent) { String action = intent.getAction(); - if (WifiManager.WIFI_AP_STATE_CHANGED_ACTION.equals(action)) { - int state = intent.getIntExtra( - WifiManager.EXTRA_WIFI_AP_STATE, WifiManager.WIFI_AP_STATE_FAILED); - int reason = intent.getIntExtra(WifiManager.EXTRA_WIFI_AP_FAILURE_REASON, - WifiManager.SAP_START_FAILURE_GENERAL); - handleWifiApStateChanged(state, reason); - } else if (ConnectivityManager.ACTION_TETHER_STATE_CHANGED.equals(action)) { - List active = intent.getStringArrayListExtra( - ConnectivityManager.EXTRA_ACTIVE_TETHER); - List errored = intent.getStringArrayListExtra( - ConnectivityManager.EXTRA_ERRORED_TETHER); - updateTetherState(active.toArray(), errored.toArray()); - } else if (Intent.ACTION_AIRPLANE_MODE_CHANGED.equals(action)) { + if (Intent.ACTION_AIRPLANE_MODE_CHANGED.equals(action)) { clearSummaryForAirplaneMode(); } } }; - private void handleWifiApStateChanged(int state, int reason) { + @VisibleForTesting + void handleWifiApStateChanged(int state, int reason) { switch (state) { case WifiManager.WIFI_AP_STATE_ENABLING: mPreference.setSummary(R.string.wifi_tether_starting); break; case WifiManager.WIFI_AP_STATE_ENABLED: - /** - * Summary on enable is handled by tether - * broadcast notice - */ + WifiConfiguration wifiConfig = mWifiManager.getWifiApConfiguration(); + updateConfigSummary(wifiConfig); break; case WifiManager.WIFI_AP_STATE_DISABLING: mPreference.setSummary(R.string.wifi_tether_stopping); @@ -208,32 +189,6 @@ public class WifiTetherPreferenceController extends AbstractPreferenceController } } - private void updateTetherState(Object[] tethered, Object[] errored) { - boolean wifiTethered = matchRegex(tethered); - boolean wifiErrored = matchRegex(errored); - - if (wifiTethered) { - WifiConfiguration wifiConfig = mWifiManager.getWifiApConfiguration(); - updateConfigSummary(wifiConfig); - } else if (wifiErrored) { - mPreference.setSummary(R.string.wifi_error); - } else { - mPreference.setSummary(R.string.wifi_hotspot_off_subtext); - } - } - - private boolean matchRegex(Object[] tethers) { - for (Object o : tethers) { - String s = (String) o; - for (String regex : mWifiRegexs) { - if (s.matches(regex)) { - return true; - } - } - } - return false; - } - private void updateConfigSummary(WifiConfiguration wifiConfig) { final String s = mContext.getString( com.android.internal.R.string.wifi_tether_configure_ssid_default); diff --git a/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java b/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java index 627bf32d215..699e5ae880b 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java @@ -21,6 +21,7 @@ import static android.net.ConnectivityManager.TETHERING_WIFI; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; +import android.content.IntentFilter; import android.net.ConnectivityManager; import android.net.wifi.WifiManager; import android.os.Handler; @@ -36,12 +37,19 @@ import com.android.settingslib.core.lifecycle.events.OnStop; public class WifiTetherSwitchBarController implements SwitchWidgetController.OnSwitchChangeListener, LifecycleObserver, OnStart, OnStop { + private static final IntentFilter WIFI_INTENT_FILTER; + private final Context mContext; private final SwitchWidgetController mSwitchBar; private final ConnectivityManager mConnectivityManager; private final DataSaverBackend mDataSaverBackend; private final WifiManager mWifiManager; + static { + WIFI_INTENT_FILTER = new IntentFilter(WifiManager.WIFI_AP_STATE_CHANGED_ACTION); + WIFI_INTENT_FILTER.addAction(Intent.ACTION_AIRPLANE_MODE_CHANGED); + } + WifiTetherSwitchBarController(Context context, SwitchWidgetController switchBar) { mContext = context; mSwitchBar = switchBar; @@ -56,8 +64,7 @@ public class WifiTetherSwitchBarController implements SwitchWidgetController.OnS @Override public void onStart() { mSwitchBar.startListening(); - mContext.registerReceiver(mReceiver, - WifiTetherPreferenceController.WIFI_TETHER_INTENT_FILTER); + mContext.registerReceiver(mReceiver, WIFI_INTENT_FILTER); } @Override diff --git a/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherPreferenceControllerTest.java index dca69748a95..4b18fcff273 100644 --- a/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherPreferenceControllerTest.java @@ -73,7 +73,8 @@ import java.util.ArrayList; }) public class WifiTetherPreferenceControllerTest { - @Mock + private static final String SSID = "Pixel"; + private Context mContext; @Mock private ConnectivityManager mConnectivityManager; @@ -81,6 +82,8 @@ public class WifiTetherPreferenceControllerTest { private WifiManager mWifiManager; @Mock private PreferenceScreen mScreen; + @Mock + private WifiConfiguration mWifiConfiguration; private WifiTetherPreferenceController mController; private Lifecycle mLifecycle; @@ -90,6 +93,8 @@ public class WifiTetherPreferenceControllerTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + + mContext = spy(RuntimeEnvironment.application); mLifecycleOwner = () -> mLifecycle; mLifecycle = new Lifecycle(mLifecycleOwner); FakeFeatureFactory.setupForTest(); @@ -98,10 +103,13 @@ public class WifiTetherPreferenceControllerTest { .thenReturn(mConnectivityManager); when(mContext.getSystemService(Context.WIFI_SERVICE)).thenReturn(mWifiManager); when(mScreen.findPreference(anyString())).thenReturn(mPreference); + when(mWifiManager.getWifiApConfiguration()).thenReturn(mWifiConfiguration); + mWifiConfiguration.SSID = SSID; when(mConnectivityManager.getTetherableWifiRegexs()).thenReturn(new String[]{"1", "2"}); mController = new WifiTetherPreferenceController(mContext, mLifecycle, false /* initSoftApManager */); + mController.displayPreference(mScreen); } @After @@ -127,7 +135,6 @@ public class WifiTetherPreferenceControllerTest { public void startAndStop_shouldRegisterUnregisterReceiver() { final BroadcastReceiver receiver = ReflectionHelpers.getField(mController, "mReceiver"); - mController.displayPreference(mScreen); mLifecycle.handleLifecycleEvent(ON_START); mLifecycle.handleLifecycleEvent(ON_STOP); @@ -166,48 +173,6 @@ public class WifiTetherPreferenceControllerTest { verify(pref).setChecked(true); } - @Test - public void testReceiver_apStateChangedToDisabled_shouldUpdatePreferenceSummary() { - mController.displayPreference(mScreen); - receiveApStateChangedBroadcast(WifiManager.WIFI_AP_STATE_DISABLED); - assertThat(mPreference.getSummary().toString()).isEqualTo( - RuntimeEnvironment.application.getString(R.string.wifi_hotspot_off_subtext)); - } - - @Test - public void testReceiver_apStateChangedToDisabling_shouldUpdatePreferenceSummary() { - mController.displayPreference(mScreen); - receiveApStateChangedBroadcast(WifiManager.WIFI_AP_STATE_DISABLING); - assertThat(mPreference.getSummary().toString()).isEqualTo( - RuntimeEnvironment.application.getString(R.string.wifi_tether_stopping)); - } - - @Test - public void testReceiver_apStateChangedToEnabling_shouldUpdatePreferenceSummary() { - mController.displayPreference(mScreen); - receiveApStateChangedBroadcast(WifiManager.WIFI_AP_STATE_ENABLING); - assertThat(mPreference.getSummary().toString()).isEqualTo( - RuntimeEnvironment.application.getString(R.string.wifi_tether_starting)); - } - - @Test - public void testReceiver_apStateChangedToEnabled_shouldNotUpdatePreferenceSummary() { - mController.displayPreference(mScreen); - receiveApStateChangedBroadcast(WifiManager.WIFI_AP_STATE_DISABLED); - assertThat(mPreference.getSummary().toString()).isEqualTo( - RuntimeEnvironment.application.getString(R.string.wifi_hotspot_off_subtext)); - - // When turning on the hotspot, we receive STATE_ENABLING followed by STATE_ENABLED. The - // first should change the status to wifi_tether_starting, and the second should not change - // this. - receiveApStateChangedBroadcast(WifiManager.WIFI_AP_STATE_ENABLING); - assertThat(mPreference.getSummary().toString()).isEqualTo( - RuntimeEnvironment.application.getString(R.string.wifi_tether_starting)); - receiveApStateChangedBroadcast(WifiManager.WIFI_AP_STATE_ENABLED); - assertThat(mPreference.getSummary().toString()).isEqualTo( - RuntimeEnvironment.application.getString(R.string.wifi_tether_starting)); - } - @Test public void testReceiver_goingToAirplaneMode_shouldClearPreferenceSummary() { final ContentResolver cr = mock(ContentResolver.class); @@ -224,22 +189,32 @@ public class WifiTetherPreferenceControllerTest { } @Test - public void testReceiver_tetherEnabled_shouldUpdatePreferenceSummary() { - mController.displayPreference(mScreen); - final BroadcastReceiver receiver = ReflectionHelpers.getField(mController, "mReceiver"); - final Intent broadcast = new Intent(ConnectivityManager.ACTION_TETHER_STATE_CHANGED); - final ArrayList activeTethers = new ArrayList<>(); - activeTethers.add("1"); - broadcast.putStringArrayListExtra(ConnectivityManager.EXTRA_ACTIVE_TETHER, activeTethers); - broadcast.putStringArrayListExtra(ConnectivityManager.EXTRA_ERRORED_TETHER, - new ArrayList<>()); - final WifiConfiguration configuration = new WifiConfiguration(); - configuration.SSID = "test-ap"; - when(mWifiManager.getWifiApConfiguration()).thenReturn(configuration); + public void testHandleWifiApStateChanged_stateEnabling_showEnablingSummary() { + mController.handleWifiApStateChanged(WifiManager.WIFI_AP_STATE_ENABLING, 0 /* reason */); - receiver.onReceive(RuntimeEnvironment.application, broadcast); + assertThat(mPreference.getSummary()).isEqualTo("Turning hotspot on\u2026"); + } - verify(mContext).getString(eq(R.string.wifi_tether_enabled_subtext), any()); + @Test + public void testHandleWifiApStateChanged_stateEnabled_showEnabledSummary() { + mController.handleWifiApStateChanged(WifiManager.WIFI_AP_STATE_ENABLED, 0 /* reason */); + + assertThat(mPreference.getSummary()).isEqualTo("Pixel is active"); + } + + @Test + public void testHandleWifiApStateChanged_stateDisabling_showDisablingSummary() { + mController.handleWifiApStateChanged(WifiManager.WIFI_AP_STATE_DISABLING, 0 /* reason */); + + assertThat(mPreference.getSummary()).isEqualTo("Turning off hotspot\u2026"); + } + + @Test + public void testHandleWifiApStateChanged_stateDisabled_showDisabledSummary() { + mController.handleWifiApStateChanged(WifiManager.WIFI_AP_STATE_DISABLED, 0 /* reason */); + + assertThat(mPreference.getSummary()).isEqualTo( + "Not sharing internet or content with other devices"); } @Implements(WifiTetherSettings.class) @@ -285,17 +260,4 @@ public class WifiTetherPreferenceControllerTest { onStopCalled = true; } } - - /** - * Helper to cause the controller to receive a WIFI_AP_STATE_CHANGED_ACTION with a specific - * state. - * - * @param state - the state, as specified by one of the WifiManager.WIFI_AP_STATE_* values - */ - private void receiveApStateChangedBroadcast(int state) { - final BroadcastReceiver receiver = ReflectionHelpers.getField(mController, "mReceiver"); - final Intent broadcast = new Intent(WifiManager.WIFI_AP_STATE_CHANGED_ACTION); - broadcast.putExtra(WifiManager.EXTRA_WIFI_AP_STATE, state); - receiver.onReceive(RuntimeEnvironment.application, broadcast); - } }