From 80462c370b26dcaca2ed5574878f234058e49616 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Sat, 29 Apr 2023 23:18:44 +0800 Subject: [PATCH 01/10] FRP bypass defense in the Settings App for SPA Over the last few years, there have been a number of Factory Reset Protection bypass bugs in the SUW flow. It's unlikely to defense all points from individual apps. Therefore, we decide to block some critical pages when user doesn't complete the SUW flow. Fix: 280154358 Test: Unit test Change-Id: I06e73386711d5ad13c89d033cf0fe3164781c0ef --- src/com/android/settings/spa/SpaActivity.kt | 20 +++++++ .../android/settings/spa/SpaActivityTest.kt | 56 ++++++++++++++++--- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/spa/SpaActivity.kt b/src/com/android/settings/spa/SpaActivity.kt index 27f7241b257..2b52b21af0c 100644 --- a/src/com/android/settings/spa/SpaActivity.kt +++ b/src/com/android/settings/spa/SpaActivity.kt @@ -22,14 +22,34 @@ import android.content.Intent import android.os.RemoteException import android.os.UserHandle import android.util.Log +import androidx.annotation.VisibleForTesting +import com.android.settings.spa.app.appinfo.AppInfoSettingsProvider import com.android.settingslib.spa.framework.BrowseActivity +import com.android.settingslib.spa.framework.common.SettingsPage import com.android.settingslib.spa.framework.util.SESSION_BROWSE import com.android.settingslib.spa.framework.util.SESSION_EXTERNAL import com.android.settingslib.spa.framework.util.appendSpaParams +import com.google.android.setupcompat.util.WizardManagerHelper class SpaActivity : BrowseActivity() { + override fun isPageEnabled(page: SettingsPage) = + super.isPageEnabled(page) && !isSuwAndPageBlocked(page.sppName) + companion object { private const val TAG = "SpaActivity" + + /** The pages that blocked from SUW. */ + private val SuwBlockedPages = setOf(AppInfoSettingsProvider.name) + + @VisibleForTesting + fun Context.isSuwAndPageBlocked(name: String): Boolean = + if (name in SuwBlockedPages && !WizardManagerHelper.isDeviceProvisioned(this)) { + Log.w(TAG, "$name blocked before SUW completed."); + true + } else { + false + } + @JvmStatic fun Context.startSpaActivity(destination: String) { val intent = Intent(this, SpaActivity::class.java) diff --git a/tests/spa_unit/src/com/android/settings/spa/SpaActivityTest.kt b/tests/spa_unit/src/com/android/settings/spa/SpaActivityTest.kt index 46b956e6ccc..1b2a7b1ee90 100644 --- a/tests/spa_unit/src/com/android/settings/spa/SpaActivityTest.kt +++ b/tests/spa_unit/src/com/android/settings/spa/SpaActivityTest.kt @@ -21,33 +21,71 @@ import android.content.Intent import android.net.Uri import android.os.UserHandle import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.dx.mockito.inline.extended.ExtendedMockito +import com.android.settings.spa.SpaActivity.Companion.isSuwAndPageBlocked import com.android.settings.spa.SpaActivity.Companion.startSpaActivity import com.android.settings.spa.SpaActivity.Companion.startSpaActivityForApp +import com.android.settings.spa.app.AllAppListPageProvider +import com.android.settings.spa.app.appinfo.AppInfoSettingsProvider import com.android.settingslib.spa.framework.util.KEY_DESTINATION +import com.google.android.setupcompat.util.WizardManagerHelper import com.google.common.truth.Truth.assertThat +import org.junit.After import org.junit.Before -import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.mockito.Answers import org.mockito.ArgumentCaptor import org.mockito.Mock import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` -import org.mockito.junit.MockitoJUnit -import org.mockito.junit.MockitoRule +import org.mockito.MockitoSession +import org.mockito.quality.Strictness +import org.mockito.Mockito.`when` as whenever @RunWith(AndroidJUnit4::class) class SpaActivityTest { - @get:Rule - val mockito: MockitoRule = MockitoJUnit.rule() + private lateinit var mockSession: MockitoSession - @Mock(answer = Answers.RETURNS_DEEP_STUBS) + @Mock private lateinit var context: Context @Before fun setUp() { - `when`(context.applicationContext.packageName).thenReturn("com.android.settings") + mockSession = ExtendedMockito.mockitoSession() + .initMocks(this) + .mockStatic(WizardManagerHelper::class.java) + .strictness(Strictness.LENIENT) + .startMocking() + whenever(context.applicationContext).thenReturn(context) + } + + @After + fun tearDown() { + mockSession.finishMocking() + } + + @Test + fun isSuwAndPageBlocked_regularPage_notBlocked() { + val isBlocked = context.isSuwAndPageBlocked(AllAppListPageProvider.name) + + assertThat(isBlocked).isFalse() + } + + @Test + fun isSuwAndPageBlocked_blocklistedPageInSuw_blocked() { + whenever(WizardManagerHelper.isDeviceProvisioned(context)).thenReturn(false) + + val isBlocked = context.isSuwAndPageBlocked(AppInfoSettingsProvider.name) + + assertThat(isBlocked).isTrue() + } + + @Test + fun isSuwAndPageBlocked_blocklistedPageNotInSuw_notBlocked() { + whenever(WizardManagerHelper.isDeviceProvisioned(context)).thenReturn(true) + + val isBlocked = context.isSuwAndPageBlocked(AppInfoSettingsProvider.name) + + assertThat(isBlocked).isFalse() } @Test From 6d1615163984182bae8f46d0913921c424043418 Mon Sep 17 00:00:00 2001 From: Oli Thompson Date: Tue, 2 May 2023 18:00:20 +0000 Subject: [PATCH 02/10] Change icon in settings work challenge Screenshots in bug Test: manually tested Bug: 279559156 Change-Id: If3c9415db476f0ca0d26e74311e7bcd8d7e33dbb --- res/drawable/ic_enterprise.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/res/drawable/ic_enterprise.xml b/res/drawable/ic_enterprise.xml index c2d9df61893..231f706b83f 100644 --- a/res/drawable/ic_enterprise.xml +++ b/res/drawable/ic_enterprise.xml @@ -17,9 +17,9 @@ + android:viewportWidth="960" + android:viewportHeight="960"> \ No newline at end of file From 0b79c92348f33903cb618a429e562181d95fcb4d Mon Sep 17 00:00:00 2001 From: danielwbhuang Date: Wed, 3 May 2023 23:59:55 +0800 Subject: [PATCH 03/10] [Fixed] Reverse scrolling setting is reversed If useTouchpadNaturalScrolling is false, "Reverse scrolling" should be on. [The API value] useTouchpadNaturalScrolling: false [The expected UX behavior] Reverse scrolling: on fingers upward, scroll up, content moves down The description of "useTouchpadNaturalScrolling": Returns true if moving two fingers upwards on the touchpad should scroll down, which is known as natural scrolling. The description of "Reverse scrolling": Content moves up when you scroll down. Bug: 280047007 Test: manual and passed atest TrackpadReverseScrollingPreferenceControllerTest Change-Id: Ia5e30fa14b599ddcffae99005114f10412ccad3c --- ...dReverseScrollingPreferenceController.java | 4 +-- ...erseScrollingPreferenceControllerTest.java | 36 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceController.java b/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceController.java index 0bbfb98ebeb..10d30139b6c 100644 --- a/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceController.java +++ b/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceController.java @@ -30,12 +30,12 @@ public class TrackpadReverseScrollingPreferenceController extends TogglePreferen @Override public boolean isChecked() { - return InputSettings.useTouchpadNaturalScrolling(mContext); + return !InputSettings.useTouchpadNaturalScrolling(mContext); } @Override public boolean setChecked(boolean isChecked) { - InputSettings.setTouchpadNaturalScrolling(mContext, isChecked); + InputSettings.setTouchpadNaturalScrolling(mContext, !isChecked); return true; } diff --git a/tests/robotests/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceControllerTest.java index b4cb862e6c6..a99abb8d38f 100644 --- a/tests/robotests/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/inputmethod/TrackpadReverseScrollingPreferenceControllerTest.java @@ -61,22 +61,9 @@ public class TrackpadReverseScrollingPreferenceControllerTest { } @Test - public void setChecked_true_shouldReturn1() { + public void setChecked_true_shouldReturn0() { mController.setChecked(true); - int result = Settings.System.getIntForUser( - mContext.getContentResolver(), - SETTING_KEY, - 0, - UserHandle.USER_CURRENT); - - assertThat(result).isEqualTo(1); - } - - @Test - public void setChecked_false_shouldReturn0() { - mController.setChecked(false); - int result = Settings.System.getIntForUser( mContext.getContentResolver(), SETTING_KEY, @@ -87,7 +74,20 @@ public class TrackpadReverseScrollingPreferenceControllerTest { } @Test - public void isChecked_providerPutInt1_returnTrue() { + public void setChecked_false_shouldReturn1() { + mController.setChecked(false); + + int result = Settings.System.getIntForUser( + mContext.getContentResolver(), + SETTING_KEY, + 0, + UserHandle.USER_CURRENT); + + assertThat(result).isEqualTo(1); + } + + @Test + public void isChecked_providerPutInt1_returnFalse() { Settings.System.putIntForUser( mContext.getContentResolver(), SETTING_KEY, @@ -96,11 +96,11 @@ public class TrackpadReverseScrollingPreferenceControllerTest { boolean result = mController.isChecked(); - assertThat(result).isTrue(); + assertThat(result).isFalse(); } @Test - public void isChecked_providerPutInt0_returnFalse() { + public void isChecked_providerPutInt0_returnTrue() { Settings.System.putIntForUser( mContext.getContentResolver(), SETTING_KEY, @@ -109,6 +109,6 @@ public class TrackpadReverseScrollingPreferenceControllerTest { boolean result = mController.isChecked(); - assertThat(result).isFalse(); + assertThat(result).isTrue(); } } From c16a088a731fa0d25d96297fad8a48914a6b55df Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Wed, 3 May 2023 20:43:59 +0800 Subject: [PATCH 04/10] Log app special permission change for SPA Align with current logging and no package name. Bug: 280575002 Test: Manually with App Op Permission pages Change-Id: Ia0dde06769b1e08f8cec7cb1d87a8ca1baa80068 --- .../specialaccess/AlarmsAndRemindersAppList.kt | 13 +++++++++++++ .../spa/app/specialaccess/AllFilesAccess.kt | 16 ++++++++++++++++ .../app/specialaccess/DisplayOverOtherApps.kt | 16 ++++++++++++++++ .../app/specialaccess/MediaManagementApps.kt | 18 ++++++++++++++++++ .../app/specialaccess/ModifySystemSettings.kt | 16 ++++++++++++++++ 5 files changed, 79 insertions(+) diff --git a/src/com/android/settings/spa/app/specialaccess/AlarmsAndRemindersAppList.kt b/src/com/android/settings/spa/app/specialaccess/AlarmsAndRemindersAppList.kt index 9b363358fc8..527c6d9bb96 100644 --- a/src/com/android/settings/spa/app/specialaccess/AlarmsAndRemindersAppList.kt +++ b/src/com/android/settings/spa/app/specialaccess/AlarmsAndRemindersAppList.kt @@ -19,12 +19,14 @@ package com.android.settings.spa.app.specialaccess import android.Manifest import android.app.AlarmManager import android.app.compat.CompatChanges +import android.app.settings.SettingsEnums import android.content.Context import android.content.pm.ApplicationInfo import android.os.PowerExemptionManager import androidx.compose.runtime.Composable import androidx.compose.runtime.livedata.observeAsState import com.android.settings.R +import com.android.settings.overlay.FeatureFactory import com.android.settingslib.spa.framework.compose.stateOf import com.android.settingslib.spaprivileged.model.app.AppRecord import com.android.settingslib.spaprivileged.model.app.IPackageManagers @@ -85,6 +87,17 @@ class AlarmsAndRemindersAppListModel( override fun setAllowed(record: AlarmsAndRemindersAppRecord, newAllowed: Boolean) { record.controller.setAllowed(newAllowed) + logPermissionChange(newAllowed) + } + + private fun logPermissionChange(newAllowed: Boolean) { + FeatureFactory.getFactory(context).metricsFeatureProvider.action( + SettingsEnums.PAGE_UNKNOWN, + SettingsEnums.ACTION_ALARMS_AND_REMINDERS_TOGGLE, + SettingsEnums.ALARMS_AND_REMINDERS, + "", + if (newAllowed) 1 else 0 + ) } private fun createRecord( diff --git a/src/com/android/settings/spa/app/specialaccess/AllFilesAccess.kt b/src/com/android/settings/spa/app/specialaccess/AllFilesAccess.kt index 6466e038006..16520fad635 100644 --- a/src/com/android/settings/spa/app/specialaccess/AllFilesAccess.kt +++ b/src/com/android/settings/spa/app/specialaccess/AllFilesAccess.kt @@ -18,9 +18,12 @@ package com.android.settings.spa.app.specialaccess import android.Manifest import android.app.AppOpsManager +import android.app.settings.SettingsEnums import android.content.Context import com.android.settings.R +import com.android.settings.overlay.FeatureFactory import com.android.settingslib.spaprivileged.template.app.AppOpPermissionListModel +import com.android.settingslib.spaprivileged.template.app.AppOpPermissionRecord import com.android.settingslib.spaprivileged.template.app.TogglePermissionAppListProvider object AllFilesAccessAppListProvider : TogglePermissionAppListProvider { @@ -35,4 +38,17 @@ class AllFilesAccessListModel(context: Context) : AppOpPermissionListModel(conte override val appOp = AppOpsManager.OP_MANAGE_EXTERNAL_STORAGE override val permission = Manifest.permission.MANAGE_EXTERNAL_STORAGE override val setModeByUid = true + + override fun setAllowed(record: AppOpPermissionRecord, newAllowed: Boolean) { + super.setAllowed(record, newAllowed) + logPermissionChange(newAllowed) + } + + private fun logPermissionChange(newAllowed: Boolean) { + val category = when { + newAllowed -> SettingsEnums.APP_SPECIAL_PERMISSION_MANAGE_EXT_STRG_ALLOW + else -> SettingsEnums.APP_SPECIAL_PERMISSION_MANAGE_EXT_STRG_DENY + } + FeatureFactory.getFactory(context).metricsFeatureProvider.action(context, category, "") + } } diff --git a/src/com/android/settings/spa/app/specialaccess/DisplayOverOtherApps.kt b/src/com/android/settings/spa/app/specialaccess/DisplayOverOtherApps.kt index d3cd2b506e9..7812675dbe9 100644 --- a/src/com/android/settings/spa/app/specialaccess/DisplayOverOtherApps.kt +++ b/src/com/android/settings/spa/app/specialaccess/DisplayOverOtherApps.kt @@ -18,9 +18,12 @@ package com.android.settings.spa.app.specialaccess import android.Manifest import android.app.AppOpsManager +import android.app.settings.SettingsEnums import android.content.Context import com.android.settings.R +import com.android.settings.overlay.FeatureFactory import com.android.settingslib.spaprivileged.template.app.AppOpPermissionListModel +import com.android.settingslib.spaprivileged.template.app.AppOpPermissionRecord import com.android.settingslib.spaprivileged.template.app.TogglePermissionAppListProvider object DisplayOverOtherAppsAppListProvider : TogglePermissionAppListProvider { @@ -34,4 +37,17 @@ class DisplayOverOtherAppsListModel(context: Context) : AppOpPermissionListModel override val footerResId = R.string.allow_overlay_description override val appOp = AppOpsManager.OP_SYSTEM_ALERT_WINDOW override val permission = Manifest.permission.SYSTEM_ALERT_WINDOW + + override fun setAllowed(record: AppOpPermissionRecord, newAllowed: Boolean) { + super.setAllowed(record, newAllowed) + logPermissionChange(newAllowed) + } + + private fun logPermissionChange(newAllowed: Boolean) { + val category = when { + newAllowed -> SettingsEnums.APP_SPECIAL_PERMISSION_APPDRAW_ALLOW + else -> SettingsEnums.APP_SPECIAL_PERMISSION_APPDRAW_DENY + } + FeatureFactory.getFactory(context).metricsFeatureProvider.action(context, category, "") + } } diff --git a/src/com/android/settings/spa/app/specialaccess/MediaManagementApps.kt b/src/com/android/settings/spa/app/specialaccess/MediaManagementApps.kt index 6c7678a0629..e8935e64819 100644 --- a/src/com/android/settings/spa/app/specialaccess/MediaManagementApps.kt +++ b/src/com/android/settings/spa/app/specialaccess/MediaManagementApps.kt @@ -18,9 +18,12 @@ package com.android.settings.spa.app.specialaccess import android.Manifest import android.app.AppOpsManager +import android.app.settings.SettingsEnums import android.content.Context import com.android.settings.R +import com.android.settings.overlay.FeatureFactory import com.android.settingslib.spaprivileged.template.app.AppOpPermissionListModel +import com.android.settingslib.spaprivileged.template.app.AppOpPermissionRecord import com.android.settingslib.spaprivileged.template.app.TogglePermissionAppListProvider object MediaManagementAppsAppListProvider : TogglePermissionAppListProvider { @@ -35,4 +38,19 @@ class MediaManagementAppsListModel(context: Context) : AppOpPermissionListModel( override val appOp = AppOpsManager.OP_MANAGE_MEDIA override val permission = Manifest.permission.MANAGE_MEDIA override val setModeByUid = true + + override fun setAllowed(record: AppOpPermissionRecord, newAllowed: Boolean) { + super.setAllowed(record, newAllowed) + logPermissionChange(newAllowed) + } + + private fun logPermissionChange(newAllowed: Boolean) { + FeatureFactory.getFactory(context).metricsFeatureProvider.action( + SettingsEnums.PAGE_UNKNOWN, + SettingsEnums.ACTION_MEDIA_MANAGEMENT_APPS_TOGGLE, + SettingsEnums.MEDIA_MANAGEMENT_APPS, + "", + if (newAllowed) 1 else 0 + ) + } } \ No newline at end of file diff --git a/src/com/android/settings/spa/app/specialaccess/ModifySystemSettings.kt b/src/com/android/settings/spa/app/specialaccess/ModifySystemSettings.kt index 9a70871b1e2..668cc8cc129 100644 --- a/src/com/android/settings/spa/app/specialaccess/ModifySystemSettings.kt +++ b/src/com/android/settings/spa/app/specialaccess/ModifySystemSettings.kt @@ -18,9 +18,12 @@ package com.android.settings.spa.app.specialaccess import android.Manifest import android.app.AppOpsManager +import android.app.settings.SettingsEnums import android.content.Context import com.android.settings.R +import com.android.settings.overlay.FeatureFactory import com.android.settingslib.spaprivileged.template.app.AppOpPermissionListModel +import com.android.settingslib.spaprivileged.template.app.AppOpPermissionRecord import com.android.settingslib.spaprivileged.template.app.TogglePermissionAppListProvider object ModifySystemSettingsAppListProvider : TogglePermissionAppListProvider { @@ -34,4 +37,17 @@ class ModifySystemSettingsListModel(context: Context) : AppOpPermissionListModel override val footerResId = R.string.write_settings_description override val appOp = AppOpsManager.OP_WRITE_SETTINGS override val permission = Manifest.permission.WRITE_SETTINGS + + override fun setAllowed(record: AppOpPermissionRecord, newAllowed: Boolean) { + super.setAllowed(record, newAllowed) + logPermissionChange(newAllowed) + } + + private fun logPermissionChange(newAllowed: Boolean) { + val category = when { + newAllowed -> SettingsEnums.APP_SPECIAL_PERMISSION_SETTINGS_CHANGE_ALLOW + else -> SettingsEnums.APP_SPECIAL_PERMISSION_SETTINGS_CHANGE_DENY + } + FeatureFactory.getFactory(context).metricsFeatureProvider.action(context, category, "") + } } \ No newline at end of file From fb7c1766a87e1cd2add800129872b84f31733833 Mon Sep 17 00:00:00 2001 From: Vania Januar Date: Thu, 4 May 2023 11:57:48 +0100 Subject: [PATCH 05/10] Replace placeholder icons with stylus icons Bug: 250909304 Test: manual Change-Id: I738a262ea0e3f03408c892adb758b99dd6d47386 --- res/drawable/ic_stylus.xml | 26 +++++++++++++++++++ .../stylus/StylusDeviceUpdater.java | 3 +-- .../stylus/StylusUsiHeaderController.java | 3 +-- 3 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 res/drawable/ic_stylus.xml diff --git a/res/drawable/ic_stylus.xml b/res/drawable/ic_stylus.xml new file mode 100644 index 00000000000..eb52fec0418 --- /dev/null +++ b/res/drawable/ic_stylus.xml @@ -0,0 +1,26 @@ + + + + + \ No newline at end of file diff --git a/src/com/android/settings/connecteddevice/stylus/StylusDeviceUpdater.java b/src/com/android/settings/connecteddevice/stylus/StylusDeviceUpdater.java index da6e1789b0c..13b0a559e5e 100644 --- a/src/com/android/settings/connecteddevice/stylus/StylusDeviceUpdater.java +++ b/src/com/android/settings/connecteddevice/stylus/StylusDeviceUpdater.java @@ -160,8 +160,7 @@ public class StylusDeviceUpdater implements InputManager.InputDeviceListener, } mUsiPreference.setKey(PREF_KEY); mUsiPreference.setTitle(R.string.stylus_connected_devices_title); - // TODO(b/250909304): pending actual icon visD - mUsiPreference.setIcon(R.drawable.ic_edit); + mUsiPreference.setIcon(R.drawable.ic_stylus); mUsiPreference.setOnPreferenceClickListener((Preference p) -> { mMetricsFeatureProvider.logClickedPreference(p, mFragment.getMetricsCategory()); launchDeviceDetails(); diff --git a/src/com/android/settings/connecteddevice/stylus/StylusUsiHeaderController.java b/src/com/android/settings/connecteddevice/stylus/StylusUsiHeaderController.java index 379815b0442..23db3cb1052 100644 --- a/src/com/android/settings/connecteddevice/stylus/StylusUsiHeaderController.java +++ b/src/com/android/settings/connecteddevice/stylus/StylusUsiHeaderController.java @@ -70,8 +70,7 @@ public class StylusUsiHeaderController extends BasePreferenceController implemen ImageView iconView = mHeaderPreference.findViewById(R.id.entity_header_icon); if (iconView != null) { - // TODO(b/250909304): get proper icon once VisD ready - iconView.setImageResource(R.drawable.ic_edit); + iconView.setImageResource(R.drawable.ic_stylus); iconView.setContentDescription("Icon for stylus"); } refresh(); From 645ba04085c80c5407c788361efd72e14bb6c5e3 Mon Sep 17 00:00:00 2001 From: Jerry Shi Date: Wed, 3 May 2023 14:16:58 -0700 Subject: [PATCH 06/10] Modify the usage of setEnabledProvider to temporarily passed in empty list. Test: local test Bug: 280492574 Change-Id: Id13aea203636527bcda745727aae417b2a8e289c --- .../credentials/CredentialManagerPreferenceController.java | 1 + .../settings/applications/credentials/DefaultCombinedPicker.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java index 069336e7000..00dadc058d9 100644 --- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java +++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java @@ -562,6 +562,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl List enabledServices = getEnabledSettings(); mCredentialManager.setEnabledProviders( + new ArrayList(), // TODO(240466271): pass down primary providers enabledServices, getUser(), mExecutor, diff --git a/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java b/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java index cfaf7a211db..2ee4f47df4f 100644 --- a/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java +++ b/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java @@ -356,6 +356,7 @@ public class DefaultCombinedPicker extends DefaultAppPickerFragment { } service.setEnabledProviders( + new ArrayList(), // TODO(240466271): pass down primary providers. credManProviders, mUserId, ContextCompat.getMainExecutor(getContext()), From 3096997718495ae16cfa99847e44eff91e2d7387 Mon Sep 17 00:00:00 2001 From: Becca Hughes Date: Wed, 3 May 2023 19:28:40 +0000 Subject: [PATCH 07/10] Use isPrimary bit to determine top provider If the cred man provider has the isPrimary bit set then we should use it as top provider. Test: ondevice Bug: 280454916 Change-Id: I8c5651909d3926f09549c64af68185f1ef633198 --- .../credentials/CombinedProviderInfo.java | 24 ++++++++++++------- ...CredentialManagerPreferenceController.java | 17 +++++++++---- .../credentials/DefaultCombinedPicker.java | 22 +++++++++++++---- .../DefaultCombinedPreferenceController.java | 13 +++++----- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/com/android/settings/applications/credentials/CombinedProviderInfo.java b/src/com/android/settings/applications/credentials/CombinedProviderInfo.java index af06a0144b9..0446c209fc6 100644 --- a/src/com/android/settings/applications/credentials/CombinedProviderInfo.java +++ b/src/com/android/settings/applications/credentials/CombinedProviderInfo.java @@ -43,18 +43,18 @@ public final class CombinedProviderInfo { private final List mCredentialProviderInfos; private final @Nullable AutofillServiceInfo mAutofillServiceInfo; private final boolean mIsDefaultAutofillProvider; - private final boolean mIsDefaultCredmanProvider; + private final boolean mIsPrimaryCredmanProvider; /** Constructs an information instance from both autofill and credential provider. */ public CombinedProviderInfo( @Nullable List cpis, @Nullable AutofillServiceInfo asi, boolean isDefaultAutofillProvider, - boolean isDefaultCredmanProvider) { + boolean IsPrimaryCredmanProvider) { mCredentialProviderInfos = new ArrayList<>(cpis); mAutofillServiceInfo = asi; mIsDefaultAutofillProvider = isDefaultAutofillProvider; - mIsDefaultCredmanProvider = isDefaultCredmanProvider; + mIsPrimaryCredmanProvider = IsPrimaryCredmanProvider; } /** Returns the credential provider info. */ @@ -149,8 +149,8 @@ public final class CombinedProviderInfo { } /** Returns whether the provider is the default credman provider. */ - public boolean isDefaultCredmanProvider() { - return mIsDefaultCredmanProvider; + public boolean isPrimaryCredmanProvider() { + return mIsPrimaryCredmanProvider; } /** Returns the settings subtitle. */ @@ -192,7 +192,13 @@ public final class CombinedProviderInfo { } } - // TODO(280454916): Add logic here. + // If there is a primary cred man provider then return that. + for (CombinedProviderInfo cpi : providers) { + if (cpi.isPrimaryCredmanProvider()) { + return cpi; + } + } + return null; } @@ -250,14 +256,14 @@ public final class CombinedProviderInfo { } // Check if we have any enabled cred man services. - boolean isDefaultCredmanProvider = false; + boolean isPrimaryCredmanProvider = false; if (!cpi.isEmpty()) { - isDefaultCredmanProvider = cpi.get(0).isEnabled(); + isPrimaryCredmanProvider = cpi.get(0).isPrimary(); } cmpi.add( new CombinedProviderInfo( - cpi, selectedAsi, isDefaultAutofillProvider, isDefaultCredmanProvider)); + cpi, selectedAsi, isDefaultAutofillProvider, isPrimaryCredmanProvider)); } return cmpi; diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java index 00dadc058d9..bd7c485743d 100644 --- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java +++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java @@ -321,7 +321,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl mEnabledPackageNames.clear(); for (CredentialProviderInfo cpi : availableServices) { - if (cpi.isEnabled()) { + if (cpi.isEnabled() && !cpi.isPrimary()) { mEnabledPackageNames.add(cpi.getServiceInfo().packageName); } } @@ -560,16 +560,25 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl return; } - List enabledServices = getEnabledSettings(); + // Get the existing primary providers since we don't touch them in + // this part of the UI we should just copy them over. + Set primaryServices = new HashSet<>(); + for (CredentialProviderInfo service : mServices) { + if (service.isPrimary()) { + primaryServices.add(service.getServiceInfo().getComponentName().flattenToString()); + } + } + mCredentialManager.setEnabledProviders( - new ArrayList(), // TODO(240466271): pass down primary providers - enabledServices, + new ArrayList<>(primaryServices), + getEnabledSettings(), getUser(), mExecutor, new OutcomeReceiver() { @Override public void onResult(Void result) { Log.i(TAG, "setEnabledProviders success"); + updateFromExternal(); } @Override diff --git a/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java b/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java index 2ee4f47df4f..793aa3c1cbe 100644 --- a/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java +++ b/src/com/android/settings/applications/credentials/DefaultCombinedPicker.java @@ -47,7 +47,9 @@ import com.android.settingslib.utils.ThreadUtils; import com.android.settingslib.widget.CandidateInfo; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; public class DefaultCombinedPicker extends DefaultAppPickerFragment { @@ -338,9 +340,9 @@ public class DefaultCombinedPicker extends DefaultAppPickerFragment { return true; } - private void setProviders(String autofillProvider, List credManProviders) { + private void setProviders(String autofillProvider, List primaryCredManProviders) { if (TextUtils.isEmpty(autofillProvider)) { - if (credManProviders.size() > 0) { + if (primaryCredManProviders.size() > 0) { autofillProvider = CredentialManagerPreferenceController .AUTOFILL_CREDMAN_ONLY_PROVIDER_PLACEHOLDER; @@ -350,13 +352,25 @@ public class DefaultCombinedPicker extends DefaultAppPickerFragment { Settings.Secure.putStringForUser( getContext().getContentResolver(), AUTOFILL_SETTING, autofillProvider, mUserId); - CredentialManager service = getCredentialProviderService(); + final CredentialManager service = getCredentialProviderService(); if (service == null) { return; } + // Get the existing secondary providers since we don't touch them in + // this part of the UI we should just copy them over. + final List credManProviders = new ArrayList<>(); + for (CredentialProviderInfo cpi : + service.getCredentialProviderServices( + mUserId, CredentialManager.PROVIDER_FILTER_USER_PROVIDERS_ONLY)) { + + if (cpi.isEnabled()) { + credManProviders.add(cpi.getServiceInfo().getComponentName().flattenToString()); + } + } + service.setEnabledProviders( - new ArrayList(), // TODO(240466271): pass down primary providers. + primaryCredManProviders, credManProviders, mUserId, ContextCompat.getMainExecutor(getContext()), diff --git a/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java b/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java index ca049bcdd48..64d4f0dd5ed 100644 --- a/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java +++ b/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java @@ -135,12 +135,12 @@ public class DefaultCombinedPreferenceController extends DefaultAppPreferenceCon /** Provides Intent to setting activity for the specified autofill service. */ static final class AutofillSettingIntentProvider { - private final String mSelectedKey; + private final String mKey; private final Context mContext; private final int mUserId; public AutofillSettingIntentProvider(Context context, int userId, String key) { - mSelectedKey = key; + mKey = key; mContext = context; mUserId = userId; } @@ -153,10 +153,9 @@ public class DefaultCombinedPreferenceController extends DefaultAppPreferenceCon for (ResolveInfo resolveInfo : resolveInfos) { final ServiceInfo serviceInfo = resolveInfo.serviceInfo; - final String flattenKey = - new ComponentName(serviceInfo.packageName, serviceInfo.name) - .flattenToString(); - if (TextUtils.equals(mSelectedKey, flattenKey)) { + + // If there are multiple autofill services then pick the first one. + if (mKey.startsWith(serviceInfo.packageName)) { final String settingsActivity; try { settingsActivity = @@ -164,7 +163,7 @@ public class DefaultCombinedPreferenceController extends DefaultAppPreferenceCon .getSettingsActivity(); } catch (SecurityException e) { // Service does not declare the proper permission, ignore it. - Log.w(TAG, "Error getting info for " + serviceInfo + ": " + e); + Log.e(TAG, "Error getting info for " + serviceInfo + ": " + e); return null; } if (TextUtils.isEmpty(settingsActivity)) { From 39bf7dd5307cafa81b4e83d75bfe86295e30d666 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Thu, 4 May 2023 19:26:44 +0800 Subject: [PATCH 08/10] Fix dialog leak in RequestPermissionActivity Dialog still show when activity destroyed will cause leak. Dismiss dialog when activity onDestroy to fix this issue. Fix: 279522922 Test: Manually with "Don't keep activities" Test: Robolectric Test Change-Id: I445f4b160020823a6f6e2883055218c1224e2c48 --- .../bluetooth/RequestPermissionActivity.java | 89 ++++++++------- .../bluetooth/RequestPermissionHelper.kt | 16 +-- .../RequestPermissionActivityTest.kt | 102 ++++++++++++++++++ .../bluetooth/RequestPermissionHelperTest.kt | 24 +++-- 4 files changed, 175 insertions(+), 56 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/bluetooth/RequestPermissionActivityTest.kt diff --git a/src/com/android/settings/bluetooth/RequestPermissionActivity.java b/src/com/android/settings/bluetooth/RequestPermissionActivity.java index 620cfb0586d..32ca2777392 100644 --- a/src/com/android/settings/bluetooth/RequestPermissionActivity.java +++ b/src/com/android/settings/bluetooth/RequestPermissionActivity.java @@ -72,6 +72,7 @@ public class RequestPermissionActivity extends Activity implements private int mRequest; private AlertDialog mDialog; + private AlertDialog mRequestDialog; private BroadcastReceiver mReceiver; @@ -96,33 +97,35 @@ public class RequestPermissionActivity extends Activity implements if (mRequest == REQUEST_DISABLE) { switch (btState) { case BluetoothAdapter.STATE_OFF: - case BluetoothAdapter.STATE_TURNING_OFF: { + case BluetoothAdapter.STATE_TURNING_OFF: proceedAndFinish(); - } break; - + break; case BluetoothAdapter.STATE_ON: - case BluetoothAdapter.STATE_TURNING_ON: { - RequestPermissionHelper.INSTANCE.requestDisable(this, mAppLabel, - () -> { - onDisableConfirmed(); - return Unit.INSTANCE; - }, - () -> { - cancelAndFinish(); - return Unit.INSTANCE; - }); - } break; - - default: { + case BluetoothAdapter.STATE_TURNING_ON: + mRequestDialog = + RequestPermissionHelper.INSTANCE.requestDisable(this, mAppLabel, + () -> { + onDisableConfirmed(); + return Unit.INSTANCE; + }, + () -> { + cancelAndFinish(); + return Unit.INSTANCE; + }); + if (mRequestDialog != null) { + mRequestDialog.show(); + } + break; + default: Log.e(TAG, "Unknown adapter state: " + btState); cancelAndFinish(); - } break; + break; } } else { switch (btState) { case BluetoothAdapter.STATE_OFF: case BluetoothAdapter.STATE_TURNING_OFF: - case BluetoothAdapter.STATE_TURNING_ON: { + case BluetoothAdapter.STATE_TURNING_ON: /* * Strictly speaking STATE_TURNING_ON belong with STATE_ON; * however, BT may not be ready when the user clicks yes and we @@ -131,20 +134,23 @@ public class RequestPermissionActivity extends Activity implements * case via the broadcast receiver. */ - // Start the helper activity to ask the user about enabling bt AND discovery - RequestPermissionHelper.INSTANCE.requestEnable(this, mAppLabel, - mRequest == REQUEST_ENABLE_DISCOVERABLE ? mTimeout : -1, - () -> { - onEnableConfirmed(); - return Unit.INSTANCE; - }, - () -> { - cancelAndFinish(); - return Unit.INSTANCE; - }); - } break; - - case BluetoothAdapter.STATE_ON: { + // Show the helper dialog to ask the user about enabling bt AND discovery + mRequestDialog = + RequestPermissionHelper.INSTANCE.requestEnable(this, mAppLabel, + mRequest == REQUEST_ENABLE_DISCOVERABLE ? mTimeout : -1, + () -> { + onEnableConfirmed(); + return Unit.INSTANCE; + }, + () -> { + cancelAndFinish(); + return Unit.INSTANCE; + }); + if (mRequestDialog != null) { + mRequestDialog.show(); + } + break; + case BluetoothAdapter.STATE_ON: if (mRequest == REQUEST_ENABLE) { // Nothing to do. Already enabled. proceedAndFinish(); @@ -152,12 +158,11 @@ public class RequestPermissionActivity extends Activity implements // Ask the user about enabling discovery mode createDialog(); } - } break; - - default: { + break; + default: Log.e(TAG, "Unknown adapter state: " + btState); cancelAndFinish(); - } break; + break; } } } @@ -275,10 +280,6 @@ public class RequestPermissionActivity extends Activity implements } } - if (mDialog != null) { - mDialog.dismiss(); - } - setResult(returnCode); finish(); } @@ -365,6 +366,14 @@ public class RequestPermissionActivity extends Activity implements unregisterReceiver(mReceiver); mReceiver = null; } + if (mDialog != null && mDialog.isShowing()) { + mDialog.dismiss(); + mDialog = null; + } + if (mRequestDialog != null && mRequestDialog.isShowing()) { + mRequestDialog.dismiss(); + mRequestDialog = null; + } } @Override diff --git a/src/com/android/settings/bluetooth/RequestPermissionHelper.kt b/src/com/android/settings/bluetooth/RequestPermissionHelper.kt index 000a7d16295..73084e4371d 100644 --- a/src/com/android/settings/bluetooth/RequestPermissionHelper.kt +++ b/src/com/android/settings/bluetooth/RequestPermissionHelper.kt @@ -30,20 +30,20 @@ object RequestPermissionHelper { timeout: Int, onAllow: () -> Unit, onDeny: () -> Unit, - ) { + ): AlertDialog? { if (context.resources.getBoolean(R.bool.auto_confirm_bluetooth_activation_dialog)) { // Don't even show the dialog if configured this way onAllow() - return + return null } - AlertDialog.Builder(context).apply { + return AlertDialog.Builder(context).apply { setMessage(context.getEnableMessage(timeout, appLabel)) setPositiveButton(R.string.allow) { _, _ -> if (context.isDisallowBluetooth()) onDeny() else onAllow() } setNegativeButton(R.string.deny) { _, _ -> onDeny() } setOnCancelListener { onDeny() } - }.show() + }.create() } fun requestDisable( @@ -51,18 +51,18 @@ object RequestPermissionHelper { appLabel: CharSequence?, onAllow: () -> Unit, onDeny: () -> Unit, - ) { + ): AlertDialog? { if (context.resources.getBoolean(R.bool.auto_confirm_bluetooth_activation_dialog)) { // Don't even show the dialog if configured this way onAllow() - return + return null } - AlertDialog.Builder(context).apply { + return AlertDialog.Builder(context).apply { setMessage(context.getDisableMessage(appLabel)) setPositiveButton(R.string.allow) { _, _ -> onAllow() } setNegativeButton(R.string.deny) { _, _ -> onDeny() } setOnCancelListener { onDeny() } - }.show() + }.create() } } diff --git a/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionActivityTest.kt b/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionActivityTest.kt new file mode 100644 index 00000000000..13cbb350feb --- /dev/null +++ b/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionActivityTest.kt @@ -0,0 +1,102 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.bluetooth + +import android.bluetooth.BluetoothAdapter +import android.content.Intent +import com.android.settings.testutils.shadow.ShadowAlertDialogCompat +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.android.controller.ActivityController +import org.robolectric.annotation.Config +import org.robolectric.shadow.api.Shadow +import org.robolectric.shadows.ShadowBluetoothAdapter + +@RunWith(RobolectricTestRunner::class) +@Config(shadows = [ShadowAlertDialogCompat::class, ShadowBluetoothAdapter::class]) +class RequestPermissionActivityTest { + private lateinit var activityController: ActivityController + private lateinit var bluetoothAdapter: ShadowBluetoothAdapter + + @Before + fun setUp() { + bluetoothAdapter = Shadow.extract(BluetoothAdapter.getDefaultAdapter()) + } + + @After + fun tearDown() { + activityController.pause().stop().destroy() + ShadowAlertDialogCompat.reset() + } + + @Test + fun requestEnable_whenBluetoothIsOff_showConfirmDialog() { + bluetoothAdapter.setState(BluetoothAdapter.STATE_OFF) + + createActivity(action = BluetoothAdapter.ACTION_REQUEST_ENABLE) + + val dialog = ShadowAlertDialogCompat.getLatestAlertDialog() + val shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog) + assertThat(shadowDialog.message.toString()) + .isEqualTo("An app wants to turn on Bluetooth") + } + + @Test + fun requestEnable_whenBluetoothIsOn_doNothing() { + bluetoothAdapter.setState(BluetoothAdapter.STATE_ON) + + createActivity(action = BluetoothAdapter.ACTION_REQUEST_ENABLE) + + val dialog = ShadowAlertDialogCompat.getLatestAlertDialog() + assertThat(dialog).isNull() + } + + @Test + fun requestDisable_whenBluetoothIsOff_doNothing() { + bluetoothAdapter.setState(BluetoothAdapter.STATE_OFF) + + createActivity(action = BluetoothAdapter.ACTION_REQUEST_DISABLE) + + val dialog = ShadowAlertDialogCompat.getLatestAlertDialog() + assertThat(dialog).isNull() + } + + @Test + fun requestDisable_whenBluetoothIsOn_showConfirmDialog() { + bluetoothAdapter.setState(BluetoothAdapter.STATE_ON) + + createActivity(action = BluetoothAdapter.ACTION_REQUEST_DISABLE) + + val dialog = ShadowAlertDialogCompat.getLatestAlertDialog() + val shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog) + assertThat(shadowDialog.message.toString()) + .isEqualTo("An app wants to turn off Bluetooth") + } + + private fun createActivity(action: String) { + activityController = + ActivityController.of(RequestPermissionActivity(), Intent(action)).apply { + create() + start() + postCreate(null) + resume() + } + } +} \ No newline at end of file diff --git a/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionHelperTest.kt b/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionHelperTest.kt index ce0792c468a..e51a2f92919 100644 --- a/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionHelperTest.kt +++ b/tests/robotests/src/com/android/settings/bluetooth/RequestPermissionHelperTest.kt @@ -49,13 +49,14 @@ class RequestPermissionHelperTest { @Test fun requestEnable_withAppLabelAndNoTimeout_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestEnable( context = activity, appLabel = "App Label", timeout = -1, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs("App Label wants to turn on Bluetooth") } @@ -63,13 +64,14 @@ class RequestPermissionHelperTest { @Test fun requestEnable_withAppLabelAndZeroTimeout_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestEnable( context = activity, appLabel = "App Label", timeout = 0, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs( "App Label wants to turn on Bluetooth and make your phone visible to other devices. " + @@ -80,13 +82,14 @@ class RequestPermissionHelperTest { @Test fun requestEnable_withAppLabelAndNormalTimeout_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestEnable( context = activity, appLabel = "App Label", timeout = 120, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs( "App Label wants to turn on Bluetooth and make your phone visible to other devices " + @@ -97,13 +100,14 @@ class RequestPermissionHelperTest { @Test fun requestEnable_withNoAppLabelAndNoTimeout_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestEnable( context = activity, appLabel = null, timeout = -1, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs("An app wants to turn on Bluetooth") } @@ -111,13 +115,14 @@ class RequestPermissionHelperTest { @Test fun requestEnable_withNoAppLabelAndZeroTimeout_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestEnable( context = activity, appLabel = null, timeout = 0, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs( "An app wants to turn on Bluetooth and make your phone visible to other devices. " + @@ -128,13 +133,14 @@ class RequestPermissionHelperTest { @Test fun requestEnable_withNoAppLabelAndNormalTimeout_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestEnable( context = activity, appLabel = null, timeout = 120, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs( "An app wants to turn on Bluetooth and make your phone visible to other devices for " + @@ -177,12 +183,13 @@ class RequestPermissionHelperTest { @Test fun requestDisable_withAppLabel_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestDisable( context = activity, appLabel = "App Label", onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs("App Label wants to turn off Bluetooth") } @@ -190,12 +197,13 @@ class RequestPermissionHelperTest { @Test fun requestDisable_withNoAppLabel_hasCorrectMessage() { val activity = activityController.get() + RequestPermissionHelper.requestDisable( context = activity, appLabel = null, onAllow = {}, onDeny = {}, - ) + )!!.show() assertLatestMessageIs("An app wants to turn off Bluetooth") } From 8d167d55f36004249d36658e082ec4a6a346d91a Mon Sep 17 00:00:00 2001 From: Jerry Shi Date: Thu, 4 May 2023 22:53:36 -0700 Subject: [PATCH 09/10] Do null check to account for the case where provider is only autofill provider. Test: local build Bug: 281047738 Change-Id: I9f3dca118a0d4182e2d727fa011dcb48db2a553f --- .../applications/credentials/CombinedProviderInfo.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/com/android/settings/applications/credentials/CombinedProviderInfo.java b/src/com/android/settings/applications/credentials/CombinedProviderInfo.java index 0446c209fc6..89a13eff6c8 100644 --- a/src/com/android/settings/applications/credentials/CombinedProviderInfo.java +++ b/src/com/android/settings/applications/credentials/CombinedProviderInfo.java @@ -51,7 +51,11 @@ public final class CombinedProviderInfo { @Nullable AutofillServiceInfo asi, boolean isDefaultAutofillProvider, boolean IsPrimaryCredmanProvider) { - mCredentialProviderInfos = new ArrayList<>(cpis); + if (cpis == null) { + mCredentialProviderInfos = new ArrayList<>(); + } else { + mCredentialProviderInfos = new ArrayList<>(cpis); + } mAutofillServiceInfo = asi; mIsDefaultAutofillProvider = isDefaultAutofillProvider; mIsPrimaryCredmanProvider = IsPrimaryCredmanProvider; @@ -257,7 +261,7 @@ public final class CombinedProviderInfo { // Check if we have any enabled cred man services. boolean isPrimaryCredmanProvider = false; - if (!cpi.isEmpty()) { + if (cpi != null && !cpi.isEmpty()) { isPrimaryCredmanProvider = cpi.get(0).isPrimary(); } From 17a0266b1656c34a9d6a3077a0f30ea5234db859 Mon Sep 17 00:00:00 2001 From: Becca Hughes Date: Fri, 5 May 2023 18:27:21 +0000 Subject: [PATCH 10/10] Fix newly installed providers bug Package monitor was not registered properly. Change-Id: Ifde2e6027b2aee2355114c8ff9b0d1f542fa2f8d Test: treehugger Bug: 280867431 --- .../credentials/CredentialManagerPreferenceController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java index bd7c485743d..67a1fa8ebdd 100644 --- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java +++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java @@ -120,6 +120,7 @@ public class CredentialManagerPreferenceController extends BasePreferenceControl mCredentialManager = getCredentialManager(context, preferenceKey.equals("credentials_test")); new SettingContentObserver(mHandler).register(context.getContentResolver()); + mSettingsPackageMonitor.register(context, context.getMainLooper(), false); } private @Nullable CredentialManager getCredentialManager(Context context, boolean isTest) {