From 71cb758286938a27d85be09145859a909144eb23 Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Fri, 23 Apr 2021 12:18:15 +0800 Subject: [PATCH] Fix IME switch dialog crash on Dual rootDA display The crash is because when the IME switch dialog created window token is removed, it should switch back to listen to ImeContainer. However, previous CL use the wrong API findAreaForToken, which only finds the DisplayArea.Token under mAreaForLayer list. In this scenario, we cannot find one because ImeContainer is moved to its sub-RootDisplayArea. This CL updates the naming and add more javaDoc to prevent from confusion. Test: atest WindowContextListenerControllerTests Test: manual - repro steps in b/186366728#comment1 fixes: 186366728 Change-Id: I50d3247b0a158e83d09be893d31802d673a60697 --- core/java/android/app/Presentation.java | 5 ++- .../android/internal/util/Preconditions.java | 2 +- .../android/server/wm/DisplayAreaPolicy.java | 4 +- .../server/wm/DisplayAreaPolicyBuilder.java | 16 ++++---- .../com/android/server/wm/DisplayContent.java | 33 +++++++++++------ .../android/server/wm/RootDisplayArea.java | 17 +++++++-- .../wm/WindowContextListenerController.java | 4 +- .../server/wm/WindowManagerService.java | 2 +- .../wm/DualDisplayAreaGroupPolicyTest.java | 14 +++---- .../WindowContextListenerControllerTests.java | 37 +++++++++++++++++++ 10 files changed, 97 insertions(+), 37 deletions(-) diff --git a/core/java/android/app/Presentation.java b/core/java/android/app/Presentation.java index ad903642c908f..e90bf86f175da 100644 --- a/core/java/android/app/Presentation.java +++ b/core/java/android/app/Presentation.java @@ -37,7 +37,8 @@ import android.view.Window; import android.view.WindowManager; import android.view.WindowManager.LayoutParams.WindowType; -import com.android.internal.util.Preconditions; +import java.util.Objects; + /** * Base class for presentations. *

