From 67b9a09cb69bfb3e7f01dfd9c4283332786850eb Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Mon, 24 Jan 2022 08:22:26 -0800 Subject: [PATCH] Introduce IInputMethodInvoker This CL introduces a wrapper class IInputMethodInvoker to consolidate RemoteException handlings out from InputMethodManagerService around IInputMethod. Basically there should be no observable behavior change, except that we logcat messages upon RemoteException when issuing IInputMethod IPCs are now shown only when InputMethodManagerService#DEBUG is configured to be true or the exception is not DeadObjectException. Hopefully we can still easily find that the IME was being killed from other messages in logcat. Bug: 215609403 Test: presubmit Change-Id: Id636d35f3b82a6376c9f370e2f65692f28e545eb --- .../inputmethod/IInputMethodInvoker.java | 224 ++++++++++++++++++ .../InputMethodBindingController.java | 6 +- .../InputMethodManagerService.java | 136 ++++------- 3 files changed, 273 insertions(+), 93 deletions(-) create mode 100644 services/core/java/com/android/server/inputmethod/IInputMethodInvoker.java diff --git a/services/core/java/com/android/server/inputmethod/IInputMethodInvoker.java b/services/core/java/com/android/server/inputmethod/IInputMethodInvoker.java new file mode 100644 index 0000000000000..c86ebd26d8713 --- /dev/null +++ b/services/core/java/com/android/server/inputmethod/IInputMethodInvoker.java @@ -0,0 +1,224 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.inputmethod; + +import android.annotation.AnyThread; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.os.Binder; +import android.os.DeadObjectException; +import android.os.IBinder; +import android.os.RemoteException; +import android.os.ResultReceiver; +import android.util.Slog; +import android.view.InputChannel; +import android.view.MotionEvent; +import android.view.inputmethod.EditorInfo; +import android.view.inputmethod.InputBinding; +import android.view.inputmethod.InputMethodSubtype; + +import com.android.internal.inputmethod.IInputMethodPrivilegedOperations; +import com.android.internal.view.IInlineSuggestionsRequestCallback; +import com.android.internal.view.IInputContext; +import com.android.internal.view.IInputMethod; +import com.android.internal.view.IInputMethodSession; +import com.android.internal.view.IInputSessionCallback; +import com.android.internal.view.InlineSuggestionsRequestInfo; + +import java.util.List; + +/** + * A wrapper class to invoke IPCs defined in {@link IInputMethod}. + */ +final class IInputMethodInvoker { + private static final String TAG = InputMethodManagerService.TAG; + private static final boolean DEBUG = InputMethodManagerService.DEBUG; + + @AnyThread + @Nullable + static IInputMethodInvoker create(@Nullable IInputMethod inputMethod) { + if (inputMethod == null) { + return null; + } + if (!Binder.isProxy(inputMethod)) { + // IInputMethodInvoker must be used only within the system_server and InputMethodService + // must not be running in the system_server. Therefore, "inputMethod" must be a Proxy. + throw new UnsupportedOperationException(inputMethod + " must have been a BinderProxy."); + } + return new IInputMethodInvoker(inputMethod); + } + + /** + * A simplified version of {@link android.os.Debug#getCaller()}. + * + * @return method name of the caller. + */ + @AnyThread + private static String getCallerMethodName() { + final StackTraceElement[] callStack = Thread.currentThread().getStackTrace(); + if (callStack.length <= 4) { + return ""; + } + return callStack[4].getMethodName(); + } + + @AnyThread + private static void logRemoteException(@NonNull RemoteException e) { + if (DEBUG || !(e instanceof DeadObjectException)) { + Slog.w(TAG, "IPC failed at IInputMethodInvoker#" + getCallerMethodName(), e); + } + } + + @AnyThread + static int getBinderIdentityHashCode(@Nullable IInputMethodInvoker obj) { + if (obj == null) { + return 0; + } + + return System.identityHashCode(obj.mTarget); + } + + @NonNull + private final IInputMethod mTarget; + + private IInputMethodInvoker(@NonNull IInputMethod target) { + mTarget = target; + } + + @AnyThread + @NonNull + IBinder asBinder() { + return mTarget.asBinder(); + } + + @AnyThread + void initializeInternal(IBinder token, IInputMethodPrivilegedOperations privOps, + int configChanges, boolean stylusHwSupported) { + try { + mTarget.initializeInternal(token, privOps, configChanges, stylusHwSupported); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void onCreateInlineSuggestionsRequest(InlineSuggestionsRequestInfo requestInfo, + IInlineSuggestionsRequestCallback cb) { + try { + mTarget.onCreateInlineSuggestionsRequest(requestInfo, cb); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void bindInput(InputBinding binding) { + try { + mTarget.bindInput(binding); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void unbindInput() { + try { + mTarget.unbindInput(); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void startInput(IBinder startInputToken, IInputContext inputContext, EditorInfo attribute, + boolean restarting) { + try { + mTarget.startInput(startInputToken, inputContext, attribute, restarting); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void createSession(InputChannel channel, IInputSessionCallback callback) { + try { + mTarget.createSession(channel, callback); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void setSessionEnabled(IInputMethodSession session, boolean enabled) { + try { + mTarget.setSessionEnabled(session, enabled); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + // TODO(b/192412909): Convert this back to void method + @AnyThread + boolean showSoftInput(IBinder showInputToken, int flags, ResultReceiver resultReceiver) { + try { + mTarget.showSoftInput(showInputToken, flags, resultReceiver); + } catch (RemoteException e) { + logRemoteException(e); + return false; + } + return true; + } + + // TODO(b/192412909): Convert this back to void method + @AnyThread + boolean hideSoftInput(IBinder hideInputToken, int flags, ResultReceiver resultReceiver) { + try { + mTarget.hideSoftInput(hideInputToken, flags, resultReceiver); + } catch (RemoteException e) { + logRemoteException(e); + return false; + } + return true; + } + + @AnyThread + void changeInputMethodSubtype(InputMethodSubtype subtype) { + try { + mTarget.changeInputMethodSubtype(subtype); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void canStartStylusHandwriting(int requestId) { + try { + mTarget.canStartStylusHandwriting(requestId); + } catch (RemoteException e) { + logRemoteException(e); + } + } + + @AnyThread + void startStylusHandwriting(InputChannel channel, List events) { + try { + mTarget.startStylusHandwriting(channel, events); + } catch (RemoteException e) { + logRemoteException(e); + } + } +} diff --git a/services/core/java/com/android/server/inputmethod/InputMethodBindingController.java b/services/core/java/com/android/server/inputmethod/InputMethodBindingController.java index 784d058c4a4b1..2230dcde08691 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodBindingController.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodBindingController.java @@ -75,7 +75,7 @@ final class InputMethodBindingController { @GuardedBy("ImfLock.class") @Nullable private String mCurId; @GuardedBy("ImfLock.class") @Nullable private String mSelectedMethodId; @GuardedBy("ImfLock.class") @Nullable private Intent mCurIntent; - @GuardedBy("ImfLock.class") @Nullable private IInputMethod mCurMethod; + @GuardedBy("ImfLock.class") @Nullable private IInputMethodInvoker mCurMethod; @GuardedBy("ImfLock.class") private int mCurMethodUid = Process.INVALID_UID; @GuardedBy("ImfLock.class") private IBinder mCurToken; @GuardedBy("ImfLock.class") private int mCurSeq; @@ -241,7 +241,7 @@ final class InputMethodBindingController { */ @GuardedBy("ImfLock.class") @Nullable - IInputMethod getCurMethod() { + IInputMethodInvoker getCurMethod() { return mCurMethod; } @@ -298,7 +298,7 @@ final class InputMethodBindingController { Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "IMMS.onServiceConnected"); synchronized (ImfLock.class) { if (mCurIntent != null && name.equals(mCurIntent.getComponent())) { - mCurMethod = IInputMethod.Stub.asInterface(service); + mCurMethod = IInputMethodInvoker.create(IInputMethod.Stub.asInterface(service)); updateCurrentMethodUid(); if (mCurToken == null) { Slog.w(TAG, "Service connected without a token!"); diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index 2a24489956a72..0d41a37caf0d4 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -167,7 +167,6 @@ import com.android.internal.util.DumpUtils; import com.android.internal.view.IInlineSuggestionsRequestCallback; import com.android.internal.view.IInlineSuggestionsResponseCallback; import com.android.internal.view.IInputContext; -import com.android.internal.view.IInputMethod; import com.android.internal.view.IInputMethodClient; import com.android.internal.view.IInputMethodManager; import com.android.internal.view.IInputMethodSession; @@ -329,7 +328,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub static class SessionState { final ClientState client; - final IInputMethod method; + final IInputMethodInvoker method; IInputMethodSession session; InputChannel channel; @@ -338,14 +337,14 @@ public class InputMethodManagerService extends IInputMethodManager.Stub public String toString() { return "SessionState{uid " + client.uid + " pid " + client.pid + " method " + Integer.toHexString( - System.identityHashCode(method)) + IInputMethodInvoker.getBinderIdentityHashCode(method)) + " session " + Integer.toHexString( System.identityHashCode(session)) + " channel " + channel + "}"; } - SessionState(ClientState _client, IInputMethod _method, + SessionState(ClientState _client, IInputMethodInvoker _method, IInputMethodSession _session, InputChannel _channel) { client = _client; method = _method; @@ -613,7 +612,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub */ @GuardedBy("ImfLock.class") @Nullable - private IInputMethod getCurMethodLocked() { + private IInputMethodInvoker getCurMethodLocked() { return mBindingController.getCurMethod(); } @@ -692,8 +691,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub /** * Internal state snapshot when - * {@link IInputMethod#startInput(IBinder, IInputContext, EditorInfo, boolean)} is about to be - * called. + * {@link com.android.internal.view.IInputMethod#startInput(IBinder, IInputContext, EditorInfo, + * boolean)} is about to be called. * *

