From 08e91345463c824a0afb721bbee4ad47926f5459 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Fri, 14 Feb 2020 12:45:22 -0800 Subject: [PATCH] Logically revert previous attempts on Bug 139806621 To explore different approaches to tackle Bug 139806621, this CL first reverts our previous attempts [1][2] that turned out to be not successful as we hoped [3]. Our high level strategy is 1) temporarily going back to our previous well-known algorithm first then 2) trying a different approach. In this way, hopefully we can maximize the chance of fixing Bug 139806621 with minimizing the total complexity. Also any unconfirmed regressions that we might have introduced in our previous attempt other than Bug 144103599 and Bug 144174015 are expected to disappear after this revert. [1]: I6aa4a664cfd0c86f75cee2457715317194bbe5e2 e0172102b9c9a9640209e3223bb6c60fe1d5cabb [2]: I98cb044d8cbfb80480312a3923f168aefa9b7e7d 322ee9bf9ec74460d98c13e91ce4749141bf79e7 [3]: Still seeing that the UI thread is being blocked at the later point. Also triaging cost of regressions such as Bug 144103599, Bug 144174015 and Bug 147331480 Bug: 139806621 Test: atest CtsInputMethodTestCases CtsInputMethodServiceHostTestCases Change-Id: Id73f0d5efb68bb4c5c8ef3dfb974c7a4f9457581 --- .../view/inputmethod/InputMethodManager.java | 113 ++++++------------ 1 file changed, 35 insertions(+), 78 deletions(-) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index dbab81b129a98..1e6abd9753387 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -93,12 +93,7 @@ import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -414,16 +409,6 @@ public final class InputMethodManager { int mCursorCandStart; int mCursorCandEnd; - /** - * Initial startInput with {@link StartInputReason.WINDOW_FOCUS_GAIN} is executed - * in a background thread. Later, if there is an actual startInput it will wait on - * main thread till the background thread completes. - */ - private Future mWindowFocusGainFuture; - - private ExecutorService mStartInputWorker = Executors.newSingleThreadExecutor( - new ImeThreadFactory("StartInputWorker")); - /** * The instance that has previously been sent to the input method. */ @@ -612,41 +597,36 @@ public final class InputMethodManager { final boolean forceNewFocus1 = forceNewFocus; final int startInputFlags = getStartInputFlags(focusedView, 0); - if (mWindowFocusGainFuture != null) { - mWindowFocusGainFuture.cancel(false /* mayInterruptIfRunning */); + final ImeFocusController controller = getFocusController(); + if (controller == null) { + return; } - mWindowFocusGainFuture = mStartInputWorker.submit(() -> { - final ImeFocusController controller = getFocusController(); - if (controller == null) { + 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 (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; - } - } + } - 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 { - if (DEBUG) Log.v(TAG, "Reporting focus gain, without startInput"); - mService.startInputOrWindowGainedFocus( - StartInputReason.WINDOW_FOCUS_GAIN_REPORT_ONLY, mClient, - focusedView.getWindowToken(), startInputFlags, softInputMode, - windowFlags, - null, null, 0 /* missingMethodFlags */, - mCurRootView.mContext.getApplicationInfo().targetSdkVersion); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + 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 { + if (DEBUG) Log.v(TAG, "Reporting focus gain, without startInput"); + mService.startInputOrWindowGainedFocus( + StartInputReason.WINDOW_FOCUS_GAIN_REPORT_ONLY, mClient, + focusedView.getWindowToken(), startInputFlags, softInputMode, + windowFlags, + null, null, 0 /* missingMethodFlags */, + mCurRootView.mContext.getApplicationInfo().targetSdkVersion); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - }); + } } /** @@ -664,10 +644,6 @@ public final class InputMethodManager { */ @Override public void setCurrentRootView(ViewRootImpl rootView) { - if (mWindowFocusGainFuture != null) { - mWindowFocusGainFuture.cancel(false /* mayInterruptIfRunning */); - mWindowFocusGainFuture = null; - } synchronized (mH) { if (mCurRootView != null) { // Reset the last served view and restart window focus state of the root view. @@ -845,19 +821,16 @@ public final class InputMethodManager { } catch (RemoteException e) { } } - } - // Check focus again in case that "onWindowFocus" is called before - // handling this message. - final View servedView; - synchronized (mH) { - servedView = getServedViewLocked(); - } - if (servedView != null && canStartInput(servedView)) { - if (mCurRootView != null && mCurRootView.getImeFocusController() - .checkFocus(mRestartOnNextWindowFocus, false)) { - final int reason = active ? StartInputReason.ACTIVATED_BY_IMMS - : StartInputReason.DEACTIVATED_BY_IMMS; - mDelegate.startInput(reason, null, 0, 0, 0); + // Check focus again in case that "onWindowFocus" is called before + // handling this message. + final View servedView = getServedViewLocked(); + if (servedView != null && canStartInput(servedView)) { + if (mCurRootView != null && mCurRootView.getImeFocusController() + .checkFocus(mRestartOnNextWindowFocus, false)) { + final int reason = active ? StartInputReason.ACTIVATED_BY_IMMS + : StartInputReason.DEACTIVATED_BY_IMMS; + mDelegate.startInput(reason, null, 0, 0, 0); + } } } return; @@ -1430,10 +1403,6 @@ public final class InputMethodManager { */ void clearBindingLocked() { if (DEBUG) Log.v(TAG, "Clearing binding!"); - if (mWindowFocusGainFuture != null) { - mWindowFocusGainFuture.cancel(false /* mayInterruptIfRunning */); - mWindowFocusGainFuture = null; - } clearConnectionLocked(); setInputChannelLocked(null); mBindSequence = -1; @@ -1826,18 +1795,6 @@ public final class InputMethodManager { boolean startInputInner(@StartInputReason int startInputReason, @Nullable IBinder windowGainingFocus, @StartInputFlags int startInputFlags, @SoftInputModeFlags int softInputMode, int windowFlags) { - if (startInputReason != StartInputReason.WINDOW_FOCUS_GAIN - && mWindowFocusGainFuture != null) { - try { - mWindowFocusGainFuture.get(); - } catch (ExecutionException | InterruptedException e) { - // do nothing - } catch (CancellationException e) { - // window no longer has focus. - return true; - } - } - final View view; synchronized (mH) { view = getServedViewLocked();