From 3ca950724ccb01a0371ba16ffd8ee74affcb365f Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Sat, 15 Oct 2022 14:54:34 -0700 Subject: [PATCH] Simplify FINISH_INPUT_NO_FALLBACK_CONNECTION handling This is the 6th CL for Bug 26851566 [1][2][3][4][5]. In the CLs [3][5], the approach we took was to extend an existing IPC method IInputMethodClient#setActive(boolean active, boolean fullscreen) by adding one more parameter "reportToImeController". However, it seems that such an approach made its handling code hard to maintain, because the IPC method needed to behave in a completely different manner depending on the newly introduced parameter. To avoid such a confusion and to make subsequent refactoring easier, this CL introduces a new IPC method IInputMethodClient#setInteractive( boolean interactive, boolean fullscreen) to clarify what we are actually doing. In most cases there must be no observable behavior change. One possible behavior change is when setInteractive() gets called when IMM#mCurRootView.getView() is not available. In the previous CL, we fell back to previous behavior as if FINISH_INPUT_NO_FALLBACK_CONNECTION was not defined. However it looks like such a fallback mechanism is not an intentional behavior. The main use case is still verified with existing CTS tests [6][7]. [1]: Ic11956fe745a829da1ec7c09ea214e9b3961c8e0 0df8812486ff0b104622dec72d6243a783b10257 [2]: If06daf71160aa44a4254ac125561974ecbdef4f2 2c6e80be1253140f1761c1bb4cc601e7e350e1b2 [3]: I8a657e75e274d842fb46b60375f6aeafeab96a59 152eacec4f5e1c78eafd42abbcf62fc12b0eee33 [4]: I16f4a34360a2f64b69978724648a9be741f140b5 3d62f739a869c97a265506ac1e1ab58f93fa3b34 [5]: Id4e71a822bfde5fe6a263bbe094c0d238017efe1 3d62f739a869c97a265506ac1e1ab58f93fa3b34 [6]: Iba0332ed3b767a9a15768b64947b9cde31105af1 e2f4138156c784746becde6a01c19f0c3de6ac32 [7]: I346ef42f2546a2925bf44ca7719ae19399e15e78 84c5ec2f81d4be5285b9c5f1d0e943f808e9158c Bug: 26851566 Bug: 156215187 Test: atest InputMethodStartInputLifecycleTest Change-Id: I0c95d7e70357ac98b336cd1b51a7405ba92a9a6f --- .../view/inputmethod/InputMethodManager.java | 44 +++++++++++++------ .../inputmethod/IInputMethodClient.aidl | 3 +- .../IInputMethodClientInvoker.java | 29 +++++++++--- .../inputmethod/ImePlatformCompatUtils.java | 2 +- .../InputMethodManagerService.java | 19 ++++---- 5 files changed, 68 insertions(+), 29 deletions(-) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index b883d0c69d713..6fb7e78a11b9c 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -649,6 +649,7 @@ public final class InputMethodManager { private static final int MSG_REPORT_FULLSCREEN_MODE = 10; private static final int MSG_BIND_ACCESSIBILITY_SERVICE = 11; private static final int MSG_UNBIND_ACCESSIBILITY_SERVICE = 12; + private static final int MSG_SET_INTERACTIVE = 13; private static final int MSG_UPDATE_VIRTUAL_DISPLAY_TO_SCREEN_MATRIX = 30; private static final int MSG_ON_SHOW_REQUESTED = 31; @@ -1160,7 +1161,6 @@ public final class InputMethodManager { case MSG_SET_ACTIVE: { final boolean active = msg.arg1 != 0; final boolean fullscreen = msg.arg2 != 0; - final boolean reportToImeController = msg.obj != null && (boolean) msg.obj; if (DEBUG) { Log.i(TAG, "handleMessage: MSG_SET_ACTIVE " + active + ", was " + mActive); } @@ -1168,15 +1168,6 @@ public final class InputMethodManager { mActive = active; mFullscreenMode = fullscreen; - // Report active state to ImeFocusController to handle IME input - // connection lifecycle callback when it allowed. - final ImeFocusController controller = getFocusController(); - final View rootView = mCurRootView != null ? mCurRootView.getView() : null; - if (controller != null && rootView != null && reportToImeController) { - rootView.post(() -> controller.onInteractiveChanged(active)); - return; - } - if (!active) { // Some other client has starting using the IME, so note // that this happened and make sure our own editor's @@ -1200,6 +1191,28 @@ public final class InputMethodManager { } return; } + case MSG_SET_INTERACTIVE: { + final boolean interactive = msg.arg1 != 0; + final boolean fullscreen = msg.arg2 != 0; + if (DEBUG) { + Log.i(TAG, "handleMessage: MSG_SET_INTERACTIVE " + interactive + + ", was " + mActive); + } + synchronized (mH) { + mActive = interactive; + mFullscreenMode = fullscreen; + + // Report active state to ImeFocusController to handle IME input + // connection lifecycle callback when it allowed. + final ImeFocusController controller = getFocusController(); + final View rootView = mCurRootView != null ? mCurRootView.getView() : null; + if (controller == null || rootView == null) { + return; + } + rootView.post(() -> controller.onInteractiveChanged(interactive)); + } + return; + } case MSG_SEND_INPUT_EVENT: { sendInputEventAndReportResultOnMainLooper((PendingEvent)msg.obj); return; @@ -1317,9 +1330,14 @@ public final class InputMethodManager { } @Override - public void setActive(boolean active, boolean fullscreen, boolean reportToImeController) { - mH.obtainMessage(MSG_SET_ACTIVE, active ? 1 : 0, fullscreen ? 1 : 0, - reportToImeController).sendToTarget(); + public void setActive(boolean active, boolean fullscreen) { + mH.obtainMessage(MSG_SET_ACTIVE, active ? 1 : 0, fullscreen ? 1 : 0).sendToTarget(); + } + + @Override + public void setInteractive(boolean interactive, boolean fullscreen) { + mH.obtainMessage(MSG_SET_INTERACTIVE, interactive ? 1 : 0, fullscreen ? 1 : 0) + .sendToTarget(); } @Override diff --git a/core/java/com/android/internal/inputmethod/IInputMethodClient.aidl b/core/java/com/android/internal/inputmethod/IInputMethodClient.aidl index 15769cc7a4473..3644fc491ffd6 100644 --- a/core/java/com/android/internal/inputmethod/IInputMethodClient.aidl +++ b/core/java/com/android/internal/inputmethod/IInputMethodClient.aidl @@ -27,7 +27,8 @@ oneway interface IInputMethodClient { void onBindAccessibilityService(in InputBindResult res, int id); void onUnbindMethod(int sequence, int unbindReason); void onUnbindAccessibilityService(int sequence, int id); - void setActive(boolean active, boolean fullscreen, boolean reportToImeController); + void setActive(boolean active, boolean fullscreen); + void setInteractive(boolean active, boolean fullscreen); void scheduleStartInputIfNecessary(boolean fullscreen); void reportFullscreenMode(boolean fullscreen); void updateVirtualDisplayToScreenMatrix(int bindSequence, in float[] matrixValues); diff --git a/services/core/java/com/android/server/inputmethod/IInputMethodClientInvoker.java b/services/core/java/com/android/server/inputmethod/IInputMethodClientInvoker.java index b0602368af173..b1580b1fe8e25 100644 --- a/services/core/java/com/android/server/inputmethod/IInputMethodClientInvoker.java +++ b/services/core/java/com/android/server/inputmethod/IInputMethodClientInvoker.java @@ -177,19 +177,36 @@ final class IInputMethodClientInvoker { } @AnyThread - void setActive(boolean active, boolean fullscreen, boolean reportToImeController) { + void setActive(boolean active, boolean fullscreen) { if (mIsProxy) { - setActiveInternal(active, fullscreen, reportToImeController); + setActiveInternal(active, fullscreen); } else { - mHandler.post(() -> setActiveInternal(active, fullscreen, reportToImeController)); + mHandler.post(() -> setActiveInternal(active, fullscreen)); } } @AnyThread - private void setActiveInternal(boolean active, boolean fullscreen, - boolean reportToImeController) { + private void setActiveInternal(boolean active, boolean fullscreen) { try { - mTarget.setActive(active, fullscreen, reportToImeController); + mTarget.setActive(active, fullscreen); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void setInteractive(boolean interactive, boolean fullscreen) { + if (mIsProxy) { + setInteractiveInternal(interactive, fullscreen); + } else { + mHandler.post(() -> setInteractiveInternal(interactive, fullscreen)); + } + } + + @AnyThread + private void setInteractiveInternal(boolean interactive, boolean fullscreen) { + try { + mTarget.setInteractive(interactive, fullscreen); } catch (RemoteException e) { logRemoteException(e); } diff --git a/services/core/java/com/android/server/inputmethod/ImePlatformCompatUtils.java b/services/core/java/com/android/server/inputmethod/ImePlatformCompatUtils.java index 83ca16d72c9b8..61582903b2773 100644 --- a/services/core/java/com/android/server/inputmethod/ImePlatformCompatUtils.java +++ b/services/core/java/com/android/server/inputmethod/ImePlatformCompatUtils.java @@ -39,7 +39,7 @@ final class ImePlatformCompatUtils { * * @param imeUid The uid of the IME application */ - public boolean shouldFinishInputWithReportToIme(int imeUid) { + public boolean shouldUseSetInteractiveProtocol(int imeUid) { return isChangeEnabledByUid(FINISH_INPUT_NO_FALLBACK_CONNECTION, imeUid); } diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index 605a2038339c6..76331fd6089c6 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -2330,8 +2330,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub // Since we set active false to current client and set mCurClient to null, let's unbind // all accessibility too. That means, when input method get disconnected (including // switching ime), we also unbind accessibility - mCurClient.mClient.setActive(false /* active */, false /* fullscreen */, - false /* reportToImeController */); + mCurClient.mClient.setActive(false /* active */, false /* fullscreen */); mCurClient.mClient.onUnbindMethod(getSequenceNumberLocked(), unbindClientReason); mCurClient.mSessionRequested = false; mCurClient.mSessionRequestedForAccessibility = false; @@ -2638,8 +2637,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub unbindCurrentClientLocked(UnbindReason.SWITCH_CLIENT); // If the screen is on, inform the new client it is active if (mIsInteractive) { - cs.mClient.setActive(true /* active */, false /* fullscreen */, - false /* reportToImeController */); + cs.mClient.setActive(true /* active */, false /* fullscreen */); } } @@ -5016,10 +5014,15 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub updateSystemUiLocked(interactive ? mImeWindowVis : 0, mBackDisposition); // Inform the current client of the change in active status - if (mCurClient != null && mCurClient.mClient != null) { - mCurClient.mClient.setActive(mIsInteractive, mInFullscreenMode, - mImePlatformCompatUtils.shouldFinishInputWithReportToIme( - getCurMethodUidLocked())); + if (mCurClient == null || mCurClient.mClient == null) { + return; + } + if (mImePlatformCompatUtils.shouldUseSetInteractiveProtocol(getCurMethodUidLocked())) { + // Eligible IME processes use new "setInteractive" protocol. + mCurClient.mClient.setInteractive(mIsInteractive, mInFullscreenMode); + } else { + // Legacy IME processes continue using legacy "setActive" protocol. + mCurClient.mClient.setActive(mIsInteractive, mInFullscreenMode); } } }