Calling that IPC endpoint basically means that * {@link InputMethodService#doStartInput(InputConnection, EditorInfo, boolean)} will be called @@ -1951,18 +1950,14 @@ public class InputMethodManagerService extends IInputMethodManager.Stub InlineSuggestionsRequestInfo requestInfo, IInlineSuggestionsRequestCallback callback) { final InputMethodInfo imi = mMethodMap.get(getSelectedMethodIdLocked()); try { - IInputMethod curMethod = getCurMethodLocked(); + IInputMethodInvoker curMethod = getCurMethodLocked(); if (userId == mSettings.getCurrentUserId() && imi != null && imi.isInlineSuggestionsEnabled() && curMethod != null) { final IInlineSuggestionsRequestCallback callbackImpl = new InlineSuggestionsRequestCallbackDecorator(callback, imi.getPackageName(), mCurTokenDisplayId, getCurTokenLocked(), this); - try { - curMethod.onCreateInlineSuggestionsRequest(requestInfo, callbackImpl); - } catch (RemoteException e) { - Slog.w(TAG, "RemoteException calling onCreateInlineSuggestionsRequest()", e); - } + curMethod.onCreateInlineSuggestionsRequest(requestInfo, callbackImpl); } else { callback.onInlineSuggestionsUnsupported(); } @@ -2190,13 +2185,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub mCurFocusedWindow, 0, null, SoftInputShowHideReason.HIDE_REMOVE_CLIENT); if (mBoundToMethod) { mBoundToMethod = false; - IInputMethod curMethod = getCurMethodLocked(); + IInputMethodInvoker curMethod = getCurMethodLocked(); if (curMethod != null) { - try { - curMethod.unbindInput(); - } catch (RemoteException e) { - // There is nothing interesting about the method dying. - } + curMethod.unbindInput(); } } mCurClient = null; @@ -2230,13 +2221,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub + mCurClient.client.asBinder()); if (mBoundToMethod) { mBoundToMethod = false; - IInputMethod curMethod = getCurMethodLocked(); + IInputMethodInvoker curMethod = getCurMethodLocked(); if (curMethod != null) { - try { - curMethod.unbindInput(); - } catch (RemoteException e) { - // There is nothing interesting about the method dying. - } + curMethod.unbindInput(); } } @@ -2285,11 +2272,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub @NonNull InputBindResult attachNewInputLocked(@StartInputReason int startInputReason, boolean initial) { if (!mBoundToMethod) { - IInputMethod curMethod = getCurMethodLocked(); - try { - curMethod.bindInput(mCurClient.binding); - } catch (RemoteException e) { - } + getCurMethodLocked().bindInput(mCurClient.binding); mBoundToMethod = true; } @@ -2316,11 +2299,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } final SessionState session = mCurClient.curSession; - try { - setEnabledSessionLocked(session); - session.method.startInput(startInputToken, mCurInputContext, mCurAttribute, restarting); - } catch (RemoteException e) { - } + setEnabledSessionLocked(session); + session.method.startInput(startInputToken, mCurInputContext, mCurAttribute, restarting); if (mShowRequested) { if (DEBUG) Slog.v(TAG, "Attach new input asks to show input"); @@ -2514,18 +2494,14 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } @GuardedBy("ImfLock.class") - void initializeImeLocked(@NonNull IInputMethod inputMethod, @NonNull IBinder token, + void initializeImeLocked(@NonNull IInputMethodInvoker inputMethod, @NonNull IBinder token, @android.content.pm.ActivityInfo.Config int configChanges, boolean supportStylusHw) { if (DEBUG) { Slog.v(TAG, "Sending attach of token: " + token + " for display: " + mCurTokenDisplayId); } - try { - inputMethod.initializeInternal(token, - new InputMethodPrivilegedOperationsImpl(this, token), configChanges, - supportStylusHw); - } catch (RemoteException e) { - } + inputMethod.initializeInternal(token, new InputMethodPrivilegedOperationsImpl(this, token), + configChanges, supportStylusHw); } @AnyThread @@ -2535,7 +2511,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } @BinderThread - void onSessionCreated(IInputMethod method, IInputMethodSession session, InputChannel channel) { + void onSessionCreated(IInputMethodInvoker method, IInputMethodSession session, + InputChannel channel) { Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "IMMS.onSessionCreated"); try { synchronized (ImfLock.class) { @@ -2545,7 +2522,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub channel.dispose(); return; } - IInputMethod curMethod = getCurMethodLocked(); + IInputMethodInvoker curMethod = getCurMethodLocked(); if (curMethod != null && method != null && curMethod.asBinder() == method.asBinder()) { if (mCurClient != null) { @@ -2609,7 +2586,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub cs.sessionRequested = true; - final IInputMethod curMethod = getCurMethodLocked(); + final IInputMethodInvoker curMethod = getCurMethodLocked(); final IInputSessionCallback.Stub callback = new IInputSessionCallback.Stub() { @Override public void sessionCreated(IInputMethodSession session) { @@ -2624,7 +2601,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub try { curMethod.createSession(clientChannel, callback); - } catch (RemoteException e) { } finally { // Dispose the channel because the remote proxy will get its own copy when // unparceled. @@ -3012,14 +2988,10 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } if (newSubtype != oldSubtype) { setSelectedInputMethodAndSubtypeLocked(info, subtypeId, true); - IInputMethod curMethod = getCurMethodLocked(); + IInputMethodInvoker curMethod = getCurMethodLocked(); if (curMethod != null) { - try { - updateSystemUiLocked(mImeWindowVis, mBackDisposition); - curMethod.changeInputMethodSubtype(newSubtype); - } catch (RemoteException e) { - Slog.w(TAG, "Failed to call changeInputMethodSubtype"); - } + updateSystemUiLocked(mImeWindowVis, mBackDisposition); + curMethod.changeInputMethodSubtype(newSubtype); } } return; @@ -3096,13 +3068,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub return; } if (DEBUG) Slog.v(TAG, "Client requesting Stylus Handwriting to be started"); - final IInputMethod curMethod = getCurMethodLocked(); + final IInputMethodInvoker curMethod = getCurMethodLocked(); if (curMethod != null) { - try { - curMethod.canStartStylusHandwriting(++mHwRequestId); - } catch (RemoteException e) { - Slog.w(TAG, "RemoteException calling canStartStylusHandwriting(): ", e); - } + curMethod.canStartStylusHandwriting(++mHwRequestId); } } finally { Binder.restoreCallingIdentity(ident); @@ -3153,21 +3121,20 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } mBindingController.setCurrentMethodVisible(); - final IInputMethod curMethod = getCurMethodLocked(); + final IInputMethodInvoker curMethod = getCurMethodLocked(); if (curMethod != null) { // create a placeholder token for IMS so that IMS cannot inject windows into client app. Binder showInputToken = new Binder(); mShowRequestWindowMap.put(showInputToken, windowToken); final int showFlags = getImeShowFlagsLocked(); - try { - if (DEBUG) { - Slog.v(TAG, "Calling " + curMethod + ".showSoftInput(" + showInputToken - + ", " + showFlags + ", " + resultReceiver + ") for reason: " - + InputMethodDebug.softInputDisplayReasonToString(reason)); - } - curMethod.showSoftInput(showInputToken, showFlags, resultReceiver); + if (DEBUG) { + Slog.v(TAG, "Calling " + curMethod + ".showSoftInput(" + showInputToken + + ", " + showFlags + ", " + resultReceiver + ") for reason: " + + InputMethodDebug.softInputDisplayReasonToString(reason)); + } + // TODO(b/192412909): Check if we can always call onShowHideSoftInputRequested() or not. + if (curMethod.showSoftInput(showInputToken, showFlags, resultReceiver)) { onShowHideSoftInputRequested(true /* show */, windowToken, reason); - } catch (RemoteException e) { } mInputShown = true; return true; @@ -3236,7 +3203,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub // since Android Eclair. That's why we need to accept IMM#hideSoftInput() even when only // IMMS#InputShown indicates that the software keyboard is shown. // TODO: Clean up, IMMS#mInputShown, IMMS#mImeWindowVis and mShowRequested. - IInputMethod curMethod = getCurMethodLocked(); + IInputMethodInvoker curMethod = getCurMethodLocked(); final boolean shouldHideSoftInput = (curMethod != null) && (mInputShown || (mImeWindowVis & InputMethodService.IME_ACTIVE) != 0); boolean res; @@ -3252,10 +3219,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub + ", " + resultReceiver + ") for reason: " + InputMethodDebug.softInputDisplayReasonToString(reason)); } - try { - curMethod.hideSoftInput(hideInputToken, 0 /* flags */, resultReceiver); + // TODO(b/192412909): Check if we can always call onShowHideSoftInputRequested() or not. + if (curMethod.hideSoftInput(hideInputToken, 0 /* flags */, resultReceiver)) { onShowHideSoftInputRequested(false /* show */, windowToken, reason); - } catch (RemoteException e) { } res = true; } else { @@ -4160,7 +4126,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } } - /** Called right after {@link IInputMethod#showSoftInput}. */ + /** Called right after {@link com.android.internal.view.IInputMethod#showSoftInput}. */ @GuardedBy("ImfLock.class") private void onShowHideSoftInputRequested(boolean show, IBinder requestToken, @SoftInputShowHideReason int reason) { @@ -4215,19 +4181,13 @@ public class InputMethodManagerService extends IInputMethodManager.Stub void setEnabledSessionLocked(SessionState session) { if (mEnabledSession != session) { if (mEnabledSession != null && mEnabledSession.session != null) { - try { - if (DEBUG) Slog.v(TAG, "Disabling: " + mEnabledSession); - mEnabledSession.method.setSessionEnabled(mEnabledSession.session, false); - } catch (RemoteException e) { - } + if (DEBUG) Slog.v(TAG, "Disabling: " + mEnabledSession); + mEnabledSession.method.setSessionEnabled(mEnabledSession.session, false); } mEnabledSession = session; if (mEnabledSession != null && mEnabledSession.session != null) { - try { - if (DEBUG) Slog.v(TAG, "Enabling: " + mEnabledSession); - mEnabledSession.method.setSessionEnabled(mEnabledSession.session, true); - } catch (RemoteException e) { - } + if (DEBUG) Slog.v(TAG, "Enabling: " + mEnabledSession); + mEnabledSession.method.setSessionEnabled(mEnabledSession.session, true); } } } @@ -4394,12 +4354,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub return; } - try { - // TODO: replace null with actual Channel, MotionEvents - getCurMethodLocked().startStylusHandwriting(null, null); - } catch (RemoteException e) { - Slog.w(TAG, "RemoteException calling startStylusHandwriting(): ", e); - } + // TODO: replace null with actual Channel, MotionEvents + getCurMethodLocked().startStylusHandwriting(null, null); } } @@ -5132,7 +5088,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub @BinderThread private void dumpAsStringNoCheck(FileDescriptor fd, PrintWriter pw, String[] args, boolean isCritical) { - IInputMethod method; + IInputMethodInvoker method; ClientState client; ClientState focusedWindowClient;