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

Commit 39233ff0 authored by Felipe Leme's avatar Felipe Leme
Browse files

Refactored AbstractRemoteService in 2 classes. 

AbstractRemoteService was spun off from RemoteFillService, which only supports
1 pending request while not bound to the service - if a new request comes,
it cancels the previous one.

This behavior is fine for Autofill, but for Content Capture all requests must be
queued. In fact, the upcoming CTS tests for Content Capture were failing because
the 1st PendingOnContentCaptureEventsRequest would cancel the pending
PendingSessionLifecycleRequest.

So, this CL fix this problem by creating 2 subclasses:
 - AbstractSinglePendingRequestRemoteService
 - AbstractMultiplePendingRequestsRemoteService

Test: atest CtsContentCaptureServiceTestCases CtsAutoFillServiceTestCases

Bug: 111276913
Bug: 117779333

Change-Id: I43bb98be16f5def037f85a415e1f77799adf156e
parent ecb08be2

core/java/android/service/intelligence/SmartSuggestionsService.java

+11 −2
Original line number Diff line number Diff line
@@ -60,6 +60,7 @@ public abstract class SmartSuggestionsService extends Service { 


    // TODO(b/111330312): STOPSHIP use dynamic value, or change to false

    static final boolean DEBUG = true;

    static final boolean VERBOSE = false;



    /**

     * The {@link Intent} that must be declared as handled by the service.
@@ -209,7 +210,11 @@ public abstract class SmartSuggestionsService extends Service { 
     * @param sessionId the session's Id

     */

    public void onCreateInteractionSession(@NonNull InteractionContext context,

            @NonNull InteractionSessionId sessionId) {}

            @NonNull InteractionSessionId sessionId) {

        if (VERBOSE) {

            Log.v(TAG, "onCreateInteractionSession(id=" + sessionId + ", ctx=" + context + ")");

        }

    }



    /**

     * Notifies the service of {@link ContentCaptureEvent events} associated with a content capture
@@ -319,7 +324,11 @@ public abstract class SmartSuggestionsService extends Service { 
     *

     * @param sessionId the id of the session to destroy

     */

    public void onDestroyInteractionSession(@NonNull InteractionSessionId sessionId) {}

    public void onDestroyInteractionSession(@NonNull InteractionSessionId sessionId) {

        if (VERBOSE) {

            Log.v(TAG, "onDestroyInteractionSession(id=" + sessionId + ")");

        }

    }



    /** @hide */

    static final class AutofillProxy {

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

+20 −7
Original line number Diff line number Diff line
@@ -40,9 +40,9 @@ import android.service.autofill.SaveRequest; 
import android.text.format.DateUtils;

import android.util.Slog;



import com.android.server.AbstractRemoteService;

import com.android.server.AbstractSinglePendingRequestRemoteService;



final class RemoteFillService extends AbstractRemoteService {

final class RemoteFillService extends AbstractSinglePendingRequestRemoteService<RemoteFillService> {



    private static final long TIMEOUT_IDLE_BIND_MILLIS = 5 * DateUtils.SECOND_IN_MILLIS;

    private static final long TIMEOUT_REMOTE_REQUEST_MILLIS = 5 * DateUtils.SECOND_IN_MILLIS;
@@ -69,8 +69,8 @@ final class RemoteFillService extends AbstractRemoteService { 
        mCallbacks = callbacks;

    }



    @Override

    protected void onConnectedStateChanged(boolean state) {

    @Override // from AbstractRemoteService

    protected void handleOnConnectedStateChanged(boolean state) {

        if (mAutoFillService == null) {

            Slog.w(mTag, "onConnectedStateChanged(): null service");

            return;
@@ -82,18 +82,18 @@ final class RemoteFillService extends AbstractRemoteService { 
        }

    }



    @Override

    @Override // from AbstractRemoteService

    protected IInterface getServiceInterface(IBinder service) {

        mAutoFillService = IAutoFillService.Stub.asInterface(service);

        return mAutoFillService;

    }



    @Override

    @Override // from AbstractRemoteService

    protected long getTimeoutIdleBindMillis() {

        return TIMEOUT_IDLE_BIND_MILLIS;

    }



    @Override

    @Override // from AbstractRemoteService

    protected long getRemoteRequestMillis() {

        return TIMEOUT_REMOTE_REQUEST_MILLIS;

    }
@@ -136,6 +136,19 @@ final class RemoteFillService extends AbstractRemoteService { 
        scheduleRequest(new PendingSaveRequest(request, this));

    }



    private boolean handleResponseCallbackCommon(

            @NonNull PendingRequest<RemoteFillService> pendingRequest) {

        if (isDestroyed()) return false;



        if (mPendingRequest == pendingRequest) {

            mPendingRequest = null;

        }

        if (mPendingRequest == null) {

            scheduleUnbind();

        }

        return true;

    }



    private void dispatchOnFillRequestSuccess(@NonNull PendingFillRequest pendingRequest,

            @Nullable FillResponse response, int requestFlags) {

        mHandler.post(() -> {

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

+1 −1
Original line number Diff line number Diff line
@@ -902,7 +902,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


    // VultureCallback

    @Override

    public void onServiceDied(AbstractRemoteService service) {

    public void onServiceDied(AbstractRemoteService<? extends AbstractRemoteService<?>> service) {

        Slog.w(TAG, "removing session because service died");

        forceRemoveSelfLocked();

    }

services/core/java/com/android/server/AbstractMultiplePendingRequestsRemoteService.java

0 → 100644
+96 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2018 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.server;



import android.annotation.NonNull;

import android.content.ComponentName;

import android.content.Context;

import android.util.Slog;



import java.io.PrintWriter;

import java.util.ArrayList;



/**

 * Base class representing a remote service that can queue multiple pending requests while not

 * bound.

 *

 * @param <S> the concrete remote service class

 *

 * @hide

 */

public abstract class AbstractMultiplePendingRequestsRemoteService<

        S extends AbstractMultiplePendingRequestsRemoteService<S>>

        extends AbstractRemoteService<S> {



    private final int mInitialCapacity;



    protected ArrayList<PendingRequest<S>> mPendingRequests;



    public AbstractMultiplePendingRequestsRemoteService(@NonNull Context context,

            @NonNull String serviceInterface, @NonNull ComponentName componentName, int userId,

            @NonNull VultureCallback callback, boolean bindInstantServiceAllowed, boolean verbose,

            int initialCapacity) {

        super(context, serviceInterface, componentName, userId, callback, bindInstantServiceAllowed,

                verbose);

        mInitialCapacity = initialCapacity;

    }



    @Override // from AbstractRemoteService

    void handlePendingRequests() {

        if (mPendingRequests != null) {

            final int size = mPendingRequests.size();

            if (mVerbose) Slog.v(mTag, "Sending " + size + " pending requests");

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

                mPendingRequests.get(i).run();

            }

            mPendingRequests = null;

        }

    }



    @Override // from AbstractRemoteService

    protected void handleOnDestroy() {

        if (mPendingRequests != null) {

            final int size = mPendingRequests.size();

            if (mVerbose) Slog.v(mTag, "Canceling " + size + " pending requests");

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

                mPendingRequests.get(i).cancel();

            }

            mPendingRequests = null;

        }

    }



    @Override // from AbstractRemoteService

    public void dump(@NonNull String prefix, @NonNull PrintWriter pw) {

        super.dump(prefix, pw);



        pw.append(prefix).append("initialCapacity=").append(String.valueOf(mInitialCapacity))

                .println();

        final int size = mPendingRequests == null ? 0 : mPendingRequests.size();

        pw.append(prefix).append("pendingRequests=").append(String.valueOf(size)).println();

    }



    @Override // from AbstractRemoteService

    void handlePendingRequestWhileUnBound(@NonNull PendingRequest<S> pendingRequest) {

        if (mPendingRequests == null) {

            mPendingRequests = new ArrayList<>(mInitialCapacity);

        }

        mPendingRequests.add(pendingRequest);

        if (mVerbose) {

            Slog.v(mTag, "queued " + mPendingRequests.size() + " requests; last=" + pendingRequest);

        }

    }

}

services/core/java/com/android/server/AbstractRemoteService.java

+62 −55
Original line number Diff line number Diff line
@@ -45,13 +45,20 @@ import java.lang.ref.WeakReference; 
 *

 * <p>All state of this class is modified on a handler thread.

 *

 * <p><b>NOTE: </b>this class should not be extended directly, you should extend either

 * {@link AbstractSinglePendingRequestRemoteService} or

 * {@link AbstractMultiplePendingRequestsRemoteService}.

 *

 * <p>See {@code com.android.server.autofill.RemoteFillService} for a concrete

 * (no pun intended) example of how to use it.

 *

 * @param <S> the concrete remote service class

 *

 * @hide

 */

//TODO(b/117779333): improve javadoc above instead of using Autofill as an example

public abstract class AbstractRemoteService implements DeathRecipient {

public abstract class AbstractRemoteService<S extends AbstractRemoteService<S>>

        implements DeathRecipient {



    private static final int MSG_UNBIND = 1;
@@ -64,8 +71,6 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
    protected final Handler mHandler;

    protected final ComponentName mComponentName;



    protected PendingRequest<? extends AbstractRemoteService> mPendingRequest;



    private final Context mContext;

    private final Intent mIntent;

    private final VultureCallback mVultureCallback;
@@ -88,10 +93,11 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
         *

         * @param service service that died!

         */

        void onServiceDied(AbstractRemoteService service);

        void onServiceDied(AbstractRemoteService<? extends AbstractRemoteService<?>> service);

    }



    public AbstractRemoteService(@NonNull Context context, @NonNull String serviceInterface,

    // NOTE: must be package-protected so this class is not extend outside

    AbstractRemoteService(@NonNull Context context, @NonNull String serviceInterface,

            @NonNull ComponentName componentName, int userId, @NonNull VultureCallback callback,

            boolean bindInstantServiceAllowed, boolean verbose) {

        mContext = context;
@@ -118,12 +124,25 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
        return mDestroyed;

    }



    private void handleOnConnectedStateChangedInternal(boolean connected) {

        if (connected) {

            handlePendingRequests();

        }

        handleOnConnectedStateChanged(connected);

    }



    /**

     * Callback called when the system connected / disconnected to the service.

     * Handles the pending requests when the connection it bounds to the remote service.

     */

    abstract void handlePendingRequests();



    /**

     * Callback called when the system connected / disconnected to the service and the pending

     * requests have been handled.

     *

     * @param state {@code true} when connected, {@code false} when disconnected.

     */

    protected void onConnectedStateChanged(boolean state) {

    protected void handleOnConnectedStateChanged(boolean state) {

    }



    /**
@@ -144,14 +163,18 @@ public abstract class AbstractRemoteService implements DeathRecipient { 


    private void handleDestroy() {

        if (checkIfDestroyed()) return;

        if (mPendingRequest != null) {

            mPendingRequest.cancel();

            mPendingRequest = null;

        }

        ensureUnbound();

        handleOnDestroy();

        handleEnsureUnbound();

        mDestroyed = true;

    }



    /**

     * Clears the state when this object is destroyed.

     *

     * <p>Typically used to cancel the pending requests.

     */

    protected abstract void handleOnDestroy();



    @Override // from DeathRecipient

    public void binderDied() {

        mHandler.sendMessage(obtainMessage(AbstractRemoteService::handleBinderDied, this));
@@ -183,9 +206,7 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
        pw.append(prefix).append(tab).append("destroyed=")

                .append(String.valueOf(mDestroyed)).println();

        pw.append(prefix).append(tab).append("bound=")

                .append(String.valueOf(isBound())).println();

        pw.append(prefix).append(tab).append("hasPendingRequest=")

                .append(String.valueOf(mPendingRequest != null)).println();

                .append(String.valueOf(handleIsBound())).println();

        pw.append(prefix).append("mBindInstantServiceAllowed=").println(mBindInstantServiceAllowed);

        pw.append(prefix).append("idleTimeout=")

            .append(Long.toString(getTimeoutIdleBindMillis() / 1000)).append("s").println();
@@ -194,7 +215,7 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
        pw.println();

    }



    protected void scheduleRequest(PendingRequest<? extends AbstractRemoteService> pendingRequest) {

    protected void scheduleRequest(@NonNull PendingRequest<S> pendingRequest) {

        mHandler.sendMessage(obtainMessage(

                AbstractRemoteService::handlePendingRequest, this, pendingRequest));

    }
@@ -215,19 +236,20 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
    private void handleUnbind() {

        if (checkIfDestroyed()) return;



        ensureUnbound();

        handleEnsureUnbound();

    }



    private void handlePendingRequest(

            PendingRequest<? extends AbstractRemoteService> pendingRequest) {

    /**

     * Handles a request, either processing it right now when bound, or saving it to be handled when

     * bound.

     */

    protected final void handlePendingRequest(@NonNull PendingRequest<S> pendingRequest) {

        if (checkIfDestroyed() || mCompleted) return;



        if (!isBound()) {

            if (mPendingRequest != null) {

                mPendingRequest.cancel();

            }

            mPendingRequest = pendingRequest;

            ensureBound();

        if (!handleIsBound()) {

            if (mVerbose) Slog.v(mTag, "handlePendingRequest(): queuing" + pendingRequest);

            handlePendingRequestWhileUnBound(pendingRequest);

            handleEnsureBound();

        } else {

            if (mVerbose) Slog.v(mTag, "handlePendingRequest(): " + pendingRequest);

            pendingRequest.run();
@@ -237,12 +259,17 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
        }

    }



    private boolean isBound() {

    /**

     * Defines what to do with a request that arrives while not bound to the service.

     */

    abstract void handlePendingRequestWhileUnBound(@NonNull PendingRequest<S> pendingRequest);



    private boolean handleIsBound() {

        return mServiceInterface != null;

    }



    private void ensureBound() {

        if (isBound() || mBinding) return;

    private void handleEnsureBound() {

        if (handleIsBound() || mBinding) return;



        if (mVerbose) Slog.v(mTag, "ensureBound()");

        mBinding = true;
@@ -265,13 +292,13 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
        }

    }



    private void ensureUnbound() {

        if (!isBound() && !mBinding) return;

    private void handleEnsureUnbound() {

        if (!handleIsBound() && !mBinding) return;



        if (mVerbose) Slog.v(mTag, "ensureUnbound()");

        mBinding = false;

        if (isBound()) {

            onConnectedStateChanged(false);

        if (handleIsBound()) {

            handleOnConnectedStateChangedInternal(false);

            if (mServiceInterface != null) {

                mServiceInterface.asBinder().unlinkToDeath(this, 0);

                mServiceInterface = null;
@@ -283,6 +310,7 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
    private class RemoteServiceConnection implements ServiceConnection {

        @Override

        public void onServiceConnected(ComponentName name, IBinder service) {

            if (mVerbose) Slog.v(mTag, "onServiceConnected()");

            if (mDestroyed || !mBinding) {

                // This is abnormal. Unbinding the connection has been requested already.

                Slog.wtf(mTag, "onServiceConnected() was dispatched after unbindService.");
@@ -296,15 +324,7 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
                handleBinderDied();

                return;

            }

            onConnectedStateChanged(true);



            if (mPendingRequest != null) {

                final PendingRequest<? extends AbstractRemoteService> pendingRequest =

                        mPendingRequest;

                mPendingRequest = null;

                handlePendingRequest(pendingRequest);

            }



            handleOnConnectedStateChangedInternal(true);

            mServiceDied = false;

        }
@@ -325,25 +345,12 @@ public abstract class AbstractRemoteService implements DeathRecipient { 
        return mDestroyed;

    }



    protected boolean handleResponseCallbackCommon(

            PendingRequest<? extends AbstractRemoteService> pendingRequest) {

        if (isDestroyed()) return false;



        if (mPendingRequest == pendingRequest) {

            mPendingRequest = null;

        }

        if (mPendingRequest == null) {

            scheduleUnbind();

        }

        return true;

    }



    /**

     * Base class for the requests serviced by the remote service.

     *

     * @param <S> the remote service class

     */

    public abstract static class PendingRequest<S extends AbstractRemoteService>

    public abstract static class PendingRequest<S extends AbstractRemoteService<S>>

            implements Runnable {

        protected final String mTag = getClass().getSimpleName();

        protected final Object mLock = new Object();