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

Commit 9e095e0e authored by Bernardo Rufino's avatar Bernardo Rufino
Browse files

Toast on service notification trampolines

To achieve this we have to propagate the originating token from the
pending intent. Once we have it on ServiceRecord there is additional
logic to propagate this token to ProcessRecord, where it's taken into
consideration for triggering or not notification manager's callback that
shows the toast introduced in ag/12184026.

In ServiceRecord, we only want to pass a non-null token to ProcessRecord
if it's the only token that's enabling bg activity starts. Rememeber
that service notification trampolines are started services, so if there
is an exemption due to binding, we don't progate the token. If the
exemption is only due to starts we only propagate the non-null token
if it's the only token allowing starts and there is no other grant
without an originating token. This logic is on
getExclusiveOriginatingToken().

When we get an exemption for bg activity starts we post a delayed
callback (10s) to revoke the grant. When we get another exemption with
an outstanding callback, we used to remove it first than repost the
callback (renewing the grace period). Since now we need to keep track of
the originating tokens for each grace period (eg. if there are 2 stacked
exemptions we need to consider both tokens to pass it to ProcessRecord
according to logic in paragraph above). Because of this we don't remove
the callback anymore, instead we check on the callback the outstanding
possibly null tokens, if the callback is for the last token then we
execute the same logic as before (which means only the last callback
effectively does what it used to do before, keeping the old behavior),
but if it's not the last we simply update our originating token with
ProcessRecord. This way we correctly attribute the grant to the
originating token for each grace period.

Test: atest BackgroundActivityLaunchTest
Test: Post a service-trampoline notification, click on it and observe
      toast
Test: Start an app A service from foreground app B, then before 10s
      click on a service-trampoline notification for same service,
      observe no toast is shown, after the first 10s passed, try again
      and observe toast.
Bug: 167676448
Change-Id: Ibb32bb42200cca6eb77d52ad0022225a55393e00
parent faead856
Loading
Loading
Loading
Loading
+4 −3
Original line number Diff line number Diff line
@@ -457,13 +457,14 @@ public final class ActiveServices {
            String callingPackage, @Nullable String callingFeatureId, final int userId)
            throws TransactionTooLargeException {
        return startServiceLocked(caller, service, resolvedType, callingPid, callingUid, fgRequired,
                hideFgNotification, callingPackage, callingFeatureId, userId, false);
                hideFgNotification, callingPackage, callingFeatureId, userId, false, null);
    }

    ComponentName startServiceLocked(IApplicationThread caller, Intent service, String resolvedType,
            int callingPid, int callingUid, boolean fgRequired, boolean hideFgNotification,
            String callingPackage, @Nullable String callingFeatureId, final int userId,
            boolean allowBackgroundActivityStarts) throws TransactionTooLargeException {
            boolean allowBackgroundActivityStarts, @Nullable IBinder backgroundActivityStartsToken)
            throws TransactionTooLargeException {
        if (DEBUG_DELAYED_STARTS) Slog.v(TAG_SERVICE, "startService: " + service
                + " type=" + resolvedType + " args=" + service.getExtras());

@@ -707,7 +708,7 @@ public final class ActiveServices {
            }
        }
        if (allowBackgroundActivityStarts) {
            r.allowBgActivityStartsOnServiceStart();
            r.allowBgActivityStartsOnServiceStart(backgroundActivityStartsToken);
        }
        ComponentName cmp = startServiceInnerLocked(smap, service, r, callerFg, addToStarting);
        return cmp;
+2 −3
Original line number Diff line number Diff line
@@ -133,7 +133,6 @@ import static com.android.server.wm.ActivityTaskManagerService.DUMP_STARTER_CMD;
import static com.android.server.wm.ActivityTaskManagerService.RELAUNCH_REASON_NONE;
import static com.android.server.wm.ActivityTaskManagerService.relaunchReasonToString;
import android.Manifest;
import android.Manifest.permission;
import android.annotation.BroadcastBehavior;
@@ -342,7 +341,6 @@ import com.android.server.PackageWatchdog;
import com.android.server.ServiceThread;
import com.android.server.SystemConfig;
import com.android.server.SystemService;
import com.android.server.SystemService.TargetUser;
import com.android.server.SystemServiceManager;
import com.android.server.ThreadPriorityBooster;
import com.android.server.UserspaceRebootLogger;
@@ -17730,7 +17728,8 @@ public class ActivityManagerService extends IActivityManager.Stub
                try {
                    res = mServices.startServiceLocked(null, service,
                            resolvedType, -1, uid, fgRequired, false, callingPackage,
                            callingFeatureId, userId, allowBackgroundActivityStarts);
                            callingFeatureId, userId, allowBackgroundActivityStarts,
                            backgroundActivityStartsToken);
                } finally {
                    Binder.restoreCallingIdentity(origId);
                }
