audio: Add permission update barrier for tests

To avoid test flakiness, we need the equivalent of purgePermissionCache
for the new permission sync scheme: tests need to block until the
permission state change they trigger via shell perms is visible to the
audio system.

Migrate the permission update tasks to use an executor service, and keep
track of futures which are scheduled on it, allowing us to block until
these futures are complete. This also allows for true task conflation,
which enables dropping the delay duration lower. Move listener start to
later in the boot sequence for performance.

Test: perfetto for performance and ensuring dispatch
Test: atest CtsMediaAudioTestCases
Bug: 338089555
Flag: com.android.media.audio.audioserver_permissions
Change-Id: I56dadf11ad397896bd5f7757e6b12eb173d6e984

7 files changed, 170 insertions, 31 deletions

diff --git a/core/api/test-current.txt b/core/api/test-current.txt
index 0db91f5667ee..f7ac9459376b 100644
--- a/core/api/test-current.txt
+++ b/core/api/test-current.txt
@@ -1992,6 +1992,7 @@ package android.media {
     method @RequiresPermission(anyOf={android.Manifest.permission.MODIFY_AUDIO_ROUTING, android.Manifest.permission.QUERY_AUDIO_STATE, android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED}) public boolean isFullVolumeDevice();
     method @RequiresPermission(android.Manifest.permission.CALL_AUDIO_INTERCEPTION) public boolean isPstnCallAudioInterceptable();
     method @RequiresPermission(android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED) public boolean isVolumeControlUsingVolumeGroups();
+    method public void permissionUpdateBarrier();
     method @RequiresPermission("android.permission.QUERY_AUDIO_STATE") public int requestAudioFocusForTest(@NonNull android.media.AudioFocusRequest, @NonNull String, int, int);
     method @RequiresPermission(android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED) public void setCsd(float);
     method @RequiresPermission(android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED) public void setNotifAliasRingForTest(boolean);
diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java
index c36eda908d32..c6305757b594 100644
--- a/media/java/android/media/AudioManager.java
+++ b/media/java/android/media/AudioManager.java
@@ -10128,6 +10128,24 @@ public class AudioManager {
 
     /**
      * @hide
+     * Blocks until permission updates have propagated through the audio system.
+     * Only useful in tests, where adoptShellPermissions can change the permission state of
+     * an app without the app being killed.
+     */
+    @TestApi
+    @SuppressWarnings("UnflaggedApi") // @TestApi without associated feature.
+    public void permissionUpdateBarrier() {
+        final IAudioService service = getService();
+        try {
+            service.permissionUpdateBarrier();
+        } catch (RemoteException e) {
+            throw e.rethrowFromSystemServer();
+        }
+    }
+
+
+    /**
+     * @hide
      * Return the list of independent stream types for volume control.
      * A stream type is considered independent when the volume changes of that type do not
      * affect any other independent volume control stream type.
diff --git a/media/java/android/media/IAudioService.aidl b/media/java/android/media/IAudioService.aidl
index d20b7f090e92..e0c346100d03 100644
--- a/media/java/android/media/IAudioService.aidl
+++ b/media/java/android/media/IAudioService.aidl
@@ -101,6 +101,8 @@ interface IAudioService {
 
     oneway void portEvent(in int portId, in int event, in @nullable PersistableBundle extras);
 
+    void permissionUpdateBarrier();
+
     // Java-only methods below.
     void adjustStreamVolume(int streamType, int direction, int flags, String callingPackage);
 
diff --git a/media/tests/mediatestutils/Android.bp b/media/tests/mediatestutils/Android.bp
index 88938e2f71d0..8c68f210ce3d 100644
--- a/media/tests/mediatestutils/Android.bp
+++ b/media/tests/mediatestutils/Android.bp
@@ -27,9 +27,11 @@ java_library {
     name: "mediatestutils",
     srcs: [
         "java/com/android/media/mediatestutils/TestUtils.java",
+        "java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java",
     ],
     static_libs: [
         "androidx.concurrent_concurrent-futures",
+        "androidx.test.runner",
         "guava",
         "mediatestutils_host",
     ],
diff --git a/media/tests/mediatestutils/java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java b/media/tests/mediatestutils/java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java
new file mode 100644
index 000000000000..c51b5dea9546
--- /dev/null
+++ b/media/tests/mediatestutils/java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.media.mediatestutils;
+
+import android.content.Context;
+import android.media.AudioManager;
+
+import androidx.test.platform.app.InstrumentationRegistry;
+
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+/**
+ * Barrier to wait for permission updates to propagate to audioserver, to avoid flakiness when using
+ * {@code com.android.compatability.common.util.AdoptShellPermissionsRule}. Note, this rule should
+ * <b> always </b> be placed after the adopt permission rule. Don't use rule when changing
+ * permission state in {@code @Before}, since that executes after all rules.
+ */
+public class PermissionUpdateBarrierRule implements TestRule {
+
+    private final Context mContext;
+
+    /**
+     * @param context the context to use
+     */
+    public PermissionUpdateBarrierRule(Context context) {
+        mContext = context;
+    }
+
+    public PermissionUpdateBarrierRule() {
+        this(InstrumentationRegistry.getInstrumentation().getContext());
+    }
+
+    @Override
+    public Statement apply(Statement base, Description description) {
+        return new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                mContext.getSystemService(AudioManager.class).permissionUpdateBarrier();
+                base.evaluate();
+            }
+        };
+    }
+}
diff --git a/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java b/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java
index c5180afcce7d..5283eddd90fb 100644
--- a/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java
+++ b/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java
@@ -33,6 +33,7 @@ import static android.Manifest.permission.WRITE_SECURE_SETTINGS;
 
 import android.annotation.Nullable;
 import android.os.RemoteException;
