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
This commit is contained in:
Darryl L Johnson
2021-02-10 23:16:32 -08:00
parent 77beac00ba
commit ab8910cee0
2 changed files with 102 additions and 4 deletions

View File

@@ -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();

View File

@@ -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;
}