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

Commit 75bb81b4 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Log when notifications in the RankingMap are not in the Collection

Test: atest NotifCollectionTest NotifCollectionInconsistencyTrackerTest
Bug: 236872736
Change-Id: Idf29cedc711b4bad5aa897c318106312f384e6ed
parent 64f07c00
Loading
Loading
Loading
Loading
+8 −15
Original line number Diff line number Diff line
@@ -88,9 +88,9 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.En
import com.android.systemui.statusbar.notification.collection.notifcollection.EntryUpdatedEvent;
import com.android.systemui.statusbar.notification.collection.notifcollection.InitEntryEvent;
import com.android.systemui.statusbar.notification.collection.notifcollection.InternalNotifUpdater;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionInconsistencyTracker;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLoggerKt;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifEvent;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;
@@ -112,7 +112,6 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;

@@ -151,6 +150,7 @@ public class NotifCollection implements Dumpable {
    private final Executor mBgExecutor;
    private final LogBufferEulogizer mEulogizer;
    private final DumpManager mDumpManager;
    private final NotifCollectionInconsistencyTracker mInconsistencyTracker;

    private final Map<String, NotificationEntry> mNotificationSet = new ArrayMap<>();
    private final Collection<NotificationEntry> mReadOnlyNotificationSet =
@@ -162,7 +162,6 @@ public class NotifCollection implements Dumpable {
    private final List<NotifLifetimeExtender> mLifetimeExtenders = new ArrayList<>();
    private final List<NotifDismissInterceptor> mDismissInterceptors = new ArrayList<>();

    private Set<String> mNotificationsWithoutRankings = Collections.emptySet();

    private Queue<NotifEvent> mEventQueue = new ArrayDeque<>();

@@ -188,6 +187,7 @@ public class NotifCollection implements Dumpable {
        mBgExecutor = bgExecutor;
        mEulogizer = logBufferEulogizer;
        mDumpManager = dumpManager;
        mInconsistencyTracker = new NotifCollectionInconsistencyTracker(mLogger);
    }

    /** Initializes the NotifCollection and registers it to receive notification events. */
@@ -199,6 +199,7 @@ public class NotifCollection implements Dumpable {
        mAttached = true;
        mDumpManager.registerDumpable(TAG, this);
        groupCoalescer.setNotificationHandler(mNotifHandler);
        mInconsistencyTracker.attach(mNotificationSet::keySet, groupCoalescer::getCoalescedKeySet);
    }

    /**
@@ -598,14 +599,9 @@ public class NotifCollection implements Dumpable {
                }
            }
        }
        NotifCollectionLoggerKt.maybeLogInconsistentRankings(
                mLogger,
                mNotificationsWithoutRankings,
                currentEntriesWithoutRankings,
                rankingMap
        );
        mNotificationsWithoutRankings = currentEntriesWithoutRankings == null
                ? Collections.emptySet() : currentEntriesWithoutRankings.keySet();

        mInconsistencyTracker.logNewMissingNotifications(rankingMap);
        mInconsistencyTracker.logNewInconsistentRankings(currentEntriesWithoutRankings, rankingMap);
        if (currentEntriesWithoutRankings != null && mNotifPipelineFlags.removeUnrankedNotifs()) {
            for (NotificationEntry entry : currentEntriesWithoutRankings.values()) {
                entry.mCancellationReason = REASON_UNKNOWN;
@@ -864,10 +860,7 @@ public class NotifCollection implements Dumpable {
                        true,
                        "\t\t"));

        pw.println("\n\tmNotificationsWithoutRankings: " + mNotificationsWithoutRankings.size());
        for (String key : mNotificationsWithoutRankings) {
            pw.println("\t * : " + key);
        }
        mInconsistencyTracker.dump(pw);
    }

    private final BatchableNotificationHandler mNotifHandler = new BatchableNotificationHandler() {
+7 −0
Original line number Diff line number Diff line
@@ -37,9 +37,11 @@ import com.android.systemui.util.time.SystemClock;

import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.inject.Inject;

@@ -116,6 +118,11 @@ public class GroupCoalescer implements Dumpable {
        mHandler = handler;
    }

    /** @return the set of notification keys currently in the coalescer */
    public Set<String> getCoalescedKeySet() {
        return Collections.unmodifiableSet(mCoalescedEvents.keySet());
    }

    private final NotificationHandler mListener = new NotificationHandler() {
        @Override
        public void onNotificationPosted(StatusBarNotification sbn, RankingMap rankingMap) {
+123 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2022 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

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

import android.service.notification.NotificationListenerService.RankingMap
import android.util.ArrayMap
import com.android.internal.annotations.VisibleForTesting
import com.android.systemui.statusbar.notification.collection.NotificationEntry
import java.io.PrintWriter

class NotifCollectionInconsistencyTracker(val logger: NotifCollectionLogger) {
    fun attach(
        collectedKeySetAccessor: () -> Set<String>,
        coalescedKeySetAccessor: () -> Set<String>,
    ) {
        if (attached) {
            throw RuntimeException("attach() called twice")
        }
        attached = true
        this.collectedKeySetAccessor = collectedKeySetAccessor
        this.coalescedKeySetAccessor = coalescedKeySetAccessor
    }

    fun logNewMissingNotifications(rankingMap: RankingMap) {
        val currentCollectedKeys = collectedKeySetAccessor()
        val currentCoalescedKeys = coalescedKeySetAccessor()
        val newMissingNotifications = rankingMap.orderedKeys.asSequence()
            .filter { it !in currentCollectedKeys }
            .filter { it !in currentCoalescedKeys }
            .toSet()
        maybeLogMissingNotifications(missingNotifications, newMissingNotifications)
        missingNotifications = newMissingNotifications
    }

    @VisibleForTesting
    fun maybeLogMissingNotifications(
        oldMissingKeys: Set<String>,
        newMissingKeys: Set<String>,
    ) {
        if (oldMissingKeys.isEmpty() && newMissingKeys.isEmpty()) return
        if (oldMissingKeys == newMissingKeys) return
        (oldMissingKeys - newMissingKeys).sorted().let { justFound ->
            if (justFound.isNotEmpty()) {
                logger.logFoundNotifications(justFound, newMissingKeys.size)
            }
        }
        (newMissingKeys - oldMissingKeys).sorted().let { goneMissing ->
            if (goneMissing.isNotEmpty()) {
                logger.logMissingNotifications(goneMissing, newMissingKeys.size)
            }
        }
    }

    fun logNewInconsistentRankings(
        currentEntriesWithoutRankings: ArrayMap<String, NotificationEntry>?,
        rankingMap: RankingMap,
    ) {
        maybeLogInconsistentRankings(
            notificationsWithoutRankings,
            currentEntriesWithoutRankings ?: emptyMap(),
            rankingMap
        )
        notificationsWithoutRankings = currentEntriesWithoutRankings?.keys ?: emptySet()
    }

    @VisibleForTesting
    fun maybeLogInconsistentRankings(
        oldKeysWithoutRankings: Set<String>,
        newEntriesWithoutRankings: Map<String, NotificationEntry>,
        rankingMap: RankingMap,
    ) {
        if (oldKeysWithoutRankings.isEmpty() && newEntriesWithoutRankings.isEmpty()) return
        if (oldKeysWithoutRankings == newEntriesWithoutRankings.keys) return
        val newlyConsistent: List<String> = oldKeysWithoutRankings
            .mapNotNull { key ->
                key.takeIf { key !in newEntriesWithoutRankings }
                    .takeIf { key in rankingMap.orderedKeys }
            }.sorted()
        if (newlyConsistent.isNotEmpty()) {
            val totalInconsistent: Int = newEntriesWithoutRankings.size
            logger.logRecoveredRankings(newlyConsistent, totalInconsistent)
        }
        val newlyInconsistent: List<NotificationEntry> = newEntriesWithoutRankings
            .mapNotNull { (key, entry) ->
                entry.takeIf { key !in oldKeysWithoutRankings }
            }.sortedBy { it.key }
        if (newlyInconsistent.isNotEmpty()) {
            val totalInconsistent: Int = newEntriesWithoutRankings.size
            logger.logMissingRankings(newlyInconsistent, totalInconsistent, rankingMap)
        }
    }

    fun dump(pw: PrintWriter) {
        pw.println("notificationsWithoutRankings: ${notificationsWithoutRankings.size}")
        for (key in notificationsWithoutRankings) {
            pw.println("\t * : $key")
        }
        pw.println("missingNotifications: ${missingNotifications.size}")
        for (key in missingNotifications) {
            pw.println("\t * : $key")
        }
    }

    private var attached: Boolean = false
    private lateinit var collectedKeySetAccessor: (() -> Set<String>)
    private lateinit var coalescedKeySetAccessor: (() -> Set<String>)
    private var notificationsWithoutRankings = emptySet<String>()
    private var missingNotifications = emptySet<String>()
}
+28 −26
Original line number Diff line number Diff line
@@ -215,8 +215,9 @@ class NotifCollectionLogger @Inject constructor(
        })
    }

    fun logRecoveredRankings(newlyConsistentKeys: List<String>) {
    fun logRecoveredRankings(newlyConsistentKeys: List<String>, totalInconsistent: Int) {
        buffer.log(TAG, INFO, {
            int1 = totalInconsistent
            int1 = newlyConsistentKeys.size
            str1 = newlyConsistentKeys.joinToString { logKey(it) ?: "null" }
        }, {
@@ -224,6 +225,32 @@ class NotifCollectionLogger @Inject constructor(
        })
    }

    fun logMissingNotifications(
        newlyMissingKeys: List<String>,
        totalMissing: Int,
    ) {
        buffer.log(TAG, WARNING, {
            int1 = totalMissing
            int2 = newlyMissingKeys.size
            str1 = newlyMissingKeys.joinToString { logKey(it) ?: "null" }
        }, {
            "Collection missing $int1 entries in ranking update. Just lost $int2: $str1"
        })
    }

    fun logFoundNotifications(
        newlyFoundKeys: List<String>,
        totalMissing: Int,
    ) {
        buffer.log(TAG, INFO, {
            int1 = totalMissing
            int2 = newlyFoundKeys.size
            str1 = newlyFoundKeys.joinToString { logKey(it) ?: "null" }
        }, {
            "Collection missing $int1 entries in ranking update. Just found $int2: $str1"
        })
    }

    fun logRemoteExceptionOnNotificationClear(entry: NotificationEntry, e: RemoteException) {
        buffer.log(TAG, WTF, {
            str1 = entry.logKey
@@ -362,29 +389,4 @@ class NotifCollectionLogger @Inject constructor(
    }
}

fun maybeLogInconsistentRankings(
    logger: NotifCollectionLogger,
    oldKeysWithoutRankings: Set<String>,
    newEntriesWithoutRankings: Map<String, NotificationEntry>?,
    rankingMap: RankingMap
) {
    if (oldKeysWithoutRankings.isEmpty() && newEntriesWithoutRankings == null) return
    val newlyConsistent: List<String> = oldKeysWithoutRankings
        .mapNotNull { key ->
            key.takeIf { key !in (newEntriesWithoutRankings ?: emptyMap()) }
                .takeIf { key in rankingMap.orderedKeys }
        }.sorted()
    if (newlyConsistent.isNotEmpty()) {
        logger.logRecoveredRankings(newlyConsistent)
    }
    val newlyInconsistent: List<NotificationEntry> = newEntriesWithoutRankings
        ?.mapNotNull { (key, entry) ->
            entry.takeIf { key !in oldKeysWithoutRankings }
        }?.sortedBy { it.key } ?: emptyList()
    if (newlyInconsistent.isNotEmpty()) {
        val totalInconsistent: Int = newEntriesWithoutRankings?.size ?: 0
        logger.logMissingRankings(newlyInconsistent, totalInconsistent, rankingMap)
    }
}

private const val TAG = "NotifCollection"
+4 −4
Original line number Diff line number Diff line
@@ -1518,7 +1518,7 @@ public class NotifCollectionTest extends SysuiTestCase {
        verify(mCollectionListener).onRankingApplied();
        verifyNoMoreInteractions(mCollectionListener);
        verify(mLogger).logMissingRankings(eq(List.of(entry1)), eq(1), any());
        verify(mLogger, never()).logRecoveredRankings(any());
        verify(mLogger, never()).logRecoveredRankings(any(), anyInt());
        clearInvocations(mCollectionListener, mLogger);

        // WHEN a ranking update includes key1 again
@@ -1530,7 +1530,7 @@ public class NotifCollectionTest extends SysuiTestCase {
        verify(mCollectionListener).onRankingApplied();
        verifyNoMoreInteractions(mCollectionListener);
        verify(mLogger, never()).logMissingRankings(any(), anyInt(), any());
        verify(mLogger).logRecoveredRankings(eq(List.of(key1)));
        verify(mLogger).logRecoveredRankings(eq(List.of(key1)), eq(0));
    }

    @Test
@@ -1556,7 +1556,7 @@ public class NotifCollectionTest extends SysuiTestCase {
        verify(mCollectionListener).onRankingApplied();
        verifyNoMoreInteractions(mCollectionListener);
        verify(mLogger).logMissingRankings(eq(List.of(entry1)), eq(1), any());
        verify(mLogger, never()).logRecoveredRankings(any());
        verify(mLogger, never()).logRecoveredRankings(any(), anyInt());
        clearInvocations(mCollectionListener, mLogger);

        // WHEN a ranking update includes key1 again
@@ -1568,7 +1568,7 @@ public class NotifCollectionTest extends SysuiTestCase {
        verify(mCollectionListener).onRankingApplied();
        verifyNoMoreInteractions(mCollectionListener);
        verify(mLogger, never()).logMissingRankings(any(), anyInt(), any());
        verify(mLogger).logRecoveredRankings(eq(List.of(key1)));
        verify(mLogger).logRecoveredRankings(eq(List.of(key1)), eq(0));
    }

    @Test
Loading