+import android.os.Trace;
 import android.os.UserHandle;
 import android.util.ArraySet;
 import android.util.IntArray;
@@ -190,6 +191,7 @@ public class AudioServerPermissionProvider {
                 mIsUpdateDeferred = true;
                 return;
             }
+            Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "audioserver_permission_update");
             try {
                 for (byte i = 0; i < PermissionEnum.ENUM_SIZE; i++) {
                     var newPerms = getUidsHoldingPerm(i);
@@ -203,6 +205,8 @@ public class AudioServerPermissionProvider {
                 mDest = null;
                 // We didn't necessarily finish
                 mIsUpdateDeferred = true;
+            } finally {
+                Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);
             }
         }
     }
diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index 53b04df4652e..930fb397a5a2 100644
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -284,11 +284,16 @@ import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BooleanSupplier;
 import java.util.stream.Collectors;
 
@@ -785,6 +790,8 @@ public class AudioService extends IAudioService.Stub
     private final BroadcastReceiver mReceiver = new AudioServiceBroadcastReceiver();
 
     private final Executor mAudioServerLifecycleExecutor;
+    private final ConcurrentLinkedQueue<Future> mScheduledPermissionTasks =
+            new ConcurrentLinkedQueue();
 
     private IMediaProjectionManager mProjectionService; // to validate projection token
 
