summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Ming-Shin Lu <lumark@google.com> 2020-07-08 12:54:07 +0800
committer Ming-Shin Lu <lumark@google.com> 2020-07-11 00:42:17 +0800
commitd43b75cf0edea197ed2ee690814eedf6844aadbe (patch)
tree8522a0c4d9936ffbf300c99bb9edbb22f71c5824
parent5a293b8eb2f0a57ee5b523872329f5e1378dc05e (diff)
Consolidate start new input scenerios
As CL[1] we introduced WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR start input reason to ignore start new input when the focused view is same as current served view and the input connection remains the same for prevent additonal onStartInput / onFinishInput callback to InputMethodService. The main idea in the CL is good but how to judge whether the input connection remains the same is not accurate. CL[1] only checking if IMM#mServedInputConnectionWrapper self and its input connection instance is stil exists, that breaks the following cases to start new input: Case 1: - When device screen off / on to go back to focused app, this case will fit WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR use case, so IMM#mServedInputConnectionWrapper won't be deactivate and clear, this makes wrong when user taps IME picker dialog to switch IME, will hit again WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR and never start new input for new IME. Actually, in InputMethodManager has an ad-hoc check mRestartOnNextWindowFocus to start new input when device screen off / on case and switching IME, we should not ignore start input since that will conflict with the above case. Case 2: - As served view is now tracked by ImeFocusController which is per-window based instance from the IME focus handling refectoring CL[2], but InputMethodManager instance is still per-display based, so IMM#mCurrentTextBoxAttribute might be changed when the same app clinet has multiple IME focusable window focus changed, because focusing to the next IME focusable window will start new input connection and changes IMM#mCurrentTextBoxAttribute, so when focusing back to the original window, the served view in the original window's ImeFocusController will same as focused view, in that case if we didn't check if IMM#mCurrentTextBoxAttribute is really aligned with the given focused view, will mis-judge start new input timing and caused user can't type because the input connection state is obsoleted. Those cases can be addessed by using new introduced method IMM#isSameEditorAndAcceptingText, if the focused view is not aligned with same editor or the editor is inactive, we should start new input for Case 2, that also can fix Case 1 that we previously ignored starting new input when switching IME. Beside, we also found CL[3] leverages InputMethodManager#mRestartOnNextWindowFocus to start new input when window focus changed, since originally this ad-hoc check is only used to re-start input for Case 1. As we re-visited the necessary start new input scenerio is only when: - Device screen off -> on - Switching IME - the input connection obsoleted (this also includes when window focus changed) As the result, we can remove all unnecessary logics in IMMS instroduced by CL[1] and remove unnecessary InputMethodManager#mRestartOnNextWindowFocus from CL[3], and preserve the behavior is almost same as Q. [1]: I2da99ae67b9ce4051dec0c0f0e975ebe6e1ab118 [2]: Ib455704fe1e9d243f93190a84f230210dbceac2a [3]: I8d4fff94ba9313b773bc27fcbd019cc88580d3e9 Fix: 160391516 Bug: 158624922 Bug: 152373385 Test: atest FocusHandlingTest Test: atest InputMethodServiceLifecycleTest Test: manual for case 1 0) pre-install 3rd party IME app 1) launch message app and taps Search bar to focus. 2) turn off screen 3) turn on screen to back to focused app 4) press IME switch icon from nav bar 5) choose next IME, expect the IME should be changed. Test: manual for case 2 0) Sample app with 2 activites, each activity the layout has EditText 1) Launch activity A, focus EditText 2) Launch activity B and focus EditText 3) Press back key to back to activity A 4) Verify if typing with keyboard is workable. Change-Id: I1ef3d341af9d473d94d52fd1890deafbae2bc9e1
-rw-r--r--core/java/android/view/ImeFocusController.java10
-rw-r--r--core/java/android/view/inputmethod/InputMethodManager.java46
-rw-r--r--services/core/java/com/android/server/inputmethod/InputMethodManagerService.java26
3 files changed, 41 insertions, 41 deletions
diff --git a/core/java/android/view/ImeFocusController.java b/core/java/android/view/ImeFocusController.java
index 825077ffd57a..ad43f9556d8d 100644
--- a/core/java/android/view/ImeFocusController.java
+++ b/core/java/android/view/ImeFocusController.java
@@ -125,11 +125,11 @@ public final class ImeFocusController {
final View viewForWindowFocus = focusedView != null ? focusedView : mViewRootImpl.mView;
onViewFocusChanged(viewForWindowFocus, true);
- // Skip starting input when the next focused view is same as served view and the served
- // input connection still exists.
+ // Starting new input when the next focused view is same as served view but the
+ // editor is not aligned with the same editor or editor is inactive.
final boolean nextFocusIsServedView = mServedView != null && mServedView == focusedView;
- if (nextFocusIsServedView && immDelegate.isAcceptingText()) {
- forceFocus = false;
+ if (nextFocusIsServedView && !immDelegate.isSameEditorAndAcceptingText(focusedView)) {
+ forceFocus = true;
}
immDelegate.startInputAsyncOnWindowFocusGain(viewForWindowFocus,
@@ -254,7 +254,7 @@ public final class ImeFocusController {
void setCurrentRootView(ViewRootImpl rootView);
boolean isCurrentRootView(ViewRootImpl rootView);
boolean isRestartOnNextWindowFocus(boolean reset);
- boolean isAcceptingText();
+ boolean isSameEditorAndAcceptingText(View view);
}
public View getServedView() {
diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java
index aedb59bfee42..dbbb5e9dfd21 100644
--- a/core/java/android/view/inputmethod/InputMethodManager.java
+++ b/core/java/android/view/inputmethod/InputMethodManager.java
@@ -633,20 +633,21 @@ public final class InputMethodManager {
// we'll just do a window focus gain and call it a day.
try {
View servedView = controller.getServedView();
- boolean nextFocusIsServedView = servedView != null && servedView == focusedView;
+ boolean nextFocusSameEditor = servedView != null && servedView == focusedView
+ && isSameEditorAndAcceptingText(focusedView);
if (DEBUG) {
Log.v(TAG, "Reporting focus gain, without startInput"
- + ", nextFocusIsServedView=" + nextFocusIsServedView);
+ + ", nextFocusIsServedView=" + nextFocusSameEditor);
}
final int startInputReason =
- nextFocusIsServedView ? WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR
+ nextFocusSameEditor ? WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR
: WINDOW_FOCUS_GAIN_REPORT_WITHOUT_EDITOR;
mService.startInputOrWindowGainedFocus(
startInputReason, mClient,
focusedView.getWindowToken(), startInputFlags, softInputMode,
windowFlags,
- nextFocusIsServedView ? mCurrentTextBoxAttribute : null,
- nextFocusIsServedView ? mServedInputConnectionWrapper : null,
+ null,
+ null,
0 /* missingMethodFlags */,
mCurRootView.mContext.getApplicationInfo().targetSdkVersion);
} catch (RemoteException e) {
@@ -671,10 +672,6 @@ public final class InputMethodManager {
@Override
public void setCurrentRootView(ViewRootImpl rootView) {
synchronized (mH) {
- if (mCurRootView != null) {
- // Restart the input when the next window focus state of the root view changed.
- mRestartOnNextWindowFocus = true;
- }
mCurRootView = rootView;
}
}
@@ -704,14 +701,33 @@ public final class InputMethodManager {
}
/**
- * For {@link ImeFocusController} to check if the currently served view is accepting full
- * text edits.
+ * For {@link ImeFocusController} to check if the given focused view aligns with the same
+ * editor and the editor is active to accept the text input.
+ *
+ * TODO(b/160968797): Remove this method and move mCurrentTextBoxAttritube to
+ * ImeFocusController.
+ * In the long-term, we should make mCurrentTextBoxAtrtribue as per-window base instance,
+ * so that we we can directly check if the current focused view aligned with the same editor
+ * in the window without using this checking.
+ *
+ * Note that this method is only use for fixing start new input may ignored issue
+ * (e.g. b/160391516), DO NOT leverage this method to do another check.
*/
- @Override
- public boolean isAcceptingText() {
+ public boolean isSameEditorAndAcceptingText(View view) {
synchronized (mH) {
- return mServedInputConnectionWrapper != null
- && mServedInputConnectionWrapper.getInputConnection() != null;
+ if (!hasServedByInputMethodLocked(view) || mCurrentTextBoxAttribute == null) {
+ return false;
+ }
+
+ final EditorInfo ic = mCurrentTextBoxAttribute;
+ // This sameEditor checking is based on using object hash comparison to check if
+ // some fields of the current EditorInfo (e.g. autoFillId, OpPackageName) the
+ // hash code is same as the given focused view.
+ final boolean sameEditor = view.onCheckIsTextEditor() && view.getId() == ic.fieldId
+ && view.getAutofillId() == ic.autofillId
+ && view.getContext().getOpPackageName() == ic.packageName;
+ return sameEditor && mServedInputConnectionWrapper != null
+ && mServedInputConnectionWrapper.isActive();
}
}
}
diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
index c027ebcfd568..6bee1e1ff413 100644
--- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
+++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
@@ -18,8 +18,6 @@ package com.android.server.inputmethod;
import static android.view.Display.DEFAULT_DISPLAY;
import static android.view.Display.INVALID_DISPLAY;
-import static com.android.internal.inputmethod.StartInputReason.WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR;
-
import static java.lang.annotation.RetentionPolicy.SOURCE;
import android.Manifest;
@@ -718,11 +716,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
*/
int mImeWindowVis;
- /**
- * Checks if the client needs to start input.
- */
- private boolean mCurClientNeedStartInput = false;
-
private AlertDialog.Builder mDialogBuilder;
private AlertDialog mSwitchingDialog;
private IBinder mSwitchingDialogToken = new Binder();
@@ -3466,20 +3459,14 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
if (mCurFocusedWindow == windowToken) {
if (DEBUG) {
Slog.w(TAG, "Window already focused, ignoring focus gain of: " + client
- + " attribute=" + attribute + ", token = " + windowToken);
- }
- // Needs to start input when the same window focus gain but not with the same editor,
- // or when the current client needs to start input (e.g. when focusing the same
- // window after device turned screen on).
- if (attribute != null && (startInputReason != WINDOW_FOCUS_GAIN_REPORT_WITH_SAME_EDITOR
- || mCurClientNeedStartInput)) {
- if (mIsInteractive) {
- mCurClientNeedStartInput = false;
- }
+ + " attribute=" + attribute + ", token = " + windowToken
+ + ", startInputReason="
+ + InputMethodDebug.startInputReasonToString(startInputReason));
+ }
+ if (attribute != null) {
return startInputUncheckedLocked(cs, inputContext, missingMethods,
attribute, startInputFlags, startInputReason);
}
-
return new InputBindResult(
InputBindResult.ResultCode.SUCCESS_REPORT_WINDOW_FOCUS_ONLY,
null, null, null, -1, null);
@@ -4435,9 +4422,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
private void handleSetInteractive(final boolean interactive) {
synchronized (mMethodMap) {
mIsInteractive = interactive;
- if (!interactive) {
- mCurClientNeedStartInput = true;
- }
updateSystemUiLocked(interactive ? mImeWindowVis : 0, mBackDisposition);
// Inform the current client of the change in active status