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

Commit cb5e62cc 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: 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: Ia6d6dbc442fe28832f1ed70b35ca64278a871237
parent 18fb866d

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java

+33 −21
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,17 +562,24 @@ 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());

            mAllNotifications.add(entry);

            mLeakDetector.trackInstance(entry);

        }



        abortExistingInflation(key, "addNotification");



        for (NotifCollectionListener listener : mNotifCollectionListeners) {

            listener.onEntryBind(entry, notification);

        }

        mAllNotifications.add(entry);



        mLeakDetector.trackInstance(entry);



        for (NotifCollectionListener listener : mNotifCollectionListeners) {

            listener.onEntryInit(entry);
@@ -581,7 +594,6 @@ public class NotificationEntryManager implements 
                            mInflationCallback);

        }



        abortExistingInflation(key, "addNotification");

        mPendingNotifications.put(key, entry);

        mLogger.logNotifAdded(entry.getKey());

        for (NotificationEntryListener listener : mNotificationEntryListeners) {

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java

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