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

Commit 4258d70a authored by Ned Burns's avatar Ned Burns
Browse files

Optimistically dismiss children of dismissed groups 

When a group summary is dismissed, system server automatically dismisses
all of its children as well. To avoid flickering on sysui side while we
wait for these dismissals to come in, we mark the summary as well as its
children as "dismissed" and filter them all out immediately.

We don't actually remove the entry or its children from the
NotifCollection until we hear back from system server. This allows us to
stay as close as possible to system server's state and to know exactly
when we are deviating.

 - dismissNotification() no longer calls removeNotification(). Instead
   it just marks the notif as locally dismissed.
 - As a side-effect of the above, onEntryRemoved() is no longer called
   when a user dismisses a notification. It will be called later, when
   system server gets back to us and confirms the retraction. Removes
   the removedByUser parameter as a result.
 - Adds HideLocallyDismissedNotifsCoordinator, which filters out any
   notif that has been marked as locally dismissed.
 - User-dismissed (i.e. locally dismissed) notifications can no longer
   be lifetime extended. If the user wants 'em gone, they're gone.
 - removeNotification() renamed to tryRemoveNotification() and is now
   solely responsibly for managing lifetime extenders and removing a
   notif from the notif set if no lifetime extenders want to
   countermand.
 - onNotificationRemoved() takes on some of removeNotification()s old
   responsibilities, mainly around updating rankings.
 - Local dismissal and lifetime extension had some funky interactions;
   those should be worked out now.
 - As a side-effect of the above, we're now tracking the cancellation
   reason for all lifetime-extended notifs.

Test: atest
Bug: 112656837
Change-Id: I3c8bbf31af6c247b3f2de659266cb1f02011d710
parent 098584d2

packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java

+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());

            }

        });

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java

+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();

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java

+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"));

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java

+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,

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java

+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;