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

Commit a4f3f81e authored by Beverly's avatar Beverly
Browse files

Don't readd pending notifs to NEM's allNotifs list

Instead, we reuse the pending notification entry so we don't
unnecessarily create and retain mutliple notification entries for the same
notification.

Additionally:
 - dump all notifications kept by NEM in a dumpsys/bugreport
 - move mLeakDetector.trackGarbage(entry) to after NEM.listeners are told to
 remove their references to the entry
 - trackGarbage(entry) on pending notifications that are removed

Test: receive phone call, see HUN still appears
Test: post high priority notification (HUN) and then send immediate
update, still see HUN
Test: atest HeadUpNotificationTests
Test: atest NotificationEntryManagerTest
Test: adb shell dumpsys activity service com.android.systemui/.SystemUIService dependency DumpController NotificationEntryManager
Test: check memory usage of com.android.systemui before and after
running NexusLauncherTests (observe view count doesn't incrementally get
worse)
Fixes: 156301621

Change-Id: I60ec7847f0b8cc3f3a3ab3fbc0004991e816dc16
parent 5c82912a
Loading
Loading
Loading
Loading
+34 −24
Original line number Diff line number Diff line
@@ -151,6 +151,16 @@ public class NotificationEntryManager implements
    @Override
    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        pw.println("NotificationEntryManager state:");
        pw.println("  mAllNotifications=");
        if (mAllNotifications.size() == 0) {
            pw.println("null");
        } else {
            int i = 0;
            for (NotificationEntry entry : mAllNotifications) {
                dumpEntry(pw, "  ", i, entry);
                i++;
            }
        }
        pw.print("  mPendingNotifications=");
        if (mPendingNotifications.size() == 0) {
            pw.println("null");
@@ -350,8 +360,8 @@ public class NotificationEntryManager implements
    private final NotificationHandler mNotifListener = new NotificationHandler() {
        @Override
        public void onNotificationPosted(StatusBarNotification sbn, RankingMap rankingMap) {
            final boolean isUpdate = mActiveNotifications.containsKey(sbn.getKey());
            if (isUpdate) {
            final boolean isUpdateToInflatedNotif = mActiveNotifications.containsKey(sbn.getKey());
            if (isUpdateToInflatedNotif) {
                updateNotification(sbn, rankingMap);
            } else {
                addNotification(sbn, rankingMap);
@@ -442,16 +452,12 @@ public class NotificationEntryManager implements
                }
                if (!lifetimeExtended) {
                    // At this point, we are guaranteed the notification will be removed
                    abortExistingInflation(key, "removeNotification");
                    mAllNotifications.remove(pendingEntry);
                    mLeakDetector.trackGarbage(pendingEntry);
                }
            }
        }

        if (!lifetimeExtended) {
            abortExistingInflation(key, "removeNotification");
        }

        if (entry != null) {
        } else {
            // If a manager needs to keep the notification around for whatever reason, we
            // keep the notification
            boolean entryDismissed = entry.isRowDismissed();
@@ -469,6 +475,8 @@ public class NotificationEntryManager implements

            if (!lifetimeExtended) {
                // At this point, we are guaranteed the notification will be removed
                abortExistingInflation(key, "removeNotification");
                mAllNotifications.remove(entry);

                // Ensure any managers keeping the lifetime extended stop managing the entry
                cancelLifetimeExtension(entry);
@@ -477,13 +485,10 @@ public class NotificationEntryManager implements
                    entry.removeRow();
                }

                mAllNotifications.remove(entry);

                // Let's remove the children if this was a summary
                handleGroupSummaryRemoved(key);
                removeVisibleNotification(key);
                updateNotifications("removeNotificationInternal");
                mLeakDetector.trackGarbage(entry);
                removedByUser |= entryDismissed;

                mLogger.logNotifRemoved(entry.getKey(), removedByUser);
@@ -497,6 +502,7 @@ public class NotificationEntryManager implements
                for (NotifCollectionListener listener : mNotifCollectionListeners) {
                    listener.onEntryCleanUp(entry);
                }
                mLeakDetector.trackGarbage(entry);
            }
        }
    }
@@ -556,21 +562,26 @@ public class NotificationEntryManager implements
        Ranking ranking = new Ranking();
        rankingMap.getRanking(key, ranking);

        NotificationEntry entry = new NotificationEntry(
        NotificationEntry entry = mPendingNotifications.get(key);
        if (entry != null) {
            entry.setSbn(notification);
        } else {
            entry = new NotificationEntry(
                    notification,
                    ranking,
                    mFgsFeatureController.isForegroundServiceDismissalEnabled(),
                    SystemClock.uptimeMillis());
        for (NotifCollectionListener listener : mNotifCollectionListeners) {
            listener.onEntryBind(entry, notification);
        }
            mAllNotifications.add(entry);

            mLeakDetector.trackInstance(entry);

            for (NotifCollectionListener listener : mNotifCollectionListeners) {
                listener.onEntryInit(entry);
            }
        }

        for (NotifCollectionListener listener : mNotifCollectionListeners) {
            listener.onEntryBind(entry, notification);
        }

        // Construct the expanded view.
        if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
@@ -581,7 +592,6 @@ public class NotificationEntryManager implements
                            mInflationCallback);
        }

        abortExistingInflation(key, "addNotification");
        mPendingNotifications.put(key, entry);
        mLogger.logNotifAdded(entry.getKey());
        for (NotificationEntryListener listener : mNotificationEntryListeners) {
+1 −1
Original line number Diff line number Diff line
@@ -390,11 +390,11 @@ public class NotifCollection implements Dumpable {
        if (entry == null) {
            // A new notification!
            entry = new NotificationEntry(sbn, ranking, SystemClock.uptimeMillis());
            mEventQueue.add(new InitEntryEvent(entry));
            mEventQueue.add(new BindEntryEvent(entry, sbn));
            mNotificationSet.put(sbn.getKey(), entry);

            mLogger.logNotifPosted(sbn.getKey());
            mEventQueue.add(new InitEntryEvent(entry));
            mEventQueue.add(new EntryAddedEvent(entry));

        } else {
+22 −0
Original line number Diff line number Diff line
@@ -209,6 +209,28 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
        setUserSentiment(mSbn.getKey(), Ranking.USER_SENTIMENT_NEUTRAL);
    }

    @Test
    public void testAddNotification_noDuplicateEntriesCreated() {
        // GIVEN a notification has been added
        mEntryManager.addNotification(mSbn, mRankingMap);

        // WHEN the same notification is added multiple times before the previous entry (with
        // the same key) didn't finish inflating
        mEntryManager.addNotification(mSbn, mRankingMap);
        mEntryManager.addNotification(mSbn, mRankingMap);
        mEntryManager.addNotification(mSbn, mRankingMap);

        // THEN getAllNotifs() only contains exactly one notification with this key
        int count = 0;
        for (NotificationEntry entry : mEntryManager.getAllNotifs()) {
            if (entry.getKey().equals(mSbn.getKey())) {
                count++;
            }
        }
        assertEquals("Should only be one entry with key=" + mSbn.getKey() + " in mAllNotifs. "
                        + "Instead there are " + count, 1, count);
    }

    @Test
    public void testAddNotification_setsUserSentiment() {
        mEntryManager.addNotification(mSbn, mRankingMap);