Revert "Do not block autofill on waiting for the IME response"
This reverts commit 69471c8f155327fe55b4bb7f9db9ac2be454ab17.
Reason for revert:
Caused a deadlock between AutofillManagerService#mLock and
InlineSuggestionsRequestCallbackImpl#mLock.
Bug: 150757314
Fix: 151580124
Test: presubmit
Change-Id: Ic021facea34cbc34ad5e113790e739ab7f472c1f
(cherry picked from commit 949ec8ad6baf5516cbfd27b7e24d0e563c1f42a9)
diff --git a/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java b/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java
index 3376f2b..7fe086d 100644
--- a/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java
+++ b/services/autofill/java/com/android/server/autofill/InlineSuggestionSession.java
@@ -23,7 +23,6 @@
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;
@@ -39,8 +38,11 @@
import java.util.Collections;
import java.util.Optional;
+import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
-import java.util.function.Consumer;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
/**
* Maintains an autofill inline suggestion session that communicates with the IME.
@@ -67,7 +69,7 @@
final class InlineSuggestionSession {
private static final String TAG = "AfInlineSuggestionSession";
- private static final int INLINE_REQUEST_TIMEOUT_MS = 200;
+ private static final int INLINE_REQUEST_TIMEOUT_MS = 1000;
@NonNull
private final InputMethodManagerInternal mInputMethodManagerInternal;
@@ -78,8 +80,6 @@
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,11 +105,10 @@
private boolean mImeInputViewStarted = false;
InlineSuggestionSession(InputMethodManagerInternal inputMethodManagerInternal,
- int userId, ComponentName componentName, Handler handler) {
+ int userId, ComponentName componentName) {
mInputMethodManagerInternal = inputMethodManagerInternal;
mUserId = userId;
mComponentName = componentName;
- mHandler = handler;
mLock = new Object();
mImeStatusListener = new ImeStatusListener() {
@Override
@@ -138,8 +137,7 @@
};
}
- public void onCreateInlineSuggestionsRequest(@NonNull AutofillId autofillId,
- @NonNull Consumer<InlineSuggestionsRequest> requestConsumer) {
+ public void onCreateInlineSuggestionsRequest(@NonNull AutofillId autofillId) {
if (sDebug) Log.d(TAG, "onCreateInlineSuggestionsRequest called for " + autofillId);
synchronized (mLock) {
@@ -156,16 +154,26 @@
mUserId,
new InlineSuggestionsRequestInfo(mComponentName, autofillId, new Bundle()),
new InlineSuggestionsRequestCallbackImpl(mPendingImeResponse,
- mImeStatusListener, requestConsumer, mHandler, mLock));
+ mImeStatusListener));
}
}
- public Optional<InlineSuggestionsRequest> getInlineSuggestionsRequest() {
+ public Optional<InlineSuggestionsRequest> waitAndGetInlineSuggestionsRequest() {
final CompletableFuture<ImeResponse> pendingImeResponse = getPendingImeResponse();
- if (pendingImeResponse == null || !pendingImeResponse.isDone()) {
+ if (pendingImeResponse == null) {
return Optional.empty();
}
- return Optional.ofNullable(pendingImeResponse.getNow(null)).map(ImeResponse::getRequest);
+ 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();
}
public boolean hideInlineSuggestionsUi(@NonNull AutofillId autofillId) {
@@ -192,7 +200,8 @@
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.
+ // There is no need to wait on the CompletableFuture since it should have been completed
+ // when {@link #waitAndGetInlineSuggestionsRequest()} was called.
ImeResponse imeResponse = completedImsResponse.getNow(null);
if (imeResponse == null) {
if (sDebug) Log.d(TAG, "onInlineSuggestionsResponseLocked with pending IMS response");
@@ -240,50 +249,20 @@
private static final class InlineSuggestionsRequestCallbackImpl
extends IInlineSuggestionsRequestCallback.Stub {
- private final Object mLock;
- @GuardedBy("mLock")
private final CompletableFuture<ImeResponse> mResponse;
- @GuardedBy("mLock")
- private final Consumer<InlineSuggestionsRequest> mRequestConsumer;
private final ImeStatusListener mImeStatusListener;
- private final Handler mHandler;
- private final Runnable mTimeoutCallback;
private InlineSuggestionsRequestCallbackImpl(CompletableFuture<ImeResponse> response,
- ImeStatusListener imeStatusListener,
- Consumer<InlineSuggestionsRequest> requestConsumer,
- Handler handler, Object lock) {
+ ImeStatusListener imeStatusListener) {
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.");
- synchronized (mLock) {
- completeIfNotLocked(null);
- }
+ mResponse.complete(null);
}
@BinderThread
@@ -302,13 +281,9 @@
mImeStatusListener.onInputMethodFinishInputView(imeFieldId);
}
if (request != null && callback != null) {
- synchronized (mLock) {
- completeIfNotLocked(new ImeResponse(request, callback));
- }
+ mResponse.complete(new ImeResponse(request, callback));
} else {
- synchronized (mLock) {
- completeIfNotLocked(null);
- }
+ mResponse.complete(null);
}
}
diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java
index 50646638..9693535 100644
--- a/services/autofill/java/com/android/server/autofill/Session.java
+++ b/services/autofill/java/com/android/server/autofill/Session.java
@@ -114,9 +114,7 @@
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.
@@ -309,47 +307,7 @@
/**
* Receiver of assist data from the app's {@link Activity}.
*/
- 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<InlineSuggestionsRequest> 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;
- }
-
+ private final IAssistDataReceiver mAssistReceiver = new IAssistDataReceiver.Stub() {
@Override
public void onHandleAssistData(Bundle resultData) throws RemoteException {
if (mRemoteFillService == null) {
@@ -444,17 +402,17 @@
final ArrayList<FillContext> contexts =
mergePreviousSessionLocked(/* forSave= */ false);
- request = new FillRequest(requestId, contexts, mClientState, flags,
- /*inlineSuggestionsRequest=*/null);
- mPendingFillRequest = request;
- mCountDownLatch.countDown();
- maybeRequestFillLocked();
+ final Optional<InlineSuggestionsRequest> inlineSuggestionsRequest =
+ mInlineSuggestionSession.waitAndGetInlineSuggestionsRequest();
+ request = new FillRequest(requestId, contexts, mClientState, flags,
+ inlineSuggestionsRequest.orElse(null));
}
if (mActivityToken != null) {
mService.sendActivityAssistDataToContentCapture(mActivityToken, resultData);
}
+ mRemoteFillService.onFillRequest(request);
}
@Override
@@ -647,15 +605,9 @@
private void maybeRequestInlineSuggestionsRequestThenFillLocked(@NonNull ViewState viewState,
int newState, int flags) {
if (isInlineSuggestionsEnabledLocked()) {
- Consumer<InlineSuggestionsRequest> inlineSuggestionsRequestConsumer =
- mAssistReceiver.newAutofillRequestLocked(/*isInlineRequest=*/ true);
- if (inlineSuggestionsRequestConsumer != null) {
- mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId,
- inlineSuggestionsRequestConsumer);
- }
- } else {
- mAssistReceiver.newAutofillRequestLocked(/*isInlineRequest=*/ false);
+ mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId);
}
+
requestNewFillResponseLocked(viewState, newState, flags);
}
@@ -756,7 +708,7 @@
setClientLocked(client);
mInlineSuggestionSession = new InlineSuggestionSession(inputMethodManagerInternal, userId,
- componentName, handler);
+ componentName);
mMetricsLogger.write(newLogMaker(MetricsEvent.AUTOFILL_SESSION_STARTED)
.addTaggedData(MetricsEvent.FIELD_AUTOFILL_FLAGS, flags));
@@ -2711,7 +2663,7 @@
private boolean requestShowInlineSuggestionsLocked(@NonNull FillResponse response,
@Nullable String filterText) {
final Optional<InlineSuggestionsRequest> inlineSuggestionsRequest =
- mInlineSuggestionSession.getInlineSuggestionsRequest();
+ mInlineSuggestionSession.waitAndGetInlineSuggestionsRequest();
if (!inlineSuggestionsRequest.isPresent()) {
Log.w(TAG, "InlineSuggestionsRequest unavailable");
return false;
@@ -2944,9 +2896,6 @@
/**
* Tries to trigger Augmented Autofill when the standard service could not fulfill a request.
*
- * <p> 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
@@ -3029,21 +2978,6 @@
final AutofillId focusedId = AutofillId.withoutSession(mCurrentViewId);
- final Consumer<InlineSuggestionsRequest> 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)
@@ -3051,12 +2985,21 @@
// doesn't want autofill
if (mForAugmentedAutofillOnly || !isInlineSuggestionsEnabledLocked()) {
if (sDebug) Slog.d(TAG, "Create inline request for augmented autofill");
- mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId,
- /*requestConsumer=*/ requestAugmentedAutofill);
- } else {
- requestAugmentedAutofill.accept(
- mInlineSuggestionSession.getInlineSuggestionsRequest().orElse(null));
+ mInlineSuggestionSession.onCreateInlineSuggestionsRequest(mCurrentViewId);
}
+
+ Optional<InlineSuggestionsRequest> 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();
}
