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

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

Merge "New Pipeline: Fix groups not grouping when shade is open."

parents c00b880d 3850e10f
Loading
Loading
Loading
Loading
+24 −13
Original line number Diff line number Diff line
@@ -30,11 +30,6 @@ data class ListAttachState private constructor(
     */
    var parent: GroupEntry?,

    /**
     * Identifies the notification order in the entire notification list
     */
    var stableIndex: Int = -1,

    /**
     * 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.
@@ -52,6 +47,11 @@ data class ListAttachState private constructor(
     */
    var promoter: NotifPromoter?,

    /**
     * If an entry's group was pruned from the list by NotifPipeline logic, the reason is here.
     */
    var groupPruneReason: String?,

    /**
     * If the [VisualStabilityManager] is suppressing group or section changes for this entry,
     * suppressedChanges will contain the new parent or section that we would have assigned to
@@ -60,14 +60,23 @@ data class ListAttachState private constructor(
    var suppressedChanges: SuppressedAttachState
) {

    /**
     * Identifies the notification order in the entire notification list.
     * NOTE: this property is intentionally excluded from equals calculation (by not making it a
     *  constructor arg) because its value changes based on the presence of other members in the
     *  list, rather than anything having to do with this entry's attachment.
     */
    var stableIndex: Int = -1

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

    /** Resets back to a "clean" state (the same as created by the factory method) */
@@ -76,8 +85,9 @@ data class ListAttachState private constructor(
        section = null
        excludingFilter = null
        promoter = null
        stableIndex = -1
        groupPruneReason = null
        suppressedChanges.reset()
        stableIndex = -1
    }

    companion object {
@@ -85,11 +95,12 @@ data class ListAttachState private constructor(
        fun create(): ListAttachState {
            return ListAttachState(
                null,
                    -1,
                null,
                null,
                null,
                SuppressedAttachState.create())
                null,
                SuppressedAttachState.create()
            )
        }
    }
}
+0 −10
Original line number Diff line number Diff line
@@ -31,8 +31,6 @@ public abstract class ListEntry {
    private final String mKey;
    private final long mCreationTime;

    int mFirstAddedIteration = -1;

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

@@ -94,14 +92,6 @@ public abstract class ListEntry {
        return mPreviousAttachState;
    }

    /**
     * True if this entry has been attached to the shade at least once in its lifetime (it may not
     * currently be attached).
     */
    public boolean hasBeenAttachedBefore() {
        return mFirstAddedIteration != -1;
    }

    /**
     * Stores the current attach state into {@link #getPreviousAttachState()}} and then starts a
     * fresh attach state (all entries will be null/default-initialized).
+172 −70
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

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

import static com.android.internal.util.Preconditions.checkArgument;
import static com.android.internal.util.Preconditions.checkState;
import static com.android.systemui.statusbar.notification.collection.GroupEntry.ROOT_ENTRY;
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_BUILD_STARTED;
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_FINALIZE_FILTERING;
@@ -34,6 +36,7 @@ import android.annotation.MainThread;
import android.annotation.Nullable;
import android.os.Trace;
import android.util.ArrayMap;
import android.util.ArraySet;

import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
@@ -50,6 +53,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.OnBefo
import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeTransformGroupsListener;
import com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState;
import com.android.systemui.statusbar.notification.collection.listbuilder.ShadeListBuilderLogger;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.DefaultNotifStabilityManager;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Invalidator;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
@@ -72,6 +76,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import javax.inject.Inject;

@@ -103,7 +108,7 @@ public class ShadeListBuilder implements Dumpable {
    private final List<NotifFilter> mNotifFinalizeFilters = new ArrayList<>();
    private final List<NotifComparator> mNotifComparators = new ArrayList<>();
    private final List<NotifSection> mNotifSections = new ArrayList<>();
    @Nullable private NotifStabilityManager mNotifStabilityManager;
    private NotifStabilityManager mNotifStabilityManager;

    private final List<OnBeforeTransformGroupsListener> mOnBeforeTransformGroupsListeners =
            new ArrayList<>();
@@ -228,7 +233,7 @@ public class ShadeListBuilder implements Dumpable {
        mNotifSections.add(new NotifSection(DEFAULT_SECTIONER, mNotifSections.size()));
    }

    void setNotifStabilityManager(NotifStabilityManager notifStabilityManager) {
    void setNotifStabilityManager(@NonNull NotifStabilityManager notifStabilityManager) {
        Assert.isMainThread();
        mPipelineState.requireState(STATE_IDLE);

@@ -244,6 +249,14 @@ public class ShadeListBuilder implements Dumpable {
        mNotifStabilityManager.setInvalidationListener(this::onReorderingAllowedInvalidated);
    }

    @NonNull
    private NotifStabilityManager getStabilityManager() {
        if (mNotifStabilityManager == null) {
            return DefaultNotifStabilityManager.INSTANCE;
        }
        return mNotifStabilityManager;
    }

    void setComparators(List<NotifComparator> comparators) {
        Assert.isMainThread();
        mPipelineState.requireState(STATE_IDLE);
@@ -460,10 +473,6 @@ public class ShadeListBuilder implements Dumpable {

        for (NotificationEntry entry : mAllEntries) {
            entry.beginNewAttachState();

            if (entry.mFirstAddedIteration == -1) {
                entry.mFirstAddedIteration = mIterationCount;
            }
        }

        mNotifList.clear();
@@ -519,7 +528,6 @@ public class ShadeListBuilder implements Dumpable {
                GroupEntry group = mGroups.get(topLevelKey);
                if (group == null) {
                    group = new GroupEntry(topLevelKey, mSystemClock.uptimeMillis());
                    group.mFirstAddedIteration = mIterationCount;
                    mGroups.put(topLevelKey, group);
                }
                if (group.getParent() == null) {
@@ -569,7 +577,7 @@ public class ShadeListBuilder implements Dumpable {
    }

    private void stabilizeGroupingNotifs(List<ListEntry> topLevelList) {
        if (mNotifStabilityManager == null) {
        if (getStabilityManager().isEveryChangeAllowed()) {
            return;
        }
        Trace.beginSection("ShadeListBuilder.stabilizeGroupingNotifs");
@@ -612,7 +620,7 @@ public class ShadeListBuilder implements Dumpable {
        final GroupEntry prevParent = entry.getPreviousAttachState().getParent();
        final GroupEntry assignedParent = entry.getParent();
        if (prevParent != assignedParent
                && !mNotifStabilityManager.isGroupChangeAllowed(entry.getRepresentativeEntry())) {
                && !getStabilityManager().isGroupChangeAllowed(entry.getRepresentativeEntry())) {
            entry.getAttachState().getSuppressedChanges().setParent(assignedParent);
            entry.setParent(prevParent);
            if (prevParent == ROOT_ENTRY) {
@@ -655,62 +663,159 @@ public class ShadeListBuilder implements Dumpable {

    private void pruneIncompleteGroups(List<ListEntry> shadeList) {
        Trace.beginSection("ShadeListBuilder.pruneIncompleteGroups");
        // Any group which lost a child on this run to stability is exempt from being pruned or
        //  having its summary promoted, regardless of how many children it has
        Set<String> groupsWithChildrenLostToStability =
                getGroupsWithChildrenLostToStability(shadeList);
        for (int i = 0; i < shadeList.size(); i++) {
            final ListEntry tle = shadeList.get(i);

            if (tle instanceof GroupEntry) {
                final GroupEntry group = (GroupEntry) tle;
                final List<NotificationEntry> children = group.getRawChildren();
                final boolean hasSummary = group.getSummary() != null;

                if (hasSummary && children.size() == 0) {
                    if (groupsWithChildrenLostToStability.contains(group.getKey())) {
                        // This group lost a child on this run to stability, so it is exempt from
                        //  having its summary promoted to the top level, so prune it.
                        //  It has no children, so it will just vanish.
                        pruneGroupAtIndexAndPromoteAnyChildren(shadeList, group, i);
                    } else {
                        // For any other summary with no children, promote the summary.
                        pruneGroupAtIndexAndPromoteSummary(shadeList, group, i);
                    }

                    i--;  // The node we visited is gone, so be sure to visit this index again.
                } else if (!hasSummary) {
                    // If the group doesn't provide a summary, ignore it and add
                    //  any children it may have directly to top-level.
                    pruneGroupAtIndexAndPromoteAnyChildren(shadeList, group, i);

                    i--;  // The node we visited is gone, so be sure to visit this index again.
                } else if (children.size() < MIN_CHILDREN_FOR_GROUP) {
                    // This group has a summary and insufficient, but nonzero children.
                    checkState(hasSummary, "group must have summary at this point");
                    checkState(!children.isEmpty(), "empty group should have been promoted");

                    if (groupsWithChildrenLostToStability.contains(group.getKey())) {
                        // This group lost a child on this run to stability, so it is exempt from
                        //  the "min children" requirement; keep it around in case more children are
                        //  added before changes are allowed again.
                        group.getAttachState().getSuppressedChanges().setWasPruneSuppressed(true);
                        continue;
                    }
                    if (group.wasAttachedInPreviousPass()
                            && !getStabilityManager().isGroupChangeAllowed(group.getSummary())) {
                        checkState(!children.isEmpty(), "empty group should have been pruned");
                        // This group was previously attached and group changes aren't
                        //  allowed; keep it around until group changes are allowed again.
                        group.getAttachState().getSuppressedChanges().setWasPruneSuppressed(true);
                        continue;
                    }

                    // The group is too small, ignore it and add
                    // its children (if any) directly to top-level.
                    pruneGroupAtIndexAndPromoteAnyChildren(shadeList, group, i);

                    i--;  // The node we visited is gone, so be sure to visit this index again.
                }
            }
        }
        Trace.endSection();
    }

    private void pruneGroupAtIndexAndPromoteSummary(List<ListEntry> shadeList,
            GroupEntry group, int index) {
        // Validate that the group has no children
        checkArgument(group.getChildren().isEmpty(), "group should have no children");

                if (group.getSummary() != null && children.size() == 0) {
        NotificationEntry summary = group.getSummary();
        summary.setParent(ROOT_ENTRY);
        // The list may be sorted; replace the group with the summary, in its place
                    shadeList.set(i, summary);
        ListEntry oldEntry = shadeList.set(index, summary);

        // Validate that the replaced entry was the group entry
        checkState(oldEntry == group);

        group.setSummary(null);
        annulAddition(group, shadeList);

                    i--;  // The node we visited is gone, so be sure to visit this index again.
                } else if (group.getSummary() == null
                        || children.size() < MIN_CHILDREN_FOR_GROUP) {

                    if (group.getSummary() != null
                            && group.wasAttachedInPreviousPass()
                            && mNotifStabilityManager != null
                            && !mNotifStabilityManager.isGroupChangeAllowed(group.getSummary())) {
                        // if this group was previously attached and group changes aren't
                        // allowed, keep it around until group changes are allowed again
                        group.getAttachState().getSuppressedChanges().setWasPruneSuppressed(true);
                        continue;
        summary.getAttachState().setGroupPruneReason(
                "SUMMARY with no children @ " + mPipelineState.getStateName());
    }

                    // If the group doesn't provide a summary or is too small, ignore it and add
                    // its children (if any) directly to top-level.
    private void pruneGroupAtIndexAndPromoteAnyChildren(List<ListEntry> shadeList,
            GroupEntry group, int index) {
        // REMOVE the GroupEntry at this index
        ListEntry oldEntry = shadeList.remove(index);

        // Validate that the replaced entry was the group entry
        checkState(oldEntry == group);

                    shadeList.remove(i);
        List<NotificationEntry> children = group.getRawChildren();
        boolean hasSummary = group.getSummary() != null;

                    if (group.getSummary() != null) {
        // Remove the group summary, if present, and leave detached.
        if (hasSummary) {
            final NotificationEntry summary = group.getSummary();
            group.setSummary(null);
            annulAddition(summary, shadeList);
            summary.getAttachState().setGroupPruneReason(
                    "SUMMARY with too few children @ " + mPipelineState.getStateName());
        }

        // Promote any children
        if (!children.isEmpty()) {
            // create the reason we will report on the child for why its group was pruned.
            String childReason = hasSummary
                    ? ("CHILD with " + (children.size() - 1) + " siblings @ "
                        + mPipelineState.getStateName())
                    : ("CHILD with no summary @ " + mPipelineState.getStateName());

            // Remove children from the group and add them to the shadeList.
            for (int j = 0; j < children.size(); j++) {
                final NotificationEntry child = children.get(j);
                child.setParent(ROOT_ENTRY);
                        // The list may be sorted, so add the children in order where the group was.
                        shadeList.add(i + j, child);
                child.getAttachState().setGroupPruneReason(requireNonNull(childReason));
            }
            // The list may be sorted, so add the children in order where the group was.
            shadeList.addAll(index, children);
            children.clear();
        }

        annulAddition(group, shadeList);
    }

                    i--;  // The node we visited is gone, so be sure to visit this index again.
    /**
     * Collect the keys of any groups which have already lost a child to stability this run.
     *
     * If stability is being enforced, then {@link #stabilizeGroupingNotifs(List)} might have
     * detached some children from their groups and left them at the top level because the child was
     * previously attached at the top level.  Doing so would set the
     * {@link SuppressedAttachState#getParent() suppressed parent} for the current attach state.
     *
     * If we've already removed a child from this group, we don't want to remove any more children
     * from the group (even if that would leave only a single notification in the group) because
     * that could cascade over multiple runs and allow a large group of notifications all show up as
     * top level (ungrouped) notifications.
     */
    @NonNull
    private Set<String> getGroupsWithChildrenLostToStability(List<ListEntry> shadeList) {
        if (getStabilityManager().isEveryChangeAllowed()) {
            return Collections.emptySet();
        }
        ArraySet<String> groupsWithChildrenLostToStability = new ArraySet<>();
        for (int i = 0; i < shadeList.size(); i++) {
            final ListEntry tle = shadeList.get(i);
            final GroupEntry suppressedParent =
                    tle.getAttachState().getSuppressedChanges().getParent();
            if (suppressedParent != null) {
                // This top-level-entry was supposed to be attached to this group,
                //  so mark the group as having lost a child to stability.
                groupsWithChildrenLostToStability.add(suppressedParent.getKey());
            }
        }
        Trace.endSection();
        return groupsWithChildrenLostToStability;
    }

    /**
@@ -727,10 +832,9 @@ public class ShadeListBuilder implements Dumpable {
        // lot of them), it will put the system into an inconsistent state. So we check all of them
        // here.

        if (entry.getParent() == null || entry.mFirstAddedIteration == -1) {
        if (entry.getParent() == null) {
            throw new IllegalStateException(
                    "Cannot nullify addition of " + entry.getKey() + ": no such addition. ("
                            + entry.getParent() + " " + entry.mFirstAddedIteration + ")");
                    "Cannot nullify addition of " + entry.getKey() + ": no parent.");
        }

        if (entry.getParent() == ROOT_ENTRY) {
@@ -771,9 +875,6 @@ public class ShadeListBuilder implements Dumpable {
        entry.setParent(null);
        entry.getAttachState().setSection(null);
        entry.getAttachState().setPromoter(null);
        if (entry.mFirstAddedIteration == mIterationCount) {
            entry.mFirstAddedIteration = -1;
        }
    }

    private void assignSections() {
@@ -804,12 +905,12 @@ public class ShadeListBuilder implements Dumpable {
        assignIndexes(mNotifList);

        // Check for suppressed order changes
        if (!mNotifStabilityManager.isEveryChangeAllowed()) {
        if (!getStabilityManager().isEveryChangeAllowed()) {
            mForceReorderable = true;
            boolean isSorted = isSorted(mNotifList, mTopLevelComparator);
            mForceReorderable = false;
            if (!isSorted) {
                mNotifStabilityManager.onEntryReorderSuppressed();
                getStabilityManager().onEntryReorderSuppressed();
            }
        }
        Trace.endSection();
@@ -897,12 +998,26 @@ public class ShadeListBuilder implements Dumpable {
                        curr.getParent());
            }

            if (curr.getSuppressedChanges().getSection() != null) {
                mLogger.logSectionChangeSuppressed(
                        mIterationCount,
                        curr.getSuppressedChanges().getSection(),
                        curr.getSection());
            }

            if (curr.getSuppressedChanges().getWasPruneSuppressed()) {
                mLogger.logGroupPruningSuppressed(
                        mIterationCount,
                        curr.getParent());
            }

            if (!Objects.equals(curr.getGroupPruneReason(), prev.getGroupPruneReason())) {
                mLogger.logPrunedReasonChanged(
                        mIterationCount,
                        prev.getGroupPruneReason(),
                        curr.getGroupPruneReason());
            }

            if (curr.getExcludingFilter() != prev.getExcludingFilter()) {
                mLogger.logFilterChanged(
                        mIterationCount,
@@ -927,20 +1042,11 @@ public class ShadeListBuilder implements Dumpable {
                        prev.getSection(),
                        curr.getSection());
            }

            if (curr.getSuppressedChanges().getSection() != null) {
                mLogger.logSectionChangeSuppressed(
                        mIterationCount,
                        curr.getSuppressedChanges().getSection(),
                        curr.getSection());
            }
        }
    }

    private void onBeginRun() {
        if (mNotifStabilityManager != null) {
            mNotifStabilityManager.onBeginRun();
        }
        getStabilityManager().onBeginRun();
    }

    private void cleanupPluggables() {
@@ -953,9 +1059,7 @@ public class ShadeListBuilder implements Dumpable {
            mNotifSections.get(i).getSectioner().onCleanup();
        }

        if (mNotifStabilityManager != null) {
            callOnCleanup(List.of(mNotifStabilityManager));
        }
        callOnCleanup(List.of(getStabilityManager()));
    }

    private void callOnCleanup(List<? extends Pluggable<?>> pluggables) {
@@ -1015,7 +1119,7 @@ public class ShadeListBuilder implements Dumpable {
    private boolean mForceReorderable = false;

    private boolean canReorder(ListEntry entry) {
        return mForceReorderable || mNotifStabilityManager.isEntryReorderingAllowed(entry);
        return mForceReorderable || getStabilityManager().isEntryReorderingAllowed(entry);
    }

    private boolean applyFilters(NotificationEntry entry, long now, List<NotifFilter> filters) {
@@ -1064,12 +1168,10 @@ public class ShadeListBuilder implements Dumpable {
        NotifSection finalSection = newSection;

        // have we seen this entry before and are we changing its section?
        if (mNotifStabilityManager != null
                && entry.wasAttachedInPreviousPass()
                && newSection != prevAttachState.getSection()) {
        if (entry.wasAttachedInPreviousPass() && newSection != prevAttachState.getSection()) {

            // are section changes allowed?
            if (!mNotifStabilityManager.isSectionChangeAllowed(entry.getRepresentativeEntry())) {
            if (!getStabilityManager().isSectionChangeAllowed(entry.getRepresentativeEntry())) {
                // record the section that we wanted to change to
                entry.getAttachState().getSuppressedChanges().setSection(newSection);

+3 −2
Original line number Diff line number Diff line
@@ -385,7 +385,7 @@ public class PreparationCoordinator implements Coordinator {
    }

    private boolean shouldWaitForGroupToInflate(GroupEntry group, long now) {
        if (group == GroupEntry.ROOT_ENTRY || group.hasBeenAttachedBefore()) {
        if (group == GroupEntry.ROOT_ENTRY || group.wasAttachedInPreviousPass()) {
            return false;
        }
        if (isBeyondGroupInitializationWindow(group, now)) {
@@ -397,11 +397,12 @@ public class PreparationCoordinator implements Coordinator {
            return true;
        }
        for (NotificationEntry child : group.getChildren()) {
            if (mInflatingNotifs.contains(child) && !child.hasBeenAttachedBefore()) {
            if (mInflatingNotifs.contains(child) && !child.wasAttachedInPreviousPass()) {
                mLogger.logDelayingGroupRelease(group.getKey(), child.getKey());
                return true;
            }
        }
        mLogger.logDoneWaitingForGroupInflation(group.getKey());
        return false;
    }

+9 −1
Original line number Diff line number Diff line
@@ -41,11 +41,19 @@ class PreparationCoordinatorLogger @Inject constructor(
        })
    }

    fun logDoneWaitingForGroupInflation(groupKey: String) {
        buffer.log(TAG, LogLevel.DEBUG, {
            str1 = groupKey
        }, {
            "Finished inflating all members of group $str1, releasing group"
        })
    }

    fun logGroupInflationTookTooLong(groupKey: String) {
        buffer.log(TAG, LogLevel.WARNING, {
            str1 = groupKey
        }, {
            "Group inflation took too long far $str1, releasing children early"
            "Group inflation took too long for $str1, releasing children early"
        })
    }

Loading