From 8e9214b4bd7e6a9d944ad1527199226e575a7530 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Tue, 24 May 2016 16:35:17 -0700 Subject: [PATCH] Make IMM more robust to spurious window focus-in InputMethodManager (IMM) has a latch switch named IMM#mHasBeenInactive to forcefully refresh IME focus state when an inactive client (IMM#mActive == false) is gaining window focus. However, it turns out that there is a race condition where the latch could be unexpectedly turned off. This is probably what we have been chasing in bug 25373872. Imagine the following scenario: 1. An app receives MSG_WINDOW_FOCUS_CHANGED w/ hasWindowFocus=false 2. IMM inside the app receives MSG_SET_ACTIVE w/ active=false 3. The app receives MSG_WINDOW_FOCUS_CHANGED w/ hasWindowFocus=true 4. The app receives MSG_WINDOW_FOCUS_CHANGED w/ hasWindowFocus=false 5. The app receives MSG_WINDOW_FOCUS_CHANGED w/ hasWindowFocus=true Here, our current strategy has been: A. Turn on the latch when MSG_SET_ACTIVE (w/active=false) is handled. B. Turn off the latch and ask IMMS to start input when MSG_WINDOW_FOCUS_CHANGED (w/ hasWindowFocus=true) is handled. The problem is that in the step B IMMS can reject the request if WindowManagerService (WMS) tells that the window in question no longer has window focus. This is not surprising because the app is just handling messages in the message queue sequentially. As a result, the IME focus is not updated expectedly in the step 5, because the latch is no longer enabled as we expected. With this CL, the latch will be re-enabled if the app fails to start input while IMM#mActive is false as a short-term solution. In future we may want to address this issue in protocol level so that we can address other known issues such as bug 26851566 at the same time. Bug: 28281870 Change-Id: I60adb38013b063918b074c7b947649eada77b2c8 --- .../view/inputmethod/InputMethodManager.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index 1618758929ef1..4013b30fce06f 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -1277,6 +1277,28 @@ public final class InputMethodManager { return true; } } + } else { + if (startInputReason + == InputMethodClient.START_INPUT_REASON_WINDOW_FOCUS_GAIN) { + // We are here probably because of an obsolete window-focus-in message sent + // to windowGainingFocus. Since IMMS determines whether a Window can have + // IME focus or not by using the latest window focus state maintained in the + // WMS, this kind of race condition cannot be avoided. One obvious example + // would be that we have already received a window-focus-out message but the + // UI thread is still handling previous window-focus-in message here. + // TODO: InputBindResult should have the error code. + if (DEBUG) Log.w(TAG, "startInputOrWindowGainedFocus failed. " + + "Window focus may have already been lost. " + + "win=" + windowGainingFocus + " view=" + dumpViewInfo(view)); + if (!mActive) { + // mHasBeenInactive is a latch switch to forcefully refresh IME focus + // state when an inactive (mActive == false) client is gaining window + // focus. In case we have unnecessary disable the latch due to this + // spurious wakeup, we re-enable the latch here. + // TODO: Come up with more robust solution. + mHasBeenInactive = true; + } + } } if (mCurMethod != null && mCompletions != null) { try {