Merge "[DO NOT MERGE] Fix race in AbstractSinglePendingRequestRemoteService" into qt-dev

This commit is contained in:
TreeHugger Robot
2019-06-20 02:58:21 +00:00
committed by Android (Google) Code Review
3 changed files with 39 additions and 31 deletions

View File

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

View File

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

View File

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