From 086d062cb28593a6bcbd54925a00dbd682280c8e Mon Sep 17 00:00:00 2001 From: Weng Su Date: Wed, 31 May 2023 20:34:30 +0800 Subject: [PATCH] Fix TetherSettings crash issue - When the user is a guest user, the UI will remove all preferences to restrict setting changes. If the ViewModel updates the UI in this situation, it will cause Settings to crash. - Avoid to setup ViewModel when UI is restricted Bug: 284435378 Test: Manual test atest -c TetherSettingsTest Change-Id: I52d4ea717c34eacc9cc2321e3950dc89408049f8 --- .../network/tether/TetherSettings.java | 35 ++++++++++--------- .../network/tether/TetherSettingsTest.java | 34 ++++++++++++++++-- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/com/android/settings/network/tether/TetherSettings.java b/src/com/android/settings/network/tether/TetherSettings.java index 9fa8730cf10..6f6ba8efa71 100644 --- a/src/com/android/settings/network/tether/TetherSettings.java +++ b/src/com/android/settings/network/tether/TetherSettings.java @@ -145,19 +145,13 @@ public class TetherSettings extends RestrictedSettingsFragment super(UserManager.DISALLOW_CONFIG_TETHERING); } - @Override - public void onAttach(Context context) { - super.onAttach(context); - TetheringManagerModel model = new ViewModelProvider(this).get(TetheringManagerModel.class); - mWifiTetherPreferenceController = - new WifiTetherPreferenceController(context, getSettingsLifecycle(), model); - mTm = model.getTetheringManager(); - model.getTetheredInterfaces().observe(this, this::onTetheredInterfacesChanged); - } - @Override public void onCreate(Bundle icicle) { super.onCreate(icicle); + setIfOnlyAvailableForAdmins(true); + if (isUiRestricted()) { + return; + } addPreferencesFromResource(R.xml.tether_prefs); mContext = getContext(); @@ -165,13 +159,8 @@ public class TetherSettings extends RestrictedSettingsFragment mDataSaverEnabled = mDataSaverBackend.isDataSaverEnabled(); mDataSaverFooter = findPreference(KEY_DATA_SAVER_FOOTER); - setIfOnlyAvailableForAdmins(true); - if (isUiRestricted()) { - getPreferenceScreen().removeAll(); - return; - } - setupTetherPreference(); + setupViewModel(); final Activity activity = getActivity(); BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter(); @@ -223,8 +212,22 @@ public class TetherSettings extends RestrictedSettingsFragment onDataSaverChanged(mDataSaverBackend.isDataSaverEnabled()); } + @VisibleForTesting + void setupViewModel() { + TetheringManagerModel model = new ViewModelProvider(this).get(TetheringManagerModel.class); + mWifiTetherPreferenceController = + new WifiTetherPreferenceController(getContext(), getSettingsLifecycle(), model); + mTm = model.getTetheringManager(); + model.getTetheredInterfaces().observe(this, this::onTetheredInterfacesChanged); + } + @Override public void onDestroy() { + if (isUiRestricted()) { + super.onDestroy(); + return; + } + mDataSaverBackend.remListener(this); BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter(); diff --git a/tests/robotests/src/com/android/settings/network/tether/TetherSettingsTest.java b/tests/robotests/src/com/android/settings/network/tether/TetherSettingsTest.java index 23d9b825ad6..bf03e82657a 100644 --- a/tests/robotests/src/com/android/settings/network/tether/TetherSettingsTest.java +++ b/tests/robotests/src/com/android/settings/network/tether/TetherSettingsTest.java @@ -45,6 +45,7 @@ import android.hardware.usb.UsbManager; import android.net.ConnectivityManager; import android.net.TetheringManager; import android.net.wifi.WifiManager; +import android.os.Bundle; import android.os.UserHandle; import android.os.UserManager; import android.util.FeatureFlagUtils; @@ -54,6 +55,7 @@ import androidx.preference.Preference; import androidx.preference.SwitchPreference; import com.android.settings.R; +import com.android.settings.RestrictedSettingsFragment; import com.android.settings.core.FeatureFlags; import com.android.settings.wifi.tether.WifiTetherPreferenceController; import com.android.settingslib.RestrictedSwitchPreference; @@ -66,6 +68,9 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; import java.util.ArrayList; import java.util.List; @@ -92,7 +97,7 @@ public class TetherSettingsTest { @Mock private Preference mDataSaverFooter; - TetherSettings mTetherSettings; + private MockTetherSettings mTetherSettings; @Before public void setUp() throws Exception { @@ -114,7 +119,7 @@ public class TetherSettingsTest { when(mTetheringManager.getTetherableUsbRegexs()).thenReturn(new String[0]); when(mTetheringManager.getTetherableBluetoothRegexs()).thenReturn(new String[0]); - mTetherSettings = spy(new TetherSettings()); + mTetherSettings = spy(new MockTetherSettings()); mTetherSettings.mContext = mContext; mTetherSettings.mWifiTetherPreferenceController = mWifiTetherPreferenceController; mTetherSettings.mUsbTether = mUsbTether; @@ -123,6 +128,16 @@ public class TetherSettingsTest { mTetherSettings.mDataSaverFooter = mDataSaverFooter; } + @Test + @Config(shadows = ShadowRestrictedSettingsFragment.class) + public void onCreate_isUiRestricted_doNotSetupViewModel() { + when(mTetherSettings.isUiRestricted()).thenReturn(true); + + mTetherSettings.onCreate(null); + + verify(mTetherSettings, never()).setupViewModel(); + } + @Test public void testTetherNonIndexableKeys_tetherAvailable_keysNotReturned() { FeatureFlagUtils.setEnabled(mContext, FeatureFlags.TETHER_ALL_IN_ONE, false); @@ -431,4 +446,19 @@ public class TetherSettingsTest { mTetherSettings.registerReceiver(); updateOnlyBluetoothState(mTetherSettings); } + + private static class MockTetherSettings extends TetherSettings { + @Override + public boolean isUiRestricted() { + return false; + } + } + + @Implements(RestrictedSettingsFragment.class) + public static final class ShadowRestrictedSettingsFragment { + @Implementation + public void onCreate(Bundle icicle) { + // do nothing + } + } }