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

Commit 9f7587d1 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

NotificationEntry logging improvements 

* Pass the NotificationEntry to Loggers, rather than the SBN or key
* Use the logKey methods to ensure that notification keys with newlines are cleaned up
* This allows engineers to set NotificationUtils.INCLUDE_HASH_CODE_IN_LIST_ENTRY_LOG_KEY=true and detect instance changes from logs.

Bug: 224598080
Bug: 231337045
Bug: 236140753
Test: atest ShadeListBuilderTest NotifCollectionTest HeadsUpViewBinderTest HeadsUpManagerTest
Test: dumpsysui NotifLog NotifHeadsUpLog
Change-Id: I9324c50ffb3384e15c52d85bfe25afd637edd100
parent ecb49e9d

packages/SystemUI/src/com/android/systemui/statusbar/AlertingNotificationManager.java

+2 −2
Original line number Diff line number Diff line
@@ -66,7 +66,7 @@ public abstract class AlertingNotificationManager implements NotificationLifetim 
     * @param entry entry to show

     */

    public void showNotification(@NonNull NotificationEntry entry) {

        mLogger.logShowNotification(entry.getKey());

        mLogger.logShowNotification(entry);

        addAlertEntry(entry);

        updateNotification(entry.getKey(), true /* alert */);

        entry.setInterruption();
@@ -320,7 +320,7 @@ public abstract class AlertingNotificationManager implements NotificationLifetim 
         * @param updatePostTime whether or not to refresh the post time

         */

        public void updateEntry(boolean updatePostTime) {

            mLogger.logUpdateEntry(mEntry.getKey(), updatePostTime);

            mLogger.logUpdateEntry(mEntry, updatePostTime);



            long currentTime = mClock.currentTimeMillis();

            mEarliestRemovaltime = currentTime + mMinimumDisplayTime;

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

+8 −7
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


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



import static com.android.systemui.statusbar.notification.NotificationUtils.logKey;

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

import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
@@ -52,7 +53,7 @@ public class ListDumper { 
                    sb,

                    true,

                    includeRecordKeeping,

                    interactionTracker.hasUserInteractedWith(entry.getKey()));

                    interactionTracker.hasUserInteractedWith(logKey(entry)));

            if (entry instanceof GroupEntry) {

                GroupEntry ge = (GroupEntry) entry;

                NotificationEntry summary = ge.getSummary();
@@ -63,7 +64,7 @@ public class ListDumper { 
                            sb,

                            true,

                            includeRecordKeeping,

                            interactionTracker.hasUserInteractedWith(summary.getKey()));

                            interactionTracker.hasUserInteractedWith(logKey(summary)));

                }

                List<NotificationEntry> children = ge.getChildren();

                for (int childIndex = 0;  childIndex < children.size(); childIndex++) {
@@ -74,7 +75,7 @@ public class ListDumper { 
                            sb,

                            true,

                            includeRecordKeeping,

                            interactionTracker.hasUserInteractedWith(child.getKey()));

                            interactionTracker.hasUserInteractedWith(logKey(child)));

                }

            }

        }
