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

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

Fixed new multi-screens autofill API: 

- Previous FillContexts should be shown on subsequent FillRequests.
- Client state should be properly propagated.

Test: atest MultiScreenLoginTest
Test: atest CtsAutoFillServiceTestCases # to make sure it didn't break anything

Fixes: 112051762
Bug: 113281366

Change-Id: I17de046385bd923279684d461afd24b49ad6f89a
parent b7cc5351

core/java/android/service/autofill/FillRequest.java

+4 −0
Original line number Diff line number Diff line
@@ -123,6 +123,10 @@ public final class FillRequest implements Parcelable { 


    /**

     * Gets the contexts associated with each previous fill request.

     *

     * <p><b>Note:</b> Starting on Android {@link android.os.Build.VERSION_CODES#Q}, it could also

     * include contexts from requests whose {@link SaveInfo} had the

     * {@link SaveInfo#FLAG_DELAY_SAVE} flag.

     */

    public @NonNull List<FillContext> getFillContexts() {

        return mContexts;

core/java/android/service/autofill/SaveInfo.java

+1 −1
Original line number Diff line number Diff line
@@ -246,7 +246,7 @@ public final class SaveInfo implements Parcelable { 
     * multiple screens to implement an autofillable workflow (for example, one screen for the

     * username field, another for password).

     */

    // TODO(b/112051762): improve documentation: add example, document relationship with other

    // TODO(b/113281366): improve documentation: add example, document relationship with other

    // flagss, etc...

    public static final int FLAG_DELAY_SAVE = 0x4;

services/autofill/java/com/android/server/autofill/AutofillManagerService.java

+2 −2
Original line number Diff line number Diff line
@@ -978,12 +978,12 @@ public final class AutofillManagerService extends SystemService { 
                throw new IllegalArgumentException(packageName + " is not a valid package", e);

            }



            // TODO(b/112051762): rather than always call AM here, call it on demand on

            // TODO(b/113281366): rather than always call AM here, call it on demand on

            // getPreviousSessionsLocked()? That way we save space / time here, and don't set

            // a callback on AM unnecessarily (see TODO below :-)

            final ActivityManagerInternal am = LocalServices

                    .getService(ActivityManagerInternal.class);

            // TODO(b/112051762): add a callback method on AM to be notified when a task is finished

            // TODO(b/113281366): add a callback method on AM to be notified when a task is finished

            // so we can clean up sessions kept alive

            final int taskId = am.getTaskIdForActivity(activityToken, false);

            final int sessionId;

services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java

+2 −2
Original line number Diff line number Diff line
@@ -617,7 +617,7 @@ final class AutofillManagerServiceImpl { 
        ArrayList<Session> previousSessions = null;

        for (int i = 0; i < size; i++) {

            final Session previousSession = mSessions.valueAt(i);

            // TODO(b/112051762): only return sessions asked to be kept alive / add CTS test

            // TODO(b/113281366): only return sessions asked to be kept alive / add CTS test

            if (previousSession.taskId == session.taskId && previousSession.id != session.id) {

                if (previousSessions == null) {

                    previousSessions = new ArrayList<>(size);
@@ -625,7 +625,7 @@ final class AutofillManagerServiceImpl { 
                previousSessions.add(previousSession);

            }

        }

        // TODO(b/112051762): remove returned sessions / add CTS test

        // TODO(b/113281366): remove returned sessions / add CTS test

        return previousSessions;

    }

services/autofill/java/com/android/server/autofill/Session.java

+47 −20
Original line number Diff line number Diff line
@@ -75,6 +75,7 @@ import android.service.autofill.ValueFinder; 
import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.LocalLog;

import android.util.Pair;

import android.util.Slog;

import android.util.SparseArray;

import android.util.TimeUtils;
@@ -329,12 +330,9 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                    fillContextWithAllowedValuesLocked(mContexts.get(i), flags);

                }



                // Dispatch a snapshot of the current contexts list since it may change

                // until the dispatch happens. The items in the list don't need to be cloned

                // since we don't hold on them anywhere else. The client state is not touched

                // by us, so no need to copy.

                request = new FillRequest(requestId, new ArrayList<>(mContexts), mClientState,

                        flags);

                final ArrayList<FillContext> contexts =

                        mergePreviousSessionLocked(/* forSave= */ false);

                request = new FillRequest(requestId, contexts, mClientState, flags);

            }



            mRemoteFillService.onFillRequest(request);
@@ -1465,7 +1463,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
        }



        if ((saveInfo.getFlags() & SaveInfo.FLAG_DELAY_SAVE) != 0) {

            // TODO(b/112051762): log metrics

            // TODO(b/113281366): log metrics

            if (sDebug) Slog.v(TAG, "showSaveLocked(): service asked to delay save");

            return false;

        }
@@ -1890,13 +1888,38 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
        // Remove pending fill requests as the session is finished.

        cancelCurrentRequestLocked();



        // Merge the previous sessions that the service asked to be kept alive

        final ArrayList<FillContext> contexts = mergePreviousSessionLocked( /* forSave= */ true);



        final SaveRequest saveRequest =

                new SaveRequest(contexts, mClientState, mSelectedDatasetIds);

        mRemoteFillService.onSaveRequest(saveRequest);

    }



    // TODO(b/113281366): rather than merge it here, it might be better to simply reuse the old

    // session instead of creating a new one. But we need to consider what would happen on corner

    // cases such as "Main Activity M -> activity A with username -> activity B with password"

    // If user follows the normal workflow, than session A would be merged with session B as

    // expected. But if when on Activity A the user taps back or somehow launches another activity,

    // session A could be merged with the wrong session.

    /**

     * Gets a list of contexts that includes not only this session's contexts but also the contexts

     * from previous sessions that were asked by the service to be delayed (if any).

     *

     * <p>As a side-effect:

     * <ul>

     *   <li>If the current {@link #mClientState} is {@code null}, sets it with the last non-

     *   {@code null} client state from previous sessions.

     *   <li>When {@code forSave} is {@code true}, calls {@link #updateValuesForSaveLocked()} in the

     *   previous sessions.

     * </ul>

     */

    @NonNull

    private ArrayList<FillContext> mergePreviousSessionLocked(boolean forSave) {

        final ArrayList<Session> previousSessions = mService.getPreviousSessionsLocked(this);

        final ArrayList<FillContext> contexts;

        final Bundle clientState;

        if (previousSessions != null) {

            if (sDebug) {

                Slog.d(TAG, "callSaveLocked(): Merging the content of " + previousSessions.size()

                Slog.d(TAG, "mergeSessions(): Merging the content of " + previousSessions.size()

                        + " sessions for task " + taskId);

            }

            contexts = new ArrayList<>();
@@ -1904,31 +1927,35 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                final Session previousSession = previousSessions.get(i);

                final ArrayList<FillContext> previousContexts = previousSession.mContexts;

                if (previousContexts == null) {

                    Slog.w(TAG, "callSaveLocked(): Not merging null contexts from "

                    Slog.w(TAG, "mergeSessions(): Not merging null contexts from "

                            + previousSession.id);

                    continue;

                }

                if (forSave) {

                    previousSession.updateValuesForSaveLocked();

                if (sVerbose) {

                    Slog.v(TAG, "callSaveLocked(): adding " + previousContexts.size()

                }

                if (sDebug) {

                    Slog.d(TAG, "mergeSessions(): adding " + previousContexts.size()

                            + " context from previous session #" + previousSession.id);

                }

                contexts.addAll(previousContexts);

                if (mClientState == null && previousSession.mClientState != null) {

                    if (sDebug) {

                        Slog.d(TAG, "mergeSessions(): setting client state from previous session"

                                + previousSession.id);

                    }

                    mClientState = previousSession.mClientState;

                }

            }

            contexts.addAll(mContexts);

            // TODO(b/112051762): decided what to do with client state / add CTS test

            clientState = mClientState;

        } else {

            // Dispatch a snapshot of the current contexts list since it may change

            // until the dispatch happens. The items in the list don't need to be cloned

            // since we don't hold on them anywhere else. The client state is not touched

            // by us, so no need to copy.

            contexts = new ArrayList<>(mContexts);

            clientState = mClientState;

        }



        final SaveRequest saveRequest = new SaveRequest(contexts, clientState, mSelectedDatasetIds);

        mRemoteFillService.onSaveRequest(saveRequest);

        return contexts;

    }



    /**