Reduce potential lock contention during start input
IMMS#startInputOrWindowGainedFocus invokes several WM internal methods
to check if the IME client and its windowToken has IME focus:
- getDisplayIdForWindow(windowToken)
- Checked at beginning of startInputOrWindowGainedFocus to know if
the displayID of the client window is same as the client reported
displayID.
- isInputMethodClientFocus(uid, pid, displayId)
- Checked next to getDisplayIdForWindow invoked, it's to know if the
client is focused on the display.
- isUidAllowedOnDisplay(displayId, uid)
- Checked in startInputUncheckedLocked, used to see if the client's
uid is allow to access on the display.
As calling these methods requres holding WM locks, if the lock
contention takes longer time, it may impact the UI
performance since startInputOrWindowGainedFocus is a sync IPC
call that requires the remote client to execute on the UI thread.
To Remove/reduce unnecessary internal module calls that requires
holding lock by followed ways:
1) Remove isUidAllowedOnDisplay & getDisplayIdForWindow validation
check & combine these checks into isInputMethodClientFocus with
adding windowToken parameter, to reduce the timing of being blocked
by WM lock.
2) Make startInputUncheckLocked as private method and add documentation
- as all call points of startInputUncheckedLocked is inside of
startInputOrWindowGainedFocus, even though 1) removed calling
isUidAllowedOnDisplay, it still be checked in the combined version of
isInputMethodClientFocus without any visible change, and adding the
documentation for invoking startInputUncheckedLocked requires
the focus check must done before.
2) Refine WindowManagerInternal#shouldRestoreImeVisibility to
smaller the synchronized block since WMS#getTaskSnapshot should
not be called when holding WM lock.
Bug: 197848765
Test: atest CtsInputMethodTestCases
Change-Id: Ie850a67c116466c46bd62d44f4fbdd5584eda5de
diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
index 5d2a900..dd02128 100644
--- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
+++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java
@@ -2347,11 +2347,20 @@
curId, getSequenceNumberLocked(), suppressesSpellChecker);
}
+ /**
+ * Called by {@link #startInputOrWindowGainedFocusInternalLocked} to bind/unbind/attach the
+ * selected InputMethod to the given focused IME client.
+ *
+ * Note that this should be called after validating if the IME client has IME focus.
+ *
+ * @see WindowManagerInternal#hasInputMethodClientFocus(IBinder, int, int, int)
+ */
@GuardedBy("ImfLock.class")
@NonNull
- InputBindResult startInputUncheckedLocked(@NonNull ClientState cs, IInputContext inputContext,
- @NonNull EditorInfo attribute, @StartInputFlags int startInputFlags,
- @StartInputReason int startInputReason, int unverifiedTargetSdkVersion) {
+ private InputBindResult startInputUncheckedLocked(@NonNull ClientState cs,
+ IInputContext inputContext, @NonNull EditorInfo attribute,
+ @StartInputFlags int startInputFlags, @StartInputReason int startInputReason,
+ int unverifiedTargetSdkVersion) {
// If no method is currently selected, do nothing.
String selectedMethodId = getSelectedMethodIdLocked();
if (selectedMethodId == null) {
@@ -2373,10 +2382,6 @@
return InputBindResult.INVALID_PACKAGE_NAME;
}
- if (!mWindowManagerInternal.isUidAllowedOnDisplay(cs.selfReportedDisplayId, cs.uid)) {
- // Wait, the client no longer has access to the display.
- return InputBindResult.INVALID_DISPLAY_ID;
- }
// Compute the final shown display ID with validated cs.selfReportedDisplayId for this
// session & other conditions.
mDisplayIdToShowIme = computeImeDisplayIdForTarget(cs.selfReportedDisplayId,
@@ -3200,14 +3205,11 @@
// be made before input is started in it.
final ClientState cs = mClients.get(client.asBinder());
if (cs == null) {
- throw new IllegalArgumentException(
- "unknown client " + client.asBinder());
+ throw new IllegalArgumentException("unknown client " + client.asBinder());
}
- if (!mWindowManagerInternal.isInputMethodClientFocus(cs.uid, cs.pid,
- cs.selfReportedDisplayId)) {
+ if (!isImeClientFocused(windowToken, cs)) {
if (DEBUG) {
- Slog.w(TAG,
- "Ignoring hideSoftInput of uid " + uid + ": " + client);
+ Slog.w(TAG, "Ignoring hideSoftInput of uid " + uid + ": " + client);
}
return false;
}
@@ -3277,6 +3279,12 @@
return res;
}
+ private boolean isImeClientFocused(IBinder windowToken, ClientState cs) {
+ final int imeClientFocus = mWindowManagerInternal.hasInputMethodClientFocus(
+ windowToken, cs.uid, cs.pid, cs.selfReportedDisplayId);
+ return imeClientFocus == WindowManagerInternal.ImeClientFocusResult.HAS_IME_FOCUS;
+ }
+
@NonNull
@Override
public InputBindResult startInputOrWindowGainedFocus(
@@ -3370,31 +3378,30 @@
+ " unverifiedTargetSdkVersion=" + unverifiedTargetSdkVersion);
}
- final int windowDisplayId = mWindowManagerInternal.getDisplayIdForWindow(windowToken);
-
final ClientState cs = mClients.get(client.asBinder());
if (cs == null) {
throw new IllegalArgumentException("unknown client " + client.asBinder());
}
- if (cs.selfReportedDisplayId != windowDisplayId) {
- Slog.e(TAG, "startInputOrWindowGainedFocusInternal: display ID mismatch."
- + " from client:" + cs.selfReportedDisplayId
- + " from window:" + windowDisplayId);
- return InputBindResult.DISPLAY_ID_MISMATCH;
- }
- if (!mWindowManagerInternal.isInputMethodClientFocus(cs.uid, cs.pid,
- cs.selfReportedDisplayId)) {
- // Check with the window manager to make sure this client actually
- // has a window with focus. If not, reject. This is thread safe
- // because if the focus changes some time before or after, the
- // next client receiving focus that has any interest in input will
- // be calling through here after that change happens.
- if (DEBUG) {
- Slog.w(TAG, "Focus gain on non-focused client " + cs.client
- + " (uid=" + cs.uid + " pid=" + cs.pid + ")");
- }
- return InputBindResult.NOT_IME_TARGET_WINDOW;
+ final int imeClientFocus = mWindowManagerInternal.hasInputMethodClientFocus(
+ windowToken, cs.uid, cs.pid, cs.selfReportedDisplayId);
+ switch (imeClientFocus) {
+ case WindowManagerInternal.ImeClientFocusResult.DISPLAY_ID_MISMATCH:
+ Slog.e(TAG, "startInputOrWindowGainedFocusInternal: display ID mismatch.");
+ return InputBindResult.DISPLAY_ID_MISMATCH;
+ case WindowManagerInternal.ImeClientFocusResult.NOT_IME_TARGET_WINDOW:
+ // Check with the window manager to make sure this client actually
+ // has a window with focus. If not, reject. This is thread safe
+ // because if the focus changes some time before or after, the
+ // next client receiving focus that has any interest in input will
+ // be calling through here after that change happens.
+ if (DEBUG) {
+ Slog.w(TAG, "Focus gain on non-focused client " + cs.client
+ + " (uid=" + cs.uid + " pid=" + cs.pid + ")");
+ }
+ return InputBindResult.NOT_IME_TARGET_WINDOW;
+ case WindowManagerInternal.ImeClientFocusResult.INVALID_DISPLAY_ID:
+ return InputBindResult.INVALID_DISPLAY_ID;
}
if (mUserSwitchHandlerTask != null) {
@@ -3622,8 +3629,7 @@
if (cs == null) {
throw new IllegalArgumentException("unknown client " + client.asBinder());
}
- if (!mWindowManagerInternal.isInputMethodClientFocus(cs.uid, cs.pid,
- cs.selfReportedDisplayId)) {
+ if (!isImeClientFocused(mCurFocusedWindow, cs)) {
Slog.w(TAG, String.format("Ignoring %s of uid %d : %s", methodName, uid, client));
return false;
}
diff --git a/services/core/java/com/android/server/wm/WindowManagerInternal.java b/services/core/java/com/android/server/wm/WindowManagerInternal.java
index 1ab191b..b9fa297 100644
--- a/services/core/java/com/android/server/wm/WindowManagerInternal.java
+++ b/services/core/java/com/android/server/wm/WindowManagerInternal.java
@@ -16,6 +16,9 @@
package com.android.server.wm;
+import static java.lang.annotation.RetentionPolicy.SOURCE;
+
+import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.ClipData;
@@ -40,6 +43,7 @@
import com.android.server.input.InputManagerService;
import com.android.server.policy.WindowManagerPolicy;
+import java.lang.annotation.Retention;
import java.util.List;
import java.util.Set;
@@ -609,6 +613,7 @@
/**
* Checks whether the specified IME client has IME focus or not.
*
+ * @param windowToken The window token of the input method client
* @param uid UID of the process to be queried
* @param pid PID of the process to be queried
* @param displayId Display ID reported from the client. Note that this method also verifies
@@ -616,7 +621,22 @@
* @return {@code true} if the IME client specified with {@code uid}, {@code pid}, and
* {@code displayId} has IME focus
*/
- public abstract boolean isInputMethodClientFocus(int uid, int pid, int displayId);
+ public abstract @ImeClientFocusResult int hasInputMethodClientFocus(IBinder windowToken,
+ int uid, int pid, int displayId);
+
+ @Retention(SOURCE)
+ @IntDef({
+ ImeClientFocusResult.HAS_IME_FOCUS,
+ ImeClientFocusResult.NOT_IME_TARGET_WINDOW,
+ ImeClientFocusResult.DISPLAY_ID_MISMATCH,
+ ImeClientFocusResult.INVALID_DISPLAY_ID
+ })
+ public @interface ImeClientFocusResult {
+ int HAS_IME_FOCUS = 0;
+ int NOT_IME_TARGET_WINDOW = -1;
+ int DISPLAY_ID_MISMATCH = -2;
+ int INVALID_DISPLAY_ID = -3;
+ }
/**
* Checks whether the given {@code uid} is allowed to use the given {@code displayId} or not.
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 3515926..03c70e1 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -7736,19 +7736,32 @@
}
@Override
- public boolean isInputMethodClientFocus(int uid, int pid, int displayId) {
+ public @ImeClientFocusResult int hasInputMethodClientFocus(IBinder windowToken,
+ int uid, int pid, int displayId) {
if (displayId == Display.INVALID_DISPLAY) {
- return false;
+ return ImeClientFocusResult.INVALID_DISPLAY_ID;
}
synchronized (mGlobalLock) {
final DisplayContent displayContent = mRoot.getTopFocusedDisplayContent();
+ final WindowState window = mWindowMap.get(windowToken);
+ if (window == null) {
+ return ImeClientFocusResult.NOT_IME_TARGET_WINDOW;
+ }
+ final int tokenDisplayId = window.getDisplayContent().getDisplayId();
+ if (tokenDisplayId != displayId) {
+ Slog.e(TAG, "isInputMethodClientFocus: display ID mismatch."
+ + " from client: " + displayId
+ + " from window: " + tokenDisplayId);
+ return ImeClientFocusResult.DISPLAY_ID_MISMATCH;
+ }
if (displayContent == null
|| displayContent.getDisplayId() != displayId
|| !displayContent.hasAccess(uid)) {
- return false;
+ return ImeClientFocusResult.INVALID_DISPLAY_ID;
}
+
if (displayContent.isInputMethodClientFocus(uid, pid)) {
- return true;
+ return ImeClientFocusResult.HAS_IME_FOCUS;
}
// Okay, how about this... what is the current focus?
// It seems in some cases we may not have moved the IM
@@ -7761,10 +7774,11 @@
final WindowState currentFocus = displayContent.mCurrentFocus;
if (currentFocus != null && currentFocus.mSession.mUid == uid
&& currentFocus.mSession.mPid == pid) {
- return currentFocus.canBeImeTarget();
+ return currentFocus.canBeImeTarget() ? ImeClientFocusResult.HAS_IME_FOCUS
+ : ImeClientFocusResult.NOT_IME_TARGET_WINDOW;
}
}
- return false;
+ return ImeClientFocusResult.NOT_IME_TARGET_WINDOW;
}
@Override
@@ -8760,20 +8774,21 @@
}
boolean shouldRestoreImeVisibility(IBinder imeTargetWindowToken) {
+ final Task imeTargetWindowTask;
synchronized (mGlobalLock) {
final WindowState imeTargetWindow = mWindowMap.get(imeTargetWindowToken);
if (imeTargetWindow == null) {
return false;
}
- final Task imeTargetWindowTask = imeTargetWindow.getTask();
+ imeTargetWindowTask = imeTargetWindow.getTask();
if (imeTargetWindowTask == null) {
return false;
}
- final TaskSnapshot snapshot = getTaskSnapshot(imeTargetWindowTask.mTaskId,
- imeTargetWindowTask.mUserId, false /* isLowResolution */,
- false /* restoreFromDisk */);
- return snapshot != null && snapshot.hasImeSurface();
}
+ final TaskSnapshot snapshot = getTaskSnapshot(imeTargetWindowTask.mTaskId,
+ imeTargetWindowTask.mUserId, false /* isLowResolution */,
+ false /* restoreFromDisk */);
+ return snapshot != null && snapshot.hasImeSurface();
}
@Override
