From 793e1bc92a1874c161d8bddca7d3969abee1c580 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Fri, 1 May 2020 18:20:01 -0700 Subject: AF: Remove the timeout for receiving InlineSuggestionsRequest from IME There are a few things the timeout was protecting against: * IME taking too long to come up or failing to come up entirely - in this case, the user cannot enter text anyway, so the incremental degradation in UX from autofill not showing either is relatively minimal. * IME taking too long to send back the InlineSuggestionsRequest - this would happen if the IME implementation is suboptimal. There's a valid case for protecting against this, however there are many other ways the IME can prevent Autofill from working correctly (e.g. just not showing the chips) that we do not have a fallback for - so introducing a fallback for just one of these cases doesn't seem worth the added complexity. Note: There doesn't seem to be any cleanup the timeout was doing that needs to be handled. Test: manual: * switching focused view multiple times, also with a Thread.sleep(7000) in the keyboard to make sure a delayed response works fine, also tested with multiple partitions. * smart reply, including clicking through from a notification * interrupting pending request with manual request Test: atest InlineAugmentedLoginActivityTest InlineLoginActivityTest \ AugmentedLoginActivityTest LoginActivityTest Fixes: 154334419 Change-Id: Id934fe3be991b5d70e9e8cb3e37b0fd869d783b7 --- .../autofill/AutofillInlineSessionController.java | 4 +-- .../AutofillInlineSuggestionsRequestSession.java | 32 +++++----------------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java b/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java index 3282870fe281..3dd2433ac2bd 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java +++ b/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java @@ -67,8 +67,8 @@ final class AutofillInlineSessionController { * Requests the IME to create an {@link InlineSuggestionsRequest} for {@code autofillId}. * * @param autofillId the Id of the field for which the request is for. - * @param requestConsumer the callback which will be invoked when IME responded or if it times - * out waiting for IME response. + * @param requestConsumer the callback to be invoked when the IME responds. Note that this is + * never invoked if the IME doesn't respond. */ @GuardedBy("mLock") void onCreateInlineSuggestionsRequestLocked(@NonNull AutofillId autofillId, diff --git a/services/autofill/java/com/android/server/autofill/AutofillInlineSuggestionsRequestSession.java b/services/autofill/java/com/android/server/autofill/AutofillInlineSuggestionsRequestSession.java index 0bf89936f2ce..2156ae5904f0 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillInlineSuggestionsRequestSession.java +++ b/services/autofill/java/com/android/server/autofill/AutofillInlineSuggestionsRequestSession.java @@ -55,16 +55,6 @@ final class AutofillInlineSuggestionsRequestSession { private static final String TAG = AutofillInlineSuggestionsRequestSession.class.getSimpleName(); - // This timeout controls how long Autofill should wait for the IME to respond either - // unsupported or an {@link InlineSuggestionsRequest}. The timeout is needed to take into - // account the latency between the two events after a field is focused, 1) an Autofill - // request is triggered on framework; 2) the InputMethodService#onStartInput() event is - // triggered on the IME side. When 1) happens, Autofill may call the IME to return an {@link - // InlineSuggestionsRequest}, but the IME will only return it after 2) happens (or return - // immediately if the IME doesn't support inline suggestions). Also there is IPC latency - // between the framework and the IME but that should be small compare to that. - private static final int CREATE_INLINE_SUGGESTIONS_REQUEST_TIMEOUT_MS = 1000; - @NonNull private final InputMethodManagerInternal mInputMethodManagerInternal; private final int mUserId; @@ -92,9 +82,6 @@ final class AutofillInlineSuggestionsRequestSession { @GuardedBy("mLock") @Nullable private IInlineSuggestionsResponseCallback mResponseCallback; - @GuardedBy("mLock") - @Nullable - private Runnable mTimeoutCallback; @GuardedBy("mLock") @Nullable @@ -174,12 +161,17 @@ final class AutofillInlineSuggestionsRequestSession { } /** - * This method must be called when the session is destroyed, to avoid further callbacks from/to - * the IME. + * Prevents further interaction with the IME. Must be called before starting a new request + * session to avoid unwanted behavior from two overlapping requests. */ @GuardedBy("mLock") void destroySessionLocked() { mDestroyed = true; + + if (!mImeRequestReceived) { + Slog.w(TAG, + "Never received an InlineSuggestionsRequest from the IME for " + mAutofillId); + } } /** @@ -196,11 +188,6 @@ final class AutofillInlineSuggestionsRequestSession { mInputMethodManagerInternal.onCreateInlineSuggestionsRequest(mUserId, new InlineSuggestionsRequestInfo(mComponentName, mAutofillId, mUiExtras), new InlineSuggestionsRequestCallbackImpl(this)); - mTimeoutCallback = () -> { - Slog.w(TAG, "Timed out waiting for IME callback InlineSuggestionsRequest."); - handleOnReceiveImeRequest(null, null); - }; - mHandler.postDelayed(mTimeoutCallback, CREATE_INLINE_SUGGESTIONS_REQUEST_TIMEOUT_MS); } /** @@ -264,11 +251,6 @@ final class AutofillInlineSuggestionsRequestSession { } mImeRequestReceived = true; - if (mTimeoutCallback != null) { - if (sVerbose) Slog.v(TAG, "removing timeout callback"); - mHandler.removeCallbacks(mTimeoutCallback); - mTimeoutCallback = null; - } if (request != null && callback != null) { mImeRequest = request; mResponseCallback = callback; -- cgit v1.2.3-59-g8ed1b