Make PermissionPolicyService use OnPermissionsChangeListener.

And remove the internal OnRuntimePermissionStateChangedListener.

The system API PackageMangaer.addOnPermissionsChangeListener()
actually works very similar to
PermissionManagerServiceInternal.addOnRuntimePermissionStateChangedListener(),
as long as some fixes are applied to the former. Specifically:

1.  OnPermissionsChangeListener is currently notified for runtime
    permission state and flag changes, as well as install permission
    flag changes. However, since permission flags are only used for
    runtime permissions, it makes no sense to notify for install
    permission flag changes while not notifying for install permission
    state changes which is far more important for install
    permissions. So we can remove the useless notification for install
    permission flag changes, and make it align with the internal
    listener to be replaced & removed in this CL.

    This means we can replace
    onInstallPermissionUpdatedNotifyListener() with
    onInstallPermissionUpdated(), since we shouldn't notify the
    listener for install permission changes.

2.  Start to notify OnPermissionsChangeListener for changes inside
    restorePermissionState(). This may happen when packages are
    updated and it's natural for callers to expect a callback if the
    permission state changed, but so far that's not true for
    OnPermissionsChangeListener, and it only works for the internal
    listener. So we should fix that and make PermissionManagerService
    actually behave consistently.

    This means we can merge onPermissionUpdatedNotifyListener() into
    onPermissionUpdated(), since the listener should always be
    notified.

Another thing to note is that AndroidPackage.getUid() returns the app
ID instead of the per-user UID despite its name.

PermissionPolicyService is also updated to work by UIDs instead of
packages, since it would just query that package for its UID to get
all the package names in that UID. Calls into package manager are also
reduced a bit, which may result in a slight performance improvement
since we skipped one PackageInfo generation.

OnPermissionsChangeListener is async (because binder calls are
possible) while the internal listener is sync. This would mean
PermissionPolicyService are now getting callbacks async, but we always
treated those callbacks as async anyway (we post work onto a worker
thread in the callback) so that's unlikely a problem.

Bug: 263504888
Test: presubmit
Change-Id: Ieb0742832ed9c30e90a79422aa4048133a2f8258
This commit is contained in:
Hai Zhang
2023-05-02 22:35:27 -07:00
parent c46674d0ca
commit 1993c88473
8 changed files with 49 additions and 257 deletions

View File

