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

Commit 7d766482 authored by Ned Burns's avatar Ned Burns Committed by Android (Google) Code Review
Browse files

Merge "Optimistically dismiss children of dismissed groups"

parents 206a1b54 4258d70a
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -89,7 +89,7 @@ public class ForegroundServiceNotificationListener {
            }

            @Override
            public void onEntryRemoved(NotificationEntry entry, int reason, boolean removedByUser) {
            public void onEntryRemoved(NotificationEntry entry, int reason) {
                removeNotification(entry.getSbn());
            }
        });
+16 −1
Original line number Diff line number Diff line
@@ -16,6 +16,9 @@

package com.android.systemui.statusbar.notification.collection;

import static com.android.systemui.statusbar.notification.collection.NotifCollection.REASON_NOT_CANCELED;
import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;

import java.util.Arrays;
import java.util.List;

@@ -132,8 +135,20 @@ public class ListDumper {
                        .append(" ");
            }

            if (notifEntry.mCancellationReason != REASON_NOT_CANCELED) {
                rksb.append("cancellationReason=")
                        .append(notifEntry.mCancellationReason)
                        .append(" ");
            }

            if (notifEntry.hasInflationError()) {
                rksb.append("hasInflationError ");
                rksb.append("(!)hasInflationError ");
            }

            if (notifEntry.getDismissState() != NOT_DISMISSED) {
                rksb.append("dismissState=")
                        .append(notifEntry.getDismissState())
                        .append(" ");
            }

            String rkString = rksb.toString();
+158 −72
Original line number Diff line number Diff line
@@ -35,9 +35,14 @@ import static android.service.notification.NotificationListenerService.REASON_TI
import static android.service.notification.NotificationListenerService.REASON_UNAUTOBUNDLED;
import static android.service.notification.NotificationListenerService.REASON_USER_STOPPED;

import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.DISMISSED;
import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.PARENT_DISMISSED;

import static java.util.Objects.requireNonNull;

import android.annotation.IntDef;
import android.annotation.MainThread;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.RemoteException;
import android.service.notification.NotificationListenerService.Ranking;
@@ -45,6 +50,8 @@ import android.service.notification.NotificationListenerService.RankingMap;
import android.service.notification.StatusBarNotification;
import android.util.ArrayMap;

import androidx.annotation.NonNull;

