From c43ded63298022c400722ab1ac571f0d2397d812 Mon Sep 17 00:00:00 2001 From: Kenneth Ford Date: Fri, 12 May 2023 01:18:06 +0000 Subject: [PATCH] Fixes IllegalStateException through getSupportedStates Fixes an IllegalStateException by returning a DeviceStateInfo even if there is no base state or committed state present. Previous changes made this safer through gating callback responses with base and committed state checks. This exception would occur previously by requesting the DeviceStateInfo before a state has been committed on the device. Bug: 260876135 Test: DeviceStateManagerServiceTest#getDeviceStateInfo_baseStateAndCommittedStateNotSet Change-Id: I3ef620d523d590b0b482f331af984998f60e3766 --- .../devicestate/IDeviceStateManager.aidl | 9 ++++++- .../DeviceStateManagerService.java | 22 +++++++++++------ .../DeviceStateManagerServiceTest.java | 24 +++++++++++++++---- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/core/java/android/hardware/devicestate/IDeviceStateManager.aidl b/core/java/android/hardware/devicestate/IDeviceStateManager.aidl index 099316099738a..3e72d81d88cfb 100644 --- a/core/java/android/hardware/devicestate/IDeviceStateManager.aidl +++ b/core/java/android/hardware/devicestate/IDeviceStateManager.aidl @@ -21,7 +21,14 @@ import android.hardware.devicestate.IDeviceStateManagerCallback; /** @hide */ interface IDeviceStateManager { - /** Returns the current device state info. */ + /** + * Returns the current device state info. This {@link DeviceStateInfo} object will always + * include the list of supported states. If there has been no base state or committed state + * provided yet to the system server, this {@link DeviceStateInfo} object will include + * {@link DeviceStateManager#INVALID_DEVICE_STATE} for each respectively. + * + * This method should not be used to notify callback clients. + */ DeviceStateInfo getDeviceStateInfo(); /** diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index c78229b9e1256..eb7fa1069b7b2 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -342,16 +342,20 @@ public final class DeviceStateManagerService extends SystemService { return supportedStates; } + /** + * Returns the current {@link DeviceStateInfo} of the device. If there has been no base state + * or committed state provided, {@link DeviceStateManager#INVALID_DEVICE_STATE} will be returned + * respectively. The supported states will always be included. + * + */ + @GuardedBy("mLock") @NonNull private DeviceStateInfo getDeviceStateInfoLocked() { - if (!mBaseState.isPresent() || !mCommittedState.isPresent()) { - throw new IllegalStateException("Trying to get the current DeviceStateInfo before the" - + " initial state has been committed."); - } - final int[] supportedStates = getSupportedStateIdentifiersLocked(); - final int baseState = mBaseState.get().getIdentifier(); - final int currentState = mCommittedState.get().getIdentifier(); + final int baseState = + mBaseState.isPresent() ? mBaseState.get().getIdentifier() : INVALID_DEVICE_STATE; + final int currentState = mCommittedState.isPresent() ? mCommittedState.get().getIdentifier() + : INVALID_DEVICE_STATE; return new DeviceStateInfo(supportedStates, baseState, currentState); } @@ -715,6 +719,9 @@ public final class DeviceStateManagerService extends SystemService { } mProcessRecords.put(pid, record); + // Callback clients should not be notified of invalid device states, so calls to + // #getDeviceStateInfoLocked should be gated on checks if a committed state is present + // before getting the device state info. DeviceStateInfo currentInfo = mCommittedState.isPresent() ? getDeviceStateInfoLocked() : null; if (currentInfo != null) { @@ -1128,6 +1135,7 @@ public final class DeviceStateManagerService extends SystemService { /** Implementation of {@link IDeviceStateManager} published as a binder service. */ private final class BinderService extends IDeviceStateManager.Stub { + @Override // Binder call public DeviceStateInfo getDeviceStateInfo() { synchronized (mLock) { 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 4881012833ea7..6684150411295 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java @@ -75,6 +75,10 @@ public final class DeviceStateManagerServiceTest { private static final DeviceState UNSUPPORTED_DEVICE_STATE = new DeviceState(255, "UNSUPPORTED", 0 /* flags */); + private static final int[] SUPPORTED_DEVICE_STATE_IDENTIFIERS = + new int[]{DEFAULT_DEVICE_STATE.getIdentifier(), OTHER_DEVICE_STATE.getIdentifier(), + DEVICE_STATE_CANCEL_WHEN_REQUESTER_NOT_ON_TOP.getIdentifier()}; + private static final int FAKE_PROCESS_ID = 100; private TestDeviceStatePolicy mPolicy; @@ -267,14 +271,26 @@ public final class DeviceStateManagerServiceTest { public void getDeviceStateInfo() throws RemoteException { DeviceStateInfo info = mService.getBinderService().getDeviceStateInfo(); assertNotNull(info); - assertArrayEquals(info.supportedStates, - new int[] { DEFAULT_DEVICE_STATE.getIdentifier(), - OTHER_DEVICE_STATE.getIdentifier(), - DEVICE_STATE_CANCEL_WHEN_REQUESTER_NOT_ON_TOP.getIdentifier()}); + assertArrayEquals(info.supportedStates, SUPPORTED_DEVICE_STATE_IDENTIFIERS); assertEquals(info.baseState, DEFAULT_DEVICE_STATE.getIdentifier()); assertEquals(info.currentState, DEFAULT_DEVICE_STATE.getIdentifier()); } + @Test + public void getDeviceStateInfo_baseStateAndCommittedStateNotSet() throws RemoteException { + // Create a provider and a service without an initial base state. + mProvider = new TestDeviceStateProvider(null /* initialState */); + mPolicy = new TestDeviceStatePolicy(mProvider); + setupDeviceStateManagerService(); + flushHandler(); // Flush the handler to ensure the initial values are committed. + + DeviceStateInfo info = mService.getBinderService().getDeviceStateInfo(); + + assertArrayEquals(info.supportedStates, SUPPORTED_DEVICE_STATE_IDENTIFIERS); + assertEquals(info.baseState, INVALID_DEVICE_STATE); + assertEquals(info.currentState, INVALID_DEVICE_STATE); + } + @FlakyTest(bugId = 223153452) @Test public void registerCallback() throws RemoteException {