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

Commit 8fa5f5e7 authored by Felipe Leme's avatar Felipe Leme
Browse files

Added more logging to diagnose a racy runtime restart. 

Also fixed a possible NPE on saveLocked() and improved locking.

Test: cts-tradefed run commandAndExit cts-dev -m CtsAutoFillServiceTestCases
Bug: 65374274

Merged-Id: I4b8368a9d19b4b4da76533dadb013ff2e2922955
Change-Id: I4b8368a9d19b4b4da76533dadb013ff2e2922955
parent 3f9f9933

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

+7 −3
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import android.annotation.IntDef; 
import android.annotation.NonNull;

import android.annotation.Nullable;

import android.os.Bundle;

import android.os.CancellationSignal;

import android.os.Parcel;

import android.os.Parcelable;

import android.view.View;
@@ -38,7 +37,7 @@ import java.util.List; 
 * interesting for saving and what are the possible ways to fill the inputs on

 * the screen if applicable.

 *

 * @see AutofillService#onFillRequest(FillRequest, CancellationSignal, FillCallback)

 * @see AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, FillCallback)

 */

public final class FillRequest implements Parcelable {
@@ -122,9 +121,14 @@ public final class FillRequest implements Parcelable { 
        return mContexts;

    }



    @Override

    public String toString() {

        return "FillRequest: [id=" + mId + ", flags=" + mFlags + ", ctxts= " + mContexts + "]";

    }



    /**

     * Gets the extra client state returned from the last {@link

     * AutofillService#onFillRequest(FillRequest, CancellationSignal, FillCallback)

     * AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, FillCallback)

     * fill request}, so the service can use it for state management.

     *

     * <p>Once a {@link AutofillService#onSaveRequest(SaveRequest, SaveCallback)

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

+11 −2
Original line number Diff line number Diff line
@@ -387,8 +387,10 @@ final class RemoteFillService implements DeathRecipient { 
                @Override

                public void executeMessage(Message message) {

                    if (mDestroyed) {

                        Slog.w(LOG_TAG, "Not handling " + message + " as service for "

                        if (sVerbose) {

                            Slog.v(LOG_TAG, "Not handling " + message + " as service for "

                                    + mComponentName + " is already destroyed");

                        }

                        return;

                    }

                    switch (message.what) {
@@ -574,6 +576,13 @@ final class RemoteFillService implements DeathRecipient { 


        @Override

        public void run() {

            synchronized (mLock) {

                if (isCancelledLocked()) {

                    // TODO(b/653742740): we should probably return here, but for now we're justing

                    // logging to confirm this is the problem if it happens again.

                    Slog.e(LOG_TAG, "run() called after canceled: " + mRequest);

                }

            }

            final RemoteFillService remoteService = getService();

            if (remoteService != null) {

                try {

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

+29 −22
Original line number Diff line number Diff line
@@ -268,7 +268,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
            value = state.getCurrentValue();

            if (value == null) {

                if (sDebug) Slog.d(TAG, "getValue(): no current value for " + id);

                value = getValueFromContexts(id);

                value = getValueFromContextsLocked(id);

            }

        }

        if (value != null) {
@@ -276,7 +276,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                return value.getTextValue().toString();

            }

            if (value.isList()) {

                final CharSequence[] options = getAutofillOptionsFromContexts(id);

                final CharSequence[] options = getAutofillOptionsFromContextsLocked(id);

                if (options != null) {

                    final int index = value.getListValue();

                    final CharSequence option = options[index];
@@ -339,21 +339,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
     * Cancels the last request sent to the {@link #mRemoteFillService}.

     */

    private void cancelCurrentRequestLocked() {

        int canceledRequest = mRemoteFillService.cancelCurrentRequest();

        final int canceledRequest = mRemoteFillService.cancelCurrentRequest();



        // Remove the FillContext as there will never be a response for the service

        if (canceledRequest != INVALID_REQUEST_ID && mContexts != null) {

            int numContexts = mContexts.size();

            final int numContexts = mContexts.size();



            // It is most likely the last context, hence search backwards

            for (int i = numContexts - 1; i >= 0; i--) {

                if (mContexts.get(i).getRequestId() == canceledRequest) {

                    if (sDebug) Slog.d(TAG, "cancelCurrentRequest(): id = " + canceledRequest);

                    mContexts.remove(i);

                    break;

                }

            }

        }



    }



