Synchronize device state update and display update callbacks
Adds a call from DisplayManager to WindowManager
that notifies WindowManager just before applying
the device state to the displays.
This is needed so we can guarantee the order of the
callbacks on the WindowManager side: now the device
state callback should be always executed before
display change callbacks.
This should fix race conditions in
PhysicalDisplaySwitchTransitionLauncher where
sometimes mIsFolded wasn't updated before
display switch and DisplayRotation where seems
like we could have a similar issue.
Test: atest DisplayManagerServiceTest
Test: manual fold/unfolds
Bug: 278040168
Change-Id: I82e297309125d8fda2e3ea3687971d345db2c0cd
diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java
index 3844529..da8eb23 100644
--- a/services/core/java/com/android/server/display/DisplayManagerService.java
+++ b/services/core/java/com/android/server/display/DisplayManagerService.java
@@ -238,6 +238,7 @@
private static final int MSG_LOAD_BRIGHTNESS_CONFIGURATIONS = 6;
private static final int MSG_DELIVER_DISPLAY_EVENT_FRAME_RATE_OVERRIDE = 7;
private static final int MSG_DELIVER_DISPLAY_GROUP_EVENT = 8;
+ private static final int MSG_RECEIVED_DEVICE_STATE = 9;
private static final int[] EMPTY_ARRAY = new int[0];
private static final HdrConversionMode HDR_CONVERSION_MODE_UNSUPPORTED = new HdrConversionMode(
HDR_CONVERSION_UNSUPPORTED);
@@ -3213,6 +3214,10 @@
mWindowManagerInternal.requestTraversalFromDisplayManager();
break;
+ case MSG_RECEIVED_DEVICE_STATE:
+ mWindowManagerInternal.onDisplayManagerReceivedDeviceState(msg.arg1);
+ break;
+
case MSG_UPDATE_VIEWPORT: {
final boolean changed;
synchronized (mSyncRoot) {
@@ -4680,6 +4685,14 @@
public void onStateChanged(int deviceState) {
boolean isDeviceStateOverrideActive = deviceState != mBaseState;
synchronized (mSyncRoot) {
+ // Notify WindowManager that we are about to handle new device state, this should
+ // be sent before any work related to the device state in DisplayManager, so
+ // WindowManager could do implement that depends on the device state and display
+ // changes (serializes device state update and display change events)
+ Message msg = mHandler.obtainMessage(MSG_RECEIVED_DEVICE_STATE);
+ msg.arg1 = deviceState;
+ mHandler.sendMessage(msg);
+
mLogicalDisplayMapper
.setDeviceStateLocked(deviceState, isDeviceStateOverrideActive);
}
diff --git a/services/core/java/com/android/server/wm/DeviceStateController.java b/services/core/java/com/android/server/wm/DeviceStateController.java
index 41c052d..31b1069 100644
--- a/services/core/java/com/android/server/wm/DeviceStateController.java
+++ b/services/core/java/com/android/server/wm/DeviceStateController.java
@@ -19,9 +19,6 @@
import android.annotation.CallbackExecutor;
import android.annotation.NonNull;
import android.content.Context;
-import android.hardware.devicestate.DeviceStateManager;
-import android.os.Handler;
-import android.os.HandlerExecutor;
import android.util.ArrayMap;
import com.android.internal.R;
@@ -36,17 +33,15 @@
import java.util.function.Consumer;
/**
- * Class that registers callbacks with the {@link DeviceStateManager} and responds to device
+ * Class that listens for a callback from display manager and responds to device state
* changes.
*/
-final class DeviceStateController implements DeviceStateManager.DeviceStateCallback {
+final class DeviceStateController {
// Used to synchronize WindowManager services call paths with DeviceStateManager's callbacks.
@NonNull
private final WindowManagerGlobalLock mWmLock;
@NonNull
- private final DeviceStateManager mDeviceStateManager;
- @NonNull
private final int[] mOpenDeviceStates;
@NonNull
private final int[] mHalfFoldedDeviceStates;
@@ -77,10 +72,8 @@
CONCURRENT,
}
- DeviceStateController(@NonNull Context context, @NonNull Handler handler,
- @NonNull WindowManagerGlobalLock wmLock) {
+ DeviceStateController(@NonNull Context context, @NonNull WindowManagerGlobalLock wmLock) {
mWmLock = wmLock;
- mDeviceStateManager = context.getSystemService(DeviceStateManager.class);
mOpenDeviceStates = context.getResources()
.getIntArray(R.array.config_openDeviceStates);
@@ -97,10 +90,6 @@
mMatchBuiltInDisplayOrientationToDefaultDisplay = context.getResources()
.getBoolean(R.bool
.config_matchSecondaryInternalDisplaysOrientationToReverseDefaultDisplay);
-
- if (mDeviceStateManager != null) {
- mDeviceStateManager.registerCallback(new HandlerExecutor(handler), this);
- }
}
/**
@@ -137,8 +126,19 @@
return mMatchBuiltInDisplayOrientationToDefaultDisplay;
}
- @Override
- public void onStateChanged(int state) {
+ /**
+ * This event is sent from DisplayManager just before the device state is applied to
+ * the displays. This is needed to make sure that we first receive this callback before
+ * any device state related display updates from the DisplayManager.
+ *
+ * The flow for this event is the following:
+ * - {@link DeviceStateManager} sends event to {@link android.hardware.display.DisplayManager}
+ * - {@link android.hardware.display.DisplayManager} sends it to {@link WindowManagerInternal}
+ * - {@link WindowManagerInternal} eventually calls this method
+ *
+ * @param state device state as defined by {@link DeviceStateManager}
+ */
+ public void onDeviceStateReceivedByDisplayManager(int state) {
mCurrentState = state;
final DeviceState deviceState;
diff --git a/services/core/java/com/android/server/wm/DisplayRotation.java b/services/core/java/com/android/server/wm/DisplayRotation.java
index bc1ddf8..8be36f0 100644
--- a/services/core/java/com/android/server/wm/DisplayRotation.java
+++ b/services/core/java/com/android/server/wm/DisplayRotation.java
@@ -1730,7 +1730,9 @@
}
/**
- * Called by the DeviceStateManager callback when the device state changes.
+ * Called by the display manager just before it applied the device state, it is guaranteed
+ * that in case of physical display change the {@link DisplayRotation#physicalDisplayChanged}
+ * method will be invoked *after* this one.
*/
void foldStateChanged(DeviceStateController.DeviceState deviceState) {
if (mFoldController != null) {
diff --git a/services/core/java/com/android/server/wm/PhysicalDisplaySwitchTransitionLauncher.java b/services/core/java/com/android/server/wm/PhysicalDisplaySwitchTransitionLauncher.java
index 7852249..f0757db 100644
--- a/services/core/java/com/android/server/wm/PhysicalDisplaySwitchTransitionLauncher.java
+++ b/services/core/java/com/android/server/wm/PhysicalDisplaySwitchTransitionLauncher.java
@@ -71,7 +71,10 @@
}
/**
- * Called by the DeviceStateManager callback when the state changes.
+ * Called by the display manager just before it applied the device state, it is guaranteed
+ * that in case of physical display change the
+ * {@link PhysicalDisplaySwitchTransitionLauncher#requestDisplaySwitchTransitionIfNeeded}
+ * method will be invoked *after* this one.
*/
void foldStateChanged(DeviceState newDeviceState) {
boolean isUnfolding = mDeviceState == FOLDED
diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java
index 5369159..9c35858 100644
--- a/services/core/java/com/android/server/wm/RootWindowContainer.java
+++ b/services/core/java/com/android/server/wm/RootWindowContainer.java
@@ -438,8 +438,7 @@
mTaskSupervisor = mService.mTaskSupervisor;
mTaskSupervisor.mRootWindowContainer = this;
mDisplayOffTokenAcquirer = mService.new SleepTokenAcquirerImpl(DISPLAY_OFF_SLEEP_TOKEN_TAG);
- mDeviceStateController = new DeviceStateController(service.mContext, service.mH,
- service.mGlobalLock);
+ mDeviceStateController = new DeviceStateController(service.mContext, service.mGlobalLock);
mDisplayRotationCoordinator = new DisplayRotationCoordinator();
}
@@ -1283,6 +1282,15 @@
false /* includingParents */);
}
+ /**
+ * Called just before display manager has applied the device state to the displays
+ * @param deviceState device state as defined by
+ * {@link android.hardware.devicestate.DeviceStateManager}
+ */
+ void onDisplayManagerReceivedDeviceState(int deviceState) {
+ mDeviceStateController.onDeviceStateReceivedByDisplayManager(deviceState);
+ }
+
// TODO(multi-display): Look at all callpoints to make sure they make sense in multi-display.
DisplayContent getDefaultDisplay() {
return mDefaultDisplay;
diff --git a/services/core/java/com/android/server/wm/WindowManagerInternal.java b/services/core/java/com/android/server/wm/WindowManagerInternal.java
index 92d4cec..9e7df00 100644
--- a/services/core/java/com/android/server/wm/WindowManagerInternal.java
+++ b/services/core/java/com/android/server/wm/WindowManagerInternal.java
@@ -370,6 +370,13 @@
public abstract void requestTraversalFromDisplayManager();
/**
+ * Called just before display manager has applied the device state to the displays
+ * @param deviceState device state as defined by
+ * {@link android.hardware.devicestate.DeviceStateManager}
+ */
+ public abstract void onDisplayManagerReceivedDeviceState(int deviceState);
+
+ /**
* Set by the accessibility layer to observe changes in the magnified region,
* rotation, and other window transformations related to display magnification
* as the window manager is responsible for doing the actual magnification
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index e33c6f0..d84c85c 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -7674,6 +7674,15 @@
}
@Override
+ public void onDisplayManagerReceivedDeviceState(int deviceState) {
+ mH.post(() -> {
+ synchronized (mGlobalLock) {
+ mRoot.onDisplayManagerReceivedDeviceState(deviceState);
+ }
+ });
+ }
+
+ @Override
public void setMagnificationSpec(int displayId, MagnificationSpec spec) {
synchronized (mGlobalLock) {
if (mAccessibilityController.hasCallbacks()) {
diff --git a/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java
index a6c5737..0e775d5 100644
--- a/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/display/DisplayManagerServiceTest.java
@@ -35,12 +35,14 @@
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
@@ -72,6 +74,7 @@
import android.hardware.display.VirtualDisplayConfig;
import android.media.projection.IMediaProjection;
import android.media.projection.IMediaProjectionManager;
+import android.os.Binder;
import android.os.Handler;
import android.os.IBinder;
import android.os.MessageQueue;
@@ -96,17 +99,18 @@
import com.android.server.LocalServices;
import com.android.server.SystemService;
import com.android.server.companion.virtual.VirtualDeviceManagerInternal;
+import com.android.server.display.DisplayManagerService.DeviceStateListener;
import com.android.server.display.DisplayManagerService.SyncRoot;
import com.android.server.input.InputManagerInternal;
import com.android.server.lights.LightsManager;
import com.android.server.sensors.SensorManagerInternal;
import com.android.server.wm.WindowManagerInternal;
-import com.google.common.collect.ImmutableMap;
-
import libcore.junit.util.compat.CoreCompatChangeRule.DisableCompatChanges;
import libcore.junit.util.compat.CoreCompatChangeRule.EnableCompatChanges;
+import com.google.common.collect.ImmutableMap;
+
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -114,6 +118,7 @@
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
+import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
@@ -547,6 +552,30 @@
}
/**
+ * Tests that we send the device state to window manager
+ */
+ @Test
+ public void testOnStateChanged_sendsStateChangedEventToWm() throws Exception {
+ DisplayManagerService displayManager =
+ new DisplayManagerService(mContext, mShortMockedInjector);
+ registerDefaultDisplays(displayManager);
+ displayManager.windowManagerAndInputReady();
+ displayManager.onBootPhase(SystemService.PHASE_BOOT_COMPLETED);
+ DeviceStateListener listener = displayManager.new DeviceStateListener();
+ Handler handler = displayManager.getDisplayHandler();
+ IDisplayManagerCallback displayChangesCallback = registerDisplayChangeCallback(
+ displayManager);
+
+ listener.onStateChanged(123);
+ waitForIdleHandler(handler);
+
+ InOrder inOrder = inOrder(mMockWindowManagerInternal, displayChangesCallback);
+ // Verify there are no display events before WM call
+ inOrder.verify(displayChangesCallback, never()).onDisplayEvent(anyInt(), anyInt());
+ inOrder.verify(mMockWindowManagerInternal).onDisplayManagerReceivedDeviceState(123);
+ }
+
+ /**
* Tests that there should be a display change notification to WindowManager to update its own
* internal state for things like display cutout when nonOverrideDisplayInfo is changed.
*/
@@ -2054,6 +2083,15 @@
updateDisplayDeviceInfo(displayManager, displayDevice, displayDeviceInfo);
}
+ private IDisplayManagerCallback registerDisplayChangeCallback(
+ DisplayManagerService displayManager) {
+ IDisplayManagerCallback displayChangesCallback = mock(IDisplayManagerCallback.class);
+ when(displayChangesCallback.asBinder()).thenReturn(new Binder());
+ DisplayManagerService.BinderService binderService = displayManager.new BinderService();
+ binderService.registerCallback(displayChangesCallback);
+ return displayChangesCallback;
+ }
+
private FakeDisplayManagerCallback registerDisplayListenerCallback(
DisplayManagerService displayManager,
DisplayManagerService.BinderService displayManagerBinderService,
diff --git a/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java
index b515a9d..07d8b8a 100644
--- a/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java
@@ -27,7 +27,6 @@
import android.content.Context;
import android.content.res.Resources;
import android.hardware.devicestate.DeviceStateManager;
-import android.os.Handler;
import android.platform.test.annotations.Presubmit;
import androidx.test.filters.SmallTest;
@@ -73,20 +72,19 @@
};
mBuilder.setDelegate(mDelegate);
mBuilder.build();
- verify(mMockDeviceStateManager).registerCallback(any(), any());
}
@Test
public void testInitialization() {
initialize(true /* supportFold */, true /* supportHalfFolded */);
- mTarget.onStateChanged(mOpenDeviceStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mOpenDeviceStates[0]);
assertEquals(DeviceStateController.DeviceState.OPEN, mCurrentState);
}
@Test
public void testInitializationWithNoFoldSupport() {
initialize(false /* supportFold */, false /* supportHalfFolded */);
- mTarget.onStateChanged(mFoldedStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mFoldedStates[0]);
// Note that the folded state is ignored.
assertEquals(DeviceStateController.DeviceState.UNKNOWN, mCurrentState);
}
@@ -94,24 +92,24 @@
@Test
public void testWithFoldSupported() {
initialize(true /* supportFold */, false /* supportHalfFolded */);
- mTarget.onStateChanged(mOpenDeviceStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mOpenDeviceStates[0]);
assertEquals(DeviceStateController.DeviceState.OPEN, mCurrentState);
- mTarget.onStateChanged(mFoldedStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mFoldedStates[0]);
assertEquals(DeviceStateController.DeviceState.FOLDED, mCurrentState);
- mTarget.onStateChanged(mHalfFoldedStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mHalfFoldedStates[0]);
assertEquals(DeviceStateController.DeviceState.UNKNOWN, mCurrentState); // Ignored
}
@Test
public void testWithHalfFoldSupported() {
initialize(true /* supportFold */, true /* supportHalfFolded */);
- mTarget.onStateChanged(mOpenDeviceStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mOpenDeviceStates[0]);
assertEquals(DeviceStateController.DeviceState.OPEN, mCurrentState);
- mTarget.onStateChanged(mFoldedStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mFoldedStates[0]);
assertEquals(DeviceStateController.DeviceState.FOLDED, mCurrentState);
- mTarget.onStateChanged(mHalfFoldedStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mHalfFoldedStates[0]);
assertEquals(DeviceStateController.DeviceState.HALF_FOLDED, mCurrentState);
- mTarget.onStateChanged(mConcurrentDisplayState);
+ mTarget.onDeviceStateReceivedByDisplayManager(mConcurrentDisplayState);
assertEquals(DeviceStateController.DeviceState.CONCURRENT, mCurrentState);
}
@@ -121,15 +119,15 @@
assertEquals(1, mTarget.mDeviceStateCallbacks.size());
assertTrue(mTarget.mDeviceStateCallbacks.containsKey(mDelegate));
- mTarget.onStateChanged(mOpenDeviceStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mOpenDeviceStates[0]);
assertEquals(DeviceStateController.DeviceState.OPEN, mCurrentState);
- mTarget.onStateChanged(mFoldedStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mFoldedStates[0]);
assertEquals(DeviceStateController.DeviceState.FOLDED, mCurrentState);
// The callback should not receive state change when the it is unregistered.
mTarget.unregisterDeviceStateCallback(mDelegate);
assertTrue(mTarget.mDeviceStateCallbacks.isEmpty());
- mTarget.onStateChanged(mOpenDeviceStates[0]);
+ mTarget.onDeviceStateReceivedByDisplayManager(mOpenDeviceStates[0]);
assertEquals(DeviceStateController.DeviceState.FOLDED /* unchanged */, mCurrentState);
}
@@ -195,9 +193,7 @@
Resources mockRes = mock(Resources.class);
when(mMockContext.getResources()).thenReturn((mockRes));
mockFold(mSupportFold, mSupportHalfFold);
- Handler mockHandler = mock(Handler.class);
- mTarget = new DeviceStateController(mMockContext, mockHandler,
- new WindowManagerGlobalLock());
+ mTarget = new DeviceStateController(mMockContext, new WindowManagerGlobalLock());
mTarget.registerDeviceStateCallback(mDelegate, MoreExecutors.directExecutor());
}
}