Merge "Don't callback twice for app op changes in PermissionPolicyService."

3 files changed, 57 insertions, 8 deletions

diff --git a/core/java/android/app/AppOpsManagerInternal.java b/core/java/android/app/AppOpsManagerInternal.java
index 9958c6a31027..c13c5a5ab9c4 100644
--- a/core/java/android/app/AppOpsManagerInternal.java
+++ b/core/java/android/app/AppOpsManagerInternal.java
@@ -21,6 +21,7 @@ import android.annotation.Nullable;
 import android.util.SparseArray;
 import android.util.SparseIntArray;
 
+import com.android.internal.app.IAppOpsCallback;
 import com.android.internal.util.function.HexFunction;
 import com.android.internal.util.function.QuadFunction;
 
@@ -91,4 +92,16 @@ public abstract class AppOpsManagerInternal {
      */
     public abstract void updateAppWidgetVisibility(SparseArray<String> uidPackageNames,
             boolean visible);
+
+    /**
+     * Like {@link AppOpsManager#setUidMode}, but allows ignoring a certain callback.
+     */
+    public abstract void setUidModeIgnoringCallback(int code, int uid, int mode,
+            @Nullable IAppOpsCallback callbackToIgnore);
+
+    /**
+     * Like {@link AppOpsManager#setMode}, but allows ignoring a certain callback.
+     */
+    public abstract void setModeIgnoringCallback(int code, int uid, @NonNull String packageName,
+            int mode, @Nullable IAppOpsCallback callbackToIgnore);
 }
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index 531bc5d1c7df..72dd6cf8352a 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -1948,6 +1948,11 @@ public class AppOpsService extends IAppOpsService.Stub {
 
     @Override
     public void setUidMode(int code, int uid, int mode) {
+        setUidMode(code, uid, mode, null);
+    }
+
+    private void setUidMode(int code, int uid, int mode,
+            @Nullable IAppOpsCallback callbackToIgnore) {
         if (DEBUG) {
             Slog.i(TAG, "uid " + uid + " OP_" + opToName(code) + " := " + modeToName(mode)
                     + " by uid " + Binder.getCallingUid());
@@ -2031,6 +2036,10 @@ public class AppOpsService extends IAppOpsService.Stub {
                     }
                 }
             }
+
+            if (callbackSpecs != null && callbackToIgnore != null) {
+                callbackSpecs.remove(mModeWatchers.get(callbackToIgnore.asBinder()));
+            }
         }
 
         if (callbackSpecs == null) {
@@ -2158,6 +2167,11 @@ public class AppOpsService extends IAppOpsService.Stub {
      */
     @Override
     public void setMode(int code, int uid, @NonNull String packageName, int mode) {
+        setMode(code, uid, packageName, mode, null);
+    }
+
+    private void setMode(int code, int uid, @NonNull String packageName, int mode,
+            @Nullable IAppOpsCallback callbackToIgnore) {
         enforceManageAppOpsModes(Binder.getCallingPid(), Binder.getCallingUid(), uid);
         verifyIncomingOp(code);
         ArraySet<ModeCallback> repCbs = null;
@@ -2201,6 +2215,9 @@ public class AppOpsService extends IAppOpsService.Stub {
                         }
                         repCbs.addAll(cbs);
                     }
+                    if (repCbs != null && callbackToIgnore != null) {
+                        repCbs.remove(mModeWatchers.get(callbackToIgnore.asBinder()));
+                    }
                     if (mode == AppOpsManager.opToDefaultMode(op.op)) {
                         // If going into the default mode, prune this op
                         // if there is nothing else interesting in it.
@@ -5587,5 +5604,17 @@ public class AppOpsService extends IAppOpsService.Stub {
                 boolean visible) {
             AppOpsService.this.updateAppWidgetVisibility(uidPackageNames, visible);
         }
+
+        @Override
+        public void setUidModeIgnoringCallback(int code, int uid, int mode,
+                @Nullable IAppOpsCallback callbackToIgnore) {
+            setUidMode(code, uid, mode, callbackToIgnore);
+        }
+
+        @Override
+        public void setModeIgnoringCallback(int code, int uid, @NonNull String packageName,
+                int mode, @Nullable IAppOpsCallback callbackToIgnore) {
+            setMode(code, uid, packageName, mode, callbackToIgnore);
+        }
     }
 }
diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java
index a86c8d7545b1..2c7795a6274b 100644
--- a/services/core/java/com/android/server/policy/PermissionPolicyService.java
+++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java
@@ -29,6 +29,7 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
 import android.app.AppOpsManager;
+import android.app.AppOpsManagerInternal;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.ApplicationInfo;
@@ -84,6 +85,8 @@ public final class PermissionPolicyService extends SystemService {
 
     private final Object mLock = new Object();
 
+    private IAppOpsCallback mAppOpsCallback;
+
     /** Whether the user is started but not yet stopped */
     @GuardedBy("mLock")
     private final SparseBooleanArray mIsStarted = new SparseBooleanArray();
@@ -138,7 +141,7 @@ public final class PermissionPolicyService extends SystemService {
         permManagerInternal.addOnRuntimePermissionStateChangedListener(
                 this::synchronizePackagePermissionsAndAppOpsAsyncForUser);
 
-        IAppOpsCallback appOpsListener = new IAppOpsCallback.Stub() {
+        mAppOpsCallback = new IAppOpsCallback.Stub() {
             public void opChanged(int op, int uid, String packageName) {
                 synchronizePackagePermissionsAndAppOpsAsyncForUser(packageName,
                         UserHandle.getUserId(uid));
@@ -155,7 +158,7 @@ public final class PermissionPolicyService extends SystemService {
                 PermissionInfo perm = dangerousPerms.get(i);
 
                 if (perm.isRuntime()) {
-                    appOpsService.startWatchingMode(getSwitchOp(perm.name), null, appOpsListener);
+                    appOpsService.startWatchingMode(getSwitchOp(perm.name), null, mAppOpsCallback);
                 }
                 if (perm.isSoftRestricted()) {
                     SoftRestrictedPermissionPolicy policy =
@@ -163,7 +166,7 @@ public final class PermissionPolicyService extends SystemService {
                                     perm.name);
                     int extraAppOp = policy.getExtraAppOpCode();
                     if (extraAppOp != OP_NONE) {
-                        appOpsService.startWatchingMode(extraAppOp, null, appOpsListener);
+                        appOpsService.startWatchingMode(extraAppOp, null, mAppOpsCallback);
                     }
                 }
             }
@@ -386,10 +389,11 @@ public final class PermissionPolicyService extends SystemService {
      * Synchronizes permission to app ops. You *must* always sync all packages
      * in a shared UID at the same time to ensure proper synchronization.
      */
-    private static class PermissionToOpSynchroniser {
+    private class PermissionToOpSynchroniser {
         private final @NonNull Context mContext;
         private final @NonNull PackageManager mPackageManager;
         private final @NonNull AppOpsManager mAppOpsManager;
+        private final @NonNull AppOpsManagerInternal mAppOpsManagerInternal;
 
         private final @NonNull ArrayMap<String, PermissionInfo> mRuntimePermissionInfos;
 
@@ -429,6 +433,7 @@ public final class PermissionPolicyService extends SystemService {
             mContext = context;
             mPackageManager = context.getPackageManager();
             mAppOpsManager = context.getSystemService(AppOpsManager.class);
+            mAppOpsManagerInternal = LocalServices.getService(AppOpsManagerInternal.class);
 
             mRuntimePermissionInfos = new ArrayMap<>();
             PermissionManagerServiceInternal permissionManagerInternal = LocalServices.getService(
@@ -668,7 +673,8 @@ public final class PermissionPolicyService extends SystemService {
                     opCode), uid, packageName);
             if (currentMode != MODE_ALLOWED) {
                 if (currentMode != MODE_IGNORED) {
-                    mAppOpsManager.setUidMode(opCode, uid, MODE_IGNORED);
+                    mAppOpsManagerInternal.setUidModeIgnoringCallback(opCode, uid, MODE_IGNORED,
+                            mAppOpsCallback);
                 }
                 return true;
             }
@@ -680,15 +686,16 @@ public final class PermissionPolicyService extends SystemService {
             final int oldMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager.opToPublicName(
                     opCode), uid, packageName);
             if (oldMode != mode) {
-                mAppOpsManager.setUidMode(opCode, uid, mode);
+                mAppOpsManagerInternal.setUidModeIgnoringCallback(opCode, uid, mode,
+                        mAppOpsCallback);
                 final int newMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager.opToPublicName(
                         opCode), uid, packageName);
                 if (newMode != mode) {
                     // Work around incorrectly-set package mode. It never makes sense for app ops
                     // related to runtime permissions, but can get in the way and we have to reset
                     // it.
-                    mAppOpsManager.setMode(opCode, uid, packageName, AppOpsManager.opToDefaultMode(
-                            opCode));
+                    mAppOpsManagerInternal.setModeIgnoringCallback(opCode, uid, packageName,
+                            AppOpsManager.opToDefaultMode(opCode), mAppOpsCallback);
                 }
             }
         }