From 69471c8f155327fe55b4bb7f9db9ac2be454ab17 Mon Sep 17 00:00:00 2001 From: Feng Cao Date: Thu, 12 Mar 2020 00:25:22 -0700 Subject: [PATCH] Do not block autofill on waiting for the IME response Test: manual Test: atest CtsAutoFillServiceTestCases Test: atest CtsInputMethodTestCases Test: atest CtsInputMethodServiceHostTestCases Bug: 150757314 Change-Id: I2e029616267bef73d96d6ae3bbc0398939206157 --- .../autofill/InlineSuggestionSession.java | 79 ++++++++----- .../com/android/server/autofill/Session.java | 105 ++++++++++++++---- 2 files changed, 133 insertions(+), 51 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java b/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java index 7fe086df07294..3376f2bd74f58 100644 --- a/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java +++ b/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java @@ -23,6 +23,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.ComponentName; import android.os.Bundle; +import android.os.Handler; import android.os.RemoteException; import android.util.Log; import android.util.Slog; @@ -38,11 +39,8 @@ import com.android.server.inputmethod.InputMethodManagerInternal; import java.util.Collections; import java.util.Optional; -import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; /** * Maintains an autofill inline suggestion session that communicates with the IME. @@ -69,7 +67,7 @@ import java.util.concurrent.TimeoutException; final class InlineSuggestionSession { private static final String TAG = "AfInlineSuggestionSession"; - private static final int INLINE_REQUEST_TIMEOUT_MS = 1000; + private static final int INLINE_REQUEST_TIMEOUT_MS = 200; @NonNull private final InputMethodManagerInternal mInputMethodManagerInternal; @@ -80,6 +78,8 @@ final class InlineSuggestionSession { private final Object mLock; @NonNull private final ImeStatusListener mImeStatusListener; + @NonNull + private final Handler mHandler; /** * To avoid the race condition, one should not access {@code mPendingImeResponse} without @@ -105,10 +105,11 @@ final class InlineSuggestionSession { private boolean mImeInputViewStarted = false; InlineSuggestionSession(InputMethodManagerInternal inputMethodManagerInternal, - int userId, ComponentName componentName) { + int userId, ComponentName componentName, Handler handler) { mInputMethodManagerInternal = inputMethodManagerInternal; mUserId = userId; mComponentName = componentName; + mHandler = handler; mLock = new Object(); mImeStatusListener = new ImeStatusListener() { @Override @@ -137,7 +138,8 @@ final class InlineSuggestionSession { }; } - public void onCreateInlineSuggestionsRequest(@NonNull AutofillId autofillId) { + public void onCreateInlineSuggestionsRequest(@NonNull AutofillId autofillId, + @NonNull Consumer requestConsumer) { if (sDebug) Log.d(TAG, "onCreateInlineSuggestionsRequest called for " + autofillId); synchronized (mLock) { @@ -154,26 +156,16 @@ final class InlineSuggestionSession { mUserId, new InlineSuggestionsRequestInfo(mComponentName, autofillId, new Bundle()), new InlineSuggestionsRequestCallbackImpl(mPendingImeResponse, - mImeStatusListener)); + mImeStatusListener, requestConsumer, mHandler, mLock)); } } - public Optional waitAndGetInlineSuggestionsRequest() { + public Optional getInlineSuggestionsRequest() { final CompletableFuture pendingImeResponse = getPendingImeResponse(); - if (pendingImeResponse == null) { + if (pendingImeResponse == null || !pendingImeResponse.isDone()) { return Optional.empty(); } - try { - return Optional.ofNullable(pendingImeResponse.get(INLINE_REQUEST_TIMEOUT_MS, - TimeUnit.MILLISECONDS)).map(ImeResponse::getRequest); - } catch (TimeoutException e) { - Log.w(TAG, "Exception getting inline suggestions request in time: " + e); - } catch (CancellationException e) { - Log.w(TAG, "Inline suggestions request cancelled"); - } catch (InterruptedException | ExecutionException e) { - throw new RuntimeException(e); - } - return Optional.empty(); + return Optional.ofNullable(pendingImeResponse.getNow(null)).map(ImeResponse::getRequest); } public boolean hideInlineSuggestionsUi(@NonNull AutofillId autofillId) { @@ -200,8 +192,7 @@ final class InlineSuggestionSession { if (sDebug) Log.d(TAG, "onInlineSuggestionsResponseLocked without IMS request"); return false; } - // There is no need to wait on the CompletableFuture since it should have been completed - // when {@link #waitAndGetInlineSuggestionsRequest()} was called. + // There is no need to wait on the CompletableFuture since it should have been completed. ImeResponse imeResponse = completedImsResponse.getNow(null); if (imeResponse == null) { if (sDebug) Log.d(TAG, "onInlineSuggestionsResponseLocked with pending IMS response"); @@ -249,20 +240,50 @@ final class InlineSuggestionSession { private static final class InlineSuggestionsRequestCallbackImpl extends IInlineSuggestionsRequestCallback.Stub { + private final Object mLock; + @GuardedBy("mLock") private final CompletableFuture mResponse; + @GuardedBy("mLock") + private final Consumer mRequestConsumer; private final ImeStatusListener mImeStatusListener; + private final Handler mHandler; + private final Runnable mTimeoutCallback; private InlineSuggestionsRequestCallbackImpl(CompletableFuture response, - ImeStatusListener imeStatusListener) { + ImeStatusListener imeStatusListener, + Consumer requestConsumer, + Handler handler, Object lock) { mResponse = response; mImeStatusListener = imeStatusListener; + mRequestConsumer = requestConsumer; + mLock = lock; + + mHandler = handler; + mTimeoutCallback = () -> { + Log.w(TAG, "Timed out waiting for IME callback InlineSuggestionsRequest."); + synchronized (mLock) { + completeIfNotLocked(null); + } + }; + mHandler.postDelayed(mTimeoutCallback, INLINE_REQUEST_TIMEOUT_MS); + } + + private void completeIfNotLocked(@Nullable ImeResponse response) { + if (mResponse.isDone()) { + return; + } + mResponse.complete(response); + mRequestConsumer.accept(response == null ? null : response.mRequest); + mHandler.removeCallbacks(mTimeoutCallback); } @BinderThread @Override public void onInlineSuggestionsUnsupported() throws RemoteException { if (sDebug) Log.d(TAG, "onInlineSuggestionsUnsupported() called."); - mResponse.complete(null); + synchronized (mLock) { + completeIfNotLocked(null); + } } @BinderThread @@ -281,9 +302,13 @@ final class InlineSuggestionSession { mImeStatusListener.onInputMethodFinishInputView(imeFieldId); } if (request != null && callback != null) { - mResponse.complete(new ImeResponse(request, callback)); + synchronized (mLock) { + completeIfNotLocked(new ImeResponse(request, callback)); + } } else { - mResponse.complete(null); + synchronized (mLock) { + completeIfNotLocked(null); + } } } diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index 538082d2f67eb..2fe9e0133d1aa 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -113,7 +113,9 @@ import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; /** * A session for a given activity. @@ -306,7 +308,47 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState /** * Receiver of assist data from the app's {@link Activity}. */ - private final IAssistDataReceiver mAssistReceiver = new IAssistDataReceiver.Stub() { + private final AssistDataReceiverImpl mAssistReceiver = new AssistDataReceiverImpl(); + + private final class AssistDataReceiverImpl extends IAssistDataReceiver.Stub { + + @GuardedBy("mLock") + private InlineSuggestionsRequest mPendingInlineSuggestionsRequest; + @GuardedBy("mLock") + private FillRequest mPendingFillRequest; + @GuardedBy("mLock") + private CountDownLatch mCountDownLatch; + + @Nullable Consumer newAutofillRequestLocked( + boolean isInlineRequest) { + mCountDownLatch = new CountDownLatch(isInlineRequest ? 2 : 1); + mPendingFillRequest = null; + mPendingInlineSuggestionsRequest = null; + return isInlineRequest ? (inlineSuggestionsRequest) -> { + synchronized (mLock) { + mPendingInlineSuggestionsRequest = inlineSuggestionsRequest; + mCountDownLatch.countDown(); + maybeRequestFillLocked(); + } + } : null; + } + + void maybeRequestFillLocked() { + if (mCountDownLatch == null || mCountDownLatch.getCount() > 0 + || mPendingFillRequest == null) { + return; + } + if (mPendingInlineSuggestionsRequest != null) { + mPendingFillRequest = new FillRequest(mPendingFillRequest.getId(), + mPendingFillRequest.getFillContexts(), mPendingFillRequest.getClientState(), + mPendingFillRequest.getFlags(), mPendingInlineSuggestionsRequest); + } + mRemoteFillService.onFillRequest(mPendingFillRequest); + mPendingInlineSuggestionsRequest = null; + mPendingFillRequest = null; + mCountDownLatch = null; + } + @Override public void onHandleAssistData(Bundle resultData) throws RemoteException { if (mRemoteFillService == null) { @@ -401,17 +443,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState final ArrayList contexts = mergePreviousSessionLocked(/* forSave= */ false); - - final Optional inlineSuggestionsRequest = - mInlineSuggestionSession.waitAndGetInlineSuggestionsRequest(); request = new FillRequest(requestId, contexts, mClientState, flags, - inlineSuggestionsRequest.orElse(null)); + /*inlineSuggestionsRequest=*/null); + + mPendingFillRequest = request; + mCountDownLatch.countDown(); + maybeRequestFillLocked(); } if (mActivityToken != null) { mService.sendActivityAssistDataToContentCapture(mActivityToken, resultData); } - mRemoteFillService.onFillRequest(request); } @Override @@ -604,9 +646,15 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState private void maybeRequestInlineSuggestionsRequestThenFillLocked(@NonNull ViewState viewState, int newState, int flags) { if (isInlineSuggestionsEnabledLocked()) { - mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId); + Consumer inlineSuggestionsRequestConsumer = + mAssistReceiver.newAutofillRequestLocked(/*isInlineRequest=*/ true); + if (inlineSuggestionsRequestConsumer != null) { + mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId, + inlineSuggestionsRequestConsumer); + } + } else { + mAssistReceiver.newAutofillRequestLocked(/*isInlineRequest=*/ false); } - requestNewFillResponseLocked(viewState, newState, flags); } @@ -707,7 +755,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState setClientLocked(client); mInlineSuggestionSession = new InlineSuggestionSession(inputMethodManagerInternal, userId, - componentName); + componentName, handler); mMetricsLogger.write(newLogMaker(MetricsEvent.AUTOFILL_SESSION_STARTED) .addTaggedData(MetricsEvent.FIELD_AUTOFILL_FLAGS, flags)); @@ -2662,7 +2710,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState private boolean requestShowInlineSuggestionsLocked(@NonNull FillResponse response, @Nullable String filterText) { final Optional inlineSuggestionsRequest = - mInlineSuggestionSession.waitAndGetInlineSuggestionsRequest(); + mInlineSuggestionSession.getInlineSuggestionsRequest(); if (!inlineSuggestionsRequest.isPresent()) { Log.w(TAG, "InlineSuggestionsRequest unavailable"); return false; @@ -2889,6 +2937,9 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState /** * Tries to trigger Augmented Autofill when the standard service could not fulfill a request. * + *

The request may not have been sent when this method returns as it may be waiting for + * the inline suggestion request asynchronously. + * * @return callback to destroy the autofill UI, or {@code null} if not supported. */ // TODO(b/123099468): might need to call it in other places, like when the service returns a @@ -2966,6 +3017,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState final AutofillId focusedId = AutofillId.withoutSession(mCurrentViewId); + final Consumer requestAugmentedAutofill = + (inlineSuggestionsRequest) -> { + remoteService.onRequestAutofillLocked(id, mClient, taskId, mComponentName, + focusedId, + currentValue, inlineSuggestionsRequest, + /*inlineSuggestionsCallback=*/ + response -> mInlineSuggestionSession.onInlineSuggestionsResponse( + mCurrentViewId, response), + /*onErrorCallback=*/ () -> { + synchronized (mLock) { + cancelAugmentedAutofillLocked(); + } + }, mService.getRemoteInlineSuggestionRenderServiceLocked()); + }; + // There are 3 cases when augmented autofill should ask IME for a new request: // 1. standard autofill provider is None // 2. standard autofill provider doesn't support inline (and returns null response) @@ -2973,21 +3039,12 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState // doesn't want autofill if (mForAugmentedAutofillOnly || !isInlineSuggestionsEnabledLocked()) { if (sDebug) Slog.d(TAG, "Create inline request for augmented autofill"); - mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId); + mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId, + /*requestConsumer=*/ requestAugmentedAutofill); + } else { + requestAugmentedAutofill.accept( + mInlineSuggestionSession.getInlineSuggestionsRequest().orElse(null)); } - - Optional inlineSuggestionsRequest = - mInlineSuggestionSession.waitAndGetInlineSuggestionsRequest(); - remoteService.onRequestAutofillLocked(id, mClient, taskId, mComponentName, focusedId, - currentValue, inlineSuggestionsRequest.orElse(null), - response -> mInlineSuggestionSession.onInlineSuggestionsResponse( - mCurrentViewId, response), - () -> { - synchronized (mLock) { - cancelAugmentedAutofillLocked(); - } - }, mService.getRemoteInlineSuggestionRenderServiceLocked()); - if (mAugmentedAutofillDestroyer == null) { mAugmentedAutofillDestroyer = () -> remoteService.onDestroyAutofillWindowsRequest(); }