Merge "Address frozen notification API feedback" into main

7 files changed, 112 insertions, 21 deletions

diff --git a/core/api/current.txt b/core/api/current.txt
index 1f452897c0a2..c96d18d66b50 100644
--- a/core/api/current.txt
+++ b/core/api/current.txt
@@ -33262,7 +33262,7 @@ package android.os {
   }
 
   public interface IBinder {
-    method @FlaggedApi("android.os.binder_frozen_state_change_callback") public default void addFrozenStateChangeCallback(@NonNull android.os.IBinder.FrozenStateChangeCallback) throws android.os.RemoteException;
+    method @FlaggedApi("android.os.binder_frozen_state_change_callback") public default void addFrozenStateChangeCallback(@NonNull java.util.concurrent.Executor, @NonNull android.os.IBinder.FrozenStateChangeCallback) throws android.os.RemoteException;
     method public void dump(@NonNull java.io.FileDescriptor, @Nullable String[]) throws android.os.RemoteException;
     method public void dumpAsync(@NonNull java.io.FileDescriptor, @Nullable String[]) throws android.os.RemoteException;
     method @Nullable public String getInterfaceDescriptor() throws android.os.RemoteException;
@@ -33886,6 +33886,7 @@ package android.os {
     method public void finishBroadcast();
     method public Object getBroadcastCookie(int);
     method public E getBroadcastItem(int);
+    method @FlaggedApi("android.os.binder_frozen_state_change_callback") @Nullable public java.util.concurrent.Executor getExecutor();
     method @FlaggedApi("android.os.binder_frozen_state_change_callback") public int getFrozenCalleePolicy();
     method @FlaggedApi("android.os.binder_frozen_state_change_callback") public int getMaxQueueSize();
     method public Object getRegisteredCallbackCookie(int);
@@ -33906,6 +33907,7 @@ package android.os {
   @FlaggedApi("android.os.binder_frozen_state_change_callback") public static final class RemoteCallbackList.Builder<E extends android.os.IInterface> {
     ctor public RemoteCallbackList.Builder(int);
     method @NonNull public android.os.RemoteCallbackList<E> build();
+    method @NonNull public android.os.RemoteCallbackList.Builder setExecutor(@NonNull java.util.concurrent.Executor);
     method @NonNull public android.os.RemoteCallbackList.Builder setInterfaceDiedCallback(@NonNull android.os.RemoteCallbackList.Builder.InterfaceDiedCallback<E>);
     method @NonNull public android.os.RemoteCallbackList.Builder setMaxQueueSize(int);
   }
diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java
index 3b5a99ed089a..01222cdd38b3 100644
--- a/core/java/android/os/BinderProxy.java
+++ b/core/java/android/os/BinderProxy.java
@@ -36,6 +36,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -651,28 +652,39 @@ public final class BinderProxy implements IBinder {
     private native boolean unlinkToDeathNative(DeathRecipient recipient, int flags);
 
     /**
-     * This list is to hold strong reference to the frozen state callbacks. The callbacks are only
-     * weakly referenced by JNI so the strong references here are needed to keep the callbacks
-     * around until the proxy is GC'ed.
+     * This map is to hold strong reference to the frozen state callbacks.
+     *
+     * The callbacks are only weakly referenced by JNI so the strong references here are needed to
+     * keep the callbacks around until the proxy is GC'ed.
+     *
+     * The key is the original callback passed into {@link #addFrozenStateChangeCallback}. The value
+     * is the wrapped callback created in {@link #addFrozenStateChangeCallback} to dispatch the
+     * calls on the desired executor.
      */
-    private List<FrozenStateChangeCallback> mFrozenStateChangeCallbacks =
-            Collections.synchronizedList(new ArrayList<>());
+    private Map<FrozenStateChangeCallback, FrozenStateChangeCallback> mFrozenStateChangeCallbacks =
+            Collections.synchronizedMap(new HashMap<>());
 
     /**
      * See {@link IBinder#addFrozenStateChangeCallback(FrozenStateChangeCallback)}
      */
-    public void addFrozenStateChangeCallback(FrozenStateChangeCallback callback)
+    public void addFrozenStateChangeCallback(Executor executor, FrozenStateChangeCallback callback)
             throws RemoteException {
-        addFrozenStateChangeCallbackNative(callback);
-        mFrozenStateChangeCallbacks.add(callback);
+        FrozenStateChangeCallback wrappedCallback = (who, state) ->
+                executor.execute(() -> callback.onFrozenStateChanged(who, state));
+        addFrozenStateChangeCallbackNative(wrappedCallback);
+        mFrozenStateChangeCallbacks.put(callback, wrappedCallback);
     }
 
     /**
      * See {@link IBinder#removeFrozenStateChangeCallback}
      */
-    public boolean removeFrozenStateChangeCallback(FrozenStateChangeCallback callback) {
-        mFrozenStateChangeCallbacks.remove(callback);
-        return removeFrozenStateChangeCallbackNative(callback);
+    public boolean removeFrozenStateChangeCallback(FrozenStateChangeCallback callback)
+            throws IllegalArgumentException {
+        FrozenStateChangeCallback wrappedCallback = mFrozenStateChangeCallbacks.remove(callback);
+        if (wrappedCallback == null) {
+            throw new IllegalArgumentException("callback not found");
+        }
+        return removeFrozenStateChangeCallbackNative(wrappedCallback);
     }
 
     private native void addFrozenStateChangeCallbackNative(FrozenStateChangeCallback callback)
diff --git a/core/java/android/os/IBinder.java b/core/java/android/os/IBinder.java
index a997f4c86704..8cfd32449537 100644
--- a/core/java/android/os/IBinder.java
+++ b/core/java/android/os/IBinder.java
@@ -16,6 +16,7 @@
 
 package android.os;
 
+import android.annotation.CallbackExecutor;
 import android.annotation.FlaggedApi;
 import android.annotation.IntDef;
 import android.annotation.NonNull;
@@ -25,6 +26,7 @@ import android.compat.annotation.UnsupportedAppUsage;
 import java.io.FileDescriptor;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.concurrent.Executor;
 
 /**
  * Base interface for a remotable object, the core part of a lightweight
@@ -397,12 +399,31 @@ public interface IBinder {
         @interface State {
         }
 
+        /**
+         * Represents the frozen state of the remote process.
+         *
+         * While in this state, the remote process won't be able to receive and handle a
+         * transaction. Therefore, any asynchronous transactions will be buffered and delivered when
+         * the process is unfrozen, and any synchronous transactions will result in an error.
+         *
+         * Buffered transactions may be stale by the time that the process is unfrozen and handles
+         * them. To avoid overwhelming the remote process with stale events or overflowing their
+         * buffers, it's best to avoid sending binder transactions to a frozen process.
+         */
         int STATE_FROZEN = 0;
+
+        /**
+         * Represents the unfrozen state of the remote process.
+         *
+         * In this state, the process hosting the object can execute and is not restricted
+         * by the freezer from using the CPU or responding to binder transactions.
+         */
         int STATE_UNFROZEN = 1;
 
         /**
          * Interface for receiving a callback when the process hosting an IBinder
          * has changed its frozen state.
+         *
          * @param who The IBinder whose hosting process has changed state.
          * @param state The latest state.
          */
@@ -427,16 +448,32 @@ public interface IBinder {
      * <p>You will only receive state change notifications for remote binders, as local binders by
      * definition can't be frozen without you being frozen too.</p>
      *
+     * @param executor The executor on which to run the callback.
+     * @param callback The callback used to deliver state change notifications.
+     *
      * <p>@throws {@link UnsupportedOperationException} if the kernel binder driver does not support
      * this feature.
      */
     @FlaggedApi(Flags.FLAG_BINDER_FROZEN_STATE_CHANGE_CALLBACK)
-    default void addFrozenStateChangeCallback(@NonNull FrozenStateChangeCallback callback)
+    default void addFrozenStateChangeCallback(
+            @NonNull @CallbackExecutor Executor executor,
+            @NonNull FrozenStateChangeCallback callback)
             throws RemoteException {
         throw new UnsupportedOperationException();
     }
 
     /**
+     * Same as {@link #addFrozenStateChangeCallback(Executor, FrozenStateChangeCallback)} except
+     * that callbacks are invoked on a binder thread.
+     *
+     * @hide
+     */
+    default void addFrozenStateChangeCallback(@NonNull FrozenStateChangeCallback callback)
+            throws RemoteException {
+        addFrozenStateChangeCallback(Runnable::run, callback);
+    }
+
+    /**
      * Unregister a {@link FrozenStateChangeCallback}. The callback will no longer be invoked when
      * the hosting process changes its frozen state.
      */
diff --git a/core/java/android/os/RemoteCallbackList.java b/core/java/android/os/RemoteCallbackList.java
index 91c482faf7d7..d5630fd46eb4 100644
--- a/core/java/android/os/RemoteCallbackList.java
+++ b/core/java/android/os/RemoteCallbackList.java
@@ -16,6 +16,7 @@
 
 package android.os;
 
+import android.annotation.CallbackExecutor;
 import android.annotation.FlaggedApi;
 import android.annotation.IntDef;
 import android.annotation.NonNull;
@@ -29,6 +30,7 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.Executor;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 
@@ -134,6 +136,7 @@ public class RemoteCallbackList<E extends IInterface> {
 
     private final @FrozenCalleePolicy int mFrozenCalleePolicy;
     private final int mMaxQueueSize;
+    private final Executor mExecutor;
 
     private final class Interface implements IBinder.DeathRecipient,
             IBinder.FrozenStateChangeCallback {
@@ -197,7 +200,7 @@ public class RemoteCallbackList<E extends IInterface> {
         void maybeSubscribeToFrozenCallback() throws RemoteException {
             if (mFrozenCalleePolicy != FROZEN_CALLEE_POLICY_UNSET) {
                 try {
-                    mBinder.addFrozenStateChangeCallback(this);
+                    mBinder.addFrozenStateChangeCallback(mExecutor, this);
                 } catch (UnsupportedOperationException e) {
                     // The kernel does not support frozen notifications. In this case we want to
                     // silently fall back to FROZEN_CALLEE_POLICY_UNSET. This is done by simply
@@ -237,6 +240,7 @@ public class RemoteCallbackList<E extends IInterface> {
         private @FrozenCalleePolicy int mFrozenCalleePolicy;
         private int mMaxQueueSize = DEFAULT_MAX_QUEUE_SIZE;
         private InterfaceDiedCallback mInterfaceDiedCallback;
+        private Executor mExecutor;
 
         /**
          * Creates a Builder for {@link RemoteCallbackList}.
@@ -285,6 +289,18 @@ public class RemoteCallbackList<E extends IInterface> {
         }
 
         /**
+         * Sets the executor to be used when invoking callbacks asynchronously.
+         *
+         * This is only used when callbacks need to be invoked asynchronously, e.g. when the process
+         * hosting a callback becomes unfrozen. Callbacks that can be invoked immediately run on the
+         * same thread that calls {@link #broadcast} synchronously.
+         */
+        public @NonNull Builder setExecutor(@NonNull @CallbackExecutor Executor executor) {
+            mExecutor = executor;
+            return this;
+        }
+
+        /**
          * For notifying when the process hosting a callback interface has died.
          *
          * @param <E> The remote callback interface type.
@@ -308,15 +324,21 @@ public class RemoteCallbackList<E extends IInterface> {
          * @return The built {@link RemoteCallbackList} object.
          */
         public @NonNull RemoteCallbackList<E> build() {
+            Executor executor = mExecutor;
+            if (executor == null && mFrozenCalleePolicy != FROZEN_CALLEE_POLICY_UNSET) {
+                // TODO Throw an exception here once the existing API caller is updated to provide
+                // an executor.
+                executor = new HandlerExecutor(Handler.getMain());
+            }
             if (mInterfaceDiedCallback != null) {
-                return new RemoteCallbackList<E>(mFrozenCalleePolicy, mMaxQueueSize) {
+                return new RemoteCallbackList<E>(mFrozenCalleePolicy, mMaxQueueSize, executor) {
                     @Override
                     public void onCallbackDied(E deadInterface, Object cookie) {
                         mInterfaceDiedCallback.onInterfaceDied(this, deadInterface, cookie);
                     }
                 };
             }
-            return new RemoteCallbackList<E>(mFrozenCalleePolicy, mMaxQueueSize);
+            return new RemoteCallbackList<E>(mFrozenCalleePolicy, mMaxQueueSize, executor);
         }
     }
 
@@ -341,13 +363,23 @@ public class RemoteCallbackList<E extends IInterface> {
     }
 
     /**
+     * Returns the executor used when invoking callbacks asynchronously.
+     *
+     * @return The executor.
+     */
+    @FlaggedApi(Flags.FLAG_BINDER_FROZEN_STATE_CHANGE_CALLBACK)
+    public @Nullable Executor getExecutor() {
+        return mExecutor;
+    }
+
+    /**
      * Creates a RemoteCallbackList with {@link #FROZEN_CALLEE_POLICY_UNSET}. This is equivalent to
      * <pre>
      * new RemoteCallbackList.Build(RemoteCallbackList.FROZEN_CALLEE_POLICY_UNSET).build()
      * </pre>
      */
     public RemoteCallbackList() {
-        this(FROZEN_CALLEE_POLICY_UNSET, DEFAULT_MAX_QUEUE_SIZE);
+        this(FROZEN_CALLEE_POLICY_UNSET, DEFAULT_MAX_QUEUE_SIZE, null);
     }
 
     /**
@@ -362,10 +394,14 @@ public class RemoteCallbackList<E extends IInterface> {
      * recipient's process is frozen. Once the limit is reached, the oldest callbacks would be
      * dropped to keep the size under limit. Ignored except for
      * {@link #FROZEN_CALLEE_POLICY_ENQUEUE_ALL}.
+     *
+     * @param executor The executor used when invoking callbacks asynchronously.
      */
-    private RemoteCallbackList(@FrozenCalleePolicy int frozenCalleePolicy, int maxQueueSize) {
+    private RemoteCallbackList(@FrozenCalleePolicy int frozenCalleePolicy, int maxQueueSize,
+            @CallbackExecutor Executor executor) {
         mFrozenCalleePolicy = frozenCalleePolicy;
         mMaxQueueSize = maxQueueSize;
+        mExecutor = executor;
     }
 
     /**
diff --git a/core/tests/coretests/BinderFrozenStateChangeCallbackTestApp/src/com/android/frameworks/coretests/bfscctestapp/BfsccTestAppCmdService.java b/core/tests/coretests/BinderFrozenStateChangeCallbackTestApp/src/com/android/frameworks/coretests/bfscctestapp/BfsccTestAppCmdService.java
index fe54aa8d87f0..945147db1ef5 100644
--- a/core/tests/coretests/BinderFrozenStateChangeCallbackTestApp/src/com/android/frameworks/coretests/bfscctestapp/BfsccTestAppCmdService.java
+++ b/core/tests/coretests/BinderFrozenStateChangeCallbackTestApp/src/com/android/frameworks/coretests/bfscctestapp/BfsccTestAppCmdService.java
@@ -18,6 +18,8 @@ package com.android.frameworks.coretests.bfscctestapp;
 
 import android.app.Service;
 import android.content.Intent;
+import android.os.Handler;
+import android.os.HandlerExecutor;
 import android.os.IBinder;
 import android.os.RemoteException;
 
@@ -36,6 +38,7 @@ public class BfsccTestAppCmdService extends Service {
         @Override
         public void listenTo(IBinder binder) throws RemoteException {
             binder.addFrozenStateChangeCallback(
+                    new HandlerExecutor(Handler.getMain()),
                     (IBinder who, int state) -> mNotifications.offer(state));
         }
 
diff --git a/core/tests/coretests/src/android/os/BinderFrozenStateChangeNotificationTest.java b/core/tests/coretests/src/android/os/BinderFrozenStateChangeNotificationTest.java
index 195a18a5f521..523fe1a8aa5d 100644
--- a/core/tests/coretests/src/android/os/BinderFrozenStateChangeNotificationTest.java
+++ b/core/tests/coretests/src/android/os/BinderFrozenStateChangeNotificationTest.java
@@ -200,7 +200,7 @@ public class BinderFrozenStateChangeNotificationTest {
         IBinder.FrozenStateChangeCallback callback =
                 (IBinder who, int state) -> results.offer(who);
         try {
-            binder.addFrozenStateChangeCallback(callback);
+            binder.addFrozenStateChangeCallback(new HandlerExecutor(Handler.getMain()), callback);
         } catch (UnsupportedOperationException e) {
             return;
         }
@@ -227,7 +227,7 @@ public class BinderFrozenStateChangeNotificationTest {
             final IBinder.FrozenStateChangeCallback callback =
                     (IBinder who, int state) ->
                             queue.offer(state == IBinder.FrozenStateChangeCallback.STATE_FROZEN);
-            binder.addFrozenStateChangeCallback(callback);
+            binder.addFrozenStateChangeCallback(new HandlerExecutor(Handler.getMain()), callback);
             return callback;
         } catch (UnsupportedOperationException e) {
             return null;
diff --git a/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java b/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java
index d0070678d4fa..f888c9ba93a9 100644
--- a/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java
+++ b/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java
@@ -42,6 +42,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.io.FileDescriptor;
+import java.util.concurrent.Executor;
 
 @SmallTest
 @RunWith(AndroidJUnit4.class)
@@ -125,7 +126,7 @@ public class BinderDeathDispatcherTest {
         }
 
         @Override
-        public void addFrozenStateChangeCallback(FrozenStateChangeCallback callback)
+        public void addFrozenStateChangeCallback(Executor e, FrozenStateChangeCallback callback)
                 throws RemoteException {
         }