Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2bc66171 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Eliminate out-of-sync IMM#mFullscreenMode error

As explained in the commit message of my previous CL [1], we have
had a design issue in how to notify the full-screen mode change
from the IME to InputMethodManager running in the target application.

Histrically we have done this by using hooking the following IPC
from the IME to the target application.

  InputConnection#reportFullscreenMode()

However, since we also want InputConnection to be deactivated in some
situations such as the when the target application is no longer
focused. In other words, InputConnection is not a reliable way to
notify something.

As a result, we have suffered from many stale state issues.
Bug 21455064 and Bug 28157836 are such examples.  In Android N, we
introduced yet another hack to work around those issues, but it is
really time to fix the protocol design instead.

The new strategy is to rely on internal IPCs provided by
InputMethodManager to deliver such critical notifications from one
process to the other. This is actually more natural because our goal
is to make sure that InputMethodManager#isFullscreenMode() always
returns the latest value as long as the caller is the focused
application.

For backword compatibility, applications that are monitoring
this callback should continue working, as InputMethodManager emulates
the previous behavior.  However, as updated in JavaDoc, IMEs are no
longer allowed to invoke InputConnection#reportFullscreenMode(),
which should be OK because even on previous releases IMEs should rely on
InputMethodService#updateFullscreenMode() instead.

 [1]: Iba184245a01a3b340f006bc4e415d304de3c2696
      1544def0

Fixes: 28406127
Test: Make sure Bug 21455064 is still fixed.
       1. Input some words in extract mode.
       2. Select a word.
       3. Perform copy.
       4. Select a word.
       5. Rotate the device.
       6. Try to select a word.
       7. Make sure he word is selected and action mode starts.
Test: Make sure Bug 28157836 is still fixed.
       1. Rotate device to landscape mode.
       2. Tap on EditText and start full screen extracted mode.
       3. Rotate device to portrait mode.
       4. Long press to start action mode.
       5. Make sure Action mode gets started.
Test: `adb shell dumpsys input_method` to make sure that fullscreen
      state is synchronized across the app, IMMS, and the IME.
Change-Id: If23e7c7c265ab3dfb48c2fb6fdb361b17d22c594
parent fd62c58e
Loading
Loading
Loading
Loading
+6 −4
Original line number Diff line number Diff line
@@ -387,8 +387,9 @@ public class InputMethodService extends AbstractInputMethodService {
            mInputConnection = binding.getConnection();
            if (DEBUG) Log.v(TAG, "bindInput(): binding=" + binding
                    + " ic=" + mInputConnection);
            InputConnection ic = getCurrentInputConnection();
            if (ic != null) ic.reportFullscreenMode(mIsFullscreen);
            if (mImm != null && mToken != null) {
                mImm.reportFullscreenMode(mToken, mIsFullscreen);
            }
            initialize();
            onBindInput();
        }
