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

Commit 6e43dd3a authored by Felipe Leme's avatar Felipe Leme
Browse files

Don't trigger autofill requests on fields present in the initial request.

On Android O, when the autofill service returned null to a fill request we'd close both the
server-side and client-side sessions. The downside is that if new views were dynamically added,
it wouldn't trigger more request (which was problematic on apps using WebView)

On Android P, we changed it so the client-side was marked as FINISHED, but we kept track of which
fields were already focused, so it wouldn't trigger more requests when their were focused again.

Now on Android Q we're optimizing it further so we don't trigger extra requests on any field that
were originally present during the fill request (but we still trigger it for fields added later).

Test: atest CtsAutoFillServiceTestCases:android.autofillservice.cts.LoginActivityTest#testAutoFillNoDatasets_multipleFields_alwaysNull

Test: atest CtsAutoFillServiceTestCases # sanity check

Fixes: 128551128

Change-Id: Iaa203dc415bfd68647f07e8d7be1ce07bb49ffc9
parent aea5a1ae
Loading
Loading
Loading
Loading
+10 −5
Original line number Diff line number Diff line
@@ -2211,12 +2211,17 @@ public final class AutofillManager {
     *  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).
     * @param autofillableIds list of ids that could trigger autofill, use to not handle a new
     *  session when they're entered.
     */
    private void setSessionFinished(int newState) {
    private void setSessionFinished(int newState, @Nullable List<AutofillId> autofillableIds) {
        synchronized (mLock) {
            if (sVerbose) {
                Log.v(TAG, "setSessionFinished(): from " + getStateAsStringLocked() + " to "
                        + getStateAsString(newState));
                        + getStateAsString(newState) + "; autofillableIds=" + autofillableIds);
            }
            if (autofillableIds != null) {
                mEnteredIds = new ArraySet<>(autofillableIds);
            }
            if (newState == STATE_UNKNOWN_COMPAT_MODE || newState == STATE_UNKNOWN_FAILED) {
                resetSessionLocked(/* resetEnteredIds= */ true);
@@ -2328,7 +2333,7 @@ public final class AutofillManager {

        if (sessionFinishedState != 0) {
            // Callback call was "hijacked" to also update the session state.
            setSessionFinished(sessionFinishedState);
            setSessionFinished(sessionFinishedState, /* autofillableIds= */ null);
        }
    }

@@ -3114,10 +3119,10 @@ public final class AutofillManager {
        }

        @Override
        public void setSessionFinished(int newState) {
        public void setSessionFinished(int newState, List<AutofillId> autofillableIds) {
            final AutofillManager afm = mAfm.get();
            if (afm != null) {
                afm.post(() -> afm.setSessionFinished(newState));
                afm.post(() -> afm.setSessionFinished(newState, autofillableIds));
            }
        }

+3 −1
Original line number Diff line number Diff line
@@ -98,8 +98,10 @@ oneway interface IAutoFillManagerClient {
     *
     * @param newState STATE_FINISHED (because the autofill service returned a null
     * FillResponse) or STATE_UNKNOWN (because the session was removed).
     * @param autofillableIds list of ids that could trigger autofill, use to not handle a new
     * session when they're entered.
     */
   void setSessionFinished(int newState);
   void setSessionFinished(int newState, in List<AutofillId> autofillableIds);

   /**
    * Gets a reference to the binder object that can be used by the Augmented Autofill service.
+2 −1
Original line number Diff line number Diff line
@@ -298,7 +298,8 @@ final class AutofillManagerServiceImpl
            final IAutoFillManagerClient client = IAutoFillManagerClient.Stub
                    .asInterface(appCallbackToken);
            try {
                client.setSessionFinished(AutofillManager.STATE_DISABLED_BY_SERVICE);
                client.setSessionFinished(AutofillManager.STATE_DISABLED_BY_SERVICE,
                        /* autofillableIds= */ null);
            } catch (RemoteException e) {
                Slog.w(TAG, "Could not notify " + shortComponentName + " that it's disabled: " + e);
            }
+28 −0
Original line number Diff line number Diff line
@@ -20,12 +20,14 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.assist.AssistStructure;
import android.app.assist.AssistStructure.ViewNode;
import android.app.assist.AssistStructure.WindowNode;
import android.content.ComponentName;
import android.metrics.LogMaker;
import android.service.autofill.Dataset;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Slog;
import android.view.View;
import android.view.WindowManager;
import android.view.autofill.AutofillId;
import android.view.autofill.AutofillValue;
@@ -205,6 +207,32 @@ public final class Helper {
        }
    }

    /**
     * Gets the {@link AutofillId} of the autofillable nodes in the {@code structure}.
     */
    @NonNull
    static ArrayList<AutofillId> getAutofillableIds(@NonNull AssistStructure structure) {
        final ArrayList<AutofillId> ids = new ArrayList<>();
        final int size = structure.getWindowNodeCount();
        for (int i = 0; i < size; i++) {
            final WindowNode node = structure.getWindowNodeAt(i);
            addAutofillableIds(node.getRootViewNode(), ids);
        }
        return ids;
    }

    private static void addAutofillableIds(@NonNull ViewNode node,
            @NonNull ArrayList<AutofillId> ids) {
        if (node.getAutofillType() != View.AUTOFILL_TYPE_NONE) {
            ids.add(node.getAutofillId());
        }
        final int size = node.getChildCount();
        for (int i = 0; i < size; i++) {
            final ViewNode child = node.getChildAt(i);
            addAutofillableIds(child, ids);
        }
    }

    private interface ViewNodeFilter {
        boolean matches(ViewNode node);
    }
+30 −13
Original line number Diff line number Diff line
@@ -699,14 +699,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                if (requestLog != null) {
                    requestLog.addTaggedData(MetricsEvent.FIELD_AUTOFILL_NUM_DATASETS, -1);
                }
                processNullResponseLocked(requestFlags);
                processNullResponseLocked(requestId, requestFlags);
                return;
            }

            fieldClassificationIds = response.getFieldClassificationIds();
            if (fieldClassificationIds != null && !mService.isFieldClassificationEnabledLocked()) {
                Slog.w(TAG, "Ignoring " + response + " because field detection is disabled");
                processNullResponseLocked(requestFlags);
                processNullResponseLocked(requestId, requestFlags);
                return;
            }
        }
