From 2265301d5320292cc9b819aee4c8163e69340175 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Wed, 22 Jan 2020 12:56:25 -0800 Subject: [PATCH 1/2] Add listener hook before finalize filter. Add a hook so coordinators can listen before the FinalizeFilter stage (previously named preRenderFilter and renamed in this CL b/c OnBeforePreRenderFilter sounds somewhat awkward). This'll be helpful for moving inflation-related filtering after sort/group stage. Bug: 112656837 Test: atest ShadeListBuilderTest Change-Id: If7c7b7375b664ddcd0bb75b0937070a03db9b0e3 --- .../collection/NotifPipeline.java | 15 ++++++-- .../collection/ShadeListBuilder.java | 37 ++++++++++++++----- .../coordinator/BubbleCoordinator.java | 2 +- .../coordinator/KeyguardCoordinator.java | 2 +- .../coordinator/PreparationCoordinator.java | 4 +- .../OnBeforeFinalizeFilterListener.java | 34 +++++++++++++++++ .../collection/listbuilder/PipelineState.java | 4 +- .../listbuilder/ShadeListBuilderLogger.kt | 4 +- .../listbuilder/pluggable/NotifFilter.java | 4 +- .../collection/ShadeListBuilderTest.java | 17 ++++++--- .../coordinator/KeyguardCoordinatorTest.java | 2 +- .../PreparationCoordinatorTest.java | 2 +- 12 files changed, 96 insertions(+), 31 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/OnBeforeFinalizeFilterListener.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java index 44cec966ba555..14903cd7bb27a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.java @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.collection; +import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeSortListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeTransformGroupsListener; @@ -63,7 +64,7 @@ import javax.inject.Singleton; * 6. Top-level entries are assigned sections by NotifSections ({@link #setSections}) * 7. Top-level entries within the same section are sorted by NotifComparators * ({@link #setComparators}) - * 8. Pre-render filters are fired on each notification ({@link #addPreRenderFilter}) + * 8. Finalize filters are fired on each notification ({@link #addFinalizeFilter}) * 9. OnBeforeRenderListListeners are fired ({@link #addOnBeforeRenderListListener}) * 9. The list is handed off to the view layer to be rendered */ @@ -168,6 +169,14 @@ public class NotifPipeline implements CommonNotifCollection { mShadeListBuilder.setComparators(comparators); } + /** + * Called after notifs have been filtered once, grouped, and sorted but before the final + * filtering. + */ + public void addOnBeforeFinalizeFilterListener(OnBeforeFinalizeFilterListener listener) { + mShadeListBuilder.addOnBeforeFinalizeFilterListener(listener); + } + /** * Registers a filter with the pipeline to filter right before rendering the list (after * pre-group filtering, grouping, promoting and sorting occurs). Filters are @@ -175,8 +184,8 @@ public class NotifPipeline implements CommonNotifCollection { * true, the notification is removed from the pipeline (and no other filters are called on that * notif). */ - public void addPreRenderFilter(NotifFilter filter) { - mShadeListBuilder.addPreRenderFilter(filter); + public void addFinalizeFilter(NotifFilter filter) { + mShadeListBuilder.addFinalizeFilter(filter); } /** 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 76311207fe6fe..5b73b1aa73882 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 @@ -18,11 +18,11 @@ package com.android.systemui.statusbar.notification.collection; import static com.android.systemui.statusbar.notification.collection.GroupEntry.ROOT_ENTRY; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_BUILD_STARTED; +import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_FINALIZE_FILTERING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_FINALIZING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_GROUPING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_IDLE; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_PRE_GROUP_FILTERING; -import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_PRE_RENDER_FILTERING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_RESETTING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_SORTING; import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_TRANSFORMING; @@ -36,6 +36,7 @@ import androidx.annotation.NonNull; import com.android.systemui.Dumpable; import com.android.systemui.dump.DumpManager; +import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeSortListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeTransformGroupsListener; @@ -82,7 +83,7 @@ public class ShadeListBuilder implements Dumpable { private final List mNotifPreGroupFilters = new ArrayList<>(); private final List mNotifPromoters = new ArrayList<>(); - private final List mNotifPreRenderFilters = new ArrayList<>(); + private final List mNotifFinalizeFilters = new ArrayList<>(); private final List mNotifComparators = new ArrayList<>(); private final List mNotifSections = new ArrayList<>(); @@ -90,6 +91,8 @@ public class ShadeListBuilder implements Dumpable { new ArrayList<>(); private final List mOnBeforeSortListeners = new ArrayList<>(); + private final List mOnBeforeFinalizeFilterListeners = + new ArrayList<>(); private final List mOnBeforeRenderListListeners = new ArrayList<>(); @Nullable private OnRenderListListener mOnRenderListListener; @@ -142,6 +145,13 @@ public class ShadeListBuilder implements Dumpable { mOnBeforeSortListeners.add(listener); } + void addOnBeforeFinalizeFilterListener(OnBeforeFinalizeFilterListener listener) { + Assert.isMainThread(); + + mPipelineState.requireState(STATE_IDLE); + mOnBeforeFinalizeFilterListeners.add(listener); + } + void addOnBeforeRenderListListener(OnBeforeRenderListListener listener) { Assert.isMainThread(); @@ -157,12 +167,12 @@ public class ShadeListBuilder implements Dumpable { filter.setInvalidationListener(this::onPreGroupFilterInvalidated); } - void addPreRenderFilter(NotifFilter filter) { + void addFinalizeFilter(NotifFilter filter) { Assert.isMainThread(); mPipelineState.requireState(STATE_IDLE); - mNotifPreRenderFilters.add(filter); - filter.setInvalidationListener(this::onPreRenderFilterInvalidated); + mNotifFinalizeFilters.add(filter); + filter.setInvalidationListener(this::onFinalizeFilterInvalidated); } void addPromoter(NotifPromoter promoter) { @@ -237,12 +247,12 @@ public class ShadeListBuilder implements Dumpable { rebuildListIfBefore(STATE_SORTING); } - private void onPreRenderFilterInvalidated(NotifFilter filter) { + private void onFinalizeFilterInvalidated(NotifFilter filter) { Assert.isMainThread(); - mLogger.logPreRenderFilterInvalidated(filter.getName(), mPipelineState.getState()); + mLogger.logFinalizeFilterInvalidated(filter.getName(), mPipelineState.getState()); - rebuildListIfBefore(STATE_PRE_RENDER_FILTERING); + rebuildListIfBefore(STATE_FINALIZE_FILTERING); } private void onNotifComparatorInvalidated(NotifComparator comparator) { @@ -298,8 +308,9 @@ public class ShadeListBuilder implements Dumpable { // Step 6: Filter out entries after pre-group filtering, grouping, promoting and sorting // Now filters can see grouping information to determine whether to filter or not. - mPipelineState.incrementTo(STATE_PRE_RENDER_FILTERING); - filterNotifs(mNotifList, mNewNotifList, mNotifPreRenderFilters); + dispatchOnBeforeFinalizeFilter(mReadOnlyNotifList); + mPipelineState.incrementTo(STATE_FINALIZE_FILTERING); + filterNotifs(mNotifList, mNewNotifList, mNotifFinalizeFilters); applyNewNotifList(); pruneIncompleteGroups(mNotifList); @@ -772,6 +783,12 @@ public class ShadeListBuilder implements Dumpable { } } + private void dispatchOnBeforeFinalizeFilter(List entries) { + for (int i = 0; i < mOnBeforeFinalizeFilterListeners.size(); i++) { + mOnBeforeFinalizeFilterListeners.get(i).onBeforeFinalizeFilter(entries); + } + } + private void dispatchOnBeforeRenderList(List entries) { for (int i = 0; i < mOnBeforeRenderListListeners.size(); i++) { mOnBeforeRenderListListeners.get(i).onBeforeRenderList(entries); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java index 8b2a07d00378d..370de838b9039 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/BubbleCoordinator.java @@ -77,7 +77,7 @@ public class BubbleCoordinator implements Coordinator { public void attach(NotifPipeline pipeline) { mNotifPipeline = pipeline; mNotifPipeline.addNotificationDismissInterceptor(mDismissInterceptor); - mNotifPipeline.addPreRenderFilter(mNotifFilter); + mNotifPipeline.addFinalizeFilter(mNotifFilter); mBubbleController.addNotifCallback(mNotifCallback); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java index a26ee5450d60a..aaf71f58cb5c1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java @@ -87,7 +87,7 @@ public class KeyguardCoordinator implements Coordinator { @Override public void attach(NotifPipeline pipeline) { setupInvalidateNotifListCallbacks(); - pipeline.addPreRenderFilter(mNotifFilter); + pipeline.addFinalizeFilter(mNotifFilter); } private final NotifFilter mNotifFilter = new NotifFilter(TAG) { 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 1c8fdac4c51bb..2f4b2e535c610 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 @@ -69,8 +69,8 @@ public class PreparationCoordinator implements Coordinator { @Override public void attach(NotifPipeline pipeline) { pipeline.addCollectionListener(mNotifCollectionListener); - pipeline.addPreRenderFilter(mNotifInflationErrorFilter); - pipeline.addPreRenderFilter(mNotifInflatingFilter); + pipeline.addFinalizeFilter(mNotifInflationErrorFilter); + pipeline.addFinalizeFilter(mNotifInflatingFilter); } private final NotifCollectionListener mNotifCollectionListener = new NotifCollectionListener() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/OnBeforeFinalizeFilterListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/OnBeforeFinalizeFilterListener.java new file mode 100644 index 0000000000000..086661ea219b5 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/OnBeforeFinalizeFilterListener.java @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification.collection.listbuilder; + +import com.android.systemui.statusbar.notification.collection.ListEntry; +import com.android.systemui.statusbar.notification.collection.NotifPipeline; + +import java.util.List; + +/** See {@link NotifPipeline#addOnBeforeFinalizeFilterListener(OnBeforeFinalizeFilterListener)} */ +public interface OnBeforeFinalizeFilterListener { + /** + * Called after the notif list has been filtered, grouped, and sorted but before they are + * filtered one last time before rendering. + * + * @param entries The current list of top-level entries. Note that this is a live view into the + * current list and will change whenever the pipeline is rerun. + */ + void onBeforeFinalizeFilter(List entries); +} 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 1897ba2319ac5..f1f7d632b6f8d 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,7 +82,7 @@ public class PipelineState { public static final int STATE_GROUPING = 4; public static final int STATE_TRANSFORMING = 5; public static final int STATE_SORTING = 6; - public static final int STATE_PRE_RENDER_FILTERING = 7; + public static final int STATE_FINALIZE_FILTERING = 7; public static final int STATE_FINALIZING = 8; @IntDef(prefix = { "STATE_" }, value = { @@ -93,7 +93,7 @@ public class PipelineState { STATE_GROUPING, STATE_TRANSFORMING, STATE_SORTING, - STATE_PRE_RENDER_FILTERING, + STATE_FINALIZE_FILTERING, STATE_FINALIZING, }) @Retention(RetentionPolicy.SOURCE) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt index 6e15043973f76..763547ce1638c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt @@ -87,12 +87,12 @@ class ShadeListBuilderLogger @Inject constructor( }) } - fun logPreRenderFilterInvalidated(name: String, pipelineState: Int) { + fun logFinalizeFilterInvalidated(name: String, pipelineState: Int) { buffer.log(TAG, DEBUG, { str1 = name int1 = pipelineState }, { - """Pre-render NotifFilter "$str1" invalidated; pipeline state is $int1""" + """Finalize NotifFilter "$str1" invalidated; pipeline state is $int1""" }) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java index 8f575cdd89187..9edb5fc89d8fa 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifFilter.java @@ -21,7 +21,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; /** * Pluggable for participating in notif filtering. - * See {@link NotifPipeline#addPreGroupFilter} and {@link NotifPipeline#addPreRenderFilter}. + * See {@link NotifPipeline#addPreGroupFilter} and {@link NotifPipeline#addFinalizeFilter}. */ public abstract class NotifFilter extends Pluggable { protected NotifFilter(String name) { @@ -37,7 +37,7 @@ public abstract class NotifFilter extends Pluggable { * @param entry The entry in question. * If this filter is registered via {@link NotifPipeline#addPreGroupFilter}, * this entry will not have any grouping nor sorting information. - * If this filter is registered via {@link NotifPipeline#addPreRenderFilter}, + * If this filter is registered via {@link NotifPipeline#addFinalizeFilter}, * this entry will have grouping and sorting information. * @param now A timestamp in SystemClock.uptimeMillis that represents "now" for the purposes of * pipeline execution. This value will be the same for all pluggable calls made diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index 546fce851fd1b..d7c72799e1802 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -42,6 +42,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.dump.DumpManager; import com.android.systemui.statusbar.notification.collection.ShadeListBuilder.OnRenderListListener; +import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeSortListener; import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeTransformGroupsListener; @@ -84,6 +85,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { @Mock private NotifCollection mNotifCollection; @Spy private OnBeforeTransformGroupsListener mOnBeforeTransformGroupsListener; @Spy private OnBeforeSortListener mOnBeforeSortListener; + @Spy private OnBeforeFinalizeFilterListener mOnBeforeFinalizeFilterListener; @Spy private OnBeforeRenderListListener mOnBeforeRenderListListener; @Spy private OnRenderListListener mOnRenderListListener = list -> mBuiltList = list; @@ -387,7 +389,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { NotifFilter preGroupFilter = spy(new PackageFilter(PACKAGE_2)); NotifFilter preRenderFilter = spy(new PackageFilter(PACKAGE_2)); mListBuilder.addPreGroupFilter(preGroupFilter); - mListBuilder.addPreRenderFilter(preRenderFilter); + mListBuilder.addFinalizeFilter(preRenderFilter); // WHEN the pipeline is kicked off on a list of notifs addNotif(0, PACKAGE_1); @@ -423,7 +425,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { public void testPreRenderNotifsAreFiltered() { // GIVEN a NotifFilter that filters out a specific package NotifFilter filter1 = spy(new PackageFilter(PACKAGE_2)); - mListBuilder.addPreRenderFilter(filter1); + mListBuilder.addFinalizeFilter(filter1); // WHEN the pipeline is kicked off on a list of notifs addNotif(0, PACKAGE_1); @@ -454,7 +456,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { final String filterTag = "FILTER_ME"; // GIVEN a NotifFilter that filters out notifications with a tag NotifFilter filter1 = spy(new NotifFilterWithTag(filterTag)); - mListBuilder.addPreRenderFilter(filter1); + mListBuilder.addFinalizeFilter(filter1); // WHEN the pipeline is kicked off on a list of notifs addGroupChildWithTag(0, PACKAGE_2, GROUP_1, filterTag); @@ -742,8 +744,9 @@ public class ShadeListBuilderTest extends SysuiTestCase { mListBuilder.addOnBeforeSortListener(mOnBeforeSortListener); mListBuilder.setComparators(Collections.singletonList(comparator)); mListBuilder.setSections(Arrays.asList(section)); + mListBuilder.addOnBeforeFinalizeFilterListener(mOnBeforeFinalizeFilterListener); + mListBuilder.addFinalizeFilter(preRenderFilter); mListBuilder.addOnBeforeRenderListListener(mOnBeforeRenderListListener); - mListBuilder.addPreRenderFilter(preRenderFilter); // WHEN a few new notifs are added addNotif(0, PACKAGE_1); @@ -763,6 +766,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { mOnBeforeSortListener, section, comparator, + mOnBeforeFinalizeFilterListener, preRenderFilter, mOnBeforeRenderListListener, mOnRenderListListener); @@ -777,6 +781,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { 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()); @@ -1075,7 +1080,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // GIVEN a PreRenderNotifFilter that gets invalidated during the finalizing stage NotifFilter filter = new PackageFilter(PACKAGE_5); OnBeforeRenderListListener listener = (list) -> filter.invalidateList(); - mListBuilder.addPreRenderFilter(filter); + mListBuilder.addFinalizeFilter(filter); mListBuilder.addOnBeforeRenderListListener(listener); // WHEN we try to run the pipeline and the PreRenderFilter is invalidated @@ -1090,7 +1095,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // GIVEN a PreRenderFilter that gets invalidated during the grouping stage NotifFilter filter = new PackageFilter(PACKAGE_5); OnBeforeTransformGroupsListener listener = (list) -> filter.invalidateList(); - mListBuilder.addPreRenderFilter(filter); + mListBuilder.addFinalizeFilter(filter); mListBuilder.addOnBeforeTransformGroupsListener(listener); // WHEN we try to run the pipeline and the filter is invalidated diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java index 5866d90f62bf6..c4f3a1611afc6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinatorTest.java @@ -88,7 +88,7 @@ public class KeyguardCoordinatorTest extends SysuiTestCase { ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(NotifFilter.class); mKeyguardCoordinator.attach(mNotifPipeline); - verify(mNotifPipeline, times(1)).addPreRenderFilter(filterCaptor.capture()); + verify(mNotifPipeline, times(1)).addFinalizeFilter(filterCaptor.capture()); mKeyguardFilter = filterCaptor.getValue(); } 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 61a4fbe3526cf..4667e910e85a8 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 @@ -79,7 +79,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(NotifFilter.class); mCoordinator.attach(mNotifPipeline); - verify(mNotifPipeline, times(2)).addPreRenderFilter(filterCaptor.capture()); + verify(mNotifPipeline, times(2)).addFinalizeFilter(filterCaptor.capture()); List filters = filterCaptor.getAllValues(); mInflationErrorFilter = filters.get(0); } From 4466434976a21434bc44e4d97c7168b464ac1e01 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Wed, 19 Feb 2020 10:39:04 -0800 Subject: [PATCH 2/2] Move preparation coordinator to after grouping Move preparation coordinator to after grouping since some inflation decisions (e.g. whether we inflate at all) will be dependent on the grouping. Bug: 145748993 Test: atest InflationCoordinatorTest Change-Id: I1169beb3cf136674b89fd713ff40e9ed3b217f75 --- .../coordinator/PreparationCoordinator.java | 115 +++++++++++++++--- .../PreparationCoordinatorTest.java | 72 ++++++++++- 2 files changed, 165 insertions(+), 22 deletions(-) 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 2f4b2e535c610..ebecf181b8080 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 @@ -16,31 +16,39 @@ package com.android.systemui.statusbar.notification.collection.coordinator; +import android.annotation.IntDef; import android.os.RemoteException; import android.service.notification.StatusBarNotification; +import android.util.ArrayMap; import com.android.internal.statusbar.IStatusBarService; +import com.android.systemui.statusbar.notification.collection.GroupEntry; +import com.android.systemui.statusbar.notification.collection.ListEntry; import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl; 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.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager; -import java.util.ArrayList; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.List; +import java.util.Map; +import java.util.Objects; import javax.inject.Inject; import javax.inject.Singleton; /** - * Kicks off notification inflation and view rebinding when a notification is added or updated. + * Kicks off core notification inflation and view rebinding when a notification is added or updated. * Aborts inflation when a notification is removed. * - * If a notification is not done inflating, this coordinator will filter the notification out - * from the {@link ShadeListBuilder}. + * If a notification was uninflated, this coordinator will filter the notification out from the + * {@link ShadeListBuilder} until it is inflated. */ @Singleton public class PreparationCoordinator implements Coordinator { @@ -49,7 +57,7 @@ public class PreparationCoordinator implements Coordinator { private final PreparationCoordinatorLogger mLogger; private final NotifInflater mNotifInflater; private final NotifInflationErrorManager mNotifErrorManager; - private final List mPendingNotifications = new ArrayList<>(); + private final Map mInflationStates = new ArrayMap<>(); private final IStatusBarService mStatusBarService; @Inject @@ -69,27 +77,44 @@ public class PreparationCoordinator implements Coordinator { @Override public void attach(NotifPipeline pipeline) { pipeline.addCollectionListener(mNotifCollectionListener); + // Inflate after grouping/sorting since that affects what views to inflate. + pipeline.addOnBeforeFinalizeFilterListener(mOnBeforeFinalizeFilterListener); pipeline.addFinalizeFilter(mNotifInflationErrorFilter); pipeline.addFinalizeFilter(mNotifInflatingFilter); } private final NotifCollectionListener mNotifCollectionListener = new NotifCollectionListener() { + @Override - public void onEntryAdded(NotificationEntry entry) { - inflateEntry(entry, "entryAdded"); + public void onEntryInit(NotificationEntry entry) { + mInflationStates.put(entry, STATE_UNINFLATED); } @Override public void onEntryUpdated(NotificationEntry entry) { - rebind(entry, "entryUpdated"); + @InflationState int state = getInflationState(entry); + if (state == STATE_INFLATED) { + mInflationStates.put(entry, STATE_INFLATED_INVALID); + } else if (state == STATE_ERROR) { + // Updated so maybe it won't error out now. + mInflationStates.put(entry, STATE_UNINFLATED); + } } @Override public void onEntryRemoved(NotificationEntry entry, int reason) { abortInflation(entry, "entryRemoved reason=" + reason); } + + @Override + public void onEntryCleanUp(NotificationEntry entry) { + mInflationStates.remove(entry); + } }; + private final OnBeforeFinalizeFilterListener mOnBeforeFinalizeFilterListener = + entries -> inflateAllRequiredViews(entries); + private final NotifFilter mNotifInflationErrorFilter = new NotifFilter( TAG + "InflationError") { /** @@ -97,10 +122,7 @@ public class PreparationCoordinator implements Coordinator { */ @Override public boolean shouldFilterOut(NotificationEntry entry, long now) { - if (mNotifErrorManager.hasInflationError(entry)) { - return true; - } - return false; + return getInflationState(entry) == STATE_ERROR; } }; @@ -110,7 +132,8 @@ public class PreparationCoordinator implements Coordinator { */ @Override public boolean shouldFilterOut(NotificationEntry entry, long now) { - return mPendingNotifications.contains(entry); + @InflationState int state = getInflationState(entry); + return (state != STATE_INFLATED) && (state != STATE_INFLATED_INVALID); } }; @@ -119,7 +142,7 @@ public class PreparationCoordinator implements Coordinator { @Override public void onInflationFinished(NotificationEntry entry) { mLogger.logNotifInflated(entry.getKey()); - mPendingNotifications.remove(entry); + mInflationStates.put(entry, STATE_INFLATED); mNotifInflatingFilter.invalidateList(); } }; @@ -128,7 +151,7 @@ public class PreparationCoordinator implements Coordinator { new NotifInflationErrorManager.NotifInflationErrorListener() { @Override public void onNotifInflationError(NotificationEntry entry, Exception e) { - mPendingNotifications.remove(entry); + mInflationStates.put(entry, STATE_ERROR); try { final StatusBarNotification sbn = entry.getSbn(); // report notification inflation errors back up @@ -152,9 +175,41 @@ public class PreparationCoordinator implements Coordinator { } }; + private void inflateAllRequiredViews(List entries) { + for (int i = 0, size = entries.size(); i < size; i++) { + ListEntry entry = entries.get(i); + if (entry instanceof GroupEntry) { + GroupEntry groupEntry = (GroupEntry) entry; + inflateNotifRequiredViews(groupEntry.getSummary()); + List children = groupEntry.getChildren(); + for (int j = 0, groupSize = children.size(); j < groupSize; j++) { + inflateNotifRequiredViews(children.get(j)); + } + } else { + NotificationEntry notifEntry = (NotificationEntry) entry; + inflateNotifRequiredViews(notifEntry); + } + } + } + + private void inflateNotifRequiredViews(NotificationEntry entry) { + @InflationState int state = mInflationStates.get(entry); + switch (state) { + case STATE_UNINFLATED: + inflateEntry(entry, "entryAdded"); + break; + case STATE_INFLATED_INVALID: + rebind(entry, "entryUpdated"); + break; + case STATE_INFLATED: + case STATE_ERROR: + default: + // Nothing to do. + } + } + private void inflateEntry(NotificationEntry entry, String reason) { abortInflation(entry, reason); - mPendingNotifications.add(entry); mNotifInflater.inflateViews(entry); } @@ -165,6 +220,32 @@ public class PreparationCoordinator implements Coordinator { private void abortInflation(NotificationEntry entry, String reason) { mLogger.logInflationAborted(entry.getKey(), reason); entry.abortTask(); - mPendingNotifications.remove(entry); } + + private @InflationState int getInflationState(NotificationEntry entry) { + Integer stateObj = mInflationStates.get(entry); + Objects.requireNonNull(stateObj, + "Asking state of a notification preparation coordinator doesn't know about"); + return stateObj; + } + + @Retention(RetentionPolicy.SOURCE) + @IntDef(prefix = {"STATE_"}, + value = {STATE_UNINFLATED, STATE_INFLATED_INVALID, STATE_INFLATED, STATE_ERROR}) + @interface InflationState {} + + /** The notification has never been inflated before. */ + private static final int STATE_UNINFLATED = 0; + + /** The notification is inflated. */ + private static final int STATE_INFLATED = 1; + + /** + * The notification is inflated, but its content may be out-of-date since the notification has + * been updated. + */ + private static final int STATE_INFLATED_INVALID = 2; + + /** The notification errored out while inflating */ + private static final int STATE_ERROR = -1; } 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 4667e910e85a8..792b4d5c707cc 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 @@ -16,8 +16,8 @@ package com.android.systemui.statusbar.notification.collection.coordinator; -import static junit.framework.Assert.assertTrue; - +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -35,13 +35,16 @@ import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl; 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.listbuilder.OnBeforeFinalizeFilterListener; import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; +import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener; import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -54,14 +57,22 @@ public class PreparationCoordinatorTest extends SysuiTestCase { private static final String TEST_MESSAGE = "TEST_MESSAGE"; private PreparationCoordinator mCoordinator; + private NotifCollectionListener mCollectionListener; + private OnBeforeFinalizeFilterListener mBeforeFilterListener; + private NotifFilter mUninflatedFilter; private NotifFilter mInflationErrorFilter; + private NotifInflaterImpl.InflationCallback mCallback; private NotifInflationErrorManager mErrorManager; private NotificationEntry mEntry; private Exception mInflationError; - @Mock - private NotifPipeline mNotifPipeline; + @Captor private ArgumentCaptor mCollectionListenerCaptor; + @Captor private ArgumentCaptor mBeforeFilterListenerCaptor; + @Captor private ArgumentCaptor mCallbackCaptor; + + @Mock private NotifPipeline mNotifPipeline; @Mock private IStatusBarService mService; + @Mock private NotifInflaterImpl mNotifInflater; @Before public void setUp() { @@ -73,7 +84,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { mCoordinator = new PreparationCoordinator( mock(PreparationCoordinatorLogger.class), - mock(NotifInflaterImpl.class), + mNotifInflater, mErrorManager, mService); @@ -82,6 +93,19 @@ public class PreparationCoordinatorTest extends SysuiTestCase { verify(mNotifPipeline, times(2)).addFinalizeFilter(filterCaptor.capture()); List filters = filterCaptor.getAllValues(); mInflationErrorFilter = filters.get(0); + mUninflatedFilter = filters.get(1); + + verify(mNotifPipeline).addCollectionListener(mCollectionListenerCaptor.capture()); + mCollectionListener = mCollectionListenerCaptor.getValue(); + + verify(mNotifPipeline).addOnBeforeFinalizeFilterListener( + mBeforeFilterListenerCaptor.capture()); + mBeforeFilterListener = mBeforeFilterListenerCaptor.getValue(); + + verify(mNotifInflater).setInflationCallback(mCallbackCaptor.capture()); + mCallback = mCallbackCaptor.getValue(); + + mCollectionListener.onEntryInit(mEntry); } @Test @@ -108,4 +132,42 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // THEN we filter it from the notification list. assertTrue(mInflationErrorFilter.shouldFilterOut(mEntry, 0)); } + + @Test + public void testInflatesNewNotification() { + // WHEN there is a new notification + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we inflate it + verify(mNotifInflater).inflateViews(mEntry); + + // THEN we filter it out until it's done inflating. + assertTrue(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testRebindsInflatedNotificationsOnUpdate() { + // GIVEN an inflated notification + mCallback.onInflationFinished(mEntry); + + // WHEN notification is updated + mCollectionListener.onEntryUpdated(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we rebind it + verify(mNotifInflater).rebindViews(mEntry); + + // THEN we do not filter it because it's not the first inflation. + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testDoesntFilterInflatedNotifs() { + // WHEN a notification is inflated + mCallback.onInflationFinished(mEntry); + + // THEN it isn't filtered from shade list + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } }