diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index 4cdf8927c68b3..72cd95128779b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java @@ -36,6 +36,7 @@ import android.os.Trace; import android.util.ArrayMap; import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; import com.android.systemui.Dumpable; import com.android.systemui.dagger.SysUISingleton; @@ -66,6 +67,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -793,9 +795,38 @@ public class ShadeListBuilder implements Dumpable { } mNotifList.sort(mTopLevelComparator); assignIndexes(mNotifList); + + // Check for suppressed order changes + if (!mNotifStabilityManager.isEveryChangeAllowed()) { + mForceReorderable = true; + boolean isSorted = isSorted(mNotifList, mTopLevelComparator); + mForceReorderable = false; + if (!isSorted) { + mNotifStabilityManager.onEntryReorderSuppressed(); + } + } Trace.endSection(); } + /** Determine whether the items in the list are sorted according to the comparator */ + @VisibleForTesting + public static boolean isSorted(List items, Comparator comparator) { + if (items.size() <= 1) { + return true; + } + Iterator iterator = items.iterator(); + T previous = iterator.next(); + T current; + while (iterator.hasNext()) { + current = iterator.next(); + if (comparator.compare(previous, current) > 0) { + return false; + } + previous = current; + } + return true; + } + /** * Assign the index of each notification relative to the total order */ @@ -970,8 +1001,14 @@ public class ShadeListBuilder implements Dumpable { return cmp; }; + /** + * A flag that is set to true when we want to run the comparators as if all reordering is + * allowed. This is used to check if the list is "out of order" after the sort is complete. + */ + private boolean mForceReorderable = false; + private boolean canReorder(ListEntry entry) { - return mNotifStabilityManager.isEntryReorderingAllowed(entry); + return mForceReorderable || mNotifStabilityManager.isEntryReorderingAllowed(entry); } private boolean applyFilters(NotificationEntry entry, long now, List filters) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java index 32b1cf6bfdcaa..75489b1faadb8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java @@ -19,11 +19,12 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static com.android.systemui.keyguard.WakefulnessLifecycle.WAKEFULNESS_AWAKE; import static com.android.systemui.keyguard.WakefulnessLifecycle.WAKEFULNESS_WAKING; -import android.annotation.NonNull; - +import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; +import com.android.systemui.Dumpable; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dump.DumpManager; import com.android.systemui.keyguard.WakefulnessLifecycle; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationViewHierarchyManager; @@ -34,6 +35,8 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga import com.android.systemui.statusbar.policy.HeadsUpManager; import com.android.systemui.util.concurrency.DelayableExecutor; +import java.io.FileDescriptor; +import java.io.PrintWriter; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -53,7 +56,7 @@ import javax.inject.Inject; */ // TODO(b/204468557): Move to @CoordinatorScope @SysUISingleton -public class VisualStabilityCoordinator implements Coordinator { +public class VisualStabilityCoordinator implements Coordinator, Dumpable { private final DelayableExecutor mDelayableExecutor; private final WakefulnessLifecycle mWakefulnessLifecycle; private final StatusBarStateController mStatusBarStateController; @@ -66,6 +69,7 @@ public class VisualStabilityCoordinator implements Coordinator { private boolean mReorderingAllowed; private boolean mIsSuppressingGroupChange = false; private final Set mEntriesWithSuppressedSectionChange = new HashSet<>(); + private boolean mIsSuppressingEntryReorder = false; // key: notification key that can temporarily change its section // value: runnable that when run removes its associated RemoveOverrideSuppressionRunnable @@ -77,6 +81,7 @@ public class VisualStabilityCoordinator implements Coordinator { @Inject public VisualStabilityCoordinator( + DumpManager dumpManager, HeadsUpManager headsUpManager, WakefulnessLifecycle wakefulnessLifecycle, StatusBarStateController statusBarStateController, @@ -86,6 +91,8 @@ public class VisualStabilityCoordinator implements Coordinator { mWakefulnessLifecycle = wakefulnessLifecycle; mStatusBarStateController = statusBarStateController; mDelayableExecutor = delayableExecutor; + + dumpManager.registerDumpable(this); } @Override @@ -99,7 +106,6 @@ public class VisualStabilityCoordinator implements Coordinator { pipeline.setVisualStabilityManager(mNotifStabilityManager); } - // TODO(b/203828145): Ensure stability manager handles minimized state changes // TODO(b/203826051): Ensure stability manager can allow reordering off-screen // HUNs to the top of the shade private final NotifStabilityManager mNotifStabilityManager = @@ -108,6 +114,7 @@ public class VisualStabilityCoordinator implements Coordinator { public void onBeginRun() { mIsSuppressingGroupChange = false; mEntriesWithSuppressedSectionChange.clear(); + mIsSuppressingEntryReorder = false; } @Override @@ -124,7 +131,7 @@ public class VisualStabilityCoordinator implements Coordinator { mReorderingAllowed || mHeadsUpManager.isAlerting(entry.getKey()) || mEntriesThatCanChangeSection.containsKey(entry.getKey()); - if (isSectionChangeAllowedForEntry) { + if (!isSectionChangeAllowedForEntry) { mEntriesWithSuppressedSectionChange.add(entry.getKey()); } return isSectionChangeAllowedForEntry; @@ -134,11 +141,22 @@ public class VisualStabilityCoordinator implements Coordinator { public boolean isEntryReorderingAllowed(ListEntry section) { return mReorderingAllowed; } + + @Override + public boolean isEveryChangeAllowed() { + return mReorderingAllowed; + } + + @Override + public void onEntryReorderSuppressed() { + mIsSuppressingEntryReorder = true; + } }; private void updateAllowedStates() { mReorderingAllowed = isReorderingAllowed(); - if (mReorderingAllowed && (mIsSuppressingGroupChange || isSuppressingSectionChange())) { + if (mReorderingAllowed && (mIsSuppressingGroupChange || isSuppressingSectionChange() + || mIsSuppressingEntryReorder)) { mNotifStabilityManager.invalidateList(); } } @@ -211,4 +229,23 @@ public class VisualStabilityCoordinator implements Coordinator { updateAllowedStates(); } }; + + @Override + public void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter pw, @NonNull String[] args) { + pw.println("reorderingAllowed: " + mReorderingAllowed); + pw.println(" screenOn: " + mScreenOn); + pw.println(" panelExpanded: " + mPanelExpanded); + pw.println(" pulsing: " + mPulsing); + pw.println("isSuppressingGroupChange: " + mIsSuppressingGroupChange); + pw.println("isSuppressingEntryReorder: " + mIsSuppressingEntryReorder); + pw.println("entriesWithSuppressedSectionChange: " + + mEntriesWithSuppressedSectionChange.size()); + for (String key : mEntriesWithSuppressedSectionChange) { + pw.println(" " + key); + } + pw.println("entriesThatCanChangeSection: " + mEntriesThatCanChangeSection.size()); + for (String key : mEntriesThatCanChangeSection.keySet()) { + pw.println(" " + key); + } + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.java index 520791c918033..cb2d3cb97468a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.java @@ -60,4 +60,19 @@ public abstract class NotifStabilityManager extends Pluggable intCmp = Integer::compare; + assertTrue(ShadeListBuilder.isSorted(Collections.emptyList(), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Collections.singletonList(1), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Arrays.asList(1, 2), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Arrays.asList(1, 2, 3), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Arrays.asList(1, 2, 3, 4), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Arrays.asList(1, 2, 3, 4, 5), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Arrays.asList(1, 1, 1, 1, 1), intCmp)); + assertTrue(ShadeListBuilder.isSorted(Arrays.asList(1, 1, 2, 2, 3, 3), intCmp)); + + assertFalse(ShadeListBuilder.isSorted(Arrays.asList(2, 1), intCmp)); + assertFalse(ShadeListBuilder.isSorted(Arrays.asList(2, 1, 2), intCmp)); + assertFalse(ShadeListBuilder.isSorted(Arrays.asList(1, 2, 1), intCmp)); + assertFalse(ShadeListBuilder.isSorted(Arrays.asList(1, 2, 3, 2, 5), intCmp)); + assertFalse(ShadeListBuilder.isSorted(Arrays.asList(5, 2, 3, 4, 5), intCmp)); + assertFalse(ShadeListBuilder.isSorted(Arrays.asList(1, 2, 3, 4, 1), intCmp)); + } + /** * Adds a notif to the collection that will be passed to the list builder when * {@link #dispatchBuild()}s is called. @@ -1918,6 +1943,15 @@ public class ShadeListBuilderTest extends SysuiTestCase { public boolean isEntryReorderingAllowed(ListEntry entry) { return mAllowEntryReodering; } + + @Override + public boolean isEveryChangeAllowed() { + return mAllowEntryReodering && mAllowGroupChanges && mAllowSectionChanges; + } + + @Override + public void onEntryReorderSuppressed() { + } } private static final String PACKAGE_1 = "com.test1"; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java index 4edca7dd43d94..5df1d28073fc9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java @@ -19,8 +19,10 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static junit.framework.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -30,6 +32,7 @@ import android.testing.TestableLooper; import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; +import com.android.systemui.dump.DumpManager; import com.android.systemui.keyguard.WakefulnessLifecycle; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -37,7 +40,6 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifStabilityManager; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable; -import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.policy.HeadsUpManager; import com.android.systemui.util.concurrency.FakeExecutor; import com.android.systemui.util.time.FakeSystemClock; @@ -57,9 +59,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { private VisualStabilityCoordinator mCoordinator; - // captured listeners and pluggables: - private NotifCollectionListener mCollectionListener; - + @Mock private DumpManager mDumpManager; @Mock private NotifPipeline mNotifPipeline; @Mock private WakefulnessLifecycle mWakefulnessLifecycle; @Mock private StatusBarStateController mStatusBarStateController; @@ -69,7 +69,6 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { @Captor private ArgumentCaptor mWakefulnessObserverCaptor; @Captor private ArgumentCaptor mSBStateListenerCaptor; @Captor private ArgumentCaptor mNotifStabilityManagerCaptor; - @Captor private ArgumentCaptor mNotifCollectionListenerCaptor; private FakeSystemClock mFakeSystemClock = new FakeSystemClock(); private FakeExecutor mFakeExecutor = new FakeExecutor(mFakeSystemClock); @@ -84,6 +83,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); mCoordinator = new VisualStabilityCoordinator( + mDumpManager, mHeadsUpManager, mWakefulnessLifecycle, mStatusBarStateController, @@ -107,6 +107,12 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { .build(); when(mHeadsUpManager.isAlerting(mEntry.getKey())).thenReturn(false); + + // Whenever we invalidate, the pipeline runs again, so we invalidate the state + doAnswer(i -> { + mNotifStabilityManager.onBeginRun(); + return null; + }).when(mInvalidateListener).onPluggableInvalidated(eq(mNotifStabilityManager)); } @Test @@ -211,7 +217,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { mCoordinator.temporarilyAllowSectionChanges(mEntry, mFakeSystemClock.uptimeMillis()); // THEN the notification list is invalidated - verifyInvalidateCalled(true); + verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); } @Test @@ -225,7 +231,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { mCoordinator.temporarilyAllowSectionChanges(mEntry, mFakeSystemClock.currentTimeMillis()); // THEN invalidate is not called because this entry was never suppressed from reordering - verifyInvalidateCalled(false); + verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); } @Test @@ -241,7 +247,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { // THEN invalidate is not called because this entry was never suppressed from reordering; // THEN section changes are allowed for this notification - verifyInvalidateCalled(false); + verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); assertTrue(mNotifStabilityManager.isSectionChangeAllowed(mEntry)); // WHEN we're pulsing (now disallowing reordering) @@ -268,13 +274,14 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { // WHEN we temporarily allow section changes for this notification entry mCoordinator.temporarilyAllowSectionChanges(mEntry, mFakeSystemClock.currentTimeMillis()); - verifyInvalidateCalled(true); // can now reorder, so invalidates + // can now reorder, so invalidates + verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); // WHEN reordering is now allowed because device isn't pulsing anymore setPulsing(false); - // THEN invalidate isn't called since reordering was already allowed - verifyInvalidateCalled(false); + // THEN invalidate isn't called a second time since reordering was already allowed + verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); } @Test @@ -292,7 +299,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { // THEN we never see any calls to invalidate since there weren't any notifications that // were being suppressed from grouping or section changes - verifyInvalidateCalled(false); + verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); } @Test @@ -308,7 +315,41 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { setPanelExpanded(false); // invalidate is called because we were previously suppressing a group change - verifyInvalidateCalled(true); + verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + } + + @Test + public void testNotSuppressingEntryReorderingAnymoreWillInvalidate() { + // GIVEN visual stability is being maintained b/c panel is expanded + setPulsing(false); + setScreenOn(true); + setPanelExpanded(true); + + assertFalse(mNotifStabilityManager.isEntryReorderingAllowed(mEntry)); + // The pipeline still has to report back that entry reordering was suppressed + mNotifStabilityManager.onEntryReorderSuppressed(); + + // WHEN the panel isn't expanded anymore + setPanelExpanded(false); + + // invalidate is called because we were previously suppressing an entry reorder + verify(mInvalidateListener, times(1)).onPluggableInvalidated(mNotifStabilityManager); + } + + @Test + public void testQueryingEntryReorderingButNotReportingReorderSuppressedDoesNotInvalidate() { + // GIVEN visual stability is being maintained b/c panel is expanded + setPulsing(false); + setScreenOn(true); + setPanelExpanded(true); + + assertFalse(mNotifStabilityManager.isEntryReorderingAllowed(mEntry)); + + // WHEN the panel isn't expanded anymore + setPanelExpanded(false); + + // invalidate is not called because we were not told that an entry reorder was suppressed + verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); } @Test @@ -345,13 +386,4 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { mStatusBarStateListener.onExpandedChanged(expanded); } - private void verifyInvalidateCalled(boolean invalidateCalled) { - if (invalidateCalled) { - verify(mInvalidateListener).onPluggableInvalidated(mNotifStabilityManager); - } else { - verify(mInvalidateListener, never()).onPluggableInvalidated(mNotifStabilityManager); - } - - reset(mInvalidateListener); - } }