From a3a0661d1bbcb5c6fe218f8a37cd917ed906e717 Mon Sep 17 00:00:00 2001 From: Hugh Chen Date: Tue, 13 Jul 2021 10:42:00 +0800 Subject: [PATCH] Fix "USB tethering" will auto turn on then off when turn off "USB tethering" This CL only updates the "USB tethering preference" when onTetheredInterfacesChanged() is invoked to avoid the "USB tethering preference" being changed when received non-tethering change intent. Bug: 185590309 Test: make -j42 RunSettingsRoboTests Change-Id: Ifed9f5c7edef670eeb21baf3841999382ee10ff2 --- src/com/android/settings/TetherSettings.java | 132 ++++++++++------- .../android/settings/TetherSettingsTest.java | 140 +++++++++++++++++- 2 files changed, 212 insertions(+), 60 deletions(-) diff --git a/src/com/android/settings/TetherSettings.java b/src/com/android/settings/TetherSettings.java index 6d98f360459..ab1c437271f 100644 --- a/src/com/android/settings/TetherSettings.java +++ b/src/com/android/settings/TetherSettings.java @@ -45,6 +45,7 @@ import android.os.UserManager; import android.provider.SearchIndexableResource; import android.text.TextUtils; import android.util.FeatureFlagUtils; +import android.util.Log; import androidx.annotation.VisibleForTesting; import androidx.preference.Preference; @@ -85,6 +86,7 @@ public class TetherSettings extends RestrictedSettingsFragment static final String KEY_TETHER_PREFS_TOP_INTRO = "tether_prefs_top_intro"; private static final String TAG = "TetheringSettings"; + private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private RestrictedSwitchPreference mUsbTether; @@ -94,7 +96,6 @@ public class TetherSettings extends RestrictedSettingsFragment private BroadcastReceiver mTetherChangeReceiver; - private String[] mUsbRegexs; private String[] mBluetoothRegexs; private String mEthernetRegex; private AtomicReference mBluetoothPan = new AtomicReference<>(); @@ -103,7 +104,6 @@ public class TetherSettings extends RestrictedSettingsFragment private OnStartTetheringCallback mStartTetheringCallback; private ConnectivityManager mCm; private EthernetManager mEm; - private TetheringManager mTm; private TetheringEventCallback mTetheringEventCallback; private EthernetListener mEthernetListener; @@ -119,6 +119,13 @@ public class TetherSettings extends RestrictedSettingsFragment private boolean mDataSaverEnabled; private Preference mDataSaverFooter; + @VisibleForTesting + String[] mUsbRegexs; + @VisibleForTesting + Context mContext; + @VisibleForTesting + TetheringManager mTm; + @Override public int getMetricsCategory() { return SettingsEnums.TETHER; @@ -140,7 +147,8 @@ public class TetherSettings extends RestrictedSettingsFragment super.onCreate(icicle); addPreferencesFromResource(R.xml.tether_prefs); - mDataSaverBackend = new DataSaverBackend(getContext()); + mContext = getContext(); + mDataSaverBackend = new DataSaverBackend(mContext); mDataSaverEnabled = mDataSaverBackend.isDataSaverEnabled(); mDataSaverFooter = findPreference(KEY_DATA_SAVER_FOOTER); @@ -169,7 +177,7 @@ public class TetherSettings extends RestrictedSettingsFragment mUsbRegexs = mTm.getTetherableUsbRegexs(); mBluetoothRegexs = mTm.getTetherableBluetoothRegexs(); - mEthernetRegex = getContext().getResources().getString( + mEthernetRegex = mContext.getResources().getString( com.android.internal.R.string.config_ethernet_iface_regex); final boolean usbAvailable = mUsbRegexs.length != 0; @@ -237,8 +245,7 @@ public class TetherSettings extends RestrictedSettingsFragment @VisibleForTesting void setTopIntroPreferenceTitle() { final Preference topIntroPreference = findPreference(KEY_TETHER_PREFS_TOP_INTRO); - final WifiManager wifiManager = - (WifiManager) getContext().getSystemService(Context.WIFI_SERVICE); + final WifiManager wifiManager = mContext.getSystemService(WifiManager.class); if (wifiManager.isStaApConcurrencySupported()) { topIntroPreference.setTitle(R.string.tethering_footer_info_sta_ap_concurrency); } else { @@ -250,27 +257,32 @@ public class TetherSettings extends RestrictedSettingsFragment @Override public void onReceive(Context content, Intent intent) { String action = intent.getAction(); - // TODO: stop using ACTION_TETHER_STATE_CHANGED and use mTetheringEventCallback instead. + if (DEBUG) { + Log.d(TAG, "onReceive() action : " + action); + } + // TODO(b/194961339): Stop using ACTION_TETHER_STATE_CHANGED and use + // mTetheringEventCallback instead. if (action.equals(TetheringManager.ACTION_TETHER_STATE_CHANGED)) { // TODO - this should understand the interface types ArrayList available = intent.getStringArrayListExtra( TetheringManager.EXTRA_AVAILABLE_TETHER); ArrayList active = intent.getStringArrayListExtra( TetheringManager.EXTRA_ACTIVE_TETHER); - ArrayList errored = intent.getStringArrayListExtra( - TetheringManager.EXTRA_ERRORED_TETHER); - updateState(available.toArray(new String[available.size()]), - active.toArray(new String[active.size()]), - errored.toArray(new String[errored.size()])); + updateBluetoothState(); + updateEthernetState(available.toArray(new String[available.size()]), + active.toArray(new String[active.size()])); } else if (action.equals(Intent.ACTION_MEDIA_SHARED)) { mMassStorageActive = true; - updateState(); + updateBluetoothAndEthernetState(); + updateUsbPreference(); } else if (action.equals(Intent.ACTION_MEDIA_UNSHARED)) { mMassStorageActive = false; - updateState(); + updateBluetoothAndEthernetState(); + updateUsbPreference(); } else if (action.equals(UsbManager.ACTION_USB_STATE)) { mUsbConnected = intent.getBooleanExtra(UsbManager.USB_CONNECTED, false); - updateState(); + updateBluetoothAndEthernetState(); + updateUsbPreference(); } else if (action.equals(BluetoothAdapter.ACTION_STATE_CHANGED)) { if (mBluetoothEnableForTether) { switch (intent @@ -289,9 +301,9 @@ public class TetherSettings extends RestrictedSettingsFragment // ignore transition states } } - updateState(); + updateBluetoothAndEthernetState(); } else if (action.equals(BluetoothPan.ACTION_TETHERING_STATE_CHANGED)) { - updateState(); + updateBluetoothAndEthernetState(); } } } @@ -320,7 +332,8 @@ public class TetherSettings extends RestrictedSettingsFragment if (mEm != null) mEm.addListener(mEthernetListener); - updateState(); + updateUsbState(); + updateBluetoothAndEthernetState(); } @Override @@ -366,60 +379,60 @@ public class TetherSettings extends RestrictedSettingsFragment if (intent != null) mTetherChangeReceiver.onReceive(activity, intent); } - private void updateState() { - final TetheringManager tm = getContext().getSystemService(TetheringManager.class); - final String[] available = tm.getTetherableIfaces(); - final String[] tethered = tm.getTetheredIfaces(); - final String[] errored = tm.getTetheringErroredIfaces(); - updateState(available, tethered, errored); + // TODO(b/194961339): Separate the updateBluetoothAndEthernetState() to two methods, + // updateBluetoothAndEthernetState() and updateBluetoothAndEthernetPreference(). + // Because we should update the state when only receiving tethering + // state changes and update preference when usb or media share changed. + private void updateBluetoothAndEthernetState() { + String[] tethered = mTm.getTetheredIfaces(); + updateBluetoothAndEthernetState(tethered); } - private void updateState(String[] available, String[] tethered, - String[] errored) { - updateUsbState(available, tethered, errored); + private void updateBluetoothAndEthernetState(String[] tethered) { + String[] available = mTm.getTetherableIfaces(); updateBluetoothState(); updateEthernetState(available, tethered); } + private void updateUsbState() { + String[] tethered = mTm.getTetheredIfaces(); + updateUsbState(tethered); + } + @VisibleForTesting - void updateUsbState(String[] available, String[] tethered, - String[] errored) { - boolean usbAvailable = mUsbConnected && !mMassStorageActive; - int usbError = ConnectivityManager.TETHER_ERROR_NO_ERROR; - for (String s : available) { - for (String regex : mUsbRegexs) { - if (s.matches(regex)) { - if (usbError == ConnectivityManager.TETHER_ERROR_NO_ERROR) { - usbError = mTm.getLastTetherError(s); - } - } - } - } + void updateUsbState(String[] tethered) { boolean usbTethered = false; for (String s : tethered) { for (String regex : mUsbRegexs) { if (s.matches(regex)) usbTethered = true; } } - boolean usbErrored = false; - for (String s: errored) { - for (String regex : mUsbRegexs) { - if (s.matches(regex)) usbErrored = true; - } + if (DEBUG) { + Log.d(TAG, "updateUsbState() mUsbConnected : " + mUsbConnected + + ", mMassStorageActive : " + mMassStorageActive + + ", usbTethered : " + usbTethered); } - if (usbTethered) { mUsbTether.setEnabled(!mDataSaverEnabled); mUsbTether.setChecked(true); - } else if (usbAvailable) { - mUsbTether.setEnabled(!mDataSaverEnabled); + mUsbTether.setDisabledByAdmin( + checkIfUsbDataSignalingIsDisabled(mContext, UserHandle.myUserId())); + } else { mUsbTether.setChecked(false); + updateUsbPreference(); + } + } + + private void updateUsbPreference() { + boolean usbAvailable = mUsbConnected && !mMassStorageActive; + + if (usbAvailable) { + mUsbTether.setEnabled(!mDataSaverEnabled); } else { mUsbTether.setEnabled(false); - mUsbTether.setChecked(false); } mUsbTether.setDisabledByAdmin( - checkIfUsbDataSignalingIsDisabled(getContext(), UserHandle.myUserId())); + checkIfUsbDataSignalingIsDisabled(mContext, UserHandle.myUserId())); } @VisibleForTesting @@ -439,7 +452,11 @@ public class TetherSettings extends RestrictedSettingsFragment private void updateBluetoothState() { final int btState = getBluetoothState(); + if (DEBUG) { + Log.d(TAG, "updateBluetoothState() btState : " + btState); + } if (btState == BluetoothAdapter.ERROR) { + Log.w(TAG, "updateBluetoothState() Bluetooth state is error!"); return; } @@ -460,7 +477,6 @@ public class TetherSettings extends RestrictedSettingsFragment @VisibleForTesting void updateEthernetState(String[] available, String[] tethered) { - boolean isAvailable = false; boolean isTethered = false; @@ -472,6 +488,11 @@ public class TetherSettings extends RestrictedSettingsFragment if (s.matches(mEthernetRegex)) isTethered = true; } + if (DEBUG) { + Log.d(TAG, "updateEthernetState() isAvailable : " + isAvailable + + ", isTethered : " + isTethered); + } + if (isTethered) { mEthernetTether.setEnabled(!mDataSaverEnabled); mEthernetTether.setChecked(true); @@ -608,7 +629,7 @@ public class TetherSettings extends RestrictedSettingsFragment private void update() { TetherSettings settings = mTetherSettings.get(); if (settings != null) { - settings.updateState(); + settings.updateBluetoothAndEthernetState(); } } } @@ -616,13 +637,16 @@ public class TetherSettings extends RestrictedSettingsFragment private final class TetheringEventCallback implements TetheringManager.TetheringEventCallback { @Override public void onTetheredInterfacesChanged(List interfaces) { - updateState(); + Log.d(TAG, "onTetheredInterfacesChanged() interfaces : " + interfaces.toString()); + String[] tethered = interfaces.toArray(new String[interfaces.size()]); + updateUsbState(tethered); + updateBluetoothAndEthernetState(tethered); } } private final class EthernetListener implements EthernetManager.Listener { public void onAvailabilityChanged(String iface, boolean isAvailable) { - mHandler.post(TetherSettings.this::updateState); + mHandler.post(() -> updateBluetoothAndEthernetState()); } } } diff --git a/tests/robotests/src/com/android/settings/TetherSettingsTest.java b/tests/robotests/src/com/android/settings/TetherSettingsTest.java index 0948cfad24e..71cb9d21045 100644 --- a/tests/robotests/src/com/android/settings/TetherSettingsTest.java +++ b/tests/robotests/src/com/android/settings/TetherSettingsTest.java @@ -16,14 +16,20 @@ package com.android.settings; +import static android.content.Intent.ACTION_MEDIA_SHARED; +import static android.content.Intent.ACTION_MEDIA_UNSHARED; +import static android.hardware.usb.UsbManager.ACTION_USB_STATE; + import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -33,6 +39,7 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.hardware.usb.UsbManager; import android.net.ConnectivityManager; import android.net.TetheringManager; import android.net.wifi.WifiManager; @@ -45,6 +52,7 @@ import androidx.preference.Preference; import androidx.preference.SwitchPreference; import com.android.settings.core.FeatureFlags; +import com.android.settingslib.RestrictedSwitchPreference; import org.junit.Before; import org.junit.Test; @@ -71,7 +79,7 @@ public class TetherSettingsTest { private TetheringManager mTetheringManager; @Before - public void setUp() { + public void setUp() throws Exception { mContext = spy(RuntimeEnvironment.application); MockitoAnnotations.initMocks(this); @@ -81,6 +89,8 @@ public class TetherSettingsTest { .when(mContext).getSystemService(Context.USER_SERVICE); doReturn(mTetheringManager) .when(mContext).getSystemService(Context.TETHERING_SERVICE); + doReturn(mContext).when(mContext).createPackageContextAsUser( + any(String.class), anyInt(), any(UserHandle.class)); setupIsTetherAvailable(true); @@ -161,7 +171,7 @@ public class TetherSettingsTest { @Test public void testSetFooterPreferenceTitle_isStaApConcurrencySupported_showStaApString() { final TetherSettings spyTetherSettings = spy(new TetherSettings()); - when(spyTetherSettings.getContext()).thenReturn(mContext); + spyTetherSettings.mContext = mContext; final Preference mockPreference = mock(Preference.class); when(spyTetherSettings.findPreference(TetherSettings.KEY_TETHER_PREFS_TOP_INTRO)) .thenReturn(mockPreference); @@ -178,7 +188,8 @@ public class TetherSettingsTest { @Test public void testBluetoothState_updateBluetoothState_bluetoothTetheringStateOn() { final TetherSettings spyTetherSettings = spy(new TetherSettings()); - when(spyTetherSettings.getContext()).thenReturn(mContext); + spyTetherSettings.mContext = mContext; + spyTetherSettings.mTm = mTetheringManager; final SwitchPreference mockSwitchPreference = mock(SwitchPreference.class); when(spyTetherSettings.findPreference(TetherSettings.KEY_ENABLE_BLUETOOTH_TETHERING)) .thenReturn(mockSwitchPreference); @@ -210,7 +221,8 @@ public class TetherSettingsTest { @Test public void testBluetoothState_updateBluetoothState_bluetoothTetheringStateOff() { final TetherSettings spyTetherSettings = spy(new TetherSettings()); - when(spyTetherSettings.getContext()).thenReturn(mContext); + spyTetherSettings.mContext = mContext; + spyTetherSettings.mTm = mTetheringManager; final SwitchPreference mockSwitchPreference = mock(SwitchPreference.class); when(spyTetherSettings.findPreference(TetherSettings.KEY_ENABLE_BLUETOOTH_TETHERING)) .thenReturn(mockSwitchPreference); @@ -239,14 +251,110 @@ public class TetherSettingsTest { verify(mockSwitchPreference).setChecked(false); } + @Test + public void updateState_usbTetheringIsEnabled_checksUsbTethering() { + String [] tethered = {"rndis0"}; + TetherSettings spyTetherSettings = spy(new TetherSettings()); + RestrictedSwitchPreference tetheringPreference = mock(RestrictedSwitchPreference.class); + when(spyTetherSettings.findPreference(TetherSettings.KEY_USB_TETHER_SETTINGS)) + .thenReturn(tetheringPreference); + spyTetherSettings.mContext = mContext; + spyTetherSettings.mTm = mTetheringManager; + spyTetherSettings.setupTetherPreference(); + spyTetherSettings.mUsbRegexs = tethered; + + spyTetherSettings.updateUsbState(tethered); + + verify(tetheringPreference).setEnabled(true); + verify(tetheringPreference).setChecked(true); + } + + @Test + public void updateState_usbTetheringIsDisabled_unchecksUsbTethering() { + String [] tethered = {"rndis0"}; + TetherSettings spyTetherSettings = spy(new TetherSettings()); + RestrictedSwitchPreference tetheringPreference = mock(RestrictedSwitchPreference.class); + when(spyTetherSettings.findPreference(TetherSettings.KEY_USB_TETHER_SETTINGS)) + .thenReturn(tetheringPreference); + spyTetherSettings.mContext = mContext; + spyTetherSettings.mTm = mTetheringManager; + spyTetherSettings.setupTetherPreference(); + spyTetherSettings.mUsbRegexs = tethered; + + spyTetherSettings.updateUsbState(new String[0]); + + verify(tetheringPreference).setEnabled(false); + verify(tetheringPreference).setChecked(false); + } + + @Test + public void onReceive_usbIsConnected_tetheringPreferenceIsEnabled() { + RestrictedSwitchPreference tetheringPreference = mock(RestrictedSwitchPreference.class); + FragmentActivity mockActivity = mock(FragmentActivity.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(BroadcastReceiver.class); + setupUsbStateComponents(tetheringPreference, captor, mockActivity); + + BroadcastReceiver receiver = captor.getValue(); + Intent usbStateChanged = new Intent(ACTION_USB_STATE); + usbStateChanged.putExtra(UsbManager.USB_CONNECTED, true); + receiver.onReceive(mockActivity, usbStateChanged); + + verify(tetheringPreference).setEnabled(true); + } + + @Test + public void onReceive_usbIsDisconnected_tetheringPreferenceIsDisabled() { + RestrictedSwitchPreference tetheringPreference = mock(RestrictedSwitchPreference.class); + FragmentActivity mockActivity = mock(FragmentActivity.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(BroadcastReceiver.class); + setupUsbStateComponents(tetheringPreference, captor, mockActivity); + + BroadcastReceiver receiver = captor.getValue(); + Intent usbStateChanged = new Intent(ACTION_USB_STATE); + usbStateChanged.putExtra(UsbManager.USB_CONNECTED, false); + receiver.onReceive(mockActivity, usbStateChanged); + + verify(tetheringPreference).setEnabled(false); + } + + @Test + public void onReceive_mediaIsShared_tetheringPreferenceIsDisabled() { + RestrictedSwitchPreference tetheringPreference = mock(RestrictedSwitchPreference.class); + FragmentActivity mockActivity = mock(FragmentActivity.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(BroadcastReceiver.class); + setupUsbStateComponents(tetheringPreference, captor, mockActivity); + + BroadcastReceiver receiver = captor.getValue(); + Intent mediaIsShared = new Intent(ACTION_MEDIA_SHARED); + receiver.onReceive(mockActivity, mediaIsShared); + + verify(tetheringPreference).setEnabled(false); + } + + @Test + public void onReceive_mediaIsUnshared_tetheringPreferenceIsEnabled() { + RestrictedSwitchPreference tetheringPreference = mock(RestrictedSwitchPreference.class); + FragmentActivity mockActivity = mock(FragmentActivity.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(BroadcastReceiver.class); + setupUsbStateComponents(tetheringPreference, captor, mockActivity); + + BroadcastReceiver receiver = captor.getValue(); + Intent mediaIsShared = new Intent(ACTION_MEDIA_UNSHARED); + Intent usbStateChanged = new Intent(ACTION_USB_STATE); + usbStateChanged.putExtra(UsbManager.USB_CONNECTED, true); + receiver.onReceive(mockActivity, usbStateChanged); + receiver.onReceive(mockActivity, mediaIsShared); + + verify(tetheringPreference, times(2)).setEnabled(true); + } + private void updateOnlyBluetoothState(TetherSettings tetherSettings) { doReturn(mTetheringManager).when(tetherSettings) .getSystemService(Context.TETHERING_SERVICE); when(mTetheringManager.getTetherableIfaces()).thenReturn(new String[0]); when(mTetheringManager.getTetheredIfaces()).thenReturn(new String[0]); when(mTetheringManager.getTetheringErroredIfaces()).thenReturn(new String[0]); - doNothing().when(tetherSettings).updateUsbState(any(String[].class), any(String[].class), - any(String[].class)); + doNothing().when(tetherSettings).updateUsbState(any(String[].class)); doNothing().when(tetherSettings).updateEthernetState(any(String[].class), any(String[].class)); } @@ -266,4 +374,24 @@ public class TetherSettingsTest { UserManager.DISALLOW_CONFIG_TETHERING, UserHandle.of(userId))) .thenReturn(!returnValue); } + + private void setupUsbStateComponents(RestrictedSwitchPreference preference, + ArgumentCaptor captor, FragmentActivity activity) { + TetherSettings spyTetherSettings = spy(new TetherSettings()); + SwitchPreference mockSwitchPreference = mock(SwitchPreference.class); + + when(spyTetherSettings.findPreference(TetherSettings.KEY_USB_TETHER_SETTINGS)) + .thenReturn(preference); + when(spyTetherSettings.findPreference(TetherSettings.KEY_ENABLE_BLUETOOTH_TETHERING)) + .thenReturn(mockSwitchPreference); + spyTetherSettings.mContext = mContext; + spyTetherSettings.mTm = mTetheringManager; + when(spyTetherSettings.getActivity()).thenReturn(activity); + when(activity.registerReceiver(captor.capture(), any(IntentFilter.class))) + .thenReturn(null); + + spyTetherSettings.setupTetherPreference(); + spyTetherSettings.registerReceiver(); + updateOnlyBluetoothState(spyTetherSettings); + } }