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

Commit 50b33dce authored by Felipe Leme's avatar Felipe Leme Committed by Winson Chung
Browse files

Refactored ContentCaptureService (and infra) to have just one RemoteService. 

The initial implementation of AbstractPerUserService assumed the
AbstractRemoteService instances would be created in demand, because that was
the aproach used by Autofill (to minimize the time system service is bound to
the autofill service process).

But for other systems like ContentCapture, it makes more sense to keep a
permanent connection to the remote service, which is running all the time, so
this change changes the infra to allow such permanent connection (which includes
defining an idle timeout value that never unbinds).

Bug: 117779333
Test: atest CtsContentCaptureServiceTestCases CtsAutoFillServiceTestCases

Change-Id: I43386a3fddc56f1dfd6e4e55f243eaa297921123
parent c28711c5

core/java/com/android/internal/infra/AbstractRemoteService.java

+38 −6
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import android.os.RemoteException; 
import android.os.SystemClock;

import android.os.UserHandle;

import android.util.Slog;

import android.util.TimeUtils;



import com.android.internal.annotations.GuardedBy;
@@ -64,6 +65,8 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
        I extends IInterface> implements DeathRecipient {

    private static final int MSG_UNBIND = 1;



    protected static final long PERMANENT_BOUND_TIMEOUT_MS = 0;



    protected static final int LAST_PRIVATE_MSG = MSG_UNBIND;



    // TODO(b/117779333): convert all booleans into an integer / flags
@@ -86,6 +89,9 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
    private boolean mServiceDied;

    private boolean mCompleted;



    // Used just for debugging purposes (on dump)

    private long mNextUnbind;



    /**

     * Callback called when the service dies.

     *
@@ -156,7 +162,9 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
    protected abstract I getServiceInterface(@NonNull IBinder service);



    /**

     * Defines How long after the last interaction with the service we would unbind.

     * Defines how long after the last interaction with the service we would unbind.

     *

     * @return time to unbind (in millis), or {@link #PERMANENT_BOUND_TIMEOUT_MS} to not unbind.

     */

    protected abstract long getTimeoutIdleBindMillis();
@@ -220,11 +228,23 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
                .append(mComponentName.flattenToString()).println();

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

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

        final boolean bound = handleIsBound();

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

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

                .append(String.valueOf(bound));

        final long idleTimeout = getTimeoutIdleBindMillis();

        if (bound) {

            if (idleTimeout > 0) {

                pw.append(" (unbind in : ");

                TimeUtils.formatDuration(mNextUnbind - SystemClock.elapsedRealtime(), pw);

                pw.append(")");

            } else {

                pw.append(" (permanently bound)");

            }

        }

        pw.println();

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

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

            .append(Long.toString(getTimeoutIdleBindMillis() / 1000)).append("s").println();

            .append(Long.toString(idleTimeout / 1000)).append("s").println();

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

            .append(Long.toString(getRemoteRequestMillis() / 1000)).append("s").println();

        pw.println();
@@ -236,6 +256,8 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
     * <p>This request must be responded by the service somehow (typically using a callback),

     * othewise it will trigger a {@link PendingRequest#onTimeout(AbstractRemoteService)} if the

     * service doesn't respond.

     *

     * <p><b>NOTE: </b>this request is responsible for calling {@link #scheduleUnbind()}.

     */

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

        cancelScheduledUnbind();
@@ -250,7 +272,7 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
     * a simple {@link Runnable}.

     */

    protected void scheduleAsyncRequest(@NonNull AsyncRequest<I> request) {

        cancelScheduledUnbind();

        scheduleUnbind();

        // TODO(b/117779333): fix generics below

        @SuppressWarnings({"unchecked", "rawtypes"})

        final MyAsyncPendingRequest<S, I> asyncRequest = new MyAsyncPendingRequest(this, request);
@@ -263,12 +285,21 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
    }



    protected void scheduleUnbind() {

        final long unbindDelay = getTimeoutIdleBindMillis();



        if (unbindDelay <= 0) {

            if (mVerbose) Slog.v(mTag, "not scheduling unbind when value is " + unbindDelay);

            return;

        }



        cancelScheduledUnbind();

        // TODO(b/111276913): implement "permanent binding"

        // TODO(b/117779333): make sure it's unbound if the service settings changing (right now

        // it's not)



        mNextUnbind = SystemClock.elapsedRealtime() + unbindDelay;

        if (mVerbose) Slog.v(mTag, "unbinding in " + unbindDelay + "ms: " + mNextUnbind);

        mHandler.sendMessageDelayed(obtainMessage(AbstractRemoteService::handleUnbind, this)

                .setWhat(MSG_UNBIND), getTimeoutIdleBindMillis());

                .setWhat(MSG_UNBIND), unbindDelay);

    }



    private void handleUnbind() {
@@ -342,6 +373,7 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I 
                mService = null;

            }

        }

        mNextUnbind = 0;

        mContext.unbindService(mServiceConnection);

    }

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

