From a5ab448ff0bfe59b92abe8f5f0864bd0e3429ba4 Mon Sep 17 00:00:00 2001 From: Darryl L Johnson Date: Fri, 2 Jul 2021 11:42:09 -0700 Subject: [PATCH 1/3] Refactor request lifecycle logic into OverrideRequestController. This refactors the request lifecycle login into a controller, OverrideRequestController, which simplies the login within DeviceStateManagerService itself. Bug: 192671286 Test: atest DeviceStateManagerServiceTest Test: atest OverrideRequestControllerTest Test: atest DeviceStateManagerTests Change-Id: Ifaf04665bf8c84d62fa08b3b8883559c6726d6af --- .../DeviceStateManagerService.java | 506 ++++++++---------- .../server/devicestate/OverrideRequest.java | 58 ++ .../OverrideRequestController.java | 290 ++++++++++ .../DeviceStateManagerServiceTest.java | 49 +- .../OverrideRequestControllerTest.java | 204 +++++++ 5 files changed, 812 insertions(+), 295 deletions(-) create mode 100644 services/core/java/com/android/server/devicestate/OverrideRequest.java create mode 100644 services/core/java/com/android/server/devicestate/OverrideRequestController.java create mode 100644 services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index a8b0994402e80..42fd9642c2db7 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -19,8 +19,13 @@ package com.android.server.devicestate; import static android.Manifest.permission.CONTROL_DEVICE_STATE; import static android.hardware.devicestate.DeviceStateManager.MAXIMUM_DEVICE_STATE; import static android.hardware.devicestate.DeviceStateManager.MINIMUM_DEVICE_STATE; -import static android.hardware.devicestate.DeviceStateRequest.FLAG_CANCEL_WHEN_BASE_CHANGES; +import static android.os.Process.THREAD_PRIORITY_DISPLAY; +import static com.android.server.devicestate.OverrideRequestController.STATUS_ACTIVE; +import static com.android.server.devicestate.OverrideRequestController.STATUS_CANCELED; +import static com.android.server.devicestate.OverrideRequestController.STATUS_SUSPENDED; + +import android.annotation.IntDef; import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; @@ -30,12 +35,12 @@ import android.hardware.devicestate.DeviceStateManager; import android.hardware.devicestate.IDeviceStateManager; import android.hardware.devicestate.IDeviceStateManagerCallback; import android.os.Binder; +import android.os.Handler; +import android.os.HandlerThread; import android.os.IBinder; import android.os.RemoteException; import android.os.ResultReceiver; import android.os.ShellCallback; -import android.util.ArrayMap; -import android.util.ArraySet; import android.util.Slog; import android.util.SparseArray; @@ -43,14 +48,18 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.DumpUtils; import com.android.internal.util.FrameworkStatsLog; +import com.android.server.ServiceThread; import com.android.server.SystemService; import com.android.server.policy.DeviceStatePolicyImpl; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.Optional; +import java.util.WeakHashMap; /** * A system service that manages the state of a device with user-configurable hardware like a @@ -81,10 +90,17 @@ public final class DeviceStateManagerService extends SystemService { private static final boolean DEBUG = false; private final Object mLock = new Object(); + // Internal system service thread used to dispatch calls to the policy and to registered + // callbacks though its handler (mHandler). Provides a guarantee of callback order when + // leveraging mHandler and also enables posting messages with the service lock held. + private final HandlerThread mHandlerThread; + private final Handler mHandler; @NonNull private final DeviceStatePolicy mDeviceStatePolicy; @NonNull private final BinderService mBinderService; + @NonNull + private final OverrideRequestController mOverrideRequestController; // All supported device states keyed by identifier. @GuardedBy("mLock") @@ -109,17 +125,16 @@ public final class DeviceStateManagerService extends SystemService { @NonNull private Optional mBaseState = Optional.empty(); + // The current active override request. When set the device state specified here will take + // precedence over mBaseState. + @GuardedBy("mLock") + @NonNull + private Optional mActiveOverride = Optional.empty(); + // List of processes registered to receive notifications about changes to device state and // request status indexed by process id. @GuardedBy("mLock") private final SparseArray mProcessRecords = new SparseArray<>(); - // List of override requests with the highest precedence request at the end. - @GuardedBy("mLock") - private final ArrayList mRequestRecords = new ArrayList<>(); - // Set of override requests that are pending a call to notifyStatusIfNeeded() to be notified - // of a change in status. - @GuardedBy("mLock") - private final ArraySet mRequestsPendingStatusChange = new ArraySet<>(); public DeviceStateManagerService(@NonNull Context context) { this(context, new DeviceStatePolicyImpl(context)); @@ -128,6 +143,13 @@ public final class DeviceStateManagerService extends SystemService { @VisibleForTesting DeviceStateManagerService(@NonNull Context context, @NonNull DeviceStatePolicy policy) { super(context); + // Service thread assigned THREAD_PRIORITY_DISPLAY because this service indirectly drives + // display (on/off) and window (position) events through its callbacks. + mHandlerThread = new ServiceThread(TAG, THREAD_PRIORITY_DISPLAY, false /* allowIo */); + mHandlerThread.start(); + mHandler = mHandlerThread.getThreadHandler(); + mOverrideRequestController = new OverrideRequestController( + this::onOverrideRequestStatusChangedLocked); mDeviceStatePolicy = policy; mDeviceStatePolicy.getDeviceStateProvider().setListener(new DeviceStateProviderListener()); mBinderService = new BinderService(); @@ -138,6 +160,11 @@ public final class DeviceStateManagerService extends SystemService { publishBinderService(Context.DEVICE_STATE_SERVICE, mBinderService); } + @VisibleForTesting + Handler getHandler() { + return mHandler; + } + /** * Returns the current state the system is in. Note that the system may be in the process of * configuring a different state. @@ -191,12 +218,10 @@ public final class DeviceStateManagerService extends SystemService { @NonNull Optional getOverrideState() { synchronized (mLock) { - if (mRequestRecords.isEmpty()) { - return Optional.empty(); + if (mActiveOverride.isPresent()) { + return getStateLocked(mActiveOverride.get().getRequestedState()); } - - OverrideRequestRecord topRequest = mRequestRecords.get(mRequestRecords.size() - 1); - return Optional.of(topRequest.mRequestedState); + return Optional.empty(); } } @@ -247,8 +272,6 @@ public final class DeviceStateManagerService extends SystemService { } private void updateSupportedStates(DeviceState[] supportedDeviceStates) { - boolean updatedPendingState; - boolean hasBaseState; synchronized (mLock) { final int[] oldStateIdentifiers = getSupportedStateIdentifiersLocked(); @@ -263,27 +286,18 @@ public final class DeviceStateManagerService extends SystemService { return; } - final int requestSize = mRequestRecords.size(); - for (int i = 0; i < requestSize; i++) { - OverrideRequestRecord request = mRequestRecords.get(i); - if (!isSupportedStateLocked(request.mRequestedState.getIdentifier())) { - request.setStatusLocked(OverrideRequestRecord.STATUS_CANCELED); - } + mOverrideRequestController.handleNewSupportedStates(newStateIdentifiers); + updatePendingStateLocked(); + + if (!mPendingState.isPresent()) { + // If the change in the supported states didn't result in a change of the pending + // state commitPendingState() will never be called and the callbacks will never be + // notified of the change. + notifyDeviceStateInfoChangedAsync(); } - updatedPendingState = updatePendingStateLocked(); - hasBaseState = mBaseState.isPresent(); + mHandler.post(this::notifyPolicyIfNeeded); } - - if (hasBaseState && !updatedPendingState) { - // If the change in the supported states didn't result in a change of the pending state - // commitPendingState() will never be called and the callbacks will never be notified - // of the change. - notifyDeviceStateInfoChanged(); - } - - notifyRequestsOfStatusChangeIfNeeded(); - notifyPolicyIfNeeded(); } /** @@ -311,7 +325,6 @@ public final class DeviceStateManagerService extends SystemService { * @see #isSupportedStateLocked(int) */ private void setBaseState(int identifier) { - boolean updatedPendingState; synchronized (mLock) { final Optional baseStateOptional = getStateLocked(identifier); if (!baseStateOptional.isPresent()) { @@ -325,26 +338,18 @@ public final class DeviceStateManagerService extends SystemService { } mBaseState = Optional.of(baseState); - final int requestSize = mRequestRecords.size(); - for (int i = 0; i < requestSize; i++) { - OverrideRequestRecord request = mRequestRecords.get(i); - if ((request.mFlags & FLAG_CANCEL_WHEN_BASE_CHANGES) > 0) { - request.setStatusLocked(OverrideRequestRecord.STATUS_CANCELED); - } + mOverrideRequestController.handleBaseStateChanged(); + updatePendingStateLocked(); + + if (!mPendingState.isPresent()) { + // If the change in base state didn't result in a change of the pending state + // commitPendingState() will never be called and the callbacks will never be + // notified of the change. + notifyDeviceStateInfoChangedAsync(); } - updatedPendingState = updatePendingStateLocked(); + mHandler.post(this::notifyPolicyIfNeeded); } - - if (!updatedPendingState) { - // If the change in base state didn't result in a change of the pending state - // commitPendingState() will never be called and the callbacks will never be notified - // of the change. - notifyDeviceStateInfoChanged(); - } - - notifyRequestsOfStatusChangeIfNeeded(); - notifyPolicyIfNeeded(); } /** @@ -362,8 +367,8 @@ public final class DeviceStateManagerService extends SystemService { } final DeviceState stateToConfigure; - if (!mRequestRecords.isEmpty()) { - stateToConfigure = mRequestRecords.get(mRequestRecords.size() - 1).mRequestedState; + if (mActiveOverride.isPresent()) { + stateToConfigure = getStateLocked(mActiveOverride.get().getRequestedState()).get(); } else if (mBaseState.isPresent() && isSupportedStateLocked(mBaseState.get().getIdentifier())) { // Base state could have recently become unsupported after a change in supported states. @@ -429,108 +434,106 @@ public final class DeviceStateManagerService extends SystemService { *

*/ private void commitPendingState() { - // Update the current state. synchronized (mLock) { final DeviceState newState = mPendingState.get(); if (DEBUG) { Slog.d(TAG, "Committing state: " + newState); } - if (!mRequestRecords.isEmpty()) { - final OverrideRequestRecord topRequest = - mRequestRecords.get(mRequestRecords.size() - 1); - if (topRequest.mRequestedState.getIdentifier() == newState.getIdentifier()) { - // The top request could have come in while the service was awaiting callback - // from the policy. In that case we only set it to active if it matches the - // current committed state, otherwise it will be set to active when its - // requested state is committed. - topRequest.setStatusLocked(OverrideRequestRecord.STATUS_ACTIVE); - } - } - FrameworkStatsLog.write(FrameworkStatsLog.DEVICE_STATE_CHANGED, newState.getIdentifier(), !mCommittedState.isPresent()); mCommittedState = Optional.of(newState); mPendingState = Optional.empty(); updatePendingStateLocked(); + + // Notify callbacks of a change. + notifyDeviceStateInfoChangedAsync(); + + // The top request could have come in while the service was awaiting callback + // from the policy. In that case we only set it to active if it matches the + // current committed state, otherwise it will be set to active when its + // requested state is committed. + OverrideRequest activeRequest = mActiveOverride.orElse(null); + if (activeRequest != null + && activeRequest.getRequestedState() == newState.getIdentifier()) { + ProcessRecord processRecord = mProcessRecords.get(activeRequest.getPid()); + if (processRecord != null) { + processRecord.notifyRequestActiveAsync(activeRequest.getToken()); + } + } + + // Try to configure the next state if needed. + mHandler.post(this::notifyPolicyIfNeeded); } - - // Notify callbacks of a change. - notifyDeviceStateInfoChanged(); - - // Notify the top request that it's active. - notifyRequestsOfStatusChangeIfNeeded(); - - // Try to configure the next state if needed. - notifyPolicyIfNeeded(); } - private void notifyDeviceStateInfoChanged() { - if (Thread.holdsLock(mLock)) { - throw new IllegalStateException( - "Attempting to notify callbacks with service lock held."); - } - - // Grab the lock and copy the process records and the current info. - ArrayList registeredProcesses; - DeviceStateInfo info; + private void notifyDeviceStateInfoChangedAsync() { synchronized (mLock) { if (mProcessRecords.size() == 0) { return; } - registeredProcesses = new ArrayList<>(); + ArrayList registeredProcesses = new ArrayList<>(); for (int i = 0; i < mProcessRecords.size(); i++) { registeredProcesses.add(mProcessRecords.valueAt(i)); } - info = getDeviceStateInfoLocked(); - } + DeviceStateInfo info = getDeviceStateInfoLocked(); - // After releasing the lock, send the notifications out. - for (int i = 0; i < registeredProcesses.size(); i++) { - registeredProcesses.get(i).notifyDeviceStateInfoAsync(info); + for (int i = 0; i < registeredProcesses.size(); i++) { + registeredProcesses.get(i).notifyDeviceStateInfoAsync(info); + } } } - /** - * Notifies all dirty requests (requests that have a change in status, but have not yet been - * notified) that their status has changed. - */ - private void notifyRequestsOfStatusChangeIfNeeded() { - if (Thread.holdsLock(mLock)) { - throw new IllegalStateException( - "Attempting to notify requests with service lock held."); - } - - ArraySet dirtyRequests; - synchronized (mLock) { - if (mRequestsPendingStatusChange.isEmpty()) { - return; + private void onOverrideRequestStatusChangedLocked(@NonNull OverrideRequest request, + @OverrideRequestController.RequestStatus int status) { + if (status == STATUS_ACTIVE) { + mActiveOverride = Optional.of(request); + } else if (status == STATUS_SUSPENDED || status == STATUS_CANCELED) { + if (mActiveOverride.isPresent() && mActiveOverride.get() == request) { + mActiveOverride = Optional.empty(); } - - dirtyRequests = new ArraySet<>(mRequestsPendingStatusChange); - mRequestsPendingStatusChange.clear(); + } else { + throw new IllegalArgumentException("Unknown request status: " + status); } - // After releasing the lock, send the notifications out. - for (int i = 0; i < dirtyRequests.size(); i++) { - dirtyRequests.valueAt(i).notifyStatusIfNeeded(); + boolean updatedPendingState = updatePendingStateLocked(); + + ProcessRecord processRecord = mProcessRecords.get(request.getPid()); + if (processRecord == null) { + // If the process is no longer registered with the service, for example if it has died, + // there is no need to notify it of a change in request status. + mHandler.post(this::notifyPolicyIfNeeded); + return; } + + if (status == STATUS_ACTIVE) { + if (!updatedPendingState && !mPendingState.isPresent()) { + // If the pending state was not updated and there is not currently a pending state + // then this newly active request will never be notified of a change in state. + // Schedule the notification now. + processRecord.notifyRequestActiveAsync(request.getToken()); + } + } else if (status == STATUS_SUSPENDED) { + processRecord.notifyRequestSuspendedAsync(request.getToken()); + } else { + processRecord.notifyRequestCanceledAsync(request.getToken()); + } + + mHandler.post(this::notifyPolicyIfNeeded); } private void registerProcess(int pid, IDeviceStateManagerCallback callback) { - DeviceStateInfo currentInfo; - ProcessRecord record; - // Grab the lock to register the callback and get the current state. synchronized (mLock) { if (mProcessRecords.contains(pid)) { throw new SecurityException("The calling process has already registered an" + " IDeviceStateManagerCallback."); } - record = new ProcessRecord(callback, pid); + ProcessRecord record = new ProcessRecord(callback, pid, this::handleProcessDied, + mHandlerThread.getThreadHandler()); try { callback.asBinder().linkToDeath(record, 0); } catch (RemoteException ex) { @@ -538,34 +541,21 @@ public final class DeviceStateManagerService extends SystemService { } mProcessRecords.put(pid, record); - currentInfo = mCommittedState.isPresent() ? getDeviceStateInfoLocked() : null; - } - - if (currentInfo != null) { - // If there is not a committed state we'll wait to notify the process of the initial - // value. - record.notifyDeviceStateInfoAsync(currentInfo); + DeviceStateInfo currentInfo = mCommittedState.isPresent() + ? getDeviceStateInfoLocked() : null; + if (currentInfo != null) { + // If there is not a committed state we'll wait to notify the process of the initial + // value. + record.notifyDeviceStateInfoAsync(currentInfo); + } } } private void handleProcessDied(ProcessRecord processRecord) { synchronized (mLock) { - // Cancel all requests from this process. - final int requestCount = processRecord.mRequestRecords.size(); - for (int i = 0; i < requestCount; i++) { - final OverrideRequestRecord request = processRecord.mRequestRecords.valueAt(i); - // Cancel the request but don't mark it as dirty since there's no need to send - // notifications if the process has died. - request.setStatusLocked(OverrideRequestRecord.STATUS_CANCELED, - false /* markDirty */); - } - mProcessRecords.remove(processRecord.mPid); - - updatePendingStateLocked(); + mOverrideRequestController.handleProcessDied(processRecord.mPid); } - - notifyPolicyIfNeeded(); } private void requestStateInternal(int state, int flags, int callingPid, @@ -577,7 +567,7 @@ public final class DeviceStateManagerService extends SystemService { + " has no registered callback."); } - if (processRecord.mRequestRecords.get(token) != null) { + if (mOverrideRequestController.hasRequest(token)) { throw new IllegalStateException("Request has already been made for the supplied" + " token: " + token); } @@ -588,27 +578,9 @@ public final class DeviceStateManagerService extends SystemService { + " is not supported."); } - OverrideRequestRecord topRecord = mRequestRecords.isEmpty() - ? null : mRequestRecords.get(mRequestRecords.size() - 1); - if (topRecord != null) { - topRecord.setStatusLocked(OverrideRequestRecord.STATUS_SUSPENDED); - } - - final OverrideRequestRecord request = - new OverrideRequestRecord(processRecord, token, deviceState.get(), flags); - mRequestRecords.add(request); - processRecord.mRequestRecords.put(request.mToken, request); - - final boolean updatedPendingState = updatePendingStateLocked(); - if (!updatedPendingState && !mPendingState.isPresent()) { - // We don't set the status of the new request to ACTIVE if the request updated the - // pending state as it will be set in commitPendingState(). - request.setStatusLocked(OverrideRequestRecord.STATUS_ACTIVE, true /* markDirty */); - } + OverrideRequest request = new OverrideRequest(token, callingPid, state, flags); + mOverrideRequestController.addRequest(request); } - - notifyRequestsOfStatusChangeIfNeeded(); - notifyPolicyIfNeeded(); } private void cancelRequestInternal(int callingPid, @NonNull IBinder token) { @@ -619,18 +591,8 @@ public final class DeviceStateManagerService extends SystemService { + " has no registered callback."); } - OverrideRequestRecord request = processRecord.mRequestRecords.get(token); - if (request == null) { - throw new IllegalStateException("No known request for the given token"); - } - - request.setStatusLocked(OverrideRequestRecord.STATUS_CANCELED); - - updatePendingStateLocked(); + mOverrideRequestController.cancelRequest(token); } - - notifyRequestsOfStatusChangeIfNeeded(); - notifyPolicyIfNeeded(); } private void dumpInternal(PrintWriter pw) { @@ -650,16 +612,7 @@ public final class DeviceStateManagerService extends SystemService { pw.println(" " + i + ": mPid=" + processRecord.mPid); } - final int requestCount = mRequestRecords.size(); - pw.println(); - pw.println("Override requests: size=" + requestCount); - for (int i = 0; i < requestCount; i++) { - OverrideRequestRecord requestRecord = mRequestRecords.get(i); - pw.println(" " + i + ": mPid=" + requestRecord.mProcessRecord.mPid - + ", mRequestedState=" + requestRecord.mRequestedState - + ", mFlags=" + requestRecord.mFlags - + ", mStatus=" + requestRecord.statusToString(requestRecord.mStatus)); - } + mOverrideRequestController.dumpInternal(pw); } } @@ -683,142 +636,107 @@ public final class DeviceStateManagerService extends SystemService { } } - private final class ProcessRecord implements IBinder.DeathRecipient { + private static final class ProcessRecord implements IBinder.DeathRecipient { + public interface DeathListener { + void onProcessDied(ProcessRecord record); + } + + private static final int STATUS_ACTIVE = 0; + + private static final int STATUS_SUSPENDED = 1; + + private static final int STATUS_CANCELED = 2; + + @IntDef(prefix = {"STATUS_"}, value = { + STATUS_ACTIVE, + STATUS_SUSPENDED, + STATUS_CANCELED + }) + @Retention(RetentionPolicy.SOURCE) + private @interface RequestStatus {} + private final IDeviceStateManagerCallback mCallback; private final int mPid; + private final DeathListener mDeathListener; + private final Handler mHandler; - private final ArrayMap mRequestRecords = new ArrayMap<>(); + private final WeakHashMap mLastNotifiedStatus = new WeakHashMap<>(); - ProcessRecord(IDeviceStateManagerCallback callback, int pid) { + ProcessRecord(IDeviceStateManagerCallback callback, int pid, DeathListener deathListener, + Handler handler) { mCallback = callback; mPid = pid; + mDeathListener = deathListener; + mHandler = handler; } @Override public void binderDied() { - handleProcessDied(this); + mDeathListener.onProcessDied(this); } public void notifyDeviceStateInfoAsync(@NonNull DeviceStateInfo info) { - try { - mCallback.onDeviceStateInfoChanged(info); - } catch (RemoteException ex) { - Slog.w(TAG, "Failed to notify process " + mPid + " that device state changed.", - ex); - } - } - - public void notifyRequestActiveAsync(OverrideRequestRecord request) { - try { - mCallback.onRequestActive(request.mToken); - } catch (RemoteException ex) { - Slog.w(TAG, "Failed to notify process " + mPid + " that request state changed.", - ex); - } - } - - public void notifyRequestSuspendedAsync(OverrideRequestRecord request) { - try { - mCallback.onRequestSuspended(request.mToken); - } catch (RemoteException ex) { - Slog.w(TAG, "Failed to notify process " + mPid + " that request state changed.", - ex); - } - } - - public void notifyRequestCanceledAsync(OverrideRequestRecord request) { - try { - mCallback.onRequestCanceled(request.mToken); - } catch (RemoteException ex) { - Slog.w(TAG, "Failed to notify process " + mPid + " that request state changed.", - ex); - } - } - } - - /** A record describing a request to override the state of the device. */ - private final class OverrideRequestRecord { - public static final int STATUS_UNKNOWN = 0; - public static final int STATUS_ACTIVE = 1; - public static final int STATUS_SUSPENDED = 2; - public static final int STATUS_CANCELED = 3; - - @Nullable - public String statusToString(int status) { - switch (status) { - case STATUS_ACTIVE: - return "ACTIVE"; - case STATUS_SUSPENDED: - return "SUSPENDED"; - case STATUS_CANCELED: - return "CANCELED"; - case STATUS_UNKNOWN: - return "UNKNOWN"; - default: - return null; - } - } - - private final ProcessRecord mProcessRecord; - @NonNull - private final IBinder mToken; - @NonNull - private final DeviceState mRequestedState; - private final int mFlags; - - private int mStatus = STATUS_UNKNOWN; - private int mLastNotifiedStatus = STATUS_UNKNOWN; - - OverrideRequestRecord(@NonNull ProcessRecord processRecord, @NonNull IBinder token, - @NonNull DeviceState requestedState, int flags) { - mProcessRecord = processRecord; - mToken = token; - mRequestedState = requestedState; - mFlags = flags; - } - - public void setStatusLocked(int status) { - setStatusLocked(status, true /* markDirty */); - } - - public void setStatusLocked(int status, boolean markDirty) { - if (mStatus != status) { - if (mStatus == STATUS_CANCELED) { - throw new IllegalStateException( - "Can not alter the status of a request after set to CANCELED."); + mHandler.post(() -> { + try { + mCallback.onDeviceStateInfoChanged(info); + } catch (RemoteException ex) { + Slog.w(TAG, "Failed to notify process " + mPid + " that device state changed.", + ex); } - - mStatus = status; - - if (mStatus == STATUS_CANCELED) { - mRequestRecords.remove(this); - mProcessRecord.mRequestRecords.remove(mToken); - } - - if (markDirty) { - mRequestsPendingStatusChange.add(this); - } - } + }); } - public void notifyStatusIfNeeded() { - int stateToReport; - synchronized (mLock) { - if (mLastNotifiedStatus == mStatus) { - return; + public void notifyRequestActiveAsync(IBinder token) { + @RequestStatus Integer lastStatus = mLastNotifiedStatus.get(token); + if (lastStatus != null + && (lastStatus == STATUS_ACTIVE || lastStatus == STATUS_CANCELED)) { + return; + } + + mLastNotifiedStatus.put(token, STATUS_ACTIVE); + mHandler.post(() -> { + try { + mCallback.onRequestActive(token); + } catch (RemoteException ex) { + Slog.w(TAG, "Failed to notify process " + mPid + " that request state changed.", + ex); } + }); + } - stateToReport = mStatus; - mLastNotifiedStatus = mStatus; + public void notifyRequestSuspendedAsync(IBinder token) { + @RequestStatus Integer lastStatus = mLastNotifiedStatus.get(token); + if (lastStatus != null + && (lastStatus == STATUS_SUSPENDED || lastStatus == STATUS_CANCELED)) { + return; } - if (stateToReport == STATUS_ACTIVE) { - mProcessRecord.notifyRequestActiveAsync(this); - } else if (stateToReport == STATUS_SUSPENDED) { - mProcessRecord.notifyRequestSuspendedAsync(this); - } else if (stateToReport == STATUS_CANCELED) { - mProcessRecord.notifyRequestCanceledAsync(this); + mLastNotifiedStatus.put(token, STATUS_SUSPENDED); + mHandler.post(() -> { + try { + mCallback.onRequestSuspended(token); + } catch (RemoteException ex) { + Slog.w(TAG, "Failed to notify process " + mPid + " that request state changed.", + ex); + } + }); + } + + public void notifyRequestCanceledAsync(IBinder token) { + @RequestStatus Integer lastStatus = mLastNotifiedStatus.get(token); + if (lastStatus != null && lastStatus == STATUS_CANCELED) { + return; } + + mLastNotifiedStatus.put(token, STATUS_CANCELED); + mHandler.post(() -> { + try { + mCallback.onRequestCanceled(token); + } catch (RemoteException ex) { + Slog.w(TAG, "Failed to notify process " + mPid + " that request state changed.", + ex); + } + }); } } diff --git a/services/core/java/com/android/server/devicestate/OverrideRequest.java b/services/core/java/com/android/server/devicestate/OverrideRequest.java new file mode 100644 index 0000000000000..35a4c844c710a --- /dev/null +++ b/services/core/java/com/android/server/devicestate/OverrideRequest.java @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2021 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.server.devicestate; + +import android.hardware.devicestate.DeviceStateRequest; +import android.os.IBinder; + +/** + * A request to override the state managed by {@link DeviceStateManagerService}. + * + * @see OverrideRequestController + */ +final class OverrideRequest { + private final IBinder mToken; + private final int mPid; + private final int mRequestedState; + @DeviceStateRequest.RequestFlags + private final int mFlags; + + OverrideRequest(IBinder token, int pid, int requestedState, + @DeviceStateRequest.RequestFlags int flags) { + mToken = token; + mPid = pid; + mRequestedState = requestedState; + mFlags = flags; + } + + IBinder getToken() { + return mToken; + } + + int getPid() { + return mPid; + } + + int getRequestedState() { + return mRequestedState; + } + + @DeviceStateRequest.RequestFlags + int getFlags() { + return mFlags; + } +} diff --git a/services/core/java/com/android/server/devicestate/OverrideRequestController.java b/services/core/java/com/android/server/devicestate/OverrideRequestController.java new file mode 100644 index 0000000000000..042bea5fcfa37 --- /dev/null +++ b/services/core/java/com/android/server/devicestate/OverrideRequestController.java @@ -0,0 +1,290 @@ +/* + * Copyright (C) 2021 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.server.devicestate; + +import android.annotation.IntDef; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.hardware.devicestate.DeviceStateRequest; +import android.os.IBinder; + +import java.io.PrintWriter; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.ArrayList; +import java.util.List; + +/** + * Manages the lifecycle of override requests. + *

+ * New requests are added with {@link #addRequest(OverrideRequest)} and are kept active until + * either: + *

    + *
  • A new request is added with {@link #addRequest(OverrideRequest)}, in which case the + * request will become suspended.
    • + *
  • The request is cancelled with {@link #cancelRequest(IBinder)} or as a side effect + * of other methods calls, such as {@link #handleProcessDied(int)}.
    • + *
+ */ +final class OverrideRequestController { + static final int STATUS_UNKNOWN = 0; + /** + * The request is the top-most request. + */ + static final int STATUS_ACTIVE = 1; + /** + * The request is still present but is being superseded by another request. + */ + static final int STATUS_SUSPENDED = 2; + /** + * The request is not longer valid. + */ + static final int STATUS_CANCELED = 3; + + @IntDef(prefix = {"STATUS_"}, value = { + STATUS_UNKNOWN, + STATUS_ACTIVE, + STATUS_SUSPENDED, + STATUS_CANCELED + }) + @Retention(RetentionPolicy.SOURCE) + @interface RequestStatus {} + + static String statusToString(@RequestStatus int status) { + switch (status) { + case STATUS_ACTIVE: + return "ACTIVE"; + case STATUS_SUSPENDED: + return "SUSPENDED"; + case STATUS_CANCELED: + return "CANCELED"; + case STATUS_UNKNOWN: + return "UNKNOWN"; + } + throw new IllegalArgumentException("Unknown status: " + status); + } + + private final StatusChangeListener mListener; + private final List mTmpRequestsToCancel = new ArrayList<>(); + + // List of override requests with the most recent override request at the end. + private final ArrayList mRequests = new ArrayList<>(); + + OverrideRequestController(@NonNull StatusChangeListener listener) { + mListener = listener; + } + + /** + * Adds a request to the top of the stack and notifies the listener of all changes to request + * status as a result of this operation. + */ + void addRequest(@NonNull OverrideRequest request) { + mRequests.add(request); + mListener.onStatusChanged(request, STATUS_ACTIVE); + + if (mRequests.size() > 1) { + OverrideRequest prevRequest = mRequests.get(mRequests.size() - 2); + mListener.onStatusChanged(prevRequest, STATUS_SUSPENDED); + } + } + + /** + * 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) { + int index = getRequestIndex(token); + if (index == -1) { + return; + } + + OverrideRequest request = mRequests.remove(index); + if (index == mRequests.size() && mRequests.size() > 0) { + // We removed the current active request so we need to set the new active request + // before cancelling this request. + OverrideRequest newTop = getLast(mRequests); + mListener.onStatusChanged(newTop, STATUS_ACTIVE); + } + mListener.onStatusChanged(request, STATUS_CANCELED); + } + + /** + * Returns {@code true} if this controller is current managing a request with the specified + * {@code token}, {@code false} otherwise. + */ + boolean hasRequest(@NonNull IBinder token) { + return getRequestIndex(token) != -1; + } + + /** + * Notifies the controller that the process with the specified {@code pid} has died. The + * controller will notify the listener of all changes to request status as a result of this + * operation. + */ + void handleProcessDied(int pid) { + if (mRequests.isEmpty()) { + return; + } + + OverrideRequest prevActiveRequest = getLast(mRequests); + for (OverrideRequest request : mRequests) { + if (request.getPid() == pid) { + mTmpRequestsToCancel.add(request); + } + } + + mRequests.removeAll(mTmpRequestsToCancel); + if (!mRequests.isEmpty()) { + OverrideRequest newActiveRequest = getLast(mRequests); + if (newActiveRequest != prevActiveRequest) { + mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); + } + } + + for (int i = 0; i < mTmpRequestsToCancel.size(); i++) { + mListener.onStatusChanged(mTmpRequestsToCancel.get(i), STATUS_CANCELED); + } + mTmpRequestsToCancel.clear(); + } + + /** + * Notifies the controller that the base state has changed. The controller will notify the + * listener of all changes to request status as a result of this change. + * + * @return {@code true} if calling this method has lead to a new active request, {@code false} + * otherwise. + */ + boolean handleBaseStateChanged() { + if (mRequests.isEmpty()) { + return false; + } + + OverrideRequest prevActiveRequest = getLast(mRequests); + for (int i = 0; i < mRequests.size(); i++) { + OverrideRequest request = mRequests.get(i); + if ((request.getFlags() & DeviceStateRequest.FLAG_CANCEL_WHEN_BASE_CHANGES) != 0) { + mTmpRequestsToCancel.add(request); + } + } + + mRequests.removeAll(mTmpRequestsToCancel); + OverrideRequest newActiveRequest = null; + if (!mRequests.isEmpty()) { + newActiveRequest = getLast(mRequests); + if (newActiveRequest != prevActiveRequest) { + mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); + } + } + + for (int i = 0; i < mTmpRequestsToCancel.size(); i++) { + mListener.onStatusChanged(mTmpRequestsToCancel.get(i), STATUS_CANCELED); + } + mTmpRequestsToCancel.clear(); + + return newActiveRequest != prevActiveRequest; + } + + /** + * Notifies the controller that the set of supported states has changed. The controller will + * notify the listener of all changes to request status as a result of this change. + * + * @return {@code true} if calling this method has lead to a new active request, {@code false} + * otherwise. + */ + boolean handleNewSupportedStates(int[] newSupportedStates) { + if (mRequests.isEmpty()) { + return false; + } + + OverrideRequest prevActiveRequest = getLast(mRequests); + for (int i = 0; i < mRequests.size(); i++) { + OverrideRequest request = mRequests.get(i); + if (!contains(newSupportedStates, request.getRequestedState())) { + mTmpRequestsToCancel.add(request); + } + } + + mRequests.removeAll(mTmpRequestsToCancel); + OverrideRequest newActiveRequest = null; + if (!mRequests.isEmpty()) { + newActiveRequest = getLast(mRequests); + if (newActiveRequest != prevActiveRequest) { + mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); + } + } + + for (int i = 0; i < mTmpRequestsToCancel.size(); i++) { + mListener.onStatusChanged(mTmpRequestsToCancel.get(i), STATUS_CANCELED); + } + mTmpRequestsToCancel.clear(); + + return newActiveRequest != prevActiveRequest; + } + + void dumpInternal(PrintWriter pw) { + final int requestCount = mRequests.size(); + pw.println(); + pw.println("Override requests: size=" + requestCount); + for (int i = 0; i < requestCount; i++) { + OverrideRequest overrideRequest = mRequests.get(i); + int status = (i == requestCount - 1) ? STATUS_ACTIVE : STATUS_SUSPENDED; + pw.println(" " + i + ": mPid=" + overrideRequest.getPid() + + ", mRequestedState=" + overrideRequest.getRequestedState() + + ", mFlags=" + overrideRequest.getFlags() + + ", mStatus=" + statusToString(status)); + } + } + + private int getRequestIndex(@NonNull IBinder token) { + final int numberOfRequests = mRequests.size(); + if (numberOfRequests == 0) { + return -1; + } + + for (int i = 0; i < numberOfRequests; i++) { + OverrideRequest request = mRequests.get(i); + if (request.getToken() == token) { + return i; + } + } + return -1; + } + + @Nullable + private static T getLast(List list) { + return list.size() > 0 ? list.get(list.size() - 1) : null; + } + + private static boolean contains(int[] array, int value) { + for (int i = 0; i < array.length; i++) { + if (array[i] == value) { + return true; + } + } + return false; + } + + public interface StatusChangeListener { + /** + * Notifies the listener of a change in request status. If a change within the controller + * causes one request to become active and one to become either suspended or cancelled, this + * method is guaranteed to be called with the active request first before the suspended or + * cancelled request. + */ + void onStatusChanged(@NonNull OverrideRequest request, @RequestStatus int newStatus); + } +} 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 ff8fbda6c83ec..5ebe8321127c0 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java @@ -69,6 +69,17 @@ public final class DeviceStateManagerServiceTest { mProvider = new TestDeviceStateProvider(); mPolicy = new TestDeviceStatePolicy(mProvider); mService = new DeviceStateManagerService(InstrumentationRegistry.getContext(), mPolicy); + flushHandler(); // Flush the handler to ensure the initial values are committed. + } + + private void flushHandler() { + flushHandler(1); + } + + private void flushHandler(int count) { + for (int i = 0; i < count; i++) { + mService.getHandler().runWithScissors(() -> {}, 0); + } } @Test @@ -80,6 +91,7 @@ public final class DeviceStateManagerServiceTest { DEFAULT_DEVICE_STATE.getIdentifier()); mProvider.setState(OTHER_DEVICE_STATE.getIdentifier()); + flushHandler(); assertEquals(mService.getCommittedState(), Optional.of(OTHER_DEVICE_STATE)); assertEquals(mService.getPendingState(), Optional.empty()); assertEquals(mService.getBaseState(), Optional.of(OTHER_DEVICE_STATE)); @@ -92,6 +104,7 @@ public final class DeviceStateManagerServiceTest { mPolicy.blockConfigure(); mProvider.setState(OTHER_DEVICE_STATE.getIdentifier()); + flushHandler(); assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE)); assertEquals(mService.getPendingState(), Optional.of(OTHER_DEVICE_STATE)); assertEquals(mService.getBaseState(), Optional.of(OTHER_DEVICE_STATE)); @@ -99,6 +112,7 @@ public final class DeviceStateManagerServiceTest { OTHER_DEVICE_STATE.getIdentifier()); mProvider.setState(DEFAULT_DEVICE_STATE.getIdentifier()); + flushHandler(); assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE)); assertEquals(mService.getPendingState(), Optional.of(OTHER_DEVICE_STATE)); assertEquals(mService.getBaseState(), Optional.of(DEFAULT_DEVICE_STATE)); @@ -106,6 +120,7 @@ public final class DeviceStateManagerServiceTest { OTHER_DEVICE_STATE.getIdentifier()); mPolicy.resumeConfigure(); + flushHandler(); assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE)); assertEquals(mService.getPendingState(), Optional.empty()); assertEquals(mService.getBaseState(), Optional.of(DEFAULT_DEVICE_STATE)); @@ -149,6 +164,7 @@ public final class DeviceStateManagerServiceTest { assertEquals(mService.getBaseState(), Optional.of(DEFAULT_DEVICE_STATE)); mProvider.notifySupportedDeviceStates(new DeviceState[]{ DEFAULT_DEVICE_STATE }); + flushHandler(); // The current committed and requests states do not change because the current state remains // supported. @@ -166,6 +182,7 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().registerCallback(callback); // An initial callback will be triggered on registration, so we clear it here. + flushHandler(); callback.clearLastNotifiedInfo(); assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE)); @@ -174,6 +191,7 @@ public final class DeviceStateManagerServiceTest { mProvider.notifySupportedDeviceStates(new DeviceState[]{ DEFAULT_DEVICE_STATE, OTHER_DEVICE_STATE }); + flushHandler(); // The current committed and requests states do not change because the current state remains // supported. @@ -203,12 +221,14 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().registerCallback(callback); mProvider.setState(OTHER_DEVICE_STATE.getIdentifier()); + flushHandler(); assertEquals(callback.getLastNotifiedInfo().baseState, OTHER_DEVICE_STATE.getIdentifier()); assertEquals(callback.getLastNotifiedInfo().currentState, OTHER_DEVICE_STATE.getIdentifier()); mProvider.setState(DEFAULT_DEVICE_STATE.getIdentifier()); + flushHandler(); assertEquals(callback.getLastNotifiedInfo().baseState, DEFAULT_DEVICE_STATE.getIdentifier()); assertEquals(callback.getLastNotifiedInfo().currentState, @@ -216,6 +236,7 @@ public final class DeviceStateManagerServiceTest { mPolicy.blockConfigure(); mProvider.setState(OTHER_DEVICE_STATE.getIdentifier()); + flushHandler(); // The callback should not have been notified of the state change as the policy is still // pending callback. assertEquals(callback.getLastNotifiedInfo().baseState, @@ -224,6 +245,7 @@ public final class DeviceStateManagerServiceTest { DEFAULT_DEVICE_STATE.getIdentifier()); mPolicy.resumeConfigure(); + flushHandler(); // Now that the policy is finished processing the callback should be notified of the state // change. assertEquals(callback.getLastNotifiedInfo().baseState, @@ -236,6 +258,7 @@ public final class DeviceStateManagerServiceTest { public void registerCallback_emitsInitialValue() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); mService.getBinderService().registerCallback(callback); + flushHandler(); assertNotNull(callback.getLastNotifiedInfo()); assertEquals(callback.getLastNotifiedInfo().baseState, DEFAULT_DEVICE_STATE.getIdentifier()); @@ -247,6 +270,7 @@ public final class DeviceStateManagerServiceTest { public void requestState() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); mService.getBinderService().registerCallback(callback); + flushHandler(); final IBinder token = new Binder(); assertEquals(callback.getLastNotifiedStatus(token), @@ -254,6 +278,10 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().requestState(token, OTHER_DEVICE_STATE.getIdentifier(), 0 /* flags */); + // Flush the handler twice. The first flush ensures the request is added and the policy is + // notified, while the second flush ensures the callback is notified once the change is + // committed. + flushHandler(2 /* count */); assertEquals(callback.getLastNotifiedStatus(token), TestDeviceStateManagerCallback.STATUS_ACTIVE); @@ -271,6 +299,7 @@ public final class DeviceStateManagerServiceTest { OTHER_DEVICE_STATE.getIdentifier()); mService.getBinderService().cancelRequest(token); + flushHandler(); assertEquals(callback.getLastNotifiedStatus(token), TestDeviceStateManagerCallback.STATUS_CANCELED); @@ -291,6 +320,7 @@ public final class DeviceStateManagerServiceTest { public void requestState_pendingStateAtRequest() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); mService.getBinderService().registerCallback(callback); + flushHandler(); mPolicy.blockConfigure(); @@ -303,6 +333,10 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().requestState(firstRequestToken, OTHER_DEVICE_STATE.getIdentifier(), 0 /* flags */); + // Flush the handler twice. The first flush ensures the request is added and the policy is + // notified, while the second flush ensures the callback is notified once the change is + // committed. + flushHandler(2 /* count */); assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE)); assertEquals(mService.getPendingState(), Optional.of(OTHER_DEVICE_STATE)); @@ -312,8 +346,8 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().requestState(secondRequestToken, DEFAULT_DEVICE_STATE.getIdentifier(), 0 /* flags */); - mPolicy.resumeConfigureOnce(); + flushHandler(); // First request status is now suspended as there is another pending request. assertEquals(callback.getLastNotifiedStatus(firstRequestToken), @@ -330,6 +364,7 @@ public final class DeviceStateManagerServiceTest { DEFAULT_DEVICE_STATE.getIdentifier()); mPolicy.resumeConfigure(); + flushHandler(); assertEquals(mService.getCommittedState(), Optional.of(DEFAULT_DEVICE_STATE)); assertEquals(mService.getPendingState(), Optional.empty()); @@ -339,6 +374,7 @@ public final class DeviceStateManagerServiceTest { // Now cancel the second request to make the first request active. mService.getBinderService().cancelRequest(secondRequestToken); + flushHandler(); assertEquals(callback.getLastNotifiedStatus(firstRequestToken), TestDeviceStateManagerCallback.STATUS_ACTIVE); @@ -356,6 +392,7 @@ public final class DeviceStateManagerServiceTest { public void requestState_sameAsBaseState() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); mService.getBinderService().registerCallback(callback); + flushHandler(); final IBinder token = new Binder(); assertEquals(callback.getLastNotifiedStatus(token), @@ -363,6 +400,7 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().requestState(token, DEFAULT_DEVICE_STATE.getIdentifier(), 0 /* flags */); + flushHandler(); assertEquals(callback.getLastNotifiedStatus(token), TestDeviceStateManagerCallback.STATUS_ACTIVE); @@ -372,6 +410,7 @@ public final class DeviceStateManagerServiceTest { public void requestState_flagCancelWhenBaseChanges() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); mService.getBinderService().registerCallback(callback); + flushHandler(); final IBinder token = new Binder(); assertEquals(callback.getLastNotifiedStatus(token), @@ -379,6 +418,10 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().requestState(token, OTHER_DEVICE_STATE.getIdentifier(), DeviceStateRequest.FLAG_CANCEL_WHEN_BASE_CHANGES); + // Flush the handler twice. The first flush ensures the request is added and the policy is + // notified, while the second flush ensures the callback is notified once the change is + // committed. + flushHandler(2 /* count */); assertEquals(callback.getLastNotifiedStatus(token), TestDeviceStateManagerCallback.STATUS_ACTIVE); @@ -391,6 +434,7 @@ public final class DeviceStateManagerServiceTest { OTHER_DEVICE_STATE.getIdentifier()); mProvider.setState(OTHER_DEVICE_STATE.getIdentifier()); + flushHandler(); // Request is canceled because the base state changed. assertEquals(callback.getLastNotifiedStatus(token), @@ -407,6 +451,7 @@ public final class DeviceStateManagerServiceTest { public void requestState_becomesUnsupported() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); mService.getBinderService().registerCallback(callback); + flushHandler(); final IBinder token = new Binder(); assertEquals(callback.getLastNotifiedStatus(token), @@ -414,6 +459,7 @@ public final class DeviceStateManagerServiceTest { mService.getBinderService().requestState(token, OTHER_DEVICE_STATE.getIdentifier(), 0 /* flags */); + flushHandler(); assertEquals(callback.getLastNotifiedStatus(token), TestDeviceStateManagerCallback.STATUS_ACTIVE); @@ -425,6 +471,7 @@ public final class DeviceStateManagerServiceTest { OTHER_DEVICE_STATE.getIdentifier()); mProvider.notifySupportedDeviceStates(new DeviceState[]{ DEFAULT_DEVICE_STATE }); + flushHandler(); // Request is canceled because the state is no longer supported. assertEquals(callback.getLastNotifiedStatus(token), diff --git a/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java b/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java new file mode 100644 index 0000000000000..6f6632fd033f7 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java @@ -0,0 +1,204 @@ +/* + * Copyright (C) 2021 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.server.devicestate; + +import static com.android.server.devicestate.OverrideRequestController.STATUS_ACTIVE; +import static com.android.server.devicestate.OverrideRequestController.STATUS_CANCELED; +import static com.android.server.devicestate.OverrideRequestController.STATUS_SUSPENDED; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNull; + +import android.annotation.Nullable; +import android.hardware.devicestate.DeviceStateRequest; +import android.os.Binder; +import android.platform.test.annotations.Presubmit; + +import androidx.annotation.NonNull; +import androidx.test.runner.AndroidJUnit4; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.HashMap; +import java.util.Map; + +/** + * Unit tests for {@link OverrideRequestController}. + *

+ * Run with atest OverrideRequestControllerTest. + */ +@Presubmit +@RunWith(AndroidJUnit4.class) +public final class OverrideRequestControllerTest { + private TestStatusChangeListener mStatusListener; + private OverrideRequestController mController; + + @Before + public void setup() { + mStatusListener = new TestStatusChangeListener(); + mController = new OverrideRequestController(mStatusListener); + } + + @Test + public void addRequest() { + OverrideRequest request = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + assertNull(mStatusListener.getLastStatus(request)); + + mController.addRequest(request); + assertEquals(mStatusListener.getLastStatus(request).intValue(), STATUS_ACTIVE); + } + + @Test + public void addRequest_suspendExistingRequest() { + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + assertNull(mStatusListener.getLastStatus(firstRequest)); + + mController.addRequest(firstRequest); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE); + + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + assertNull(mStatusListener.getLastStatus(secondRequest)); + + mController.addRequest(secondRequest); + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + } + + @Test + public void addRequest_cancelActiveRequest() { + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + + mController.addRequest(firstRequest); + mController.addRequest(secondRequest); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.cancelRequest(secondRequest.getToken()); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_CANCELED); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE); + } + + @Test + public void addRequest_cancelSuspendedRequest() { + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + + mController.addRequest(firstRequest); + mController.addRequest(secondRequest); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.cancelRequest(firstRequest.getToken()); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_CANCELED); + } + + @Test + public void handleBaseStateChanged() { + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, + DeviceStateRequest.FLAG_CANCEL_WHEN_BASE_CHANGES /* flags */); + + mController.addRequest(firstRequest); + mController.addRequest(secondRequest); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.handleBaseStateChanged(); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_CANCELED); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE); + } + + @Test + public void handleProcessDied() { + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 1 /* pid */, + 0 /* requestedState */, 0 /* flags */); + + mController.addRequest(firstRequest); + mController.addRequest(secondRequest); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.handleProcessDied(1); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_CANCELED); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE); + + mController.handleProcessDied(0); + + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_CANCELED); + } + + @Test + public void handleNewSupportedStates() { + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 1 /* requestedState */, 0 /* flags */); + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 2 /* requestedState */, 0 /* flags */); + + mController.addRequest(firstRequest); + mController.addRequest(secondRequest); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.handleNewSupportedStates(new int[]{ 0, 1 }); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_CANCELED); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE); + + mController.handleNewSupportedStates(new int[]{ 0 }); + + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_CANCELED); + } + + private static final class TestStatusChangeListener implements + OverrideRequestController.StatusChangeListener { + private Map mLastStatusMap = new HashMap<>(); + + @Override + public void onStatusChanged(@NonNull OverrideRequest request, int newStatus) { + mLastStatusMap.put(request, newStatus); + } + + @Nullable + public Integer getLastStatus(OverrideRequest request) { + return mLastStatusMap.get(request); + } + } +} From 7dde391fc1153d008b1bc84d40a184ace6667ca3 Mon Sep 17 00:00:00 2001 From: Darryl L Johnson Date: Fri, 23 Jul 2021 17:06:17 -0700 Subject: [PATCH 2/3] Allow override requests to outlive the requesting process. This change introduces a flag, FLAG_CANCEL_STICKY_REQUESTS, on the device states that are provided by the DeviceStateProvider. If the flag is set on a device state it indicates that the device state should cancel any outstanding sticky requests when entered. If at least one device state is configured to be a terminal device state (has the flag set) the service will be configured to allow sticky requests, otherwise requests will continue to be cancelled when the requesting process dies. Fixes: 192671286 Test: atest DeviceStateManagerServiceTest Test: atest DeviceStateProviderImplTest Test: atest DeviceStateTest Test: atest OverrideRequestControllerTest Change-Id: I7a6bdc9334342fa818920b56a4f0c22629bb101f --- .../devicestate/DeviceStateManager.java | 8 +- .../server/devicestate/DeviceState.java | 33 ++++- .../DeviceStateManagerService.java | 12 ++ .../OverrideRequestController.java | 114 +++++++++++------- .../policy/DeviceStateProviderImpl.java | 24 +++- .../device-state-config.xsd | 9 ++ .../device-state-config/schema/current.txt | 7 ++ .../DeviceStateManagerServiceTest.java | 9 +- .../server/devicestate/DeviceStateTest.java | 10 +- .../OverrideRequestControllerTest.java | 26 ++++ .../policy/DeviceStateProviderImplTest.java | 23 ++-- 11 files changed, 210 insertions(+), 65 deletions(-) diff --git a/core/java/android/hardware/devicestate/DeviceStateManager.java b/core/java/android/hardware/devicestate/DeviceStateManager.java index 52dad3efefb89..bb3bffdbe6783 100644 --- a/core/java/android/hardware/devicestate/DeviceStateManager.java +++ b/core/java/android/hardware/devicestate/DeviceStateManager.java @@ -75,13 +75,13 @@ public final class DeviceStateManager { /** * Submits a {@link DeviceStateRequest request} to modify the device state. *

- * By default, the request is kept active until a call to - * {@link #cancelRequest(DeviceStateRequest)} or until one of the following occurs: + * By default, the request is kept active until one of the following occurs: *

    + *
  • The system deems the request can no longer be honored, for example if the requested + * state becomes unsupported. + *
  • A call to {@link #cancelRequest(DeviceStateRequest)}. *
  • Another processes submits a request succeeding this request in which case the request * will be suspended until the interrupting request is canceled. - *
  • The requested state has become unsupported. - *
  • The process submitting the request dies. *
* However, this behavior can be changed by setting flags on the {@link DeviceStateRequest}. * diff --git a/services/core/java/com/android/server/devicestate/DeviceState.java b/services/core/java/com/android/server/devicestate/DeviceState.java index e693bcc93f8f5..7fe24ff1f069b 100644 --- a/services/core/java/com/android/server/devicestate/DeviceState.java +++ b/services/core/java/com/android/server/devicestate/DeviceState.java @@ -19,11 +19,14 @@ package com.android.server.devicestate; import static android.hardware.devicestate.DeviceStateManager.MAXIMUM_DEVICE_STATE; import static android.hardware.devicestate.DeviceStateManager.MINIMUM_DEVICE_STATE; +import android.annotation.IntDef; import android.annotation.IntRange; import android.annotation.NonNull; import com.android.internal.util.Preconditions; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.Objects; /** @@ -39,6 +42,19 @@ import java.util.Objects; * @see DeviceStateManagerService */ public final class DeviceState { + /** + * Flag that indicates sticky requests should be cancelled when this device state becomes the + * base device state. + */ + public static final int FLAG_CANCEL_STICKY_REQUESTS = 1 << 0; + + /** @hide */ + @IntDef(prefix = {"FLAG_"}, flag = true, value = { + FLAG_CANCEL_STICKY_REQUESTS, + }) + @Retention(RetentionPolicy.SOURCE) + public @interface DeviceStateFlags {} + /** Unique identifier for the device state. */ @IntRange(from = MINIMUM_DEVICE_STATE, to = MAXIMUM_DEVICE_STATE) private final int mIdentifier; @@ -47,14 +63,19 @@ public final class DeviceState { @NonNull private final String mName; + @DeviceStateFlags + private final int mFlags; + public DeviceState( @IntRange(from = MINIMUM_DEVICE_STATE, to = MAXIMUM_DEVICE_STATE) int identifier, - @NonNull String name) { + @NonNull String name, + @DeviceStateFlags int flags) { Preconditions.checkArgumentInRange(identifier, MINIMUM_DEVICE_STATE, MAXIMUM_DEVICE_STATE, "identifier"); mIdentifier = identifier; mName = name; + mFlags = flags; } /** Returns the unique identifier for the device state. */ @@ -69,6 +90,11 @@ public final class DeviceState { return mName; } + @DeviceStateFlags + public int getFlags() { + return mFlags; + } + @Override public String toString() { return "DeviceState{" + "identifier=" + mIdentifier + ", name='" + mName + '\'' + '}'; @@ -80,11 +106,12 @@ public final class DeviceState { if (o == null || getClass() != o.getClass()) return false; DeviceState that = (DeviceState) o; return mIdentifier == that.mIdentifier - && Objects.equals(mName, that.mName); + && Objects.equals(mName, that.mName) + && mFlags == that.mFlags; } @Override public int hashCode() { - return Objects.hash(mIdentifier, mName); + return Objects.hash(mIdentifier, mName, mFlags); } } diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index 42fd9642c2db7..b0c36aac73d80 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -275,12 +275,21 @@ public final class DeviceStateManagerService extends SystemService { synchronized (mLock) { final int[] oldStateIdentifiers = getSupportedStateIdentifiersLocked(); + // Whether or not at least one device state has the flag FLAG_CANCEL_STICKY_REQUESTS + // set. If set to true, the OverrideRequestController will be configured to allow sticky + // requests. + boolean hasTerminalDeviceState = false; mDeviceStates.clear(); for (int i = 0; i < supportedDeviceStates.length; i++) { DeviceState state = supportedDeviceStates[i]; + if ((state.getFlags() & DeviceState.FLAG_CANCEL_STICKY_REQUESTS) != 0) { + hasTerminalDeviceState = true; + } mDeviceStates.put(state.getIdentifier(), state); } + mOverrideRequestController.setStickyRequestsAllowed(hasTerminalDeviceState); + final int[] newStateIdentifiers = getSupportedStateIdentifiersLocked(); if (Arrays.equals(oldStateIdentifiers, newStateIdentifiers)) { return; @@ -338,6 +347,9 @@ public final class DeviceStateManagerService extends SystemService { } mBaseState = Optional.of(baseState); + if ((baseState.getFlags() & DeviceState.FLAG_CANCEL_STICKY_REQUESTS) != 0) { + mOverrideRequestController.cancelStickyRequests(); + } mOverrideRequestController.handleBaseStateChanged(); updatePendingStateLocked(); diff --git a/services/core/java/com/android/server/devicestate/OverrideRequestController.java b/services/core/java/com/android/server/devicestate/OverrideRequestController.java index 042bea5fcfa37..05c9eb2c5bbe8 100644 --- a/services/core/java/com/android/server/devicestate/OverrideRequestController.java +++ b/services/core/java/com/android/server/devicestate/OverrideRequestController.java @@ -84,10 +84,28 @@ final class OverrideRequestController { // List of override requests with the most recent override request at the end. private final ArrayList mRequests = new ArrayList<>(); + private boolean mStickyRequestsAllowed; + // List of override requests that have outlived their process and will only be cancelled through + // a call to cancelStickyRequests(). + private final ArrayList mStickyRequests = new ArrayList<>(); + OverrideRequestController(@NonNull StatusChangeListener listener) { mListener = listener; } + /** + * Sets sticky requests as either allowed or disallowed. When sticky requests are allowed a call + * to {@link #handleProcessDied(int)} will not result in the request being cancelled + * immediately. Instead, the request will be marked sticky and must be cancelled with a call + * to {@link #cancelStickyRequests()}. + */ + void setStickyRequestsAllowed(boolean stickyRequestsAllowed) { + mStickyRequestsAllowed = stickyRequestsAllowed; + if (!mStickyRequestsAllowed) { + cancelStickyRequests(); + } + } + /** * Adds a request to the top of the stack and notifies the listener of all changes to request * status as a result of this operation. @@ -122,6 +140,18 @@ final class OverrideRequestController { mListener.onStatusChanged(request, STATUS_CANCELED); } + /** + * Cancels all requests that are currently marked sticky and notifies the listener of all + * changes to request status as a result of this operation. + * + * @see #setStickyRequestsAllowed(boolean) + */ + void cancelStickyRequests() { + mTmpRequestsToCancel.clear(); + mTmpRequestsToCancel.addAll(mStickyRequests); + cancelRequestsLocked(mTmpRequestsToCancel); + } + /** * Returns {@code true} if this controller is current managing a request with the specified * {@code token}, {@code false} otherwise. @@ -140,6 +170,7 @@ final class OverrideRequestController { return; } + mTmpRequestsToCancel.clear(); OverrideRequest prevActiveRequest = getLast(mRequests); for (OverrideRequest request : mRequests) { if (request.getPid() == pid) { @@ -147,18 +178,14 @@ final class OverrideRequestController { } } - mRequests.removeAll(mTmpRequestsToCancel); - if (!mRequests.isEmpty()) { - OverrideRequest newActiveRequest = getLast(mRequests); - if (newActiveRequest != prevActiveRequest) { - mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); - } + if (mStickyRequestsAllowed) { + // Do not cancel the requests now because sticky requests are allowed. These + // requests will be cancelled on a call to cancelStickyRequests(). + mStickyRequests.addAll(mTmpRequestsToCancel); + return; } - for (int i = 0; i < mTmpRequestsToCancel.size(); i++) { - mListener.onStatusChanged(mTmpRequestsToCancel.get(i), STATUS_CANCELED); - } - mTmpRequestsToCancel.clear(); + cancelRequestsLocked(mTmpRequestsToCancel); } /** @@ -173,6 +200,7 @@ final class OverrideRequestController { return false; } + mTmpRequestsToCancel.clear(); OverrideRequest prevActiveRequest = getLast(mRequests); for (int i = 0; i < mRequests.size(); i++) { OverrideRequest request = mRequests.get(i); @@ -181,21 +209,8 @@ final class OverrideRequestController { } } - mRequests.removeAll(mTmpRequestsToCancel); - OverrideRequest newActiveRequest = null; - if (!mRequests.isEmpty()) { - newActiveRequest = getLast(mRequests); - if (newActiveRequest != prevActiveRequest) { - mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); - } - } - - for (int i = 0; i < mTmpRequestsToCancel.size(); i++) { - mListener.onStatusChanged(mTmpRequestsToCancel.get(i), STATUS_CANCELED); - } - mTmpRequestsToCancel.clear(); - - return newActiveRequest != prevActiveRequest; + final boolean newActiveRequest = cancelRequestsLocked(mTmpRequestsToCancel); + return newActiveRequest; } /** @@ -210,7 +225,7 @@ final class OverrideRequestController { return false; } - OverrideRequest prevActiveRequest = getLast(mRequests); + mTmpRequestsToCancel.clear(); for (int i = 0; i < mRequests.size(); i++) { OverrideRequest request = mRequests.get(i); if (!contains(newSupportedStates, request.getRequestedState())) { @@ -218,21 +233,8 @@ final class OverrideRequestController { } } - mRequests.removeAll(mTmpRequestsToCancel); - OverrideRequest newActiveRequest = null; - if (!mRequests.isEmpty()) { - newActiveRequest = getLast(mRequests); - if (newActiveRequest != prevActiveRequest) { - mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); - } - } - - for (int i = 0; i < mTmpRequestsToCancel.size(); i++) { - mListener.onStatusChanged(mTmpRequestsToCancel.get(i), STATUS_CANCELED); - } - mTmpRequestsToCancel.clear(); - - return newActiveRequest != prevActiveRequest; + final boolean newActiveRequest = cancelRequestsLocked(mTmpRequestsToCancel); + return newActiveRequest; } void dumpInternal(PrintWriter pw) { @@ -249,6 +251,36 @@ final class OverrideRequestController { } } + /** + * Handles cancelling a set of requests. If the set of requests to cancel will lead to a new + * request becoming active this request will also be notified of its change in state. + * + * @return {@code true} if calling this method has lead to a new active request, {@code false} + * otherwise. + */ + private boolean cancelRequestsLocked(List requestsToCancel) { + if (requestsToCancel.isEmpty()) { + return false; + } + + OverrideRequest prevActiveRequest = getLast(mRequests); + boolean causedNewRequestToBecomeActive = false; + mRequests.removeAll(requestsToCancel); + mStickyRequests.removeAll(requestsToCancel); + if (!mRequests.isEmpty()) { + OverrideRequest newActiveRequest = getLast(mRequests); + if (newActiveRequest != prevActiveRequest) { + mListener.onStatusChanged(newActiveRequest, STATUS_ACTIVE); + causedNewRequestToBecomeActive = true; + } + } + + for (int i = 0; i < requestsToCancel.size(); i++) { + mListener.onStatusChanged(requestsToCancel.get(i), STATUS_CANCELED); + } + return causedNewRequestToBecomeActive; + } + private int getRequestIndex(@NonNull IBinder token) { final int numberOfRequests = mRequests.size(); if (numberOfRequests == 0) { diff --git a/services/core/java/com/android/server/policy/DeviceStateProviderImpl.java b/services/core/java/com/android/server/policy/DeviceStateProviderImpl.java index edd5f5f415c62..ff6511f065771 100644 --- a/services/core/java/com/android/server/policy/DeviceStateProviderImpl.java +++ b/services/core/java/com/android/server/policy/DeviceStateProviderImpl.java @@ -41,6 +41,7 @@ import com.android.server.devicestate.DeviceState; import com.android.server.devicestate.DeviceStateProvider; import com.android.server.policy.devicestate.config.Conditions; import com.android.server.policy.devicestate.config.DeviceStateConfig; +import com.android.server.policy.devicestate.config.Flags; import com.android.server.policy.devicestate.config.LidSwitchCondition; import com.android.server.policy.devicestate.config.NumericRange; import com.android.server.policy.devicestate.config.SensorCondition; @@ -87,7 +88,7 @@ public final class DeviceStateProviderImpl implements DeviceStateProvider, @VisibleForTesting static final DeviceState DEFAULT_DEVICE_STATE = new DeviceState(MINIMUM_DEVICE_STATE, - "DEFAULT"); + "DEFAULT", 0 /* flags */); private static final String VENDOR_CONFIG_FILE_PATH = "etc/devicestate/"; private static final String DATA_CONFIG_FILE_PATH = "system/devicestate/"; @@ -131,7 +132,26 @@ public final class DeviceStateProviderImpl implements DeviceStateProvider, config.getDeviceState()) { final int state = stateConfig.getIdentifier().intValue(); final String name = stateConfig.getName() == null ? "" : stateConfig.getName(); - deviceStateList.add(new DeviceState(state, name)); + + int flags = 0; + final Flags configFlags = stateConfig.getFlags(); + if (configFlags != null) { + List configFlagStrings = configFlags.getFlag(); + for (int i = 0; i < configFlagStrings.size(); i++) { + final String configFlagString = configFlagStrings.get(i); + switch (configFlagString) { + case "FLAG_CANCEL_STICKY_REQUESTS": + flags |= DeviceState.FLAG_CANCEL_STICKY_REQUESTS; + break; + default: + Slog.w(TAG, "Parsed unknown flag with name: " + + configFlagString); + break; + } + } + } + + deviceStateList.add(new DeviceState(state, name, flags)); final Conditions condition = stateConfig.getConditions(); conditionsList.add(condition); diff --git a/services/core/xsd/device-state-config/device-state-config.xsd b/services/core/xsd/device-state-config/device-state-config.xsd index 94a398f2cdb7f..86f41769008d1 100644 --- a/services/core/xsd/device-state-config/device-state-config.xsd +++ b/services/core/xsd/device-state-config/device-state-config.xsd @@ -40,10 +40,19 @@ + + + + + + + + + diff --git a/services/core/xsd/device-state-config/schema/current.txt b/services/core/xsd/device-state-config/schema/current.txt index 08fccf8ad9498..a98d4e569cd66 100644 --- a/services/core/xsd/device-state-config/schema/current.txt +++ b/services/core/xsd/device-state-config/schema/current.txt @@ -11,9 +11,11 @@ package com.android.server.policy.devicestate.config { public class DeviceState { ctor public DeviceState(); method public com.android.server.policy.devicestate.config.Conditions getConditions(); + method public com.android.server.policy.devicestate.config.Flags getFlags(); method public java.math.BigInteger getIdentifier(); method @Nullable public String getName(); method public void setConditions(com.android.server.policy.devicestate.config.Conditions); + method public void setFlags(com.android.server.policy.devicestate.config.Flags); method public void setIdentifier(java.math.BigInteger); method public void setName(@Nullable String); } @@ -23,6 +25,11 @@ package com.android.server.policy.devicestate.config { method public java.util.List getDeviceState(); } + public class Flags { + ctor public Flags(); + method @Nullable public java.util.List getFlag(); + } + public class LidSwitchCondition { ctor public LidSwitchCondition(); method public boolean getOpen(); 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 5ebe8321127c0..c8f81742822f6 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java @@ -55,10 +55,13 @@ import javax.annotation.Nullable; @Presubmit @RunWith(AndroidJUnit4.class) public final class DeviceStateManagerServiceTest { - private static final DeviceState DEFAULT_DEVICE_STATE = new DeviceState(0, "DEFAULT"); - private static final DeviceState OTHER_DEVICE_STATE = new DeviceState(1, "OTHER"); + private static final DeviceState DEFAULT_DEVICE_STATE = + new DeviceState(0, "DEFAULT", 0 /* flags */); + private static final DeviceState OTHER_DEVICE_STATE = + new DeviceState(1, "OTHER", 0 /* flags */); // A device state that is not reported as being supported for the default test provider. - private static final DeviceState UNSUPPORTED_DEVICE_STATE = new DeviceState(255, "UNSUPPORTED"); + private static final DeviceState UNSUPPORTED_DEVICE_STATE = + new DeviceState(255, "UNSUPPORTED", 0 /* flags */); private TestDeviceStatePolicy mPolicy; private TestDeviceStateProvider mProvider; diff --git a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateTest.java b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateTest.java index b5c8053ad77ea..e286cb27cc418 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateTest.java @@ -41,24 +41,26 @@ public final class DeviceStateTest { @Test public void testConstruct() { final DeviceState state = new DeviceState(MINIMUM_DEVICE_STATE /* identifier */, - "CLOSED" /* name */); + "CLOSED" /* name */, DeviceState.FLAG_CANCEL_STICKY_REQUESTS /* flags */); assertEquals(state.getIdentifier(), MINIMUM_DEVICE_STATE); assertEquals(state.getName(), "CLOSED"); + assertEquals(state.getFlags(), DeviceState.FLAG_CANCEL_STICKY_REQUESTS); } @Test public void testConstruct_nullName() { final DeviceState state = new DeviceState(MAXIMUM_DEVICE_STATE /* identifier */, - null /* name */); + null /* name */, 0/* flags */); assertEquals(state.getIdentifier(), MAXIMUM_DEVICE_STATE); assertNull(state.getName()); + assertEquals(state.getFlags(), 0); } @Test public void testConstruct_tooLargeIdentifier() { assertThrows(IllegalArgumentException.class, () -> { final DeviceState state = new DeviceState(MAXIMUM_DEVICE_STATE + 1 /* identifier */, - null /* name */); + null /* name */, 0 /* flags */); }); } @@ -66,7 +68,7 @@ public final class DeviceStateTest { public void testConstruct_tooSmallIdentifier() { assertThrows(IllegalArgumentException.class, () -> { final DeviceState state = new DeviceState(MINIMUM_DEVICE_STATE - 1 /* identifier */, - null /* name */); + null /* name */, 0 /* flags */); }); } } 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 6f6632fd033f7..c9cf2f06640d8 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/OverrideRequestControllerTest.java @@ -164,6 +164,32 @@ public final class OverrideRequestControllerTest { assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_CANCELED); } + @Test + public void handleProcessDied_stickyRequests() { + mController.setStickyRequestsAllowed(true); + + OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, + 0 /* requestedState */, 0 /* flags */); + OverrideRequest secondRequest = new OverrideRequest(new Binder(), 1 /* pid */, + 0 /* requestedState */, 0 /* flags */); + + mController.addRequest(firstRequest); + mController.addRequest(secondRequest); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.handleProcessDied(1); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_ACTIVE); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_SUSPENDED); + + mController.cancelStickyRequests(); + + assertEquals(mStatusListener.getLastStatus(secondRequest).intValue(), STATUS_CANCELED); + assertEquals(mStatusListener.getLastStatus(firstRequest).intValue(), STATUS_ACTIVE); + } + @Test public void handleNewSupportedStates() { OverrideRequest firstRequest = new OverrideRequest(new Binder(), 0 /* pid */, diff --git a/services/tests/servicestests/src/com/android/server/policy/DeviceStateProviderImplTest.java b/services/tests/servicestests/src/com/android/server/policy/DeviceStateProviderImplTest.java index 4d2d2f1a4b7d8..8e2c1f051279d 100644 --- a/services/tests/servicestests/src/com/android/server/policy/DeviceStateProviderImplTest.java +++ b/services/tests/servicestests/src/com/android/server/policy/DeviceStateProviderImplTest.java @@ -150,8 +150,9 @@ public final class DeviceStateProviderImplTest { provider.setListener(listener); verify(listener).onSupportedDeviceStatesChanged(mDeviceStateArrayCaptor.capture()); - final DeviceState[] expectedStates = new DeviceState[]{ new DeviceState(1, ""), - new DeviceState(2, "") }; + final DeviceState[] expectedStates = new DeviceState[]{ + new DeviceState(1, "", 0 /* flags */), + new DeviceState(2, "", 0 /* flags */) }; assertArrayEquals(expectedStates, mDeviceStateArrayCaptor.getValue()); verify(listener).onStateChanged(mIntegerCaptor.capture()); @@ -187,8 +188,9 @@ public final class DeviceStateProviderImplTest { provider.setListener(listener); verify(listener).onSupportedDeviceStatesChanged(mDeviceStateArrayCaptor.capture()); - final DeviceState[] expectedStates = new DeviceState[]{ new DeviceState(1, ""), - new DeviceState(2, "CLOSED") }; + final DeviceState[] expectedStates = new DeviceState[]{ + new DeviceState(1, "", 0 /* flags */), + new DeviceState(2, "CLOSED", 0 /* flags */) }; assertArrayEquals(expectedStates, mDeviceStateArrayCaptor.getValue()); // onStateChanged() should not be called because the provider has not yet been notified of @@ -264,8 +266,11 @@ public final class DeviceStateProviderImplTest { verify(listener).onSupportedDeviceStatesChanged(mDeviceStateArrayCaptor.capture()); assertArrayEquals( - new DeviceState[]{ new DeviceState(1, "CLOSED"), new DeviceState(2, "HALF_OPENED"), - new DeviceState(3, "OPENED") }, mDeviceStateArrayCaptor.getValue()); + new DeviceState[]{ + new DeviceState(1, "CLOSED", 0 /* flags */), + new DeviceState(2, "HALF_OPENED", 0 /* flags */), + new DeviceState(3, "OPENED", 0 /* flags */) }, + mDeviceStateArrayCaptor.getValue()); // onStateChanged() should not be called because the provider has not yet been notified of // the initial sensor state. verify(listener, never()).onStateChanged(mIntegerCaptor.capture()); @@ -350,8 +355,10 @@ public final class DeviceStateProviderImplTest { verify(listener).onSupportedDeviceStatesChanged(mDeviceStateArrayCaptor.capture()); assertArrayEquals( - new DeviceState[]{ new DeviceState(1, "CLOSED"), new DeviceState(2, "HALF_OPENED"), - }, mDeviceStateArrayCaptor.getValue()); + new DeviceState[]{ + new DeviceState(1, "CLOSED", 0 /* flags */), + new DeviceState(2, "HALF_OPENED", 0 /* flags */) + }, mDeviceStateArrayCaptor.getValue()); // onStateChanged() should be called because the provider could not find the sensor. verify(listener).onStateChanged(mIntegerCaptor.capture()); assertEquals(1, mIntegerCaptor.getValue().intValue()); From 1f70abe98d33f9e673dc2e4022fbb4dad8025ed8 Mon Sep 17 00:00:00 2001 From: Kenneth Ford Date: Thu, 5 Aug 2021 23:23:57 +0000 Subject: [PATCH 3/3] Allow top focused activities to call requestState Previously you could only modify the device state if you held the CONTROL_DEVICE_STATE permission. This change allows top focused activities to do the same Bug: 192671754 Test: atest DeviceStateManagerTests Change-Id: Iba0c1800a1fb17024020e657b76a54eec9c89348 --- core/api/test-current.txt | 4 +-- .../devicestate/DeviceStateManager.java | 14 ++++---- .../DeviceStateManagerService.java | 33 +++++++++++++++---- .../DeviceStateManagerServiceTest.java | 16 +++++++++ 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/core/api/test-current.txt b/core/api/test-current.txt index f6b437405049c..0732c61364016 100644 --- a/core/api/test-current.txt +++ b/core/api/test-current.txt @@ -1133,10 +1133,10 @@ package android.hardware.camera2 { package android.hardware.devicestate { public final class DeviceStateManager { - method @RequiresPermission(android.Manifest.permission.CONTROL_DEVICE_STATE) public void cancelRequest(@NonNull android.hardware.devicestate.DeviceStateRequest); + method @RequiresPermission(value=android.Manifest.permission.CONTROL_DEVICE_STATE, conditional=true) public void cancelRequest(@NonNull android.hardware.devicestate.DeviceStateRequest); method @NonNull public int[] getSupportedStates(); method public void registerCallback(@NonNull java.util.concurrent.Executor, @NonNull android.hardware.devicestate.DeviceStateManager.DeviceStateCallback); - method @RequiresPermission(android.Manifest.permission.CONTROL_DEVICE_STATE) public void requestState(@NonNull android.hardware.devicestate.DeviceStateRequest, @Nullable java.util.concurrent.Executor, @Nullable android.hardware.devicestate.DeviceStateRequest.Callback); + 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); method public void unregisterCallback(@NonNull android.hardware.devicestate.DeviceStateManager.DeviceStateCallback); field public static final int MAXIMUM_DEVICE_STATE = 255; // 0xff field public static final int MINIMUM_DEVICE_STATE = 0; // 0x0 diff --git a/core/java/android/hardware/devicestate/DeviceStateManager.java b/core/java/android/hardware/devicestate/DeviceStateManager.java index bb3bffdbe6783..95892aa7af813 100644 --- a/core/java/android/hardware/devicestate/DeviceStateManager.java +++ b/core/java/android/hardware/devicestate/DeviceStateManager.java @@ -86,12 +86,13 @@ public final class DeviceStateManager { * However, this behavior can be changed by setting flags on the {@link DeviceStateRequest}. * * @throws IllegalArgumentException if the requested state is unsupported. - * @throws SecurityException if the {@link android.Manifest.permission#CONTROL_DEVICE_STATE} - * permission is not held. + * @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. * * @see DeviceStateRequest */ - @RequiresPermission(android.Manifest.permission.CONTROL_DEVICE_STATE) + @RequiresPermission(value = android.Manifest.permission.CONTROL_DEVICE_STATE, + conditional = true) public void requestState(@NonNull DeviceStateRequest request, @Nullable @CallbackExecutor Executor executor, @Nullable DeviceStateRequest.Callback callback) { @@ -105,10 +106,11 @@ public final class DeviceStateManager { * This method is noop if the {@code request} has not been submitted with a call to * {@link #requestState(DeviceStateRequest, Executor, DeviceStateRequest.Callback)}. * - * @throws SecurityException if the {@link android.Manifest.permission#CONTROL_DEVICE_STATE} - * permission is not held. + * @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(android.Manifest.permission.CONTROL_DEVICE_STATE) + @RequiresPermission(value = android.Manifest.permission.CONTROL_DEVICE_STATE, + conditional = true) public void cancelRequest(@NonNull DeviceStateRequest request) { mGlobal.cancelRequest(request); } diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index b0c36aac73d80..383b3928e5e60 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -48,9 +48,12 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.DumpUtils; import com.android.internal.util.FrameworkStatsLog; +import com.android.server.LocalServices; import com.android.server.ServiceThread; import com.android.server.SystemService; import com.android.server.policy.DeviceStatePolicyImpl; +import com.android.server.wm.ActivityTaskManagerInternal; +import com.android.server.wm.WindowProcessController; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -101,6 +104,9 @@ public final class DeviceStateManagerService extends SystemService { private final BinderService mBinderService; @NonNull private final OverrideRequestController mOverrideRequestController; + @VisibleForTesting + @NonNull + public ActivityTaskManagerInternal mActivityTaskManagerInternal; // All supported device states keyed by identifier. @GuardedBy("mLock") @@ -153,6 +159,7 @@ public final class DeviceStateManagerService extends SystemService { mDeviceStatePolicy = policy; mDeviceStatePolicy.getDeviceStateProvider().setListener(new DeviceStateProviderListener()); mBinderService = new BinderService(); + mActivityTaskManagerInternal = LocalServices.getService(ActivityTaskManagerInternal.class); } @Override @@ -778,14 +785,21 @@ public final class DeviceStateManagerService extends SystemService { @Override // Binder call public void requestState(IBinder token, int state, int flags) { - getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE, - "Permission required to request device state."); + final int callingPid = Binder.getCallingPid(); + // 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."); + } if (token == null) { throw new IllegalArgumentException("Request token must not be null."); } - final int callingPid = Binder.getCallingPid(); final long callingIdentity = Binder.clearCallingIdentity(); try { requestStateInternal(state, flags, callingPid, token); @@ -796,14 +810,21 @@ public final class DeviceStateManagerService extends SystemService { @Override // Binder call public void cancelRequest(IBinder token) { - getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE, - "Permission required to clear requested device state."); + 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."); } - final int callingPid = Binder.getCallingPid(); final long callingIdentity = Binder.clearCallingIdentity(); try { cancelRequestInternal(callingPid, token); 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 c8f81742822f6..b1b6e5341f38e 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java @@ -18,6 +18,7 @@ package com.android.server.devicestate; import static android.hardware.devicestate.DeviceStateManager.INVALID_DEVICE_STATE; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; @@ -35,6 +36,11 @@ import android.platform.test.annotations.Presubmit; import androidx.test.InstrumentationRegistry; import androidx.test.runner.AndroidJUnit4; +import static org.mockito.Mockito.mock; + +import com.android.server.wm.ActivityTaskManagerInternal; +import com.android.server.wm.WindowProcessController; + import junit.framework.Assert; import org.junit.Before; @@ -63,6 +69,8 @@ public final class DeviceStateManagerServiceTest { private static final DeviceState UNSUPPORTED_DEVICE_STATE = new DeviceState(255, "UNSUPPORTED", 0 /* flags */); + private static final int FAKE_PROCESS_ID = 100; + private TestDeviceStatePolicy mPolicy; private TestDeviceStateProvider mProvider; private DeviceStateManagerService mService; @@ -72,6 +80,14 @@ public final class DeviceStateManagerServiceTest { mProvider = new TestDeviceStateProvider(); mPolicy = new TestDeviceStatePolicy(mProvider); mService = new DeviceStateManagerService(InstrumentationRegistry.getContext(), mPolicy); + + // Necessary to allow us to check for top app process id in tests + mService.mActivityTaskManagerInternal = mock(ActivityTaskManagerInternal.class); + WindowProcessController windowProcessController = mock(WindowProcessController.class); + when(mService.mActivityTaskManagerInternal.getTopApp()) + .thenReturn(windowProcessController); + when(windowProcessController.getPid()).thenReturn(FAKE_PROCESS_ID); + flushHandler(); // Flush the handler to ensure the initial values are committed. }