OSDN Git Service

Fix stale InputMethodManager#mFullscreenMode.
authorYohei Yukawa <yukawa@google.com>
Wed, 27 Apr 2016 00:13:38 +0000 (17:13 -0700)
committerYohei Yukawa <yukawa@google.com>
Thu, 28 Apr 2016 12:41:11 +0000 (12:41 +0000)
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

core/java/android/view/inputmethod/InputMethodManager.java
core/java/com/android/internal/view/IInputConnectionWrapper.java

index 1ce8043..5a9a212 100644 (file)
@@ -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();
index 3a4afad..59b4b35 100644 (file)
@@ -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: {