From 5aeef97caed227a0cb97fb84e0538f36db9469e5 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Wed, 12 Apr 2023 17:35:13 -0700 Subject: [PATCH] Handle corner cases in "Alarms and Reminders" page There are few corner cases and new updates that need to be incorporated: 1. Apps that declare USE_EXACT_ALARM, do not need SCHEDULE_EXACT_ALARM. So these should be filtered out, regardless of whather they declared the latter. 2. Apps that are in the power allowlist do not need either of the permission, and so these should be filtered out as well. In either case, if the user somehow ends up in the app detail page for this setting, the switch should get disabled based on existing logic. Test: make -j RunSettingsRoboTests Test: Manually by UI inspection: Settings -> Apps -> Special App access -> Alarms and Reminders or by running: adb shell am start -a android.settings.REQUEST_SCHEDULE_EXACT_ALARM Bug: 232460265 Change-Id: I5aeab49f95260218878bc36f5a4d73a49e5082e4 --- .../AppStateAlarmsAndRemindersBridge.java | 77 ++++--- .../AppStateAlarmsAndRemindersBridgeTest.java | 218 +++++++++++++----- 2 files changed, 217 insertions(+), 78 deletions(-) diff --git a/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridge.java b/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridge.java index ffe87678016..c96e8fbaeb8 100644 --- a/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridge.java +++ b/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridge.java @@ -18,11 +18,11 @@ package com.android.settings.applications; import android.Manifest; import android.app.AlarmManager; -import android.app.AppGlobals; import android.app.compat.CompatChanges; import android.content.Context; -import android.content.pm.IPackageManager; -import android.os.RemoteException; +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; +import android.os.PowerExemptionManager; import android.os.UserHandle; import android.util.Log; @@ -32,8 +32,6 @@ import com.android.settingslib.applications.ApplicationsState; import com.android.settingslib.applications.ApplicationsState.AppEntry; import com.android.settingslib.applications.ApplicationsState.AppFilter; -import libcore.util.EmptyArray; - import java.util.List; /** @@ -42,26 +40,24 @@ import java.util.List; * Also provides app filters that can use the info. */ public class AppStateAlarmsAndRemindersBridge extends AppStateBaseBridge { - private static final String PERMISSION = Manifest.permission.SCHEDULE_EXACT_ALARM; + private static final String SEA_PERMISSION = Manifest.permission.SCHEDULE_EXACT_ALARM; + private static final String UEA_PERMISSION = Manifest.permission.USE_EXACT_ALARM; private static final String TAG = "AlarmsAndRemindersBridge"; @VisibleForTesting AlarmManager mAlarmManager; @VisibleForTesting - String[] mRequesterPackages; + PowerExemptionManager mPowerExemptionManager; + @VisibleForTesting + PackageManager mPackageManager; public AppStateAlarmsAndRemindersBridge(Context context, ApplicationsState appState, Callback callback) { super(appState, callback); + mPowerExemptionManager = context.getSystemService(PowerExemptionManager.class); mAlarmManager = context.getSystemService(AlarmManager.class); - final IPackageManager iPm = AppGlobals.getPackageManager(); - try { - mRequesterPackages = iPm.getAppOpPermissionPackages(PERMISSION, context.getUserId()); - } catch (RemoteException re) { - Log.e(TAG, "Cannot reach package manager", re); - mRequesterPackages = EmptyArray.STRING; - } + mPackageManager = context.getPackageManager(); } private boolean isChangeEnabled(String packageName, int userId) { @@ -69,6 +65,22 @@ public class AppStateAlarmsAndRemindersBridge extends AppStateBaseBridge { packageName, UserHandle.of(userId)); } + private boolean isUeaChangeEnabled(String packageName, int userId) { + return CompatChanges.isChangeEnabled(AlarmManager.ENABLE_USE_EXACT_ALARM, packageName, + UserHandle.of(userId)); + } + + private String[] getRequestedPermissions(String packageName, int userId) { + try { + final PackageInfo info = mPackageManager.getPackageInfoAsUser(packageName, + PackageManager.GET_PERMISSIONS, userId); + return info.requestedPermissions; + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Could not find package " + packageName, e); + } + return null; + } + /** * Returns information regarding {@link Manifest.permission#SCHEDULE_EXACT_ALARM} for the given * package and uid. @@ -76,10 +88,17 @@ public class AppStateAlarmsAndRemindersBridge extends AppStateBaseBridge { public AlarmsAndRemindersState createPermissionState(String packageName, int uid) { final int userId = UserHandle.getUserId(uid); - final boolean permissionRequested = ArrayUtils.contains(mRequesterPackages, packageName) + final String[] requestedPermissions = getRequestedPermissions(packageName, userId); + + final boolean seaRequested = ArrayUtils.contains(requestedPermissions, SEA_PERMISSION) && isChangeEnabled(packageName, userId); - final boolean permissionGranted = mAlarmManager.hasScheduleExactAlarm(packageName, userId); - return new AlarmsAndRemindersState(permissionRequested, permissionGranted); + final boolean ueaRequested = ArrayUtils.contains(requestedPermissions, UEA_PERMISSION) + && isUeaChangeEnabled(packageName, userId); + + final boolean seaGranted = mAlarmManager.hasScheduleExactAlarm(packageName, userId); + final boolean allowListed = mPowerExemptionManager.isAllowListed(packageName, true); + + return new AlarmsAndRemindersState(seaRequested, ueaRequested, seaGranted, allowListed); } @Override @@ -113,26 +132,32 @@ public class AppStateAlarmsAndRemindersBridge extends AppStateBaseBridge { }; /** - * Class to denote the state of an app regarding - * {@link Manifest.permission#SCHEDULE_EXACT_ALARM}. + * Class to denote the state of an app regarding "Alarms and Reminders" permission. + * This permission state is a combination of {@link Manifest.permission#SCHEDULE_EXACT_ALARM}, + * {@link Manifest.permission#USE_EXACT_ALARM} and the power allowlist state. */ public static class AlarmsAndRemindersState { - private boolean mPermissionRequested; - private boolean mPermissionGranted; + private boolean mSeaPermissionRequested; + private boolean mUeaPermissionRequested; + private boolean mSeaPermissionGranted; + private boolean mAllowListed; - AlarmsAndRemindersState(boolean permissionRequested, boolean permissionGranted) { - mPermissionRequested = permissionRequested; - mPermissionGranted = permissionGranted; + AlarmsAndRemindersState(boolean seaPermissionRequested, boolean ueaPermissionRequested, + boolean seaPermissionGranted, boolean allowListed) { + mSeaPermissionRequested = seaPermissionRequested; + mUeaPermissionRequested = ueaPermissionRequested; + mSeaPermissionGranted = seaPermissionGranted; + mAllowListed = allowListed; } /** Should the app associated with this state appear on the Settings screen */ public boolean shouldBeVisible() { - return mPermissionRequested; + return mSeaPermissionRequested && !mUeaPermissionRequested && !mAllowListed; } /** Is the permission granted to the app associated with this state */ public boolean isAllowed() { - return mPermissionGranted; + return mSeaPermissionGranted || mUeaPermissionRequested || mAllowListed; } } } diff --git a/tests/unit/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridgeTest.java b/tests/unit/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridgeTest.java index f56da05eefc..cd38767035e 100644 --- a/tests/unit/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridgeTest.java +++ b/tests/unit/src/com/android/settings/applications/AppStateAlarmsAndRemindersBridgeTest.java @@ -16,15 +16,24 @@ package com.android.settings.applications; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import android.Manifest; import android.app.AlarmManager; import android.content.Context; +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; +import android.os.PowerExemptionManager; import android.os.UserHandle; import androidx.test.ext.junit.runners.AndroidJUnit4; +import libcore.util.EmptyArray; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,13 +43,15 @@ import org.mockito.MockitoAnnotations; @RunWith(AndroidJUnit4.class) public class AppStateAlarmsAndRemindersBridgeTest { - private static final String TEST_PACKAGE_1 = "com.example.test.1"; - private static final String TEST_PACKAGE_2 = "com.example.test.2"; - private static final int UID_1 = 12345; - private static final int UID_2 = 7654321; + private static final String TEST_PACKAGE = "com.example.test.1"; + private static final int TEST_UID = 12345; @Mock private AlarmManager mAlarmManager; + @Mock + private PowerExemptionManager mPowerExemptionManager; + @Mock + private PackageManager mPackageManager; @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Context mContext; @@ -50,65 +61,168 @@ public class AppStateAlarmsAndRemindersBridgeTest { } @Test - public void shouldBeVisible_permissionRequestedIsTrue_isTrue() { - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - true /* permissionRequested */, - true /* permissionGranted */) - .shouldBeVisible()).isTrue(); - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - true /* permissionRequested */, - false /* permissionGranted */) - .shouldBeVisible()).isTrue(); - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - false /* permissionRequested */, - true /* permissionGranted */) - .shouldBeVisible()).isFalse(); - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - false /* permissionRequested */, - false /* permissionGranted */) - .shouldBeVisible()).isFalse(); + public void alarmsAndRemindersState_shouldBeVisible() { + boolean seaPermissionRequested; + boolean ueaPermissionRequested; + boolean seaPermissionGranted; + boolean allowListed; + + for (int i = 0; i < (1 << 4); i++) { + seaPermissionRequested = (i & 1) != 0; + ueaPermissionRequested = (i & (1 << 1)) != 0; + seaPermissionGranted = (i & (1 << 2)) != 0; + allowListed = (i & (1 << 3)) != 0; + + final boolean visible = new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( + seaPermissionRequested, + ueaPermissionRequested, + seaPermissionGranted, + allowListed).shouldBeVisible(); + + assertWithMessage("Wrong return value " + visible + + " for {seaPermissionRequested = " + seaPermissionRequested + + ", ueaPermissionRequested = " + ueaPermissionRequested + + ", seaPermissionGranted = " + seaPermissionGranted + + ", allowListed = " + allowListed + "}") + .that(visible) + .isEqualTo(seaPermissionRequested && !ueaPermissionRequested && !allowListed); + } } @Test - public void isAllowed_permissionGrantedIsTrue_isTrue() { - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - true /* permissionRequested */, - true /* permissionGranted */) - .isAllowed()).isTrue(); - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - true /* permissionRequested */, - false /* permissionGranted */) - .isAllowed()).isFalse(); - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - false /* permissionRequested */, - true /* permissionGranted */) - .isAllowed()).isTrue(); - assertThat(new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( - false /* permissionRequested */, - false /* permissionGranted */) - .isAllowed()).isFalse(); + public void alarmsAndRemindersState_isAllowed() { + boolean seaPermissionRequested; + boolean ueaPermissionRequested; + boolean seaPermissionGranted; + boolean allowListed; + + for (int i = 0; i < (1 << 4); i++) { + seaPermissionRequested = (i & 1) != 0; + ueaPermissionRequested = (i & (1 << 1)) != 0; + seaPermissionGranted = (i & (1 << 2)) != 0; + allowListed = (i & (1 << 3)) != 0; + + final boolean allowed = new AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState( + seaPermissionRequested, + ueaPermissionRequested, + seaPermissionGranted, + allowListed).isAllowed(); + + assertWithMessage("Wrong return value " + allowed + + " for {seaPermissionRequested = " + seaPermissionRequested + + ", ueaPermissionRequested = " + ueaPermissionRequested + + ", seaPermissionGranted = " + seaPermissionGranted + + ", allowListed = " + allowListed + "}") + .that(allowed) + .isEqualTo(seaPermissionGranted || ueaPermissionRequested || allowListed); + } + } + + private PackageInfo createPackageInfoWithPermissions(String... requestedPermissions) { + final PackageInfo info = new PackageInfo(); + info.requestedPermissions = requestedPermissions; + return info; } @Test - public void createPermissionState() { + public void createPermissionState_SeaGrantedNoUeaNoAllowlist() throws Exception { AppStateAlarmsAndRemindersBridge bridge = new AppStateAlarmsAndRemindersBridge(mContext, null, null); bridge.mAlarmManager = mAlarmManager; - bridge.mRequesterPackages = new String[]{TEST_PACKAGE_1, "some.other.package"}; + bridge.mPackageManager = mPackageManager; + bridge.mPowerExemptionManager = mPowerExemptionManager; - doReturn(false).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE_1, - UserHandle.getUserId(UID_1)); - doReturn(true).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE_2, - UserHandle.getUserId(UID_2)); + doReturn(true).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE, + UserHandle.getUserId(TEST_UID)); + doReturn(createPackageInfoWithPermissions(Manifest.permission.SCHEDULE_EXACT_ALARM)) + .when(mPackageManager).getPackageInfoAsUser(eq(TEST_PACKAGE), anyInt(), anyInt()); + doReturn(false).when(mPowerExemptionManager).isAllowListed(TEST_PACKAGE, true); - AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state1 = - bridge.createPermissionState(TEST_PACKAGE_1, UID_1); - assertThat(state1.shouldBeVisible()).isTrue(); - assertThat(state1.isAllowed()).isFalse(); + AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state = + bridge.createPermissionState(TEST_PACKAGE, TEST_UID); + assertThat(state.shouldBeVisible()).isTrue(); + assertThat(state.isAllowed()).isTrue(); + } - AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state2 = - bridge.createPermissionState(TEST_PACKAGE_2, UID_2); - assertThat(state2.shouldBeVisible()).isFalse(); - assertThat(state2.isAllowed()).isTrue(); + @Test + public void createPermissionState_requestsBothSeaDeniedNoAllowlist() throws Exception { + AppStateAlarmsAndRemindersBridge bridge = new AppStateAlarmsAndRemindersBridge(mContext, + null, null); + bridge.mAlarmManager = mAlarmManager; + bridge.mPackageManager = mPackageManager; + bridge.mPowerExemptionManager = mPowerExemptionManager; + + doReturn(false).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE, + UserHandle.getUserId(TEST_UID)); + doReturn(createPackageInfoWithPermissions( + Manifest.permission.SCHEDULE_EXACT_ALARM, + Manifest.permission.USE_EXACT_ALARM)) + .when(mPackageManager).getPackageInfoAsUser(eq(TEST_PACKAGE), anyInt(), anyInt()); + doReturn(false).when(mPowerExemptionManager).isAllowListed(TEST_PACKAGE, true); + + AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state = + bridge.createPermissionState(TEST_PACKAGE, TEST_UID); + assertThat(state.shouldBeVisible()).isFalse(); + assertThat(state.isAllowed()).isTrue(); + } + + @Test + public void createPermissionState_requestsNoneNoAllowlist() throws Exception { + AppStateAlarmsAndRemindersBridge bridge = new AppStateAlarmsAndRemindersBridge(mContext, + null, null); + bridge.mAlarmManager = mAlarmManager; + bridge.mPackageManager = mPackageManager; + bridge.mPowerExemptionManager = mPowerExemptionManager; + + doReturn(false).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE, + UserHandle.getUserId(TEST_UID)); + doReturn(createPackageInfoWithPermissions(EmptyArray.STRING)) + .when(mPackageManager).getPackageInfoAsUser(eq(TEST_PACKAGE), anyInt(), anyInt()); + doReturn(false).when(mPowerExemptionManager).isAllowListed(TEST_PACKAGE, true); + + AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state = + bridge.createPermissionState(TEST_PACKAGE, TEST_UID); + assertThat(state.shouldBeVisible()).isFalse(); + assertThat(state.isAllowed()).isFalse(); + } + + @Test + public void createPermissionState_requestsOnlyUeaNoAllowlist() throws Exception { + AppStateAlarmsAndRemindersBridge bridge = new AppStateAlarmsAndRemindersBridge(mContext, + null, null); + bridge.mAlarmManager = mAlarmManager; + bridge.mPackageManager = mPackageManager; + bridge.mPowerExemptionManager = mPowerExemptionManager; + + doReturn(false).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE, + UserHandle.getUserId(TEST_UID)); + doReturn(createPackageInfoWithPermissions(Manifest.permission.USE_EXACT_ALARM)) + .when(mPackageManager).getPackageInfoAsUser(eq(TEST_PACKAGE), anyInt(), anyInt()); + doReturn(false).when(mPowerExemptionManager).isAllowListed(TEST_PACKAGE, true); + + AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state = + bridge.createPermissionState(TEST_PACKAGE, TEST_UID); + assertThat(state.shouldBeVisible()).isFalse(); + assertThat(state.isAllowed()).isTrue(); + } + + @Test + public void createPermissionState_requestsNoneButAllowlisted() throws Exception { + AppStateAlarmsAndRemindersBridge bridge = new AppStateAlarmsAndRemindersBridge(mContext, + null, null); + bridge.mAlarmManager = mAlarmManager; + bridge.mPackageManager = mPackageManager; + bridge.mPowerExemptionManager = mPowerExemptionManager; + + doReturn(false).when(mAlarmManager).hasScheduleExactAlarm(TEST_PACKAGE, + UserHandle.getUserId(TEST_UID)); + doReturn(createPackageInfoWithPermissions(EmptyArray.STRING)) + .when(mPackageManager).getPackageInfoAsUser(eq(TEST_PACKAGE), anyInt(), anyInt()); + doReturn(true).when(mPowerExemptionManager).isAllowListed(TEST_PACKAGE, true); + + AppStateAlarmsAndRemindersBridge.AlarmsAndRemindersState state = + bridge.createPermissionState(TEST_PACKAGE, TEST_UID); + assertThat(state.shouldBeVisible()).isFalse(); + assertThat(state.isAllowed()).isTrue(); } }