Merge "Fix overlogging of filter and section changes" into rvc-dev

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

@@ -108,6 +108,12 @@ public final class NotificationEntry extends ListEntry {
     /** If this notification was filtered out, then the filter that did the filtering. */
     @Nullable NotifFilter mExcludingFilter;
 
+    /**
+     * The NotifFilter, if any, that was active on this notification during the previous run of
+     * the list builder.
+     */
+    @Nullable NotifFilter mPreviousExcludingFilter;
+
     /** If this was a group child that was promoted to the top level, then who did the promoting. */
     @Nullable NotifPromoter mNotifPromoter;
 

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

@@ -316,6 +316,7 @@ public class ShadeListBuilder implements Dumpable {
 
         // Step 7: Lock in our group structure and log anything that's changed since the last run
         mPipelineState.incrementTo(STATE_FINALIZING);
+        logFilterChanges();
         logParentingChanges();
         freeEmptyGroups();
 
@@ -363,6 +364,9 @@ public class ShadeListBuilder implements Dumpable {
             entry.setPreviousParent(entry.getParent());
             entry.setParent(null);
 
+            entry.mPreviousExcludingFilter = entry.mExcludingFilter;
+            entry.mExcludingFilter = null;
+
             if (entry.mFirstAddedIteration == -1) {
                 entry.mFirstAddedIteration = mIterationCount;
             }
@@ -371,8 +375,10 @@ public class ShadeListBuilder implements Dumpable {
         mNotifList.clear();
     }
 
-    private void filterNotifs(Collection<? extends ListEntry> entries,
-            List<ListEntry> out, List<NotifFilter> filters) {
+    private void filterNotifs(
+            Collection<? extends ListEntry> entries,
+            List<ListEntry> out,
+            List<NotifFilter> filters) {
         final long now = mSystemClock.uptimeMillis();
         for (ListEntry entry : entries)  {
             if (entry instanceof GroupEntry) {
@@ -585,8 +591,9 @@ public class ShadeListBuilder implements Dumpable {
      * filtered out during any of the filtering steps.
      */
     private void annulAddition(ListEntry entry) {
-        entry.setSection(-1);
-        entry.mNotifSection = null;
+        // TODO: We should null out the entry's section and promoter here. However, if we do that,
+        //  future runs will think that the section changed. We need a mPreviousNotifSection,
+        //  similar to what we do for parents.
         entry.setParent(null);
         if (entry.mFirstAddedIteration == mIterationCount) {
             entry.mFirstAddedIteration = -1;
@@ -615,6 +622,17 @@ public class ShadeListBuilder implements Dumpable {
         mGroups.values().removeIf(ge -> ge.getSummary() == null && ge.getChildren().isEmpty());
     }
 
+    private void logFilterChanges() {
+        for (NotificationEntry entry : mAllEntries) {
+            if (entry.mExcludingFilter != entry.mPreviousExcludingFilter) {
+                mLogger.logFilterChanged(
+                        entry.getKey(),
+                        entry.mPreviousExcludingFilter,
+                        entry.mExcludingFilter);
+            }
+        }
+    }
+
     private void logParentingChanges() {
         for (NotificationEntry entry : mAllEntries) {
             if (entry.getParent() != entry.getPreviousParent()) {
@@ -680,21 +698,8 @@ public class ShadeListBuilder implements Dumpable {
     };
 
     private boolean applyFilters(NotificationEntry entry, long now, List<NotifFilter> filters) {
-        NotifFilter filter = findRejectingFilter(entry, now, filters);
-
-        if (filter != entry.mExcludingFilter) {
-            mLogger.logFilterChanged(
-                    entry.getKey(),
-                    entry.mExcludingFilter != null ? entry.mExcludingFilter.getName() : null,
-                    filter != null ? filter.getName() : null);
-
-            // Note that groups and summaries can also be filtered out later if they're part of a
-            // malformed group. We currently don't have a great way to track that beyond parenting
-            // change logs. Consider adding something similar to mExcludingFilter for them.
-            entry.mExcludingFilter = filter;
-        }
-
-        return filter != null;
+        entry.mExcludingFilter = findRejectingFilter(entry, now, filters);
+        return entry.mExcludingFilter != null;
     }
 
     @Nullable private static NotifFilter findRejectingFilter(NotificationEntry entry, long now,

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt

@@ -23,6 +23,7 @@ import com.android.systemui.log.LogLevel.WARNING
 import com.android.systemui.log.dagger.NotificationLog
 import com.android.systemui.statusbar.notification.collection.GroupEntry
 import com.android.systemui.statusbar.notification.collection.ListEntry
+import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter
 import javax.inject.Inject
 
 class ShadeListBuilderLogger @Inject constructor(
@@ -126,13 +127,13 @@ class ShadeListBuilderLogger @Inject constructor(
 
     fun logFilterChanged(
         key: String,
-        prevFilter: String?,
-        newFilter: String?
+        prevFilter: NotifFilter?,
+        newFilter: NotifFilter?
     ) {
         buffer.log(TAG, INFO, {
             str1 = key
-            str2 = prevFilter
-            str3 = newFilter
+            str2 = prevFilter?.name
+            str3 = newFilter?.name
         }, {
             "Filter changed for $str1: $str2 -> $str3"
         })

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

@@ -48,7 +48,7 @@ public class NotificationEntryBuilder {
 
     /* ListEntry properties */
     private GroupEntry mParent;
-    private int mSection;
+    private int mSection = -1;
 
     public NotificationEntry build() {
         StatusBarNotification sbn = mSbn != null ? mSbn : mSbnBuilder.build();