@@ -739,7 +739,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                        && response.getAuthentication() == null)
                || disableDuration > 0) {
            // Response is "empty" from an UI point of view, need to notify client.
            notifyUnavailableToClient(sessionFinishedState);
            notifyUnavailableToClient(sessionFinishedState, /* autofillableIds= */ null);
        }

        if (requestLog != null) {
@@ -802,7 +802,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                }
            }
        }
        notifyUnavailableToClient(AutofillManager.STATE_UNKNOWN_FAILED);
        notifyUnavailableToClient(AutofillManager.STATE_UNKNOWN_FAILED,
                /* autofillableIds= */ null);
        if (showMessage) {
            getUiForShowing().showError(message, this);
        }
@@ -1131,7 +1132,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
            }
            logAuthenticationStatusLocked(requestId,
                    MetricsEvent.AUTOFILL_INVALID_AUTHENTICATION);
            processNullResponseLocked(0);
            processNullResponseLocked(requestId, 0);
        }
    }

@@ -2428,14 +2429,15 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
        }
    }

    private void notifyUnavailableToClient(int sessionFinishedState) {
    private void notifyUnavailableToClient(int sessionFinishedState,
            @Nullable ArrayList<AutofillId> autofillableIds) {
        synchronized (mLock) {
            if (mCurrentViewId == null) return;
            try {
                if (mHasCallback) {
                    mClient.notifyNoFillUi(id, mCurrentViewId, sessionFinishedState);
                } else if (sessionFinishedState != 0) {
                    mClient.setSessionFinished(sessionFinishedState);
                    mClient.setSessionFinished(sessionFinishedState, autofillableIds);
                }
            } catch (RemoteException e) {
                Slog.e(TAG, "Error notifying client no fill UI: id=" + mCurrentViewId, e);
@@ -2551,10 +2553,22 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
    }

    @GuardedBy("mLock")
    private void processNullResponseLocked(int flags) {
    private void processNullResponseLocked(int requestId, int flags) {
        if ((flags & FLAG_MANUAL_REQUEST) != 0) {
            getUiForShowing().showError(R.string.autofill_error_cannot_autofill, this);
        }

        final FillContext context = getFillContextByRequestIdLocked(requestId);

        final ArrayList<AutofillId> autofillableIds;
        if (context != null) {
            final AssistStructure structure = context.getStructure();
            autofillableIds = Helper.getAutofillableIds(structure);
        } else {
            Slog.w(TAG, "processNullResponseLocked(): no context for req " + requestId);
            autofillableIds = null;
        }

        mService.resetLastResponse();

        // The default autofill service cannot fullfill the request, let's check if the augmented
@@ -2563,18 +2577,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
        if (mAugmentedAutofillDestroyer == null) {
            if (sVerbose) {
                Slog.v(TAG, "canceling session " + id + " when server returned null and there is no"
                        + " AugmentedAutofill for user");
                        + " AugmentedAutofill for user. AutofillableIds: " + autofillableIds);
            }
            // Nothing to be done, but need to notify client.
            notifyUnavailableToClient(AutofillManager.STATE_FINISHED);
            notifyUnavailableToClient(AutofillManager.STATE_FINISHED, autofillableIds);
            removeSelf();
        } else {
            // TODO(b/123099468, b/119638958): must set internal state so when user focus other
            // fields it does not generate a new call to the standard autofill service (right now
            // it does). Must also add CTS tests to exercise this scenario.
            // it does). In other words, must also pass the autofillableIds - we'll be handled in
            // a separate change (with new CTS tests to exercise this scenario).

            if (sVerbose) {
                Slog.v(TAG, "keeping session " + id + " when server returned null but "
                        + "there is an AugmentedAutofill for user");
                        + "there is an AugmentedAutofill for user. AutofillableIds: "
                        + autofillableIds);
            }
        }
    }
@@ -3134,7 +3151,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
        mUi.destroyAll(mPendingSaveUi, this, false);
        if (!isPendingSaveUi) {
            try {
                mClient.setSessionFinished(clientState);
                mClient.setSessionFinished(clientState, /* autofillableIds= */ null);
            } catch (RemoteException e) {
                Slog.e(TAG, "Error notifying client to finish session", e);
            }