From cae38bac77c7282360da7506d25e6ce8d8a747f9 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Wed, 23 Mar 2022 08:50:10 -0400 Subject: [PATCH] Remove unneeded notif vis checks The visibility of ExpandableNotificationRows does not need to be checked inside of NotificationStackSizeCalculator; it is already handled as part of the Notification Pipeline (inside of KeyguardCoordinator). The View layer (NSSL and associated classes, such as this one) can assume that any Notifications that are present should actually be there. Fixes: 226132311 Test: Verify multiple notifications visible on the lockscreen Change-Id: I1117f3c139422a9110a9f5353fc27bbcbb534f75 --- .../stack/NotificationStackSizeCalculator.kt | 28 +------ .../NotificationStackSizeCalculatorTest.kt | 84 +++---------------- 2 files changed, 14 insertions(+), 98 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt index 3f97155242b4a..6c6ed850f45af 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt @@ -22,10 +22,8 @@ import android.view.View.GONE import com.android.systemui.R import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Main -import com.android.systemui.statusbar.NotificationLockscreenUserManager import com.android.systemui.statusbar.StatusBarState.KEYGUARD import com.android.systemui.statusbar.SysuiStatusBarStateController -import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow import com.android.systemui.statusbar.notification.row.ExpandableView import com.android.systemui.util.children @@ -41,8 +39,6 @@ private const val DEBUG = false class NotificationStackSizeCalculator @Inject constructor( - private val groupManager: NotificationGroupManagerLegacy, - private val lockscreenUserManager: NotificationLockscreenUserManager, private val statusBarStateController: SysuiStatusBarStateController, @Main private val resources: Resources ) { @@ -191,7 +187,7 @@ constructor( if (onLockscreen) { when (this) { is ExpandableNotificationRow -> { - if (isSummaryOfSuppressedGroup() || !canShowViewOnLockscreen() || isRemoved) { + if (!canShowViewOnLockscreen() || isRemoved) { return false } } @@ -208,9 +204,6 @@ constructor( visibleIndex: Int ) = stack.calculateGapHeight(previous, /* current= */ this, visibleIndex) - private fun ExpandableNotificationRow.isSummaryOfSuppressedGroup() = - groupManager.isSummaryOfSuppressedGroup(entry.sbn) - /** * Can a view be shown on the lockscreen when calculating the number of allowed notifications to * show? @@ -220,31 +213,12 @@ constructor( private fun ExpandableView.canShowViewOnLockscreen(): Boolean { if (hasNoContentHeight()) { return false - } - if (this is ExpandableNotificationRow && !canShowRowOnLockscreen()) { - return false } else if (visibility == GONE) { return false } return true } - /** - * Can a row be shown on the lockscreen when calculating the number of allowed notifications to - * show? - * - * @return true if it can be shown - */ - private fun ExpandableNotificationRow.canShowRowOnLockscreen(): Boolean { - if (isSummaryOfSuppressedGroup()) { - return false - } - if (!lockscreenUserManager.shouldShowOnKeyguard(entry)) { - return false - } - return !isRemoved - } - private fun log(s: () -> String) { if (DEBUG) { Log.d(TAG, s()) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt index d1848e38ca06d..9a4e10cec1596 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt @@ -22,12 +22,10 @@ import android.view.View.VISIBLE import androidx.test.filters.SmallTest import com.android.systemui.R import com.android.systemui.SysuiTestCase -import com.android.systemui.statusbar.NotificationLockscreenUserManager import com.android.systemui.statusbar.StatusBarState.KEYGUARD import com.android.systemui.statusbar.StatusBarState.SHADE import com.android.systemui.statusbar.SysuiStatusBarStateController 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.ExpandableView import com.android.systemui.util.mockito.any @@ -38,17 +36,13 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.Mockito.mock -import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations +import org.mockito.Mockito.`when` as whenever @SmallTest @RunWith(AndroidTestingRunner::class) class NotificationStackSizeCalculatorTest : SysuiTestCase() { - @Mock private lateinit var groupManager: NotificationGroupManagerLegacy - - @Mock private lateinit var notificationLockscreenUserManager: NotificationLockscreenUserManager - @Mock private lateinit var sysuiStatusBarStateController: SysuiStatusBarStateController @Mock private lateinit var stackLayout: NotificationStackScrollLayout @@ -63,7 +57,6 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { whenever(stackLayout.calculateGapHeight(nullable(), nullable(), any())) .thenReturn(GAP_HEIGHT) - whenever(groupManager.isSummaryOfSuppressedGroup(any())).thenReturn(false) with(testableResources) { addOverride(R.integer.keyguard_max_notification_count, -1) addOverride(R.dimen.notification_divider_height, NOTIFICATION_PADDING.toInt()) @@ -71,15 +64,13 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { sizeCalculator = NotificationStackSizeCalculator( - groupManager = groupManager, - lockscreenUserManager = notificationLockscreenUserManager, statusBarStateController = sysuiStatusBarStateController, testableResources.resources) } @Test fun computeMaxKeyguardNotifications_zeroSpace_returnZero() { - val rows = listOf(createMockRow(height = ROW_HEIGHT, visibleOnLockscreen = true)) + val rows = listOf(createMockRow(height = ROW_HEIGHT)) val maxNotifications = computeMaxKeyguardNotifications(rows, availableSpace = 0f, shelfHeight = 0f) @@ -106,8 +97,8 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { val spaceForOne = totalSpaceForEachRow val rows = listOf( - createMockRow(rowHeight, visibleOnLockscreen = true), - createMockRow(rowHeight, visibleOnLockscreen = true)) + createMockRow(rowHeight), + createMockRow(rowHeight)) val maxNotifications = computeMaxKeyguardNotifications( @@ -124,8 +115,8 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { val spaceForOne = totalSpaceForEachRow val rows = listOf( - createMockRow(rowHeight, visibleOnLockscreen = true), - createMockRow(rowHeight, visibleOnLockscreen = true)) + createMockRow(rowHeight), + createMockRow(rowHeight)) val maxNotifications = computeMaxKeyguardNotifications( @@ -134,15 +125,6 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { assertThat(maxNotifications).isEqualTo(1) } - @Test - fun computeMaxKeyguardNotifications_invisibleOnLockscreen_returnsZero() { - val rows = listOf(createMockRow(visibleOnLockscreen = false)) - - val maxNotifications = computeMaxKeyguardNotifications(rows, Float.MAX_VALUE) - - assertThat(maxNotifications).isEqualTo(0) - } - @Test fun computeMaxKeyguardNotifications_spaceForTwo_returnsTwo() { val rowHeight = ROW_HEIGHT @@ -150,9 +132,9 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { val spaceForTwo = totalSpaceForEachRow * 2 + NOTIFICATION_PADDING val rows = listOf( - createMockRow(rowHeight, visibleOnLockscreen = true), - createMockRow(rowHeight, visibleOnLockscreen = true), - createMockRow(rowHeight, visibleOnLockscreen = true)) + createMockRow(rowHeight), + createMockRow(rowHeight), + createMockRow(rowHeight)) val maxNotifications = computeMaxKeyguardNotifications(rows, spaceForTwo, shelfHeight = 0f) @@ -167,9 +149,9 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { val availableSpace = totalSpaceForEachRow * 2 val rows = listOf( - createMockRow(rowHeight, visibleOnLockscreen = true), - createMockRow(rowHeight, visibleOnLockscreen = true), - createMockRow(rowHeight, visibleOnLockscreen = true)) + createMockRow(rowHeight), + createMockRow(rowHeight), + createMockRow(rowHeight)) val maxNotifications = computeMaxKeyguardNotifications(rows, availableSpace, shelfHeight) assertThat(maxNotifications).isEqualTo(2) @@ -178,31 +160,6 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { assertThat(height).isAtMost(availableSpace + SHELF_HEIGHT) } - @Test - fun computeHeight_allInvisibleToLockscreen_NotInLockscreen_returnsHigherThanZero() { - setOnLockscreen(false) - val rowHeight = 10f - setupChildren(listOf(createMockRow(rowHeight, visibleOnLockscreen = false))) - - val height = - sizeCalculator.computeHeight( - stackLayout, maxNotifications = Int.MAX_VALUE, SHELF_HEIGHT) - - assertThat(height).isGreaterThan(rowHeight) - } - - @Test - fun computeHeight_allInvisibleToLockscreen_onLockscreen_returnsZero() { - setOnLockscreen(true) - setupChildren(listOf(createMockRow(visibleOnLockscreen = false))) - - val height = - sizeCalculator.computeHeight( - stackLayout, maxNotifications = Int.MAX_VALUE, SHELF_HEIGHT) - - assertThat(height).isEqualTo(0) - } - private fun computeMaxKeyguardNotifications( rows: List, availableSpace: Float, @@ -222,14 +179,12 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { } private fun createLockscreenRows(number: Int): List = - (1..number).map { createMockRow(visibleOnLockscreen = true) }.toList() + (1..number).map { createMockRow() }.toList() private fun createMockRow( height: Float = ROW_HEIGHT, - visibleOnLockscreen: Boolean = true, isRemoved: Boolean = false, visibility: Int = VISIBLE, - summaryOfSuppressed: Boolean = false ): ExpandableNotificationRow { val row = mock(ExpandableNotificationRow::class.java) val entry = mock(NotificationEntry::class.java) @@ -238,24 +193,11 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { whenever(row.entry).thenReturn(entry) whenever(row.isRemoved).thenReturn(isRemoved) whenever(row.visibility).thenReturn(visibility) - whenever(notificationLockscreenUserManager.shouldShowOnKeyguard(entry)) - .thenReturn(visibleOnLockscreen) - whenever(groupManager.isSummaryOfSuppressedGroup(sbn)).thenReturn(summaryOfSuppressed) whenever(row.getMinHeight(any())).thenReturn(height.toInt()) whenever(row.intrinsicHeight).thenReturn(height.toInt()) return row } - private fun setOnLockscreen(onLockscreen: Boolean) { - whenever(sysuiStatusBarStateController.state) - .thenReturn( - if (onLockscreen) { - KEYGUARD - } else { - SHADE - }) - } - /** Default dimensions for tests that don't overwrite them. */ companion object { const val GAP_HEIGHT = 12f