@@ -724,18 +724,6 @@ public class PermissionManagerService extends IPermissionManager.Stub {
pkg, sharedUserPkgs, userId);
}
@Override
public void addOnRuntimePermissionStateChangedListener(
OnRuntimePermissionStateChangedListener listener) {
mPermissionManagerServiceImpl.addOnRuntimePermissionStateChangedListener(listener);
}
@Override
public void removeOnRuntimePermissionStateChangedListener(
OnRuntimePermissionStateChangedListener listener) {
mPermissionManagerServiceImpl.removeOnRuntimePermissionStateChangedListener(listener);
}
@Override
public void onSystemReady() {
mPermissionManagerServiceImpl.onSystemReady();

View File

@@ -125,7 +125,6 @@ import com.android.internal.util.CollectionUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.IntPair;
import com.android.internal.util.Preconditions;
import com.android.internal.util.function.pooled.PooledLambda;
import com.android.server.FgThread;
import com.android.server.LocalServices;
import com.android.server.PermissionThread;
@@ -311,12 +310,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
@GuardedBy("mLock")
private final SparseBooleanArray mHasNoDelayedPermBackup = new SparseBooleanArray();
/** Listeners for permission state (granting and flags) changes */
@GuardedBy("mLock")
private final ArrayList<PermissionManagerServiceInternal
.OnRuntimePermissionStateChangedListener>
mRuntimePermissionStateChangedListeners = new ArrayList<>();
private final boolean mIsLeanback;
@NonNull
@@ -395,7 +388,11 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
mPackageManagerInt.writeSettings(true);
}
@Override
public void onPermissionUpdated(int[] userIds, boolean sync) {
public void onPermissionUpdated(int[] userIds, boolean sync, int appId) {
for (int i = 0; i < userIds.length; i++) {
int uid = UserHandle.getUid(userIds[i], appId);
mOnPermissionChangeListeners.onPermissionsChanged(uid);
}
mPackageManagerInt.writePermissionSettings(userIds, !sync);
}
@Override
@@ -406,18 +403,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
public void onPermissionRemoved() {
mPackageManagerInt.writeSettings(false);
}
public void onPermissionUpdatedNotifyListener(@UserIdInt int[] updatedUserIds, boolean sync,
int uid) {
onPermissionUpdated(updatedUserIds, sync);
for (int i = 0; i < updatedUserIds.length; i++) {
int userUid = UserHandle.getUid(updatedUserIds[i], UserHandle.getAppId(uid));
mOnPermissionChangeListeners.onPermissionsChanged(userUid);
}
}
public void onInstallPermissionUpdatedNotifyListener(int uid) {
onInstallPermissionUpdated();
mOnPermissionChangeListeners.onPermissionsChanged(uid);
}
};
public PermissionManagerServiceImpl(@NonNull Context context,
@@ -879,17 +864,13 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
permissionUpdated = uidState.updatePermissionFlags(bp, flagMask, flagValues);
}
if (permissionUpdated && isRuntimePermission) {
notifyRuntimePermissionStateChanged(packageName, userId);
}
if (permissionUpdated && callback != null) {
// Install and runtime permissions are stored in different places,
// so figure out what permission changed and persist the change.
if (!isRuntimePermission) {
int userUid = UserHandle.getUid(userId, pkg.getUid());
callback.onInstallPermissionUpdatedNotifyListener(userUid);
callback.onInstallPermissionUpdated();
} else {
callback.onPermissionUpdatedNotifyListener(new int[]{userId}, false, pkg.getUid());
callback.onPermissionUpdated(new int[]{ userId }, false, pkg.getUid());
}
}
}
@@ -1499,10 +1480,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
callback.onGidsChanged(UserHandle.getAppId(pkg.getUid()), userId);
}
}
if (isRuntimePermission) {
notifyRuntimePermissionStateChanged(packageName, userId);
}
}
@Override
@@ -1657,10 +1634,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
mDefaultPermissionCallback.onInstallPermissionRevoked();
}
}
if (isRuntimePermission) {
notifyRuntimePermissionStateChanged(packageName, userId);
}
}
private boolean mayManageRolePermission(int uid) {
@@ -1716,8 +1689,9 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
mDefaultPermissionCallback.onInstallPermissionRevoked();
}
public void onPermissionUpdated(int[] updatedUserIds, boolean sync) {
for (int userId : updatedUserIds) {
public void onPermissionUpdated(int[] userIds, boolean sync, int appId) {
mOnPermissionChangeListeners.onPermissionsChanged(appId);
for (int userId : userIds) {
if (sync) {
syncUpdatedUsers.add(userId);
asyncUpdatedUsers.remove(userId);
@@ -1737,16 +1711,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
public void onInstallPermissionUpdated() {
mDefaultPermissionCallback.onInstallPermissionUpdated();
}
public void onPermissionUpdatedNotifyListener(@UserIdInt int[] updatedUserIds,
boolean sync, int uid) {
onPermissionUpdated(updatedUserIds, sync);
mOnPermissionChangeListeners.onPermissionsChanged(uid);
}
public void onInstallPermissionUpdatedNotifyListener(int uid) {
mDefaultPermissionCallback.onInstallPermissionUpdatedNotifyListener(uid);
}
};
if (filterPkg != null) {
@@ -2074,45 +2038,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
});
}
@Override
public void addOnRuntimePermissionStateChangedListener(
PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener listener) {
synchronized (mLock) {
mRuntimePermissionStateChangedListeners.add(listener);
}
}
@Override
public void removeOnRuntimePermissionStateChangedListener(
PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener listener) {
synchronized (mLock) {
mRuntimePermissionStateChangedListeners.remove(listener);
}
}
private void notifyRuntimePermissionStateChanged(@NonNull String packageName,
@UserIdInt int userId) {
FgThread.getHandler().sendMessage(PooledLambda.obtainMessage(
PermissionManagerServiceImpl::doNotifyRuntimePermissionStateChanged,
PermissionManagerServiceImpl.this, packageName, userId));
}
private void doNotifyRuntimePermissionStateChanged(@NonNull String packageName,
@UserIdInt int userId) {
final ArrayList<PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener>
listeners;
synchronized (mLock) {
if (mRuntimePermissionStateChangedListeners.isEmpty()) {
return;
}
listeners = new ArrayList<>(mRuntimePermissionStateChangedListeners);
}
final int listenerCount = listeners.size();
for (int i = 0; i < listenerCount; i++) {
listeners.get(i).onRuntimePermissionStateChanged(packageName, userId);
}
}
/**
* If the app is updated, and has scoped storage permissions, then it is possible that the
* app updated in an attempt to get unscoped storage. If so, revoke all storage permissions.
@@ -3014,11 +2939,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
if (callback != null) {
callback.onPermissionUpdated(updatedUserIds,
(changingPackageName != null && replace && installPermissionsChanged)
|| runtimePermissionsRevoked);
}
for (int userId : updatedUserIds) {
notifyRuntimePermissionStateChanged(pkg.getPackageName(), userId);
|| runtimePermissionsRevoked, pkg.getUid());
}
}
@@ -3864,7 +3785,8 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
isGranted = uidState.isPermissionGranted(permissionName);
}
if (!isGranted) {
mDefaultPermissionCallback.onPermissionRevoked(pkg.getUid(), userId, null);
mDefaultPermissionCallback.onPermissionRevoked(
UserHandle.getUid(userId, pkg.getUid()), userId, null);
break;
}
}
@@ -5348,25 +5270,14 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
public void onPermissionGranted(int uid, @UserIdInt int userId) {}
public void onInstallPermissionGranted() {}
public void onPermissionRevoked(int uid, @UserIdInt int userId, String reason) {
onPermissionRevoked(uid, userId, reason, false);
}
public void onPermissionRevoked(int uid, @UserIdInt int userId, String reason,
boolean overrideKill) {
onPermissionRevoked(uid, userId, reason, false, null);
}
public void onPermissionRevoked(int uid, @UserIdInt int userId, String reason,
boolean overrideKill, @Nullable String permissionName) {}
public void onInstallPermissionRevoked() {}
public void onPermissionUpdated(@UserIdInt int[] updatedUserIds, boolean sync) {}
public void onPermissionUpdatedNotifyListener(@UserIdInt int[] updatedUserIds, boolean sync,
int uid) {
onPermissionUpdated(updatedUserIds, sync);
}
public void onPermissionUpdated(@UserIdInt int[] userIds, boolean sync, int appId) {}
public void onPermissionRemoved() {}
public void onInstallPermissionUpdated() {}
public void onInstallPermissionUpdatedNotifyListener(int uid) {
onInstallPermissionUpdated();
}
}
private static final class OnPermissionChangeListeners extends Handler {

View File

@@ -382,24 +382,6 @@ public interface PermissionManagerServiceInterface extends PermissionManagerInte
*/
int checkUidPermission(int uid, String permName);
/**
* Adds a listener for runtime permission state (permissions or flags) changes.
*
* @param listener The listener.
*/
void addOnRuntimePermissionStateChangedListener(
@NonNull PermissionManagerServiceInternal
.OnRuntimePermissionStateChangedListener listener);
/**
* Removes a listener for runtime permission state (permissions or flags) changes.
*
* @param listener The listener.
*/
void removeOnRuntimePermissionStateChangedListener(
@NonNull PermissionManagerServiceInternal
.OnRuntimePermissionStateChangedListener listener);
/**
* Get all the package names requesting app op permissions.
*

View File

@@ -65,22 +65,6 @@ public interface PermissionManagerServiceInternal extends PermissionManagerInter
//@SystemApi(client = SystemApi.Client.SYSTEM_SERVER)
int checkUidPermission(int uid, @NonNull String permissionName);
/**
* Adds a listener for runtime permission state (permissions or flags) changes.
*
* @param listener The listener.
*/
void addOnRuntimePermissionStateChangedListener(
@NonNull OnRuntimePermissionStateChangedListener listener);
/**
* Removes a listener for runtime permission state (permissions or flags) changes.
*
* @param listener The listener.
*/
void removeOnRuntimePermissionStateChangedListener(
@NonNull OnRuntimePermissionStateChangedListener listener);
/**
* Get whether permission review is required for a package.
*
@@ -312,22 +296,6 @@ public interface PermissionManagerServiceInternal extends PermissionManagerInter
@Nullable PackageState packageState, @Nullable AndroidPackage pkg,
@NonNull List<AndroidPackage> sharedUserPkgs, @UserIdInt int userId);
/**
* Listener for package permission state (permissions or flags) changes.
*/
interface OnRuntimePermissionStateChangedListener {
/**
* Called when the runtime permission state (permissions or flags) changed.
*
* @param packageName The package for which the change happened.
* @param userId the user id for which the change happened.
*/
@Nullable
void onRuntimePermissionStateChanged(@NonNull String packageName,
@UserIdInt int userId);
}
/**
* The permission-related parameters passed in for package installation.
*

View File

@@ -237,23 +237,6 @@ public class PermissionManagerServiceLoggingDecorator implements PermissionManag
return mService.checkUidPermission(uid, permName);
}
@Override
public void addOnRuntimePermissionStateChangedListener(
@NonNull PermissionManagerServiceInternal
.OnRuntimePermissionStateChangedListener listener) {
Log.i(LOG_TAG, "addOnRuntimePermissionStateChangedListener(listener = " + listener + ")");
mService.addOnRuntimePermissionStateChangedListener(listener);
}
@Override
public void removeOnRuntimePermissionStateChangedListener(
@NonNull PermissionManagerServiceInternal
.OnRuntimePermissionStateChangedListener listener) {
Log.i(LOG_TAG, "removeOnRuntimePermissionStateChangedListener(listener = " + listener
+ ")");
mService.removeOnRuntimePermissionStateChangedListener(listener);
}
@Override
public Map<String, Set<String>> getAllAppOpPermissionPackages() {
Log.i(LOG_TAG, "getAllAppOpPermissionPackages()");

View File

@@ -312,20 +312,6 @@ public class PermissionManagerServiceTestingShim implements PermissionManagerSer
return newVal;
}
@Override
public void addOnRuntimePermissionStateChangedListener(@NonNull
PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener listener) {
mOldImplementation.addOnRuntimePermissionStateChangedListener(listener);
mNewImplementation.addOnRuntimePermissionStateChangedListener(listener);
}
@Override
public void removeOnRuntimePermissionStateChangedListener(@NonNull
PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener listener) {
mOldImplementation.removeOnRuntimePermissionStateChangedListener(listener);
mNewImplementation.removeOnRuntimePermissionStateChangedListener(listener);
}
@Override
public Map<String, Set<String>> getAllAppOpPermissionPackages() {
Map<String, Set<String>> oldVal = mOldImplementation.getAllAppOpPermissionPackages();

View File

@@ -58,7 +58,6 @@ import android.content.pm.PackageManager.NameNotFoundException;
import android.content.pm.PackageManagerInternal;
import android.content.pm.PackageManagerInternal.PackageListObserver;
import android.content.pm.PermissionInfo;
import android.content.pm.UserPackage;
import android.content.res.Resources;
import android.os.Build;
import android.os.Bundle;
@@ -142,18 +141,18 @@ public final class PermissionPolicyService extends SystemService {
private OnInitializedCallback mOnInitializedCallback;
/**
* Whether an async {@link #synchronizePackagePermissionsAndAppOpsForUser} is currently
* scheduled for a package/user.
* Whether an async {@link #synchronizeUidPermissionsAndAppOps} is currently
* scheduled for a UID.
*/
@GuardedBy("mLock")
private final ArraySet<UserPackage> mIsPackageSyncsScheduled = new ArraySet<>();
private final SparseBooleanArray mIsUidSyncScheduled = new SparseBooleanArray();
/**
* Whether an async {@link #resetAppOpPermissionsIfNotRequestedForUid} is currently
* scheduled for a uid.
*/
@GuardedBy("mLock")
private final SparseBooleanArray mIsUidSyncScheduled = new SparseBooleanArray();
private final SparseBooleanArray mIsUidResetScheduled = new SparseBooleanArray();
/**
* This change reflects the presence of the new Notification Permission
@@ -199,7 +198,8 @@ public final class PermissionPolicyService extends SystemService {
.getUserIds();
for (final int userId : userIds) {
if (isStarted(userId)) {
synchronizePackagePermissionsAndAppOpsForUser(packageName, userId);
final int uid = UserHandle.getUid(userId, appId);
synchronizeUidPermissionsAndAppOps(uid);
}
}
}
@@ -210,8 +210,8 @@ public final class PermissionPolicyService extends SystemService {
.getUserIds();
for (final int userId : userIds) {
if (isStarted(userId)) {
synchronizePackagePermissionsAndAppOpsForUser(packageName, userId);
final int uid = UserHandle.getUid(userId, appId);
synchronizeUidPermissionsAndAppOps(uid);
resetAppOpPermissionsIfNotRequestedForUid(uid);
}
}
@@ -230,14 +230,13 @@ public final class PermissionPolicyService extends SystemService {
}
});
mPermissionManagerInternal.addOnRuntimePermissionStateChangedListener(
this::synchronizePackagePermissionsAndAppOpsAsyncForUser);
mPackageManager.addOnPermissionsChangeListener(
this::synchronizeUidPermissionsAndAppOpsAsync);
mAppOpsCallback = new IAppOpsCallback.Stub() {
public void opChanged(int op, int uid, @Nullable String packageName) {
if (packageName != null) {
synchronizePackagePermissionsAndAppOpsAsyncForUser(packageName,
UserHandle.getUserId(uid));
synchronizeUidPermissionsAndAppOpsAsync(uid);
}
resetAppOpPermissionsIfNotRequestedForUidAsync(uid);
}
@@ -381,24 +380,23 @@ public final class PermissionPolicyService extends SystemService {
return AppOpsManager.opToSwitch(op);
}
private void synchronizePackagePermissionsAndAppOpsAsyncForUser(@NonNull String packageName,
@UserIdInt int changedUserId) {
if (isStarted(changedUserId)) {
private void synchronizeUidPermissionsAndAppOpsAsync(int uid) {
final int userId = UserHandle.getUserId(uid);
if (isStarted(userId)) {
synchronized (mLock) {
if (mIsPackageSyncsScheduled.add(UserPackage.of(changedUserId, packageName))) {
if (!mIsUidSyncScheduled.get(uid)) {
// TODO(b/165030092): migrate this to PermissionThread.getHandler().
// synchronizePackagePermissionsAndAppOpsForUser is a heavy operation.
// synchronizeUidPermissionsAndAppOps is a heavy operation.
// Dispatched on a PermissionThread, it interferes with user switch.
// FgThread is busy and schedules it after most of the switch is done.
// A possible solution is to delay the callback.
FgThread.getHandler().sendMessage(PooledLambda.obtainMessage(
PermissionPolicyService
::synchronizePackagePermissionsAndAppOpsForUser,
this, packageName, changedUserId));
PermissionPolicyService::synchronizeUidPermissionsAndAppOps, this,
uid));
mIsUidSyncScheduled.put(uid, true);
} else {
if (DEBUG) {
Slog.v(LOG_TAG, "sync for " + packageName + "/" + changedUserId
+ " already scheduled");
Slog.v(LOG_TAG, "sync for UID " + uid + " already scheduled");
}
}
}
@@ -648,39 +646,27 @@ public final class PermissionPolicyService extends SystemService {
}
/**
* Synchronize a single package.
* Synchronize a single UID.
*/
private void synchronizePackagePermissionsAndAppOpsForUser(@NonNull String packageName,
@UserIdInt int userId) {
private void synchronizeUidPermissionsAndAppOps(int uid) {
synchronized (mLock) {
mIsPackageSyncsScheduled.remove(UserPackage.of(userId, packageName));
mIsUidSyncScheduled.delete(uid);
}
if (DEBUG) {
Slog.v(LOG_TAG,
"synchronizePackagePermissionsAndAppOpsForUser(" + packageName + ", "
+ userId + ")");
"synchronizePackagePermissionsAndAppOpsForUser(" + uid + ")");
}
final PackageManagerInternal packageManagerInternal = LocalServices.getService(
PackageManagerInternal.class);
final PackageInfo pkg = packageManagerInternal.getPackageInfo(packageName, 0,
Process.SYSTEM_UID, userId);
if (pkg == null) {
return;
}
final UserHandle user = UserHandle.getUserHandleForUid(uid);
final PermissionToOpSynchroniser synchroniser = new PermissionToOpSynchroniser(
getUserContext(getContext(), UserHandle.of(userId)));
synchroniser.addPackage(pkg.packageName);
final String[] sharedPkgNames = packageManagerInternal.getSharedUserPackagesForPackage(
pkg.packageName, userId);
for (String sharedPkgName : sharedPkgNames) {
final AndroidPackage sharedPkg = packageManagerInternal
.getPackage(sharedPkgName);
if (sharedPkg != null) {
synchroniser.addPackage(sharedPkg.getPackageName());
}
getUserContext(getContext(), user));
final int appId = UserHandle.getAppId(uid);
final List<AndroidPackage> pkgs = mPackageManagerInternal.getPackagesForAppId(appId);
final int pkgsSize = pkgs.size();
for (int i = 0; i < pkgsSize; i++) {
final AndroidPackage pkg = pkgs.get(i);
synchroniser.addPackage(pkg.getPackageName());
}
synchroniser.syncPackages();
}
@@ -708,8 +694,8 @@ public final class PermissionPolicyService extends SystemService {
private void resetAppOpPermissionsIfNotRequestedForUidAsync(int uid) {
if (isStarted(UserHandle.getUserId(uid))) {
synchronized (mLock) {
if (!mIsUidSyncScheduled.get(uid)) {
mIsUidSyncScheduled.put(uid, true);
if (!mIsUidResetScheduled.get(uid)) {
mIsUidResetScheduled.put(uid, true);
PermissionThread.getHandler().sendMessage(PooledLambda.obtainMessage(
PermissionPolicyService::resetAppOpPermissionsIfNotRequestedForUid,
this, uid));
@@ -720,7 +706,7 @@ public final class PermissionPolicyService extends SystemService {
private void resetAppOpPermissionsIfNotRequestedForUid(int uid) {
synchronized (mLock) {
mIsUidSyncScheduled.delete(uid);
mIsUidResetScheduled.delete(uid);
}
final Context context = getContext();
@@ -858,7 +844,7 @@ public final class PermissionPolicyService extends SystemService {
/**
* Set app ops that were added in {@link #addPackage}.
*
* <p>This processes ops previously added by {@link #addAppOps(PackageInfo, String)}
* <p>This processes ops previously added by {@link #addPackage(String)})}
*/
private void syncPackages() {
// Remember which ops were already set. This makes sure that we always set the most

View File

@@ -1642,18 +1642,6 @@ class PermissionService(
onPermissionsChangeListeners.removeListener(listener)
}
override fun addOnRuntimePermissionStateChangedListener(
listener: PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener
) {
// TODO: Should be removed once we remove PermissionPolicyService.
}
override fun removeOnRuntimePermissionStateChangedListener(
listener: PermissionManagerServiceInternal.OnRuntimePermissionStateChangedListener
) {
// TODO: Should be removed once we remove PermissionPolicyService.
}
override fun getSplitPermissions(): List<SplitPermissionInfoParcelable> {
return PermissionManager.splitPermissionInfoListToParcelableList(
systemConfig.splitPermissions