[DO NOT MERGE] Fix race in AbstractSinglePendingRequestRemoteService

Fixes: 135355306
Test: presubmit
Change-Id: I78d9840a9d7e04cd373c967b259a614990153841

core/java/com/android/internal/infra/AbstractSinglePendingRequestRemoteService.java

@@ -60,10 +60,16 @@ public abstract class AbstractSinglePendingRequestRemoteService<S
 
     @Override // from AbstractRemoteService
     protected void handleOnDestroy() {
-        if (mPendingRequest != null) {
-            mPendingRequest.cancel();
+        handleCancelPendingRequest();
+    }
+
+    protected BasePendingRequest<S, I> handleCancelPendingRequest() {
+        BasePendingRequest<S, I> pendingRequest = mPendingRequest;
+        if (pendingRequest != null) {
+            pendingRequest.cancel();
             mPendingRequest = null;
         }
+        return pendingRequest;
     }
 
     @Override // from AbstractRemoteService

services/autofill/java/com/android/server/autofill/RemoteFillService.java

@@ -41,6 +41,8 @@ import android.util.Slog;
 
 import com.android.internal.infra.AbstractSinglePendingRequestRemoteService;
 
+import java.util.concurrent.CompletableFuture;
+
 final class RemoteFillService
         extends AbstractSinglePendingRequestRemoteService<RemoteFillService, IAutoFillService> {
 
@@ -103,26 +105,21 @@ final class RemoteFillService
      * <p>This can be used when the request is unnecessary or will be superceeded by a request that
      * will soon be queued.
      *
-     * @return the id of the canceled request, or {@link FillRequest#INVALID_REQUEST_ID} if no
-     *         {@link PendingFillRequest} was canceled.
+     * @return the future id of the canceled request, or {@link FillRequest#INVALID_REQUEST_ID} if
+     *          no {@link PendingFillRequest} was canceled.
      */
-    // TODO(b/117779333): move this logic to super class (and make mPendingRequest private)
-    public int cancelCurrentRequest() {
-        if (isDestroyed()) {
-            return INVALID_REQUEST_ID;
-        }
-
-        int requestId = INVALID_REQUEST_ID;
-        if (mPendingRequest != null) {
-            if (mPendingRequest instanceof PendingFillRequest) {
-                requestId = ((PendingFillRequest) mPendingRequest).mRequest.getId();
+    public CompletableFuture<Integer> cancelCurrentRequest() {
+        return CompletableFuture.supplyAsync(() -> {
+            if (isDestroyed()) {
+                return INVALID_REQUEST_ID;
             }
 
-            mPendingRequest.cancel();
-            mPendingRequest = null;
-        }
-
-        return requestId;
+            BasePendingRequest<RemoteFillService, IAutoFillService> canceledRequest =
+                    handleCancelPendingRequest();
+            return canceledRequest instanceof PendingFillRequest
+                    ? ((PendingFillRequest) canceledRequest).mRequest.getId()
+                    : INVALID_REQUEST_ID;
+        }, mHandler::post);
     }
 
     public void onFillRequest(@NonNull FillRequest request) {

services/autofill/java/com/android/server/autofill/Session.java

@@ -546,21 +546,26 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                     + "mForAugmentedAutofillOnly: %s", mForAugmentedAutofillOnly);
             return;
         }
-        final int canceledRequest = mRemoteFillService.cancelCurrentRequest();
+        mRemoteFillService.cancelCurrentRequest().whenComplete((canceledRequest, err) -> {
+            if (err != null) {
+                Slog.e(TAG, "cancelCurrentRequest(): unexpected exception", err);
+                return;
+            }
 
-        // Remove the FillContext as there will never be a response for the service
-        if (canceledRequest != INVALID_REQUEST_ID && mContexts != null) {
-            final int numContexts = mContexts.size();
+            // Remove the FillContext as there will never be a response for the service
+            if (canceledRequest != INVALID_REQUEST_ID && mContexts != null) {
+                final int numContexts = mContexts.size();
 
-            // It is most likely the last context, hence search backwards
-            for (int i = numContexts - 1; i >= 0; i--) {
-                if (mContexts.get(i).getRequestId() == canceledRequest) {
-                    if (sDebug) Slog.d(TAG, "cancelCurrentRequest(): id = " + canceledRequest);
-                    mContexts.remove(i);
-                    break;
+                // It is most likely the last context, hence search backwards
+                for (int i = numContexts - 1; i >= 0; i--) {
+                    if (mContexts.get(i).getRequestId() == canceledRequest) {
+                        if (sDebug) Slog.d(TAG, "cancelCurrentRequest(): id = " + canceledRequest);
+                        mContexts.remove(i);
+                        break;
+                    }
                 }
             }
-        }
+        });
     }
 
     /**
@@ -2090,8 +2095,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         updateValuesForSaveLocked();
 
         // Remove pending fill requests as the session is finished.
-        cancelCurrentRequestLocked();
 
+        cancelCurrentRequestLocked();
         final ArrayList<FillContext> contexts = mergePreviousSessionLocked( /* forSave= */ true);
 
         final SaveRequest saveRequest =