From 1544def0facda69c210b0ae64b17394ea2860d39 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Tue, 26 Apr 2016 17:13:38 -0700 Subject: [PATCH] Fix stale InputMethodManager#mFullscreenMode. The current mechanism to sync InputMethodService#mIsFullscreen to InputMethodManager#mFullscreenMode is really fragile because 1. Currently the state change is notified via InputConnection#reportFullscreenMode(), where InputConnection is designed to be valid only while the IME has input focus to the target widget. 2. In favor of performance InputMethodService (IMS) calls InputConnection#reportFullscreenMode() only when #mIsFullscreen changed. If InputConnection#reportFullscreenMode() failed, there is no recovery mechanism. 3. Screen oriantation change is likely to cause Window/View focus state change in the target application, which is likely to invalidate the current InputConnection. What our previous workaround [1] did for Bug 21455064 was actually relaxing the rule 1 only for InputConnection#reportFullscreenMode(). However, my another CL [2] made the lifetime check of InputConnection a bit more strict again, which revived the issue as Bug 28157836. Probably a long-term fix would be to stop using InputConnection to sync that boolean state between IMS and the application. However, it's too late to do such a refactoring in N, hence this CL relaxes the rule 1 again keeping it as secure as possible. The idea is that we allow InputConnection#reportFullscreenMode() to update InputMethodManager#mFullscreenMode regardless of whether InputConnection is active or not, as long as the InputConnection is bound to the curent IME. Doing this as a short-term solution is supporsed to not introduce any new risk because the active IME is already able to mess up the InputMethodManager#mFullscreenMode by calling InputConnection#reportFullscreenMode() on any other active InputConnection. Bug 28406127 will track the long-term solution. [1]: Id10315efc41d86407ccfb0a2d3956bcd7c0909b8 da589dffddaf4046d3b4fd8d14d5f984a1c4324a [2]: If2a03bc84d318775fd4a197fa43acde086eda442 aaa38c9f1ae019f0fe8c3ba80630f26e582cc89c Bug: 28157836 Change-Id: Iba184245a01a3b340f006bc4e415d304de3c2696 --- .../view/inputmethod/InputMethodManager.java | 25 ++++++++++---- .../view/IInputConnectionWrapper.java | 34 +++++++++++++++---- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index 1ce804342c437..5a9a212538b68 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -39,6 +39,7 @@ import android.os.RemoteException; import android.os.ResultReceiver; import android.os.ServiceManager; import android.os.Trace; +import android.text.TextUtils; import android.text.style.SuggestionSpan; import android.util.Log; import android.util.Pools.Pool; @@ -553,8 +554,9 @@ public final class InputMethodManager { } @Override - protected void onReportFullscreenMode(boolean enabled) { - mParentInputMethodManager.setFullscreenMode(enabled); + protected void onReportFullscreenMode(boolean enabled, boolean calledInBackground) { + mParentInputMethodManager.onReportFullscreenMode(enabled, calledInBackground, + getInputMethodId()); } @Override @@ -563,6 +565,7 @@ public final class InputMethodManager { + "connection=" + getInputConnection() + " finished=" + isFinished() + " mParentInputMethodManager.mActive=" + mParentInputMethodManager.mActive + + " mInputMethodId=" + getInputMethodId() + "}"; } } @@ -718,8 +721,13 @@ public final class InputMethodManager { } /** @hide */ - public void setFullscreenMode(boolean fullScreen) { - mFullscreenMode = fullScreen; + public void onReportFullscreenMode(boolean fullScreen, boolean calledInBackground, + String inputMethodId) { + synchronized (mH) { + if (!calledInBackground || TextUtils.equals(mCurId, inputMethodId)) { + mFullscreenMode = fullScreen; + } + } } /** @hide */ @@ -746,9 +754,11 @@ public final class InputMethodManager { * your UI, else returns false. */ public boolean isFullscreenMode() { - return mFullscreenMode; + synchronized (mH) { + return mFullscreenMode; + } } - + /** * Return true if the given view is the currently active view for the * input method. @@ -1254,6 +1264,9 @@ public final class InputMethodManager { mCurId = res.id; mNextUserActionNotificationSequenceNumber = res.userActionNotificationSequenceNumber; + if (mServedInputConnectionWrapper != null) { + mServedInputConnectionWrapper.setInputMethodId(mCurId); + } } else { if (res.channel != null && res.channel != mCurChannel) { res.channel.dispose(); diff --git a/core/java/com/android/internal/view/IInputConnectionWrapper.java b/core/java/com/android/internal/view/IInputConnectionWrapper.java index 3a4afad3f3670..59b4b35fa24ef 100644 --- a/core/java/com/android/internal/view/IInputConnectionWrapper.java +++ b/core/java/com/android/internal/view/IInputConnectionWrapper.java @@ -71,6 +71,8 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub { private Object mLock = new Object(); @GuardedBy("mLock") private boolean mFinished = false; + @GuardedBy("mLock") + private String mInputMethodId; static class SomeArgs { Object arg1; @@ -109,6 +111,18 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub { } } + public String getInputMethodId() { + synchronized (mLock) { + return mInputMethodId; + } + } + + public void setInputMethodId(final String inputMethodId) { + synchronized (mLock) { + mInputMethodId = inputMethodId; + } + } + abstract protected boolean isActive(); /** @@ -119,9 +133,11 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub { /** * Called when the input method started or stopped full-screen mode. - * + * @param enabled {@code true} if the input method starts full-screen mode. + * @param calledInBackground {@code true} if this input connection is in a state when incoming + * events are usually ignored. */ - abstract protected void onReportFullscreenMode(boolean enabled); + abstract protected void onReportFullscreenMode(boolean enabled, boolean calledInBackground); public void getTextAfterCursor(int length, int flags, int seq, IInputContextCallback callback) { dispatchMessage(obtainMessageIISC(DO_GET_TEXT_AFTER_CURSOR, length, flags, seq, callback)); @@ -464,13 +480,19 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub { } case DO_REPORT_FULLSCREEN_MODE: { InputConnection ic = getInputConnection(); - if (ic == null) { + boolean isBackground = false; + if (ic == null || !isActive()) { Log.w(TAG, "reportFullscreenMode on inexistent InputConnection"); - return; + isBackground = true; } final boolean enabled = msg.arg1 == 1; - ic.reportFullscreenMode(enabled); - onReportFullscreenMode(enabled); + if (!isBackground) { + ic.reportFullscreenMode(enabled); + } + // Due to the nature of asynchronous event handling, currently InputMethodService + // has relied on the fact that #reportFullscreenMode() can be handled even when the + // InputConnection is inactive. We have to notify this event to InputMethodManager. + onReportFullscreenMode(enabled, isBackground); return; } case DO_PERFORM_PRIVATE_COMMAND: {