Deprecate IMMS#executeOrSendMessage()

This CL addresses the remaining TODO from Bug 215609403.

With this CL, IInputMethodClientInvoker takes over the responsibility
of what

  InputMethodManagerService#executeOrSendMessage()

has been doing so that InputMethodManagerService does not need to
worry about whether IInputMethodClient is a BinderProxy or not.

This CL is expected to be really safe.  At least for normal cases when
the IME client is not system_server, there is literally no semantic
change.

Bug: 234882948
Test: presubmit
Change-Id: I0a7dd5162c4b94599b896420b8427913fa772414
This commit is contained in:
Yohei Yukawa
2022-06-07 14:27:28 -07:00
parent cc4c229ec4
commit 1112d366b6
2 changed files with 132 additions and 134 deletions

View File

@@ -21,6 +21,7 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.Binder;
import android.os.DeadObjectException;
import android.os.Handler;
import android.os.IBinder;
import android.os.RemoteException;
import android.util.Slog;
@@ -30,6 +31,17 @@ import com.android.internal.inputmethod.InputBindResult;
/**
* A stateless thin wrapper for {@link IInputMethodClient}.
*
* <p>This class also takes care of a special case when system_server is also an IME client. In this
* scenario methods defined in {@link IInputMethodClient} are invoked as an direct (sync) invocation
* despite the fact that all of them are marked {@code oneway}. This can easily cause dead lock
* because {@link InputMethodManagerService} assumes that it's safe to make one-way IPCs while
* holding the lock. This assumption becomes invalid when {@link IInputMethodClient} is not a
* {@link android.os.BinderProxy}.</p>
*
* <p>To work around such a special scenario, this wrapper re-dispatch the method invocation into
* the given {@link Handler} thread if {@link IInputMethodClient} is not a proxy object. Be careful
* about its call ordering characteristics.</p>
*/
final class IInputMethodClientInvoker {
private static final String TAG = InputMethodManagerService.TAG;
@@ -38,19 +50,25 @@ final class IInputMethodClientInvoker {
@NonNull
private final IInputMethodClient mTarget;
private final boolean mIsProxy;
@Nullable
private final Handler mHandler;
@AnyThread
@Nullable
static IInputMethodClientInvoker create(@Nullable IInputMethodClient inputMethodClient) {
static IInputMethodClientInvoker create(@Nullable IInputMethodClient inputMethodClient,
@NonNull Handler handler) {
if (inputMethodClient == null) {
return null;
}
return new IInputMethodClientInvoker(inputMethodClient);
final boolean isProxy = Binder.isProxy(inputMethodClient);
return new IInputMethodClientInvoker(inputMethodClient, isProxy, isProxy ? null : handler);
}
private IInputMethodClientInvoker(@NonNull IInputMethodClient target) {
private IInputMethodClientInvoker(@NonNull IInputMethodClient target,
boolean isProxy, @Nullable Handler handler) {
mTarget = target;
mIsProxy = Binder.isProxy(mTarget);
mIsProxy = isProxy;
mHandler = handler;
}
/**
@@ -76,6 +94,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void onBindMethod(@NonNull InputBindResult res) {
if (mIsProxy) {
onBindMethodInternal(res);
} else {
mHandler.post(() -> onBindMethodInternal(res));
}
}
@AnyThread
private void onBindMethodInternal(@NonNull InputBindResult res) {
try {
mTarget.onBindMethod(res);
} catch (RemoteException e) {
@@ -91,6 +118,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void onBindAccessibilityService(@NonNull InputBindResult res, int id) {
if (mIsProxy) {
onBindAccessibilityServiceInternal(res, id);
} else {
mHandler.post(() -> onBindAccessibilityServiceInternal(res, id));
}
}
@AnyThread
private void onBindAccessibilityServiceInternal(@NonNull InputBindResult res, int id) {
try {
mTarget.onBindAccessibilityService(res, id);
} catch (RemoteException e) {
@@ -106,6 +142,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void onUnbindMethod(int sequence, int unbindReason) {
if (mIsProxy) {
onUnbindMethodInternal(sequence, unbindReason);
} else {
mHandler.post(() -> onUnbindMethodInternal(sequence, unbindReason));
}
}
@AnyThread
private void onUnbindMethodInternal(int sequence, int unbindReason) {
try {
mTarget.onUnbindMethod(sequence, unbindReason);
} catch (RemoteException e) {
@@ -115,6 +160,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void onUnbindAccessibilityService(int sequence, int id) {
if (mIsProxy) {
onUnbindAccessibilityServiceInternal(sequence, id);
} else {
mHandler.post(() -> onUnbindAccessibilityServiceInternal(sequence, id));
}
}
@AnyThread
private void onUnbindAccessibilityServiceInternal(int sequence, int id) {
try {
mTarget.onUnbindAccessibilityService(sequence, id);
} catch (RemoteException e) {
@@ -124,6 +178,16 @@ final class IInputMethodClientInvoker {
@AnyThread
void setActive(boolean active, boolean fullscreen, boolean reportToImeController) {
if (mIsProxy) {
setActiveInternal(active, fullscreen, reportToImeController);
} else {
mHandler.post(() -> setActiveInternal(active, fullscreen, reportToImeController));
}
}
@AnyThread
private void setActiveInternal(boolean active, boolean fullscreen,
boolean reportToImeController) {
try {
mTarget.setActive(active, fullscreen, reportToImeController);
} catch (RemoteException e) {
@@ -133,6 +197,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void scheduleStartInputIfNecessary(boolean fullscreen) {
if (mIsProxy) {
scheduleStartInputIfNecessaryInternal(fullscreen);
} else {
mHandler.post(() -> scheduleStartInputIfNecessaryInternal(fullscreen));
}
}
@AnyThread
private void scheduleStartInputIfNecessaryInternal(boolean fullscreen) {
try {
mTarget.scheduleStartInputIfNecessary(fullscreen);
} catch (RemoteException e) {
@@ -142,6 +215,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void reportFullscreenMode(boolean fullscreen) {
if (mIsProxy) {
reportFullscreenModeInternal(fullscreen);
} else {
mHandler.post(() -> reportFullscreenModeInternal(fullscreen));
}
}
@AnyThread
private void reportFullscreenModeInternal(boolean fullscreen) {
try {
mTarget.reportFullscreenMode(fullscreen);
} catch (RemoteException e) {
@@ -151,6 +233,17 @@ final class IInputMethodClientInvoker {
@AnyThread
void updateVirtualDisplayToScreenMatrix(int bindSequence, float[] matrixValues) {
if (mIsProxy) {
updateVirtualDisplayToScreenMatrixInternal(bindSequence, matrixValues);
} else {
mHandler.post(() ->
updateVirtualDisplayToScreenMatrixInternal(bindSequence, matrixValues));
}
}
@AnyThread
private void updateVirtualDisplayToScreenMatrixInternal(int bindSequence,
float[] matrixValues) {
try {
mTarget.updateVirtualDisplayToScreenMatrix(bindSequence, matrixValues);
} catch (RemoteException e) {
@@ -160,6 +253,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void setImeTraceEnabled(boolean enabled) {
if (mIsProxy) {
setImeTraceEnabledInternal(enabled);
} else {
mHandler.post(() -> setImeTraceEnabledInternal(enabled));
}
}
@AnyThread
private void setImeTraceEnabledInternal(boolean enabled) {
try {
mTarget.setImeTraceEnabled(enabled);
} catch (RemoteException e) {
@@ -169,6 +271,15 @@ final class IInputMethodClientInvoker {
@AnyThread
void throwExceptionFromSystem(String message) {
if (mIsProxy) {
throwExceptionFromSystemInternal(message);
} else {
mHandler.post(() -> throwExceptionFromSystemInternal(message));
}
}
@AnyThread
private void throwExceptionFromSystemInternal(String message) {
try {
mTarget.throwExceptionFromSystem(message);
} catch (RemoteException e) {

View File

@@ -244,13 +244,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
private static final int MSG_START_HANDWRITING = 1100;
private static final int MSG_FINISH_HANDWRITING = 1110;
private static final int MSG_UNBIND_CLIENT = 3000;
private static final int MSG_UNBIND_ACCESSIBILITY_SERVICE = 3001;
private static final int MSG_BIND_CLIENT = 3010;
private static final int MSG_BIND_ACCESSIBILITY_SERVICE = 3011;
private static final int MSG_SET_ACTIVE = 3020;
private static final int MSG_SET_INTERACTIVE = 3030;
private static final int MSG_REPORT_FULLSCREEN_MODE = 3045;
private static final int MSG_HARD_KEYBOARD_SWITCH_CHANGED = 4000;
@@ -488,10 +482,11 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
+ " pid=" + pid + " displayId=" + selfReportedDisplayId + "}";
}
ClientState(IInputMethodClient _client, IRemoteInputConnection _fallbackInputConnection,
ClientState(IInputMethodClientInvoker _client,
IRemoteInputConnection _fallbackInputConnection,
int _uid, int _pid, int _selfReportedDisplayId,
ClientDeathRecipient _clientDeathRecipient) {
client = IInputMethodClientInvoker.create(_client);
client = _client;
fallbackInputConnection = _fallbackInputConnection;
uid = _uid;
pid = _pid;
@@ -2452,8 +2447,10 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
// have the client crash. Thus we do not verify the display ID at all here. Instead we
// later check the display ID every time the client needs to interact with the specified
// display.
mClients.put(client.asBinder(), new ClientState(client, inputConnection, callerUid,
callerPid, selfReportedDisplayId, deathRecipient));
final IInputMethodClientInvoker clientInvoker =
IInputMethodClientInvoker.create(client, mHandler);
mClients.put(client.asBinder(), new ClientState(clientInvoker, inputConnection,
callerUid, callerPid, selfReportedDisplayId, deathRecipient));
}
}
@@ -2497,57 +2494,6 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
}
}
@NonNull
private Message obtainMessageOO(int what, Object arg1, Object arg2) {
final SomeArgs args = SomeArgs.obtain();
args.arg1 = arg1;
args.arg2 = arg2;
return mHandler.obtainMessage(what, 0, 0, args);
}
@NonNull
private Message obtainMessageOOO(int what, Object arg1, Object arg2, Object arg3) {
SomeArgs args = SomeArgs.obtain();
args.arg1 = arg1;
args.arg2 = arg2;
args.arg3 = arg3;
return mHandler.obtainMessage(what, 0, 0, args);
}
@NonNull
private Message obtainMessageIIOO(int what, int arg1, int arg2,
Object arg3, Object arg4) {
SomeArgs args = SomeArgs.obtain();
args.arg1 = arg3;
args.arg2 = arg4;
return mHandler.obtainMessage(what, arg1, arg2, args);
}
@NonNull
private Message obtainMessageIIIO(int what, int argi1, int argi2, int argi3, Object arg1) {
final SomeArgs args = SomeArgs.obtain();
args.arg1 = arg1;
args.argi1 = argi1;
args.argi2 = argi2;
args.argi3 = argi3;
return mHandler.obtainMessage(what, 0, 0, args);
}
private void executeOrSendMessage(IInputMethodClientInvoker target, Message msg) {
if (target.asBinder() instanceof Binder) {
// This is supposed to be emulating the one-way semantics when the IME client is
// system_server itself, which has not been explicitly prohibited so far while we have
// never ever officially supported such a use case...
// We probably should create a simple wrapper of IInputMethodClient as the first step
// to get rid of executeOrSendMessage() then should prohibit system_server to be the
// IME client for long term.
msg.sendToTarget();
} else {
handleMessage(msg);
msg.recycle();
}
}
@GuardedBy("ImfLock.class")
void unbindCurrentClientLocked(@UnbindReason int unbindClientReason) {
if (mCurClient != null) {
@@ -2565,11 +2511,9 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
// Since we set active false to current client and set mCurClient to null, let's unbind
// all accessibility too. That means, when input method get disconnected (including
// switching ime), we also unbind accessibility
scheduleSetActiveToClient(mCurClient, false /* active */, false /* fullscreen */,
mCurClient.client.setActive(false /* active */, false /* fullscreen */,
false /* reportToImeController */);
executeOrSendMessage(mCurClient.client, mHandler.obtainMessage(
MSG_UNBIND_CLIENT, getSequenceNumberLocked(), unbindClientReason,
mCurClient.client));
mCurClient.client.onUnbindMethod(getSequenceNumberLocked(), unbindClientReason);
mCurClient.sessionRequested = false;
mCurClient.mSessionRequestedForAccessibility = false;
mCurClient = null;
@@ -2901,7 +2845,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
unbindCurrentClientLocked(UnbindReason.SWITCH_CLIENT);
// If the screen is on, inform the new client it is active
if (mIsInteractive) {
scheduleSetActiveToClient(cs, true /* active */, false /* fullscreen */,
cs.client.setActive(true /* active */, false /* fullscreen */,
false /* reportToImeController */);
}
}
@@ -3020,8 +2964,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
attachNewAccessibilityLocked(StartInputReason.SESSION_CREATED_BY_IME,
true, -1);
if (res.method != null) {
executeOrSendMessage(mCurClient.client, obtainMessageOO(
MSG_BIND_CLIENT, mCurClient.client, res));
mCurClient.client.onBindMethod(res);
}
return;
}
@@ -4529,7 +4472,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
public void reportVirtualDisplayGeometryAsync(IInputMethodClient parentClient,
int childDisplayId, float[] matrixValues) {
final IInputMethodClientInvoker parentClientInvoker =
IInputMethodClientInvoker.create(parentClient);
IInputMethodClientInvoker.create(parentClient, mHandler);
try {
final DisplayInfo displayInfo = mDisplayManagerInternal.getDisplayInfo(childDisplayId);
if (displayInfo == null) {
@@ -4997,54 +4940,9 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
// ---------------------------------------------------------
case MSG_UNBIND_CLIENT: {
// This unbinds all accessibility services too.
((IInputMethodClientInvoker) msg.obj).onUnbindMethod(msg.arg1, msg.arg2);
return true;
}
case MSG_UNBIND_ACCESSIBILITY_SERVICE: {
args = (SomeArgs) msg.obj;
IInputMethodClientInvoker client = (IInputMethodClientInvoker) args.arg1;
int id = (int) args.arg2;
client.onUnbindAccessibilityService(msg.arg1, id);
args.recycle();
return true;
}
case MSG_BIND_CLIENT: {
args = (SomeArgs)msg.obj;
IInputMethodClientInvoker client = (IInputMethodClientInvoker) args.arg1;
InputBindResult res = (InputBindResult) args.arg2;
client.onBindMethod(res);
args.recycle();
return true;
}
case MSG_BIND_ACCESSIBILITY_SERVICE: {
args = (SomeArgs) msg.obj;
IInputMethodClientInvoker client = (IInputMethodClientInvoker) args.arg1;
InputBindResult res = (InputBindResult) args.arg2;
int id = (int) args.arg3;
client.onBindAccessibilityService(res, id);
args.recycle();
return true;
}
case MSG_SET_ACTIVE: {
args = (SomeArgs) msg.obj;
final ClientState clientState = (ClientState) args.arg1;
clientState.client.setActive(args.argi1 != 0 /* active */,
args.argi2 != 0 /* fullScreen */,
args.argi3 != 0 /* reportToImeController */);
args.recycle();
return true;
}
case MSG_SET_INTERACTIVE:
handleSetInteractive(msg.arg1 != 0);
return true;
case MSG_REPORT_FULLSCREEN_MODE: {
final boolean fullscreen = msg.arg1 != 0;
final ClientState clientState = (ClientState)msg.obj;
clientState.client.reportFullscreenMode(fullscreen);
return true;
}
// --------------------------------------------------------------
case MSG_HARD_KEYBOARD_SWITCH_CHANGED:
@@ -5138,19 +5036,13 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
// Inform the current client of the change in active status
if (mCurClient != null && mCurClient.client != null) {
scheduleSetActiveToClient(mCurClient, mIsInteractive, mInFullscreenMode,
mCurClient.client.setActive(mIsInteractive, mInFullscreenMode,
mImePlatformCompatUtils.shouldFinishInputWithReportToIme(
getCurMethodUidLocked()));
}
}
}
private void scheduleSetActiveToClient(ClientState state, boolean active, boolean fullscreen,
boolean reportToImeController) {
executeOrSendMessage(state.client, obtainMessageIIIO(MSG_SET_ACTIVE,
active ? 1 : 0, fullscreen ? 1 : 0, reportToImeController ? 1 : 0, state));
}
@GuardedBy("ImfLock.class")
private boolean chooseNewDefaultIMELocked() {
final InputMethodInfo imi = InputMethodUtils.getMostApplicableDefaultIME(
@@ -5724,9 +5616,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
InputBindResult res = attachNewAccessibilityLocked(
StartInputReason.SESSION_CREATED_BY_ACCESSIBILITY, true,
accessibilityConnectionId);
executeOrSendMessage(mCurClient.client, obtainMessageOOO(
MSG_BIND_ACCESSIBILITY_SERVICE, mCurClient.client, res,
accessibilityConnectionId));
mCurClient.client.onBindAccessibilityService(res, accessibilityConnectionId);
}
}
}
@@ -5741,9 +5631,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
}
// A11yManagerService unbinds the disabled accessibility service. We don't need
// to do it here.
executeOrSendMessage(mCurClient.client, obtainMessageIIOO(
MSG_UNBIND_ACCESSIBILITY_SERVICE, getSequenceNumberLocked(),
0 /* unused */, mCurClient.client, accessibilityConnectionId));
mCurClient.client.onUnbindAccessibilityService(getSequenceNumberLocked(),
accessibilityConnectionId);
}
// We only have sessions when we bound to an input method. Remove this session
// from all clients.
@@ -5837,9 +5726,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
}
if (mCurClient != null && mCurClient.client != null) {
mInFullscreenMode = fullscreen;
executeOrSendMessage(mCurClient.client, mHandler.obtainMessage(
MSG_REPORT_FULLSCREEN_MODE, fullscreen ? 1 : 0, 0 /* unused */,
mCurClient));
mCurClient.client.reportFullscreenMode(fullscreen);
}
}
}