From 20fddbc609559e439fb38f46669a6bc914aa7357 Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Mon, 4 Oct 2021 22:32:31 +0800 Subject: [PATCH] [RESTRICT AUTOMERGE] Send WindowContext config only when Display is active WMS may provide stale configuration value when secondary display is off. This CL makes WindowContext only receives Configuration updates if the display of associated WindowContainer is not suspended. Test: atest WindowContextListenerControllerTests Test: atest InputMethodMenuControllerTest Bug: 198965093 Change-Id: I3c2871823f1b104f89bb801f3bf5ecef01966bb3 --- .../com/android/server/wm/DisplayContent.java | 20 +++-- .../wm/WindowContextListenerController.java | 43 +++++++--- .../wm/InputMethodMenuControllerTest.java | 4 +- .../WindowContextListenerControllerTests.java | 85 +++++++++++++++++-- 4 files changed, 127 insertions(+), 25 deletions(-) diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 36579d96d6c50..21c67d953d94b 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -44,6 +44,8 @@ import static android.view.Display.FLAG_PRIVATE; import static android.view.Display.FLAG_SHOULD_SHOW_SYSTEM_DECORATIONS; import static android.view.Display.INVALID_DISPLAY; import static android.view.Display.REMOVE_MODE_DESTROY_CONTENT; +import static android.view.Display.STATE_UNKNOWN; +import static android.view.Display.isSuspendedState; import static android.view.InsetsState.ITYPE_IME; import static android.view.InsetsState.ITYPE_LEFT_GESTURES; import static android.view.InsetsState.ITYPE_NAVIGATION_BAR; @@ -322,11 +324,6 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp */ private Rect mLastMirroredDisplayAreaBounds = null; - /** - * The last state of the display. - */ - private int mLastDisplayState; - // Contains all IME window containers. Note that the z-ordering of the IME windows will depend // on the IME target. We mainly have this container grouping so we can keep track of all the IME // window containers together and move them in-sync if/when needed. We use a subclass of @@ -5623,12 +5620,13 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp void onDisplayChanged() { mDisplay.getRealSize(mTmpDisplaySize); setBounds(0, 0, mTmpDisplaySize.x, mTmpDisplaySize.y); + final int lastDisplayState = mDisplayInfo.state; updateDisplayInfo(); // The window policy is responsible for stopping activities on the default display. final int displayId = mDisplay.getDisplayId(); + final int displayState = mDisplayInfo.state; if (displayId != DEFAULT_DISPLAY) { - final int displayState = mDisplay.getState(); if (displayState == Display.STATE_OFF) { mOffTokenAcquirer.acquire(mDisplayId); } else if (displayState == Display.STATE_ON) { @@ -5637,12 +5635,18 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp ProtoLog.v(WM_DEBUG_LAYER_MIRRORING, "Display %d state is now (%d), so update layer mirroring?", mDisplayId, displayState); - if (mLastDisplayState != displayState) { + if (lastDisplayState != displayState) { // If state is on due to surface being added, then start layer mirroring. // If state is off due to surface being removed, then stop layer mirroring. updateMirroring(); } - mLastDisplayState = displayState; + } + // Dispatch pending Configuration to WindowContext if the associated display changes to + // un-suspended state from suspended. + if (isSuspendedState(lastDisplayState) + && !isSuspendedState(displayState) && displayState != STATE_UNKNOWN) { + mWmService.mWindowContextListenerController + .dispatchPendingConfigurationIfNeeded(mDisplayId); } mWmService.requestTraversal(); } diff --git a/services/core/java/com/android/server/wm/WindowContextListenerController.java b/services/core/java/com/android/server/wm/WindowContextListenerController.java index 86e356a876b54..cc527136eb518 100644 --- a/services/core/java/com/android/server/wm/WindowContextListenerController.java +++ b/services/core/java/com/android/server/wm/WindowContextListenerController.java @@ -17,7 +17,9 @@ package com.android.server.wm; import static android.view.Display.INVALID_DISPLAY; +import static android.view.Display.isSuspendedState; import static android.view.WindowManager.LayoutParams.INVALID_WINDOW_TYPE; +import static android.window.WindowProviderService.isWindowProviderService; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_ADD_REMOVE; import static com.android.internal.protolog.ProtoLogGroup.WM_ERROR; @@ -80,7 +82,7 @@ class WindowContextListenerController { * @param options a bundle used to pass window-related options. */ void registerWindowContainerListener(@NonNull IBinder clientToken, - @NonNull WindowContainer container, int ownerUid, @WindowType int type, + @NonNull WindowContainer container, int ownerUid, @WindowType int type, @Nullable Bundle options) { WindowContextListenerImpl listener = mListeners.get(clientToken); if (listener == null) { @@ -103,6 +105,16 @@ class WindowContextListenerController { listener.unregister(); } + void dispatchPendingConfigurationIfNeeded(int displayId) { + for (int i = mListeners.size() - 1; i >= 0; --i) { + final WindowContextListenerImpl listener = mListeners.valueAt(i); + if (listener.getWindowContainer().getDisplayContent().getDisplayId() == displayId + && listener.mHasPendingConfiguration) { + listener.reportConfigToWindowTokenClient(); + } + } + } + /** * Verifies if the caller is allowed to do the operation to the listener specified by * {@code clientToken}. @@ -138,7 +150,7 @@ class WindowContextListenerController { return listener != null ? listener.mOptions : null; } - @Nullable WindowContainer getContainer(IBinder clientToken) { + @Nullable WindowContainer getContainer(IBinder clientToken) { final WindowContextListenerImpl listener = mListeners.get(clientToken); return listener != null ? listener.mContainer : null; } @@ -163,7 +175,7 @@ class WindowContextListenerController { class WindowContextListenerImpl implements WindowContainerListener { @NonNull private final IBinder mClientToken; private final int mOwnerUid; - @NonNull private WindowContainer mContainer; + @NonNull private WindowContainer mContainer; /** * The options from {@link Context#createWindowContext(int, Bundle)}. *

