diff --git a/core/java/android/service/autofill/AutofillService.java b/core/java/android/service/autofill/AutofillService.java index 6c18b4560d294..573d57779df7d 100644 --- a/core/java/android/service/autofill/AutofillService.java +++ b/core/java/android/service/autofill/AutofillService.java @@ -651,14 +651,11 @@ public abstract class AutofillService extends Service { /** * Called when the user requests the service to save the contents of a screen. * - *
Service must call one of the {@link SaveCallback} methods (like - * {@link SaveCallback#onSuccess()} or {@link SaveCallback#onFailure(CharSequence)}) - * to notify the Android System of the result of the request. - * *
If the service could not handle the request right away—for example, because it must * launch an activity asking the user to authenticate first or because the network is * down—the service could keep the {@link SaveRequest request} and reuse it later, - * but the service must call {@link SaveCallback#onSuccess()} right away. + * but the service must always call {@link SaveCallback#onSuccess()} or + * {@link SaveCallback#onSuccess(android.content.IntentSender)} right away. * *
Note: To retrieve the actual value of fields input by the user, the service * should call diff --git a/core/java/android/service/autofill/FillCallback.java b/core/java/android/service/autofill/FillCallback.java index 3893f2a6033d2..06e2896a7f8ad 100644 --- a/core/java/android/service/autofill/FillCallback.java +++ b/core/java/android/service/autofill/FillCallback.java @@ -83,19 +83,24 @@ public final class FillCallback { * fulfill the request; in this case, the service should call {@link #onSuccess(FillResponse) * onSuccess(null)} instead. * - *
Note: on Android versions up to {@link android.os.Build.VERSION_CODES#P}, this - * method is not working as intended, and the service should call + *
Note: prior to {@link android.os.Build.VERSION_CODES#Q}, this + * method was not working as intended and the service should always call * {@link #onSuccess(FillResponse) onSuccess(null)} instead. * - * @param message error message to be displayed to the user. Note: this message is - * displayed on {@code logcat} logs and should not contain PII (Personally Identifiable - * Information, such as username or email address). + *
Note: for apps targeting {@link android.os.Build.VERSION_CODES#Q} or higher, this + * method just logs the message on {@code logcat}; for apps targetting older SDKs, it also + * displays the message to user using a {@link android.widget.Toast}. Generally speaking, you + * should not display an error to the user if the request failed, unless the request had the + * {@link FillRequest#FLAG_MANUAL_REQUEST} flag. + * + * @param message error message. Note: this message should not contain PII + * (Personally Identifiable Information, such as username or email address). * * @throws IllegalStateException if this method or {@link #onSuccess(FillResponse)} was already * called. */ public void onFailure(@Nullable CharSequence message) { - Log.w(TAG, "onFailure(): " + (message == null ? null : message.length() + "_chars")); + Log.w(TAG, "onFailure(): " + message); assertNotCalled(); mCalled = true; try { diff --git a/core/java/android/service/autofill/SaveCallback.java b/core/java/android/service/autofill/SaveCallback.java index 0625095cfe3f7..1753ecfa4e234 100644 --- a/core/java/android/service/autofill/SaveCallback.java +++ b/core/java/android/service/autofill/SaveCallback.java @@ -83,28 +83,30 @@ public final class SaveCallback { } } + + + /** * Notifies the Android System that an * {@link AutofillService#onSaveRequest(SaveRequest, SaveCallback)} could not be handled * by the service. * - *
This method should only be called when the service could not handle the request right away - * and could not recover or retry it. If the service could retry or recover, it could keep - * the {@link SaveRequest} and call {@link #onSuccess()} instead. + *
This method is just used for logging purposes, the Android System won't call the service + * again in case of failures—if you need to recover from the failure, just save the + * {@link SaveRequest} and try again later. * - *
Note: The Android System displays an UI with the supplied error message; if - * you prefer to show your own message, call {@link #onSuccess()} or - * {@link #onSuccess(IntentSender)} instead. + *
Note: for apps targeting {@link android.os.Build.VERSION_CODES#Q} or higher, this + * method just logs the message on {@code logcat}; for apps targetting older SDKs, it also + * displays the message to user using a {@link android.widget.Toast}. * - * @param message error message to be displayed to the user. Note: this message is - * displayed on {@code logcat} logs and should not contain PII (Personally Identifiable - * Information, such as username or email address). + * @param message error message. Note: this message should not contain PII + * (Personally Identifiable Information, such as username or email address). * * @throws IllegalStateException if this method, {@link #onSuccess()}, * or {@link #onSuccess(IntentSender)} was already called. */ public void onFailure(CharSequence message) { - Log.w(TAG, "onFailure(): " + (message == null ? null : message.length() + "_chars")); + Log.w(TAG, "onFailure(): " + message); assertNotCalled(); mCalled = true; try { diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java index e87048e1a5cb2..7abe19e795f4e 100644 --- a/core/java/android/view/autofill/AutofillManager.java +++ b/core/java/android/view/autofill/AutofillManager.java @@ -301,6 +301,14 @@ public final class AutofillManager { */ public static final int STATE_UNKNOWN_COMPAT_MODE = 5; + /** + * Same as {@link #STATE_UNKNOWN}, but used on + * {@link AutofillManagerClient#setSessionFinished(int)} when the session was finished because + * the service failed to fullfil a request. + * + * @hide + */ + public static final int STATE_UNKNOWN_FAILED = 6; /** * Timeout in ms for calls to the field classification service. @@ -2023,8 +2031,10 @@ public final class AutofillManager { * @param newState {@link #STATE_FINISHED} (because the autofill service returned a {@code null} * FillResponse), {@link #STATE_UNKNOWN} (because the session was removed), * {@link #STATE_UNKNOWN_COMPAT_MODE} (beucase the session was finished when the URL bar - * changed on compat mode), or {@link #STATE_DISABLED_BY_SERVICE} (because the autofill service - * disabled further autofill requests for the activity). + * changed on compat mode), {@link #STATE_UNKNOWN_FAILED} (because the session was finished + * when the service failed to fullfil the request, or {@link #STATE_DISABLED_BY_SERVICE} + * (because the autofill service or {@link #STATE_DISABLED_BY_SERVICE} (because the autofill + * service disabled further autofill requests for the activity). */ private void setSessionFinished(int newState) { synchronized (mLock) { @@ -2032,7 +2042,7 @@ public final class AutofillManager { Log.v(TAG, "setSessionFinished(): from " + getStateAsStringLocked() + " to " + getStateAsString(newState)); } - if (newState == STATE_UNKNOWN_COMPAT_MODE) { + if (newState == STATE_UNKNOWN_COMPAT_MODE || newState == STATE_UNKNOWN_FAILED) { resetSessionLocked(/* resetEnteredIds= */ true); mState = STATE_UNKNOWN; } else { @@ -2229,6 +2239,8 @@ public final class AutofillManager { return "DISABLED_BY_SERVICE"; case STATE_UNKNOWN_COMPAT_MODE: return "UNKNOWN_COMPAT_MODE"; + case STATE_UNKNOWN_FAILED: + return "UNKNOWN_FAILED"; default: return "INVALID:" + state; } diff --git a/proto/src/metrics_constants/metrics_constants.proto b/proto/src/metrics_constants/metrics_constants.proto index 41b3febef31ff..de85dcb16adbf 100644 --- a/proto/src/metrics_constants/metrics_constants.proto +++ b/proto/src/metrics_constants/metrics_constants.proto @@ -4026,6 +4026,8 @@ message MetricsEvent { // - AUTOFILL_DATASET_AUTHENTICATED // - AUTOFILL_INVALID_AUTHENTICATION // - AUTOFILL_INVALID_DATASET_AUTHENTICATION + // NOTE: starting on OS Q, it also added the following fields: + // Tag FIELD_AUTOFILL_TEXT_LEN: length of the error message provided by the service AUTOFILL_REQUEST = 907; // Tag of a field for a package of an autofill service @@ -4102,6 +4104,8 @@ message MetricsEvent { // Tag FIELD_CLASS_NAME: Class name of the activity that is autofilled. // Tag FIELD_AUTOFILL_SESSION_ID: id of the autofill session associated with this metric. // Tag FIELD_AUTOFILL_COMPAT_MODE: package is being autofilled on compatibility mode. + // NOTE: starting on OS Q, it also added the following fields: + // Tag FIELD_AUTOFILL_TEXT_LEN: length of the error message provided by the service AUTOFILL_DATA_SAVE_REQUEST = 918; // An auto-fill session was finished @@ -6517,6 +6521,9 @@ message MetricsEvent { // OS: Q MOBILE_NETWORK = 1571; + // Tag of a field for the length of a text + FIELD_AUTOFILL_TEXT_LEN = 1572; + // ---- End Q Constants, all Q constants go above this line ---- // Add new aosp constants above this line. diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index 8c8352f81df5b..78facf84b03c8 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -235,6 +235,13 @@ final class AutofillManagerServiceImpl { } } + int getTargedSdkLocked() { + if (mInfo == null) { + return 0; + } + return mInfo.getServiceInfo().applicationInfo.targetSdkVersion; + } + private boolean isSetupCompletedLocked() { final String setupComplete = Settings.Secure.getStringForUser( mContext.getContentResolver(), Settings.Secure.USER_SETUP_COMPLETE, mUserId); @@ -953,6 +960,7 @@ final class AutofillManagerServiceImpl { pw.println(); mInfo.dump(prefix2, pw); pw.print(prefix); pw.print("Service Label: "); pw.println(getServiceLabel()); + pw.print(prefix); pw.print("Target SDK: "); pw.println(getTargedSdkLocked()); } pw.print(prefix); pw.print("Component from settings: "); pw.println(getComponentNameFromSettings()); diff --git a/services/autofill/java/com/android/server/autofill/RemoteFillService.java b/services/autofill/java/com/android/server/autofill/RemoteFillService.java index 2671327350be9..d1b09ca1aca8e 100644 --- a/services/autofill/java/com/android/server/autofill/RemoteFillService.java +++ b/services/autofill/java/com/android/server/autofill/RemoteFillService.java @@ -29,6 +29,7 @@ import android.content.Context; import android.content.Intent; import android.content.IntentSender; import android.content.ServiceConnection; +import android.os.Build; import android.os.Handler; import android.os.IBinder; import android.os.IBinder.DeathRecipient; diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index bf6964fcfcb37..cf323fbd24b70 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -50,6 +50,7 @@ import android.graphics.Bitmap; import android.graphics.Rect; import android.metrics.LogMaker; import android.os.Binder; +import android.os.Build; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; @@ -63,6 +64,7 @@ import android.service.autofill.AutofillService; import android.service.autofill.Dataset; import android.service.autofill.FieldClassification; import android.service.autofill.FieldClassification.Match; +import android.text.TextUtils; import android.service.autofill.FillContext; import android.service.autofill.FillRequest; import android.service.autofill.FillResponse; @@ -744,12 +746,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState private void onFillRequestFailureOrTimeout(int requestId, boolean timedOut, @Nullable CharSequence message) { + boolean showMessage = !TextUtils.isEmpty(message); synchronized (mLock) { if (mDestroyed) { Slog.w(TAG, "Call to Session#onFillRequestFailureOrTimeout(req=" + requestId + ") rejected - session: " + id + " destroyed"); return; } + if (sDebug) { + Slog.d(TAG, "finishing session due to service " + + (timedOut ? "timeout" : "failure")); + } mService.resetLastResponse(); final LogMaker requestLog = mRequestLogs.get(requestId); if (requestLog == null) { @@ -757,8 +764,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } else { requestLog.setType(timedOut ? MetricsEvent.TYPE_CLOSE : MetricsEvent.TYPE_FAILURE); } + if (showMessage) { + final int targetSdk = mService.getTargedSdkLocked(); + if (targetSdk >= Build.VERSION_CODES.Q) { + showMessage = false; + Slog.w(TAG, "onFillRequestFailureOrTimeout(): not showing '" + message + + "' because service's targetting API " + targetSdk); + } + if (message != null) { + requestLog.addTaggedData(MetricsEvent.FIELD_AUTOFILL_TEXT_LEN, + message.length()); + } + } } - if (message != null) { + notifyUnavailableToClient(AutofillManager.STATE_UNKNOWN_FAILED); + if (showMessage) { getUiForShowing().showError(message, this); } removeSelf(); @@ -793,6 +813,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState @Override public void onSaveRequestFailure(@Nullable CharSequence message, @NonNull String servicePackageName) { + boolean showMessage = !TextUtils.isEmpty(message); synchronized (mLock) { mIsSaving = false; @@ -801,12 +822,26 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState + id + " destroyed"); return; } + if (showMessage) { + final int targetSdk = mService.getTargedSdkLocked(); + if (targetSdk >= Build.VERSION_CODES.Q) { + showMessage = false; + Slog.w(TAG, "onSaveRequestFailure(): not showing '" + message + + "' because service's targetting API " + targetSdk); + } + } } - LogMaker log = newLogMaker(MetricsEvent.AUTOFILL_DATA_SAVE_REQUEST, servicePackageName) + final LogMaker log = + newLogMaker(MetricsEvent.AUTOFILL_DATA_SAVE_REQUEST, servicePackageName) .setType(MetricsEvent.TYPE_FAILURE); + if (message != null) { + log.addTaggedData(MetricsEvent.FIELD_AUTOFILL_TEXT_LEN, message.length()); + } mMetricsLogger.write(log); - getUiForShowing().showError(message, this); + if (showMessage) { + getUiForShowing().showError(message, this); + } removeSelf(); }