From 2e30c6f371be6211f1fa2b2257084df24a535795 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Tue, 20 Jun 2017 10:55:01 -0700 Subject: [PATCH] Fixed how FillEventHistory is reset and clarified javadoc. Test: existing CtsAutoFillServiceTestCases pass Test: checkFillSelectionIsResetAfterReturningNull() Test: checkFillSelectionIsResetAfterReturningError() Test: checkFillSelectionIsResetAfterTimeout() Bug: 62802026 Change-Id: I389d95fc13c08f1056960e5f67e131c8dc306401 --- .../service/autofill/AutofillService.java | 18 ++++++++++--- .../service/autofill/FillEventHistory.java | 21 +++++++++++++--- .../autofill/AutofillManagerServiceImpl.java | 25 +++++++++++++++++++ .../server/autofill/RemoteFillService.java | 1 + .../com/android/server/autofill/Session.java | 2 ++ 5 files changed, 60 insertions(+), 7 deletions(-) diff --git a/core/java/android/service/autofill/AutofillService.java b/core/java/android/service/autofill/AutofillService.java index a90ee93d98522..9f35552a6defb 100644 --- a/core/java/android/service/autofill/AutofillService.java +++ b/core/java/android/service/autofill/AutofillService.java @@ -215,12 +215,22 @@ public abstract class AutofillService extends Service { } /** - * Returns the {@link FillEventHistory.Event events} since the last {@link FillResponse} was - * returned. + * Gets the events that happened after the last + * {@link AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, FillCallback)} + * call. * - *

The history is not persisted over reboots. + *

This method is typically used to keep track of previous user actions to optimize further + * requests. For example, the service might return email addresses in alphabetical order by + * default, but change that order based on the address the user picked on previous requests. * - * @return The history or {@code null} if there are not events. + *

The history is not persisted over reboots, and it's cleared every time the service + * replies to a {@link #onFillRequest(FillRequest, CancellationSignal, FillCallback)} by calling + * {@link FillCallback#onSuccess(FillResponse)} or {@link FillCallback#onFailure(CharSequence)} + * (if the service doesn't call any of these methods, the history will clear out after some + * pre-defined time). Hence, the service should call {@link #getFillEventHistory()} before + * finishing the {@link FillCallback}. + * + * @return The history or {@code null} if there are no events. */ @Nullable public final FillEventHistory getFillEventHistory() { AutofillManager afm = getSystemService(AutofillManager.class); diff --git a/core/java/android/service/autofill/FillEventHistory.java b/core/java/android/service/autofill/FillEventHistory.java index 3d72fccece9c6..58160ccd062d4 100644 --- a/core/java/android/service/autofill/FillEventHistory.java +++ b/core/java/android/service/autofill/FillEventHistory.java @@ -33,7 +33,20 @@ import java.util.ArrayList; import java.util.List; /** - * Describes what happened after the latest call to {@link FillCallback#onSuccess(FillResponse)}. + * Describes what happened after the last + * {@link AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, FillCallback)} + * call. + * + *

This history is typically used to keep track of previous user actions to optimize further + * requests. For example, the service might return email addresses in alphabetical order by + * default, but change that order based on the address the user picked on previous requests. + * + *

The history is not persisted over reboots, and it's cleared every time the service + * replies to a + * {@link AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, FillCallback)} + * by calling {@link FillCallback#onSuccess(FillResponse)} or + * {@link FillCallback#onFailure(CharSequence)} (if the service doesn't call any of these methods, + * the history will clear out after some pre-defined time). */ public final class FillEventHistory implements Parcelable { /** @@ -56,9 +69,11 @@ public final class FillEventHistory implements Parcelable { } /** - * Returns the client state of the {@link FillResponse}. + * Returns the client state set in the previous {@link FillResponse}. * - * @return The client state set by the last {@link FillResponse} + *

NOTE: the state is associated with the app that was autofilled in the previous + * {@link AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, FillCallback)} + * , which is not necessary the same app being autofilled now. */ @Nullable public Bundle getClientState() { return mClientState; diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index 38b796b316de4..62cd0cf6d6d51 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -495,11 +495,24 @@ final class AutofillManagerServiceImpl { } } + /** + * Resets the last fill selection. + */ + void resetLastResponse() { + synchronized (mLock) { + mEventHistory = null; + } + } + /** * Updates the last fill selection when an authentication was selected. */ void setAuthenticationSelected() { synchronized (mLock) { + if (mEventHistory == null) { + Slog.w(TAG, "setAuthenticationSelected(): ignored when history is null"); + return; + } mEventHistory.addEvent(new Event(Event.TYPE_AUTHENTICATION_SELECTED, null)); } } @@ -509,6 +522,10 @@ final class AutofillManagerServiceImpl { */ void setDatasetAuthenticationSelected(@Nullable String selectedDataset) { synchronized (mLock) { + if (mEventHistory == null) { + Slog.w(TAG, "setDatasetAuthenticationSelected(): ignored when history is null"); + return; + } mEventHistory.addEvent( new Event(Event.TYPE_DATASET_AUTHENTICATION_SELECTED, selectedDataset)); } @@ -519,6 +536,10 @@ final class AutofillManagerServiceImpl { */ void setSaveShown() { synchronized (mLock) { + if (mEventHistory == null) { + Slog.w(TAG, "setSaveShown(): ignored when history is null"); + return; + } mEventHistory.addEvent(new Event(Event.TYPE_SAVE_SHOWN, null)); } } @@ -528,6 +549,10 @@ final class AutofillManagerServiceImpl { */ void setDatasetSelected(@Nullable String selectedDataset) { synchronized (mLock) { + if (mEventHistory == null) { + Slog.w(TAG, "setDatasetSelected(): ignored when history is null"); + return; + } mEventHistory.addEvent(new Event(Event.TYPE_DATASET_SELECTED, selectedDataset)); } } diff --git a/services/autofill/java/com/android/server/autofill/RemoteFillService.java b/services/autofill/java/com/android/server/autofill/RemoteFillService.java index 9aebf6d2bacb3..aebe92e1687dd 100644 --- a/services/autofill/java/com/android/server/autofill/RemoteFillService.java +++ b/services/autofill/java/com/android/server/autofill/RemoteFillService.java @@ -427,6 +427,7 @@ final class RemoteFillService implements DeathRecipient { mCompleted = true; } + Slog.w(LOG_TAG, getClass().getSimpleName() + " timed out"); final RemoteFillService remoteService = mWeakService.get(); if (remoteService != null) { fail(remoteService); diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index 3ae05115217a7..4de55dcff8522 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -407,6 +407,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState if ((requestFlags & FLAG_MANUAL_REQUEST) != 0) { getUiForShowing().showError(R.string.autofill_error_cannot_autofill, this); } + mService.resetLastResponse(); // Nothing to be done, but need to notify client. notifyUnavailableToClient(); removeSelf(); @@ -444,6 +445,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState + id + " destroyed"); return; } + mService.resetLastResponse(); } LogMaker log = (new LogMaker(MetricsEvent.AUTOFILL_REQUEST)) .setType(MetricsEvent.TYPE_FAILURE)