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

Commit c07fd4c2 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Move notifyUserAction() to IInputMethodPrivilegedOperations 

This CL re-implements the way to propagate user action on an IME to
InputMethodManagerService (IMMS) so that we can dynamically update IME
Subtype rotation list discussed as requested in Bug 7043015.

It turns out that my previous CLs [1][2][3][4] are unnecessarily
complex because I tried to monitor user behavior in the IME client
process rather than in the IME process.  In the end, I ended up
introducing a sequence number protocol for the sake of performance
with a ton of complexity.

This could have been implemented in a much safer and simpler way by
sending user action signals from the IME process to IMMS, because

 A. IME already knows when it switches to a new subtype. IME needs to
    send a signal only once per subtype change.  There is no need to
    use sequence counter.
 B. Malicious IME client is unable to disturb IME rotation list by
    sending a fake signal because the IPC endpoint is no longer exposed
    to IME client processes.

In case there remain some applitations that still call this hidden API
via reflection without gracefully handling exceptions, this CL keeps
InputMethodManager.notifyUserAction() as a stub method so as not to
break such applications.

 [1]: I11ed9a767588f8080753cd9bce011dac7db579ad
      d7443c83
 [2]: I7f3e13a7226ef0dceee82b67e8a0d8536f7e9807
      2a6a8d2fbb063c84e388c185402c4ca788618c72
 [3]: I19ad8542659bc092b92ee13eb9f1d68ddd4b815a
      b56c6c721fc01fba8e36632d8e28f5123831abc5
 [4]: I03fa436df0a5e348b3f93170aab3a8ac5a7e1677
      c21ccc151631663d71230a3c1c756d94b575ab9e

Bug: 113177698
Fix: 114159783
Test: Manually verified as follows
  1. Build and flush aosp_taimen-userdebug
  2. make -j SoftKeyboard
  3. adb install -r $OUT/system/app/SoftKeyboard/SoftKeyboard.apk
  4. adb shell ime enable com.example.android.softkeyboard/.SoftKeyboard
  5. Open AOSP Keyboard settings
  6. Enable "English (US)", "French", and "German"
  7. Open SoftKeyboard settings
  8. Enable "English (United States)", "English (GB)"
  9. Open the Dialer app and tap the top edit field.
 10. Make sure that the IME layout rotation order when tapping the
     globe key will be updated only when you tap the keyboard to enter
     some character.
 11. Also confirm it with "adb shell dumpsys input_method" by checking
     "mSwitchingController:" section there.
Change-Id: Icc1f9c7f530f0144ecfd460e86114e109ae0044e
parent a086f954

core/java/android/inputmethodservice/AbstractInputMethodService.java

+8 −0
Original line number Diff line number Diff line
@@ -252,4 +252,12 @@ public abstract class AbstractInputMethodService extends Service 
        return;

    }



    /**

     * Called when the user took some actions that should be taken into consideration to update the

     * MRU list for input method rotation.

     *

     * @hide

     */

    public void notifyUserActionIfNecessary() {

    }

}

core/java/android/inputmethodservice/InputMethodService.java

+30 −1
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import static android.view.WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACK 


import static java.lang.annotation.RetentionPolicy.SOURCE;



import android.annotation.AnyThread;

import android.annotation.CallSuper;

import android.annotation.DrawableRes;

import android.annotation.IntDef;
@@ -79,6 +80,7 @@ import android.widget.ImageButton; 
import android.widget.LinearLayout;

import android.widget.TextView;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.inputmethod.IInputContentUriToken;

import com.android.internal.inputmethod.IInputMethodPrivilegedOperations;

