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

Commit d7a03922 authored by Jeff DeCew's avatar Jeff DeCew Committed by Android (Google) Code Review
Browse files

Merge "Log when notifications in the RankingMap are not in the Collection" into tm-qpr-dev

parents a9b3416a 75bb81b4
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, PipelineDumpable {
    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, PipelineDumpable {
    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, PipelineDumpable {
        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, PipelineDumpable {
        mAttached = true;
        mDumpManager.registerDumpable(TAG, this);
        groupCoalescer.setNotificationHandler(mNotifHandler);
        mInconsistencyTracker.attach(mNotificationSet::keySet, groupCoalescer::getCoalescedKeySet);
    }

    /**
@@ -598,14 +599,9 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
                }
            }
        }
        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, PipelineDumpable {
                        true,
                        "\t\t"));

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

    @Override
+7 −0
Original line number Diff line number Diff line
@@ -39,9 +39,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;

@@ -118,6 +120,11 @@ public class GroupCoalescer implements Dumpable, PipelineDumpable {
        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