From d9b9cf9c501842a6a46392c54180986c5abedff6 Mon Sep 17 00:00:00 2001 From: Julia Tuttle Date: Wed, 12 Jan 2022 12:58:39 -0500 Subject: [PATCH] [New Pipeline] Hide section headers when keyguard showing Replicate the behavior of the old pipeline. Bug: 205564736 Test: updated NodeSpecBuilderTest; KeyguardCoordinator test TBD Change-Id: Ia4740ca60a2f8f44293ac710e2a7efe5a5ec2ded --- .../SectionHeaderVisibilityProvider.kt | 34 +++++++++++++++++ .../coordinator/KeyguardCoordinator.java | 12 +++++- .../collection/render/NodeSpecBuilder.kt | 5 ++- .../collection/render/ShadeViewManager.kt | 5 ++- .../coordinator/KeyguardCoordinatorTest.java | 4 +- .../collection/render/NodeSpecBuilderTest.kt | 37 ++++++++++++++++++- 6 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/SectionHeaderVisibilityProvider.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/SectionHeaderVisibilityProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/SectionHeaderVisibilityProvider.kt new file mode 100644 index 0000000000000..03b978e7784ca --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/SectionHeaderVisibilityProvider.kt @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2022 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 + +import com.android.systemui.dagger.SysUISingleton +import javax.inject.Inject + +/** + * A class which keeps track of whether section headers should be shown in the notification shade. + * + * (In an ideal world, this would directly monitor the state of the keyguard and invalidate the + * pipeline to show/hide headers, but the KeyguardController already invalidates the pipeline when + * the keyguard's state changes. Instead of having both classes monitor for state changes and ending + * up with duplicate runs of the pipeline, we let the KeyguardController update the header + * visibility when it invalidates, and we just store that state here.) + */ +@SysUISingleton +class SectionHeaderVisibilityProvider @Inject constructor() { + var sectionHeadersVisible = true +} \ No newline at end of file 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 33005b34ff988..733be9c1ca2ce 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 @@ -36,6 +36,8 @@ import com.android.keyguard.KeyguardUpdateMonitorCallback; import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationLockscreenUserManager; +import com.android.systemui.statusbar.StatusBarState; +import com.android.systemui.statusbar.notification.SectionHeaderVisibilityProvider; import com.android.systemui.statusbar.notification.collection.GroupEntry; import com.android.systemui.statusbar.notification.collection.ListEntry; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -48,7 +50,8 @@ import com.android.systemui.statusbar.policy.KeyguardStateController; import javax.inject.Inject; /** - * Filters low priority and privacy-sensitive notifications from the lockscreen. + * Filters low priority and privacy-sensitive notifications from the lockscreen, and hides section + * headers on the lockscreen. */ @CoordinatorScope public class KeyguardCoordinator implements Coordinator { @@ -62,6 +65,7 @@ public class KeyguardCoordinator implements Coordinator { private final StatusBarStateController mStatusBarStateController; private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private final HighPriorityProvider mHighPriorityProvider; + private final SectionHeaderVisibilityProvider mSectionHeaderVisibilityProvider; private boolean mHideSilentNotificationsOnLockscreen; @@ -74,7 +78,8 @@ public class KeyguardCoordinator implements Coordinator { BroadcastDispatcher broadcastDispatcher, StatusBarStateController statusBarStateController, KeyguardUpdateMonitor keyguardUpdateMonitor, - HighPriorityProvider highPriorityProvider) { + HighPriorityProvider highPriorityProvider, + SectionHeaderVisibilityProvider sectionHeaderVisibilityProvider) { mContext = context; mMainHandler = mainThreadHandler; mKeyguardStateController = keyguardStateController; @@ -83,6 +88,7 @@ public class KeyguardCoordinator implements Coordinator { mStatusBarStateController = statusBarStateController; mKeyguardUpdateMonitor = keyguardUpdateMonitor; mHighPriorityProvider = highPriorityProvider; + mSectionHeaderVisibilityProvider = sectionHeaderVisibilityProvider; } @Override @@ -214,6 +220,8 @@ public class KeyguardCoordinator implements Coordinator { } private void invalidateListFromFilter(String reason) { + mSectionHeaderVisibilityProvider.setSectionHeadersVisible( + mStatusBarStateController.getState() != StatusBarState.KEYGUARD); mNotifFilter.invalidateList(); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt index f13470ec2c945..607500edfd3a5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.collection.render +import com.android.systemui.statusbar.notification.SectionHeaderVisibilityProvider import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager import com.android.systemui.statusbar.notification.collection.GroupEntry import com.android.systemui.statusbar.notification.collection.ListEntry @@ -35,6 +36,7 @@ import com.android.systemui.util.traceSection class NodeSpecBuilder( private val mediaContainerController: MediaContainerController, private val sectionsFeatureManager: NotificationSectionsFeatureManager, + private val sectionHeaderVisibilityProvider: SectionHeaderVisibilityProvider, private val viewBarn: NotifViewBarn ) { fun buildNodeSpec( @@ -51,6 +53,7 @@ class NodeSpecBuilder( var currentSection: NotifSection? = null val prevSections = mutableSetOf() + val showHeaders = sectionHeaderVisibilityProvider.sectionHeadersVisible for (entry in notifList) { val section = entry.section!! @@ -61,7 +64,7 @@ class NodeSpecBuilder( // If this notif begins a new section, first add the section's header view if (section != currentSection) { - if (section.headerController != currentSection?.headerController) { + if (section.headerController != currentSection?.headerController && showHeaders) { section.headerController?.let { headerController -> root.children.add(NodeSpecImpl(root, headerController)) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewManager.kt index ad973927f21e4..484707241b4d4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewManager.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewManager.kt @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.notification.collection.render import android.content.Context import android.view.View import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager +import com.android.systemui.statusbar.notification.SectionHeaderVisibilityProvider import com.android.systemui.statusbar.notification.collection.GroupEntry import com.android.systemui.statusbar.notification.collection.ListEntry import com.android.systemui.statusbar.notification.collection.NotificationEntry @@ -38,13 +39,15 @@ class ShadeViewManager @AssistedInject constructor( @Assisted private val stackController: NotifStackController, mediaContainerController: MediaContainerController, featureManager: NotificationSectionsFeatureManager, + sectionHeaderVisibilityProvider: SectionHeaderVisibilityProvider, logger: ShadeViewDifferLogger, private val viewBarn: NotifViewBarn ) { // We pass a shim view here because the listContainer may not actually have a view associated // with it and the differ never actually cares about the root node's view. private val rootController = RootNodeController(listContainer, View(context)) - private val specBuilder = NodeSpecBuilder(mediaContainerController, featureManager, viewBarn) + private val specBuilder = NodeSpecBuilder(mediaContainerController, featureManager, + sectionHeaderVisibilityProvider, viewBarn) private val viewDiffer = ShadeViewDiffer(rootController, logger) /** Method for attaching this manager to the pipeline. */ 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 917c049fd5780..d0947497f0ece 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 @@ -41,6 +41,7 @@ import com.android.systemui.broadcast.BroadcastDispatcher; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.RankingBuilder; +import com.android.systemui.statusbar.notification.SectionHeaderVisibilityProvider; import com.android.systemui.statusbar.notification.collection.GroupEntry; import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -70,6 +71,7 @@ public class KeyguardCoordinatorTest extends SysuiTestCase { @Mock private StatusBarStateController mStatusBarStateController; @Mock private KeyguardUpdateMonitor mKeyguardUpdateMonitor; @Mock private HighPriorityProvider mHighPriorityProvider; + @Mock private SectionHeaderVisibilityProvider mSectionHeaderVisibilityProvider; @Mock private NotifPipeline mNotifPipeline; private NotificationEntry mEntry; @@ -81,7 +83,7 @@ public class KeyguardCoordinatorTest extends SysuiTestCase { KeyguardCoordinator keyguardCoordinator = new KeyguardCoordinator( mContext, mMainHandler, mKeyguardStateController, mLockscreenUserManager, mBroadcastDispatcher, mStatusBarStateController, - mKeyguardUpdateMonitor, mHighPriorityProvider); + mKeyguardUpdateMonitor, mHighPriorityProvider, mSectionHeaderVisibilityProvider); mEntry = new NotificationEntryBuilder() .setUser(new UserHandle(NOTIF_USER_ID)) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilderTest.kt index f77381000ae2a..4e309d49c6d2c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilderTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilderTest.kt @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.notification.collection.render import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager +import com.android.systemui.statusbar.notification.SectionHeaderVisibilityProvider import com.android.systemui.statusbar.notification.collection.GroupEntry import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder import com.android.systemui.statusbar.notification.collection.ListEntry @@ -43,6 +44,7 @@ class NodeSpecBuilderTest : SysuiTestCase() { private val mediaContainerController: MediaContainerController = mock() private val sectionsFeatureManager: NotificationSectionsFeatureManager = mock() + private val sectionHeaderVisibilityProvider: SectionHeaderVisibilityProvider = mock() private val viewBarn: NotifViewBarn = mock() private var rootController: NodeController = buildFakeController("rootController") @@ -72,11 +74,13 @@ class NodeSpecBuilderTest : SysuiTestCase() { fakeViewBarn.getViewByEntry(it.getArgument(0)) } - specBuilder = NodeSpecBuilder(mediaContainerController, sectionsFeatureManager, viewBarn) + specBuilder = NodeSpecBuilder(mediaContainerController, sectionsFeatureManager, + sectionHeaderVisibilityProvider, viewBarn) } @Test fun testMultipleSectionsWithSameController() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( listOf( notif(0, section0), @@ -95,6 +99,7 @@ class NodeSpecBuilderTest : SysuiTestCase() { @Test(expected = RuntimeException::class) fun testMultipleSectionsWithSameControllerNonConsecutive() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( listOf( notif(0, section0), @@ -108,6 +113,7 @@ class NodeSpecBuilderTest : SysuiTestCase() { @Test fun testSimpleMapping() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( // GIVEN a simple flat list of notifications all in the same headerless section listOf( @@ -129,6 +135,7 @@ class NodeSpecBuilderTest : SysuiTestCase() { @Test fun testSimpleMappingWithMedia() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) // WHEN media controls are enabled whenever(sectionsFeatureManager.isMediaControlsEnabled()).thenReturn(true) @@ -154,6 +161,8 @@ class NodeSpecBuilderTest : SysuiTestCase() { @Test fun testHeaderInjection() { + // WHEN section headers are supposed to be visible + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( // GIVEN a flat list of notifications, spread across three sections listOf( @@ -176,8 +185,32 @@ class NodeSpecBuilderTest : SysuiTestCase() { ) } + @Test + fun testHeaderSuppression() { + // WHEN section headers are supposed to be hidden + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(false) + checkOutput( + // GIVEN a flat list of notifications, spread across three sections + listOf( + notif(0, section0), + notif(1, section0), + notif(2, section1), + notif(3, section2) + ), + + // THEN each section has its header injected + tree( + notifNode(0), + notifNode(1), + notifNode(2), + notifNode(3) + ) + ) + } + @Test fun testGroups() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( // GIVEN a mixed list of top-level notifications and groups listOf( @@ -218,6 +251,7 @@ class NodeSpecBuilderTest : SysuiTestCase() { @Test fun testSecondSectionWithNoHeader() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( // GIVEN a middle section with no associated header view listOf( @@ -247,6 +281,7 @@ class NodeSpecBuilderTest : SysuiTestCase() { @Test(expected = RuntimeException::class) fun testRepeatedSectionsThrow() { + whenever(sectionHeaderVisibilityProvider.sectionHeadersVisible).thenReturn(true) checkOutput( // GIVEN a malformed list where sections are not contiguous listOf(