Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d9dc954e authored by Felipe Leme's avatar Felipe Leme
Browse files

Fixed FillCallback.onFailure() and SaveCallback.onFailure() behavior. 

FillCallback.onFailure() was not working as intented - it finished the session
on AutofillManagerService, but didn't update the client state on
AutofillManager.

And both of these methods were displaying Toasts to the user, which is something
the autofill services could take care of. Hence, for services target with SDK
Q, the message is ignored.

Also added a new Autofill Metric: FIELD_AUTOFILL_MESSAGE_LEN

Test: atest CtsAutoFillServiceTestCases:android.autofillservice.cts.LoginActivityTest#testAutofillAgainAfterOnFailure
Test: atest CtsAutoFillServiceTestCases # to make sure it didn't break anything
Test: time mmm -j frameworks/base/:doc-comment-check-docs
Test: m -j update-api

Bug: 112192360
Fixes: 116103297

Change-Id: I499909200980943dedf1fc8524dd1f14b49e2158
parent 82b66a4f

core/java/android/service/autofill/AutofillService.java

+2 −5
Original line number Diff line number Diff line
@@ -651,14 +651,11 @@ public abstract class AutofillService extends Service { 
    /**

     * Called when the user requests the service to save the contents of a screen.

     *

     * <p>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.

     *

     * <p>If the service could not handle the request right away&mdash;for example, because it must

     * launch an activity asking the user to authenticate first or because the network is

     * down&mdash;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 <b>must always</b> call {@link SaveCallback#onSuccess()} or

     * {@link SaveCallback#onSuccess(android.content.IntentSender)} right away.

     *

     * <p><b>Note:</b> To retrieve the actual value of fields input by the user, the service

     * should call

core/java/android/service/autofill/FillCallback.java

+11 −6
Original line number Diff line number Diff line
@@ -83,19 +83,24 @@ public final class FillCallback { 
     * fulfill the request; in this case, the service should call {@link #onSuccess(FillResponse)

     * onSuccess(null)} instead.

     *

     * <p><b>Note: </b>on Android versions up to {@link android.os.Build.VERSION_CODES#P}, this

     * method is not working as intended, and the service should call

     * <p><b>Note: </b>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. <b>Note: </b> this message is

     * displayed on {@code logcat} logs and should not contain PII (Personally Identifiable

     * Information, such as username or email address).

     * <p><b>Note: </b>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. <b>Note: </b> this message should <b>not</b> 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 {

core/java/android/service/autofill/SaveCallback.java

+12 −10
Original line number Diff line number Diff line
@@ -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.

     *

     * <p>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.

     * <p>This method is just used for logging purposes, the Android System won't call the service

     * again in case of failures&mdash;if you need to recover from the failure, just save the

     * {@link SaveRequest} and try again later.

     *

     * <p><b>Note:</b> 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.

     * <p><b>Note: </b>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. <b>Note: </b> 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. <b>Note: </b> this message should <b>not</b> 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 {

core/java/android/view/autofill/AutofillManager.java

+15 −3
Original line number Diff line number Diff line
@@ -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;

        }

proto/src/metrics_constants/metrics_constants.proto

+7 −0
Original line number Diff line number Diff line
@@ -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.