From a49ea6f22c31749bc6aa79b012bb4c747ad179fa Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Fri, 7 May 2021 13:13:38 +0800 Subject: [PATCH] Fix some IME transition issues - Add InsetsSource#getVisibleFrame to check if the launching task needs to disable fixed rotation: As CL[1] disables fixed rotation when IME surface exists for fixing IME janky when IME receiving different orientations. Even the CL used StartingData#hasImeSurface to forecast if needs to disable fixed rotation, it may hit some timing cases that StartingData not yet be prepared when launching the task next time quickly. Add InsetsSource#getVisibleFrame to prevent this edge case happen since this value will be updated when app client requests relayout. - Consolide DC#isImeAttachedToApp() check: When recents animation started, will check whether the task has to hide soft-keyboard when it's not able to attach IME surface through DC#isImeAttachedToApp. Since this method only checks the layering target window states but not check if the IME surface has actually attached to the app. it might have potential issue that IME may state persistents when switching between tasks if we don't check if the actual IME container surface. Consolide DC#isImeSurfaceAttachedOnApp check to ensure hiding keyboard in RecentsAnimationController as the above potential case. (Also renamed the original DC#isImeAttachedToApp as DC#shouldImeAttachedToApp to make it easier understand the difference within the new isImeAttachedToApp.) [1]: Idfdf129adbfbee6634d8a27aa78da1f631bd213b Fix: 160451808 Bug: 184020899 Test: manual as below steps 1) launching the app with focusing IME in portrait mode. 2) swiping out to launcher 3) rotate the device in landscape 4) tapping the shortcut to launch the app again 5) repeat 1-4, see if IME surface will mess up to see both portrait and landcape surface. Change-Id: I966ab69f260f828b6e96b3479a36467181288504 --- .../com/android/server/wm/ActivityRecord.java | 2 +- .../com/android/server/wm/DisplayContent.java | 24 +++++++++++++++---- .../server/wm/TaskSnapshotController.java | 2 +- .../com/android/server/wm/WindowState.java | 2 +- .../server/wm/DisplayContentTests.java | 2 +- .../android/server/wm/SizeCompatTests.java | 6 ++--- .../server/wm/TaskSnapshotControllerTest.java | 4 ++-- 7 files changed, 29 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 9f51d97d3707a..27fe3579c42fe 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -6851,7 +6851,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A } // An activity in size compatibility mode may have override bounds which equals to its // parent bounds, so the exact bounds should also be checked to allow IME window to attach - // to the activity. See {@link DisplayContent#isImeAttachedToApp}. + // to the activity. See {@link DisplayContent#shouldImeAttachedToApp}. final WindowContainer parent = getParent(); return parent == null || parent.getBounds().equals(overrideBounds); } diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 206f67ec6751e..4c6cc86bdb720 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -1531,7 +1531,9 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp // to cover the activity configuration change. return false; } - if (r.mStartingData != null && r.mStartingData.hasImeSurface()) { + if ((r.mStartingData != null && r.mStartingData.hasImeSurface()) + || (mInsetsStateController.getImeSourceProvider() + .getSource().getVisibleFrame() != null)) { // Currently it is unknown that when will IME window be ready. Reject the case to // avoid flickering by showing IME in inconsistent orientation. return false; @@ -3621,7 +3623,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp return mImeInputTarget != null && !mImeInputTarget.inMultiWindowMode(); } - boolean isImeAttachedToApp() { + boolean shouldImeAttachedToApp() { return isImeControlledByApp() && mImeLayeringTarget != null && mImeLayeringTarget.mActivityRecord != null @@ -3634,6 +3636,20 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp && mImeLayeringTarget.matchesDisplayAreaBounds(); } + /** + * Unlike {@link #shouldImeAttachedToApp()}, this method returns {@code @true} only when both + * the IME layering target is valid to attach the IME surface to the app, and the + * {@link #mInputMethodSurfaceParent} of the {@link ImeContainer} has actually attached to + * the app. (i.e. Even if {@link #shouldImeAttachedToApp()} returns {@code true}, calling this + * method will return {@code false} if the IME surface doesn't actually attach to the app.) + */ + boolean isImeAttachedToApp() { + return shouldImeAttachedToApp() + && mInputMethodSurfaceParent != null + && mInputMethodSurfaceParent.isSameSurface( + mImeLayeringTarget.mActivityRecord.getSurfaceControl()); + } + /** * Finds the window which can host IME if IME target cannot host it. * e.g. IME target cannot host IME when it's display has a parent display OR when display @@ -3762,7 +3778,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp @VisibleForTesting void attachAndShowImeScreenshotOnTarget() { // No need to attach screenshot if the IME target not exists or screen is off. - if (!isImeAttachedToApp() || !mWmService.mPolicy.isScreenOn()) { + if (!shouldImeAttachedToApp() || !mWmService.mPolicy.isScreenOn()) { return; } @@ -3930,7 +3946,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp // Attach it to app if the target is part of an app and such app is covering the entire // screen. If it's not covering the entire screen the IME might extend beyond the apps // bounds. - if (allowAttachToApp && isImeAttachedToApp()) { + if (allowAttachToApp && shouldImeAttachedToApp()) { return mImeLayeringTarget.mActivityRecord.getSurfaceControl(); } diff --git a/services/core/java/com/android/server/wm/TaskSnapshotController.java b/services/core/java/com/android/server/wm/TaskSnapshotController.java index d49b6a0af42fd..9ffb2b1769153 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotController.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotController.java @@ -425,7 +425,7 @@ class TaskSnapshotController { final WindowState imeWindow = task.getDisplayContent().mInputMethodWindow; // Exclude IME window snapshot when IME isn't proper to attach to app. final boolean excludeIme = imeWindow != null && imeWindow.getSurfaceControl() != null - && !task.getDisplayContent().isImeAttachedToApp(); + && !task.getDisplayContent().shouldImeAttachedToApp(); final WindowState navWindow = task.getDisplayContent().getDisplayPolicy().getNavigationBar(); // If config_attachNavBarToAppDuringTransition is true, the nav bar will be reparent to the diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 540035fb6c976..0ec2b81290fbd 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -2309,7 +2309,7 @@ class WindowState extends WindowContainer implements WindowManagerP // When the window configuration changed, we need to update the IME control target in // case the app may lose the IME inets control when exiting from split-screen mode, or the // IME parent may failed to attach to the app during rotating the screen. - // See DisplayContent#isImeAttachedToApp, DisplayContent#isImeControlledByApp + // See DisplayContent#shouldImeAttachedToApp, DisplayContent#isImeControlledByApp if (windowConfigChanged) { getDisplayContent().updateImeControlTarget(); } diff --git a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java index e09606e3389f2..ff4e5a60faac0 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java @@ -1904,7 +1904,7 @@ public class DisplayContentTests extends WindowTestsBase { mDisplayContent.setImeInputTarget(appWin2); mDisplayContent.computeImeTarget(true); assertEquals(appWin2, mDisplayContent.getImeTarget(IME_TARGET_LAYERING)); - assertTrue(mDisplayContent.isImeAttachedToApp()); + assertTrue(mDisplayContent.shouldImeAttachedToApp()); verify(mDisplayContent, atLeast(1)).attachAndShowImeScreenshotOnTarget(); verify(mWm.mTaskSnapshotController).snapshotImeFromAttachedTask(appWin1.getTask()); diff --git a/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java b/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java index 0925e12224bef..d27c120d2b682 100644 --- a/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/SizeCompatTests.java @@ -316,14 +316,14 @@ public class SizeCompatTests extends WindowTestsBase { mActivity.mDisplayContent.setImeLayeringTarget(addWindowToActivity(mActivity)); // Make sure IME cannot attach to the app, otherwise IME window will also be shifted. - assertFalse(mActivity.mDisplayContent.isImeAttachedToApp()); + assertFalse(mActivity.mDisplayContent.shouldImeAttachedToApp()); // Recompute the natural configuration without resolving size compat configuration. mActivity.clearSizeCompatMode(); mActivity.onConfigurationChanged(mTask.getConfiguration()); // It should keep non-attachable because the resolved bounds will be computed according to // the aspect ratio that won't match its parent bounds. - assertFalse(mActivity.mDisplayContent.isImeAttachedToApp()); + assertFalse(mActivity.mDisplayContent.shouldImeAttachedToApp()); // Activity max bounds should be sandboxed since it is letterboxed. assertActivityMaxBoundsSandboxed(); } @@ -358,7 +358,7 @@ public class SizeCompatTests extends WindowTestsBase { // Because the aspect ratio of display doesn't exceed the max aspect ratio of activity. // The activity should still fill its parent container and IME can attach to the activity. assertTrue(mActivity.matchParentBounds()); - assertTrue(mActivity.mDisplayContent.isImeAttachedToApp()); + assertTrue(mActivity.mDisplayContent.shouldImeAttachedToApp()); final Rect letterboxInnerBounds = new Rect(); mActivity.getLetterboxInnerBounds(letterboxInnerBounds); diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskSnapshotControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskSnapshotControllerTest.java index 81712c6897c4b..e9e201327aa46 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskSnapshotControllerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskSnapshotControllerTest.java @@ -193,7 +193,7 @@ public class TaskSnapshotControllerTest extends WindowTestsBase { Task task = mAppWindow.mActivityRecord.getTask(); spyOn(task); spyOn(mDisplayContent); - when(task.getDisplayContent().isImeAttachedToApp()).thenReturn(false); + when(task.getDisplayContent().shouldImeAttachedToApp()).thenReturn(false); // Intentionally set the SurfaceControl of input method window as null. mDisplayContent.mInputMethodWindow.setSurfaceControl(null); // Verify no NPE happens when calling createTaskSnapshot. @@ -213,7 +213,7 @@ public class TaskSnapshotControllerTest extends WindowTestsBase { spyOn(task); spyOn(mDisplayContent); spyOn(mDisplayContent.mInputMethodWindow); - when(task.getDisplayContent().isImeAttachedToApp()).thenReturn(true); + when(task.getDisplayContent().shouldImeAttachedToApp()).thenReturn(true); // Intentionally set the IME window is in drawn state. doReturn(true).when(mDisplayContent.mInputMethodWindow).isDrawn(); // Verify no NPE happens when calling createTaskSnapshot.