From 6c42636cb8c4ecaf1ba948677d2908b1d9004e8d Mon Sep 17 00:00:00 2001 From: Weng Su Date: Fri, 8 Apr 2022 05:35:14 +0800 Subject: [PATCH] Fix unexpected Wi-Fi hotspot shutdown - Filter out unnecessary onSwitchChanged callbacks when the switch is disabled, which should not be triggered by user input. - Refine the state handling function to avoid unnecessary onSwitchChanged callback. - Refine the error handling of isHotspotPasswordValid function. Bug: 227719584 Test: manual test make RunSettingsRoboTests \ ROBOTEST_FILTER=WifiTetherSwitchBarControllerTest Change-Id: If62aaadc8ddb214769b1367d7801b6125bb5377c --- src/com/android/settings/wifi/WifiUtils.java | 2 +- .../tether/WifiTetherSwitchBarController.java | 48 ++++---- .../WifiTetherSwitchBarControllerTest.java | 107 +++++++++++++++--- 3 files changed, 109 insertions(+), 48 deletions(-) diff --git a/src/com/android/settings/wifi/WifiUtils.java b/src/com/android/settings/wifi/WifiUtils.java index 10277927019..4b94c81fda0 100644 --- a/src/com/android/settings/wifi/WifiUtils.java +++ b/src/com/android/settings/wifi/WifiUtils.java @@ -72,7 +72,7 @@ public class WifiUtils extends com.android.settingslib.wifi.WifiUtils { } } configBuilder.setPassphrase(password, securityType); - } catch (IllegalArgumentException e) { + } catch (Exception e) { return false; } return true; diff --git a/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java b/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java index 6f86ddd1832..580bc39937e 100644 --- a/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java +++ b/src/com/android/settings/wifi/tether/WifiTetherSwitchBarController.java @@ -17,6 +17,11 @@ package com.android.settings.wifi.tether; import static android.net.ConnectivityManager.TETHERING_WIFI; +import static android.net.wifi.WifiManager.EXTRA_WIFI_AP_STATE; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLING; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLED; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLING; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_FAILED; import android.content.BroadcastReceiver; import android.content.Context; @@ -51,7 +56,7 @@ public class WifiTetherSwitchBarController implements private final WifiManager mWifiManager; @VisibleForTesting - final DataSaverBackend mDataSaverBackend; + DataSaverBackend mDataSaverBackend; @VisibleForTesting final ConnectivityManager.OnStartTetheringCallback mOnStartTetheringCallback = new ConnectivityManager.OnStartTetheringCallback() { @@ -75,7 +80,7 @@ public class WifiTetherSwitchBarController implements mConnectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); mWifiManager = (WifiManager) context.getSystemService(Context.WIFI_SERVICE); - mSwitchBar.setChecked(mWifiManager.getWifiApState() == WifiManager.WIFI_AP_STATE_ENABLED); + mSwitchBar.setChecked(mWifiManager.getWifiApState() == WIFI_AP_STATE_ENABLED); updateWifiSwitch(); } @@ -95,6 +100,9 @@ public class WifiTetherSwitchBarController implements @Override public void onSwitchChanged(Switch switchView, boolean isChecked) { + // Filter out unnecessary callbacks when switch is disabled. + if (!switchView.isEnabled()) return; + if (isChecked) { startTether(); } else { @@ -118,39 +126,21 @@ public class WifiTetherSwitchBarController implements public void onReceive(Context context, Intent intent) { String action = intent.getAction(); if (WifiManager.WIFI_AP_STATE_CHANGED_ACTION.equals(action)) { - final int state = intent.getIntExtra( - WifiManager.EXTRA_WIFI_AP_STATE, WifiManager.WIFI_AP_STATE_FAILED); + final int state = intent.getIntExtra(EXTRA_WIFI_AP_STATE, WIFI_AP_STATE_FAILED); handleWifiApStateChanged(state); } } }; - private void handleWifiApStateChanged(int state) { - switch (state) { - case WifiManager.WIFI_AP_STATE_ENABLING: - mSwitchBar.setEnabled(false); - break; - case WifiManager.WIFI_AP_STATE_ENABLED: - if (!mSwitch.isChecked()) { - mSwitch.setChecked(true); - } - updateWifiSwitch(); - break; - case WifiManager.WIFI_AP_STATE_DISABLING: - if (mSwitch.isChecked()) { - mSwitch.setChecked(false); - } - mSwitchBar.setEnabled(false); - break; - case WifiManager.WIFI_AP_STATE_DISABLED: - mSwitch.setChecked(false); - updateWifiSwitch(); - break; - default: - mSwitch.setChecked(false); - updateWifiSwitch(); - break; + @VisibleForTesting + void handleWifiApStateChanged(int state) { + if (state == WIFI_AP_STATE_ENABLING || state == WIFI_AP_STATE_DISABLING) return; + + final boolean shouldBeChecked = (state == WIFI_AP_STATE_ENABLED); + if (mSwitch.isChecked() != shouldBeChecked) { + mSwitch.setChecked(shouldBeChecked); } + updateWifiSwitch(); } private void updateWifiSwitch() { diff --git a/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherSwitchBarControllerTest.java b/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherSwitchBarControllerTest.java index a27743e2bd7..7c17c5fa9bd 100644 --- a/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherSwitchBarControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/tether/WifiTetherSwitchBarControllerTest.java @@ -16,66 +16,76 @@ package com.android.settings.wifi.tether; -import static android.net.ConnectivityManager.TETHERING_WIFI; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLED; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLING; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLED; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLING; +import static android.net.wifi.WifiManager.WIFI_AP_STATE_FAILED; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.spy; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.Context; import android.net.ConnectivityManager; -import android.net.NetworkPolicyManager; import android.net.wifi.WifiManager; import android.widget.Switch; +import androidx.test.core.app.ApplicationProvider; + +import com.android.settings.datausage.DataSaverBackend; import com.android.settings.widget.SettingsMainSwitchBar; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; import org.robolectric.RobolectricTestRunner; -import org.robolectric.RuntimeEnvironment; @RunWith(RobolectricTestRunner.class) public class WifiTetherSwitchBarControllerTest { + + @Rule + public final MockitoRule mMockitoRule = MockitoJUnit.rule(); + @Spy + Context mContext = ApplicationProvider.getApplicationContext(); @Mock private WifiManager mWifiManager; @Mock private ConnectivityManager mConnectivityManager; @Mock - private NetworkPolicyManager mNetworkPolicyManager; + private DataSaverBackend mDataSaverBackend; @Mock private Switch mSwitch; - private Context mContext; private SettingsMainSwitchBar mSwitchBar; private WifiTetherSwitchBarController mController; @Before public void setUp() { - MockitoAnnotations.initMocks(this); - - mContext = spy(RuntimeEnvironment.application); mSwitchBar = new SettingsMainSwitchBar(mContext); when(mContext.getSystemService(Context.WIFI_SERVICE)).thenReturn(mWifiManager); when(mContext.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn( mConnectivityManager); - when(mContext.getSystemService(Context.NETWORK_POLICY_SERVICE)).thenReturn( - mNetworkPolicyManager); + when(mDataSaverBackend.isDataSaverEnabled()).thenReturn(false); + when(mSwitch.isEnabled()).thenReturn(true); mController = new WifiTetherSwitchBarController(mContext, mSwitchBar); + mController.mDataSaverBackend = mDataSaverBackend; } @Test public void startTether_fail_resetSwitchBar() { - when(mNetworkPolicyManager.getRestrictBackground()).thenReturn(false); + when(mDataSaverBackend.isDataSaverEnabled()).thenReturn(false); mController.startTether(); mController.mOnStartTetheringCallback.onTetheringFailed(); @@ -89,23 +99,33 @@ public class WifiTetherSwitchBarControllerTest { assertThat(mSwitchBar.isEnabled()).isTrue(); // try to turn data saver on - when(mNetworkPolicyManager.getRestrictBackground()).thenReturn(true); + when(mDataSaverBackend.isDataSaverEnabled()).thenReturn(true); mController.onDataSaverChanged(true); assertThat(mSwitchBar.isEnabled()).isFalse(); // lets turn data saver off again - when(mNetworkPolicyManager.getRestrictBackground()).thenReturn(false); + when(mDataSaverBackend.isDataSaverEnabled()).thenReturn(false); mController.onDataSaverChanged(false); assertThat(mSwitchBar.isEnabled()).isTrue(); } + @Test + public void onSwitchChanged_switchNotEnabled_doNothingForTethering() { + when(mSwitch.isEnabled()).thenReturn(false); + + mController.onSwitchChanged(mSwitch, mSwitch.isChecked()); + + verify(mConnectivityManager, never()).startTethering(anyInt(), anyBoolean(), any(), any()); + verify(mConnectivityManager, never()).stopTethering(anyInt()); + } + @Test public void onSwitchChanged_isChecked_startTethering() { when(mSwitch.isChecked()).thenReturn(true); mController.onSwitchChanged(mSwitch, mSwitch.isChecked()); - verify(mConnectivityManager).startTethering(eq(TETHERING_WIFI), anyBoolean(), any(), any()); + verify(mConnectivityManager).startTethering(anyInt(), anyBoolean(), any(), any()); } @Test @@ -114,6 +134,57 @@ public class WifiTetherSwitchBarControllerTest { mController.onSwitchChanged(mSwitch, mSwitch.isChecked()); - verify(mConnectivityManager).stopTethering(TETHERING_WIFI); + verify(mConnectivityManager).stopTethering(anyInt()); + } + + @Test + public void handleWifiApStateChanged_stateIsEnabling_notEnabledSwitchBar() { + mSwitchBar.setEnabled(false); + + mController.handleWifiApStateChanged(WIFI_AP_STATE_ENABLING); + + assertThat(mSwitchBar.isEnabled()).isFalse(); + } + + @Test + public void handleWifiApStateChanged_stateIsDisabling_notEnabledSwitchBar() { + mSwitchBar.setEnabled(false); + + mController.handleWifiApStateChanged(WIFI_AP_STATE_DISABLING); + + assertThat(mSwitchBar.isEnabled()).isFalse(); + } + + @Test + public void handleWifiApStateChanged_stateIsEnabled_enabledAndCheckedSwitchBar() { + mSwitchBar.setEnabled(false); + mSwitchBar.setChecked(false); + + mController.handleWifiApStateChanged(WIFI_AP_STATE_ENABLED); + + assertThat(mSwitchBar.isEnabled()).isTrue(); + assertThat(mSwitchBar.isChecked()).isTrue(); + } + + @Test + public void handleWifiApStateChanged_stateIsDisabled_enabledAndUncheckedSwitchBar() { + mSwitchBar.setEnabled(false); + mSwitchBar.setChecked(true); + + mController.handleWifiApStateChanged(WIFI_AP_STATE_DISABLED); + + assertThat(mSwitchBar.isEnabled()).isTrue(); + assertThat(mSwitchBar.isChecked()).isFalse(); + } + + @Test + public void handleWifiApStateChanged_stateIsFailed_enabledAndUncheckedSwitchBar() { + mSwitchBar.setEnabled(false); + mSwitchBar.setChecked(true); + + mController.handleWifiApStateChanged(WIFI_AP_STATE_FAILED); + + assertThat(mSwitchBar.isEnabled()).isTrue(); + assertThat(mSwitchBar.isChecked()).isFalse(); } }