+1 −1
Original line number Diff line number Diff line
@@ -1686,7 +1686,7 @@ public final class BroadcastQueue {
        // that request - we don't want the token to be swept from under our feet...
        mHandler.removeCallbacksAndMessages(msgToken);
        // ...then add the token
        proc.addAllowBackgroundActivityStartsToken(r, r.mBackgroundActivityStartsToken);
        proc.addOrUpdateAllowBackgroundActivityStartsToken(r, r.mBackgroundActivityStartsToken);
    }

    final void setBroadcastTimeoutLocked(long timeoutTime) {
+7 −4
Original line number Diff line number Diff line
@@ -77,6 +77,7 @@ import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;

/**
@@ -1338,13 +1339,15 @@ class ProcessRecord implements WindowProcessListener {
     * {@param originatingToken} if you have one such originating token, this is useful for tracing
     * back the grant in the case of the notification token.
     */
    void addAllowBackgroundActivityStartsToken(Binder entity, @Nullable IBinder originatingToken) {
        if (entity == null) return;
        mWindowProcessController.addAllowBackgroundActivityStartsToken(entity, originatingToken);
    void addOrUpdateAllowBackgroundActivityStartsToken(Binder entity,
            @Nullable IBinder originatingToken) {
        Objects.requireNonNull(entity);
        mWindowProcessController.addOrUpdateAllowBackgroundActivityStartsToken(entity,
                originatingToken);
    }

    void removeAllowBackgroundActivityStartsToken(Binder entity) {
        if (entity == null) return;
        Objects.requireNonNull(entity);
        mWindowProcessController.removeAllowBackgroundActivityStartsToken(entity);
    }

+74 −25
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import static android.app.PendingIntent.FLAG_UPDATE_CURRENT;
import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM;
import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME;

import android.annotation.Nullable;
import android.app.Notification;
import android.app.PendingIntent;
import android.content.ComponentName;
@@ -139,6 +140,12 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
    // used to clean up the state of mIsAllowedBgActivityStartsByStart after a timeout
    private Runnable mCleanUpAllowBgActivityStartsByStartCallback;
    private ProcessRecord mAppForAllowingBgActivityStartsByStart;
    // These are the originating tokens that currently allow bg activity starts by service start.
    // This is used to trace back the grant when starting activities. We only pass such token to the
    // ProcessRecord if it's the *only* cause for bg activity starts exemption, otherwise we pass
    // null.
    @GuardedBy("ams")
    private List<IBinder> mBgActivityStartsByStartOriginatingTokens = new ArrayList<>();

    // allow while-in-use permissions in foreground service or not.
    // while-in-use permissions in FGS started from background might be restricted.
@@ -588,7 +595,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
                    ? _proc : null;
            if (mIsAllowedBgActivityStartsByStart
                    || mIsAllowedBgActivityStartsByBinding) {
                _proc.addAllowBackgroundActivityStartsToken(this, null);
                _proc.addOrUpdateAllowBackgroundActivityStartsToken(this,
                        getExclusiveOriginatingToken());
            } else {
                _proc.removeAllowBackgroundActivityStartsToken(this);
            }
@@ -679,11 +687,9 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
    }

    void setAllowedBgActivityStartsByBinding(boolean newValue) {
        if (mIsAllowedBgActivityStartsByBinding != newValue) {
        mIsAllowedBgActivityStartsByBinding = newValue;
        updateParentProcessBgActivityStartsToken();
    }
    }

    /**
     * Called when the service is started with allowBackgroundActivityStarts set. We allow
@@ -691,7 +697,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
     * timeout. Note that the ability for starting background activities persists for the process
     * even if the service is subsequently stopped.
     */
    void allowBgActivityStartsOnServiceStart() {
    void allowBgActivityStartsOnServiceStart(@Nullable IBinder originatingToken) {
        mBgActivityStartsByStartOriginatingTokens.add(originatingToken);
        setAllowedBgActivityStartsByStart(true);
        if (app != null) {
            mAppForAllowingBgActivityStartsByStart = app;
@@ -701,33 +708,50 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
        if (mCleanUpAllowBgActivityStartsByStartCallback == null) {
            mCleanUpAllowBgActivityStartsByStartCallback = () -> {
                synchronized (ams) {
                    mBgActivityStartsByStartOriginatingTokens.remove(0);
                    if (!mBgActivityStartsByStartOriginatingTokens.isEmpty()) {
                        // There are other callbacks in the queue, let's just update the originating
                        // token
                        if (mIsAllowedBgActivityStartsByStart) {
                            mAppForAllowingBgActivityStartsByStart
                                    .addOrUpdateAllowBackgroundActivityStartsToken(
                                            this, getExclusiveOriginatingToken());
                        } else {
                            Slog.wtf(TAG,
                                    "Service callback to revoke bg activity starts by service "
                                            + "start triggered but "
                                            + "mIsAllowedBgActivityStartsByStart = false. This "
                                            + "should never happen.");
                        }
                    } else {
                        // Last callback on the queue
                        if (app == mAppForAllowingBgActivityStartsByStart) {
                            // The process we allowed is still running the service. We remove
                        // the ability by start, but it may still be allowed via bound connections.
                            // the ability by start, but it may still be allowed via bound
                            // connections.
                            setAllowedBgActivityStartsByStart(false);
                        } else if (mAppForAllowingBgActivityStartsByStart != null) {
                        // The process we allowed is not running the service. It therefore can't be
                        // bound so we can unconditionally remove the ability.
                            // The process we allowed is not running the service. It therefore can't
                            // be bound so we can unconditionally remove the ability.
                            mAppForAllowingBgActivityStartsByStart
                                    .removeAllowBackgroundActivityStartsToken(ServiceRecord.this);
                        }
                        mAppForAllowingBgActivityStartsByStart = null;
                    }
                }
            };
        }

        // if there's a request pending from the past, drop it before scheduling a new one
        ams.mHandler.removeCallbacks(mCleanUpAllowBgActivityStartsByStartCallback);
        // Existing callbacks will only update the originating token, only when the last callback is
        // executed is the grant revoked.
        ams.mHandler.postDelayed(mCleanUpAllowBgActivityStartsByStartCallback,
                ams.mConstants.SERVICE_BG_ACTIVITY_START_TIMEOUT);
    }

    private void setAllowedBgActivityStartsByStart(boolean newValue) {
        if (mIsAllowedBgActivityStartsByStart != newValue) {
        mIsAllowedBgActivityStartsByStart = newValue;
        updateParentProcessBgActivityStartsToken();
    }
    }

    /**
     * Whether the process this service runs in should be temporarily allowed to start
@@ -736,8 +760,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
     * {@code mIsAllowedBgActivityStartsByBinding}. If either is true, this ServiceRecord
     * should be contributing as a token in parent ProcessRecord.
     *
     * @see com.android.server.am.ProcessRecord#addAllowBackgroundActivityStartsToken(Binder,
     * IBinder)
     * @see com.android.server.am.ProcessRecord#addOrUpdateAllowBackgroundActivityStartsToken(
     * Binder, IBinder)
     * @see com.android.server.am.ProcessRecord#removeAllowBackgroundActivityStartsToken(Binder)
     */
    private void updateParentProcessBgActivityStartsToken() {
@@ -747,12 +771,37 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
        if (mIsAllowedBgActivityStartsByStart || mIsAllowedBgActivityStartsByBinding) {
            // if the token is already there it's safe to "re-add it" - we're dealing with
            // a set of Binder objects
            app.addAllowBackgroundActivityStartsToken(this, null);
            app.addOrUpdateAllowBackgroundActivityStartsToken(this, getExclusiveOriginatingToken());
        } else {
            app.removeAllowBackgroundActivityStartsToken(this);
        }
    }

    /**
     * Returns the originating token if that's the only reason background activity starts are
     * allowed. In order for that to happen the service has to be allowed only due to starts, since
     * bindings are not associated with originating tokens, and all the start tokens have to be the
     * same and there can't be any null originating token in the queue.
     *
     * Originating tokens are optional, so the caller could provide null when it allows bg activity
     * starts.
     */
    @Nullable
    private IBinder getExclusiveOriginatingToken() {
        if (mIsAllowedBgActivityStartsByBinding
                || mBgActivityStartsByStartOriginatingTokens.isEmpty()) {
            return null;
        }
        IBinder firstToken = mBgActivityStartsByStartOriginatingTokens.get(0);
        for (int i = 1, n = mBgActivityStartsByStartOriginatingTokens.size(); i < n; i++) {
            IBinder token = mBgActivityStartsByStartOriginatingTokens.get(i);
            if (token != firstToken) {
                return null;
            }
        }
        return firstToken;
    }

    @GuardedBy("ams")
    void updateKeepWarmLocked() {
        mKeepWarming = ams.mConstants.KEEP_WARMING_SERVICES.contains(name)
Loading