import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.DumpController;
import com.android.systemui.Dumpable;
@@ -172,15 +179,65 @@ public class NotifCollection implements Dumpable {
    /**
     * Dismiss a notification on behalf of the user.
     */
    void dismissNotification(
            NotificationEntry entry,
            @CancellationReason int reason,
            @NonNull DismissedByUserStats stats) {
    void dismissNotification(NotificationEntry entry, @NonNull DismissedByUserStats stats) {
        Assert.isMainThread();
        Objects.requireNonNull(stats);
        requireNonNull(stats);
        checkForReentrantCall();

        removeNotification(entry.getKey(), null, reason, stats);
        if (entry != mNotificationSet.get(entry.getKey())) {
            throw new IllegalStateException("Invalid entry: " + entry.getKey());
        }

        if (entry.getDismissState() == DISMISSED) {
            return;
        }

        // Optimistically mark the notification as dismissed -- we'll wait for the signal from
        // system server before removing it from our notification set.
        entry.setDismissState(DISMISSED);
        mLogger.logNotifDismissed(entry.getKey());

        List<NotificationEntry> canceledEntries = new ArrayList<>();

        if (isCanceled(entry)) {
            canceledEntries.add(entry);
        } else {
            // Ask system server to remove it for us
            try {
                mStatusBarService.onNotificationClear(
                        entry.getSbn().getPackageName(),
                        entry.getSbn().getTag(),
                        entry.getSbn().getId(),
                        entry.getSbn().getUser().getIdentifier(),
                        entry.getSbn().getKey(),
                        stats.dismissalSurface,
                        stats.dismissalSentiment,
                        stats.notificationVisibility);
            } catch (RemoteException e) {
                // system process is dead if we're here.
            }

            // Also mark any children as dismissed as system server will auto-dismiss them as well
            if (entry.getSbn().getNotification().isGroupSummary()) {
                for (NotificationEntry otherEntry : mNotificationSet.values()) {
                    if (otherEntry.getSbn().getGroupKey().equals(entry.getSbn().getGroupKey())
                            && otherEntry.getDismissState() != DISMISSED) {
                        otherEntry.setDismissState(PARENT_DISMISSED);
                        if (isCanceled(otherEntry)) {
                            canceledEntries.add(otherEntry);
                        }
                    }
                }
            }
        }

        // Immediately remove any dismissed notifs that have already been canceled by system server
        // (probably due to being lifetime-extended up until this point).
        for (NotificationEntry canceledEntry : canceledEntries) {
            tryRemoveNotification(canceledEntry);
        }

        rebuildList();
    }

    private void onNotificationPosted(StatusBarNotification sbn, RankingMap rankingMap) {
@@ -208,7 +265,15 @@ public class NotifCollection implements Dumpable {
        Assert.isMainThread();

        mLogger.logNotifRemoved(sbn.getKey(), reason);
        removeNotification(sbn.getKey(), rankingMap, reason, null);

        final NotificationEntry entry = mNotificationSet.get(sbn.getKey());
        if (entry == null) {
            throw new IllegalStateException("No notification to remove with key " + sbn.getKey());
        }
        entry.mCancellationReason = reason;
        applyRanking(rankingMap);
        tryRemoveNotification(entry);
        rebuildList();
    }

    private void onNotificationRankingUpdate(RankingMap rankingMap) {
@@ -242,9 +307,12 @@ public class NotifCollection implements Dumpable {
            // Update to an existing entry
            mLogger.logNotifUpdated(sbn.getKey());

            cancelLocalDismissal(entry);

            // Notification is updated so it is essentially re-added and thus alive again. Don't
            // need to keep its lifetime extended.
            cancelLifetimeExtension(entry);
            entry.mCancellationReason = REASON_NOT_CANCELED;

            entry.setSbn(sbn);
            if (rankingMap != null) {
@@ -255,59 +323,42 @@ public class NotifCollection implements Dumpable {
        }
    }

    private void removeNotification(
            String key,
            @Nullable RankingMap rankingMap,
            @CancellationReason int reason,
            @Nullable DismissedByUserStats dismissedByUserStats) {

        NotificationEntry entry = mNotificationSet.get(key);
        if (entry == null) {
            throw new IllegalStateException("No notification to remove with key " + key);
    /**
     * Tries to remove a notification from the notification set. This removal may be blocked by
     * lifetime extenders. Does not trigger a rebuild of the list; caller must do that manually.
     *
     * @return True if the notification was removed, false otherwise.
     */
    private boolean tryRemoveNotification(NotificationEntry entry) {
        if (mNotificationSet.get(entry.getKey()) != entry) {
            throw new IllegalStateException("No notification to remove with key " + entry.getKey());
        }

        entry.mLifetimeExtenders.clear();
        mAmDispatchingToOtherCode = true;
        for (NotifLifetimeExtender extender : mLifetimeExtenders) {
            if (extender.shouldExtendLifetime(entry, reason)) {
                entry.mLifetimeExtenders.add(extender);
        if (!isCanceled(entry)) {
            throw new IllegalStateException("Cannot remove notification " + entry.getKey()
                        + ": has not been marked for removal");
        }

        if (isDismissedByUser(entry)) {
            // User-dismissed notifications cannot be lifetime-extended
            cancelLifetimeExtension(entry);
        } else {
            updateLifetimeExtension(entry);
        }
        mAmDispatchingToOtherCode = false;

        if (!isLifetimeExtended(entry)) {
            mNotificationSet.remove(entry.getKey());

            if (dismissedByUserStats != null) {
                try {
                    mStatusBarService.onNotificationClear(
                            entry.getSbn().getPackageName(),
                            entry.getSbn().getTag(),
                            entry.getSbn().getId(),
                            entry.getSbn().getUser().getIdentifier(),
                            entry.getSbn().getKey(),
                            dismissedByUserStats.dismissalSurface,
                            dismissedByUserStats.dismissalSentiment,
                            dismissedByUserStats.notificationVisibility);
                } catch (RemoteException e) {
                    // system process is dead if we're here.
                }
            }

            if (rankingMap != null) {
                applyRanking(rankingMap);
            }

            dispatchOnEntryRemoved(entry, reason, dismissedByUserStats != null /* removedByUser */);
            dispatchOnEntryRemoved(entry, entry.mCancellationReason);
            dispatchOnEntryCleanUp(entry);
            return true;
        } else {
            return false;
        }

        rebuildList();
    }

    private void applyRanking(@NonNull RankingMap rankingMap) {
        for (NotificationEntry entry : mNotificationSet.values()) {
            if (!isLifetimeExtended(entry)) {
            if (!isCanceled(entry)) {

                // TODO: (b/148791039) We should crash if we are ever handed a ranking with
                //  incomplete entries. Right now, there's a race condition in NotificationListener
@@ -355,9 +406,9 @@ public class NotifCollection implements Dumpable {
        }

        if (!isLifetimeExtended(entry)) {
            // TODO: This doesn't need to be undefined -- we can set either EXTENDER_EXPIRED or
            // save the original reason
            removeNotification(entry.getKey(), null, REASON_UNKNOWN, null);
            if (tryRemoveNotification(entry)) {
                rebuildList();
            }
        }
    }

@@ -374,6 +425,31 @@ public class NotifCollection implements Dumpable {
        return entry.mLifetimeExtenders.size() > 0;
    }

    private void updateLifetimeExtension(NotificationEntry entry) {
        entry.mLifetimeExtenders.clear();
        mAmDispatchingToOtherCode = true;
        for (NotifLifetimeExtender extender : mLifetimeExtenders) {
            if (extender.shouldExtendLifetime(entry, entry.mCancellationReason)) {
                entry.mLifetimeExtenders.add(extender);
            }
        }
        mAmDispatchingToOtherCode = false;
    }

    private void cancelLocalDismissal(NotificationEntry entry) {
        if (isDismissedByUser(entry)) {
            entry.setDismissState(NOT_DISMISSED);
            if (entry.getSbn().getNotification().isGroupSummary()) {
                for (NotificationEntry otherEntry : mNotificationSet.values()) {
                    if (otherEntry.getSbn().getGroupKey().equals(entry.getSbn().getGroupKey())
                            && otherEntry.getDismissState() == PARENT_DISMISSED) {
                        otherEntry.setDismissState(NOT_DISMISSED);
                    }
                }
            }
        }
    }

    private void checkForReentrantCall() {
        if (mAmDispatchingToOtherCode) {
            throw new IllegalStateException("Reentrant call detected");
@@ -389,6 +465,19 @@ public class NotifCollection implements Dumpable {
        return ranking;
    }

    /**
     * True if the notification has been canceled by system server. Usually, such notifications are
     * immediately removed from the collection, but can sometimes stick around due to lifetime
     * extenders.
     */
    private static boolean isCanceled(NotificationEntry entry) {
        return entry.mCancellationReason != REASON_NOT_CANCELED;
    }

    private static boolean isDismissedByUser(NotificationEntry entry) {
        return entry.getDismissState() != NOT_DISMISSED;
    }

    private void dispatchOnEntryInit(NotificationEntry entry) {
        mAmDispatchingToOtherCode = true;
        for (NotifCollectionListener listener : mNotifCollectionListeners) {
@@ -421,13 +510,10 @@ public class NotifCollection implements Dumpable {
        mAmDispatchingToOtherCode = false;
    }

    private void dispatchOnEntryRemoved(
            NotificationEntry entry,
            @CancellationReason int reason,
            boolean removedByUser) {
    private void dispatchOnEntryRemoved(NotificationEntry entry, @CancellationReason int reason) {
        mAmDispatchingToOtherCode = true;
        for (NotifCollectionListener listener : mNotifCollectionListeners) {
            listener.onEntryRemoved(entry, reason, removedByUser);
            listener.onEntryRemoved(entry, reason);
        }
        mAmDispatchingToOtherCode = false;
    }
@@ -439,6 +525,20 @@ public class NotifCollection implements Dumpable {
        }
        mAmDispatchingToOtherCode = false;
    }
    @Override
    public void dump(@NonNull FileDescriptor fd, PrintWriter pw, @NonNull String[] args) {
        final List<NotificationEntry> entries = new ArrayList<>(getActiveNotifs());

        pw.println("\t" + TAG + " unsorted/unfiltered notifications:");
        if (entries.size() == 0) {
            pw.println("\t\t None");
        }
        pw.println(
                ListDumper.dumpList(
                        entries,
                        true,
                        "\t\t"));
    }

    private final BatchableNotificationHandler mNotifHandler = new BatchableNotificationHandler() {
        @Override
@@ -494,20 +594,6 @@ public class NotifCollection implements Dumpable {
    @Retention(RetentionPolicy.SOURCE)
    public @interface CancellationReason {}

    public static final int REASON_NOT_CANCELED = -1;
    public static final int REASON_UNKNOWN = 0;

    @Override
    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        final List<NotificationEntry> entries = new ArrayList<>(getActiveNotifs());

        pw.println("\t" + TAG + " unsorted/unfiltered notifications:");
        if (entries.size() == 0) {
            pw.println("\t\t None");
        }
        pw.println(
                ListDumper.dumpList(
                        entries,
                        true,
                        "\t\t"));
    }
}
+1 −2
Original line number Diff line number Diff line
@@ -98,13 +98,12 @@ public class NotifInflaterImpl implements NotifInflater {
            @Override
            public void run() {
                int dismissalSurface = NotificationStats.DISMISSAL_SHADE;
                /**
                /*
                 * TODO: determine dismissal surface (ie: shade / headsup / aod)
                 * see {@link NotificationLogger#logNotificationClear}
                 */
                mNotifCollection.dismissNotification(
                        entry,
                        0,
                        new DismissedByUserStats(
                                dismissalSurface,
                                DISMISS_SENTIMENT_NEUTRAL,
+59 −21
Original line number Diff line number Diff line
@@ -29,9 +29,11 @@ import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_NOTIFICAT
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_PEEK;
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_STATUS_BAR;

import static com.android.systemui.statusbar.notification.collection.NotifCollection.REASON_NOT_CANCELED;
import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING;

import android.annotation.NonNull;
import static java.util.Objects.requireNonNull;

import android.app.Notification;
import android.app.Notification.MessagingStyle.Message;
import android.app.NotificationChannel;
@@ -50,6 +52,7 @@ import android.util.ArraySet;
import android.view.View;
import android.widget.ImageView;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.android.internal.annotations.VisibleForTesting;
@@ -59,6 +62,7 @@ import com.android.internal.util.ContrastColorUtil;
import com.android.systemui.statusbar.InflationTask;
import com.android.systemui.statusbar.StatusBarIconView;
import com.android.systemui.statusbar.notification.InflationException;
import com.android.systemui.statusbar.notification.collection.NotifCollection.CancellationReason;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;
@@ -105,9 +109,18 @@ public final class NotificationEntry extends ListEntry {
    /** If this was a group child that was promoted to the top level, then who did the promoting. */
    @Nullable NotifPromoter mNotifPromoter;

    /** If this notification had an issue with inflating. Only used with the NewNotifPipeline **/
    /**
     * If this notification was cancelled by system server, then the reason that was supplied.
     * Uncancelled notifications always have REASON_NOT_CANCELED. Note that lifetime-extended
     * notifications will have this set even though they are still in the active notification set.
     */
    @CancellationReason int mCancellationReason = REASON_NOT_CANCELED;

    /** @see #hasInflationError() */
    private boolean mHasInflationError;

    /** @see #getDismissState() */
    @NonNull private DismissState mDismissState = DismissState.NOT_DISMISSED;

    /*
    * Old members
@@ -169,9 +182,9 @@ public final class NotificationEntry extends ListEntry {
    public NotificationEntry(
            @NonNull StatusBarNotification sbn,
            @NonNull Ranking ranking) {
        super(Objects.requireNonNull(Objects.requireNonNull(sbn).getKey()));
        super(requireNonNull(requireNonNull(sbn).getKey()));

        Objects.requireNonNull(ranking);
        requireNonNull(ranking);

        mKey = sbn.getKey();
        setSbn(sbn);
@@ -201,8 +214,8 @@ public final class NotificationEntry extends ListEntry {
     * TODO: Make this package-private
     */
    public void setSbn(@NonNull StatusBarNotification sbn) {
        Objects.requireNonNull(sbn);
        Objects.requireNonNull(sbn.getKey());
        requireNonNull(sbn);
        requireNonNull(sbn.getKey());

        if (!sbn.getKey().equals(mKey)) {
            throw new IllegalArgumentException("New key " + sbn.getKey()
@@ -227,8 +240,8 @@ public final class NotificationEntry extends ListEntry {
     * TODO: Make this package-private
     */
    public void setRanking(@NonNull Ranking ranking) {
        Objects.requireNonNull(ranking);
        Objects.requireNonNull(ranking.getKey());
        requireNonNull(ranking);
        requireNonNull(ranking.getKey());

        if (!ranking.getKey().equals(mKey)) {
            throw new IllegalArgumentException("New key " + ranking.getKey()
@@ -238,6 +251,34 @@ public final class NotificationEntry extends ListEntry {
        mRanking = ranking;
    }

    /*
     * Bookkeeping getters and setters
     */

    /**
     * Whether this notification had an error when attempting to inflate. This is only used in
     * the NewNotifPipeline
     */
    public boolean hasInflationError() {
        return mHasInflationError;
    }

    void setHasInflationError(boolean hasError) {
        mHasInflationError = hasError;
    }

    /**
     * Set if the user has dismissed this notif but we haven't yet heard back from system server to
     * confirm the dismissal.
     */
    @NonNull public DismissState getDismissState() {
        return mDismissState;
    }

    void setDismissState(@NonNull DismissState dismissState) {
        mDismissState = requireNonNull(dismissState);
    }

    /*
     * Convenience getters for SBN and Ranking members
     */
@@ -275,7 +316,6 @@ public final class NotificationEntry extends ListEntry {
        return mRanking.canBubble();
    }


    public @NonNull List<Notification.Action> getSmartActions() {
        return mRanking.getSmartActions();
    }
@@ -578,18 +618,6 @@ public final class NotificationEntry extends ListEntry {
        remoteInputTextWhenReset = null;
    }

    void setHasInflationError(boolean hasError) {
        mHasInflationError = hasError;
    }

    /**
     * Whether this notification had an error when attempting to inflate. This is only used in
     * the NewNotifPipeline
     */
    public boolean hasInflationError() {
        return mHasInflationError;
    }

    public void setHasSentReply() {
        hasSentReply = true;
    }
@@ -974,6 +1002,16 @@ public final class NotificationEntry extends ListEntry {
        }
    }

    /** @see #getDismissState() */
    public enum DismissState {
        /** User has not dismissed this notif or its parent */
        NOT_DISMISSED,
        /** User has dismissed this notif specifically */
        DISMISSED,
        /** User has dismissed this notif's parent (which implicitly dismisses this one as well) */
        PARENT_DISMISSED,
    }

    private static final long LAUNCH_COOLDOWN = 2000;
    private static final long REMOTE_INPUT_COOLDOWN = 500;
    private static final long INITIALIZATION_DELAY = 400;
Loading