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

Commit d97aab04 authored by Ned Burns's avatar Ned Burns
Browse files

Revamp how changes are tracked in ShadeListBuilder

In general, we want to log whenever something changes between runs of
the ShadeListBuilder: when a notif is added, when a notif changes
sections, when it gets promoted to top-level, etc.

Knowing that something has changed (and how it has changed) is not
as easy to detect as it once was: a notif might be added early in the
pipeline but removed later on, resulting in no real change. A notif
might have a section assigned to it but have it set back to null
because its associated group gets destroyed later on. And so on.

To better track these changes, this CL packages up all of the "list
state" that we care about (parent, filter, section, etc) into a simple
object, ListAttachState. Every ListEntry has two attach states: the
current one and the previous one. Whenever we finish a run of the list
builder, we iterate over every ListEntry and check to see if its attach
state now differs from its previous attach state. If it does, we log the
differences.

Test: atest, manual
Bug: 112656837
Change-Id: I9f44ed05a455ebac2706c18bc1718a9697e805ce
parent 0460375f
Loading
Loading
Loading
Loading
+81 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2020 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

import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSection

/**
 * Stores the state that [ShadeListBuilder] assigns to this [ListEntry]
 */
data class ListAttachState private constructor(
    /**
     * Null if not attached to the current shade list. If top-level, then the shade list root. If
     * part of a group, then that group's GroupEntry.
     */
    var parent: GroupEntry?,

    /**
     * The section that this ListEntry was sorted into. If the child of the group, this will be the
     * parent's section. Null if not attached to the list.
     */
    var section: NotifSection?,
    var sectionIndex: Int,

    /**
     * If a [NotifFilter] is excluding this entry from the list, then that filter. Always null for
     * [GroupEntry]s.
     */
    var excludingFilter: NotifFilter?,

    /**
     * The [NotifPromoter] promoting this entry to top-level, if any. Always null for [GroupEntry]s.
     */
    var promoter: NotifPromoter?
) {

    /** Copies the state of another instance. */
    fun clone(other: ListAttachState) {
        parent = other.parent
        section = other.section
        sectionIndex = other.sectionIndex
        excludingFilter = other.excludingFilter
        promoter = other.promoter
    }

    /** Resets back to a "clean" state (the same as created by the factory method) */
    fun reset() {
        parent = null
        section = null
        sectionIndex = -1
        excludingFilter = null
        promoter = null
    }

    companion object {
        @JvmStatic
        fun create(): ListAttachState {
            return ListAttachState(
                    null,
                    null,
                    -1,
                    null,
                    null)
        }
    }
}
+6 −6
Original line number Diff line number Diff line
@@ -102,11 +102,11 @@ public class ListDumper {
                    .append(")");
        }

        if (entry.mNotifSection != null) {
        if (entry.getNotifSection() != null) {
            sb.append(" sectionIndex=")
                    .append(entry.getSection())
                    .append(" sectionName=")
                    .append(entry.mNotifSection.getName());
                    .append(entry.getNotifSection().getName());
        }

        if (includeRecordKeeping) {
@@ -133,15 +133,15 @@ public class ListDumper {
                        .append(" ");
            }

            if (notifEntry.mExcludingFilter != null) {
            if (notifEntry.getExcludingFilter() != null) {
                rksb.append("filter=")
                        .append(notifEntry.mExcludingFilter)
                        .append(notifEntry.getExcludingFilter().getName())
                        .append(" ");
            }

            if (notifEntry.mNotifPromoter != null) {
            if (notifEntry.getNotifPromoter() != null) {
                rksb.append("promoter=")
                        .append(notifEntry.mNotifPromoter)
                        .append(notifEntry.getNotifPromoter().getName())
                        .append(" ");
            }

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

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

import android.annotation.Nullable;

import androidx.annotation.Nullable;

import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSection;

@@ -27,13 +28,11 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga
public abstract class ListEntry {
    private final String mKey;

    @Nullable private GroupEntry mParent;
    @Nullable private GroupEntry mPreviousParent;
    @Nullable NotifSection mNotifSection;

    private int mSection = -1;
    int mFirstAddedIteration = -1;

    private final ListAttachState mPreviousAttachState = ListAttachState.create();
    private final ListAttachState mAttachState = ListAttachState.create();

    ListEntry(String key) {
        mKey = key;
    }
@@ -51,27 +50,40 @@ public abstract class ListEntry {
    public abstract @Nullable NotificationEntry getRepresentativeEntry();

    @Nullable public GroupEntry getParent() {
        return mParent;
        return mAttachState.getParent();
    }

    void setParent(@Nullable GroupEntry parent) {
        mParent = parent;
        mAttachState.setParent(parent);
    }

    @Nullable public GroupEntry getPreviousParent() {
        return mPreviousParent;
    }

    void setPreviousParent(@Nullable GroupEntry previousParent) {
        mPreviousParent = previousParent;
        return mPreviousAttachState.getParent();
    }

    /** The section this notification was assigned to (0 to N-1, where N is number of sections). */
    public int getSection() {
        return mSection;
        return mAttachState.getSectionIndex();
    }

    @Nullable public NotifSection getNotifSection() {
        return mAttachState.getSection();
    }

    void setSection(int section) {
        mSection = section;
    ListAttachState getAttachState() {
        return mAttachState;
    }

    ListAttachState getPreviousAttachState() {
        return mPreviousAttachState;
    }

    /**
     * Stores the current attach state into {@link #getPreviousAttachState()}} and then starts a
     * fresh attach state (all entries will be null/default-initialized).
     */
    void beginNewAttachState() {
        mPreviousAttachState.clone(mAttachState);
        mAttachState.reset();
    }
}
+8 −12
Original line number Diff line number Diff line
@@ -105,18 +105,6 @@ public final class NotificationEntry extends ListEntry {
    /** List of dismiss interceptors that are intercepting the dismissal of this notification. */
    final List<NotifDismissInterceptor> mDismissInterceptors = new ArrayList<>();

    /** 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;

    /**
     * If this notification was cancelled by system server, then the reason that was supplied.
     * Uncancelled notifications always have REASON_NOT_CANCELED. Note that lifetime-extended
@@ -283,6 +271,14 @@ public final class NotificationEntry extends ListEntry {
        mDismissState = requireNonNull(dismissState);
    }

    @Nullable public NotifFilter getExcludingFilter() {
        return getAttachState().getExcludingFilter();
    }

    @Nullable public NotifPromoter getNotifPromoter() {
        return getAttachState().getPromoter();
    }

    /*
     * Convenience getters for SBN and Ranking members
     */
+70 −66
Original line number Diff line number Diff line
@@ -58,6 +58,7 @@ import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import javax.inject.Inject;
import javax.inject.Singleton;
@@ -314,8 +315,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();
        logChanges();
        freeEmptyGroups();

        // Step 8: Dispatch the new list, first to any listeners and then to the view layer
@@ -325,7 +325,10 @@ public class ShadeListBuilder implements Dumpable {
        }

        // Step 9: We're done!
        mLogger.logEndBuildList(mIterationCount, mReadOnlyNotifList.size());
        mLogger.logEndBuildList(
                mIterationCount,
                mReadOnlyNotifList.size(),
                countChildren(mReadOnlyNotifList));
        if (mIterationCount % 10 == 0) {
            mLogger.logFinalList(mNotifList);
        }
@@ -352,18 +355,13 @@ public class ShadeListBuilder implements Dumpable {

    private void resetNotifs() {
        for (GroupEntry group : mGroups.values()) {
            group.setPreviousParent(group.getParent());
            group.setParent(null);
            group.beginNewAttachState();
            group.clearChildren();
            group.setSummary(null);
        }

        for (NotificationEntry entry : mAllEntries) {
            entry.setPreviousParent(entry.getParent());
            entry.setParent(null);

            entry.mPreviousExcludingFilter = entry.mExcludingFilter;
            entry.mExcludingFilter = null;
            entry.beginNewAttachState();

            if (entry.mFirstAddedIteration == -1) {
                entry.mFirstAddedIteration = mIterationCount;
@@ -590,10 +588,10 @@ public class ShadeListBuilder implements Dumpable {
     * filtered out during any of the filtering steps.
     */
    private void annulAddition(ListEntry entry) {
        // 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);
        entry.getAttachState().setSectionIndex(-1);
        entry.getAttachState().setSection(null);
        entry.getAttachState().setPromoter(null);
        if (entry.mFirstAddedIteration == mIterationCount) {
            entry.mFirstAddedIteration = -1;
        }
@@ -606,8 +604,8 @@ public class ShadeListBuilder implements Dumpable {
            if (entry instanceof GroupEntry) {
                GroupEntry parent = (GroupEntry) entry;
                for (NotificationEntry child : parent.getChildren()) {
                    child.mNotifSection = sectionWithIndex.first;
                    child.setSection(sectionWithIndex.second);
                    child.getAttachState().setSection(sectionWithIndex.first);
                    child.getAttachState().setSectionIndex(sectionWithIndex.second);
                }
                parent.sortChildren(sChildComparator);
            }
@@ -621,39 +619,52 @@ public class ShadeListBuilder implements Dumpable {
        mGroups.values().removeIf(ge -> ge.getSummary() == null && ge.getChildren().isEmpty());
    }

    private void logFilterChanges() {
    private void logChanges() {
        for (NotificationEntry entry : mAllEntries) {
            if (entry.mExcludingFilter != entry.mPreviousExcludingFilter) {
                mLogger.logFilterChanged(
                        mIterationCount,
                        entry.getKey(),
                        entry.mPreviousExcludingFilter,
                        entry.mExcludingFilter);
            logAttachStateChanges(entry);
        }
        for (GroupEntry group : mGroups.values()) {
            logAttachStateChanges(group);
        }
    }

    private void logParentingChanges() {
        for (NotificationEntry entry : mAllEntries) {
            if (entry.getParent() != entry.getPreviousParent()) {
                mLogger.logParentChanged(
    private void logAttachStateChanges(ListEntry entry) {

        final ListAttachState curr = entry.getAttachState();
        final ListAttachState prev = entry.getPreviousAttachState();

        if (!Objects.equals(curr, prev)) {
            mLogger.logEntryAttachStateChanged(
                    mIterationCount,
                    entry.getKey(),
                        entry.getPreviousParent() == null
                                ? null : entry.getPreviousParent().getKey(),
                        entry.getParent() == null
                                ? null : entry.getParent().getKey());
                    prev.getParent(),
                    curr.getParent());

            if (curr.getParent() != prev.getParent()) {
                mLogger.logParentChanged(mIterationCount, prev.getParent(), curr.getParent());
            }

            if (curr.getExcludingFilter() != prev.getExcludingFilter()) {
                mLogger.logFilterChanged(
                        mIterationCount, prev.getExcludingFilter(), curr.getExcludingFilter());
            }
        for (GroupEntry group : mGroups.values()) {
            if (group.getParent() != group.getPreviousParent()) {
                mLogger.logParentChanged(

            // When something gets detached, its promoter and section are always set to null, so
            // don't bother logging those changes.
            final boolean wasDetached = curr.getParent() == null && prev.getParent() != null;

            if (!wasDetached && curr.getPromoter() != prev.getPromoter()) {
                mLogger.logPromoterChanged(
                        mIterationCount, prev.getPromoter(), curr.getPromoter());
            }

            if (!wasDetached && curr.getSection() != prev.getSection()) {
                mLogger.logSectionChanged(
                        mIterationCount,
                        group.getKey(),
                        group.getPreviousParent() == null
                                ? null : group.getPreviousParent().getKey(),
                        group.getParent() == null
                                ? null : group.getParent().getKey());
                        prev.getSection(),
                        prev.getSectionIndex(),
                        curr.getSection(),
                        curr.getSectionIndex());
            }
        }
    }
@@ -700,8 +711,9 @@ public class ShadeListBuilder implements Dumpable {
    };

    private boolean applyFilters(NotificationEntry entry, long now, List<NotifFilter> filters) {
        entry.mExcludingFilter = findRejectingFilter(entry, now, filters);
        return entry.mExcludingFilter != null;
        final NotifFilter filter = findRejectingFilter(entry, now, filters);
        entry.getAttachState().setExcludingFilter(filter);
        return filter != null;
    }

    @Nullable private static NotifFilter findRejectingFilter(NotificationEntry entry, long now,
@@ -719,16 +731,7 @@ public class ShadeListBuilder implements Dumpable {

    private boolean applyTopLevelPromoters(NotificationEntry entry) {
        NotifPromoter promoter = findPromoter(entry);

        if (promoter != entry.mNotifPromoter) {
            mLogger.logPromoterChanged(
                    mIterationCount,
                    entry.getKey(),
                    entry.mNotifPromoter != null ? entry.mNotifPromoter.getName() : null,
                    promoter != null ? promoter.getName() : null);
            entry.mNotifPromoter = promoter;
        }

        entry.getAttachState().setPromoter(promoter);
        return promoter != null;
    }

@@ -747,18 +750,8 @@ public class ShadeListBuilder implements Dumpable {
        final NotifSection section = sectionWithIndex.first;
        final Integer sectionIndex = sectionWithIndex.second;

        if (section != entry.mNotifSection) {
            mLogger.logSectionChanged(
                    mIterationCount,
                    entry.getKey(),
                    entry.mNotifSection != null ? entry.mNotifSection.getName() : null,
                    entry.getSection(),
                    section.getName(),
                    sectionIndex);

            entry.mNotifSection = section;
            entry.setSection(sectionIndex);
        }
        entry.getAttachState().setSection(section);
        entry.getAttachState().setSectionIndex(sectionIndex);

        return sectionWithIndex;
    }
@@ -780,6 +773,17 @@ public class ShadeListBuilder implements Dumpable {
        }
    }

    private static int countChildren(List<ListEntry> entries) {
        int count = 0;
        for (int i = 0; i < entries.size(); i++) {
            final ListEntry entry = entries.get(i);
            if (entry instanceof GroupEntry) {
                count += ((GroupEntry) entry).getChildren().size();
            }
        }
        return count;
    }

    private void dispatchOnBeforeTransformGroups(List<ListEntry> entries) {
        for (int i = 0; i < mOnBeforeTransformGroupsListeners.size(); i++) {
            mOnBeforeTransformGroupsListeners.get(i).onBeforeTransformGroups(entries);
Loading