OSDN Git Service

Make sure to call back reportFinish() on the desired Handler.
authorYohei Yukawa <yukawa@google.com>
Wed, 23 Mar 2016 04:27:48 +0000 (21:27 -0700)
committerYohei Yukawa <yukawa@google.com>
Wed, 23 Mar 2016 04:27:48 +0000 (21:27 -0700)
Before exposing #reportFinish() as a public API, we have to fix an
existing bug that my previous CL [1] for Bug 26945674 forgot to take care.

Currently BaseInputConnection#reportFinish() is always called by using
the root view's Handler.  We should move the logic to call
BaseInputConnection#reportFinishInputConnection() from ViewRootImpl to
IInputConnectionWrapper to make sure that the method in question can
always be called on the desired Handler.

To make things simple, instead of explicitly calling #reportFinish()
from IMM, this CL let ControlledInputConnectionWrapper#diactivate()
internally call #reportFinish() as needed.  This makes it easier to make
sure that #reportFinish() is called after all the queued method calls
are handled.

  [1]: Id9e579bb3e2966986cdcb1c34bc8cacfeca2e1a9
       612cce92ad96eda1146c3abd2afa7aaa4d4f2b3f

Bug: 25332806
Change-Id: Ibe94f115e607a198d12ecd3d4e4f91a7d9469c98

core/java/android/view/ViewRootImpl.java
core/java/android/view/inputmethod/BaseInputConnection.java
core/java/android/view/inputmethod/InputMethodManager.java
core/java/com/android/internal/view/IInputConnectionWrapper.java
core/java/com/android/internal/widget/EditableInputConnection.java

index a2295ce..da7799c 100644 (file)
@@ -3287,7 +3287,6 @@ public final class ViewRootImpl implements ViewParent,
     private final static int MSG_DISPATCH_APP_VISIBILITY = 8;
     private final static int MSG_DISPATCH_GET_NEW_SURFACE = 9;
     private final static int MSG_DISPATCH_KEY_FROM_IME = 11;
-    private final static int MSG_FINISH_INPUT_CONNECTION = 12;
     private final static int MSG_CHECK_FOCUS = 13;
     private final static int MSG_CLOSE_SYSTEM_DIALOGS = 14;
     private final static int MSG_DISPATCH_DRAG_EVENT = 15;
@@ -3327,8 +3326,6 @@ public final class ViewRootImpl implements ViewParent,
                     return "MSG_DISPATCH_GET_NEW_SURFACE";
                 case MSG_DISPATCH_KEY_FROM_IME:
                     return "MSG_DISPATCH_KEY_FROM_IME";
-                case MSG_FINISH_INPUT_CONNECTION:
-                    return "MSG_FINISH_INPUT_CONNECTION";
                 case MSG_CHECK_FOCUS:
                     return "MSG_CHECK_FOCUS";
                 case MSG_CLOSE_SYSTEM_DIALOGS:
@@ -3549,12 +3546,6 @@ public final class ViewRootImpl implements ViewParent,
                 }
                 enqueueInputEvent(event, null, QueuedInputEvent.FLAG_DELIVER_POST_IME, true);
             } break;
-            case MSG_FINISH_INPUT_CONNECTION: {
-                InputMethodManager imm = InputMethodManager.peekInstance();
-                if (imm != null) {
-                    imm.reportFinishInputConnection((InputConnection)msg.obj);
-                }
-            } break;
             case MSG_CHECK_FOCUS: {
                 InputMethodManager imm = InputMethodManager.peekInstance();
                 if (imm != null) {
@@ -5864,11 +5855,6 @@ public final class ViewRootImpl implements ViewParent,
         }
     }
 