@@ -116,11 +117,11 @@ public class ListDumper { 
        sb.append(indent)

                .append("[").append(index).append("] ")

                .append(index.length() == 1 ? " " : "")

                .append(entry.getKey());

                .append(logKey(entry));



        if (includeParent) {

            sb.append(" (parent=")

                    .append(entry.getParent() != null ? entry.getParent().getKey() : null)

                    .append(logKey(entry.getParent()))

                    .append(")");



            NotificationEntry notifEntry = entry.getRepresentativeEntry();
@@ -185,8 +186,8 @@ public class ListDumper { 


            if (notifEntry.getAttachState().getSuppressedChanges().getParent() != null) {

                rksb.append("suppressedParent=")

                        .append(notifEntry.getAttachState().getSuppressedChanges()

                                .getParent().getKey())

                        .append(logKey(notifEntry.getAttachState().getSuppressedChanges()

                                .getParent()))

                        .append(" ");

            }

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

+20 −22
Original line number Diff line number Diff line
@@ -267,13 +267,14 @@ public class NotifCollection implements Dumpable { 
            requireNonNull(stats);

            NotificationEntry storedEntry = mNotificationSet.get(entry.getKey());

            if (storedEntry == null) {

                mLogger.logNonExistentNotifDismissed(entry.getKey());

                mLogger.logNonExistentNotifDismissed(entry);

                continue;

            }

            if (entry != storedEntry) {

                throw mEulogizer.record(

                        new IllegalStateException("Invalid entry: "

                                + "different stored and dismissed entries for " + entry.getKey()));

                                + "different stored and dismissed entries for " + logKey(entry)

                                + " stored=@" + Integer.toHexString(storedEntry.hashCode())));

            }



            if (entry.getDismissState() == DISMISSED) {
@@ -282,7 +283,7 @@ public class NotifCollection implements Dumpable { 


            updateDismissInterceptors(entry);

            if (isDismissIntercepted(entry)) {

                mLogger.logNotifDismissedIntercepted(entry.getKey());

                mLogger.logNotifDismissedIntercepted(entry);

                continue;

            }
@@ -299,7 +300,7 @@ public class NotifCollection implements Dumpable { 
                            stats.notificationVisibility);

                } catch (RemoteException e) {

                    // system process is dead if we're here.

                    mLogger.logRemoteExceptionOnNotificationClear(entry.getKey(), e);

                    mLogger.logRemoteExceptionOnNotificationClear(entry, e);

                }

            }

        }
@@ -342,7 +343,7 @@ public class NotifCollection implements Dumpable { 
                // interceptors the chance to filter the notification

                updateDismissInterceptors(entry);

                if (isDismissIntercepted(entry)) {

                    mLogger.logNotifClearAllDismissalIntercepted(entry.getKey());

                    mLogger.logNotifClearAllDismissalIntercepted(entry);

                }

                entries.remove(i);

            }
@@ -363,7 +364,7 @@ public class NotifCollection implements Dumpable { 
            NotificationEntry entry = entries.get(i);



            entry.setDismissState(DISMISSED);

            mLogger.logNotifDismissed(entry.getKey());

            mLogger.logNotifDismissed(entry);



            if (isCanceled(entry)) {

                canceledEntries.add(entry);
@@ -416,12 +417,12 @@ public class NotifCollection implements Dumpable { 
            int reason) {

        Assert.isMainThread();



        mLogger.logNotifRemoved(sbn.getKey(), reason);

        mLogger.logNotifRemoved(sbn, reason);



        final NotificationEntry entry = mNotificationSet.get(sbn.getKey());

        if (entry == null) {

            // TODO (b/160008901): Throw an exception here

            mLogger.logNoNotificationToRemoveWithKey(sbn.getKey(), reason);

            mLogger.logNoNotificationToRemoveWithKey(sbn, reason);

            return;

        }
@@ -464,7 +465,7 @@ public class NotifCollection implements Dumpable { 
            mEventQueue.add(new BindEntryEvent(entry, sbn));

            mNotificationSet.put(sbn.getKey(), entry);



            mLogger.logNotifPosted(sbn.getKey());

            mLogger.logNotifPosted(entry);

            mEventQueue.add(new EntryAddedEvent(entry));



        } else {
@@ -483,7 +484,7 @@ public class NotifCollection implements Dumpable { 
            entry.setSbn(sbn);

            mEventQueue.add(new BindEntryEvent(entry, sbn));



            mLogger.logNotifUpdated(sbn.getKey());

            mLogger.logNotifUpdated(entry);

            mEventQueue.add(new EntryUpdatedEvent(entry, true /* fromSystem */));

        }

    }
@@ -498,12 +499,12 @@ public class NotifCollection implements Dumpable { 
        if (mNotificationSet.get(entry.getKey()) != entry) {

            throw mEulogizer.record(

                    new IllegalStateException("No notification to remove with key "

                            + entry.getKey()));

                            + logKey(entry)));

        }



        if (!isCanceled(entry)) {

            throw mEulogizer.record(

                    new IllegalStateException("Cannot remove notification " + entry.getKey()

                    new IllegalStateException("Cannot remove notification " + logKey(entry)

                            + ": has not been marked for removal"));

        }
@@ -514,7 +515,7 @@ public class NotifCollection implements Dumpable { 
        }



        if (!isLifetimeExtended(entry)) {

            mLogger.logNotifReleased(entry.getKey());

            mLogger.logNotifReleased(entry);

            mNotificationSet.remove(entry.getKey());

            cancelDismissInterception(entry);

            mEventQueue.add(new EntryRemovedEvent(entry, entry.mCancellationReason));
@@ -580,7 +581,7 @@ public class NotifCollection implements Dumpable { 
                        }

                    }

                } else {

                    mLogger.logRankingMissing(entry.getKey(), rankingMap);

                    mLogger.logRankingMissing(entry, rankingMap);

                }

            }

        }
@@ -627,10 +628,7 @@ public class NotifCollection implements Dumpable { 
                            extender.getName(), logKey, collectionEntryIs)));

        }



        mLogger.logLifetimeExtensionEnded(

                entry.getKey(),

                extender,

                entry.mLifetimeExtenders.size());

        mLogger.logLifetimeExtensionEnded(entry, extender, entry.mLifetimeExtenders.size());



        if (!isLifetimeExtended(entry)) {

            if (tryRemoveNotification(entry)) {
@@ -657,7 +655,7 @@ public class NotifCollection implements Dumpable { 
        mAmDispatchingToOtherCode = true;

        for (NotifLifetimeExtender extender : mLifetimeExtenders) {

            if (extender.maybeExtendLifetime(entry, entry.mCancellationReason)) {

                mLogger.logLifetimeExtended(entry.getKey(), extender);

                mLogger.logLifetimeExtended(entry, extender);

                entry.mLifetimeExtenders.add(extender);

            }

        }
@@ -916,17 +914,17 @@ public class NotifCollection implements Dumpable { 
        // Make sure we have the notification to update

        NotificationEntry entry = mNotificationSet.get(sbn.getKey());

        if (entry == null) {

            mLogger.logNotifInternalUpdateFailed(sbn.getKey(), name, reason);

            mLogger.logNotifInternalUpdateFailed(sbn, name, reason);

            return;

        }

        mLogger.logNotifInternalUpdate(sbn.getKey(), name, reason);

        mLogger.logNotifInternalUpdate(entry, name, reason);



        // First do the pieces of postNotification which are not about assuming the notification

        // was sent by the app

        entry.setSbn(sbn);

        mEventQueue.add(new BindEntryEvent(entry, sbn));



        mLogger.logNotifUpdated(sbn.getKey());

        mLogger.logNotifUpdated(entry);

        mEventQueue.add(new EntryUpdatedEvent(entry, false /* fromSystem */));



        // Skip the applyRanking step and go straight to dispatching the events

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

+2 −6
Original line number Diff line number Diff line
@@ -579,11 +579,7 @@ public class ShadeListBuilder implements Dumpable { 
                    if (existingSummary == null) {

                        group.setSummary(entry);

                    } else {

                        mLogger.logDuplicateSummary(

                                mIterationCount,

                                group.getKey(),

                                existingSummary.getKey(),

                                entry.getKey());

                        mLogger.logDuplicateSummary(mIterationCount, group, existingSummary, entry);



                        // Use whichever one was posted most recently

                        if (entry.getSbn().getPostTime()
@@ -1070,7 +1066,7 @@ public class ShadeListBuilder implements Dumpable { 
        if (!Objects.equals(curr, prev)) {

            mLogger.logEntryAttachStateChanged(

                    mIterationCount,

                    entry.getKey(),

                    entry,

                    prev.getParent(),

                    curr.getParent());

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

+6 −6
Original line number Diff line number Diff line
@@ -360,13 +360,13 @@ public class PreparationCoordinator implements Coordinator { 
    }



    private void abortInflation(NotificationEntry entry, String reason) {

        mLogger.logInflationAborted(entry.getKey(), reason);

        mLogger.logInflationAborted(entry, reason);

        mNotifInflater.abortInflation(entry);

        mInflatingNotifs.remove(entry);

    }



    private void onInflationFinished(NotificationEntry entry, NotifViewController controller) {

        mLogger.logNotifInflated(entry.getKey());

        mLogger.logNotifInflated(entry);

        mInflatingNotifs.remove(entry);

        mViewBarn.registerViewForEntry(entry, controller);

        mInflationStates.put(entry, STATE_INFLATED);
@@ -398,20 +398,20 @@ public class PreparationCoordinator implements Coordinator { 
            return false;

        }

        if (isBeyondGroupInitializationWindow(group, now)) {

            mLogger.logGroupInflationTookTooLong(group.getKey());

            mLogger.logGroupInflationTookTooLong(group);

            return false;

        }

        if (mInflatingNotifs.contains(group.getSummary())) {

            mLogger.logDelayingGroupRelease(group.getKey(), group.getSummary().getKey());

            mLogger.logDelayingGroupRelease(group, group.getSummary());

            return true;

        }

        for (NotificationEntry child : group.getChildren()) {

            if (mInflatingNotifs.contains(child) && !child.wasAttachedInPreviousPass()) {

                mLogger.logDelayingGroupRelease(group.getKey(), child.getKey());

                mLogger.logDelayingGroupRelease(group, child);

                return true;

            }

        }

        mLogger.logDoneWaitingForGroupInflation(group.getKey());

        mLogger.logDoneWaitingForGroupInflation(group);

        return false;

    }