From fc837a1811443cc9eb4a18e5c6627a183df2b352 Mon Sep 17 00:00:00 2001 From: Wilson Wu Date: Thu, 23 Feb 2023 14:36:41 +0800 Subject: [PATCH] Fix wrong ime parent in embedded activity(2nd try) We have CL[1] to polish the ime transition. But in ActivityEmbedding, the ime layering target may higher than input target. And the ime parent didn't be updated since input target and layering target are unsync. Update ime parent for this case. Also we ensure both ime targets are attached and are in same embedded task before window hierarchy comparison to prevent the exception. [1]: I332c0e4fff62df5d7b793eda2767bb58fe85a938 Bug: 260387203 Bug: 270290011 Test: Manual test with test apk in the bug Test: Verify no exception through bug steps Test: atest TaskFragmentTest#testUpdateImeParentForActivityEmbedding Change-Id: I937560f7479535f0e2dc77ebe0f5056fa85bbb04 --- .../com/android/server/wm/DisplayContent.java | 3 +- .../server/wm/ImeTargetVisibilityPolicy.java | 53 +++++++++++++++++-- .../android/server/wm/TaskFragmentTest.java | 31 +++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index de7c867bb9e17..dfa1a62ff506b 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -4589,8 +4589,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp */ @VisibleForTesting SurfaceControl computeImeParent() { - if (!ImeTargetVisibilityPolicy.isReadyToComputeImeParent(mImeLayeringTarget, - mImeInputTarget)) { + if (!ImeTargetVisibilityPolicy.canComputeImeParent(mImeLayeringTarget, mImeInputTarget)) { return null; } // Attach it to app if the target is part of an app and such app is covering the entire diff --git a/services/core/java/com/android/server/wm/ImeTargetVisibilityPolicy.java b/services/core/java/com/android/server/wm/ImeTargetVisibilityPolicy.java index 49218ad07dff6..71dd91785384a 100644 --- a/services/core/java/com/android/server/wm/ImeTargetVisibilityPolicy.java +++ b/services/core/java/com/android/server/wm/ImeTargetVisibilityPolicy.java @@ -19,6 +19,7 @@ package com.android.server.wm; import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_STARTING; +import android.annotation.Nullable; import android.os.IBinder; import android.view.WindowManager; @@ -50,13 +51,18 @@ public abstract class ImeTargetVisibilityPolicy { * Called when {@link DisplayContent#computeImeParent()} to check if it's valid to keep * computing the ime parent. * + * @param imeLayeringTarget The window which IME target to layer on top of it. + * @param imeInputTarget The window which start the input connection, receive input from IME. * @return {@code true} to keep computing the ime parent, {@code false} to defer this operation */ - public static boolean isReadyToComputeImeParent(WindowState imeLayeringTarget, - InputTarget imeInputTarget) { + public static boolean canComputeImeParent(@Nullable WindowState imeLayeringTarget, + @Nullable InputTarget imeInputTarget) { if (imeLayeringTarget == null) { return false; } + if (shouldComputeImeParentForEmbeddedActivity(imeLayeringTarget, imeInputTarget)) { + return true; + } // Ensure changing the IME parent when the layering target that may use IME has // became to the input target for preventing IME flickers. // Note that: @@ -69,7 +75,6 @@ public abstract class ImeTargetVisibilityPolicy { boolean imeLayeringTargetMayUseIme = WindowManager.LayoutParams.mayUseInputMethod(imeLayeringTarget.mAttrs.flags) || imeLayeringTarget.mAttrs.type == TYPE_APPLICATION_STARTING; - // Do not change parent if the window hasn't requested IME. var inputAndLayeringTargetsDisagree = (imeInputTarget == null || imeLayeringTarget.mActivityRecord != imeInputTarget.getActivityRecord()); @@ -77,4 +82,46 @@ public abstract class ImeTargetVisibilityPolicy { return !inputTargetStale; } + + + /** + * Called from {@link DisplayContent#computeImeParent()} to check the given IME targets if the + * IME surface parent should be updated in ActivityEmbeddings. + * + * As the IME layering target is calculated according to the window hierarchy by + * {@link DisplayContent#computeImeTarget}, the layering target and input target may be + * different when the window hasn't started input connection, WindowManagerService hasn't yet + * received the input target which reported from InputMethodManagerService. To make the IME + * surface will be shown on the best fit IME layering target, we basically won't update IME + * parent until both IME input and layering target updated for better IME transition. + * + * However, in activity embedding, tapping a window won't update it to the top window so the + * calculated IME layering target may higher than input target. Update IME parent for this case. + * + * @return {@code true} means the layer of IME layering target is higher than the input target + * and {@link DisplayContent#computeImeParent()} should keep progressing to update the IME + * surface parent on the display in case the IME surface left behind. + */ + private static boolean shouldComputeImeParentForEmbeddedActivity( + @Nullable WindowState imeLayeringTarget, @Nullable InputTarget imeInputTarget) { + if (imeInputTarget == null || imeLayeringTarget == null) { + return false; + } + final WindowState inputTargetWindow = imeInputTarget.getWindowState(); + if (inputTargetWindow == null || !imeLayeringTarget.isAttached() + || !inputTargetWindow.isAttached()) { + return false; + } + + final ActivityRecord inputTargetRecord = imeInputTarget.getActivityRecord(); + final ActivityRecord layeringTargetRecord = imeLayeringTarget.getActivityRecord(); + if (inputTargetRecord == null || layeringTargetRecord == null + || inputTargetRecord == layeringTargetRecord + || (inputTargetRecord.getTask() != layeringTargetRecord.getTask()) + || !inputTargetRecord.isEmbedded() || !layeringTargetRecord.isEmbedded()) { + // Check whether the input target and layering target are embedded in the same Task. + return false; + } + return imeLayeringTarget.compareTo(inputTargetWindow) > 0; + } } diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java index dab842c4aa5aa..df3af7d2f4faa 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java @@ -24,6 +24,7 @@ import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE; import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_PORTRAIT; import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSET; import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED; +import static android.view.WindowManager.LayoutParams.TYPE_BASE_APPLICATION; import static com.android.dx.mockito.inline.extended.ExtendedMockito.any; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing; @@ -613,4 +614,34 @@ public class TaskFragmentTest extends WindowTestsBase { assertEquals(SCREEN_ORIENTATION_UNSPECIFIED, tf1.getOrientation(SCREEN_ORIENTATION_UNSET)); assertEquals(SCREEN_ORIENTATION_UNSPECIFIED, task.getOrientation(SCREEN_ORIENTATION_UNSET)); } + + @Test + public void testUpdateImeParentForActivityEmbedding() { + // Setup two activities in ActivityEmbedding. + final Task task = createTask(mDisplayContent); + final TaskFragment tf0 = new TaskFragmentBuilder(mAtm) + .setParentTask(task) + .createActivityCount(1) + .setOrganizer(mOrganizer) + .setFragmentToken(new Binder()) + .build(); + final TaskFragment tf1 = new TaskFragmentBuilder(mAtm) + .setParentTask(task) + .createActivityCount(1) + .setOrganizer(mOrganizer) + .setFragmentToken(new Binder()) + .build(); + final ActivityRecord activity0 = tf0.getTopMostActivity(); + final ActivityRecord activity1 = tf1.getTopMostActivity(); + final WindowState win0 = createWindow(null, TYPE_BASE_APPLICATION, activity0, "win0"); + final WindowState win1 = createWindow(null, TYPE_BASE_APPLICATION, activity1, "win1"); + doReturn(false).when(mDisplayContent).shouldImeAttachedToApp(); + + mDisplayContent.setImeInputTarget(win0); + mDisplayContent.setImeLayeringTarget(win1); + + // The ImeParent should be the display. + assertEquals(mDisplayContent.getImeContainer().getParent().getSurfaceControl(), + mDisplayContent.computeImeParent()); + } }