From 3747c15a65148a9579bf3773457afb48e5e1f575 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 7 Feb 2022 16:31:11 +0000 Subject: [PATCH] New Pipeline: Fix some feature flagging * Calling addKeyguardNotificationSuppressor was supposed to be only when the old pipeline was enabled, but it was still being used on the new pipeline, so I've guarded the calls. This makes it easier to remove the old pipeline cleanly. Test: manual Bug: 218105118 Change-Id: I681dfeaf6ca352f3feb287064581b746e130c701 --- .../notification/collection/NotifPipeline.kt | 4 ++++ .../coordinator/BubbleCoordinator.java | 6 +----- .../coordinator/CommunalCoordinator.java | 6 ++++-- .../SmartspaceDedupingCoordinator.kt | 8 +++++--- .../coordinator/ViewConfigCoordinator.kt | 17 ++++++----------- .../coordinator/CommunalCoordinatorTest.java | 9 ++++++--- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt index 2009b522482ed..e3ee8139d71c4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipeline.kt @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.collection import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.listbuilder.OnAfterRenderEntryListener import com.android.systemui.statusbar.notification.collection.listbuilder.OnAfterRenderGroupListener import com.android.systemui.statusbar.notification.collection.listbuilder.OnAfterRenderListListener @@ -75,6 +76,7 @@ import javax.inject.Inject */ @SysUISingleton class NotifPipeline @Inject constructor( + notifPipelineFlags: NotifPipelineFlags, private val mNotifCollection: NotifCollection, private val mShadeListBuilder: ShadeListBuilder, private val mRenderStageManager: RenderStageManager @@ -105,6 +107,8 @@ class NotifPipeline @Inject constructor( return mNotifCollection.getEntry(key) } + val isNewPipelineEnabled: Boolean = notifPipelineFlags.isNewPipelineEnabled() + /** * Registers a lifetime extender. Lifetime extenders can cause notifications that have been * dismissed or retracted by system server to be temporarily retained in the collection. 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 85c0064e79832..b923fdfe6be80 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 @@ -16,7 +16,6 @@ package com.android.systemui.statusbar.notification.collection.coordinator; -import com.android.systemui.statusbar.notification.NotifPipelineFlags; import com.android.systemui.statusbar.notification.collection.NotifCollection; import com.android.systemui.statusbar.notification.collection.NotifPipeline; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -58,7 +57,6 @@ import javax.inject.Inject; public class BubbleCoordinator implements Coordinator { private static final String TAG = "BubbleCoordinator"; - private final NotifPipelineFlags mNotifPipelineFlags; private final Optional mBubblesManagerOptional; private final Optional mBubblesOptional; private final NotifCollection mNotifCollection; @@ -68,11 +66,9 @@ public class BubbleCoordinator implements Coordinator { @Inject public BubbleCoordinator( - NotifPipelineFlags notifPipelineFlags, Optional bubblesManagerOptional, Optional bubblesOptional, NotifCollection notifCollection) { - mNotifPipelineFlags = notifPipelineFlags; mBubblesManagerOptional = bubblesManagerOptional; mBubblesOptional = bubblesOptional; mNotifCollection = notifCollection; @@ -131,7 +127,7 @@ public class BubbleCoordinator implements Coordinator { DismissedByUserStats dismissedByUserStats, int reason ) { - if (!mNotifPipelineFlags.isNewPipelineEnabled()) { + if (!mNotifPipeline.isNewPipelineEnabled()) { // The `entry` will be from whichever pipeline is active, so if the old pipeline is // running, make sure that we use the new pipeline's entry (if it still exists). NotificationEntry newPipelineEntry = mNotifPipeline.getEntry(entry.getKey()); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java index bd011c3847cee..5396b86382257 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinator.java @@ -74,7 +74,9 @@ public class CommunalCoordinator implements Coordinator { public void attach(@NonNull NotifPipeline pipeline) { pipeline.addPreGroupFilter(mFilter); mCommunalStateController.addCallback(mStateCallback); - mNotificationLockscreenUserManager.addKeyguardNotificationSuppressor( - entry -> mCommunalStateController.getCommunalViewShowing()); + if (!pipeline.isNewPipelineEnabled()) { + mNotificationLockscreenUserManager.addKeyguardNotificationSuppressor( + entry -> mCommunalStateController.getCommunalViewShowing()); + } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt index 519d75ff07d06..48f00ac4bf551 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SmartspaceDedupingCoordinator.kt @@ -65,9 +65,11 @@ class SmartspaceDedupingCoordinator @Inject constructor( statusBarStateController.addCallback(statusBarStateListener) smartspaceController.addListener(this::onNewSmartspaceTargets) - // TODO (b/173126564): Remove this once the old pipeline is no longer necessary - notificationLockscreenUserManager.addKeyguardNotificationSuppressor { entry -> - isDupedWithSmartspaceContent(entry) + if (!pipeline.isNewPipelineEnabled) { + // TODO (b/173126564): Remove this once the old pipeline is no longer necessary + notificationLockscreenUserManager.addKeyguardNotificationSuppressor { entry -> + isDupedWithSmartspaceContent(entry) + } } recordStatusBarState(statusBarStateController.state) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt index 832df4d56be6d..08b2483520168 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt @@ -21,7 +21,6 @@ import com.android.internal.widget.MessagingMessage import com.android.keyguard.KeyguardUpdateMonitor import com.android.systemui.statusbar.NotificationLockscreenUserManager.UserChangedListener import com.android.systemui.statusbar.NotificationLockscreenUserManagerImpl -import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope import com.android.systemui.statusbar.notification.row.NotificationGutsManager @@ -35,9 +34,8 @@ import javax.inject.Inject */ @CoordinatorScope class ViewConfigCoordinator @Inject internal constructor( - configurationController: ConfigurationController, - lockscreenUserManager: NotificationLockscreenUserManagerImpl, - notifPipelineFlags: NotifPipelineFlags, + private val mConfigurationController: ConfigurationController, + private val mLockscreenUserManager: NotificationLockscreenUserManagerImpl, private val mGutsManager: NotificationGutsManager, private val mKeyguardUpdateMonitor: KeyguardUpdateMonitor ) : Coordinator, UserChangedListener, ConfigurationController.ConfigurationListener { @@ -46,15 +44,12 @@ class ViewConfigCoordinator @Inject internal constructor( private var mDispatchUiModeChangeOnUserSwitched = false private var mPipeline: NotifPipeline? = null - init { - if (notifPipelineFlags.isNewPipelineEnabled()) { - lockscreenUserManager.addUserChangedListener(this) - configurationController.addCallback(this) - } - } - override fun attach(pipeline: NotifPipeline) { mPipeline = pipeline + if (pipeline.isNewPipelineEnabled) { + mLockscreenUserManager.addUserChangedListener(this) + mConfigurationController.addCallback(this) + } } override fun onDensityOrFontScaleChanged() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java index f4452bc248b15..1f52b9cc41952 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/CommunalCoordinatorTest.java @@ -16,9 +16,12 @@ package com.android.systemui.statusbar.notification.collection.coordinator; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.test.suitebuilder.annotation.SmallTest; @@ -82,12 +85,13 @@ public class CommunalCoordinatorTest extends SysuiTestCase { final NotifFilter filter = filterCaptor.getValue(); // Verify that notifications are not filtered out by default. - assert (!filter.shouldFilterOut(mNotificationEntry, 0)); + assertThat(filter.shouldFilterOut(mNotificationEntry, 0)).isFalse(); filter.setInvalidationListener(mFilterListener); // Verify that notifications are filtered out when communal is showing and that the filter // pipeline is notified. + when(mCommunalStateController.getCommunalViewShowing()).thenReturn(true); stateCallback.onCommunalViewShowingChanged(); // Make sure callback depends on executor to run. verify(mFilterListener, never()).onPluggableInvalidated(any()); @@ -97,7 +101,6 @@ public class CommunalCoordinatorTest extends SysuiTestCase { verify(mFilterListener).onPluggableInvalidated(any()); verify(mNotificationEntryManager).updateNotifications(any()); - assert (filter.shouldFilterOut(mNotificationEntry, 0)); - + assertThat(filter.shouldFilterOut(mNotificationEntry, 0)).isTrue(); } }