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
This commit is contained in:
Jeff DeCew
2022-02-07 16:31:11 +00:00
parent a75676fdfb
commit 3747c15a65
6 changed files with 26 additions and 24 deletions

View File

@@ -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.

View File

@@ -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<BubblesManager> mBubblesManagerOptional;
private final Optional<Bubbles> mBubblesOptional;
private final NotifCollection mNotifCollection;
@@ -68,11 +66,9 @@ public class BubbleCoordinator implements Coordinator {
@Inject
public BubbleCoordinator(
NotifPipelineFlags notifPipelineFlags,
Optional<BubblesManager> bubblesManagerOptional,
Optional<Bubbles> 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());

View File

@@ -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());
}
}
}

View File

@@ -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)

View File

@@ -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() {

View File

@@ -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();
}
}