Merge "Revert sync app op chain changes" into sc-dev

4 files changed, 111 insertions, 102 deletions

diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java
index 4ddb546a351e..bb7cdfa1a1ea 100644
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -2849,14 +2849,14 @@ public class AppOpsManager {
     private static final ThreadLocal<Integer> sBinderThreadCallingUid = new ThreadLocal<>();
 
     /**
-     * If a thread is currently executing a two-way binder transaction, this stores the
-     * ops that were noted blaming any app (the caller, the caller of the caller, etc).
+     * If a thread is currently executing a two-way binder transaction, this stores the op-codes of
+     * the app-ops that were noted during this transaction.
      *
      * @see #getNotedOpCollectionMode
      * @see #collectNotedOpSync
      */
-    private static final ThreadLocal<ArrayMap<String, ArrayMap<String, long[]>>>
-            sAppOpsNotedInThisBinderTransaction = new ThreadLocal<>();
+    private static final ThreadLocal<ArrayMap<String, long[]>> sAppOpsNotedInThisBinderTransaction =
+            new ThreadLocal<>();
 
     /** Whether noting for an appop should be collected */
     private static final @ShouldCollectNoteOp byte[] sAppOpsToNote = new byte[_NUM_OP];
@@ -9052,6 +9052,66 @@ public class AppOpsManager {
     }
 
     /**
+     * State of a temporarily paused noted app-ops collection.
+     *
+     * @see #pauseNotedAppOpsCollection()
+     *
+     * @hide
+     */
+    public static class PausedNotedAppOpsCollection {
+        final int mUid;
+        final @Nullable ArrayMap<String, long[]> mCollectedNotedAppOps;
+
+        PausedNotedAppOpsCollection(int uid, @Nullable ArrayMap<String,
+                long[]> collectedNotedAppOps) {
+            mUid = uid;
+            mCollectedNotedAppOps = collectedNotedAppOps;
+        }
+    }
+
+    /**
+     * Temporarily suspend collection of noted app-ops when binder-thread calls into the other
+     * process. During such a call there might be call-backs coming back on the same thread which
+     * should not be accounted to the current collection.
+     *
+     * @return a state needed to resume the collection
+     *
+     * @hide
+     */
+    public static @Nullable PausedNotedAppOpsCollection pauseNotedAppOpsCollection() {
+        Integer previousUid = sBinderThreadCallingUid.get();
+        if (previousUid != null) {
+            ArrayMap<String, long[]> previousCollectedNotedAppOps =
+                    sAppOpsNotedInThisBinderTransaction.get();
+
+            sBinderThreadCallingUid.remove();
+            sAppOpsNotedInThisBinderTransaction.remove();
+
+            return new PausedNotedAppOpsCollection(previousUid, previousCollectedNotedAppOps);
+        }
+
+        return null;
+    }
+
+    /**
+     * Resume a collection paused via {@link #pauseNotedAppOpsCollection}.
+     *
+     * @param prevCollection The state of the previous collection
+     *
+     * @hide
+     */
+    public static void resumeNotedAppOpsCollection(
+            @Nullable PausedNotedAppOpsCollection prevCollection) {
+        if (prevCollection != null) {
+            sBinderThreadCallingUid.set(prevCollection.mUid);
+
+            if (prevCollection.mCollectedNotedAppOps != null) {
+                sAppOpsNotedInThisBinderTransaction.set(prevCollection.mCollectedNotedAppOps);
+            }
+        }
+    }
+
+    /**
      * Finish collection of noted appops on this thread.
      *
      * <p>Called at the end of a two way binder transaction.
@@ -9091,47 +9151,26 @@ public class AppOpsManager {
      */
     @TestApi
     public static void collectNotedOpSync(@NonNull SyncNotedAppOp syncOp) {
-        collectNotedOpSync(sOpStrToOp.get(syncOp.getOp()), syncOp.getAttributionTag(),
-                syncOp.getPackageName());
-    }
-
-    /**
-     * Collect a noted op when inside of a two-way binder call.
-     *
-     * <p> Delivered to caller via {@link #prefixParcelWithAppOpsIfNeeded}
-     *
-     * @param code the op code to note for
-     * @param attributionTag the attribution tag to note for
-     * @param packageName the package to note for
-     */
-    private static void collectNotedOpSync(int code, @Nullable String attributionTag,
-            @NonNull String packageName) {
         // If this is inside of a two-way binder call:
         // We are inside of a two-way binder call. Delivered to caller via
         // {@link #prefixParcelWithAppOpsIfNeeded}
-        ArrayMap<String, ArrayMap<String, long[]>> appOpsNoted =
-                sAppOpsNotedInThisBinderTransaction.get();
+        int op = sOpStrToOp.get(syncOp.getOp());
+        ArrayMap<String, long[]> appOpsNoted = sAppOpsNotedInThisBinderTransaction.get();
         if (appOpsNoted == null) {
             appOpsNoted = new ArrayMap<>(1);
             sAppOpsNotedInThisBinderTransaction.set(appOpsNoted);
         }
 
-        ArrayMap<String, long[]> packageAppOpsNotedForAttribution = appOpsNoted.get(packageName);
-        if (packageAppOpsNotedForAttribution == null) {
-            packageAppOpsNotedForAttribution = new ArrayMap<>(1);
-            appOpsNoted.put(packageName, packageAppOpsNotedForAttribution);
-        }
-
-        long[] appOpsNotedForAttribution = packageAppOpsNotedForAttribution.get(attributionTag);
+        long[] appOpsNotedForAttribution = appOpsNoted.get(syncOp.getAttributionTag());
         if (appOpsNotedForAttribution == null) {
             appOpsNotedForAttribution = new long[2];
-            packageAppOpsNotedForAttribution.put(attributionTag, appOpsNotedForAttribution);
+            appOpsNoted.put(syncOp.getAttributionTag(), appOpsNotedForAttribution);
         }
 
-        if (code < 64) {
-            appOpsNotedForAttribution[0] |= 1L << code;
+        if (op < 64) {
+            appOpsNotedForAttribution[0] |= 1L << op;
         } else {
-            appOpsNotedForAttribution[1] |= 1L << (code - 64);
+            appOpsNotedForAttribution[1] |= 1L << (op - 64);
         }
     }
 
@@ -9185,7 +9224,9 @@ public class AppOpsManager {
             }
         }
 
-        if (isListeningForOpNotedInBinderTransaction()) {
+        Integer binderUid = sBinderThreadCallingUid.get();
+
+        if (binderUid != null && binderUid == uid) {
             return COLLECT_SYNC;
         } else {
             return COLLECT_ASYNC;
@@ -9204,32 +9245,20 @@ public class AppOpsManager {
      */
     // TODO (b/186872903) Refactor how sync noted ops are propagated.
     public static void prefixParcelWithAppOpsIfNeeded(@NonNull Parcel p) {
-        if (!isListeningForOpNotedInBinderTransaction()) {
-            return;
-        }
-        final ArrayMap<String, ArrayMap<String, long[]>> notedAppOps =
-                sAppOpsNotedInThisBinderTransaction.get();
+        ArrayMap<String, long[]> notedAppOps = sAppOpsNotedInThisBinderTransaction.get();
         if (notedAppOps == null) {
             return;
         }
 
         p.writeInt(Parcel.EX_HAS_NOTED_APPOPS_REPLY_HEADER);
 
-        final int packageCount = notedAppOps.size();
-        p.writeInt(packageCount);
+        int numAttributionWithNotesAppOps = notedAppOps.size();
+        p.writeInt(numAttributionWithNotesAppOps);
 
-        for (int i = 0; i < packageCount; i++) {
+        for (int i = 0; i < numAttributionWithNotesAppOps; i++) {
             p.writeString(notedAppOps.keyAt(i));
-
-            final ArrayMap<String, long[]> notedTagAppOps = notedAppOps.valueAt(i);
-            final int tagCount = notedTagAppOps.size();
-            p.writeInt(tagCount);
-
-            for (int j = 0; j < tagCount; j++) {
-                p.writeString(notedTagAppOps.keyAt(j));
-                p.writeLong(notedTagAppOps.valueAt(j)[0]);
-                p.writeLong(notedTagAppOps.valueAt(j)[1]);
-            }
+            p.writeLong(notedAppOps.valueAt(i)[0]);
+            p.writeLong(notedAppOps.valueAt(i)[1]);
         }
     }
 
@@ -9244,55 +9273,37 @@ public class AppOpsManager {
      * @hide
      */
     public static void readAndLogNotedAppops(@NonNull Parcel p) {
-        final int packageCount = p.readInt();
-        if (packageCount <= 0) {
-            return;
-        }
+        int numAttributionsWithNotedAppOps = p.readInt();
 
-        final String myPackageName = ActivityThread.currentPackageName();
+        for (int i = 0; i < numAttributionsWithNotedAppOps; i++) {
+            String attributionTag = p.readString();
+            long[] rawNotedAppOps = new long[2];
+            rawNotedAppOps[0] = p.readLong();
+            rawNotedAppOps[1] = p.readLong();
 
-        synchronized (sLock) {
-            for (int i = 0; i < packageCount; i++) {
-                final String packageName = p.readString();
-
-                final int tagCount = p.readInt();
-                for (int j = 0; j < tagCount; j++) {
-                    final String attributionTag = p.readString();
-                    final long[] rawNotedAppOps = new long[2];
-                    rawNotedAppOps[0] = p.readLong();
-                    rawNotedAppOps[1] = p.readLong();
-
-                    if (rawNotedAppOps[0] == 0 && rawNotedAppOps[1] == 0) {
-                        continue;
-                    }
+            if (rawNotedAppOps[0] != 0 || rawNotedAppOps[1] != 0) {
+                BitSet notedAppOps = BitSet.valueOf(rawNotedAppOps);
 
-                    final BitSet notedAppOps = BitSet.valueOf(rawNotedAppOps);
+                synchronized (sLock) {
                     for (int code = notedAppOps.nextSetBit(0); code != -1;
                             code = notedAppOps.nextSetBit(code + 1)) {
-                        if (Objects.equals(myPackageName, packageName)) {
-                            if (sOnOpNotedCallback != null) {
-                                sOnOpNotedCallback.onNoted(new SyncNotedAppOp(code,
-                                        attributionTag, packageName));
-                            } else {
-                                String message = getFormattedStackTrace();
-                                sUnforwardedOps.add(new AsyncNotedAppOp(code, Process.myUid(),
-                                        attributionTag, message, System.currentTimeMillis()));
-                                if (sUnforwardedOps.size() > MAX_UNFORWARDED_OPS) {
-                                    sUnforwardedOps.remove(0);
-                                }
+                        if (sOnOpNotedCallback != null) {
+                            sOnOpNotedCallback.onNoted(new SyncNotedAppOp(code, attributionTag));
+                        } else {
+                            String message = getFormattedStackTrace();
+                            sUnforwardedOps.add(
+                                    new AsyncNotedAppOp(code, Process.myUid(), attributionTag,
+                                            message, System.currentTimeMillis()));
+                            if (sUnforwardedOps.size() > MAX_UNFORWARDED_OPS) {
+                                sUnforwardedOps.remove(0);
                             }
-                        } else if (isListeningForOpNotedInBinderTransaction()) {
-                            collectNotedOpSync(code, attributionTag, packageName);
-                        }
-                    }
-                    for (int code = notedAppOps.nextSetBit(0); code != -1;
-                            code = notedAppOps.nextSetBit(code + 1)) {
-                        if (Objects.equals(myPackageName, packageName)) {
-                            sMessageCollector.onNoted(new SyncNotedAppOp(code,
-                                    attributionTag, packageName));
                         }
                     }
                 }
+                for (int code = notedAppOps.nextSetBit(0); code != -1;
+                        code = notedAppOps.nextSetBit(code + 1)) {
+                    sMessageCollector.onNoted(new SyncNotedAppOp(code, attributionTag));
+                }
             }
         }
     }
@@ -9398,15 +9409,7 @@ public class AppOpsManager {
      * @hide
      */
     public static boolean isListeningForOpNoted() {
-        return sOnOpNotedCallback != null || isListeningForOpNotedInBinderTransaction()
-                || isCollectingStackTraces();
-    }
-
-    /**
-     * @return whether we are in a binder transaction and collecting appops.
-     */
-    private static boolean isListeningForOpNotedInBinderTransaction() {
-        return sBinderThreadCallingUid.get() != null;
+        return sOnOpNotedCallback != null || isCollectingStackTraces();
     }
 
     /**
diff --git a/core/java/android/app/SyncNotedAppOp.java b/core/java/android/app/SyncNotedAppOp.java
index 32d889e81cb0..7c0c08a7fc35 100644
--- a/core/java/android/app/SyncNotedAppOp.java
+++ b/core/java/android/app/SyncNotedAppOp.java
@@ -21,7 +21,6 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.TestApi;
 import android.os.Parcelable;
-import android.os.Process;
 
 import com.android.internal.annotations.Immutable;
 import com.android.internal.util.DataClass;
@@ -29,6 +28,8 @@ import com.android.internal.util.DataClass;
 /**
  * Description of an app-op that was noted for the current process.
  *
+ * Note: package name is currently unused in the system.
+ *
  * <p>This is either delivered after a
  * {@link AppOpsManager.OnOpNotedCallback#onNoted(SyncNotedAppOp) two way binder call} or
  * when the app
diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java
index d44b016cb5d0..3d466a0bf007 100644
--- a/core/java/android/os/BinderProxy.java
+++ b/core/java/android/os/BinderProxy.java
@@ -560,6 +560,9 @@ public final class BinderProxy implements IBinder {
             }
         }
 
+        final AppOpsManager.PausedNotedAppOpsCollection prevCollection =
+                AppOpsManager.pauseNotedAppOpsCollection();
+
         if ((flags & FLAG_ONEWAY) == 0 && AppOpsManager.isListeningForOpNoted()) {
             flags |= FLAG_COLLECT_NOTED_APP_OPS;
         }
@@ -567,6 +570,8 @@ public final class BinderProxy implements IBinder {
         try {
             return transactNative(code, data, reply, flags);
         } finally {
+            AppOpsManager.resumeNotedAppOpsCollection(prevCollection);
+
             if (transactListener != null) {
                 transactListener.onTransactEnded(session);
             }
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index a9905dcf8904..6d7966f1eb7c 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -3438,7 +3438,7 @@ public class AppOpsService extends IAppOpsService.Stub {
                         + " package " + packageName + "flags: " +
                         AppOpsManager.flagsToString(flags));
                 return new SyncNotedAppOp(AppOpsManager.MODE_ERRORED, code, attributionTag,
-                        packageName + " flags: " + AppOpsManager.flagsToString(flags));
+                        packageName);
             }
             final Op op = getOpLocked(ops, code, uid, true);
             final AttributedOp attributedOp = op.getOrCreateAttribution(op, attributionTag);