It can be used for choosing the {@link DisplayArea} where the window context @@ -177,7 +189,9 @@ class WindowContextListenerController { private int mLastReportedDisplay = INVALID_DISPLAY; private Configuration mLastReportedConfig; - private WindowContextListenerImpl(IBinder clientToken, WindowContainer container, + private boolean mHasPendingConfiguration; + + private WindowContextListenerImpl(IBinder clientToken, WindowContainer container, int ownerUid, @WindowType int type, @Nullable Bundle options) { mClientToken = clientToken; mContainer = Objects.requireNonNull(container); @@ -197,11 +211,11 @@ class WindowContextListenerController { /** TEST ONLY: returns the {@link WindowContainer} of the listener */ @VisibleForTesting - WindowContainer getWindowContainer() { + WindowContainer getWindowContainer() { return mContainer; } - private void updateContainer(@NonNull WindowContainer newContainer) { + private void updateContainer(@NonNull WindowContainer newContainer) { Objects.requireNonNull(newContainer); if (mContainer.equals(newContainer)) { @@ -246,12 +260,20 @@ class WindowContextListenerController { if (mDeathRecipient == null) { throw new IllegalStateException("Invalid client token: " + mClientToken); } - - if (mLastReportedConfig == null) { - mLastReportedConfig = new Configuration(); + // If the display of window context associated window container is suspended, don't + // report the configuration update. Note that we still dispatch the configuration update + // to WindowProviderService to make it compatible with Service#onConfigurationChanged. + // Service always receives #onConfigurationChanged callback regardless of display state. + if (!isWindowProviderService(mOptions) + && isSuspendedState(mContainer.getDisplayContent().getDisplayInfo().state)) { + mHasPendingConfiguration = true; + return; } final Configuration config = mContainer.getConfiguration(); final int displayId = mContainer.getDisplayContent().getDisplayId(); + if (mLastReportedConfig == null) { + mLastReportedConfig = new Configuration(); + } if (config.equals(mLastReportedConfig) && displayId == mLastReportedDisplay) { // No changes since last reported time. return; @@ -266,6 +288,7 @@ class WindowContextListenerController { } catch (RemoteException e) { ProtoLog.w(WM_ERROR, "Could not report config changes to the window token client."); } + mHasPendingConfiguration = false; } @Override @@ -283,7 +306,7 @@ class WindowContextListenerController { // If we cannot obtain the DisplayContent, the DisplayContent may also be removed. // We should proceed the removal process. if (dc != null) { - final DisplayArea da = dc.findAreaForToken(windowToken); + final DisplayArea da = dc.findAreaForToken(windowToken); updateContainer(da); return; } diff --git a/services/tests/wmtests/src/com/android/server/wm/InputMethodMenuControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/InputMethodMenuControllerTest.java index d7daa57cc9da0..407f9cfdbe3e4 100644 --- a/services/tests/wmtests/src/com/android/server/wm/InputMethodMenuControllerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/InputMethodMenuControllerTest.java @@ -17,6 +17,7 @@ package com.android.server.wm; import static android.view.Display.DEFAULT_DISPLAY; +import static android.view.Display.STATE_ON; import static android.view.WindowManager.LayoutParams.TYPE_INPUT_METHOD_DIALOG; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; @@ -70,7 +71,7 @@ public class InputMethodMenuControllerTest extends WindowTestsBase { @Before public void setUp() throws Exception { - // Let the Display to be created with the DualDisplay policy. + // Let the Display be created with the DualDisplay policy. final DisplayAreaPolicy.Provider policyProvider = new DualDisplayAreaGroupPolicyTest.DualDisplayTestPolicyProvider(); Mockito.doReturn(policyProvider).when(mWm).getDisplayAreaPolicyProvider(); @@ -78,6 +79,7 @@ public class InputMethodMenuControllerTest extends WindowTestsBase { mController = new InputMethodMenuController(mock(InputMethodManagerService.class)); mSecondaryDisplay = new DualDisplayAreaGroupPolicyTest.DualDisplayContent .Builder(mAtm, 1000, 1000).build(); + mSecondaryDisplay.getDisplayInfo().state = STATE_ON; // Mock addWindowTokenWithOptions to create a test window token. mIWindowManager = WindowManagerGlobal.getWindowManagerService(); diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContextListenerControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContextListenerControllerTests.java index e5eba57f223db..646647fcc4ca9 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContextListenerControllerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContextListenerControllerTests.java @@ -17,22 +17,35 @@ package com.android.server.wm; import static android.view.Display.DEFAULT_DISPLAY; +import static android.view.Display.STATE_OFF; +import static android.view.Display.STATE_ON; import static android.view.WindowManager.LayoutParams.TYPE_ACCESSIBILITY_MAGNIFICATION_OVERLAY; import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_OVERLAY; import static android.view.WindowManager.LayoutParams.TYPE_INPUT_METHOD_DIALOG; +import static android.window.WindowProvider.KEY_IS_WINDOW_PROVIDER_SERVICE; + +import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import android.app.IWindowToken; import android.content.res.Configuration; import android.graphics.Rect; import android.os.Binder; +import android.os.Bundle; import android.os.IBinder; import android.platform.test.annotations.Presubmit; +import android.view.Display; +import android.view.DisplayInfo; import androidx.test.filters.SmallTest; @@ -55,12 +68,15 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { private static final int ANOTHER_UID = 1000; private final IBinder mClientToken = new Binder(); - private WindowContainer mContainer; + private WindowContainer mContainer; @Before public void setUp() { mController = new WindowContextListenerController(); mContainer = createTestWindowToken(TYPE_APPLICATION_OVERLAY, mDisplayContent); + // Make display on to verify configuration propagation. + mDefaultDisplay.getDisplayInfo().state = STATE_ON; + mDisplayContent.getDisplayInfo().state = STATE_ON; } @Test @@ -76,7 +92,7 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { assertEquals(2, mController.mListeners.size()); - final WindowContainer container = createTestWindowToken(TYPE_APPLICATION_OVERLAY, + final WindowContainer container = createTestWindowToken(TYPE_APPLICATION_OVERLAY, mDefaultDisplay); mController.registerWindowContainerListener(mClientToken, container, -1, TYPE_APPLICATION_OVERLAY, null /* options */); @@ -89,6 +105,7 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { assertEquals(container, listener.getWindowContainer()); } + @UseTestDisplay @Test public void testRegisterWindowContextListenerClientConfigPropagation() { final TestWindowTokenClient clientToken = new TestWindowTokenClient(); @@ -107,7 +124,7 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { assertEquals(mDisplayContent.mDisplayId, clientToken.mDisplayId); // Update the WindowContainer. - final WindowContainer container = createTestWindowToken(TYPE_APPLICATION_OVERLAY, + final WindowContainer container = createTestWindowToken(TYPE_APPLICATION_OVERLAY, mDefaultDisplay); final Configuration config2 = container.getConfiguration(); final Rect bounds2 = new Rect(0, 0, 20, 20); @@ -174,7 +191,7 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { .setDisplayContent(mDefaultDisplay) .setFromClientToken(true) .build(); - final DisplayArea da = windowContextCreatedToken.getDisplayArea(); + final DisplayArea da = windowContextCreatedToken.getDisplayArea(); mController.registerWindowContainerListener(mClientToken, windowContextCreatedToken, TEST_UID, TYPE_ACCESSIBILITY_MAGNIFICATION_OVERLAY, null /* options */); @@ -192,11 +209,12 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { // Let the Display to be created with the DualDisplay policy. final DisplayAreaPolicy.Provider policyProvider = new DualDisplayAreaGroupPolicyTest.DualDisplayTestPolicyProvider(); - Mockito.doReturn(policyProvider).when(mWm).getDisplayAreaPolicyProvider(); + doReturn(policyProvider).when(mWm).getDisplayAreaPolicyProvider(); // Create a DisplayContent with dual RootDisplayArea DualDisplayAreaGroupPolicyTest.DualDisplayContent dualDisplayContent = new DualDisplayAreaGroupPolicyTest.DualDisplayContent .Builder(mAtm, 1000, 1000).build(); + dualDisplayContent.getDisplayInfo().state = STATE_ON; final DisplayArea.Tokens imeContainer = dualDisplayContent.getImeContainer(); // Put the ImeContainer to the first sub-RootDisplayArea dualDisplayContent.mFirstRoot.placeImeContainer(imeContainer); @@ -222,7 +240,62 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { assertThat(mController.getContainer(mClientToken)).isEqualTo(imeContainer); } - private class TestWindowTokenClient extends IWindowToken.Stub { + @Test + public void testConfigUpdateForSuspendedWindowContext() { + final TestWindowTokenClient mockToken = new TestWindowTokenClient(); + spyOn(mockToken); + + mContainer.getDisplayContent().getDisplayInfo().state = STATE_OFF; + + final Configuration config1 = mContainer.getConfiguration(); + final Rect bounds1 = new Rect(0, 0, 10, 10); + config1.windowConfiguration.setBounds(bounds1); + config1.densityDpi = 100; + mContainer.onRequestedOverrideConfigurationChanged(config1); + + mController.registerWindowContainerListener(mockToken, mContainer, -1, + TYPE_APPLICATION_OVERLAY, null /* options */); + + verify(mockToken, never()).onConfigurationChanged(any(), anyInt()); + + // Turn on the display and verify if the client receive the callback + Display display = mContainer.getDisplayContent().getDisplay(); + spyOn(display); + Mockito.doAnswer(invocation -> { + final DisplayInfo info = mContainer.getDisplayContent().getDisplayInfo(); + info.state = STATE_ON; + ((DisplayInfo) invocation.getArgument(0)).copyFrom(info); + return null; + }).when(display).getDisplayInfo(any(DisplayInfo.class)); + + mContainer.getDisplayContent().onDisplayChanged(); + + assertThat(mockToken.mConfiguration).isEqualTo(config1); + assertThat(mockToken.mDisplayId).isEqualTo(mContainer.getDisplayContent().getDisplayId()); + } + + @Test + public void testReportConfigUpdateForSuspendedWindowProviderService() { + final TestWindowTokenClient clientToken = new TestWindowTokenClient(); + final Bundle options = new Bundle(); + options.putBoolean(KEY_IS_WINDOW_PROVIDER_SERVICE, true); + + mContainer.getDisplayContent().getDisplayInfo().state = STATE_OFF; + + final Configuration config1 = mContainer.getConfiguration(); + final Rect bounds1 = new Rect(0, 0, 10, 10); + config1.windowConfiguration.setBounds(bounds1); + config1.densityDpi = 100; + mContainer.onRequestedOverrideConfigurationChanged(config1); + + mController.registerWindowContainerListener(clientToken, mContainer, -1, + TYPE_APPLICATION_OVERLAY, options); + + assertThat(clientToken.mConfiguration).isEqualTo(config1); + assertThat(clientToken.mDisplayId).isEqualTo(mDisplayContent.mDisplayId); + } + + private static class TestWindowTokenClient extends IWindowToken.Stub { private Configuration mConfiguration; private int mDisplayId; private boolean mRemoved;