Rename cancelRequest API to cancelStateRequest and remove the

request parameter

With the change to only having one override request at a time,
we can simplify the cancelRequest API in DeviceStateManager
to not require a request object and have the current
override request canceled. This API is still gated
around having the CONTROL_DEVICE_STATE permission or being the
top level app.

Bug: 215400374
Test: DeviceStateManagerGlobalTest, DeviceStateManagerServiceTest &&
 OverrideRequestControllerTest
Change-Id: I565b608372c21384d401db502b16553ed85b86ee

11 files changed, 66 insertions, 95 deletions

diff --git a/core/api/test-current.txt b/core/api/test-current.txt
index 39e12f40a79d..e8ed9138bbe2 100644
--- a/core/api/test-current.txt
+++ b/core/api/test-current.txt
@@ -1092,7 +1092,7 @@ package android.hardware.camera2 {
 package android.hardware.devicestate {
 
   public final class DeviceStateManager {
-    method @RequiresPermission(value=android.Manifest.permission.CONTROL_DEVICE_STATE, conditional=true) public void cancelRequest(@NonNull android.hardware.devicestate.DeviceStateRequest);
+    method @RequiresPermission(value=android.Manifest.permission.CONTROL_DEVICE_STATE, conditional=true) public void cancelStateRequest();
     method @NonNull public int[] getSupportedStates();
     method public void registerCallback(@NonNull java.util.concurrent.Executor, @NonNull android.hardware.devicestate.DeviceStateManager.DeviceStateCallback);
     method @RequiresPermission(value=android.Manifest.permission.CONTROL_DEVICE_STATE, conditional=true) public void requestState(@NonNull android.hardware.devicestate.DeviceStateRequest, @Nullable java.util.concurrent.Executor, @Nullable android.hardware.devicestate.DeviceStateRequest.Callback);
diff --git a/core/java/android/hardware/devicestate/DeviceStateManager.java b/core/java/android/hardware/devicestate/DeviceStateManager.java
index b06d076fe08e..30aa4db938da 100644
--- a/core/java/android/hardware/devicestate/DeviceStateManager.java
+++ b/core/java/android/hardware/devicestate/DeviceStateManager.java
@@ -79,9 +79,9 @@ public final class DeviceStateManager {
      * <ul>
      *     <li>The system deems the request can no longer be honored, for example if the requested
      *     state becomes unsupported.
-     *     <li>A call to {@link #cancelRequest(DeviceStateRequest)}.
+     *     <li>A call to {@link #cancelStateRequest}.
      *     <li>Another processes submits a request succeeding this request in which case the request
-     *     will be suspended until the interrupting request is canceled.
+     *     will be canceled.
      * </ul>
      * However, this behavior can be changed by setting flags on the {@link DeviceStateRequest}.
      *
@@ -100,19 +100,18 @@ public final class DeviceStateManager {
     }
 
     /**
-     * Cancels a {@link DeviceStateRequest request} previously submitted with a call to
+     * Cancels the active {@link DeviceStateRequest} previously submitted with a call to
      * {@link #requestState(DeviceStateRequest, Executor, DeviceStateRequest.Callback)}.
      * <p>
-     * This method is noop if the {@code request} has not been submitted with a call to
-     * {@link #requestState(DeviceStateRequest, Executor, DeviceStateRequest.Callback)}.
+     * This method is noop if there is no request currently active.
      *
      * @throws SecurityException if the caller is neither the current top-focused activity nor if
      * the {@link android.Manifest.permission#CONTROL_DEVICE_STATE} permission is held.
      */
     @RequiresPermission(value = android.Manifest.permission.CONTROL_DEVICE_STATE,
             conditional = true)
-    public void cancelRequest(@NonNull DeviceStateRequest request) {
-        mGlobal.cancelRequest(request);
+    public void cancelStateRequest() {
+        mGlobal.cancelStateRequest();
     }
 
     /**
diff --git a/core/java/android/hardware/devicestate/DeviceStateManagerGlobal.java b/core/java/android/hardware/devicestate/DeviceStateManagerGlobal.java
index 44b27ec686d8..aba538f51043 100644
--- a/core/java/android/hardware/devicestate/DeviceStateManagerGlobal.java
+++ b/core/java/android/hardware/devicestate/DeviceStateManagerGlobal.java
@@ -151,20 +151,14 @@ public final class DeviceStateManagerGlobal {
      * Cancels a {@link DeviceStateRequest request} previously submitted with a call to
      * {@link #requestState(DeviceStateRequest, Executor, DeviceStateRequest.Callback)}.
      *
-     * @see DeviceStateManager#cancelRequest(DeviceStateRequest)
+     * @see DeviceStateManager#cancelStateRequest
      */
-    public void cancelRequest(@NonNull DeviceStateRequest request) {
+    public void cancelStateRequest() {
         synchronized (mLock) {
             registerCallbackIfNeededLocked();
 
-            final IBinder token = findRequestTokenLocked(request);
-            if (token == null) {
-                // This request has not been submitted.
-                return;
-            }
-
             try {
-                mDeviceStateManager.cancelRequest(token);
+                mDeviceStateManager.cancelStateRequest();
             } catch (RemoteException ex) {
                 throw ex.rethrowFromSystemServer();
             }
diff --git a/core/java/android/hardware/devicestate/DeviceStateRequest.java b/core/java/android/hardware/devicestate/DeviceStateRequest.java
index df488d2f6df1..893d765e48da 100644
--- a/core/java/android/hardware/devicestate/DeviceStateRequest.java
+++ b/core/java/android/hardware/devicestate/DeviceStateRequest.java
@@ -32,8 +32,7 @@ import java.util.concurrent.Executor;
  * DeviceStateRequest.Callback)}.
  * <p>
  * By default, the request is kept active until a call to
- * {@link DeviceStateManager#cancelRequest(DeviceStateRequest)} or until one of the following
- * occurs:
+ * {@link DeviceStateManager#cancelStateRequest} or until one of the following occurs:
  * <ul>
  *     <li>Another processes submits a request succeeding this request in which case the request
  *     will be suspended until the interrupting request is canceled.
diff --git a/core/java/android/hardware/devicestate/IDeviceStateManager.aidl b/core/java/android/hardware/devicestate/IDeviceStateManager.aidl
index 14ed03d09fd0..e450e42497a0 100644
--- a/core/java/android/hardware/devicestate/IDeviceStateManager.aidl
+++ b/core/java/android/hardware/devicestate/IDeviceStateManager.aidl
@@ -41,8 +41,9 @@ interface IDeviceStateManager {
      * previously registered with {@link #registerCallback(IDeviceStateManagerCallback)} before a
      * call to this method.
      *
-     * @param token the request token previously registered with
-     *        {@link #requestState(IBinder, int, int)}
+     * @param token the request token provided
+     * @param state the state of device the request is asking for
+     * @param flags any flags that correspond to the request
      *
      * @throws IllegalStateException if a callback has not yet been registered for the calling
      *         process.
@@ -52,14 +53,11 @@ interface IDeviceStateManager {
     void requestState(IBinder token, int state, int flags);
 
     /**
-     * Cancels a request previously submitted with a call to
+     * Cancels the active request previously submitted with a call to
      * {@link #requestState(IBinder, int, int)}.
      *
-     * @param token the request token previously registered with
-     *        {@link #requestState(IBinder, int, int)}
-     *
-     * @throws IllegalStateException if the supplied {@code token} has not been previously
-     *         requested.
+     * @throws IllegalStateException if a callback has not yet been registered for the calling
+     *         process.
      */
-    void cancelRequest(IBinder token);
+    void cancelStateRequest();
 }
diff --git a/core/tests/devicestatetests/src/android/hardware/devicestate/DeviceStateManagerGlobalTest.java b/core/tests/devicestatetests/src/android/hardware/devicestate/DeviceStateManagerGlobalTest.java
index 2aed7a5a041f..4c247427ef8f 100644
--- a/core/tests/devicestatetests/src/android/hardware/devicestate/DeviceStateManagerGlobalTest.java
+++ b/core/tests/devicestatetests/src/android/hardware/devicestate/DeviceStateManagerGlobalTest.java
@@ -38,7 +38,6 @@ import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 import org.mockito.Mockito;
 
-import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -155,7 +154,7 @@ public final class DeviceStateManagerGlobalTest {
         verify(callback).onStateChanged(eq(OTHER_DEVICE_STATE));
         Mockito.reset(callback);
 
-        mDeviceStateManagerGlobal.cancelRequest(request);
+        mDeviceStateManagerGlobal.cancelStateRequest();
 
         verify(callback).onStateChanged(eq(mService.getBaseState()));
     }
@@ -172,7 +171,7 @@ public final class DeviceStateManagerGlobalTest {
         verify(callback).onRequestActivated(eq(request));
         Mockito.reset(callback);
 
-        mDeviceStateManagerGlobal.cancelRequest(request);
+        mDeviceStateManagerGlobal.cancelStateRequest();
 
         verify(callback).onRequestCanceled(eq(request));
     }
@@ -203,13 +202,13 @@ public final class DeviceStateManagerGlobalTest {
 
         private int[] mSupportedStates = new int[] { DEFAULT_DEVICE_STATE, OTHER_DEVICE_STATE };
         private int mBaseState = DEFAULT_DEVICE_STATE;
-        private ArrayList<Request> mRequests = new ArrayList<>();
+        private Request mRequest;
 
         private Set<IDeviceStateManagerCallback> mCallbacks = new HashSet<>();
 
         private DeviceStateInfo getInfo() {
-            final int mergedState = mRequests.isEmpty()
-                    ? mBaseState : mRequests.get(mRequests.size() - 1).state;
+            final int mergedState = mRequest == null
+                    ? mBaseState : mRequest.state;
             return new DeviceStateInfo(mSupportedStates, mBaseState, mergedState);
         }
 
@@ -245,11 +244,10 @@ public final class DeviceStateManagerGlobalTest {
 
         @Override
         public void requestState(IBinder token, int state, int flags) {
-            if (!mRequests.isEmpty()) {
-                final Request topRequest = mRequests.get(mRequests.size() - 1);
+            if (mRequest != null) {
                 for (IDeviceStateManagerCallback callback : mCallbacks) {
                     try {
-                        callback.onRequestCanceled(topRequest.token);
+                        callback.onRequestCanceled(mRequest.token);
                     } catch (RemoteException e) {
                         // Do nothing. Should never happen.
                     }
@@ -257,7 +255,7 @@ public final class DeviceStateManagerGlobalTest {
             }
 
             final Request request = new Request(token, state, flags);
-            mRequests.add(request);
+            mRequest = request;
             notifyDeviceStateInfoChanged();
 
             for (IDeviceStateManagerCallback callback : mCallbacks) {
@@ -270,20 +268,9 @@ public final class DeviceStateManagerGlobalTest {
         }
 
         @Override
-        public void cancelRequest(IBinder token) {
-            int index = -1;
-            for (int i = 0; i < mRequests.size(); i++) {
-                if (mRequests.get(i).token.equals(token)) {
-                    index = i;
-                    break;
-                }
-            }
-
-            if (index == -1) {
-                throw new IllegalArgumentException("Unknown request: " + token);
-            }
-
-            mRequests.remove(index);
+        public void cancelStateRequest() {
+            IBinder token = mRequest.token;
+            mRequest = null;
             for (IDeviceStateManagerCallback callback : mCallbacks) {
                 try {
                     callback.onRequestCanceled(token);
diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java
index 75583e40a5de..d0e39ccb3214 100644
--- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java
+++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java
@@ -592,15 +592,14 @@ public final class DeviceStateManagerService extends SystemService {
         }
     }
 
-    private void cancelRequestInternal(int callingPid, @NonNull IBinder token) {
+    private void cancelStateRequestInternal(int callingPid) {
         synchronized (mLock) {
             final ProcessRecord processRecord = mProcessRecords.get(callingPid);
             if (processRecord == null) {
                 throw new IllegalStateException("Process " + callingPid
                         + " has no registered callback.");
             }
-
-            mOverrideRequestController.cancelRequest(token);
+            mActiveOverride.ifPresent(mOverrideRequestController::cancelRequest);
         }
     }
 
@@ -625,6 +624,23 @@ public final class DeviceStateManagerService extends SystemService {
         }
     }
 
+    /**
+     * Allow top processes to request or cancel a device state change. If the calling process ID is
+     * not the top app, then check if this process holds the CONTROL_DEVICE_STATE permission.
+     * @param callingPid
+     */
+    private void checkCanControlDeviceState(int callingPid) {
+        // Allow top processes to request a device state change
+        // If the calling process ID is not the top app, then we check if this process
+        // holds a permission to CONTROL_DEVICE_STATE
+        final WindowProcessController topApp = mActivityTaskManagerInternal.getTopApp();
+        if (topApp == null || topApp.getPid() != callingPid) {
+            getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE,
+                    "Permission required to request device state, "
+                            + "or the call must come from the top focused app.");
+        }
+    }
+
     private final class DeviceStateProviderListener implements DeviceStateProvider.Listener {
         @Override
         public void onSupportedDeviceStatesChanged(DeviceState[] newDeviceStates) {
@@ -761,12 +777,7 @@ public final class DeviceStateManagerService extends SystemService {
             // Allow top processes to request a device state change
             // If the calling process ID is not the top app, then we check if this process
             // holds a permission to CONTROL_DEVICE_STATE
-            final WindowProcessController topApp = mActivityTaskManagerInternal.getTopApp();
-            if (topApp.getPid() != callingPid) {
-                getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE,
-                        "Permission required to request device state, "
-                                + "or the call must come from the top focused app.");
-            }
+            checkCanControlDeviceState(callingPid);
 
             if (token == null) {
                 throw new IllegalArgumentException("Request token must not be null.");
@@ -781,25 +792,16 @@ public final class DeviceStateManagerService extends SystemService {
         }
 
         @Override // Binder call
-        public void cancelRequest(IBinder token) {
+        public void cancelStateRequest() {
             final int callingPid = Binder.getCallingPid();
             // Allow top processes to cancel a device state change
             // If the calling process ID is not the top app, then we check if this process
             // holds a permission to CONTROL_DEVICE_STATE
-            final WindowProcessController topApp = mActivityTaskManagerInternal.getTopApp();
-            if (topApp.getPid() != callingPid) {
-                getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE,
-                        "Permission required to cancel device state, "
-                                + "or the call must come from the top focused app.");
-            }
-
-            if (token == null) {
-                throw new IllegalArgumentException("Request token must not be null.");
-            }
+            checkCanControlDeviceState(callingPid);
 
             final long callingIdentity = Binder.clearCallingIdentity();
             try {
-                cancelRequestInternal(callingPid, token);
+                cancelStateRequestInternal(callingPid);
             } finally {
                 Binder.restoreCallingIdentity(callingIdentity);
             }
diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerShellCommand.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerShellCommand.java
index eed68f8b1300..659ee75de453 100644
--- a/services/core/java/com/android/server/devicestate/DeviceStateManagerShellCommand.java
+++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerShellCommand.java
@@ -97,7 +97,7 @@ public class DeviceStateManagerShellCommand extends ShellCommand {
         try {
             if ("reset".equals(nextArg)) {
                 if (sLastRequest != null) {
-                    mClient.cancelRequest(sLastRequest);
+                    mClient.cancelStateRequest();
                     sLastRequest = null;
                 }
             } else {
@@ -105,9 +105,6 @@ public class DeviceStateManagerShellCommand extends ShellCommand {
                 DeviceStateRequest request = DeviceStateRequest.newBuilder(requestedState).build();
 
                 mClient.requestState(request, null /* executor */, null /* callback */);
-                if (sLastRequest != null) {
-                    mClient.cancelRequest(sLastRequest);
-                }
 
                 sLastRequest = request;
             }
diff --git a/services/core/java/com/android/server/devicestate/OverrideRequestController.java b/services/core/java/com/android/server/devicestate/OverrideRequestController.java
index 4f856cc41f39..01f5a09323cf 100644
--- a/services/core/java/com/android/server/devicestate/OverrideRequestController.java
+++ b/services/core/java/com/android/server/devicestate/OverrideRequestController.java
@@ -34,7 +34,7 @@ import java.lang.annotation.RetentionPolicy;
  * <ul>
  *     <li>A new request is added with {@link #addRequest(OverrideRequest)}, in which case the
  *     request will become suspended.</li>
- *     <li>The request is cancelled with {@link #cancelRequest(IBinder)} or as a side effect
+ *     <li>The request is cancelled with {@link #cancelRequest} or as a side effect
  *     of other methods calls, such as {@link #handleProcessDied(int)}.</li>
  * </ul>
  */
@@ -115,9 +115,9 @@ final class OverrideRequestController {
      * Cancels the request with the specified {@code token} and notifies the listener of all changes
      * to request status as a result of this operation.
      */
-    void cancelRequest(@NonNull IBinder token) {
-        // Either don't have an active request or attempting to cancel an already cancelled request
-        if (!hasRequest(token)) {
+    void cancelRequest(@NonNull OverrideRequest request) {
+        // Either don't have a current request or attempting to cancel an already cancelled request
+        if (!hasRequest(request.getToken())) {
             return;
         }
         cancelCurrentRequestLocked();
@@ -136,7 +136,7 @@ final class OverrideRequestController {
     }
 
     /**
-     * Cancels all override requests, this could be due to the device being put
+     * Cancels the current override request, this could be due to the device being put
      * into a hardware state that declares the flag "FLAG_CANCEL_OVERRIDE_REQUESTS"
      */
     void cancelOverrideRequest() {
diff --git a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java
index 1a26dd534090..fe3034dc569d 100644
--- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java
@@ -326,7 +326,7 @@ public final class DeviceStateManagerServiceTest {
         assertEquals(callback.getLastNotifiedInfo().currentState,
                 OTHER_DEVICE_STATE.getIdentifier());
 
-        mService.getBinderService().cancelRequest();
+        mService.getBinderService().cancelStateRequest();
         flushHandler();
 
         assertEquals(callback.getLastNotifiedStatus(token),
@@ -378,9 +378,9 @@ public final class DeviceStateManagerServiceTest {
         mPolicy.resumeConfigureOnce();
         flushHandler();
 
-        // First request status is now suspended as there is another pending request.
+        // First request status is now canceled as there is another pending request.
         assertEquals(callback.getLastNotifiedStatus(firstRequestToken),
-                TestDeviceStateManagerCallback.STATUS_SUSPENDED);
+                TestDeviceStateManagerCallback.STATUS_CANCELED);
         // Second request status still unknown because the service is still awaiting policy
         // callback.
         assertEquals(callback.getLastNotifiedStatus(secondRequestToken),
@@ -402,19 +402,19 @@ public final class DeviceStateManagerServiceTest {
                 DEFAULT_DEVICE_STATE.getIdentifier());
 
         // Now cancel the second request to make the first request active.
-        mService.getBinderService().cancelRequest();
+        mService.getBinderService().cancelStateRequest();
         flushHandler();
 
         assertEquals(callback.getLastNotifiedStatus(firstRequestToken),
-                TestDeviceStateManagerCallback.STATUS_ACTIVE);
+                TestDeviceStateManagerCallback.STATUS_CANCELED);
         assertEquals(callback.getLastNotifiedStatus(secondRequestToken),
                 TestDeviceStateManagerCallback.STATUS_CANCELED);
 
-        assertEquals(mService.getCommittedState(), Optional.of(OTHER_DEVICE_STATE));
+        assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE));
         assertEquals(mService.getPendingState(), Optional.empty());
         assertEquals(mService.getBaseState(), Optional.of(DEFAULT_DEVICE_STATE));
         assertEquals(mPolicy.getMostRecentRequestedStateToConfigure(),
-                OTHER_DEVICE_STATE.getIdentifier());
+                DEFAULT_DEVICE_STATE.getIdentifier());
     }
 
     @Test
@@ -656,11 +656,6 @@ public final class DeviceStateManagerServiceTest {
         }
 
         @Override
-        public void onRequestSuspended(IBinder token) {
-            mLastNotifiedStatus.put(token, STATUS_SUSPENDED);
-        }
-
-        @Override
         public void onRequestCanceled(IBinder token) {
             mLastNotifiedStatus.put(token, STATUS_CANCELED);
         }
diff --git a/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java b/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java
index 1accba368ce6..2297c91818c0 100644
--- a/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java
@@ -91,7 +91,7 @@ public final class OverrideRequestControllerTest {
 
         assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE);
 
-        mController.cancelRequest(firstRequest.getToken());
+        mController.cancelOverrideRequest();
 
         assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_CANCELED);
     }