-    public void dispatchFinishInputConnection(InputConnection connection) {
-        Message msg = mHandler.obtainMessage(MSG_FINISH_INPUT_CONNECTION, connection);
-        mHandler.sendMessage(msg);
-    }
-
     public void dispatchResized(Rect frame, Rect overscanInsets, Rect contentInsets,
             Rect visibleInsets, Rect stableInsets, Rect outsets, boolean reportDraw,
             Configuration newConfig, Rect backDropFrame, boolean forceLayout,
index 6a830f8..a3c49c5 100644 (file)
@@ -158,8 +158,8 @@ public class BaseInputConnection implements InputConnection {
      *
      * @hide
      */
-    protected void reportFinish() {
-        // Intentionaly empty
+    public void reportFinish() {
+        // Intentionally empty
     }
 
     /**
index 2a17989..ca36829 100644 (file)
@@ -531,22 +531,25 @@ public final class InputMethodManager {
 
     private static class ControlledInputConnectionWrapper extends IInputConnectionWrapper {
         private final InputMethodManager mParentInputMethodManager;
-        private boolean mActive;
 
         public ControlledInputConnectionWrapper(final Looper mainLooper, final InputConnection conn,
                 final InputMethodManager inputMethodManager) {
             super(mainLooper, conn);
             mParentInputMethodManager = inputMethodManager;
-            mActive = true;
         }
 
         @Override
         public boolean isActive() {
-            return mParentInputMethodManager.mActive && mActive;
+            return mParentInputMethodManager.mActive && !isFinished();
         }
 
         void deactivate() {
-            mActive = false;
+            if (isFinished()) {
+                // This is a small performance optimization.  Still only the 1st call of
+                // reportFinish() will take effect.
+                return;
+            }
+            reportFinish();
         }
 
         @Override
@@ -563,7 +566,7 @@ public final class InputMethodManager {
         public String toString() {
             return "ControlledInputConnectionWrapper{"
                     + "connection=" + getInputConnection()
-                    + " mActive=" + mActive
+                    + " finished=" + isFinished()
                     + " mParentInputMethodManager.mActive=" + mParentInputMethodManager.mActive
                     + "}";
         }
@@ -837,7 +840,6 @@ public final class InputMethodManager {
                     throw e.rethrowFromSystemServer();
                 }
             }
-            notifyInputConnectionFinished();
             mServedView = null;
             mCompletions = null;
             mServedConnecting = false;
@@ -845,48 +847,6 @@ public final class InputMethodManager {
         }
     }
 
-    /**
-     * Notifies the served view that the current InputConnection will no longer be used.
-     */
-    private void notifyInputConnectionFinished() {
-        if (mServedView == null || mServedInputConnectionWrapper == null) {
-            return;
-        }
-        final InputConnection inputConnection = mServedInputConnectionWrapper.getInputConnection();
-        if (inputConnection == null) {
-            return;
-        }
-        // We need to tell the previously served view that it is no
-        // longer the input target, so it can reset its state.  Schedule
-        // this call on its window's Handler so it will be on the correct
-        // thread and outside of our lock.
-        ViewRootImpl viewRootImpl = mServedView.getViewRootImpl();
-        if (viewRootImpl != null) {
-            // This will result in a call to reportFinishInputConnection() below.
-            viewRootImpl.dispatchFinishInputConnection(inputConnection);
-        }
-    }
-
-    /**
-     * Called from the FINISH_INPUT_CONNECTION message above.
-     * @hide
-     */
-    public void reportFinishInputConnection(InputConnection ic) {
-        final InputConnection currentConnection;
-        if (mServedInputConnectionWrapper == null) {
-            currentConnection = null;
-        } else {
-            currentConnection = mServedInputConnectionWrapper.getInputConnection();
-        }
-        if (currentConnection != ic) {
-            ic.finishComposingText();
-            // To avoid modifying the public InputConnection interface
-            if (ic instanceof BaseInputConnection) {
-                ((BaseInputConnection) ic).reportFinish();
-            }
-        }
-    }
-
     public void displayCompletions(View view, CompletionInfo[] completions) {
         checkFocus();
         synchronized (mH) {
@@ -1232,8 +1192,10 @@ public final class InputMethodManager {
             // Hook 'em up and let 'er rip.
             mCurrentTextBoxAttribute = tba;
             mServedConnecting = false;
-            // Notify the served view that its previous input connection is finished
-            notifyInputConnectionFinished();
+            if (mServedInputConnectionWrapper != null) {
+                mServedInputConnectionWrapper.deactivate();
+                mServedInputConnectionWrapper = null;
+            }
             ControlledInputConnectionWrapper servedContext;
             final int missingMethodFlags;
             if (ic != null) {
@@ -1258,9 +1220,6 @@ public final class InputMethodManager {
                 servedContext = null;
                 missingMethodFlags = 0;
             }
-            if (mServedInputConnectionWrapper != null) {
-                mServedInputConnectionWrapper.deactivate();
-            }
             mServedInputConnectionWrapper = servedContext;
 
             try {
index afa1554..02b9db3 100644 (file)
@@ -16,6 +16,8 @@
 
 package com.android.internal.view;
 
+import com.android.internal.annotations.GuardedBy;
+
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.os.Bundle;
@@ -25,6 +27,7 @@ import android.os.Message;
 import android.os.RemoteException;
 import android.util.Log;
 import android.view.KeyEvent;
+import android.view.inputmethod.BaseInputConnection;
 import android.view.inputmethod.CompletionInfo;
 import android.view.inputmethod.CorrectionInfo;
 import android.view.inputmethod.ExtractedTextRequest;
@@ -58,12 +61,16 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
     private static final int DO_PERFORM_PRIVATE_COMMAND = 120;
     private static final int DO_CLEAR_META_KEY_STATES = 130;
     private static final int DO_REQUEST_UPDATE_CURSOR_ANCHOR_INFO = 140;
+    private static final int DO_REPORT_FINISH = 150;
 
     @NonNull
     private final WeakReference<InputConnection> mInputConnection;
 
     private Looper mMainLooper;
     private Handler mH;
+    private Object mLock = new Object();
+    @GuardedBy("mLock")
+    private boolean mFinished = false;
 
     static class SomeArgs {
         Object arg1;
@@ -94,6 +101,12 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
         return mInputConnection.get();
     }
 
+    protected boolean isFinished() {
+        synchronized (mLock) {
+            return mFinished;
+        }
+    }
+
     abstract protected boolean isActive();
 
     /**
@@ -206,6 +219,10 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
                 seq, callback));
     }
 
+    public void reportFinish() {
+        dispatchMessage(obtainMessage(DO_REPORT_FINISH));
+    }
+
     void dispatchMessage(Message msg) {
         // If we are calling this from the main thread, then we can call
         // right through.  Otherwise, we need to send the message to the
@@ -218,7 +235,7 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
         
         mH.sendMessage(msg);
     }
-    
+
     void executeMessage(Message msg) {
         switch (msg.what) {
             case DO_GET_TEXT_AFTER_CURSOR: {
@@ -481,6 +498,36 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
                 }
                 return;
             }
+            case DO_REPORT_FINISH: {
+                // Note that we do not need to worry about race condition here, because 1) mFinished
+                // is updated only inside this block, and 2) the code here is running on a Handler
+                // hence we assume multiple DO_REPORT_FINISH messages will not be handled at the
+                // same time.
+                if (isFinished()) {
+                    return;
+                }
+                try {
+                    InputConnection ic = mInputConnection.get();
+                    // Note we do NOT check isActive() here, because this is safe
+                    // for an IME to call at any time, and we need to allow it
+                    // through to clean up our state after the IME has switched to
+                    // another client.
+                    if (ic == null) {
+                        return;
+                    }
+                    ic.finishComposingText();
+                    // TODO: Make reportFinish() public method of InputConnection to remove this
+                    // check.
+                    if (ic instanceof BaseInputConnection) {
+                        ((BaseInputConnection) ic).reportFinish();
+                    }
+                } finally {
+                    synchronized (mLock) {
+                        mFinished = true;
+                    }
+                }
+                return;
+            }
         }
         Log.w(TAG, "Unhandled message code: " + msg.what);
     }
index f211ff2..a96b5a0 100644 (file)
@@ -83,8 +83,11 @@ public class EditableInputConnection extends BaseInputConnection {
         return false;
     }
 
+    /**
+     * @hide
+     */
     @Override
-    protected void reportFinish() {
+    public void reportFinish() {
         super.reportFinish();
 
         synchronized(this) {