From 08d9bca51c3d4f57b818f7303849941d70514260 Mon Sep 17 00:00:00 2001 From: Jiaming Liu Date: Thu, 11 May 2023 22:26:45 +0000 Subject: [PATCH] Fix IllegalStateException in DeviceStateManagerService When the base state is unknown, DeviceStateManagerService should not notify listeners. Bug: 260876135 Test: atest DeviceStateManagerServiceTest Change-Id: Ib56e2794eacb88fdfdc8a5252c1a8a3fbd3871b8 --- .../DeviceStateManagerService.java | 30 ++++++++-------- .../DeviceStateManagerServiceTest.java | 36 +++++++++++++++++-- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index 964569008accc..fb0fc439e3ef0 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -203,7 +203,7 @@ public final class DeviceStateManagerService extends SystemService { @VisibleForTesting DeviceStateManagerService(@NonNull Context context, @NonNull DeviceStatePolicy policy, - @NonNull SystemPropertySetter systemPropertySetter) { + @NonNull SystemPropertySetter systemPropertySetter) { super(context); mSystemPropertySetter = systemPropertySetter; // We use the DisplayThread because this service indirectly drives @@ -389,12 +389,7 @@ public final class DeviceStateManagerService extends SystemService { setRearDisplayStateLocked(); - 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(); - } + notifyDeviceStateInfoChangedAsync(); mHandler.post(this::notifyPolicyIfNeeded); } @@ -458,12 +453,7 @@ public final class DeviceStateManagerService extends SystemService { mOverrideRequestController.handleBaseStateChanged(identifier); 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(); - } + notifyDeviceStateInfoChangedAsync(); mHandler.post(this::notifyPolicyIfNeeded); } @@ -591,6 +581,18 @@ public final class DeviceStateManagerService extends SystemService { private void notifyDeviceStateInfoChangedAsync() { synchronized (mLock) { + if (mPendingState.isPresent()) { + Slog.i(TAG, + "Cannot notify device state info change when pending state is present."); + return; + } + + if (!mBaseState.isPresent() || !mCommittedState.isPresent()) { + Slog.e(TAG, "Cannot notify device state info change before the initial state has" + + " been committed."); + return; + } + if (mProcessRecords.size() == 0) { return; } @@ -656,7 +658,7 @@ public final class DeviceStateManagerService extends SystemService { } } else { throw new IllegalArgumentException( - "Unknown OverrideRest type: " + request.getRequestType()); + "Unknown OverrideRest type: " + request.getRequestType()); } boolean updatedPendingState = updatePendingStateLocked(); 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 4d068554129fc..4881012833ea7 100644 --- a/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/devicestate/DeviceStateManagerServiceTest.java @@ -88,6 +88,11 @@ public final class DeviceStateManagerServiceTest { mProvider = new TestDeviceStateProvider(); mPolicy = new TestDeviceStatePolicy(mProvider); mSysPropSetter = new TestSystemPropertySetter(); + setupDeviceStateManagerService(); + flushHandler(); // Flush the handler to ensure the initial values are committed. + } + + private void setupDeviceStateManagerService() { mService = new DeviceStateManagerService(InstrumentationRegistry.getContext(), mPolicy, mSysPropSetter); @@ -97,8 +102,6 @@ public final class DeviceStateManagerServiceTest { when(mService.mActivityTaskManagerInternal.getTopApp()) .thenReturn(mWindowProcessController); when(mWindowProcessController.getPid()).thenReturn(FAKE_PROCESS_ID); - - flushHandler(); // Flush the handler to ensure the initial values are committed. } private void flushHandler() { @@ -324,6 +327,21 @@ public final class DeviceStateManagerServiceTest { DEFAULT_DEVICE_STATE.getIdentifier()); } + @Test + public void registerCallback_initialValueUnavailable() 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. + + TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); + mService.getBinderService().registerCallback(callback); + flushHandler(); + // The callback should never be called when the base state is not set yet. + assertNull(callback.getLastNotifiedInfo()); + } + @Test public void requestState() throws RemoteException { TestDeviceStateManagerCallback callback = new TestDeviceStateManagerCallback(); @@ -939,8 +957,18 @@ public final class DeviceStateManagerServiceTest { DEFAULT_DEVICE_STATE, OTHER_DEVICE_STATE, DEVICE_STATE_CANCEL_WHEN_REQUESTER_NOT_ON_TOP}; + + @Nullable private final DeviceState mInitialState; private Listener mListener; + private TestDeviceStateProvider() { + this(DEFAULT_DEVICE_STATE); + } + + private TestDeviceStateProvider(@Nullable DeviceState initialState) { + mInitialState = initialState; + } + @Override public void setListener(Listener listener) { if (mListener != null) { @@ -950,7 +978,9 @@ public final class DeviceStateManagerServiceTest { mListener = listener; mListener.onSupportedDeviceStatesChanged(mSupportedDeviceStates, SUPPORTED_DEVICE_STATES_CHANGED_INITIALIZED); - mListener.onStateChanged(mSupportedDeviceStates[0].getIdentifier()); + if (mInitialState != null) { + mListener.onStateChanged(mInitialState.getIdentifier()); + } } public void notifySupportedDeviceStates(DeviceState[] supportedDeviceStates) {