@@ -1027,8 +1028,9 @@ public class InputMethodService extends AbstractInputMethodService {
        if (mIsFullscreen != isFullscreen || !mFullscreenApplied) {
            changed = true;
            mIsFullscreen = isFullscreen;
            InputConnection ic = getCurrentInputConnection();
            if (ic != null) ic.reportFullscreenMode(isFullscreen);
            if (mImm != null && mToken != null) {
                mImm.reportFullscreenMode(mToken, mIsFullscreen);
            }
            mFullscreenApplied = true;
            initialize();
            LinearLayout.LayoutParams lp = (LinearLayout.LayoutParams)
+14 −7
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.view.inputmethod;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.inputmethodservice.InputMethodService;
import android.os.Bundle;
import android.os.Handler;
import android.view.KeyCharacterMap;
@@ -751,13 +752,19 @@ public interface InputConnection {
    public boolean clearMetaKeyStates(int states);

    /**
     * Called by the IME to tell the client when it switches between
     * fullscreen and normal modes. This will normally be called for
     * you by the standard implementation of
     * {@link android.inputmethodservice.InputMethodService}.
     * Called back when the connected IME switches between fullscreen and normal modes.
     *
     * @return true on success, false if the input connection is no longer
     * valid.
     * <p>Note: On {@link android.os.Build.VERSION_CODES#O} and later devices, input methods are no
     * longer allowed to directly call this method at any time. To signal this event in the target
     * application, input methods should always call
     * {@link InputMethodService#updateFullscreenMode()} instead. This approach should work on API
     * {@link android.os.Build.VERSION_CODES#N_MR1} and prior devices.</p>
     *
     * @return For editor authors, the return value will always be ignored. For IME authors, this
     *         always returns {@code true} on {@link android.os.Build.VERSION_CODES#N_MR1} and prior
     *         devices and {@code false} on {@link android.os.Build.VERSION_CODES#O} and later
     *         devices.
     * @see InputMethodManager#isFullscreenMode()
     */
    public boolean reportFullscreenMode(boolean enabled);

+43 −28
Original line number Diff line number Diff line
@@ -34,7 +34,6 @@ import android.os.ResultReceiver;
import android.os.ServiceManager;
import android.os.ServiceManager.ServiceNotFoundException;
import android.os.Trace;
import android.text.TextUtils;
import android.text.style.SuggestionSpan;
import android.util.Log;
import android.util.Pools.Pool;
@@ -396,6 +395,7 @@ public final class InputMethodManager {
    static final int MSG_TIMEOUT_INPUT_EVENT = 6;
    static final int MSG_FLUSH_INPUT_EVENT = 7;
    static final int MSG_SET_USER_ACTION_NOTIFICATION_SEQUENCE_NUMBER = 9;
    static final int MSG_REPORT_FULLSCREEN_MODE = 10;

    class H extends Handler {
        H(Looper looper) {
@@ -476,12 +476,13 @@ public final class InputMethodManager {
                }
                case MSG_SET_ACTIVE: {
                    final boolean active = msg.arg1 != 0;
                    final boolean fullscreen = msg.arg2 != 0;
                    if (DEBUG) {
                        Log.i(TAG, "handleMessage: MSG_SET_ACTIVE " + active + ", was " + mActive);
                    }
                    synchronized (mH) {
                        mActive = active;
                        mFullscreenMode = false;
                        mFullscreenMode = fullscreen;
                        if (!active) {
                            // Some other client has starting using the IME, so note
                            // that this happened and make sure our own editor's
@@ -523,6 +524,21 @@ public final class InputMethodManager {
                    synchronized (mH) {
                        mNextUserActionNotificationSequenceNumber = msg.arg1;
                    }
                    return;
                }
                case MSG_REPORT_FULLSCREEN_MODE: {
                    final boolean fullscreen = msg.arg1 != 0;
                    InputConnection ic = null;
                    synchronized (mH) {
                        mFullscreenMode = fullscreen;
                        if (mServedInputConnectionWrapper != null) {
                            ic = mServedInputConnectionWrapper.getInputConnection();
                        }
                    }
                    if (ic != null) {
                        ic.reportFullscreenMode(fullscreen);
                    }
                    return;
                }
            }
        }
@@ -556,19 +572,12 @@ public final class InputMethodManager {
            mParentInputMethodManager.notifyUserAction();
        }

        @Override
        protected void onReportFullscreenMode(boolean enabled, boolean calledInBackground) {
            mParentInputMethodManager.onReportFullscreenMode(enabled, calledInBackground,
                    getInputMethodId());
        }

        @Override
        public String toString() {
            return "ControlledInputConnectionWrapper{"
                    + "connection=" + getInputConnection()
                    + " finished=" + isFinished()
                    + " mParentInputMethodManager.mActive=" + mParentInputMethodManager.mActive
                    + " mInputMethodId=" + getInputMethodId()
                    + "}";
        }
    }
@@ -600,24 +609,31 @@ public final class InputMethodManager {

        @Override
        public void onBindMethod(InputBindResult res) {
            mH.sendMessage(mH.obtainMessage(MSG_BIND, res));
            mH.obtainMessage(MSG_BIND, res).sendToTarget();
        }

        @Override
        public void onUnbindMethod(int sequence, @InputMethodClient.UnbindReason int unbindReason) {
            mH.sendMessage(mH.obtainMessage(MSG_UNBIND, sequence, unbindReason));
            mH.obtainMessage(MSG_UNBIND, sequence, unbindReason).sendToTarget();
        }

        @Override
        public void setActive(boolean active) {
            mH.sendMessage(mH.obtainMessage(MSG_SET_ACTIVE, active ? 1 : 0, 0));
        public void setActive(boolean active, boolean fullscreen) {
            mH.obtainMessage(MSG_SET_ACTIVE, active ? 1 : 0, fullscreen ? 1 : 0).sendToTarget();
        }

        @Override
        public void setUserActionNotificationSequenceNumber(int sequenceNumber) {
            mH.sendMessage(mH.obtainMessage(MSG_SET_USER_ACTION_NOTIFICATION_SEQUENCE_NUMBER,
                    sequenceNumber, 0));
            mH.obtainMessage(MSG_SET_USER_ACTION_NOTIFICATION_SEQUENCE_NUMBER, sequenceNumber, 0)
                    .sendToTarget();
        }

        @Override
        public void reportFullscreenMode(boolean fullscreen) {
            mH.obtainMessage(MSG_REPORT_FULLSCREEN_MODE, fullscreen ? 1 : 0, 0)
                    .sendToTarget();
        }

    };

    final InputConnection mDummyInputConnection = new BaseInputConnection(this, false);
@@ -730,16 +746,6 @@ public final class InputMethodManager {
        }
    }

    /** @hide */
    public void onReportFullscreenMode(boolean fullScreen, boolean calledInBackground,
            String inputMethodId) {
        synchronized (mH) {
            if (!calledInBackground || TextUtils.equals(mCurId, inputMethodId)) {
                mFullscreenMode = fullScreen;
            }
        }
    }

    /** @hide */
    public void registerSuggestionSpansForNotification(SuggestionSpan[] spans) {
        try {
@@ -769,6 +775,17 @@ public final class InputMethodManager {
        }
    }

    /**
     * @hide
     */
    public void reportFullscreenMode(IBinder token, boolean fullscreen) {
        try {
            mService.reportFullscreenMode(token, fullscreen);
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
    }

    /**
     * Return true if the given view is the currently active view for the
     * input method.
@@ -1274,9 +1291,6 @@ 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();
@@ -2341,6 +2355,7 @@ public final class InputMethodManager {
                + " mHasBeenInactive=" + mHasBeenInactive
                + " mBindSequence=" + mBindSequence
                + " mCurId=" + mCurId);
        p.println("  mFullscreenMode=" + mFullscreenMode);
        p.println("  mCurMethod=" + mCurMethod);
        p.println("  mCurRootView=" + mCurRootView);
        p.println("  mServedView=" + mServedView);
+0 −44
Original line number Diff line number Diff line
@@ -57,7 +57,6 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
    private static final int DO_DELETE_SURROUNDING_TEXT_IN_CODE_POINTS = 81;
    private static final int DO_BEGIN_BATCH_EDIT = 90;
    private static final int DO_END_BATCH_EDIT = 95;
    private static final int DO_REPORT_FULLSCREEN_MODE = 100;
    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;
@@ -73,8 +72,6 @@ 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;
@@ -113,18 +110,6 @@ 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();

    /**
@@ -133,14 +118,6 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
     */
    abstract protected void onUserAction();

    /**
     * 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, boolean calledInBackground);

    public void getTextAfterCursor(int length, int flags, int seq, IInputContextCallback callback) {
        dispatchMessage(obtainMessageIISC(DO_GET_TEXT_AFTER_CURSOR, length, flags, seq, callback));
    }
@@ -225,10 +202,6 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
        dispatchMessage(obtainMessage(DO_END_BATCH_EDIT));
    }

    public void reportFullscreenMode(boolean enabled) {
        dispatchMessage(obtainMessageII(DO_REPORT_FULLSCREEN_MODE, enabled ? 1 : 0, 0));
    }

    public void performPrivateCommand(String action, Bundle data) {
        dispatchMessage(obtainMessageOO(DO_PERFORM_PRIVATE_COMMAND, action, data));
    }
@@ -486,23 +459,6 @@ public abstract class IInputConnectionWrapper extends IInputContext.Stub {
                ic.endBatchEdit();
                return;
            }
            case DO_REPORT_FULLSCREEN_MODE: {
                InputConnection ic = getInputConnection();
                boolean isBackground = false;
                if (ic == null || !isActive()) {
                    Log.w(TAG, "reportFullscreenMode on inexistent InputConnection");
                    isBackground = true;
                }
                final boolean enabled = msg.arg1 == 1;
                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: {
                InputConnection ic = getInputConnection();
                if (ic == null || !isActive()) {
+1 −3
Original line number Diff line number Diff line
@@ -63,8 +63,6 @@ import com.android.internal.view.IInputContextCallback;
    
    void endBatchEdit();

    void reportFullscreenMode(boolean enabled);
    
    void sendKeyEvent(in KeyEvent event);
    
    void clearMetaKeyStates(int states);
Loading