From a07738266b24a0b12c2d7d767668b38d59942c56 Mon Sep 17 00:00:00 2001 From: Rambo Wang Date: Wed, 19 Jun 2024 02:09:41 +0000 Subject: [PATCH 1/3] Fix Settings restart during Reset mobile nework settings flow This CL avoids restarting Settings in the reset mobile flow when phone process is restarted, by switching the usage of the stable content provider connection to the unstable client. The CL also arranges restarting phone process as the last reset operation in the flow (later than RILD reset) to avoid any reset operation get impacted by phone process restarting. Since the permission to protect the TelephonyContentProvider has been renamed, the CL also renames the requsted permision. Bug: 347047105 Test: atest ResetNetworkOperationBuilderTest Test: Reset mobile network feature test Flag: EXEMPT resource update with minor refactoring (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5ac9d9c8fa0005f787b75052010dcc9935efcb4f) Merged-In: I7bfa79bc9d7451a4a03269704b0009a3730e287f Change-Id: I7bfa79bc9d7451a4a03269704b0009a3730e287f --- AndroidManifest.xml | 2 +- .../android/settings/ResetNetworkRequest.java | 6 +-- .../network/ResetNetworkOperationBuilder.java | 51 +++++++++++-------- .../ResetNetworkOperationBuilderTest.java | 35 ++++++------- 4 files changed, 50 insertions(+), 44 deletions(-) diff --git a/AndroidManifest.xml b/AndroidManifest.xml index 7daf136a40b..69d000a9e89 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -140,7 +140,7 @@ - + diff --git a/src/com/android/settings/ResetNetworkRequest.java b/src/com/android/settings/ResetNetworkRequest.java index 7632ea01d71..8df67e771b2 100644 --- a/src/com/android/settings/ResetNetworkRequest.java +++ b/src/com/android/settings/ResetNetworkRequest.java @@ -271,12 +271,12 @@ public class ResetNetworkRequest { builder.resetIms(mSubscriptionIdToResetIms); } // Reset phone process and RILD may impact above components, keep them at the end - if ((mResetOptions & RESET_PHONE_PROCESS) != 0) { - builder.restartPhoneProcess(); - } if ((mResetOptions & RESET_RILD) != 0) { builder.restartRild(); } + if ((mResetOptions & RESET_PHONE_PROCESS) != 0) { + builder.restartPhoneProcess(); + } return builder; } } diff --git a/src/com/android/settings/network/ResetNetworkOperationBuilder.java b/src/com/android/settings/network/ResetNetworkOperationBuilder.java index 6f36074d145..47c06d4480d 100644 --- a/src/com/android/settings/network/ResetNetworkOperationBuilder.java +++ b/src/com/android/settings/network/ResetNetworkOperationBuilder.java @@ -18,6 +18,7 @@ package com.android.settings.network; import android.bluetooth.BluetoothAdapter; import android.bluetooth.BluetoothManager; +import android.content.ContentProviderClient; import android.content.ContentResolver; import android.content.Context; import android.net.ConnectivityManager; @@ -28,11 +29,14 @@ import android.net.wifi.WifiManager; import android.net.wifi.p2p.WifiP2pManager; import android.os.Looper; import android.os.RecoverySystem; +import android.os.RemoteException; import android.os.SystemClock; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; import android.util.Log; +import androidx.annotation.Nullable; + import com.android.internal.annotations.VisibleForTesting; import com.android.settings.R; import com.android.settings.ResetNetworkRequest; @@ -257,15 +261,15 @@ public class ResetNetworkOperationBuilder { */ public ResetNetworkOperationBuilder restartPhoneProcess() { Runnable runnable = () -> { - try { - mContext.getContentResolver().call( - getResetTelephonyContentProviderAuthority(), - METHOD_RESTART_PHONE_PROCESS, - /* arg= */ null, - /* extras= */ null); - Log.i(TAG, "Phone process was restarted."); - } catch (IllegalArgumentException iae) { - Log.w(TAG, "Fail to restart phone process: " + iae); + // Unstable content provider can avoid us getting killed together with phone process + try (ContentProviderClient client = getUnstableTelephonyContentProviderClient()) { + if (client != null) { + client.call(METHOD_RESTART_PHONE_PROCESS, /* arg= */ null, /* extra= */ null); + Log.i(TAG, "Phone process was restarted."); + } + } catch (RemoteException re) { + // It's normal to throw RE since phone process immediately dies + Log.i(TAG, "Phone process has been restarted: " + re); } }; mResetSequence.add(runnable); @@ -279,15 +283,13 @@ public class ResetNetworkOperationBuilder { */ public ResetNetworkOperationBuilder restartRild() { Runnable runnable = () -> { - try { - mContext.getContentResolver().call( - getResetTelephonyContentProviderAuthority(), - METHOD_RESTART_RILD, - /* arg= */ null, - /* extras= */ null); - Log.i(TAG, "RILD was restarted."); - } catch (IllegalArgumentException iae) { - Log.w(TAG, "Fail to restart RILD: " + iae); + try (ContentProviderClient client = getUnstableTelephonyContentProviderClient()) { + if (client != null) { + client.call(METHOD_RESTART_RILD, /* arg= */ null, /* extra= */ null); + Log.i(TAG, "RILD was restarted."); + } + } catch (RemoteException re) { + Log.w(TAG, "Fail to restart RILD: " + re); } }; mResetSequence.add(runnable); @@ -322,9 +324,18 @@ public class ResetNetworkOperationBuilder { * @return the authority of the telephony content provider that support methods * resetPhoneProcess and resetRild. */ - @VisibleForTesting - String getResetTelephonyContentProviderAuthority() { + private String getResetTelephonyContentProviderAuthority() { return mContext.getResources().getString( R.string.reset_telephony_stack_content_provider_authority); } + + /** + * @return the unstable content provider to avoid us getting killed with phone process + */ + @Nullable + @VisibleForTesting + public ContentProviderClient getUnstableTelephonyContentProviderClient() { + return mContext.getContentResolver().acquireUnstableContentProviderClient( + getResetTelephonyContentProviderAuthority()); + } } diff --git a/tests/unit/src/com/android/settings/network/ResetNetworkOperationBuilderTest.java b/tests/unit/src/com/android/settings/network/ResetNetworkOperationBuilderTest.java index 5f544064924..7f1c475dbfd 100644 --- a/tests/unit/src/com/android/settings/network/ResetNetworkOperationBuilderTest.java +++ b/tests/unit/src/com/android/settings/network/ResetNetworkOperationBuilderTest.java @@ -16,20 +16,16 @@ package com.android.settings.network; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.doThrow; 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 android.content.ContentProvider; -import android.content.ContentResolver; +import android.content.ContentProviderClient; import android.content.Context; import android.net.ConnectivityManager; import android.net.NetworkPolicyManager; @@ -67,7 +63,7 @@ public class ResetNetworkOperationBuilderTest { @Mock private NetworkPolicyManager mNetworkPolicyManager; @Mock - private ContentProvider mContentProvider;; + private ContentProviderClient mContentProviderClient; private Context mContext; @@ -77,9 +73,8 @@ public class ResetNetworkOperationBuilderTest { public void setUp() { MockitoAnnotations.initMocks(this); mContext = spy(ApplicationProvider.getApplicationContext()); - doReturn(ContentResolver.wrap(mContentProvider)).when(mContext).getContentResolver(); - mBuilder = spy(new ResetNetworkOperationBuilder(mContext)); + doReturn(mContentProviderClient).when(mBuilder).getUnstableTelephonyContentProviderClient(); } @Test @@ -184,38 +179,38 @@ public class ResetNetworkOperationBuilderTest { } @Test - public void restartPhoneProcess_withoutTelephonyContentProvider_shouldNotCrash() { - doThrow(new IllegalArgumentException()).when(mContentProvider).call( - anyString(), anyString(), anyString(), any()); + public void restartPhoneProcess_withoutTelephonyContentProvider_shouldNotCrash() + throws Exception { + doReturn(null).when(mBuilder).getUnstableTelephonyContentProviderClient(); mBuilder.restartPhoneProcess().build().run(); } @Test - public void restartRild_withoutTelephonyContentProvider_shouldNotCrash() { - doThrow(new IllegalArgumentException()).when(mContentProvider).call( - anyString(), anyString(), anyString(), any()); + public void restartRild_withoutTelephonyContentProvider_shouldNotCrash() + throws Exception { + doReturn(null).when(mBuilder).getUnstableTelephonyContentProviderClient(); mBuilder.restartRild().build().run(); } @Test - public void restartPhoneProcess_withTelephonyContentProvider_shouldCallRestartPhoneProcess() { + public void restartPhoneProcess_withTelephonyContentProvider_shouldCallRestartPhoneProcess() + throws Exception { mBuilder.restartPhoneProcess().build().run(); - verify(mContentProvider).call( - eq(mBuilder.getResetTelephonyContentProviderAuthority()), + verify(mContentProviderClient).call( eq(ResetNetworkOperationBuilder.METHOD_RESTART_PHONE_PROCESS), isNull(), isNull()); } @Test - public void restartRild_withTelephonyContentProvider_shouldCallRestartRild() { + public void restartRild_withTelephonyContentProvider_shouldCallRestartRild() + throws Exception { mBuilder.restartRild().build().run(); - verify(mContentProvider).call( - eq(mBuilder.getResetTelephonyContentProviderAuthority()), + verify(mContentProviderClient).call( eq(ResetNetworkOperationBuilder.METHOD_RESTART_RILD), isNull(), isNull()); From 8822907513872445183a9c486bdd76a2c615f142 Mon Sep 17 00:00:00 2001 From: Haijie Hong Date: Tue, 25 Jun 2024 13:23:07 +0800 Subject: [PATCH 2/3] Add device to to cached device manager if it's not present Bug: 346923808 Test: atest BluetoothDeviceDetailsFragmentTest Flag: EXEMPT minor bug fix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a074f274991f6a172855e0253f49b0a6dd63c758) Merged-In: Ia4987bc7ec93cb6b54d188922b7232d83d528f2f Change-Id: Ia4987bc7ec93cb6b54d188922b7232d83d528f2f --- .../bluetooth/BluetoothDeviceDetailsFragment.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/com/android/settings/bluetooth/BluetoothDeviceDetailsFragment.java b/src/com/android/settings/bluetooth/BluetoothDeviceDetailsFragment.java index 44915fe2829..5f9957b9121 100644 --- a/src/com/android/settings/bluetooth/BluetoothDeviceDetailsFragment.java +++ b/src/com/android/settings/bluetooth/BluetoothDeviceDetailsFragment.java @@ -142,13 +142,23 @@ public class BluetoothDeviceDetailsFragment extends RestrictedDashboardFragment } @VisibleForTesting + @Nullable CachedBluetoothDevice getCachedDevice(String deviceAddress) { if (sTestDataFactory != null) { return sTestDataFactory.getDevice(deviceAddress); } BluetoothDevice remoteDevice = mManager.getBluetoothAdapter().getRemoteDevice(deviceAddress); - return mManager.getCachedDeviceManager().findDevice(remoteDevice); + if (remoteDevice == null) { + return null; + } + CachedBluetoothDevice cachedDevice = + mManager.getCachedDeviceManager().findDevice(remoteDevice); + if (cachedDevice != null) { + return cachedDevice; + } + Log.i(TAG, "Add device to cached device manager: " + remoteDevice.getAnonymizedAddress()); + return mManager.getCachedDeviceManager().addDevice(remoteDevice); } @VisibleForTesting From a6149ddf36031c6b3d4449903da8585980562b4e Mon Sep 17 00:00:00 2001 From: yomna Date: Tue, 16 Jul 2024 00:54:15 +0000 Subject: [PATCH 3/3] Remove searchability of CellularSecuritySettingsFragment Test: m & manual Bug: b/335554085 Flag: EXEMPT bug fix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:62004763e1f131085b0e834b6c8b01b563f55b73) Merged-In: Ia226b0848d70a307b80060049ec24b9714ee476a Change-Id: Ia226b0848d70a307b80060049ec24b9714ee476a --- .../network/telephony/CellularSecuritySettingsFragment.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/com/android/settings/network/telephony/CellularSecuritySettingsFragment.java b/src/com/android/settings/network/telephony/CellularSecuritySettingsFragment.java index 3e37352867d..20385b93fc0 100644 --- a/src/com/android/settings/network/telephony/CellularSecuritySettingsFragment.java +++ b/src/com/android/settings/network/telephony/CellularSecuritySettingsFragment.java @@ -20,13 +20,10 @@ import android.os.Bundle; import com.android.settings.R; import com.android.settings.dashboard.DashboardFragment; -import com.android.settings.search.BaseSearchIndexProvider; -import com.android.settingslib.search.SearchIndexable; /** * Cellular Security features (insecure network notifications, network security controls, etc) */ -@SearchIndexable public class CellularSecuritySettingsFragment extends DashboardFragment { private static final String TAG = "CellularSecuritySettingsFragment"; @@ -53,7 +50,4 @@ public class CellularSecuritySettingsFragment extends DashboardFragment { super.onCreatePreferences(bundle, rootKey); setPreferencesFromResource(R.xml.cellular_security, rootKey); } - - public static final BaseSearchIndexProvider SEARCH_INDEX_DATA_PROVIDER = - new BaseSearchIndexProvider(R.xml.cellular_security); }