From 3f733cc7efa6342a849960fdcf1a4cf28d36bae5 Mon Sep 17 00:00:00 2001 From: lumark Date: Mon, 20 Jan 2020 12:14:04 +0800 Subject: [PATCH] Fix some thread handling issues in InputMethodManager - Make sure onCheckIsTextEditor should be called on the UI thread for preventing exception in some cases that will break start input background thread. - Refine IMM#checkFocus and DelegateImpl#startInput to be out of lock object to prevent start input background thread may blocked by other methods which already inside the lock. - Cache InputMethodManagerDelegate object for ImeFocusController#getDelegate. Fix: 147447217 Bug: 141738570 Test: manual as test steps: 1) Launch Keeeps app, type some web-URL link. 2) Long press on any URL link > From the toolbar select "open / browser" and observe. 3) Expect the soft-keyboard should be hidden after entering the browser page. Test: atest CtsInputMethodTestCases Change-Id: I098df41f935438bf1889fb030c7a3e7a0645a168 --- .../java/android/view/ImeFocusController.java | 7 +- .../view/inputmethod/InputMethodManager.java | 70 +++++++++++++------ 2 files changed, 54 insertions(+), 23 deletions(-) diff --git a/core/java/android/view/ImeFocusController.java b/core/java/android/view/ImeFocusController.java index 5c494c17669a5..271566acb74e6 100644 --- a/core/java/android/view/ImeFocusController.java +++ b/core/java/android/view/ImeFocusController.java @@ -38,6 +38,7 @@ public final class ImeFocusController { private boolean mHasImeFocus = false; private View mServedView; private View mNextServedView; + private InputMethodManagerDelegate mDelegate; @UiThread ImeFocusController(@NonNull ViewRootImpl viewRootImpl) { @@ -45,7 +46,11 @@ public final class ImeFocusController { } private InputMethodManagerDelegate getImmDelegate() { - return mViewRootImpl.mContext.getSystemService(InputMethodManager.class).getDelegate(); + if (mDelegate == null) { + mDelegate = mViewRootImpl.mContext.getSystemService( + InputMethodManager.class).getDelegate(); + } + return mDelegate; } @UiThread diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index 904e736d214e4..298eb70cc4506 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -59,12 +59,12 @@ import android.util.PrintWriterPrinter; import android.util.Printer; import android.util.SparseArray; import android.view.Display; +import android.view.ImeFocusController; import android.view.ImeInsetsSourceConsumer; import android.view.InputChannel; import android.view.InputEvent; import android.view.InputEventSender; import android.view.KeyEvent; -import android.view.ImeFocusController; import android.view.View; import android.view.ViewRootImpl; import android.view.WindowManager.LayoutParams.SoftInputModeFlags; @@ -562,14 +562,23 @@ public final class InputMethodManager { public boolean startInput(@StartInputReason int startInputReason, View focusedView, @StartInputFlags int startInputFlags, @SoftInputModeFlags int softInputMode, int windowFlags) { + final View servedView; synchronized (mH) { mCurrentTextBoxAttribute = null; mCompletions = null; mServedConnecting = true; - if (getServedViewLocked() != null && !getServedViewLocked().onCheckIsTextEditor()) { - // servedView has changed and it's not editable. - maybeCallServedViewChangedLocked(null); - } + servedView = getServedViewLocked(); + } + if (servedView != null && servedView.getHandler() != null) { + // Make sure View checks should be on the UI thread. + servedView.getHandler().post(() -> { + if (!servedView.onCheckIsTextEditor()) { + // servedView has changed and it's not editable. + synchronized (mH) { + maybeCallServedViewChangedLocked(null); + } + } + }); } return startInputInner(startInputReason, focusedView != null ? focusedView.getWindowToken() : null, startInputFlags, @@ -607,21 +616,22 @@ public final class InputMethodManager { mWindowFocusGainFuture.cancel(false /* mayInterruptIfRunning */); } mWindowFocusGainFuture = mStartInputWorker.submit(() -> { - synchronized (mH) { - if (mCurRootView == null) { + final ImeFocusController controller = getFocusController(); + if (controller == null) { + return; + } + if (controller.checkFocus(forceNewFocus1, false)) { + // We need to restart input on the current focus view. This + // should be done in conjunction with telling the system service + // about the window gaining focus, to help make the transition + // smooth. + if (startInput(StartInputReason.WINDOW_FOCUS_GAIN, + focusedView, startInputFlags, softInputMode, windowFlags)) { return; } - if (mCurRootView.getImeFocusController().checkFocus(forceNewFocus1, false)) { - // We need to restart input on the current focus view. This - // should be done in conjunction with telling the system service - // about the window gaining focus, to help make the transition - // smooth. - if (startInput(StartInputReason.WINDOW_FOCUS_GAIN, - focusedView, startInputFlags, softInputMode, windowFlags)) { - return; - } - } + } + synchronized (mH) { // For some reason we didn't do a startInput + windowFocusGain, so // we'll just do a window focus gain and call it a day. try { @@ -716,6 +726,15 @@ public final class InputMethodManager { } } + private ImeFocusController getFocusController() { + synchronized (mH) { + if (mCurRootView != null) { + return mCurRootView.getImeFocusController(); + } + return null; + } + } + /** * Returns {@code true} when the given view has been served by Input Method. */ @@ -1793,6 +1812,12 @@ public final class InputMethodManager { startInputInner(StartInputReason.APP_CALLED_RESTART_INPUT_API, null, 0, 0, 0); } + /** + * Called when {@link DelegateImpl#startInput}, {@link #restartInput(View)}, + * {@link #MSG_BIND} or {@link #MSG_UNBIND}. + * Note that this method should *NOT* be called inside of {@code mH} lock to prevent start input + * background thread may blocked by other methods which already inside {@code mH} lock. + */ boolean startInputInner(@StartInputReason int startInputReason, @Nullable IBinder windowGainingFocus, @StartInputFlags int startInputFlags, @SoftInputModeFlags int softInputMode, int windowFlags) { @@ -1976,15 +2001,16 @@ public final class InputMethodManager { } /** + * Check the next served view from {@link ImeFocusController} if needs to start input. + * Note that this method should *NOT* be called inside of {@code mH} lock to prevent start input + * background thread may blocked by other methods which already inside {@code mH} lock. * @hide */ @UnsupportedAppUsage public void checkFocus() { - synchronized (mH) { - if (mCurRootView != null) { - mCurRootView.getImeFocusController().checkFocus(false /* forceNewFocus */, - true /* startInput */); - } + final ImeFocusController controller = getFocusController(); + if (controller != null) { + controller.checkFocus(false /* forceNewFocus */, true /* startInput */); } }