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

Commit 46a8d25f authored by Evan Laird's avatar Evan Laird Committed by android-build-team Robot
Browse files

Force FGS notifications to show for a minimum time

It's possible for a service to do a start/stop foreground and cause a
couple of things to happen:

NotificationManagerService will enqueue a EnqueueNotificationRunnable,
post a PostNotificationRunnable (for the startForeground), and then also
enqueue a CancelNotificationRunnable. There is some racy behavior here
in that the cancel runnable can get triggered in between enqueue and
post runnables. If the cancel happens first, then
NotificationListenerServices will never get the message.

This behavior is technically allowed, however for foreground services we
want to ensure that there is a minmum amount of time that notification
listeners are aware of the foreground service so that (for instance) the
FGS notification can be shown.

This CL does two things to mitigate this problem:

1. Introduce checking in the CancelNotificationRunnable such that it
will not cancel until after PostNotificationRunnable has finished
executing.

2. Introduce a NotificationLifetimeExtender method that will allow a
lifetime extender to manage the lifetime of a notification that has been
enqueued but not inflated yet.

Bug: 119041698
Test: atest NotificationManagerServiceTest
Test: atest ForegroundServiceNotificationListenerTest
Change-Id: I0680034ed9315aa2c05282524d48faaed066ebd0
Merged-In: I0680034ed9315aa2c05282524d48faaed066ebd0
(cherry picked from commit 5136eefe)
parent 9d19f716
Loading
Loading
Loading
Loading
+90 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2019 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.systemui;

import android.annotation.NonNull;
import android.app.Notification;
import android.os.Handler;
import android.os.Looper;
import android.util.ArraySet;

import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.statusbar.NotificationLifetimeExtender;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;

/**
 * Extends the lifetime of foreground notification services such that they show for at least
 * five seconds
 */
public class ForegroundServiceLifetimeExtender implements NotificationLifetimeExtender {

    private static final String TAG = "FGSLifetimeExtender";
    @VisibleForTesting
    static final int MIN_FGS_TIME_MS = 5000;

    private NotificationSafeToRemoveCallback mNotificationSafeToRemoveCallback;
    private ArraySet<NotificationEntry> mManagedEntries = new ArraySet<>();
    private Handler mHandler = new Handler(Looper.getMainLooper());

    public ForegroundServiceLifetimeExtender() {
    }

    @Override
    public void setCallback(@NonNull NotificationSafeToRemoveCallback callback) {
        mNotificationSafeToRemoveCallback = callback;
    }

    @Override
    public boolean shouldExtendLifetime(@NonNull NotificationEntry entry) {
        if ((entry.notification.getNotification().flags
                & Notification.FLAG_FOREGROUND_SERVICE) == 0) {
            return false;
        }

        long currentTime = System.currentTimeMillis();
        return currentTime - entry.notification.getPostTime() < MIN_FGS_TIME_MS;
    }

    @Override
    public boolean shouldExtendLifetimeForPendingNotification(
            @NonNull NotificationEntry entry) {
        return shouldExtendLifetime(entry);
    }

    @Override
    public void setShouldManageLifetime(
            @NonNull NotificationEntry entry, boolean shouldManage) {
        if (!shouldManage) {
            mManagedEntries.remove(entry);
            return;
        }

        mManagedEntries.add(entry);

        Runnable r = () -> {
            if (mManagedEntries.contains(entry)) {
                mManagedEntries.remove(entry);
                if (mNotificationSafeToRemoveCallback != null) {
                    mNotificationSafeToRemoveCallback.onSafeToRemove(entry.key);
                }
            }
        };
        long delayAmt = MIN_FGS_TIME_MS
                - (System.currentTimeMillis() - entry.notification.getPostTime());
        mHandler.postDelayed(r, delayAmt);
    }
}
+3 −0
Original line number Diff line number Diff line
@@ -66,6 +66,9 @@ public class ForegroundServiceNotificationListener {
                removeNotification(entry.notification);
            }
        });

        notificationEntryManager.addNotificationLifetimeExtender(
                new ForegroundServiceLifetimeExtender());
    }

    /**
+12 −0
Original line number Diff line number Diff line
@@ -26,6 +26,18 @@ public interface NotificationLifetimeExtender {
     */
    boolean shouldExtendLifetime(@NonNull NotificationEntry entry);

    /**
     * It's possible that a notification was canceled before it ever became visible. This callback
     * gives lifetime extenders a chance to make sure it shows up. For example if a foreground
     * service is canceled too quickly but we still want to make sure a FGS notification shows.
     * @param pendingEntry the canceled (but pending) entry
     * @return true if the notification lifetime should be extended
     */
    default boolean shouldExtendLifetimeForPendingNotification(
            @NonNull NotificationEntry pendingEntry) {
        return false;
    }

    /**
     * Sets whether or not the lifetime should be managed by the extender.  In practice, if
     * shouldManage is true, this is where the extender starts managing the entry internally and is
+16 −2
Original line number Diff line number Diff line
@@ -281,10 +281,24 @@ public class NotificationEntryManager implements
        }

        final NotificationEntry entry = mNotificationData.get(key);
        boolean lifetimeExtended = false;

        abortExistingInflation(key);
        // Notification was canceled before it got inflated
        if (entry == null) {
            NotificationEntry pendingEntry = mPendingNotifications.get(key);
            if (pendingEntry != null) {
                for (NotificationLifetimeExtender extender : mNotificationLifetimeExtenders) {
                    if (extender.shouldExtendLifetimeForPendingNotification(pendingEntry)) {
                        extendLifetime(pendingEntry, extender);
                        lifetimeExtended = true;
                    }
                }
            }
        }

        boolean lifetimeExtended = false;
        if (!lifetimeExtended) {
            abortExistingInflation(key);
        }

        if (entry != null) {
            // If a manager needs to keep the notification around for whatever reason, we
+1 −1
Original line number Diff line number Diff line
@@ -224,7 +224,7 @@ public class StatusBarNotificationPresenter implements NotificationPresenter,
            mVisualStabilityManager.setUpWithPresenter(this);
            mGutsManager.setUpWithPresenter(this,
                    notifListContainer, mCheckSaveListener, mOnSettingsClickListener);
            // ForegroundServiceControllerListener adds its listener in its constructor
            // ForegroundServiceNotificationListener adds its listener in its constructor
            // but we need to request it here in order for it to be instantiated.
            // TODO: figure out how to do this correctly once Dependency.get() is gone.
            Dependency.get(ForegroundServiceNotificationListener.class);
Loading