From ab8910cee01b88dbe55a55b6cf78f29e69f0f8fc Mon Sep 17 00:00:00 2001 From: Darryl L Johnson Date: Wed, 10 Feb 2021 23:16:32 -0800 Subject: [PATCH] Mark requests as active if they don't cause a change in the pending state. This is the case when a request comes in that matches the current committed state. This also fixes an issue where a older requested state would become committed and always set the newest request as active, which isn't always the case if a request has come in (with a different state) while the service is awaiting callback from the policy. Fixes: 179961602 Test: atest DeviceStateManagerServiceTest Change-Id: Ifd818840de0f39a33cca733c23d6859582006aa8 --- .../DeviceStateManagerService.java | 17 +++- .../DeviceStateManagerServiceTest.java | 89 +++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index 835b4688dd4e0..658d27f433139 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -425,7 +425,13 @@ public final class DeviceStateManagerService extends SystemService { if (!mRequestRecords.isEmpty()) { final OverrideRequestRecord topRequest = mRequestRecords.get(mRequestRecords.size() - 1); - topRequest.setStatusLocked(OverrideRequestRecord.STATUS_ACTIVE); + if (topRequest.mRequestedState.getIdentifier() == mCommittedState.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); + } } mPendingState = Optional.empty(); @@ -563,10 +569,13 @@ public final class DeviceStateManagerService extends SystemService { new OverrideRequestRecord(processRecord, token, deviceState.get(), flags); mRequestRecords.add(request); processRecord.mRequestRecords.put(request.mToken, request); - // We don't set the status of the new request to ACTIVE here as it will be set in - // commitPendingState(). - updatePendingStateLocked(); + 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 */); + } } notifyRequestsOfStatusChangeIfNeeded(); 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 26a549d776643..a97ea268b1c86 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java @@ -273,6 +273,87 @@ public final class DeviceStateManagerServiceTest { DEFAULT_DEVICE_STATE.getIdentifier()); } + @Test + public void requestState_pendingStateAtRequest() throws RemoteException { + TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); + mService.getBinderService().registerCallback(callback); + + mPolicy.blockConfigure(); + + final IBinder firstRequestToken = new Binder(); + final IBinder secondRequestToken = new Binder(); + assertEquals(callback.getLastNotifiedStatus(firstRequestToken), + TestDeviceStateManagerCallback.STATUS_UNKNOWN); + assertEquals(callback.getLastNotifiedStatus(secondRequestToken), + TestDeviceStateManagerCallback.STATUS_UNKNOWN); + + mService.getBinderService().requestState(firstRequestToken, + OTHER_DEVICE_STATE.getIdentifier(), 0 /* flags */); + + assertEquals(mService.getCommittedState(), DEFAULT_DEVICE_STATE); + assertEquals(mService.getPendingState().get(), OTHER_DEVICE_STATE); + assertEquals(mService.getBaseState(), DEFAULT_DEVICE_STATE); + assertEquals(mPolicy.getMostRecentRequestedStateToConfigure(), + OTHER_DEVICE_STATE.getIdentifier()); + + mService.getBinderService().requestState(secondRequestToken, + DEFAULT_DEVICE_STATE.getIdentifier(), 0 /* flags */); + + mPolicy.resumeConfigureOnce(); + + // First request status is now suspended as there is another pending request. + assertEquals(callback.getLastNotifiedStatus(firstRequestToken), + TestDeviceStateManagerCallback.STATUS_SUSPENDED); + // Second request status still unknown because the service is still awaiting policy + // callback. + assertEquals(callback.getLastNotifiedStatus(secondRequestToken), + TestDeviceStateManagerCallback.STATUS_UNKNOWN); + + assertEquals(mService.getCommittedState(), OTHER_DEVICE_STATE); + assertEquals(mService.getPendingState().get(), DEFAULT_DEVICE_STATE); + assertEquals(mService.getBaseState(), DEFAULT_DEVICE_STATE); + assertEquals(mPolicy.getMostRecentRequestedStateToConfigure(), + DEFAULT_DEVICE_STATE.getIdentifier()); + + mPolicy.resumeConfigure(); + + assertEquals(mService.getCommittedState(), DEFAULT_DEVICE_STATE); + assertEquals(mService.getPendingState(), Optional.empty()); + assertEquals(mService.getBaseState(), DEFAULT_DEVICE_STATE); + assertEquals(mPolicy.getMostRecentRequestedStateToConfigure(), + DEFAULT_DEVICE_STATE.getIdentifier()); + + // Now cancel the second request to make the first request active. + mService.getBinderService().cancelRequest(secondRequestToken); + + assertEquals(callback.getLastNotifiedStatus(firstRequestToken), + TestDeviceStateManagerCallback.STATUS_ACTIVE); + assertEquals(callback.getLastNotifiedStatus(secondRequestToken), + TestDeviceStateManagerCallback.STATUS_CANCELED); + + assertEquals(mService.getCommittedState(), OTHER_DEVICE_STATE); + assertEquals(mService.getPendingState(), Optional.empty()); + assertEquals(mService.getBaseState(), DEFAULT_DEVICE_STATE); + assertEquals(mPolicy.getMostRecentRequestedStateToConfigure(), + OTHER_DEVICE_STATE.getIdentifier()); + } + + @Test + public void requestState_sameAsBaseState() throws RemoteException { + TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); + mService.getBinderService().registerCallback(callback); + + final IBinder token = new Binder(); + assertEquals(callback.getLastNotifiedStatus(token), + TestDeviceStateManagerCallback.STATUS_UNKNOWN); + + mService.getBinderService().requestState(token, DEFAULT_DEVICE_STATE.getIdentifier(), + 0 /* flags */); + + assertEquals(callback.getLastNotifiedStatus(token), + TestDeviceStateManagerCallback.STATUS_ACTIVE); + } + @Test public void requestState_flagCancelWhenBaseChanges() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); @@ -407,6 +488,14 @@ public final class DeviceStateManagerServiceTest { } } + public void resumeConfigureOnce() { + if (mPendingConfigureCompleteRunnable != null) { + Runnable onComplete = mPendingConfigureCompleteRunnable; + mPendingConfigureCompleteRunnable = null; + onComplete.run(); + } + } + public int getMostRecentRequestedStateToConfigure() { return mLastDeviceStateRequestedToConfigure; }