From e4a15a4d72f8f59ddae86a7a9796fc4e55532272 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Sat, 6 Nov 2021 08:53:09 -0400 Subject: [PATCH] New Pipeline: Fix minimized notifications not appearing minimized. * The bug was that the LowPriorityInflationHelper checks the parent after a delay to inflate an empty ExpandableNotificationRow, by which time the ListEntry's parent was cleared by the FinalizeFilter. In order to solve this, we need to pass state into the inflation pipeline for this particular parameter. I created the NotifInflater.Params class to hold this, and then shoehorned that class into the not-yet-modernized NotificationRowBinder interface/impl. * Another bug I discovered when looking for params like this was that a ranking update (such as the addition of SmartReplies) would not result in reinflating the notification, because the system had no way to detect that a ranking update would affect the view itself. This is why I added the NotifUiAdjustment class (copied, kotlinized, and tweaked to include the 'minimized' state from the NotificationUiAdjustment). Fixes: 205360544 Test: atest SystemUITests Change-Id: Ic1aa2dded98b14efe0219684aa754549d9d53d81 --- .../NotificationViewHierarchyManager.java | 2 +- .../dagger/StatusBarDependenciesModule.java | 4 +- .../NotificationEntryManager.java | 4 +- .../collection/NotifInflaterImpl.java | 24 +-- .../collection/ShadeListBuilder.java | 7 +- .../coordinator/PreparationCoordinator.java | 82 +++++++-- .../coordinator/RankingCoordinator.java | 6 + .../{NotifInflater.java => NotifInflater.kt} | 25 +-- .../collection/inflation/NotifUiAdjustment.kt | 95 ++++++++++ .../inflation/NotifUiAdjustmentProvider.kt | 63 +++++++ .../inflation/NotificationRowBinder.java | 4 +- .../inflation/NotificationRowBinderImpl.java | 36 +++- .../LowPriorityInflationHelper.java | 14 +- .../collection/listbuilder/NotifSection.kt | 3 +- .../NotificationViewHierarchyManagerTest.java | 2 +- .../collection/NotificationEntryBuilder.java | 16 ++ .../PreparationCoordinatorTest.java | 162 +++++++++++++++--- .../coordinator/RankingCoordinatorTest.java | 10 +- ...NotificationEntryManagerInflationTest.java | 3 +- 19 files changed, 465 insertions(+), 97 deletions(-) rename packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/{NotifInflater.java => NotifInflater.kt} (73%) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustment.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotifUiAdjustmentProvider.kt rename packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/{inflation => legacy}/LowPriorityInflationHelper.java (84%) 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/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index 464250090e1a1..4cdf8927c68b3 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 @@ -1041,8 +1041,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 71979735a030f..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 @@ -24,6 +24,7 @@ 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; @@ -33,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; @@ -49,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; @@ -59,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; @@ -72,6 +77,7 @@ 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); 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/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java index cf58c63e3d263..acd62b2fabe7a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java @@ -44,7 +44,7 @@ import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationActivityStarter; 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.logging.NotificationLogger; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java index fc94262465b13..b91f7e6b6169e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java @@ -30,6 +30,7 @@ import android.service.notification.StatusBarNotification; import com.android.internal.logging.InstanceId; import com.android.systemui.statusbar.RankingBuilder; import com.android.systemui.statusbar.SbnBuilder; +import com.android.systemui.statusbar.notification.collection.listbuilder.NotifSection; import com.android.systemui.util.time.FakeSystemClock; import java.util.ArrayList; @@ -53,6 +54,7 @@ public class NotificationEntryBuilder { /* ListEntry properties */ private GroupEntry mParent; + private NotifSection mNotifSection; /* If set, use this creation time instead of mClock.uptimeMillis */ private long mCreationTime = -1; @@ -71,6 +73,11 @@ public class NotificationEntryBuilder { mCreationTime = source.getCreationTime(); } + /** Update an the parent on an existing entry */ + public static void setNewParent(NotificationEntry entry, GroupEntry parent) { + entry.setParent(parent); + } + /** Build a new instance of NotificationEntry */ public NotificationEntry build() { return buildOrApply(null); @@ -103,6 +110,7 @@ public class NotificationEntryBuilder { /* ListEntry properties */ entry.setParent(mParent); + entry.getAttachState().setSection(mNotifSection); entry.getAttachState().setStableIndex(mStableIndex); return entry; } @@ -115,6 +123,14 @@ public class NotificationEntryBuilder { return this; } + /** + * Sets the parent. + */ + public NotificationEntryBuilder setSection(@Nullable NotifSection section) { + mNotifSection = section; + return this; + } + /** * Sets the SBN directly. If set, causes all calls to delegated SbnBuilder methods to be * ignored. 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 1d45aff00a035..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 @@ -43,6 +43,7 @@ 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; @@ -66,6 +67,7 @@ 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; @@ -87,8 +89,12 @@ 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()); mEntry.setRanking(getRankingForUnfilteredNotif().build()); 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,