@@ -153,7 +154,7 @@ public class Presentation extends Dialog { private final Display mDisplay; private final DisplayManager mDisplayManager; - private final Handler mHandler = new Handler(Preconditions.checkNotNull(Looper.myLooper(), + private final Handler mHandler = new Handler(Objects.requireNonNull(Looper.myLooper(), "Presentation must be constructed on a looper thread.")); /** diff --git a/core/java/com/android/internal/util/Preconditions.java b/core/java/com/android/internal/util/Preconditions.java index 88e4e355000a5..d2d822083a95e 100644 --- a/core/java/com/android/internal/util/Preconditions.java +++ b/core/java/com/android/internal/util/Preconditions.java @@ -158,7 +158,7 @@ public class Preconditions { * be converted to a string using {@link String#valueOf(Object)} * @return the non-null reference that was validated * @throws NullPointerException if {@code reference} is null - * @deprecated - use {@link java.util.Objects.requireNonNull} instead. + * @deprecated - use {@link java.util.Objects#requireNonNull} instead. */ @Deprecated @UnsupportedAppUsage diff --git a/services/core/java/com/android/server/wm/DisplayAreaPolicy.java b/services/core/java/com/android/server/wm/DisplayAreaPolicy.java index eeb7fac309448..47622bc83417d 100644 --- a/services/core/java/com/android/server/wm/DisplayAreaPolicy.java +++ b/services/core/java/com/android/server/wm/DisplayAreaPolicy.java @@ -74,8 +74,8 @@ public abstract class DisplayAreaPolicy { */ public abstract void addWindow(WindowToken token); - /** Gets the {@link DisplayArea} which a {@link WindowToken} is about to be attached to. */ - public abstract DisplayArea.Tokens getDisplayAreaForWindowToken(int type, Bundle options, + /** Gets the {@link DisplayArea} with given window type and launched options */ + public abstract DisplayArea.Tokens findAreaForWindowType(int type, Bundle options, boolean ownerCanManageAppTokens, boolean roundedCornerOverlay); /** diff --git a/services/core/java/com/android/server/wm/DisplayAreaPolicyBuilder.java b/services/core/java/com/android/server/wm/DisplayAreaPolicyBuilder.java index a7312b321e4d8..47d7c9d1279d9 100644 --- a/services/core/java/com/android/server/wm/DisplayAreaPolicyBuilder.java +++ b/services/core/java/com/android/server/wm/DisplayAreaPolicyBuilder.java @@ -756,7 +756,14 @@ class DisplayAreaPolicyBuilder { @VisibleForTesting DisplayArea.Tokens findAreaForToken(WindowToken token) { return mSelectRootForWindowFunc.apply(token.windowType, token.mOptions) - .findAreaForToken(token); + .findAreaForTokenInLayer(token); + } + + @Override + public DisplayArea.Tokens findAreaForWindowType(int type, Bundle options, + boolean ownerCanManageAppTokens, boolean roundedCornerOverlay) { + return mSelectRootForWindowFunc.apply(type, options).findAreaForWindowTypeInLayer(type, + ownerCanManageAppTokens, roundedCornerOverlay); } @VisibleForTesting @@ -794,13 +801,6 @@ class DisplayAreaPolicyBuilder { public TaskDisplayArea getDefaultTaskDisplayArea() { return mDefaultTaskDisplayArea; } - - @Override - public DisplayArea.Tokens getDisplayAreaForWindowToken(int type, Bundle options, - boolean ownerCanManageAppTokens, boolean roundedCornerOverlay) { - return mSelectRootForWindowFunc.apply(type, options).findAreaForToken(type, - ownerCanManageAppTokens, roundedCornerOverlay); - } } static class PendingArea { diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index e28ab26b0c1c2..f9f5752545d28 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -1123,15 +1123,8 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp token.mDisplayContent = this; // Add non-app token to container hierarchy on the display. App tokens are added through // the parent container managing them (e.g. Tasks). - switch (token.windowType) { - case TYPE_INPUT_METHOD: - case TYPE_INPUT_METHOD_DIALOG: - mImeWindowsContainer.addChild(token); - break; - default: - mDisplayAreaPolicy.addWindow(token); - break; - } + final DisplayArea.Tokens da = findAreaForToken(token).asTokens(); + da.addChild(token); } } @@ -5979,7 +5972,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp return mMagnificationSpec; } - DisplayArea getAreaForWindowToken(int windowType, Bundle options, + DisplayArea findAreaForWindowType(int windowType, Bundle options, boolean ownerCanManageAppToken, boolean roundedCornerOverlay) { // TODO(b/159767464): figure out how to find an appropriate TDA. if (windowType >= FIRST_APPLICATION_WINDOW && windowType <= LAST_APPLICATION_WINDOW) { @@ -5991,10 +5984,28 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp if (windowType == TYPE_INPUT_METHOD || windowType == TYPE_INPUT_METHOD_DIALOG) { return getImeContainer(); } - return mDisplayAreaPolicy.getDisplayAreaForWindowToken(windowType, options, + return mDisplayAreaPolicy.findAreaForWindowType(windowType, options, ownerCanManageAppToken, roundedCornerOverlay); } + /** + * Finds the {@link DisplayArea} for the {@link WindowToken} to attach to. + *

+ * Note that the differences between this API and + * {@link RootDisplayArea#findAreaForTokenInLayer(WindowToken)} is that this API finds a + * {@link DisplayArea} in {@link DisplayContent} level, which may find a {@link DisplayArea} + * from multiple {@link RootDisplayArea RootDisplayAreas} under this {@link DisplayContent}'s + * hierarchy, while {@link RootDisplayArea#findAreaForTokenInLayer(WindowToken)} finds a + * {@link DisplayArea.Tokens} from a {@link DisplayArea.Tokens} list mapped to window layers. + *

+ * + * @see DisplayContent#findAreaForTokenInLayer(WindowToken) + */ + DisplayArea findAreaForToken(WindowToken windowToken) { + return findAreaForWindowType(windowToken.getWindowType(), windowToken.mOptions, + windowToken.mOwnerCanManageAppTokens, windowToken.mRoundedCornerOverlay); + } + @Override DisplayContent asDisplayContent() { return this; diff --git a/services/core/java/com/android/server/wm/RootDisplayArea.java b/services/core/java/com/android/server/wm/RootDisplayArea.java index cd20c8242d81b..64e9faca7c0f2 100644 --- a/services/core/java/com/android/server/wm/RootDisplayArea.java +++ b/services/core/java/com/android/server/wm/RootDisplayArea.java @@ -101,15 +101,24 @@ class RootDisplayArea extends DisplayArea { "There is no FEATURE_IME_PLACEHOLDER in this root to place the IME container"); } - /** Finds the {@link DisplayArea.Tokens} that this type of window should be attached to. */ + /** + * Finds the {@link DisplayArea.Tokens} in {@code mAreaForLayer} that this type of window + * should be attached to. + *

+ * Note that in most cases, users are expected to call + * {@link DisplayContent#findAreaForToken(WindowToken)} to find a {@link DisplayArea} in + * {@link DisplayContent} level instead of calling this inner method. + *

+ */ @Nullable - DisplayArea.Tokens findAreaForToken(WindowToken token) { - return findAreaForToken(token.windowType, token.mOwnerCanManageAppTokens, + DisplayArea.Tokens findAreaForTokenInLayer(WindowToken token) { + return findAreaForWindowTypeInLayer(token.windowType, token.mOwnerCanManageAppTokens, token.mRoundedCornerOverlay); } + /** @see #findAreaForTokenInLayer(WindowToken) */ @Nullable - DisplayArea.Tokens findAreaForToken(int windowType, boolean ownerCanManageAppTokens, + DisplayArea.Tokens findAreaForWindowTypeInLayer(int windowType, boolean ownerCanManageAppTokens, boolean roundedCornerOverlay) { int windowLayerFromType = mWmService.mPolicy.getWindowLayerFromTypeLw(windowType, ownerCanManageAppTokens, roundedCornerOverlay); diff --git a/services/core/java/com/android/server/wm/WindowContextListenerController.java b/services/core/java/com/android/server/wm/WindowContextListenerController.java index b417832d3be13..bc530416c8cd6 100644 --- a/services/core/java/com/android/server/wm/WindowContextListenerController.java +++ b/services/core/java/com/android/server/wm/WindowContextListenerController.java @@ -201,7 +201,9 @@ class WindowContextListenerController { return mContainer; } - private void updateContainer(WindowContainer newContainer) { + private void updateContainer(@NonNull WindowContainer newContainer) { + Objects.requireNonNull(newContainer); + if (mContainer.equals(newContainer)) { return; } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 9e8b6a34e1d52..145d3ed8c8406 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2683,7 +2683,7 @@ public class WindowManagerService extends IWindowManager.Stub } // TODO(b/155340867): Investigate if we still need roundedCornerOverlay after // the feature b/155340867 is completed. - final DisplayArea da = dc.getAreaForWindowToken(type, options, + final DisplayArea da = dc.findAreaForWindowType(type, options, callerCanManageAppTokens, false /* roundedCornerOverlay */); mWindowContextListenerController.registerWindowContainerListener(clientToken, da, callingUid, type, options); diff --git a/services/tests/wmtests/src/com/android/server/wm/DualDisplayAreaGroupPolicyTest.java b/services/tests/wmtests/src/com/android/server/wm/DualDisplayAreaGroupPolicyTest.java index 2c3f52e05addf..4509ff48206e4 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DualDisplayAreaGroupPolicyTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/DualDisplayAreaGroupPolicyTest.java @@ -268,7 +268,7 @@ public class DualDisplayAreaGroupPolicyTest extends WindowTestsBase { // By default, the ime container is attached to DC as defined in DAPolicy. assertThat(imeContainer.getRootDisplayArea()).isEqualTo(mDisplay); - assertThat(mDisplay.findAreaForToken(imeToken)).isEqualTo(imeContainer); + assertThat(mDisplay.findAreaForTokenInLayer(imeToken)).isEqualTo(imeContainer); final WindowState firstActivityWin = createWindow(null /* parent */, TYPE_APPLICATION_STARTING, mFirstActivity, @@ -290,9 +290,9 @@ public class DualDisplayAreaGroupPolicyTest extends WindowTestsBase { assertThat(imeContainer.getRootDisplayArea()).isEqualTo(mFirstRoot); assertThat(imeContainer.getParent().asDisplayArea().mFeatureId) .isEqualTo(FEATURE_IME_PLACEHOLDER); - assertThat(mDisplay.findAreaForToken(imeToken)).isNull(); - assertThat(mFirstRoot.findAreaForToken(imeToken)).isEqualTo(imeContainer); - assertThat(mSecondRoot.findAreaForToken(imeToken)).isNull(); + assertThat(mDisplay.findAreaForTokenInLayer(imeToken)).isNull(); + assertThat(mFirstRoot.findAreaForTokenInLayer(imeToken)).isEqualTo(imeContainer); + assertThat(mSecondRoot.findAreaForTokenInLayer(imeToken)).isNull(); // secondActivityWin should be the target doReturn(false).when(firstActivityWin).canBeImeTarget(); @@ -305,9 +305,9 @@ public class DualDisplayAreaGroupPolicyTest extends WindowTestsBase { assertThat(imeContainer.getRootDisplayArea()).isEqualTo(mSecondRoot); assertThat(imeContainer.getParent().asDisplayArea().mFeatureId) .isEqualTo(FEATURE_IME_PLACEHOLDER); - assertThat(mDisplay.findAreaForToken(imeToken)).isNull(); - assertThat(mFirstRoot.findAreaForToken(imeToken)).isNull(); - assertThat(mSecondRoot.findAreaForToken(imeToken)).isEqualTo(imeContainer); + assertThat(mDisplay.findAreaForTokenInLayer(imeToken)).isNull(); + assertThat(mFirstRoot.findAreaForTokenInLayer(imeToken)).isNull(); + assertThat(mSecondRoot.findAreaForTokenInLayer(imeToken)).isEqualTo(imeContainer); } @Test 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 5d0fe170885a2..e5eba57f223db 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContextListenerControllerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContextListenerControllerTests.java @@ -19,6 +19,7 @@ package com.android.server.wm; import static android.view.Display.DEFAULT_DISPLAY; 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 com.google.common.truth.Truth.assertThat; @@ -38,6 +39,7 @@ import androidx.test.filters.SmallTest; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; /** * Build/Install/Run: @@ -185,6 +187,41 @@ public class WindowContextListenerControllerTests extends WindowTestsBase { assertThat(mController.getContainer(mClientToken)).isEqualTo(da); } + @Test + public void testImeSwitchDialogWindowTokenRemovedOnDualDisplayContent_ListenToImeContainer() { + // Let the Display to be created with the DualDisplay policy. + final DisplayAreaPolicy.Provider policyProvider = + new DualDisplayAreaGroupPolicyTest.DualDisplayTestPolicyProvider(); + Mockito.doReturn(policyProvider).when(mWm).getDisplayAreaPolicyProvider(); + // Create a DisplayContent with dual RootDisplayArea + DualDisplayAreaGroupPolicyTest.DualDisplayContent dualDisplayContent = + new DualDisplayAreaGroupPolicyTest.DualDisplayContent + .Builder(mAtm, 1000, 1000).build(); + final DisplayArea.Tokens imeContainer = dualDisplayContent.getImeContainer(); + // Put the ImeContainer to the first sub-RootDisplayArea + dualDisplayContent.mFirstRoot.placeImeContainer(imeContainer); + + assertThat(imeContainer.getRootDisplayArea()).isEqualTo(dualDisplayContent.mFirstRoot); + + // Simulate the behavior to show IME switch dialog: its context switches to register to + // context created WindowToken. + WindowToken windowContextCreatedToken = new WindowToken.Builder(mWm, mClientToken, + TYPE_INPUT_METHOD_DIALOG) + .setDisplayContent(dualDisplayContent) + .setFromClientToken(true) + .build(); + mController.registerWindowContainerListener(mClientToken, windowContextCreatedToken, + TEST_UID, TYPE_INPUT_METHOD_DIALOG, null /* options */); + + assertThat(mController.getContainer(mClientToken)).isEqualTo(windowContextCreatedToken); + + // Remove WindowToken + windowContextCreatedToken.removeImmediately(); + + // Now context should listen to ImeContainer. + assertThat(mController.getContainer(mClientToken)).isEqualTo(imeContainer); + } + private class TestWindowTokenClient extends IWindowToken.Stub { private Configuration mConfiguration; private int mDisplayId;