From 2260ce4dafdd1fdaf8cbaf7579119ed7ecded088 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Thu, 2 Jul 2020 18:57:22 +0200 Subject: Fix IME flicker: move hiding the surface into the control target Fixes a flicker that occurs during transitions between windows. This happens for two reasons: 1.) Control is immediately transferred to the new window, and the previous window didn't get a chance to play the animation. This is addressed by adding logic to keep control on the exiting window for the duration of the transition - similar to what we do with the target for z-ordering purposes. 2.) Upon the input connection being severed, the InputMethodService immediately hides its window, preventing any animations whenever the input connection changes This is addressed by moving hiding of the surface into the controlling windows - where upon receiving control, we now trigger removal of the IME surface if we don't show it. Additionally: - Now ensures that any requests from the ImeInsetsSourceConsumer ensure that they come from the window that is currently served by IMM. - Removes the transparancy clause from isImeTargetFromDisplayContentAndImeSame to match the updated IME target computation in DisplayContent in [1]. [1]: Iedd5f7407926167f4891ce9b7e9a79e22751e668 Fixes: 153145997 Fixes: 150902448 Test: atest WindowInsetsAnimationControllerTests Test: atest DisplayContentTests InsetsSourceConsumerTest Test: Open app with IME, press HOME button, verify IME smoothly animates away Test: Open Messages, open a thread, open IME. Click search icon, verify IME opens in the search activity Change-Id: I4910c2a06cc67b0470477b245fc1de54b75f10f9 --- .../inputmethodservice/InputMethodService.java | 3 -- .../java/android/view/ImeInsetsSourceConsumer.java | 12 ++++--- core/java/android/view/InsetsSourceConsumer.java | 3 ++ core/java/android/view/ViewRootImpl.java | 5 +++ .../view/inputmethod/InputMethodManager.java | 30 ++++++++++------ .../android/internal/view/IInputMethodManager.aidl | 3 ++ .../src/android/view/InsetsSourceConsumerTest.java | 40 ++++++++++++++++++---- .../android/systemui/wm/DisplayImeController.java | 26 ++++++++------ .../inputmethod/InputMethodManagerInternal.java | 9 +++++ .../inputmethod/InputMethodManagerService.java | 37 +++++++++++++++++--- .../MultiClientInputMethodManagerService.java | 11 ++++++ .../java/com/android/server/wm/DisplayContent.java | 16 +++++++-- .../android/server/wm/ImeInsetsSourceProvider.java | 18 +++++++--- .../com/android/server/wm/InsetsControlTarget.java | 4 +++ .../android/server/wm/InsetsStateController.java | 17 ++++++++- .../java/com/android/server/wm/WindowState.java | 3 ++ .../com/android/server/wm/DisplayContentTests.java | 20 +++++++++++ 17 files changed, 211 insertions(+), 46 deletions(-) diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java index 5647bf90d2fb..c5a11abe1136 100644 --- a/core/java/android/inputmethodservice/InputMethodService.java +++ b/core/java/android/inputmethodservice/InputMethodService.java @@ -605,9 +605,6 @@ public class InputMethodService extends AbstractInputMethodService { if (DEBUG) Log.v(TAG, "unbindInput(): binding=" + mInputBinding + " ic=" + mInputConnection); // Unbind input is per process per display. - // TODO(b/150902448): free-up IME surface when target is changing. - // e.g. DisplayContent#setInputMethodTarget() - removeImeSurface(); onUnbindInput(); mInputBinding = null; mInputConnection = null; diff --git a/core/java/android/view/ImeInsetsSourceConsumer.java b/core/java/android/view/ImeInsetsSourceConsumer.java index ef9d990168d2..c1998c6009cf 100644 --- a/core/java/android/view/ImeInsetsSourceConsumer.java +++ b/core/java/android/view/ImeInsetsSourceConsumer.java @@ -119,11 +119,11 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { // If we had a request before to show from IME (tracked with mImeRequestedShow), reaching // this code here means that we now got control, so we can start the animation immediately. // If client window is trying to control IME and IME is already visible, it is immediate. - if (fromIme || mState.getSource(getType()).isVisible()) { + if (fromIme || mState.getSource(getType()).isVisible() && getControl() != null) { return ShowResult.SHOW_IMMEDIATELY; } - return getImm().requestImeShow(null /* resultReceiver */) + return getImm().requestImeShow(mController.getHost().getWindowToken()) ? ShowResult.IME_SHOW_DELAYED : ShowResult.IME_SHOW_FAILED; } @@ -132,12 +132,15 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { */ @Override void notifyHidden() { - getImm().notifyImeHidden(); + getImm().notifyImeHidden(mController.getHost().getWindowToken()); } @Override public void removeSurface() { - getImm().removeImeSurface(); + final IBinder window = mController.getHost().getWindowToken(); + if (window != null) { + getImm().removeImeSurface(window); + } } @Override @@ -146,6 +149,7 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { super.setControl(control, showTypes, hideTypes); if (control == null && !mIsRequestedVisibleAwaitingControl) { hide(); + removeSurface(); } } diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java index 40e6f4b2fce8..700dc66fab55 100644 --- a/core/java/android/view/InsetsSourceConsumer.java +++ b/core/java/android/view/InsetsSourceConsumer.java @@ -153,6 +153,9 @@ public class InsetsSourceConsumer { if (oldLeash == null || newLeash == null || !oldLeash.isSameSurface(newLeash)) { applyHiddenToControl(); } + if (!requestedVisible && !mIsAnimationPending) { + removeSurface(); + } } } if (lastControl != null) { diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index a58642517a20..1a15230413d6 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -9377,6 +9377,11 @@ public final class ViewRootImpl implements ViewParent, return mInputEventReceiver.getToken(); } + @NonNull + public IBinder getWindowToken() { + return mAttachInfo.mWindowToken; + } + /** * Class for managing the accessibility interaction connection * based on the global accessibility state. diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index aedb59bfee42..3be0a4d39a56 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -2109,28 +2109,36 @@ public final class InputMethodManager { /** * Call showSoftInput with currently focused view. - * @return {@code true} if IME can be shown. + * + * @param windowToken the window from which this request originates. If this doesn't match the + * currently served view, the request is ignored and returns {@code false}. + * + * @return {@code true} if IME can (eventually) be shown, {@code false} otherwise. * @hide */ - public boolean requestImeShow(ResultReceiver resultReceiver) { + public boolean requestImeShow(IBinder windowToken) { synchronized (mH) { final View servedView = getServedViewLocked(); - if (servedView == null) { + if (servedView == null || servedView.getWindowToken() != windowToken) { return false; } - showSoftInput(servedView, 0 /* flags */, resultReceiver); + showSoftInput(servedView, 0 /* flags */, null /* resultReceiver */); return true; } } /** * Notify IME directly that it is no longer visible. + * + * @param windowToken the window from which this request originates. If this doesn't match the + * currently served view, the request is ignored. * @hide */ - public void notifyImeHidden() { + public void notifyImeHidden(IBinder windowToken) { synchronized (mH) { try { - if (mCurMethod != null) { + if (mCurMethod != null && mCurRootView != null + && mCurRootView.getWindowToken() == windowToken) { mCurMethod.notifyImeHidden(); } } catch (RemoteException re) { @@ -2140,15 +2148,15 @@ public final class InputMethodManager { /** * Notify IME directly to remove surface as it is no longer visible. + * @param windowToken The client window token that requests the IME to remove its surface. * @hide */ - public void removeImeSurface() { + public void removeImeSurface(IBinder windowToken) { synchronized (mH) { try { - if (mCurMethod != null) { - mCurMethod.removeImeSurface(); - } - } catch (RemoteException re) { + mService.removeImeSurfaceFromWindow(windowToken); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } } } diff --git a/core/java/com/android/internal/view/IInputMethodManager.aidl b/core/java/com/android/internal/view/IInputMethodManager.aidl index 8ec51b89d240..a1cbd3fcae79 100644 --- a/core/java/com/android/internal/view/IInputMethodManager.aidl +++ b/core/java/com/android/internal/view/IInputMethodManager.aidl @@ -73,5 +73,8 @@ interface IInputMethodManager { in float[] matrixValues); oneway void reportPerceptible(in IBinder windowToken, boolean perceptible); + /** Remove the IME surface. Requires INTERNAL_SYSTEM_WINDOW permission. */ void removeImeSurface(); + /** Remove the IME surface. Requires passing the currently focused window. */ + void removeImeSurfaceFromWindow(in IBinder windowToken); } diff --git a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java index 1b3272572db0..7efd616c5607 100644 --- a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java +++ b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java @@ -71,6 +71,9 @@ public class InsetsSourceConsumerTest { private SurfaceControl mLeash; @Mock Transaction mMockTransaction; private InsetsSource mSpyInsetsSource; + private boolean mRemoveSurfaceCalled = false; + private InsetsController mController; + private InsetsState mState; @Before public void setup() { @@ -89,13 +92,19 @@ public class InsetsSourceConsumerTest { } catch (BadTokenException e) { // activity isn't running, lets ignore BadTokenException. } - InsetsState state = new InsetsState(); + mState = new InsetsState(); mSpyInsetsSource = Mockito.spy(new InsetsSource(ITYPE_STATUS_BAR)); - state.addSource(mSpyInsetsSource); - - mConsumer = new InsetsSourceConsumer(ITYPE_STATUS_BAR, state, - () -> mMockTransaction, - new InsetsController(new ViewRootInsetsControllerHost(viewRootImpl))); + mState.addSource(mSpyInsetsSource); + + mController = new InsetsController(new ViewRootInsetsControllerHost(viewRootImpl)); + mConsumer = new InsetsSourceConsumer(ITYPE_STATUS_BAR, mState, + () -> mMockTransaction, mController) { + @Override + public void removeSurface() { + super.removeSurface(); + mRemoveSurfaceCalled = true; + } + }; }); instrumentation.waitForIdleSync(); @@ -171,6 +180,25 @@ public class InsetsSourceConsumerTest { mConsumer.setControl(new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, new Point()), new int[1], hideTypes); assertEquals(statusBars(), hideTypes[0]); + assertFalse(mRemoveSurfaceCalled); + }); + } + + @Test + public void testRestore_noAnimation() { + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + mConsumer.hide(); + mController.onStateChanged(mState); + mConsumer.setControl(null, new int[1], new int[1]); + reset(mMockTransaction); + verifyZeroInteractions(mMockTransaction); + mRemoveSurfaceCalled = false; + int[] hideTypes = new int[1]; + mConsumer.setControl(new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, new Point()), + new int[1], hideTypes); + assertTrue(mRemoveSurfaceCalled); + assertEquals(0, hideTypes[0]); }); + } } diff --git a/packages/SystemUI/src/com/android/systemui/wm/DisplayImeController.java b/packages/SystemUI/src/com/android/systemui/wm/DisplayImeController.java index 8ba5b9951c54..c0089e53f8b6 100644 --- a/packages/SystemUI/src/com/android/systemui/wm/DisplayImeController.java +++ b/packages/SystemUI/src/com/android/systemui/wm/DisplayImeController.java @@ -226,6 +226,8 @@ public class DisplayImeController implements DisplayController.OnDisplaysChanged if (!activeControl.getSurfacePosition().equals(lastSurfacePosition) && mAnimation != null) { startAnimation(mImeShowing, true /* forceRestart */); + } else if (!mImeShowing) { + removeImeSurface(); } }); } @@ -370,16 +372,7 @@ public class DisplayImeController implements DisplayController.OnDisplaysChanged dispatchEndPositioning(mDisplayId, mCancelled, t); if (mAnimationDirection == DIRECTION_HIDE && !mCancelled) { t.hide(mImeSourceControl.getLeash()); - final IInputMethodManager imms = getImms(); - if (imms != null) { - try { - // Remove the IME surface to make the insets invisible for - // non-client controlled insets. - imms.removeImeSurface(); - } catch (RemoteException e) { - Slog.e(TAG, "Failed to remove IME surface.", e); - } - } + removeImeSurface(); } t.apply(); mTransactionPool.release(t); @@ -402,6 +395,19 @@ public class DisplayImeController implements DisplayController.OnDisplaysChanged } } + void removeImeSurface() { + final IInputMethodManager imms = getImms(); + if (imms != null) { + try { + // Remove the IME surface to make the insets invisible for + // non-client controlled insets. + imms.removeImeSurface(); + } catch (RemoteException e) { + Slog.e(TAG, "Failed to remove IME surface.", e); + } + } + } + /** * Allows other things to synchronize with the ime position */ diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerInternal.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerInternal.java index 70f0399d1070..05cf40a091b6 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerInternal.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerInternal.java @@ -119,6 +119,11 @@ public abstract class InputMethodManagerInternal { */ public abstract void reportImeControl(@Nullable IBinder windowToken); + /** + * Destroys the IME surface. + */ + public abstract void removeImeSurface(); + /** * Fake implementation of {@link InputMethodManagerInternal}. All the methods do nothing. */ @@ -166,6 +171,10 @@ public abstract class InputMethodManagerInternal { @Override public void reportImeControl(@Nullable IBinder windowToken) { } + + @Override + public void removeImeSurface() { + } }; /** diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index 15e8a92ba0e4..41c3e9562be0 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -211,6 +211,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub static final int MSG_INITIALIZE_IME = 1040; static final int MSG_CREATE_SESSION = 1050; static final int MSG_REMOVE_IME_SURFACE = 1060; + static final int MSG_REMOVE_IME_SURFACE_FROM_WINDOW = 1061; static final int MSG_START_INPUT = 2000; @@ -4000,6 +4001,13 @@ public class InputMethodManagerService extends IInputMethodManager.Stub mHandler.sendMessage(mHandler.obtainMessage(MSG_REMOVE_IME_SURFACE)); } + @Override + public void removeImeSurfaceFromWindow(IBinder windowToken) { + // No permission check, because we'll only execute the request if the calling window is + // also the current IME client. + mHandler.obtainMessage(MSG_REMOVE_IME_SURFACE_FROM_WINDOW, windowToken).sendToTarget(); + } + @BinderThread private void notifyUserAction(@NonNull IBinder token) { if (DEBUG) { @@ -4273,11 +4281,27 @@ public class InputMethodManagerService extends IInputMethodManager.Stub return true; } case MSG_REMOVE_IME_SURFACE: { - try { - if (mEnabledSession != null && mEnabledSession.session != null) { - mEnabledSession.session.removeImeSurface(); + synchronized (mMethodMap) { + try { + if (mEnabledSession != null && mEnabledSession.session != null + && !mShowRequested) { + mEnabledSession.session.removeImeSurface(); + } + } catch (RemoteException e) { + } + } + return true; + } + case MSG_REMOVE_IME_SURFACE_FROM_WINDOW: { + IBinder windowToken = (IBinder) msg.obj; + synchronized (mMethodMap) { + try { + if (windowToken == mCurFocusedWindow + && mEnabledSession != null && mEnabledSession.session != null) { + mEnabledSession.session.removeImeSurface(); + } + } catch (RemoteException e) { } - } catch (RemoteException e) { } return true; } @@ -5111,6 +5135,11 @@ public class InputMethodManagerService extends IInputMethodManager.Stub public void reportImeControl(@Nullable IBinder windowToken) { mService.reportImeControl(windowToken); } + + @Override + public void removeImeSurface() { + mService.mHandler.sendMessage(mService.mHandler.obtainMessage(MSG_REMOVE_IME_SURFACE)); + } } @BinderThread diff --git a/services/core/java/com/android/server/inputmethod/MultiClientInputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/MultiClientInputMethodManagerService.java index 2e3d3963c9df..19dff9807075 100644 --- a/services/core/java/com/android/server/inputmethod/MultiClientInputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/MultiClientInputMethodManagerService.java @@ -225,6 +225,11 @@ public final class MultiClientInputMethodManagerService { @Override public void reportImeControl(@Nullable IBinder windowToken) { } + + @Override + public void removeImeSurface() { + reportNotSupported(); + } }); } @@ -1471,6 +1476,12 @@ public final class MultiClientInputMethodManagerService { reportNotSupported(); } + @BinderThread + @Override + public void removeImeSurfaceFromWindow(IBinder windowToken) { + reportNotSupported(); + } + @BinderThread @Override public boolean showSoftInput( diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index c56440785bba..273b91354205 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -3561,12 +3561,11 @@ class DisplayContent extends WindowContainer @@ -3590,6 +3589,17 @@ class DisplayContent extends WindowContainer mImeTargetFromIme.mSubLayer) || mImeTargetFromIme == mDisplayContent.getImeFallback() - // If IME target is transparent but control target matches requesting window. - || (controlTarget == mImeTargetFromIme - && PixelFormat.formatHasAlpha(dcTarget.mAttrs.format)); + || (!mImeTargetFromIme.isClosing() && controlTarget == mImeTargetFromIme); + } + + @Override + public void dump(PrintWriter pw, String prefix) { + super.dump(pw, prefix); + if (mImeTargetFromIme != null) { + pw.print(prefix); + pw.print("showImePostLayout pending for mImeTargetFromIme="); + pw.print(mImeTargetFromIme); + pw.println(); + } } } diff --git a/services/core/java/com/android/server/wm/InsetsControlTarget.java b/services/core/java/com/android/server/wm/InsetsControlTarget.java index c50f296504fc..3ffc26a7a8ad 100644 --- a/services/core/java/com/android/server/wm/InsetsControlTarget.java +++ b/services/core/java/com/android/server/wm/InsetsControlTarget.java @@ -62,4 +62,8 @@ interface InsetsControlTarget { return false; } + /** Returns {@code target.getWindow()}, or null if {@code target} is {@code null}. */ + static WindowState asWindowOrNull(InsetsControlTarget target) { + return target != null ? target.getWindow() : null; + } } diff --git a/services/core/java/com/android/server/wm/InsetsStateController.java b/services/core/java/com/android/server/wm/InsetsStateController.java index 63083faaddb1..77bd4a47a884 100644 --- a/services/core/java/com/android/server/wm/InsetsStateController.java +++ b/services/core/java/com/android/server/wm/InsetsStateController.java @@ -44,6 +44,7 @@ import android.view.InsetsState; import android.view.InsetsState.InternalInsetsType; import android.view.WindowManager; +import com.android.server.inputmethod.InputMethodManagerInternal; import com.android.server.protolog.common.ProtoLog; import java.io.PrintWriter; @@ -74,7 +75,21 @@ class InsetsStateController { w.notifyInsetsChanged(); } }; - private final InsetsControlTarget mEmptyImeControlTarget = new InsetsControlTarget() { }; + private final InsetsControlTarget mEmptyImeControlTarget = new InsetsControlTarget() { + @Override + public void notifyInsetsControlChanged() { + InsetsSourceControl[] controls = getControlsForDispatch(this); + if (controls == null) { + return; + } + for (InsetsSourceControl control : controls) { + if (control.getType() == ITYPE_IME) { + mDisplayContent.mWmService.mH.post(() -> + InputMethodManagerInternal.get().removeImeSurface()); + } + } + } + }; InsetsStateController(DisplayContent displayContent) { mDisplayContent = displayContent; diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index b65020d7eaa6..881401de9b97 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -2163,6 +2163,9 @@ class WindowState extends WindowContainer implements WindowManagerP if (isInputMethodTarget()) { dc.computeImeTarget(true /* updateImeTarget */); } + if (dc.mInputMethodControlTarget == this) { + dc.updateImeControlTarget(); + } final int type = mAttrs.type; if (WindowManagerService.excludeWindowTypeFromTapOutTask(type)) { 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 ddb186a1d2da..d3f3c9c03cf8 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java @@ -908,6 +908,26 @@ public class DisplayContentTests extends WindowTestsBase { assertEquals(dc.mInputMethodInputTarget, dc.computeImeControlTarget()); } + @Test + public void testComputeImeControlTarget_exitingApp() throws Exception { + final DisplayContent dc = createNewDisplay(); + + WindowState exitingWin = createWindow(null, TYPE_BASE_APPLICATION, "exiting app"); + makeWindowVisible(exitingWin); + exitingWin.mWinAnimator.mDrawState = WindowStateAnimator.HAS_DRAWN; + exitingWin.mAnimatingExit = true; + + dc.mInputMethodControlTarget = exitingWin; + dc.mInputMethodTarget = dc.mInputMethodInputTarget = + createWindow(null, TYPE_BASE_APPLICATION, "starting app"); + + assertEquals(exitingWin, dc.computeImeControlTarget()); + + exitingWin.removeImmediately(); + + assertEquals(dc.mInputMethodInputTarget, dc.computeImeControlTarget()); + } + @Test public void testComputeImeControlTarget_splitscreen() throws Exception { final DisplayContent dc = createNewDisplay(); -- cgit v1.2.3-59-g8ed1b