Frozen-aware AppOpsService op noted callback

Use the new RemoteCallbackList API in AppOpsService to drop op noted
callbacks when the process hosting the callback is frozen.

Flag: android.permission.flags.use_frozen_aware_remote_callback_list
Change-Id: I88efec347f9779b9cdd77da73d5612a7e361c146
Bug: 327047060
Bug: 361157077
Test: atest AppOpsLoggingTest

6 files changed, 113 insertions, 11 deletions

diff --git a/core/java/android/permission/flags.aconfig b/core/java/android/permission/flags.aconfig
index 1d654e1322c7..7944be140960 100644
--- a/core/java/android/permission/flags.aconfig
+++ b/core/java/android/permission/flags.aconfig
@@ -239,6 +239,13 @@ flag {
 }
 
 flag {
+  name: "use_frozen_aware_remote_callback_list"
+  namespace: "permissions"
+  description: "Whether to use the new frozen-aware RemoteCallbackList API for op noted callbacks."
+  bug: "361157077"
+}
+
+flag {
     name: "wallet_role_icon_property_enabled"
     is_exported: true
     namespace: "wallet_integration"
diff --git a/core/tests/coretests/AppThatUsesAppOps/src/android/app/appops/appthatusesappops/AppOpsUserService.kt b/core/tests/coretests/AppThatUsesAppOps/src/android/app/appops/appthatusesappops/AppOpsUserService.kt
index 48053c11f2d1..c5f07ff8f853 100644
--- a/core/tests/coretests/AppThatUsesAppOps/src/android/app/appops/appthatusesappops/AppOpsUserService.kt
+++ b/core/tests/coretests/AppThatUsesAppOps/src/android/app/appops/appthatusesappops/AppOpsUserService.kt
@@ -22,7 +22,9 @@ import android.app.AsyncNotedAppOp
 import android.app.Service
 import android.app.SyncNotedAppOp
 import android.content.Intent
+import android.os.Handler
 import android.os.IBinder
+import android.os.Looper
 import android.util.Log
 import com.android.frameworks.coretests.aidl.IAppOpsUserClient
 import com.android.frameworks.coretests.aidl.IAppOpsUserService
@@ -71,6 +73,7 @@ class AppOpsUserService : Service() {
     override fun onBind(intent: Intent?): IBinder {
         return object : IAppOpsUserService.Stub() {
             private val appOpsManager = getSystemService(AppOpsManager::class.java)!!
+            private val handler = Handler(Looper.getMainLooper())
 
             // Collected note-op calls inside of this process
             private val noted = mutableListOf<Pair<SyncNotedAppOp, Array<StackTraceElement>>>()
@@ -182,6 +185,18 @@ class AppOpsUserService : Service() {
                 }
             }
 
+            override fun callFreezeAndNoteSyncOp(client: IAppOpsUserClient) {
+                handler.post {
+                    client.freezeAndNoteSyncOp()
+                }
+            }
+
+            override fun assertEmptyAsyncNoted() {
+                forwardThrowableFrom {
+                    assertThat(asyncNoted).isEmpty()
+                }
+            }
+
             override fun callApiThatNotesAsyncOpNativelyAndCheckCustomMessage(
                 client: IAppOpsUserClient
             ) {
diff --git a/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserClient.aidl b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserClient.aidl
index 68b393c0ced2..11300b08df5e 100644
--- a/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserClient.aidl
+++ b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserClient.aidl
@@ -20,6 +20,7 @@ interface IAppOpsUserClient {
     void noteSyncOpNative();
     void noteNonPermissionSyncOpNative();
     oneway void noteSyncOpOnewayNative();
+    void freezeAndNoteSyncOp();
     void noteSyncOpOtherUidNative();
     void noteAsyncOpNative();
     void noteAsyncOpNativeWithCustomMessage();
diff --git a/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserService.aidl b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserService.aidl
index f5673c43df09..c6dc7b0621c4 100644
--- a/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserService.aidl
+++ b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/IAppOpsUserService.aidl
@@ -25,4 +25,6 @@ interface IAppOpsUserService {
     void callApiThatNotesSyncOpOtherUidNativelyAndCheckLog(in IAppOpsUserClient client);
     void callApiThatNotesAsyncOpNativelyAndCheckCustomMessage(in IAppOpsUserClient client);
     void callApiThatNotesAsyncOpNativelyAndCheckLog(in IAppOpsUserClient client);
+    void callFreezeAndNoteSyncOp(in IAppOpsUserClient client);
+    void assertEmptyAsyncNoted();
 }
diff --git a/core/tests/coretests/src/android/app/AppOpsLoggingTest.kt b/core/tests/coretests/src/android/app/AppOpsLoggingTest.kt
index a10d6a9e0823..ff4e532a272c 100644
--- a/core/tests/coretests/src/android/app/AppOpsLoggingTest.kt
+++ b/core/tests/coretests/src/android/app/AppOpsLoggingTest.kt
@@ -33,6 +33,9 @@ import android.os.IBinder
 import android.os.Looper
 import android.os.Process
 import android.platform.test.annotations.AppModeFull
+import android.platform.test.annotations.RequiresFlagsEnabled
+import android.platform.test.flag.junit.CheckFlagsRule
+import android.platform.test.flag.junit.DeviceFlagsValueProvider
 import android.util.Log
 import androidx.test.platform.app.InstrumentationRegistry
 import com.android.frameworks.coretests.aidl.IAppOpsUserClient
@@ -41,9 +44,15 @@ import com.google.common.truth.Truth.assertThat
 import org.junit.After
 import org.junit.Assert.fail
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import java.util.concurrent.CompletableFuture
+import java.util.concurrent.LinkedBlockingQueue
 import java.util.concurrent.TimeUnit.MILLISECONDS
+import java.util.concurrent.TimeUnit
+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.TimeSource
 
 private const val LOG_TAG = "AppOpsLoggingTest"
 
@@ -71,8 +80,11 @@ class AppOpsLoggingTest {
 
     private var wasLocationEnabled = false
 
+    @get:Rule val checkFlagsRule: CheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule()
+
     private lateinit var testService: IAppOpsUserService
     private lateinit var serviceConnection: ServiceConnection
+    private lateinit var freezingTestCompletion: CompletableFuture<Unit>
 
     // Collected note-op calls inside of this process
     private val noted = mutableListOf<Pair<SyncNotedAppOp, Array<StackTraceElement>>>()
@@ -123,6 +135,7 @@ class AppOpsLoggingTest {
 
         context.bindService(serviceIntent, serviceConnection, BIND_AUTO_CREATE)
         testService = newService.get(TIMEOUT_MILLIS, MILLISECONDS)
+        freezingTestCompletion = CompletableFuture<Unit>()
     }
 
     private fun clearCollectedNotedOps() {
@@ -253,6 +266,26 @@ class AppOpsLoggingTest {
         }
     }
 
+    @Test
+    @RequiresFlagsEnabled(android.os.Flags.FLAG_BINDER_FROZEN_STATE_CHANGE_CALLBACK,
+            android.permission.flags.Flags.FLAG_USE_FROZEN_AWARE_REMOTE_CALLBACK_LIST)
+    fun dropAsyncOpNotedWhenFrozen() {
+        // Here's what the test does:
+        // 1. AppOpsLoggingTest calls AppOpsUserService
+        // 2. AppOpsUserService calls freezeAndNoteSyncOp in AppOpsLoggingTest
+        // 3. freezeAndNoteSyncOp freezes AppOpsUserService
+        // 4. freezeAndNoteSyncOp calls nativeNoteOp which leads to an async op noted callback
+        // 5. AppOpsService is expected to drop the callback (via RemoteCallbackList) since
+        //    AppOpsUserService is frozen
+        // 6. freezeAndNoteSyncOp unfreezes AppOpsUserService
+        // 7. AppOpsLoggingTest calls AppOpsUserService.assertEmptyAsyncNoted
+        rethrowThrowableFrom {
+            testService.callFreezeAndNoteSyncOp(AppOpsUserClient(context))
+            freezingTestCompletion.get()
+            testService.assertEmptyAsyncNoted()
+        }
+    }
+
     @After
     fun removeNotedAppOpsCollector() {
         appOpsManager.setOnOpNotedCallback(null, null)
@@ -263,6 +296,20 @@ class AppOpsLoggingTest {
         context.unbindService(serviceConnection)
     }
 
+    fun <T> waitForState(queue: LinkedBlockingQueue<T>, state: T, duration: Duration): T? {
+        val timeSource = TimeSource.Monotonic
+        val start = timeSource.markNow()
+        var remaining = duration
+        while (remaining.inWholeMilliseconds > 0) {
+            val v = queue.poll(remaining.inWholeMilliseconds, TimeUnit.MILLISECONDS)
+            if (v == state) {
+                return v
+            }
+            remaining -= timeSource.markNow() - start
+        }
+        return null
+    }
+
     private inner class AppOpsUserClient(
         context: Context
     ) : IAppOpsUserClient.Stub() {
@@ -285,6 +332,31 @@ class AppOpsLoggingTest {
             nativeNoteOp(strOpToOp(OPSTR_COARSE_LOCATION), Binder.getCallingUid(), TEST_SERVICE_PKG)
         }
 
+        override fun freezeAndNoteSyncOp() {
+            handler.post {
+                var stateChanges = LinkedBlockingQueue<Int>()
+                // Leave some time for any pending binder transactions to complete.
+                //
+                // TODO(327047060) Remove this sleep and instead make am freeze wait for binder
+                // transactions to complete
+                Thread.sleep(1000)
+                testService.asBinder().addFrozenStateChangeCallback {
+                    _, state -> stateChanges.put(state)
+                }
+                InstrumentationRegistry.getInstrumentation().uiAutomation
+                    .executeShellCommand("am freeze $TEST_SERVICE_PKG")
+                waitForState(stateChanges, IBinder.FrozenStateChangeCallback.STATE_FROZEN,
+                    1000.milliseconds)
+                nativeNoteOp(strOpToOp(OPSTR_COARSE_LOCATION), Binder.getCallingUid(),
+                    TEST_SERVICE_PKG)
+                InstrumentationRegistry.getInstrumentation().uiAutomation
+                    .executeShellCommand("am unfreeze $TEST_SERVICE_PKG")
+                waitForState(stateChanges, IBinder.FrozenStateChangeCallback.STATE_UNFROZEN,
+                    1000.milliseconds)
+                freezingTestCompletion.complete(Unit)
+            }
+        }
+
         override fun noteSyncOpOtherUidNative() {
             nativeNoteOp(strOpToOp(OPSTR_COARSE_LOCATION), myUid, myPackage)
         }
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index b4cce7da4416..03afaeaf4fae 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -70,8 +70,10 @@ import static android.content.Intent.ACTION_PACKAGE_REMOVED;
 import static android.content.Intent.EXTRA_REPLACING;
 import static android.content.pm.PermissionInfo.PROTECTION_DANGEROUS;
 import static android.content.pm.PermissionInfo.PROTECTION_FLAG_APPOP;
-import static android.permission.flags.Flags.deviceAwareAppOpNewSchemaEnabled;
+import static android.os.Flags.binderFrozenStateChangeCallback;
 import static android.permission.flags.Flags.checkOpValidatePackage;
+import static android.permission.flags.Flags.deviceAwareAppOpNewSchemaEnabled;
+import static android.permission.flags.Flags.useFrozenAwareRemoteCallbackList;
 
 import static com.android.internal.util.FrameworkStatsLog.APP_OP_NOTE_OP_OR_CHECK_OP_BINDER_API_CALLED;
 import static com.android.internal.util.FrameworkStatsLog.APP_OP_NOTE_OP_OR_CHECK_OP_BINDER_API_CALLED__BINDER_API__CHECK_OPERATION;
@@ -3543,20 +3545,23 @@ public class AppOpsService extends IAppOpsService.Stub {
 
         synchronized (this) {
             RemoteCallbackList<IAppOpsAsyncNotedCallback> callbacks = mAsyncOpWatchers.get(key);
+            if (callbacks == null && binderFrozenStateChangeCallback()
+                    && useFrozenAwareRemoteCallbackList()) {
+                callbacks = new RemoteCallbackList.Builder<IAppOpsAsyncNotedCallback>(
+                        RemoteCallbackList.FROZEN_CALLEE_POLICY_DROP)
+                        .setInterfaceDiedCallback((rcl, cb, cookie) ->
+                            stopWatchingAsyncNoted(packageName, callback)
+                        ).build();
+            }
             if (callbacks == null) {
                 callbacks = new RemoteCallbackList<IAppOpsAsyncNotedCallback>() {
-                    @Override
-                    public void onCallbackDied(IAppOpsAsyncNotedCallback callback) {
-                        synchronized (AppOpsService.this) {
-                            if (getRegisteredCallbackCount() == 0) {
-                                mAsyncOpWatchers.remove(key);
-                            }
+                        @Override
+                        public void onCallbackDied(IAppOpsAsyncNotedCallback cb) {
+                            stopWatchingAsyncNoted(packageName, callback);
                         }
-                    }
-                };
-                mAsyncOpWatchers.put(key, callbacks);
+                    };
             }
-
+            mAsyncOpWatchers.put(key, callbacks);
             callbacks.register(callback);
         }
     }