diff options
| author | 2017-02-08 11:13:25 -0800 | |
|---|---|---|
| committer | 2017-02-09 00:51:18 +0000 | |
| commit | 2bc66171cce4d5ae7bee2c3920e82e45a9d245af (patch) | |
| tree | c2822968e4dbe767d146b979ce23c714b7f014ff | |
| parent | fd62c58ede768be5830a052ee2d5ee6ade787839 (diff) | |
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
1544def0facda69c210b0ae64b17394ea2860d39
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
10 files changed, 118 insertions, 98 deletions
diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java index 5996fe2b14a3..5ae1fd057122 100644 --- a/core/java/android/inputmethodservice/InputMethodService.java +++ b/core/java/android/inputmethodservice/InputMethodService.java @@ -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) diff --git a/core/java/android/view/inputmethod/InputConnection.java b/core/java/android/view/inputmethod/InputConnection.java index 71c1d624976b..57f9895f45fa 100644 --- a/core/java/android/view/inputmethod/InputConnection.java +++ b/core/java/android/view/inputmethod/InputConnection.java @@ -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}. - * - * @return true on success, false if the input connection is no longer - * valid. + * Called back when the connected IME switches between fullscreen and normal modes. + * + * <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); diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index 2e99092006f0..13abe7c6471d 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -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; } } } @@ -557,18 +573,11 @@ public final class InputMethodManager { } @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); @@ -731,16 +747,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 { mService.registerSuggestionSpansForNotification(spans); @@ -770,6 +776,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); diff --git a/core/java/com/android/internal/view/IInputConnectionWrapper.java b/core/java/com/android/internal/view/IInputConnectionWrapper.java index 0185e306118f..cb2d88522f3b 100644 --- a/core/java/com/android/internal/view/IInputConnectionWrapper.java +++ b/core/java/com/android/internal/view/IInputConnectionWrapper.java @@ -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()) { diff --git a/core/java/com/android/internal/view/IInputContext.aidl b/core/java/com/android/internal/view/IInputContext.aidl index 728c55786c49..c22799179b72 100644 --- a/core/java/com/android/internal/view/IInputContext.aidl +++ b/core/java/com/android/internal/view/IInputContext.aidl @@ -62,9 +62,7 @@ import com.android.internal.view.IInputContextCallback; void beginBatchEdit(); void endBatchEdit(); - - void reportFullscreenMode(boolean enabled); - + void sendKeyEvent(in KeyEvent event); void clearMetaKeyStates(int states); diff --git a/core/java/com/android/internal/view/IInputMethodClient.aidl b/core/java/com/android/internal/view/IInputMethodClient.aidl index 81056f2edd96..ffa9f75e4370 100644 --- a/core/java/com/android/internal/view/IInputMethodClient.aidl +++ b/core/java/com/android/internal/view/IInputMethodClient.aidl @@ -27,6 +27,7 @@ oneway interface IInputMethodClient { void onBindMethod(in InputBindResult res); // unbindReason corresponds to InputMethodClient.UnbindReason. void onUnbindMethod(int sequence, int unbindReason); - void setActive(boolean active); + void setActive(boolean active, boolean fullscreen); void setUserActionNotificationSequenceNumber(int sequenceNumber); + void reportFullscreenMode(boolean fullscreen); } diff --git a/core/java/com/android/internal/view/IInputMethodManager.aidl b/core/java/com/android/internal/view/IInputMethodManager.aidl index 9e4b43b6c007..2279a6706524 100644 --- a/core/java/com/android/internal/view/IInputMethodManager.aidl +++ b/core/java/com/android/internal/view/IInputMethodManager.aidl @@ -86,5 +86,7 @@ interface IInputMethodManager { IInputContentUriToken createInputContentUriToken(in IBinder token, in Uri contentUri, in String packageName); + void reportFullscreenMode(in IBinder token, boolean fullscreen); + oneway void notifyUserAction(int sequenceNumber); } diff --git a/core/java/com/android/internal/view/InputConnectionWrapper.java b/core/java/com/android/internal/view/InputConnectionWrapper.java index 9a09dcccd1a7..cc0ef756c6b3 100644 --- a/core/java/com/android/internal/view/InputConnectionWrapper.java +++ b/core/java/com/android/internal/view/InputConnectionWrapper.java @@ -467,12 +467,8 @@ public class InputConnectionWrapper implements InputConnection { } public boolean reportFullscreenMode(boolean enabled) { - try { - mIInputContext.reportFullscreenMode(enabled); - return true; - } catch (RemoteException e) { - return false; - } + // Nothing should happen when called from input method. + return false; } public boolean performPrivateCommand(String action, Bundle data) { diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index 22eca77c3a06..c5e91f5de80c 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -181,6 +181,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub static final int MSG_SET_ACTIVE = 3020; static final int MSG_SET_INTERACTIVE = 3030; static final int MSG_SET_USER_ACTION_NOTIFICATION_SEQUENCE_NUMBER = 3040; + static final int MSG_REPORT_FULLSCREEN_MODE = 3045; static final int MSG_SWITCH_IME = 3050; static final int MSG_HARD_KEYBOARD_SWITCH_CHANGED = 4000; @@ -411,6 +412,11 @@ public class InputMethodManagerService extends IInputMethodManager.Stub boolean mInputShown; /** + * {@code true} if the current input method is in fullscreen mode. + */ + boolean mInFullscreenMode; + + /** * The Intent used to connect to the current input method. */ Intent mCurIntent; @@ -1265,8 +1271,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } } - executeOrSendMessage(mCurClient.client, mCaller.obtainMessageIO( - MSG_SET_ACTIVE, 0, mCurClient)); + executeOrSendMessage(mCurClient.client, mCaller.obtainMessageIIO( + MSG_SET_ACTIVE, 0, 0, mCurClient)); executeOrSendMessage(mCurClient.client, mCaller.obtainMessageIIO( MSG_UNBIND_CLIENT, mCurSeq, unbindClientReason, mCurClient.client)); mCurClient.sessionRequested = false; @@ -1646,6 +1652,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub if (mStatusBar != null) { mStatusBar.setIconVisibility(mSlotIme, false); } + mInFullscreenMode = false; } @Override @@ -2912,7 +2919,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } case MSG_SET_ACTIVE: try { - ((ClientState)msg.obj).client.setActive(msg.arg1 != 0); + ((ClientState)msg.obj).client.setActive(msg.arg1 != 0, msg.arg2 != 0); } catch (RemoteException e) { Slog.w(TAG, "Got RemoteException sending setActive(false) notification to pid " + ((ClientState)msg.obj).pid + " uid " @@ -2939,6 +2946,18 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } return true; } + case MSG_REPORT_FULLSCREEN_MODE: { + final boolean fullscreen = msg.arg1 != 0; + final ClientState clientState = (ClientState)msg.obj; + try { + clientState.client.reportFullscreenMode(fullscreen); + } catch (RemoteException e) { + Slog.w(TAG, "Got RemoteException sending " + + "reportFullscreen(" + fullscreen + ") notification to pid=" + + clientState.pid + " uid=" + clientState.uid); + } + return true; + } // -------------------------------------------------------------- case MSG_HARD_KEYBOARD_SWITCH_CHANGED: @@ -2959,8 +2978,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub // Inform the current client of the change in active status if (mCurClient != null && mCurClient.client != null) { - executeOrSendMessage(mCurClient.client, mCaller.obtainMessageIO( - MSG_SET_ACTIVE, mIsInteractive ? 1 : 0, mCurClient)); + executeOrSendMessage(mCurClient.client, mCaller.obtainMessageIIO( + MSG_SET_ACTIVE, mIsInteractive ? 1 : 0, mInFullscreenMode ? 1 : 0, + mCurClient)); } } } @@ -3960,6 +3980,23 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } @Override + public void reportFullscreenMode(IBinder token, boolean fullscreen) { + if (!calledFromValidUser()) { + return; + } + synchronized (mMethodMap) { + if (!calledWithValidToken(token)) { + return; + } + if (mCurClient != null && mCurClient.client != null) { + mInFullscreenMode = fullscreen; + executeOrSendMessage(mCurClient.client, mCaller.obtainMessageIO( + MSG_REPORT_FULLSCREEN_MODE, fullscreen ? 1 : 0, mCurClient)); + } + } + } + + @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) != PackageManager.PERMISSION_GRANTED) { @@ -4011,6 +4048,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub + " mShowExplicitlyRequested=" + mShowExplicitlyRequested + " mShowForced=" + mShowForced + " mInputShown=" + mInputShown); + p.println(" mInFullscreenMode=" + mInFullscreenMode); p.println(" mCurUserActionNotificationSequenceNumber=" + mCurUserActionNotificationSequenceNumber); p.println(" mSystemReady=" + mSystemReady + " mInteractive=" + mIsInteractive); diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeIInputMethodManager.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeIInputMethodManager.java index ab73a8b820cd..468710bbb470 100644 --- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeIInputMethodManager.java +++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeIInputMethodManager.java @@ -248,4 +248,9 @@ public class BridgeIInputMethodManager implements IInputMethodManager { // TODO Auto-generated method stub return null; } + + @Override + public void reportFullscreenMode(IBinder token, boolean fullscreen) { + // TODO Auto-generated method stub + } } |