+1 −1
Original line number Diff line number Diff line
@@ -205,7 +205,7 @@ final class AutofillManagerServiceImpl 
    }



    @Override // from PerUserSystemService

    protected ServiceInfo newServiceInfo(@NonNull ComponentName serviceComponent)

    protected ServiceInfo newServiceInfoLocked(@NonNull ComponentName serviceComponent)

            throws NameNotFoundException {

        mInfo = new AutofillServiceInfo(getContext(), serviceComponent, mUserId);

        return mInfo.getServiceInfo();

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

+1 −3
Original line number Diff line number Diff line
@@ -47,8 +47,6 @@ final class RemoteAugmentedAutofillService 


    private static final String TAG = RemoteAugmentedAutofillService.class.getSimpleName();



    // TODO(b/117779333): changed it so it's permanentely bound

    private static final long TIMEOUT_IDLE_BIND_MILLIS = 2 * DateUtils.MINUTE_IN_MILLIS;

    private static final long TIMEOUT_REMOTE_REQUEST_MILLIS = 2 * DateUtils.SECOND_IN_MILLIS;



    RemoteAugmentedAutofillService(Context context, ComponentName serviceName,
@@ -90,7 +88,7 @@ final class RemoteAugmentedAutofillService 


    @Override // from AbstractRemoteService

    protected long getTimeoutIdleBindMillis() {

        return TIMEOUT_IDLE_BIND_MILLIS;

        return PERMANENT_BOUND_TIMEOUT_MS;

    }



    @Override // from AbstractRemoteService

services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java

+2 −2
Original line number Diff line number Diff line
@@ -76,7 +76,7 @@ public final class ContentCaptureManagerService extends 
    @Override // from AbstractMasterSystemService

    protected ContentCapturePerUserService newServiceLocked(@UserIdInt int resolvedUserId,

            boolean disabled) {

        return new ContentCapturePerUserService(this, mLock, resolvedUserId);

        return new ContentCapturePerUserService(this, mLock, disabled, resolvedUserId);

    }



    @Override // from SystemService
@@ -181,7 +181,7 @@ public final class ContentCaptureManagerService extends 
            synchronized (mLock) {

                final ContentCapturePerUserService service = getServiceForUserLocked(userId);

                service.startSessionLocked(activityToken, componentName, taskId, displayId,

                        sessionId, Binder.getCallingUid(), flags, mAllowInstantService, result);

                        sessionId, Binder.getCallingUid(), flags, result);

            }

        }

services/contentcapture/java/com/android/server/contentcapture/ContentCapturePerUserService.java

+82 −13
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import android.content.pm.ServiceInfo; 
import android.os.Bundle;

import android.os.IBinder;

import android.os.RemoteException;

import android.service.contentcapture.ContentCaptureService;

import android.service.contentcapture.SnapshotData;

import android.util.ArrayMap;

import android.util.Slog;
@@ -42,6 +43,7 @@ import android.view.contentcapture.ContentCaptureSession; 


import com.android.internal.annotations.GuardedBy;

import com.android.internal.os.IResultReceiver;

import com.android.server.contentcapture.RemoteContentCaptureService.ContentCaptureServiceCallbacks;

import com.android.server.infra.AbstractPerUserSystemService;



import java.io.PrintWriter;
@@ -52,7 +54,8 @@ import java.util.ArrayList; 
 */

