From 69947c067109b6c9b92a2bdd792e5a74d598611a Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Wed, 1 Jul 2020 16:23:05 +0200 Subject: [PATCH] Revert new logic around FLAG_ALT_FOCUSABLE_IM This broke several apps because it means certain NON_FOCUSABLE windows are now layered wrong w.r.t the IME, and also no longer get IME insets. Originally, this was introduced because the IME target was used to compute which window gets control, but this is now computed based on the window actually connecting to the IME as reported per IMMS. Reverts I941571c97145d77b0a59d030cf2a8c8318f3b59f. Bug: 143898978 Bug: 140641950 Bug: 145812508 Bug: 141738570 Bug: 144619551 Fixes: 159438771 Test: atest WindowStateTests atest FocusHandlingTest atest WindowManager_LayoutParamsTest Also manually using steps: 1. Launch gmail compose activity 2. start typing in receipient field 3. verify that even thoug the suggestions popup window w/ FLAG_NOT_FOCUSABLE becomes the IME target, control and insets still work. Change-Id: If451276b1a8c485ec88965d8d30ec8fd3836620f --- core/java/android/view/WindowManager.java | 28 +++++++++++++------ .../com/android/server/wm/WindowState.java | 21 ++++++-------- .../android/server/wm/WindowStateTests.java | 18 ++---------- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/core/java/android/view/WindowManager.java b/core/java/android/view/WindowManager.java index 76071278edf83..5c6269421a1f6 100644 --- a/core/java/android/view/WindowManager.java +++ b/core/java/android/view/WindowManager.java @@ -1454,11 +1454,22 @@ public interface WindowManager extends ViewManager { @Deprecated public static final int FLAG_LAYOUT_INSET_DECOR = 0x00010000; - /** Window flag: When set, input method can't interact with the focusable window - * and can be placed to use more space and cover the input method. - * Note: When combined with {@link #FLAG_NOT_FOCUSABLE}, this flag has no - * effect since input method cannot interact with windows having {@link #FLAG_NOT_FOCUSABLE} - * flag set. + /** Window flag: when set, inverts the input method focusability of the window. + * + * The effect of setting this flag depends on whether {@link #FLAG_NOT_FOCUSABLE} is set: + *

+ * If {@link #FLAG_NOT_FOCUSABLE} is not set, i.e. when the window is focusable, + * setting this flag prevents this window from becoming the target of the input method. + * Consequently, it will not be able to interact with the input method, + * and will be layered above the input method (unless there is another input method + * target above it). + * + *

+ * If {@link #FLAG_NOT_FOCUSABLE} is set, setting this flag requests for the window + * to be the input method target even though the window is not focusable. + * Consequently, it will be layered below the input method. + * Note: Windows that set {@link #FLAG_NOT_FOCUSABLE} cannot interact with the input method, + * regardless of this flag. */ public static final int FLAG_ALT_FOCUSABLE_IM = 0x00020000; @@ -2142,13 +2153,12 @@ public interface WindowManager extends ViewManager { * focus. In particular, this checks the * {@link #FLAG_NOT_FOCUSABLE} and {@link #FLAG_ALT_FOCUSABLE_IM} * flags and returns true if the combination of the two corresponds - * to a window that needs to be behind the input method so that the - * user can type into it. + * to a window that can use the input method. * * @param flags The current window manager flags. * - * @return Returns {@code true} if such a window should be behind/interact - * with an input method, {@code false} if not. + * @return Returns {@code true} if a window with the given flags would be able to + * use the input method, {@code false} if not. */ public static boolean mayUseInputMethod(int flags) { return (flags & FLAG_NOT_FOCUSABLE) != FLAG_NOT_FOCUSABLE diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index d4d2f4d7a4920..b65020d7eaa66 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -2394,12 +2394,15 @@ class WindowState extends WindowContainer implements WindowManagerP if (mAttrs.type == TYPE_APPLICATION_STARTING) { // Ignore mayUseInputMethod for starting window for now. // TODO(b/159911356): Remove this special casing (originally added in commit e75d872). - } else if (PixelFormat.formatHasAlpha(mAttrs.format)) { - // Support legacy use cases where transparent windows can still be ime target with - // FLAG_NOT_FOCUSABLE and ALT_FOCUSABLE_IM set. - // Certain apps listen for IME insets using transparent windows and ADJUST_NOTHING to - // manually synchronize app content to IME animation b/144619551. - // TODO(b/145812508): remove this once new focus management is complete b/141738570 + } else { + // TODO(b/145812508): Clean this up in S, may depend on b/141738570 + // The current logic lets windows become the "ime target" even though they are + // not-focusable and can thus never actually start input. + // Ideally, this would reject windows where mayUseInputMethod() == false, but this + // also impacts Z-ordering of and delivery of IME insets to child windows, which means + // that simply disallowing non-focusable windows would break apps. + // See b/159438771, b/144619551. + final int fl = mAttrs.flags & (FLAG_NOT_FOCUSABLE | FLAG_ALT_FOCUSABLE_IM); // Can only be an IME target if both FLAG_NOT_FOCUSABLE and FLAG_ALT_FOCUSABLE_IM are @@ -2407,12 +2410,6 @@ class WindowState extends WindowContainer implements WindowManagerP if (fl != 0 && fl != (FLAG_NOT_FOCUSABLE | FLAG_ALT_FOCUSABLE_IM)) { return false; } - } else if (!WindowManager.LayoutParams.mayUseInputMethod(mAttrs.flags)) { - // Can be an IME target only if: - // 1. FLAG_NOT_FOCUSABLE is not set - // 2. FLAG_ALT_FOCUSABLE_IM is not set - // 3. not a starting window. - return false; } if (DEBUG_INPUT_METHOD) { diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java index 4a0f48cf2ccb9..360d73b5bd872 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java @@ -67,7 +67,6 @@ import static org.mockito.Mockito.when; import android.graphics.Insets; import android.graphics.Matrix; -import android.graphics.PixelFormat; import android.graphics.Rect; import android.os.RemoteException; import android.platform.test.annotations.Presubmit; @@ -229,22 +228,11 @@ public class WindowStateTests extends WindowTestsBase { appWindow.mAttrs.flags |= (FLAG_NOT_FOCUSABLE | FLAG_ALT_FOCUSABLE_IM); imeWindow.mAttrs.flags |= (FLAG_NOT_FOCUSABLE | FLAG_ALT_FOCUSABLE_IM); - // Visible app window with flags FLAG_NOT_FOCUSABLE or FLAG_ALT_FOCUSABLE_IM can't be IME - // target while an IME window can never be an IME target regardless of its visibility - // or flags. - assertFalse(appWindow.canBeImeTarget()); + // Visible app window with flags can be IME target while an IME window can never be an IME + // target regardless of its visibility or flags. + assertTrue(appWindow.canBeImeTarget()); assertFalse(imeWindow.canBeImeTarget()); - // b/145812508: special legacy use-case for transparent/translucent windows. - appWindow.mAttrs.format = PixelFormat.TRANSPARENT; - assertTrue(appWindow.canBeImeTarget()); - - appWindow.mAttrs.format = PixelFormat.OPAQUE; - appWindow.mAttrs.flags &= ~FLAG_ALT_FOCUSABLE_IM; - assertFalse(appWindow.canBeImeTarget()); - appWindow.mAttrs.flags &= ~FLAG_NOT_FOCUSABLE; - assertTrue(appWindow.canBeImeTarget()); - // Verify PINNED windows can't be IME target. int initialMode = appWindow.mActivityRecord.getWindowingMode(); appWindow.mActivityRecord.setWindowingMode(WINDOWING_MODE_PINNED);