    /**
@@ -585,18 +585,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
    @Override

    public void authenticate(int requestId, int datasetIndex, IntentSender intent, Bundle extras) {

        final Intent fillInIntent;

        synchronized (mLock) {

        synchronized (mLock) {

            if (mDestroyed) {

                Slog.w(TAG, "Call to Session#authenticate() rejected - session: "

                        + id + " destroyed");

                return;

            }

            fillInIntent = createAuthFillInIntentLocked(requestId, extras);

        }

            fillInIntent = createAuthFillInIntent(

                    getFillContextByRequestIdLocked(requestId).getStructure(), extras);

        }



        mService.setAuthenticationSelected(id);



        final int authenticationId = AutofillManager.makeAuthenticationId(requestId, datasetIndex);
@@ -845,7 +841,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


                AutofillValue value = viewState.getCurrentValue();

                if (value == null || value.isEmpty()) {

                    final AutofillValue initialValue = getValueFromContexts(id);

                    final AutofillValue initialValue = getValueFromContextsLocked(id);

                    if (initialValue != null) {

                        if (sDebug) {

                            Slog.d(TAG, "Value of required field " + id + " didn't change; "
@@ -899,7 +895,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                        }

                    } else {

                        // Update current values cache based on initial value

                        final AutofillValue initialValue = getValueFromContexts(id);

                        final AutofillValue initialValue = getValueFromContextsLocked(id);

                        if (sDebug) {

                            Slog.d(TAG, "no current value for " + id + "; initial value is "

                                    + initialValue);
@@ -1006,7 +1002,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
     * Gets the latest non-empty value for the given id in the autofill contexts.

     */

    @Nullable

    private AutofillValue getValueFromContexts(AutofillId id) {

    private AutofillValue getValueFromContextsLocked(AutofillId id) {

        final int numContexts = mContexts.size();

        for (int i = numContexts - 1; i >= 0; i--) {

            final FillContext context = mContexts.get(i);
@@ -1028,7 +1024,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
     * Gets the latest autofill options for the given id in the autofill contexts.

     */

    @Nullable

    private CharSequence[] getAutofillOptionsFromContexts(AutofillId id) {

    private CharSequence[] getAutofillOptionsFromContextsLocked(AutofillId id) {

        final int numContexts = mContexts.size();



        for (int i = numContexts - 1; i >= 0; i--) {
@@ -1053,6 +1049,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


        if (sVerbose) Slog.v(TAG, "callSaveLocked(): mViewStates=" + mViewStates);



        if (mContexts == null) {

            Slog.w(TAG, "callSaveLocked(): no contexts");

            return;

        }



        final int numContexts = mContexts.size();



        for (int contextNum = 0; contextNum < numContexts; contextNum++) {
@@ -1541,8 +1542,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
            // ...or handle authentication.

            mService.setDatasetAuthenticationSelected(dataset.getId(), id);

            setViewStatesLocked(null, dataset, ViewState.STATE_WAITING_DATASET_AUTH, false);

            final Intent fillInIntent = createAuthFillInIntent(

                    getFillContextByRequestIdLocked(requestId).getStructure(), mClientState);

            final Intent fillInIntent = createAuthFillInIntentLocked(requestId, mClientState);



            final int authenticationId = AutofillManager.makeAuthenticationId(requestId,

                    datasetIndex);
@@ -1556,9 +1556,16 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
        }

    }



    private Intent createAuthFillInIntent(AssistStructure structure, Bundle extras) {

    private Intent createAuthFillInIntentLocked(int requestId, Bundle extras) {

        final Intent fillInIntent = new Intent();

        fillInIntent.putExtra(AutofillManager.EXTRA_ASSIST_STRUCTURE, structure);



        final FillContext context = getFillContextByRequestIdLocked(requestId);

        if (context == null) {

            // TODO(b/653742740): this will crash system_server. We need to handle it, but we're

            // keeping it crashing for now so we can diagnose when it happens again

            Slog.wtf(TAG, "no FillContext for requestId" + requestId + "; mContexts= " + mContexts);

        }

        fillInIntent.putExtra(AutofillManager.EXTRA_ASSIST_STRUCTURE, context.getStructure());

        fillInIntent.putExtra(AutofillManager.EXTRA_CLIENT_STATE, extras);

        return fillInIntent;

    }