@@ -1092,7 +1099,8 @@ public class AudioService extends IAudioService.Stub
 
         public Lifecycle(Context context) {
             super(context);
-            var audioserverLifecycleExecutor = Executors.newSingleThreadExecutor();
+            var audioserverLifecycleExecutor = Executors.newSingleThreadScheduledExecutor(
+                    (Runnable r) -> new Thread(r, "audioserver_lifecycle"));
             var audioPolicyFacade = new DefaultAudioPolicyFacade(audioserverLifecycleExecutor);
             mService = new AudioService(context,
                               AudioSystemAdapter.getDefaultAdapter(),
@@ -1222,34 +1230,6 @@ public class AudioService extends IAudioService.Stub
         mBroadcastHandlerThread = new HandlerThread("AudioService Broadcast");
         mBroadcastHandlerThread.start();
 
-        // Listen to permission invalidations for the PermissionProvider
-        if (audioserverPermissions()) {
-            final Handler broadcastHandler = mBroadcastHandlerThread.getThreadHandler();
-            mAudioSystem.listenForSystemPropertyChange(PermissionManager.CACHE_KEY_PACKAGE_INFO,
-                    new Runnable() {
-                        // Roughly chosen to be long enough to suppress the autocork behavior
-                        // of the permission cache (50ms), and longer than the task could reasonably
-                        // take, even with many packages and users, while not introducing visible
-                        // permission leaks - since the app needs to restart, and trigger an action
-                        // which requires permissions from audioserver before this delay.
-                        // For RECORD_AUDIO, we are additionally protected by appops.
-                        final long UPDATE_DELAY_MS = 110;
-                        final AtomicLong scheduledUpdateTimestamp = new AtomicLong(0);
-                        @Override
-                        public void run() {
-                            var currentTime = SystemClock.uptimeMillis();
-                            if (currentTime > scheduledUpdateTimestamp.get()) {
-                                scheduledUpdateTimestamp.set(currentTime + UPDATE_DELAY_MS);
-                                broadcastHandler.postAtTime( () ->
-                                        mAudioServerLifecycleExecutor.execute(mPermissionProvider
-                                            ::onPermissionStateChanged),
-                                        currentTime + UPDATE_DELAY_MS
-                                    );
-                            }
-                        }
-            });
-        }
-
         mDeviceBroker = new AudioDeviceBroker(mContext, this, mAudioSystem);
 
         mIsSingleVolume = AudioSystem.isSingleVolume(context);
@@ -1717,8 +1697,10 @@ public class AudioService extends IAudioService.Stub
 
     public void onSystemReady() {
         mSystemReady = true;
+        if (audioserverPermissions()) {
+            setupPermissionListener();
+        }
         scheduleLoadSoundEffects();
-
         mDeviceBroker.onSystemReady();
 
         if (mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_HDMI_CEC)) {
@@ -10608,6 +10590,63 @@ public class AudioService extends IAudioService.Stub
         }
     }
 
+    /* Listen to permission invalidations for the PermissionProvider */
+    private void setupPermissionListener() {
+        // Roughly chosen to be long enough to suppress the autocork behavior of the permission
+        // cache (50ms), while not introducing visible permission leaks - since the app needs to
+        // restart, and trigger an action which requires permissions from audioserver before this
+        // delay. For RECORD_AUDIO, we are additionally protected by appops.
+        final long UPDATE_DELAY_MS = 60;
+        // instanceof to simplify the construction requirements of AudioService for testing: no
+        // delayed execution during unit tests.
+        if (mAudioServerLifecycleExecutor instanceof ScheduledExecutorService exec) {
+            // We schedule and add from a this callback thread only (serially), so the task order on
+            // the serial executor matches the order on the task list.  This list should almost
+            // always have only two elements, except in cases of serious system contention.
+            Runnable task = () -> mScheduledPermissionTasks.add(exec.schedule(() -> {
+                    try {
+                        // Clean up completed tasks before us to bound the queue length.  Cancel any
+                        // pending permission refresh tasks, after our own, since we are about to
+                        // fulfill all of them.  We must be the first non-completed task in the
+                        // queue, since the execution order matches the queue order.  Note, this
+                        // task is the only writer on elements in the queue, and the task is
+                        // serialized, so
+                        //  => no in-flight cancellation
+                        //  => exists at least one non-completed task (ourselves)
+                        //  => the queue is non-empty (only completed tasks removed)
+                        final var iter = mScheduledPermissionTasks.iterator();
+                        while (iter.next().isDone()) {
+                            iter.remove();
+                        }
+                        // iter is on the first element which is not completed (us)
+                        while (iter.hasNext()) {
+                            if (!iter.next().cancel(false)) {
+                                throw new AssertionError(
+                                        "Cancel should be infallible since we" +
+                                        "cancel from the executor");
+                            }
+                            iter.remove();
+                        }
+                        mPermissionProvider.onPermissionStateChanged();
+                    } catch (Exception e) {
+                        // Handle executor routing exceptions to nowhere
+                        Thread.getDefaultUncaughtExceptionHandler()
+                                .uncaughtException(Thread.currentThread(), e);
+                    }
+                },
+                UPDATE_DELAY_MS,
+                TimeUnit.MILLISECONDS));
+            mAudioSystem.listenForSystemPropertyChange(
+                    PermissionManager.CACHE_KEY_PACKAGE_INFO,
+                    task);
+            task.run();
+        } else {
+            mAudioSystem.listenForSystemPropertyChange(
+                    PermissionManager.CACHE_KEY_PACKAGE_INFO,
+                    () -> mAudioServerLifecycleExecutor.execute(
+                                mPermissionProvider::onPermissionStateChanged));
+        }
+    }
 
     //==========================================================================================
     // Audio Focus
@@ -14663,6 +14702,20 @@ public class AudioService extends IAudioService.Stub
         return activeAssistantUids;
     }
 
+    @Override
+    /** @see AudioManager#permissionUpdateBarrier() */
+    public void permissionUpdateBarrier() {
+        for (var x : List.copyOf(mScheduledPermissionTasks)) {
+            try {
+                x.get();
+            } catch (CancellationException e) {
+                // Task completed
+            } catch (InterruptedException | ExecutionException e) {
+                Log.wtf(TAG, "Exception which should never occur", e);
+            }
+        }
+    }
+
     List<String> getDeviceIdentityAddresses(AudioDeviceAttributes device) {
         return mDeviceBroker.getDeviceIdentityAddresses(device);
     }