import com.android.internal.inputmethod.InputMethodPrivilegedOperations;
@@ -402,6 +404,10 @@ public class InputMethodService extends AbstractInputMethodService { 
    @BackDispositionMode

    int mBackDisposition;



    private Object mLock = new Object();

    @GuardedBy("mLock")

    private boolean mNotifyUserActionSent;



    /**

     * {@code true} when the previous IME had non-empty inset at the bottom of the screen and we

     * have not shown our own window yet.  In this situation, the previous inset continues to be
@@ -594,7 +600,7 @@ public class InputMethodService extends AbstractInputMethodService { 
        @MainThread

        @Override

        public void changeInputMethodSubtype(InputMethodSubtype subtype) {

            onCurrentInputMethodSubtypeChanged(subtype);

            dispatchOnCurrentInputMethodSubtypeChanged(subtype);

        }

    }
@@ -2792,6 +2798,13 @@ public class InputMethodService extends AbstractInputMethodService { 
        }

    }



    private void dispatchOnCurrentInputMethodSubtypeChanged(InputMethodSubtype newSubtype) {

        synchronized (mLock) {

            mNotifyUserActionSent = false;

        }

        onCurrentInputMethodSubtypeChanged(newSubtype);

    }



    // TODO: Handle the subtype change event

    /**

     * Called when the subtype was changed.
@@ -2847,6 +2860,22 @@ public class InputMethodService extends AbstractInputMethodService { 
        exposeContentInternal(inputContentInfo, getCurrentInputEditorInfo());

    }



    /**

     * {@inheritDoc}

     * @hide

     */

    @AnyThread

    @Override

    public final void notifyUserActionIfNecessary() {

        synchronized (mLock) {

            if (mNotifyUserActionSent) {

                return;

            }

            mPrivOps.notifyUserActionAsync();

            mNotifyUserActionSent = true;

        }

    }



    /**

     * Allow the receiver of {@link InputContentInfo} to obtain a temporary read-only access

     * permission to the content.

core/java/android/view/inputmethod/InputMethodManager.java

+6 −73
Original line number Diff line number Diff line
@@ -353,28 +353,6 @@ public final class InputMethodManager { 
    int mCursorCandStart;

    int mCursorCandEnd;



    /**

     * Represents an invalid action notification sequence number.

     * {@link com.android.server.InputMethodManagerService} always issues a positive integer for

     * action notification sequence numbers. Thus {@code -1} is guaranteed to be different from any

     * valid sequence number.

     */

    private static final int NOT_AN_ACTION_NOTIFICATION_SEQUENCE_NUMBER = -1;

    /**

     * The next sequence number that is to be sent to

     * {@link com.android.server.InputMethodManagerService} via

     * {@link IInputMethodManager#notifyUserAction(int)} at once when a user action is observed.

     */

    private int mNextUserActionNotificationSequenceNumber =

            NOT_AN_ACTION_NOTIFICATION_SEQUENCE_NUMBER;



    /**

     * The last sequence number that is already sent to

     * {@link com.android.server.InputMethodManagerService}.

     */

    private int mLastSentUserActionNotificationSequenceNumber =

            NOT_AN_ACTION_NOTIFICATION_SEQUENCE_NUMBER;



    /**

     * The instance that has previously been sent to the input method.

     */
@@ -421,7 +399,6 @@ public final class InputMethodManager { 
    static final int MSG_SEND_INPUT_EVENT = 5;

    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;



    private static boolean isAutofillUIShowing(View servedView) {
@@ -558,12 +535,6 @@ public final class InputMethodManager { 
                    finishedInputEvent(msg.arg1, false, false);

                    return;

                }

                case MSG_SET_USER_ACTION_NOTIFICATION_SEQUENCE_NUMBER: {

                    synchronized (mH) {

                        mNextUserActionNotificationSequenceNumber = msg.arg1;

                    }

                    return;

                }

                case MSG_REPORT_FULLSCREEN_MODE: {

                    final boolean fullscreen = msg.arg1 != 0;

                    InputConnection ic = null;
@@ -605,11 +576,6 @@ public final class InputMethodManager { 
            closeConnection();

        }



        @Override

        protected void onUserAction() {

            mParentInputMethodManager.notifyUserAction();

        }



        @Override

        public String toString() {

            return "ControlledInputConnectionWrapper{"
@@ -656,12 +622,6 @@ public final class InputMethodManager { 
            mH.obtainMessage(MSG_SET_ACTIVE, active ? 1 : 0, fullscreen ? 1 : 0).sendToTarget();

        }



        @Override

        public void setUserActionNotificationSequenceNumber(int sequenceNumber) {

            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)
@@ -1355,8 +1315,6 @@ public final class InputMethodManager { 
                    mBindSequence = res.sequence;

                    mCurMethod = res.method;

                    mCurId = res.id;

                    mNextUserActionNotificationSequenceNumber =

                            res.userActionNotificationSequenceNumber;

                } else if (res.channel != null && res.channel != mCurChannel) {

                    res.channel.dispose();

                }
@@ -2190,37 +2148,16 @@ public final class InputMethodManager { 


    /**

     * Notify that a user took some action with this input method.

     *

     * @deprecated Just kept to avoid possible app compat issue.

     * @hide

     */

    @Deprecated

    @UnsupportedAppUsage(trackingBug = 114740982, maxTargetSdk = Build.VERSION_CODES.P)

    public void notifyUserAction() {

        synchronized (mH) {

            if (mLastSentUserActionNotificationSequenceNumber ==

                    mNextUserActionNotificationSequenceNumber) {

                if (DEBUG) {

                    Log.w(TAG, "Ignoring notifyUserAction as it has already been sent."

                            + " mLastSentUserActionNotificationSequenceNumber: "

                            + mLastSentUserActionNotificationSequenceNumber

                            + " mNextUserActionNotificationSequenceNumber: "

                            + mNextUserActionNotificationSequenceNumber);

                }

                return;

            }

            try {

                if (DEBUG) {

                    Log.w(TAG, "notifyUserAction: "

                            + " mLastSentUserActionNotificationSequenceNumber: "

                            + mLastSentUserActionNotificationSequenceNumber

                            + " mNextUserActionNotificationSequenceNumber: "

                            + mNextUserActionNotificationSequenceNumber);

                }

                mService.notifyUserAction(mNextUserActionNotificationSequenceNumber);

                mLastSentUserActionNotificationSequenceNumber =

                        mNextUserActionNotificationSequenceNumber;

            } catch (RemoteException e) {

                throw e.rethrowFromSystemServer();

            }

        }

        Log.w(TAG, "notifyUserAction() is a hidden method, which is now just a stub method"

                + " that does nothing.  Leave comments in b.android.com/114740982 if your "

                + " application still depends on the previous behavior of this method.");

    }



    /**
@@ -2421,10 +2358,6 @@ public final class InputMethodManager { 
                + " mCursorSelEnd=" + mCursorSelEnd

                + " mCursorCandStart=" + mCursorCandStart

                + " mCursorCandEnd=" + mCursorCandEnd);

        p.println("  mNextUserActionNotificationSequenceNumber="

                + mNextUserActionNotificationSequenceNumber

                + " mLastSentUserActionNotificationSequenceNumber="

                + mLastSentUserActionNotificationSequenceNumber);

    }



    /**

core/java/com/android/internal/inputmethod/IInputMethodPrivilegedOperations.aidl

+2 −0
Original line number Diff line number Diff line
@@ -39,4 +39,6 @@ interface IInputMethodPrivilegedOperations { 
    boolean switchToPreviousInputMethod();

    boolean switchToNextInputMethod(boolean onlyCurrentIme);

    boolean shouldOfferSwitchingToNextInputMethod();



    oneway void notifyUserActionAsync();

}

core/java/com/android/internal/inputmethod/InputMethodPrivilegedOperations.java

+16 −0
Original line number Diff line number Diff line
@@ -346,4 +346,20 @@ public final class InputMethodPrivilegedOperations { 
            throw e.rethrowFromSystemServer();

        }

    }



    /**

     * Calls {@link IInputMethodPrivilegedOperations#notifyUserActionAsync()}

     */

    @AnyThread

    public void notifyUserActionAsync() {

        final IInputMethodPrivilegedOperations ops = mOps.getAndWarnIfNull();

        if (ops == null) {

            return;

        }

        try {

            ops.notifyUserActionAsync();

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

        }

    }

}