diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 464b2b69c58e8..ff3e97af72a7f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -35,7 +35,7 @@ import com.android.systemui.statusbar.notification.DynamicChildBindController; import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.notification.collection.inflation.LowPriorityInflationHelper; +import com.android.systemui.statusbar.notification.collection.legacy.LowPriorityInflationHelper; import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy; import com.android.systemui.statusbar.notification.collection.legacy.VisualStabilityManager; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java index aa86daaae1258..d5cba72f99d3c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java @@ -55,7 +55,7 @@ import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotifCollection; import com.android.systemui.statusbar.notification.collection.NotifPipeline; -import com.android.systemui.statusbar.notification.collection.inflation.LowPriorityInflationHelper; +import com.android.systemui.statusbar.notification.collection.legacy.LowPriorityInflationHelper; import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy; import com.android.systemui.statusbar.notification.collection.legacy.VisualStabilityManager; import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection; @@ -73,8 +73,6 @@ import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallLogger; import com.android.systemui.statusbar.policy.RemoteInputUriController; import com.android.systemui.statusbar.window.StatusBarWindowController; -import com.android.systemui.statusbar.window.StatusBarWindowModule; -import com.android.systemui.statusbar.window.StatusBarWindowView; import com.android.systemui.tracing.ProtoTracer; import com.android.systemui.util.concurrency.DelayableExecutor; import com.android.systemui.util.time.SystemClock; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index 82f35a814d226..2437415d0c7e2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -638,7 +638,7 @@ public class NotificationEntryManager implements // Construct the expanded view. if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { - mNotificationRowBinderLazy.get().inflateViews(entry, mInflationCallback); + mNotificationRowBinderLazy.get().inflateViews(entry, null, mInflationCallback); } mPendingNotifications.put(key, entry); @@ -695,7 +695,7 @@ public class NotificationEntryManager implements } if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { - mNotificationRowBinderLazy.get().inflateViews(entry, mInflationCallback); + mNotificationRowBinderLazy.get().inflateViews(entry, null, mInflationCallback); } updateNotifications("updateNotificationInternal"); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java index 8562a2e55a4fc..4f3c287d5f1e2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java @@ -16,7 +16,8 @@ package com.android.systemui.statusbar.notification.collection; -import com.android.internal.statusbar.IStatusBarService; +import androidx.annotation.NonNull; + import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.statusbar.notification.InflationException; import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater; @@ -34,23 +35,13 @@ import javax.inject.Inject; @SysUISingleton public class NotifInflaterImpl implements NotifInflater { - private final IStatusBarService mStatusBarService; - private final NotifCollection mNotifCollection; private final NotifInflationErrorManager mNotifErrorManager; - private final NotifPipeline mNotifPipeline; private NotificationRowBinderImpl mNotificationRowBinder; @Inject - public NotifInflaterImpl( - IStatusBarService statusBarService, - NotifCollection notifCollection, - NotifInflationErrorManager errorManager, - NotifPipeline notifPipeline) { - mStatusBarService = statusBarService; - mNotifCollection = notifCollection; + public NotifInflaterImpl(NotifInflationErrorManager errorManager) { mNotifErrorManager = errorManager; - mNotifPipeline = notifPipeline; } /** @@ -61,8 +52,9 @@ public class NotifInflaterImpl implements NotifInflater { } @Override - public void rebindViews(NotificationEntry entry, InflationCallback callback) { - inflateViews(entry, callback); + public void rebindViews(@NonNull NotificationEntry entry, @NonNull Params params, + @NonNull InflationCallback callback) { + inflateViews(entry, params, callback); } /** @@ -70,10 +62,12 @@ public class NotifInflaterImpl implements NotifInflater { * views are bound. */ @Override - public void inflateViews(NotificationEntry entry, InflationCallback callback) { + public void inflateViews(@NonNull NotificationEntry entry, @NonNull Params params, + @NonNull InflationCallback callback) { try { requireBinder().inflateViews( entry, + params, wrapInflationCallback(callback)); } catch (InflationException e) { mNotifErrorManager.setInflationError(entry, e); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt index 50d7324df2b44..9ae9fe5089441 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt @@ -58,12 +58,13 @@ import javax.inject.Inject * appropriately). * 3. OnBeforeTransformGroupListeners are fired ([.addOnBeforeTransformGroupsListener]) * 4. NotifPromoters are called on each notification with a parent ([.addPromoter]) - * 5. Finalize filters are fired on each notification ([.addFinalizeFilter]) - * 6. OnBeforeSortListeners are fired ([.addOnBeforeSortListener]) - * 7. Top-level entries are assigned sections by NotifSections ([.setSections]) - * 8. Top-level entries within the same section are sorted by NotifComparators ([.setComparators]) - * 9. OnBeforeRenderListListeners are fired ([.addOnBeforeRenderListListener]) - * 10. The list is handed off to the view layer to be rendered + * 5. OnBeforeSortListeners are fired ([.addOnBeforeSortListener]) + * 6. Top-level entries are assigned sections by NotifSections ([.setSections]) + * 7. Top-level entries within the same section are sorted by NotifComparators ([.setComparators]) + * 8. OnBeforeFinalizeFilterListeners are fired ([.addOnBeforeFinalizeFilterListener]) + * 9. Finalize filters are fired on each notification ([.addFinalizeFilter]) + * 10. OnBeforeRenderListListeners are fired ([.addOnBeforeRenderListListener]) + * 11. The list is handed off to the view layer to be rendered */ @SysUISingleton class NotifPipeline @Inject constructor( 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 15872da144162..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 @@ -28,12 +28,15 @@ import static com.android.systemui.statusbar.notification.collection.listbuilder import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_SORTING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_TRANSFORMING; +import static java.util.Objects.requireNonNull; + import android.annotation.MainThread; import android.annotation.Nullable; 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; @@ -64,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; @@ -363,22 +367,24 @@ public class ShadeListBuilder implements Dumpable { mPipelineState.incrementTo(STATE_GROUP_STABILIZING); stabilizeGroupingNotifs(mNotifList); + // Step 5: Section & Sort + // Assign each top-level entry a section, and copy to all of its children + dispatchOnBeforeSort(mReadOnlyNotifList); + mPipelineState.incrementTo(STATE_SORTING); + assignSections(); + notifySectionEntriesUpdated(); + // Sort the list by section and then within section by our list of custom comparators + sortListAndGroups(); - // Step 5: Filter out entries after pre-group filtering, grouping and promoting - // Now filters can see grouping information to determine whether to filter or not. + // Step 6: Filter out entries after pre-group filtering, grouping, promoting, and sorting + // Now filters can see grouping, sectioning, and order information to determine whether + // to filter or not. dispatchOnBeforeFinalizeFilter(mReadOnlyNotifList); mPipelineState.incrementTo(STATE_FINALIZE_FILTERING); filterNotifs(mNotifList, mNewNotifList, mNotifFinalizeFilters); applyNewNotifList(); pruneIncompleteGroups(mNotifList); - // Step 6: Sort - // Assign each top-level entry a section, then sort the list by section and then within - // section by our list of custom comparators - dispatchOnBeforeSort(mReadOnlyNotifList); - mPipelineState.incrementTo(STATE_SORTING); - sortListAndNotifySections(); - // Step 7: Lock in our group structure and log anything that's changed since the last run mPipelineState.incrementTo(STATE_FINALIZING); logChanges(); @@ -408,18 +414,15 @@ public class ShadeListBuilder implements Dumpable { private void notifySectionEntriesUpdated() { Trace.beginSection("ShadeListBuilder.notifySectionEntriesUpdated"); - NotifSection currentSection = null; mTempSectionMembers.clear(); - for (int i = 0; i < mNotifList.size(); i++) { - ListEntry currentEntry = mNotifList.get(i); - if (currentSection != currentEntry.getSection()) { - if (currentSection != null) { - currentSection.getSectioner().onEntriesUpdated(mTempSectionMembers); - mTempSectionMembers.clear(); + for (NotifSection section : mNotifSections) { + for (ListEntry entry : mNotifList) { + if (section == entry.getSection()) { + mTempSectionMembers.add(entry); } - currentSection = currentEntry.getSection(); } - mTempSectionMembers.add(currentEntry); + section.getSectioner().onEntriesUpdated(mTempSectionMembers); + mTempSectionMembers.clear(); } Trace.endSection(); } @@ -653,16 +656,15 @@ public class ShadeListBuilder implements Dumpable { final List children = group.getRawChildren(); if (group.getSummary() != null && children.size() == 0) { - shadeList.remove(i); - i--; - NotificationEntry summary = group.getSummary(); summary.setParent(ROOT_ENTRY); - shadeList.add(summary); + // The list may be sorted; replace the group with the summary, in its place + shadeList.set(i, summary); 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) { @@ -680,7 +682,6 @@ public class ShadeListBuilder implements Dumpable { // its children (if any) directly to top-level. shadeList.remove(i); - i--; if (group.getSummary() != null) { final NotificationEntry summary = group.getSummary(); @@ -691,11 +692,14 @@ public class ShadeListBuilder implements Dumpable { for (int j = 0; j < children.size(); j++) { final NotificationEntry child = children.get(j); child.setParent(ROOT_ENTRY); - shadeList.add(child); + // The list may be sorted, so add the children in order where the group was. + shadeList.add(i + j, child); } children.clear(); annulAddition(group, shadeList); + + i--; // The node we visited is gone, so be sure to visit this index again. } } } @@ -765,9 +769,9 @@ public class ShadeListBuilder implements Dumpable { } } - private void sortListAndNotifySections() { - Trace.beginSection("ShadeListBuilder.sortListAndNotifySections"); - // Assign sections to top-level elements and sort their children + private void assignSections() { + Trace.beginSection("ShadeListBuilder.assignSections"); + // Assign sections to top-level elements and their children for (ListEntry entry : mNotifList) { NotifSection section = applySections(entry); if (entry instanceof GroupEntry) { @@ -775,27 +779,64 @@ public class ShadeListBuilder implements Dumpable { for (NotificationEntry child : parent.getChildren()) { setEntrySection(child, section); } + } + } + Trace.endSection(); + } + + private void sortListAndGroups() { + Trace.beginSection("ShadeListBuilder.sortListAndGroups"); + // Assign sections to top-level elements and sort their children + for (ListEntry entry : mNotifList) { + if (entry instanceof GroupEntry) { + GroupEntry parent = (GroupEntry) entry; parent.sortChildren(mGroupChildrenComparator); } } mNotifList.sort(mTopLevelComparator); assignIndexes(mNotifList); - notifySectionEntriesUpdated(); + // 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 - * @param notifList */ - private void assignIndexes(List notifList) { + private static void assignIndexes(List notifList) { if (notifList.size() == 0) return; - NotifSection currentSection = notifList.get(0).getSection(); + NotifSection currentSection = requireNonNull(notifList.get(0).getSection()); int sectionMemberIndex = 0; for (int i = 0; i < notifList.size(); i++) { ListEntry entry = notifList.get(i); - NotifSection section = entry.getSection(); + NotifSection section = requireNonNull(entry.getSection()); if (section.getIndex() != currentSection.getIndex()) { sectionMemberIndex = 0; currentSection = section; @@ -960,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) { @@ -1031,8 +1078,11 @@ public class ShadeListBuilder implements Dumpable { private void setEntrySection(ListEntry entry, NotifSection finalSection) { entry.getAttachState().setSection(finalSection); NotificationEntry representativeEntry = entry.getRepresentativeEntry(); - if (representativeEntry != null && finalSection != null) { - representativeEntry.setBucket(finalSection.getBucket()); + if (representativeEntry != null) { + representativeEntry.getAttachState().setSection(finalSection); + if (finalSection != null) { + representativeEntry.setBucket(finalSection.getBucket()); + } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java index afdfb3bdeef60..644f248fca008 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java @@ -26,6 +26,9 @@ import android.service.notification.StatusBarNotification; import android.util.ArrayMap; import android.util.ArraySet; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + import com.android.internal.annotations.VisibleForTesting; import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.dagger.SysUISingleton; @@ -35,6 +38,8 @@ import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.ShadeListBuilder; import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater; +import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustment; +import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustmentProvider; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; @@ -46,7 +51,6 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.List; import java.util.Map; -import java.util.Set; import javax.inject.Inject; @@ -66,14 +70,24 @@ public class PreparationCoordinator implements Coordinator { private final NotifInflater mNotifInflater; private final NotifInflationErrorManager mNotifErrorManager; private final NotifViewBarn mViewBarn; - private final Map mInflationStates = new ArrayMap<>(); + private final NotifUiAdjustmentProvider mAdjustmentProvider; + private final ArrayMap mInflationStates = new ArrayMap<>(); + + /** + * The map of notifications to the NotifUiAdjustment (i.e. parameters) that were calculated + * when the inflation started. If an update of any kind results in the adjustment changing, + * then the row must be reinflated. If the row is being inflated, then the inflation must be + * aborted and restarted. + */ + private final ArrayMap mInflationAdjustments = + new ArrayMap<>(); /** * The set of notifications that are currently inflating something. Note that this is * separate from inflation state as a view could either be uninflated or inflated and still be * inflating something. */ - private final Set mInflatingNotifs = new ArraySet<>(); + private final ArraySet mInflatingNotifs = new ArraySet<>(); private final IStatusBarService mStatusBarService; @@ -92,12 +106,14 @@ public class PreparationCoordinator implements Coordinator { NotifInflater notifInflater, NotifInflationErrorManager errorManager, NotifViewBarn viewBarn, + NotifUiAdjustmentProvider adjustmentProvider, IStatusBarService service) { this( logger, notifInflater, errorManager, viewBarn, + adjustmentProvider, service, CHILD_BIND_CUTOFF, MAX_GROUP_INFLATION_DELAY); @@ -109,6 +125,7 @@ public class PreparationCoordinator implements Coordinator { NotifInflater notifInflater, NotifInflationErrorManager errorManager, NotifViewBarn viewBarn, + NotifUiAdjustmentProvider adjustmentProvider, IStatusBarService service, int childBindCutoff, long maxGroupInflationDelay) { @@ -116,6 +133,7 @@ public class PreparationCoordinator implements Coordinator { mNotifInflater = notifInflater; mNotifErrorManager = errorManager; mViewBarn = viewBarn; + mAdjustmentProvider = adjustmentProvider; mStatusBarService = service; mChildBindCutoff = childBindCutoff; mMaxGroupInflationDelay = maxGroupInflationDelay; @@ -160,6 +178,7 @@ public class PreparationCoordinator implements Coordinator { public void onEntryCleanUp(NotificationEntry entry) { mInflationStates.remove(entry); mViewBarn.removeViewForEntry(entry); + mInflationAdjustments.remove(entry); } }; @@ -269,39 +288,78 @@ public class PreparationCoordinator implements Coordinator { } private void inflateRequiredNotifViews(NotificationEntry entry) { + NotifUiAdjustment newAdjustment = mAdjustmentProvider.calculateAdjustment(entry); if (mInflatingNotifs.contains(entry)) { // Already inflating this entry + String errorIfNoOldAdjustment = "Inflating notification has no adjustments"; + if (needToReinflate(entry, newAdjustment, errorIfNoOldAdjustment)) { + inflateEntry(entry, newAdjustment, "adjustment changed while inflating"); + } return; } @InflationState int state = mInflationStates.get(entry); switch (state) { case STATE_UNINFLATED: - inflateEntry(entry, "entryAdded"); + inflateEntry(entry, newAdjustment, "entryAdded"); break; case STATE_INFLATED_INVALID: - rebind(entry, "entryUpdated"); + rebind(entry, newAdjustment, "entryUpdated"); break; case STATE_INFLATED: + String errorIfNoOldAdjustment = "Fully inflated notification has no adjustments"; + if (needToReinflate(entry, newAdjustment, errorIfNoOldAdjustment)) { + rebind(entry, newAdjustment, "adjustment changed after inflated"); + } + break; case STATE_ERROR: + if (needToReinflate(entry, newAdjustment, null)) { + inflateEntry(entry, newAdjustment, "adjustment changed after error"); + } + break; default: // Nothing to do. } } - private void inflateEntry(NotificationEntry entry, String reason) { - abortInflation(entry, reason); - mInflatingNotifs.add(entry); - mNotifInflater.inflateViews(entry, this::onInflationFinished); + private boolean needToReinflate(@NonNull NotificationEntry entry, + @NonNull NotifUiAdjustment newAdjustment, @Nullable String oldAdjustmentMissingError) { + NotifUiAdjustment oldAdjustment = mInflationAdjustments.get(entry); + if (oldAdjustment == null) { + if (oldAdjustmentMissingError == null) { + return true; + } else { + throw new IllegalStateException(oldAdjustmentMissingError); + } + } + return NotifUiAdjustment.needReinflate(oldAdjustment, newAdjustment); } - private void rebind(NotificationEntry entry, String reason) { + private void inflateEntry(NotificationEntry entry, + NotifUiAdjustment newAdjustment, + String reason) { + abortInflation(entry, reason); + mInflationAdjustments.put(entry, newAdjustment); mInflatingNotifs.add(entry); - mNotifInflater.rebindViews(entry, this::onInflationFinished); + NotifInflater.Params params = getInflaterParams(newAdjustment, reason); + mNotifInflater.inflateViews(entry, params, this::onInflationFinished); + } + + private void rebind(NotificationEntry entry, + NotifUiAdjustment newAdjustment, + String reason) { + mInflationAdjustments.put(entry, newAdjustment); + mInflatingNotifs.add(entry); + NotifInflater.Params params = getInflaterParams(newAdjustment, reason); + mNotifInflater.rebindViews(entry, params, this::onInflationFinished); + } + + NotifInflater.Params getInflaterParams(NotifUiAdjustment adjustment, String reason) { + return new NotifInflater.Params(adjustment.isMinimized(), reason); } private void abortInflation(NotificationEntry entry, String reason) { mLogger.logInflationAborted(entry.getKey(), reason); - entry.abortTask(); + mNotifInflater.abortInflation(entry); mInflatingNotifs.remove(entry); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java index 79eb0898bc0b8..c60ebcdc5fd12 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinator.java @@ -19,12 +19,12 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import android.annotation.NonNull; import android.annotation.Nullable; -import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.collection.ListEntry; import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope; +import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustmentProvider; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner; import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider; @@ -34,6 +34,7 @@ import com.android.systemui.statusbar.notification.dagger.AlertingHeader; import com.android.systemui.statusbar.notification.dagger.SilentHeader; import com.android.systemui.statusbar.notification.stack.NotificationPriorityBucketKt; +import java.util.Collections; import java.util.List; import javax.inject.Inject; @@ -50,6 +51,7 @@ public class RankingCoordinator implements Coordinator { public static final boolean SHOW_ALL_SECTIONS = false; private final StatusBarStateController mStatusBarStateController; private final HighPriorityProvider mHighPriorityProvider; + private final NotifUiAdjustmentProvider mAdjustmentProvider; private final NodeController mSilentNodeController; private final SectionHeaderController mSilentHeaderController; private final NodeController mAlertingHeaderController; @@ -60,11 +62,13 @@ public class RankingCoordinator implements Coordinator { public RankingCoordinator( StatusBarStateController statusBarStateController, HighPriorityProvider highPriorityProvider, + NotifUiAdjustmentProvider adjustmentProvider, @AlertingHeader NodeController alertingHeaderController, @SilentHeader SectionHeaderController silentHeaderController, @SilentHeader NodeController silentNodeController) { mStatusBarStateController = statusBarStateController; mHighPriorityProvider = highPriorityProvider; + mAdjustmentProvider = adjustmentProvider; mAlertingHeaderController = alertingHeaderController; mSilentNodeController = silentNodeController; mSilentHeaderController = silentHeaderController; @@ -73,10 +77,10 @@ public class RankingCoordinator implements Coordinator { @Override public void attach(NotifPipeline pipeline) { mStatusBarStateController.addCallback(mStatusBarStateCallback); + mAdjustmentProvider.setLowPrioritySections(Collections.singleton(mMinimizedNotifSectioner)); pipeline.addPreGroupFilter(mSuspendedFilter); pipeline.addPreGroupFilter(mDndVisualEffectsFilter); - pipeline.addOnBeforeSortListener(entries -> resetClearAllFlags()); } public NotifSectioner getAlertingSectioner() { @@ -126,6 +130,7 @@ public class RankingCoordinator implements Coordinator { @Nullable @Override public void onEntriesUpdated(@NonNull List entries) { + mHasSilentEntries = false; for (int i = 0; i < entries.size(); i++) { if (entries.get(i).getRepresentativeEntry().getSbn().isClearable()) { mHasSilentEntries = true; @@ -154,6 +159,7 @@ public class RankingCoordinator implements Coordinator { @Nullable @Override public void onEntriesUpdated(@NonNull List entries) { + mHasMinimizedEntries = false; for (int i = 0; i < entries.size(); i++) { if (entries.get(i).getRepresentativeEntry().getSbn().isClearable()) { mHasMinimizedEntries = true; @@ -189,12 +195,6 @@ public class RankingCoordinator implements Coordinator { } }; - @VisibleForTesting - protected void resetClearAllFlags() { - mHasSilentEntries = false; - mHasMinimizedEntries = false; - } - private final StatusBarStateController.StateListener mStatusBarStateCallback = new StatusBarStateController.StateListener() { @Override 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/inflation/NotifInflater.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt similarity index 73% rename from packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.java rename to packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt index e3d76113d5378..c59f18436b74a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifInflater.kt @@ -14,22 +14,22 @@ * limitations under the License. */ -package com.android.systemui.statusbar.notification.collection.inflation; -import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.notification.collection.coordinator.PreparationCoordinator; +package com.android.systemui.statusbar.notification.collection.inflation + +import com.android.systemui.statusbar.notification.collection.NotificationEntry /** - * Used by the {@link PreparationCoordinator}. When notifications are added or updated, the + * Used by the [PreparationCoordinator]. When notifications are added or updated, the * NotifInflater is asked to (re)inflated and prepare their views. This inflation occurs off the * main thread. When the inflation is finished, NotifInflater will trigger its InflationCallback. */ -public interface NotifInflater { +interface NotifInflater { /** * Called to rebind the entry's views. * * @param callback callback called after inflation finishes */ - void rebindViews(NotificationEntry entry, InflationCallback callback); + fun rebindViews(entry: NotificationEntry, params: Params, callback: InflationCallback) /** * Called to inflate the views of an entry. Views are not considered inflated until all of its @@ -37,18 +37,23 @@ public interface NotifInflater { * * @param callback callback called after inflation finishes */ - void inflateViews(NotificationEntry entry, InflationCallback callback); + fun inflateViews(entry: NotificationEntry, params: Params, callback: InflationCallback) /** * Request to stop the inflation of an entry. For example, called when a notification is * removed and no longer needs to be inflated. */ - void abortInflation(NotificationEntry entry); + fun abortInflation(entry: NotificationEntry) /** * Callback once all the views are inflated and bound for a given NotificationEntry. */ interface InflationCallback { - void onInflationFinished(NotificationEntry entry); + fun onInflationFinished(entry: NotificationEntry) } -} + + /** + * A class holding parameters used when inflating the notification row + */ + class Params(val isLowPriority: Boolean, val reason: String) +} \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustment.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustment.kt new file mode 100644 index 0000000000000..9d86b7831f184 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustment.kt @@ -0,0 +1,95 @@ +/* + * Copyright 2021 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.inflation + +import android.app.Notification +import android.app.RemoteInput +import android.graphics.drawable.Icon +import android.text.TextUtils + +/** + * An immutable object which contains minimal state extracted from an entry that represents state + * which can change without a direct app update (e.g. with a ranking update). + * Diffing two entries determines if view re-inflation is needed. + */ +class NotifUiAdjustment internal constructor( + val key: String, + val smartActions: List, + val smartReplies: List, + val isConversation: Boolean, + val isMinimized: Boolean +) { + companion object { + @JvmStatic + fun needReinflate( + oldAdjustment: NotifUiAdjustment, + newAdjustment: NotifUiAdjustment + ): Boolean = when { + oldAdjustment === newAdjustment -> false + oldAdjustment.isConversation != newAdjustment.isConversation -> true + oldAdjustment.isMinimized != newAdjustment.isMinimized -> true + areDifferent(oldAdjustment.smartActions, newAdjustment.smartActions) -> true + newAdjustment.smartReplies != oldAdjustment.smartReplies -> true + else -> false + } + + private fun areDifferent( + first: List, + second: List + ): Boolean = when { + first === second -> false + first.size != second.size -> true + else -> first.asSequence().zip(second.asSequence()).any { + (!TextUtils.equals(it.first.title, it.second.title)) || + (areDifferent(it.first.getIcon(), it.second.getIcon())) || + (it.first.actionIntent != it.second.actionIntent) || + (areDifferent(it.first.remoteInputs, it.second.remoteInputs)) + } + } + + private fun areDifferent(first: Icon?, second: Icon?): Boolean = when { + first === second -> false + first == null || second == null -> true + else -> !first.sameAs(second) + } + + private fun areDifferent( + first: Array?, + second: Array? + ): Boolean = when { + first === second -> false + first == null || second == null -> true + first.size != second.size -> true + else -> first.asSequence().zip(second.asSequence()).any { + (!TextUtils.equals(it.first.label, it.second.label)) || + (areDifferent(it.first.choices, it.second.choices)) + } + } + + private fun areDifferent( + first: Array?, + second: Array? + ): Boolean = when { + first === second -> false + first == null || second == null -> true + first.size != second.size -> true + else -> first.asSequence().zip(second.asSequence()).any { + !TextUtils.equals(it.first, it.second) + } + } + } +} \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustmentProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustmentProvider.kt new file mode 100644 index 0000000000000..3290cdffdceb5 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustmentProvider.kt @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2021 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.inflation + +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.statusbar.notification.collection.GroupEntry +import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner +import javax.inject.Inject + +/** + * A class which provides an adjustment object to the preparation coordinator which is uses + * to ensure that notifications are reinflated when ranking-derived information changes. + */ +@SysUISingleton +open class NotifUiAdjustmentProvider @Inject constructor() { + + private lateinit var lowPrioritySections: Set + + /** + * Feed the provider the information it needs about which sections should have minimized top + * level views, so that it can calculate the correct minimized value in the adjustment. + */ + fun setLowPrioritySections(sections: Collection) { + lowPrioritySections = sections.toSet() + } + + private fun isEntryMinimized(entry: NotificationEntry): Boolean { + val section = entry.section ?: error("Entry must have a section to determine if minimized") + val parent = entry.parent ?: error("Entry must have a parent to determine if minimized") + val isLowPrioritySection = lowPrioritySections.contains(section.sectioner) + val isTopLevelEntry = parent == GroupEntry.ROOT_ENTRY + val isGroupSummary = parent.summary == entry + return isLowPrioritySection && (isTopLevelEntry || isGroupSummary) + } + + /** + * Returns a adjustment object for the given entry. This can be compared to a previous instance + * from the same notification using [NotifUiAdjustment.needReinflate] to determine if it + * should be reinflated. + */ + fun calculateAdjustment(entry: NotificationEntry) = NotifUiAdjustment( + key = entry.key, + smartActions = entry.ranking.smartActions, + smartReplies = entry.ranking.smartReplies, + isConversation = entry.ranking.isConversation, + isMinimized = isEntryMinimized(entry) + ) +} \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java index 1215ade2abb1b..3a4701c9ac765 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinder.java @@ -32,12 +32,10 @@ public interface NotificationRowBinder { /** * Called when a notification has been added or updated. The binder must asynchronously inflate * and bind the views associated with the notification. - * - * TODO: The caller is notified when the inflation completes, but this is currently a very - * roundabout business. Add an explicit completion/failure callback to this method. */ void inflateViews( NotificationEntry entry, + NotifInflater.Params params, NotificationRowContentBinder.InflationCallback callback) throws InflationException; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java index e5425cfc8c93c..5c8e8b244abb7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java @@ -25,6 +25,7 @@ import android.view.ViewGroup; import com.android.internal.util.NotificationMessagingUtil; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.flags.FeatureFlags; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.NotificationRemoteInputManager; @@ -32,6 +33,7 @@ import com.android.systemui.statusbar.NotificationUiAdjustment; import com.android.systemui.statusbar.notification.InflationException; import com.android.systemui.statusbar.notification.NotificationClicker; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.collection.legacy.LowPriorityInflationHelper; import com.android.systemui.statusbar.notification.icon.IconManager; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRowController; @@ -53,6 +55,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { private static final String TAG = "NotificationViewManager"; private final Context mContext; + private final FeatureFlags mFeatureFlags; private final NotificationMessagingUtil mMessagingUtil; private final NotificationRemoteInputManager mNotificationRemoteInputManager; private final NotificationLockscreenUserManager mNotificationLockscreenUserManager; @@ -72,6 +75,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { @Inject public NotificationRowBinderImpl( Context context, + FeatureFlags featureFlags, NotificationMessagingUtil notificationMessagingUtil, NotificationRemoteInputManager notificationRemoteInputManager, NotificationLockscreenUserManager notificationLockscreenUserManager, @@ -82,6 +86,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { IconManager iconManager, LowPriorityInflationHelper lowPriorityInflationHelper) { mContext = context; + mFeatureFlags = featureFlags; mNotifBindPipeline = notifBindPipeline; mRowContentBindStage = rowContentBindStage; mMessagingUtil = notificationMessagingUtil; @@ -116,8 +121,13 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { @Override public void inflateViews( NotificationEntry entry, + NotifInflater.Params params, NotificationRowContentBinder.InflationCallback callback) throws InflationException { + if (params == null) { + // weak assert that the params should always be passed in the new pipeline + mFeatureFlags.checkLegacyPipelineEnabled(); + } ViewGroup parent = mListContainer.getViewParentForNotification(entry); if (entry.rowExists()) { @@ -125,7 +135,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { ExpandableNotificationRow row = entry.getRow(); row.reset(); updateRow(entry, row); - inflateContentViews(entry, row, callback); + inflateContentViews(entry, params, row, callback); } else { mIconManager.createIcons(entry); mRowInflaterTaskProvider.get().inflate(mContext, parent, entry, @@ -144,7 +154,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { entry.setRowController(rowController); bindRow(entry, row); updateRow(entry, row); - inflateContentViews(entry, row, callback); + inflateContentViews(entry, params, row, callback); }); } } @@ -177,12 +187,13 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { NotificationUiAdjustment oldAdjustment, NotificationUiAdjustment newAdjustment, NotificationRowContentBinder.InflationCallback callback) { + mFeatureFlags.checkLegacyPipelineEnabled(); if (NotificationUiAdjustment.needReinflate(oldAdjustment, newAdjustment)) { if (entry.rowExists()) { ExpandableNotificationRow row = entry.getRow(); row.reset(); updateRow(entry, row); - inflateContentViews(entry, row, callback); + inflateContentViews(entry, null, row, callback); } else { // Once the RowInflaterTask is done, it will pick up the updated entry, so // no-op here. @@ -216,15 +227,24 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { */ private void inflateContentViews( NotificationEntry entry, + NotifInflater.Params inflaterParams, ExpandableNotificationRow row, @Nullable NotificationRowContentBinder.InflationCallback inflationCallback) { final boolean useIncreasedCollapsedHeight = mMessagingUtil.isImportantMessaging(entry.getSbn(), entry.getImportance()); - // If this is our first time inflating, we don't actually know the groupings for real - // yet, so we might actually inflate a low priority content view incorrectly here and have - // to correct it later in the pipeline. On subsequent inflations (i.e. updates), this - // should inflate the correct view. - final boolean isLowPriority = mLowPriorityInflationHelper.shouldUseLowPriorityView(entry); + final boolean isLowPriority; + if (inflaterParams != null) { + // NEW pipeline + isLowPriority = inflaterParams.isLowPriority(); + } else { + // LEGACY pipeline + mFeatureFlags.checkLegacyPipelineEnabled(); + // If this is our first time inflating, we don't actually know the groupings for real + // yet, so we might actually inflate a low priority content view incorrectly here and + // have to correct it later in the pipeline. On subsequent inflations (i.e. updates), + // this should inflate the correct view. + isLowPriority = mLowPriorityInflationHelper.shouldUseLowPriorityView(entry); + } RowContentBindParams params = mRowContentBindStage.getStageParams(entry); params.setUseIncreasedCollapsedHeight(useIncreasedCollapsedHeight); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/LowPriorityInflationHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LowPriorityInflationHelper.java similarity index 84% rename from packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/LowPriorityInflationHelper.java rename to packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LowPriorityInflationHelper.java index 518c3f1d19483..dd1f9485a4f3f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/LowPriorityInflationHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/LowPriorityInflationHelper.java @@ -14,13 +14,11 @@ * limitations under the License. */ -package com.android.systemui.statusbar.notification.collection.inflation; +package com.android.systemui.statusbar.notification.collection.legacy; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.flags.FeatureFlags; -import com.android.systemui.statusbar.notification.collection.GroupEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.RowContentBindParams; import com.android.systemui.statusbar.notification.row.RowContentBindStage; @@ -61,6 +59,7 @@ public class LowPriorityInflationHelper { public void recheckLowPriorityViewAndInflate( NotificationEntry entry, ExpandableNotificationRow row) { + mFeatureFlags.checkLegacyPipelineEnabled(); RowContentBindParams params = mRowContentBindStage.getStageParams(entry); final boolean shouldBeLowPriority = shouldUseLowPriorityView(entry); if (!row.isRemoved() && row.isLowPriority() != shouldBeLowPriority) { @@ -74,12 +73,7 @@ public class LowPriorityInflationHelper { * Whether the notification should inflate a low priority version of its content views. */ public boolean shouldUseLowPriorityView(NotificationEntry entry) { - boolean isGroupChild; - if (mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { - isGroupChild = (entry.getParent() != GroupEntry.ROOT_ENTRY); - } else { - isGroupChild = mGroupManager.isChildInGroup(entry); - } - return entry.isAmbient() && !isGroupChild; + mFeatureFlags.checkLegacyPipelineEnabled(); + return entry.isAmbient() && !mGroupManager.isChildInGroup(entry); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/NotifSection.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/NotifSection.kt index 6424e37ad3282..8444287cbf60f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/NotifSection.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/NotifSection.kt @@ -27,8 +27,7 @@ data class NotifSection( val label: String get() = "Section($index, $bucket, \"${sectioner.name}\")" - val headerController: NodeController? - get() = sectioner.headerNodeController + val headerController: NodeController? = sectioner.headerNodeController @PriorityBucket val bucket: Int = sectioner.bucket } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/PipelineState.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/PipelineState.java index 027ac0f66b350..798bfe7f39d08 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/PipelineState.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/PipelineState.java @@ -82,8 +82,8 @@ public class PipelineState { public static final int STATE_GROUPING = 4; public static final int STATE_TRANSFORMING = 5; public static final int STATE_GROUP_STABILIZING = 6; - public static final int STATE_FINALIZE_FILTERING = 7; - public static final int STATE_SORTING = 8; + public static final int STATE_SORTING = 7; + public static final int STATE_FINALIZE_FILTERING = 8; public static final int STATE_FINALIZING = 9; @IntDef(prefix = { "STATE_" }, value = { @@ -94,8 +94,8 @@ public class PipelineState { STATE_GROUPING, STATE_TRANSFORMING, STATE_GROUP_STABILIZING, - STATE_FINALIZE_FILTERING, STATE_SORTING, + STATE_FINALIZE_FILTERING, STATE_FINALIZING, }) @Retention(RetentionPolicy.SOURCE) 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 pkg1Entries = new ArrayList<>(); + ArrayList pkg2Entries = new ArrayList<>(); + ArrayList pkg3Entries = new ArrayList<>(); final NotifSectioner pkg1Sectioner = spy(new PackageSectioner(PACKAGE_1) { - @Nullable @Override public void onEntriesUpdated(List entries) { super.onEntriesUpdated(entries); - validChildren.set(entries.size() == 2); + pkg1Entries.addAll(entries); } }); - mListBuilder.setSectioners(asList(pkg1Sectioner)); + final NotifSectioner pkg2Sectioner = spy(new PackageSectioner(PACKAGE_2) { + @Override + public void onEntriesUpdated(List entries) { + super.onEntriesUpdated(entries); + pkg2Entries.addAll(entries); + } + }); + final NotifSectioner pkg3Sectioner = spy(new PackageSectioner(PACKAGE_3) { + @Override + public void onEntriesUpdated(List entries) { + super.onEntriesUpdated(entries); + pkg3Entries.addAll(entries); + } + }); + mListBuilder.setSectioners(asList(pkg1Sectioner, pkg2Sectioner, pkg3Sectioner)); - addNotif(0, PACKAGE_4); + addNotif(0, PACKAGE_1); addNotif(1, PACKAGE_1); - addNotif(2, PACKAGE_1); + addNotif(2, PACKAGE_3); addNotif(3, PACKAGE_3); + addNotif(4, PACKAGE_3); dispatchBuild(); - verify(pkg1Sectioner, times(1)).onEntriesUpdated(any()); - assertTrue(validChildren.get()); + verify(pkg1Sectioner).onEntriesUpdated(any()); + verify(pkg2Sectioner).onEntriesUpdated(any()); + verify(pkg3Sectioner).onEntriesUpdated(any()); + assertThat(pkg1Entries).containsExactly( + mEntrySet.get(0), + mEntrySet.get(1) + ).inOrder(); + assertThat(pkg2Entries).isEmpty(); + assertThat(pkg3Entries).containsExactly( + mEntrySet.get(2), + mEntrySet.get(3), + mEntrySet.get(4) + ).inOrder(); } @Test @@ -835,13 +865,13 @@ public class ShadeListBuilderTest extends SysuiTestCase { .onBeforeTransformGroups(anyList()); inOrder.verify(promoter, atLeastOnce()) .shouldPromoteToTopLevel(any(NotificationEntry.class)); - inOrder.verify(mOnBeforeFinalizeFilterListener).onBeforeFinalizeFilter(anyList()); - inOrder.verify(preRenderFilter, atLeastOnce()) - .shouldFilterOut(any(NotificationEntry.class), anyLong()); inOrder.verify(mOnBeforeSortListener).onBeforeSort(anyList()); inOrder.verify(section, atLeastOnce()).isInSection(any(ListEntry.class)); inOrder.verify(comparator, atLeastOnce()) .compare(any(ListEntry.class), any(ListEntry.class)); + inOrder.verify(mOnBeforeFinalizeFilterListener).onBeforeFinalizeFilter(anyList()); + inOrder.verify(preRenderFilter, atLeastOnce()) + .shouldFilterOut(any(NotificationEntry.class), anyLong()); inOrder.verify(mOnBeforeRenderListListener).onBeforeRenderList(anyList()); inOrder.verify(mOnRenderListListener).onRenderList(anyList()); } @@ -947,13 +977,11 @@ public class ShadeListBuilderTest extends SysuiTestCase { ); // THEN all the new notifs, including the new GroupEntry, are passed to the listener - assertEquals( - asList( - mEntrySet.get(0), - mBuiltList.get(1), - mEntrySet.get(4)), - listener.mEntriesReceived - ); + assertThat(listener.mEntriesReceived).containsExactly( + mEntrySet.get(0), + mBuiltList.get(1), + mEntrySet.get(4) + ).inOrder(); // Order is a bonus because this listener is before sort } @Test @@ -993,14 +1021,12 @@ public class ShadeListBuilderTest extends SysuiTestCase { ); // THEN all the new notifs, including the new GroupEntry, are passed to the listener - assertEquals( - asList( - mEntrySet.get(0), - mBuiltList.get(2), - mEntrySet.get(7), - mEntrySet.get(1)), - listener.mEntriesReceived - ); + assertThat(listener.mEntriesReceived).containsExactly( + mEntrySet.get(0), + mEntrySet.get(1), + mBuiltList.get(2), + mEntrySet.get(7) + ).inOrder(); // Order is a bonus because this listener is before sort } @Test @@ -1090,10 +1116,94 @@ public class ShadeListBuilderTest extends SysuiTestCase { ); } + @Test + public void testFinalizeFilteringGroupSummaryDoesNotBreakSort() { + // GIVEN children from 3 packages, with one in the middle of the sort order being a group + addNotif(0, PACKAGE_1); + addNotif(1, PACKAGE_2); + addNotif(2, PACKAGE_3); + addNotif(3, PACKAGE_1); + addNotif(4, PACKAGE_2); + addNotif(5, PACKAGE_3); + addGroupSummary(6, PACKAGE_2, GROUP_1); + addGroupChild(7, PACKAGE_2, GROUP_1); + addGroupChild(8, PACKAGE_2, GROUP_1); + + // GIVEN that they should be sorted by package + mListBuilder.setComparators(asList( + new HypeComparator(PACKAGE_1), + new HypeComparator(PACKAGE_2), + new HypeComparator(PACKAGE_3) + )); + + // WHEN a finalize filter removes the summary + mListBuilder.addFinalizeFilter(new NotifFilter("Test") { + @Override + public boolean shouldFilterOut(@NonNull NotificationEntry entry, long now) { + return entry == notif(6).entry; + } + }); + + dispatchBuild(); + + // THEN the notifications remain ordered by package, even though the children were promoted + verifyBuiltList( + notif(0), + notif(3), + notif(1), + notif(4), + notif(7), // promoted child + notif(8), // promoted child + notif(2), + notif(5) + ); + } + + @Test + public void testFinalizeFilteringGroupChildDoesNotBreakSort() { + // GIVEN children from 3 packages, with one in the middle of the sort order being a group + addNotif(0, PACKAGE_1); + addNotif(1, PACKAGE_2); + addNotif(2, PACKAGE_3); + addNotif(3, PACKAGE_1); + addNotif(4, PACKAGE_2); + addNotif(5, PACKAGE_3); + addGroupSummary(6, PACKAGE_2, GROUP_1); + addGroupChild(7, PACKAGE_2, GROUP_1); + addGroupChild(8, PACKAGE_2, GROUP_1); + + // GIVEN that they should be sorted by package + mListBuilder.setComparators(asList( + new HypeComparator(PACKAGE_1), + new HypeComparator(PACKAGE_2), + new HypeComparator(PACKAGE_3) + )); + + // WHEN a finalize filter one of the 2 children from a group + mListBuilder.addFinalizeFilter(new NotifFilter("Test") { + @Override + public boolean shouldFilterOut(@NonNull NotificationEntry entry, long now) { + return entry == notif(7).entry; + } + }); + + dispatchBuild(); + + // THEN the notifications remain ordered by package, even though the children were promoted + verifyBuiltList( + notif(0), + notif(3), + notif(1), + notif(4), + notif(8), // promoted child + notif(2), + notif(5) + ); + } + @Test public void testBrokenGroupNotificationOrdering() { // GIVEN two group children with different sections & without a summary yet - addGroupChild(0, PACKAGE_2, GROUP_1); addNotif(1, PACKAGE_1); addGroupChild(2, PACKAGE_2, GROUP_1); @@ -1224,13 +1334,11 @@ public class ShadeListBuilderTest extends SysuiTestCase { dispatchBuild(); // THEN all the new notifs are passed to the listener out of order - assertEquals( - asList( - mEntrySet.get(0), - mEntrySet.get(1), - mEntrySet.get(2)), - listener.mEntriesReceived - ); + assertThat(listener.mEntriesReceived).containsExactly( + mEntrySet.get(0), + mEntrySet.get(1), + mEntrySet.get(2) + ).inOrder(); // Checking out-of-order input to validate sorted output // THEN the final list is in order verifyBuiltList( @@ -1256,13 +1364,11 @@ public class ShadeListBuilderTest extends SysuiTestCase { dispatchBuild(); // THEN all the new notifs are passed to the listener - assertEquals( - asList( - mEntrySet.get(0), - mEntrySet.get(1), - mEntrySet.get(2)), - listener.mEntriesReceived - ); + assertThat(listener.mEntriesReceived).containsExactly( + mEntrySet.get(0), + mEntrySet.get(1), + mEntrySet.get(2) + ).inOrder(); } @Test @@ -1365,6 +1471,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { assertOrder("ABCDEFG", "ACDEFBG", "ABCDEFG"); // no change assertOrder("ABCDEFG", "ACDEFBXZG", "XZABCDEFG"); // Z and X assertOrder("ABCDEFG", "AXCDEZFBG", "XZABCDEFG"); // Z and X + gap + verify(mStabilityManager, times(4)).onEntryReorderSuppressed(); } @Test @@ -1373,6 +1480,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { assertOrder("ABCDEFG", "ACDEFBG", "ACDEFBG"); // no change assertOrder("ABCDEFG", "ACDEFBXZG", "ACDEFBXZG"); // Z and X assertOrder("ABCDEFG", "AXCDEZFBG", "AXCDEZFBG"); // Z and X + gap + verify(mStabilityManager, never()).onEntryReorderSuppressed(); } @Test @@ -1410,6 +1518,26 @@ public class ShadeListBuilderTest extends SysuiTestCase { // THEN no exception thrown } + @Test + public void testIsSorted() { + Comparator 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. @@ -1815,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/PreparationCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java index bec5174aceba6..c3e10aa3178ff 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static java.util.Objects.requireNonNull; @@ -33,10 +34,12 @@ import android.os.RemoteException; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; +import androidx.annotation.NonNull; import androidx.test.filters.SmallTest; import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.SysuiTestCase; +import com.android.systemui.statusbar.RankingBuilder; import com.android.systemui.statusbar.notification.collection.GroupEntry; import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder; import com.android.systemui.statusbar.notification.collection.ListEntry; @@ -44,8 +47,11 @@ import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater; +import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustmentProvider; +import com.android.systemui.statusbar.notification.collection.listbuilder.NotifSection; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.collection.render.NotifViewBarn; import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager; @@ -60,6 +66,7 @@ import org.mockito.MockitoAnnotations; import org.mockito.Spy; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -79,24 +86,36 @@ public class PreparationCoordinatorTest extends SysuiTestCase { @Captor private ArgumentCaptor mCollectionListenerCaptor; @Captor private ArgumentCaptor mBeforeFilterListenerCaptor; @Captor private ArgumentCaptor mCallbackCaptor; + @Captor private ArgumentCaptor mParamsCaptor; + @Mock private NotifSectioner mNotifSectioner; + @Mock private NotifSection mNotifSection; @Mock private NotifPipeline mNotifPipeline; @Mock private IStatusBarService mService; @Spy private FakeNotifInflater mNotifInflater = new FakeNotifInflater(); + private final TestableAdjustmentProvider mAdjustmentProvider = new TestableAdjustmentProvider(); + + @NonNull + private NotificationEntryBuilder getNotificationEntryBuilder() { + return new NotificationEntryBuilder().setSection(mNotifSection); + } @Before public void setUp() { MockitoAnnotations.initMocks(this); - mEntry = new NotificationEntryBuilder().setParent(ROOT_ENTRY).build(); + mEntry = getNotificationEntryBuilder().setParent(ROOT_ENTRY).build(); mInflationError = new Exception(TEST_MESSAGE); mErrorManager = new NotifInflationErrorManager(); + when(mNotifSection.getSectioner()).thenReturn(mNotifSectioner); + mAdjustmentProvider.setSectionIsLowPriority(false); PreparationCoordinator coordinator = new PreparationCoordinator( mock(PreparationCoordinatorLogger.class), mNotifInflater, mErrorManager, mock(NotifViewBarn.class), + mAdjustmentProvider, mService, TEST_CHILD_BIND_CUTOFF, TEST_MAX_GROUP_DELAY); @@ -150,7 +169,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); // THEN we inflate it - verify(mNotifInflater).inflateViews(eq(mEntry), any()); + verify(mNotifInflater).inflateViews(eq(mEntry), any(), any()); // THEN we filter it out until it's done inflating. assertTrue(mUninflatedFilter.shouldFilterOut(mEntry, 0)); @@ -161,7 +180,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // GIVEN an inflated notification mCollectionListener.onEntryAdded(mEntry); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); - verify(mNotifInflater).inflateViews(eq(mEntry), mCallbackCaptor.capture()); + verify(mNotifInflater).inflateViews(eq(mEntry), any(), mCallbackCaptor.capture()); mCallbackCaptor.getValue().onInflationFinished(mEntry); // WHEN notification is updated @@ -169,7 +188,90 @@ public class PreparationCoordinatorTest extends SysuiTestCase { mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); // THEN we rebind it - verify(mNotifInflater).rebindViews(eq(mEntry), any()); + verify(mNotifInflater).rebindViews(eq(mEntry), any(), any()); + + // THEN we do not filter it because it's not the first inflation. + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testEntrySmartReplyAdditionWillRebindViews() { + // GIVEN an inflated notification + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + verify(mNotifInflater).inflateViews(eq(mEntry), any(), mCallbackCaptor.capture()); + mCallbackCaptor.getValue().onInflationFinished(mEntry); + + // WHEN notification ranking now has smart replies + mEntry.setRanking(new RankingBuilder(mEntry.getRanking()).setSmartReplies("yes").build()); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we rebind it + verify(mNotifInflater).rebindViews(eq(mEntry), any(), any()); + + // THEN we do not filter it because it's not the first inflation. + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testEntryChangedToMinimizedSectionWillRebindViews() { + // GIVEN an inflated notification + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + verify(mNotifInflater).inflateViews(eq(mEntry), + mParamsCaptor.capture(), mCallbackCaptor.capture()); + assertFalse(mParamsCaptor.getValue().isLowPriority()); + mCallbackCaptor.getValue().onInflationFinished(mEntry); + + // WHEN notification moves to a min priority section + mAdjustmentProvider.setSectionIsLowPriority(true); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we rebind it + verify(mNotifInflater).rebindViews(eq(mEntry), mParamsCaptor.capture(), any()); + assertTrue(mParamsCaptor.getValue().isLowPriority()); + + // THEN we do not filter it because it's not the first inflation. + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testMinimizedEntryMovedIntoGroupWillRebindViews() { + // GIVEN an inflated, minimized notification + mAdjustmentProvider.setSectionIsLowPriority(true); + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + verify(mNotifInflater).inflateViews(eq(mEntry), + mParamsCaptor.capture(), mCallbackCaptor.capture()); + assertTrue(mParamsCaptor.getValue().isLowPriority()); + mCallbackCaptor.getValue().onInflationFinished(mEntry); + + // WHEN notification is moved under a parent + NotificationEntryBuilder.setNewParent(mEntry, mock(GroupEntry.class)); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we rebind it as not-minimized + verify(mNotifInflater).rebindViews(eq(mEntry), mParamsCaptor.capture(), any()); + assertFalse(mParamsCaptor.getValue().isLowPriority()); + + // THEN we do not filter it because it's not the first inflation. + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testEntryRankChangeWillNotRebindViews() { + // GIVEN an inflated notification + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + verify(mNotifInflater).inflateViews(eq(mEntry), any(), mCallbackCaptor.capture()); + mCallbackCaptor.getValue().onInflationFinished(mEntry); + + // WHEN notification ranking changes rank, which does not affect views + mEntry.setRanking(new RankingBuilder(mEntry.getRanking()).setRank(100).build()); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we do not rebind it + verify(mNotifInflater, never()).rebindViews(eq(mEntry), any(), any()); // THEN we do not filter it because it's not the first inflation. assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); @@ -180,7 +282,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // GIVEN an inflated notification mCollectionListener.onEntryAdded(mEntry); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); - verify(mNotifInflater).inflateViews(eq(mEntry), mCallbackCaptor.capture()); + verify(mNotifInflater).inflateViews(eq(mEntry), any(), mCallbackCaptor.capture()); mCallbackCaptor.getValue().onInflationFinished(mEntry); // THEN it isn't filtered from shade list @@ -191,13 +293,13 @@ public class PreparationCoordinatorTest extends SysuiTestCase { public void testCutoffGroupChildrenNotInflated() { // WHEN there is a new notification group is posted int id = 0; - NotificationEntry summary = new NotificationEntryBuilder() + NotificationEntry summary = getNotificationEntryBuilder() .setOverrideGroupKey(TEST_GROUP_KEY) .setId(id++) .build(); List children = new ArrayList<>(); for (int i = 0; i < TEST_CHILD_BIND_CUTOFF + 1; i++) { - NotificationEntry child = new NotificationEntryBuilder() + NotificationEntry child = getNotificationEntryBuilder() .setOverrideGroupKey(TEST_GROUP_KEY) .setId(id++) .build(); @@ -224,9 +326,9 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // THEN we inflate up to the cut-off only for (int i = 0; i < children.size(); i++) { if (i < TEST_CHILD_BIND_CUTOFF) { - verify(mNotifInflater).inflateViews(eq(children.get(i)), any()); + verify(mNotifInflater).inflateViews(eq(children.get(i)), any(), any()); } else { - verify(mNotifInflater, never()).inflateViews(eq(children.get(i)), any()); + verify(mNotifInflater, never()).inflateViews(eq(children.get(i)), any(), any()); } } } @@ -236,9 +338,9 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // GIVEN a newly-posted group with a summary and two children final GroupEntry group = new GroupEntryBuilder() .setCreationTime(400) - .setSummary(new NotificationEntryBuilder().setId(1).build()) - .addChild(new NotificationEntryBuilder().setId(2).build()) - .addChild(new NotificationEntryBuilder().setId(3).build()) + .setSummary(getNotificationEntryBuilder().setId(1).build()) + .addChild(getNotificationEntryBuilder().setId(2).build()) + .addChild(getNotificationEntryBuilder().setId(3).build()) .build(); fireAddEvents(List.of(group)); final NotificationEntry child0 = group.getChildren().get(0); @@ -256,9 +358,9 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // GIVEN a newly-posted group with a summary and two children final GroupEntry group = new GroupEntryBuilder() .setCreationTime(400) - .setSummary(new NotificationEntryBuilder().setId(1).build()) - .addChild(new NotificationEntryBuilder().setId(2).build()) - .addChild(new NotificationEntryBuilder().setId(3).build()) + .setSummary(getNotificationEntryBuilder().setId(1).build()) + .addChild(getNotificationEntryBuilder().setId(2).build()) + .addChild(getNotificationEntryBuilder().setId(3).build()) .build(); fireAddEvents(List.of(group)); final NotificationEntry summary = group.getSummary(); @@ -281,9 +383,9 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // GIVEN a newly-posted group with a summary and two children final GroupEntry group = new GroupEntryBuilder() .setCreationTime(400) - .setSummary(new NotificationEntryBuilder().setId(1).build()) - .addChild(new NotificationEntryBuilder().setId(2).build()) - .addChild(new NotificationEntryBuilder().setId(3).build()) + .setSummary(getNotificationEntryBuilder().setId(1).build()) + .addChild(getNotificationEntryBuilder().setId(2).build()) + .addChild(getNotificationEntryBuilder().setId(3).build()) .build(); fireAddEvents(List.of(group)); final NotificationEntry summary = group.getSummary(); @@ -307,9 +409,9 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // GIVEN a newly-posted group with a summary and two children final GroupEntry group = new GroupEntryBuilder() .setCreationTime(400) - .setSummary(new NotificationEntryBuilder().setId(1).build()) - .addChild(new NotificationEntryBuilder().setId(2).build()) - .addChild(new NotificationEntryBuilder().setId(3).build()) + .setSummary(getNotificationEntryBuilder().setId(1).build()) + .addChild(getNotificationEntryBuilder().setId(2).build()) + .addChild(getNotificationEntryBuilder().setId(3).build()) .build(); fireAddEvents(List.of(group)); final NotificationEntry child0 = group.getChildren().get(0); @@ -324,19 +426,21 @@ public class PreparationCoordinatorTest extends SysuiTestCase { } private static class FakeNotifInflater implements NotifInflater { - private Map mInflateCallbacks = new HashMap<>(); + private final Map mInflateCallbacks = new HashMap<>(); @Override - public void inflateViews(NotificationEntry entry, InflationCallback callback) { + public void inflateViews(@NonNull NotificationEntry entry, @NonNull Params params, + @NonNull InflationCallback callback) { mInflateCallbacks.put(entry, callback); } @Override - public void rebindViews(NotificationEntry entry, InflationCallback callback) { + public void rebindViews(@NonNull NotificationEntry entry, @NonNull Params params, + @NonNull InflationCallback callback) { } @Override - public void abortInflation(NotificationEntry entry) { + public void abortInflation(@NonNull NotificationEntry entry) { } public InflationCallback getInflateCallback(NotificationEntry entry) { @@ -365,4 +469,12 @@ public class PreparationCoordinatorTest extends SysuiTestCase { private static final String TEST_GROUP_KEY = "TEST_GROUP_KEY"; private static final int TEST_CHILD_BIND_CUTOFF = 9; private static final int TEST_MAX_GROUP_DELAY = 100; + + private class TestableAdjustmentProvider extends NotifUiAdjustmentProvider { + private void setSectionIsLowPriority(boolean lowPriority) { + setLowPrioritySections(lowPriority + ? Collections.singleton(mNotifSection.getSectioner()) + : Collections.emptyList()); + } + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinatorTest.java index 2091cf8cb0283..abe33aae7fc68 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RankingCoordinatorTest.java @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator; import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_AMBIENT; import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_NOTIFICATION_LIST; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; @@ -28,8 +29,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.Notification; -import android.service.notification.NotificationListenerService; -import android.service.notification.StatusBarNotification; +import android.app.NotificationManager; import android.testing.AndroidTestingRunner; import androidx.annotation.Nullable; @@ -38,10 +38,12 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.RankingBuilder; +import com.android.systemui.statusbar.SbnBuilder; import com.android.systemui.statusbar.notification.collection.ListEntry; import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; +import com.android.systemui.statusbar.notification.collection.inflation.NotifUiAdjustmentProvider; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner; import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider; @@ -54,7 +56,6 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import java.util.ArrayList; @@ -66,12 +67,11 @@ public class RankingCoordinatorTest extends SysuiTestCase { @Mock private StatusBarStateController mStatusBarStateController; @Mock private HighPriorityProvider mHighPriorityProvider; + @Mock private NotifUiAdjustmentProvider mAdjustmentProvider; @Mock private NotifPipeline mNotifPipeline; @Mock private NodeController mAlertingHeaderController; @Mock private NodeController mSilentNodeController; @Mock private SectionHeaderController mSilentHeaderController; - @Mock private NotificationListenerService.Ranking mRanking; - @Mock private StatusBarNotification mSbn; @Captor private ArgumentCaptor mNotifFilterCaptor; @@ -89,12 +89,14 @@ public class RankingCoordinatorTest extends SysuiTestCase { public void setup() { MockitoAnnotations.initMocks(this); mRankingCoordinator = new RankingCoordinator( - mStatusBarStateController, mHighPriorityProvider, mAlertingHeaderController, - mSilentHeaderController, mSilentNodeController); + mStatusBarStateController, + mHighPriorityProvider, + mAdjustmentProvider, + mAlertingHeaderController, + mSilentHeaderController, + mSilentNodeController); mEntry = spy(new NotificationEntryBuilder().build()); - mRanking = spy(getRankingForUnfilteredNotif().build()); - mEntry.setRanking(mRanking); - when(mEntry.getSbn()).thenReturn(mSbn); + mEntry.setRanking(getRankingForUnfilteredNotif().build()); mRankingCoordinator.attach(mNotifPipeline); verify(mNotifPipeline, times(2)).addPreGroupFilter(mNotifFilterCaptor.capture()); @@ -109,23 +111,19 @@ public class RankingCoordinatorTest extends SysuiTestCase { @Test public void testSilentHeaderClearableChildrenUpdate() { - StatusBarNotification sbn = Mockito.mock(StatusBarNotification.class); - Mockito.doReturn("key").when(sbn).getKey(); - Mockito.doReturn(Mockito.mock(Notification.class)).when(sbn).getNotification(); - NotificationEntry entry = new NotificationEntryBuilder().setSbn(sbn).build(); - ListEntry listEntry = new ListEntry("key", 0L) { + ListEntry listEntry = new ListEntry(mEntry.getKey(), 0L) { @Nullable @Override public NotificationEntry getRepresentativeEntry() { - return entry; + return mEntry; } }; - Mockito.doReturn(true).when(sbn).isClearable(); + setRankingAmbient(false); + setSbnClearable(true); mSilentSectioner.onEntriesUpdated(Arrays.asList(listEntry)); - when(mRanking.isAmbient()).thenReturn(false); verify(mSilentHeaderController).setClearSectionEnabled(eq(true)); - mRankingCoordinator.resetClearAllFlags(); - Mockito.doReturn(false).when(sbn).isClearable(); + + setSbnClearable(false); mSilentSectioner.onEntriesUpdated(Arrays.asList(listEntry)); verify(mSilentHeaderController).setClearSectionEnabled(eq(false)); } @@ -204,7 +202,7 @@ public class RankingCoordinatorTest extends SysuiTestCase { public void testIncludeInSectionSilent() { // GIVEN the entry isn't high priority when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mRanking.isAmbient()).thenReturn(false); + setRankingAmbient(false); // THEN entry is in the silent section assertFalse(mAlertingSectioner.isInSection(mEntry)); @@ -213,24 +211,23 @@ public class RankingCoordinatorTest extends SysuiTestCase { @Test public void testMinSection() { - when(mEntry.getRanking()).thenReturn(mRanking); when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mRanking.isAmbient()).thenReturn(true); + setRankingAmbient(true); assertInSection(mEntry, mMinimizedSectioner); } @Test public void testSilentSection() { when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mRanking.isAmbient()).thenReturn(false); + setRankingAmbient(false); assertInSection(mEntry, mSilentSectioner); } @Test public void testClearableSilentSection() { when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mSbn.isClearable()).thenReturn(true); - when(mRanking.isAmbient()).thenReturn(false); + setSbnClearable(true); + setRankingAmbient(false); mSilentSectioner.onEntriesUpdated(Arrays.asList(mEntry)); verify(mSilentHeaderController).setClearSectionEnabled(eq(true)); } @@ -238,17 +235,17 @@ public class RankingCoordinatorTest extends SysuiTestCase { @Test public void testClearableMinimizedSection() { when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mSbn.isClearable()).thenReturn(true); - when(mRanking.isAmbient()).thenReturn(true); + setSbnClearable(true); + setRankingAmbient(true); mMinimizedSectioner.onEntriesUpdated(Arrays.asList(mEntry)); verify(mSilentHeaderController).setClearSectionEnabled(eq(true)); } @Test public void testNotClearableSilentSection() { - when(mSbn.isClearable()).thenReturn(false); + setSbnClearable(false); when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mRanking.isAmbient()).thenReturn(false); + setRankingAmbient(false); mSilentSectioner.onEntriesUpdated(Arrays.asList(mEntry)); mMinimizedSectioner.onEntriesUpdated(Arrays.asList(mEntry)); mAlertingSectioner.onEntriesUpdated(Arrays.asList(mEntry)); @@ -257,9 +254,9 @@ public class RankingCoordinatorTest extends SysuiTestCase { @Test public void testNotClearableMinimizedSection() { - when(mSbn.isClearable()).thenReturn(false); + setSbnClearable(false); when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false); - when(mRanking.isAmbient()).thenReturn(true); + setRankingAmbient(true); mSilentSectioner.onEntriesUpdated(Arrays.asList(mEntry)); mMinimizedSectioner.onEntriesUpdated(Arrays.asList(mEntry)); mAlertingSectioner.onEntriesUpdated(Arrays.asList(mEntry)); @@ -277,9 +274,24 @@ public class RankingCoordinatorTest extends SysuiTestCase { } private RankingBuilder getRankingForUnfilteredNotif() { - return new RankingBuilder() - .setKey(mEntry.getKey()) + return new RankingBuilder(mEntry.getRanking()) .setSuppressedVisualEffects(0) .setSuspended(false); } + + private void setSbnClearable(boolean clearable) { + mEntry.setSbn(new SbnBuilder(mEntry.getSbn()) + .setFlag(mContext, Notification.FLAG_NO_CLEAR, !clearable) + .build()); + assertEquals(clearable, mEntry.getSbn().isClearable()); + } + + private void setRankingAmbient(boolean ambient) { + mEntry.setRanking(new RankingBuilder(mEntry.getRanking()) + .setImportance(ambient + ? NotificationManager.IMPORTANCE_MIN + : NotificationManager.IMPORTANCE_DEFAULT) + .build()); + assertEquals(ambient, mEntry.getRanking().isAmbient()); + } } 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); - } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java index ed42ac3efe806..0d996025392f7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java @@ -70,8 +70,8 @@ import com.android.systemui.statusbar.notification.NotificationFilter; import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationRankingManager; -import com.android.systemui.statusbar.notification.collection.inflation.LowPriorityInflationHelper; import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinderImpl; +import com.android.systemui.statusbar.notification.collection.legacy.LowPriorityInflationHelper; import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy; import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider; import com.android.systemui.statusbar.notification.icon.IconBuilder; @@ -283,6 +283,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase { mRowBinder = new NotificationRowBinderImpl( mContext, + mFeatureFlags, new NotificationMessagingUtil(mContext), mRemoteInputManager, mLockscreenUserManager,