final class ContentCapturePerUserService

        extends

        AbstractPerUserSystemService<ContentCapturePerUserService, ContentCaptureManagerService> {

        AbstractPerUserSystemService<ContentCapturePerUserService, ContentCaptureManagerService>

        implements ContentCaptureServiceCallbacks {



    private static final String TAG = ContentCaptureManagerService.class.getSimpleName();
@@ -60,15 +63,52 @@ final class ContentCapturePerUserService 
    private final ArrayMap<String, ContentCaptureServerSession> mSessions =

            new ArrayMap<>();



    /**

     * Reference to the remote service.

     *

     * <p>It's set in the constructor, but it's also updated when the service's updated in the

     * master's cache (for example, because a temporary service was set).

     */

    @GuardedBy("mLock")

    private RemoteContentCaptureService mRemoteService;



    // TODO(b/111276913): add mechanism to prune stale sessions, similar to Autofill's



    protected ContentCapturePerUserService(

            ContentCaptureManagerService master, Object lock, @UserIdInt int userId) {

    ContentCapturePerUserService(@NonNull ContentCaptureManagerService master,

            @NonNull Object lock, boolean disabled, @UserIdInt int userId) {

        super(master, lock, userId);



        updateRemoteServiceLocked(disabled);

    }



    /**

     * Updates the reference to the remote service.

     */

    private void updateRemoteServiceLocked(boolean disabled) {

        if (mRemoteService != null) {

            if (mMaster.debug) Slog.d(TAG, "updateRemoteService(): destroying old remote service");

            mRemoteService.destroy();

            mRemoteService = null;

        }



        // Updates the component name

        final ComponentName serviceComponentName = updateServiceInfoLocked();



        if (serviceComponentName == null) {

            Slog.w(TAG, "updateRemoteService(): no service componennt name");

            return;

        }



        if (!disabled) {

            mRemoteService = new RemoteContentCaptureService(

                  mMaster.getContext(),

                  ContentCaptureService.SERVICE_INTERFACE, serviceComponentName, mUserId, this,

                  mMaster.isBindInstantServiceAllowed(), mMaster.verbose);

        }

    }



    @Override // from PerUserSystemService

    protected ServiceInfo newServiceInfo(@NonNull ComponentName serviceComponent)

    protected ServiceInfo newServiceInfoLocked(@NonNull ComponentName serviceComponent)

            throws NameNotFoundException {



        int flags = PackageManager.GET_META_DATA;
@@ -103,14 +143,24 @@ final class ContentCapturePerUserService 
    @GuardedBy("mLock")

    protected boolean updateLocked(boolean disabled) {

        destroyLocked();

        return super.updateLocked(disabled);

        final boolean disabledStateChanged = super.updateLocked(disabled);

        updateRemoteServiceLocked(disabled);

        return disabledStateChanged;

    }



    @Override // from ContentCaptureServiceCallbacks

    public void onServiceDied(@NonNull RemoteContentCaptureService service) {

        if (mMaster.debug) Slog.d(TAG, "remote service died: " + service);

        synchronized (mLock) {

            removeSelfFromCacheLocked();

        }

    }



    // TODO(b/111276913): log metrics

    // TODO(b/119613670): log metrics

    @GuardedBy("mLock")

    public void startSessionLocked(@NonNull IBinder activityToken,

            @NonNull ComponentName componentName, int taskId, int displayId,

            @NonNull String sessionId, int uid, int flags, boolean bindInstantServiceAllowed,

            @NonNull String sessionId, int uid, int flags,

            @NonNull IResultReceiver clientReceiver) {

        if (!isEnabledLocked()) {

            setClientState(clientReceiver, ContentCaptureSession.STATE_DISABLED, /* binder=*/ null);
@@ -136,10 +186,23 @@ final class ContentCapturePerUserService 
            return;

        }



        final ContentCaptureServerSession newSession = new ContentCaptureServerSession(getContext(),

                mUserId, mLock, activityToken, this, serviceComponentName, componentName, taskId,

                displayId, sessionId, uid, flags, bindInstantServiceAllowed,

                mMaster.verbose);

        if (mRemoteService == null) {

            updateRemoteServiceLocked(/* disabled= */ false); // already checked for isEnabled

        }



        if (mRemoteService == null) {

            // TODO(b/119613670): log metrics

            Slog.w(TAG, "startSession(id=" + existingSession + ", token=" + activityToken

                    + ": ignoring because service is not set");

            // TODO(b/111276913): use a new disabled state?

            setClientState(clientReceiver, ContentCaptureSession.STATE_DISABLED,

                    /* binder=*/ null);

            return;

        }



        final ContentCaptureServerSession newSession = new ContentCaptureServerSession(

                activityToken, this, mRemoteService, componentName, taskId,

                displayId, sessionId, uid, flags);

        if (mMaster.verbose) {

            Slog.v(TAG, "startSession(): new session for "

                    + ComponentName.flattenToShortString(componentName) + " and id " + sessionId);
@@ -148,7 +211,7 @@ final class ContentCapturePerUserService 
        newSession.notifySessionStartedLocked(clientReceiver);

    }



    // TODO(b/111276913): log metrics

    // TODO(b/119613670): log metrics

    @GuardedBy("mLock")

    public void finishSessionLocked(@NonNull String sessionId) {

        if (!isEnabledLocked()) {
@@ -239,12 +302,18 @@ final class ContentCapturePerUserService 
    @Override

    protected void dumpLocked(String prefix, PrintWriter pw) {

        super.dumpLocked(prefix, pw);



        final String prefix2 = prefix + "  ";

        if (mRemoteService != null) {

            pw.print(prefix); pw.println("remote service:");

            mRemoteService.dump(prefix2, pw);

        }



        if (mSessions.isEmpty()) {

            pw.print(prefix); pw.println("no sessions");

        } else {

            final int size = mSessions.size();

            pw.print(prefix); pw.print("number sessions: "); pw.println(size);

            final String prefix2 = prefix + "  ";

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

                pw.print(prefix); pw.print("session@"); pw.println(i);

                final ContentCaptureServerSession session = mSessions.valueAt(i);