Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4fe5eb56 authored by Joe Castro's avatar Joe Castro
Browse files

More judicious permission-change handling

There is a steady report of ANRs from AccountManagerService when it's processing changes to PermissionState. The optimistic canceling of a possible notification seems like it's backlogging the NotificationService messages operation queue. The specific AppOps/Permissions that AccountManager cares about are pretty narrow and not terribly volatile, while other kinds of changes can be *very* chatty.

This changes things to just use the AppOps listener (which allows for listening to just the specific AppOps) and uses changes to the three related settings as a trigger to revoke the notifications. This still could be reduced more if necessary/appropriate. Like by keeping track of the IDs of posted notifications at this layer and then only trying to cancel things that have been posted. But this change keeps most of the original logic as a way to minimize risk of regressions.

Bug: b/322509266
Flag: EXEMPT bugfix
Test: presubmit
Change-Id: I373aa33657dc7e87da9bd12406bcfc9634dc99dc
parent fdd6b883
Loading
Loading
Loading
Loading
+41 −62
Original line number Diff line number Diff line
@@ -185,6 +185,12 @@ public class AccountManagerService

    final Context mContext;

    private static final int[] INTERESTING_APP_OPS = new int[] {
        AppOpsManager.OP_GET_ACCOUNTS,
        AppOpsManager.OP_READ_CONTACTS,
        AppOpsManager.OP_WRITE_CONTACTS,
    };

    private final PackageManager mPackageManager;
    private final AppOpsManager mAppOpsManager;
    private UserManager mUserManager;
@@ -388,75 +394,48 @@ public class AccountManagerService
        }.register(mContext, mHandler.getLooper(), UserHandle.ALL, true);

        // Cancel account request notification if an app op was preventing the account access
        mAppOpsManager.startWatchingMode(AppOpsManager.OP_GET_ACCOUNTS, null,
                new AppOpsManager.OnOpChangedInternalListener() {
        for (int i = 0; i < INTERESTING_APP_OPS.length; ++i) {
            mAppOpsManager.startWatchingMode(INTERESTING_APP_OPS[i], null,
                    new OnInterestingAppOpChangedListener());
        }

        // Clear the accounts cache on permission changes.
        // The specific permissions we care about are backed by AppOps, so just
        // let the change events on those handle clearing any notifications.
        mPackageManager.addOnPermissionsChangeListener((int uid) -> {
            AccountManager.invalidateLocalAccountsDataCaches();
        });
    }

    private class OnInterestingAppOpChangedListener implements AppOpsManager.OnOpChangedListener {
        @Override
            public void onOpChanged(int op, String packageName) {
                try {
        public void onOpChanged(String op, String packageName) {
            final int userId = ActivityManager.getCurrentUser();
                    final int uid = mPackageManager.getPackageUidAsUser(packageName, userId);
                    final int mode = mAppOpsManager.checkOpNoThrow(
                            AppOpsManager.OP_GET_ACCOUNTS, uid, packageName);
                    if (mode == AppOpsManager.MODE_ALLOWED) {
                        final long identity = Binder.clearCallingIdentity();
            final int packageUid;
            try {
                            UserAccounts accounts = getUserAccounts(userId);
                            cancelAccountAccessRequestNotificationIfNeeded(
                                    packageName, uid, true, accounts);
                        } finally {
                            Binder.restoreCallingIdentity(identity);
                        }
                    }
                packageUid = mPackageManager.getPackageUidAsUser(packageName, userId);
            } catch (NameNotFoundException e) {
                /* ignore */
                } catch (SQLiteCantOpenDatabaseException e) {
                    Log.w(TAG, "Can't read accounts database", e);
                return;
            }
            }
        });

        // Cancel account request notification if a permission was preventing the account access
        mPackageManager.addOnPermissionsChangeListener(
                (int uid) -> {
            // Permission changes cause requires updating accounts cache.
            AccountManager.invalidateLocalAccountsDataCaches();
            final int mode = mAppOpsManager.checkOpNoThrow(op, packageUid, packageName);
            if (mode != AppOpsManager.MODE_ALLOWED) {
                return;
            }

            Account[] accounts = null;
            String[] packageNames = mPackageManager.getPackagesForUid(uid);
            if (packageNames != null) {
                final int userId = UserHandle.getUserId(uid);
            final long identity = Binder.clearCallingIdentity();
            try {
                    for (String packageName : packageNames) {
                                // if app asked for permission we need to cancel notification even
                                // for O+ applications.
                                if (mPackageManager.checkPermission(
                                        Manifest.permission.GET_ACCOUNTS,
                                        packageName) != PackageManager.PERMISSION_GRANTED) {
                                    continue;
                                }

                        if (accounts == null) {
                            accounts = getAccountsOrEmptyArray(null, userId, "android");
                            if (ArrayUtils.isEmpty(accounts)) {
                                return;
                            }
                        }
                        UserAccounts userAccounts = getUserAccounts(UserHandle.getUserId(uid));
                        for (Account account : accounts) {
                cancelAccountAccessRequestNotificationIfNeeded(
                                    account, uid, packageName, true, userAccounts);
                        }
                    }
                        packageName, packageUid, true, getUserAccounts(userId));
            } catch (SQLiteCantOpenDatabaseException e) {
                Log.w(TAG, "Can't read accounts database", e);
            } finally {
                Binder.restoreCallingIdentity(identity);
            }
        }
        });
    }


    boolean getBindInstantServiceAllowed(int userId) {
        return  mAuthenticatorCache.getBindInstantServiceAllowed(userId);
    }