diff options
7 files changed, 180 insertions, 36 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/CommonVisualInterruptionSuppressors.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/CommonVisualInterruptionSuppressors.kt index 538be142b8f2..9ff416a02ee6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/CommonVisualInterruptionSuppressors.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/CommonVisualInterruptionSuppressors.kt @@ -43,9 +43,9 @@ import com.android.systemui.util.time.SystemClock class PeekDisabledSuppressor( private val globalSettings: GlobalSettings, private val headsUpManager: HeadsUpManager, - private val logger: NotificationInterruptLogger, + private val logger: VisualInterruptionDecisionLogger, @Main private val mainHandler: Handler, -) : VisualInterruptionCondition(types = setOf(PEEK), reason = "peek setting disabled") { +) : VisualInterruptionCondition(types = setOf(PEEK), reason = "peek disabled by global setting") { private var isEnabled = false override fun shouldSuppress(): Boolean = !isEnabled @@ -87,16 +87,13 @@ class PeekDisabledSuppressor( class PulseDisabledSuppressor( private val ambientDisplayConfiguration: AmbientDisplayConfiguration, private val userTracker: UserTracker, -) : VisualInterruptionCondition(types = setOf(PULSE), reason = "pulse setting disabled") { +) : VisualInterruptionCondition(types = setOf(PULSE), reason = "pulse disabled by user setting") { override fun shouldSuppress(): Boolean = !ambientDisplayConfiguration.pulseOnNotificationEnabled(userTracker.userId) } class PulseBatterySaverSuppressor(private val batteryController: BatteryController) : - VisualInterruptionCondition( - types = setOf(PULSE), - reason = "pulsing disabled by battery saver" - ) { + VisualInterruptionCondition(types = setOf(PULSE), reason = "pulse disabled by battery saver") { override fun shouldSuppress() = batteryController.isAodPowerSave() } @@ -128,14 +125,14 @@ class PeekDndSuppressor() : } class PeekNotImportantSuppressor() : - VisualInterruptionFilter(types = setOf(PEEK), reason = "not important") { + VisualInterruptionFilter(types = setOf(PEEK), reason = "importance < HIGH") { override fun shouldSuppress(entry: NotificationEntry) = entry.importance < IMPORTANCE_HIGH } class PeekDeviceNotInUseSuppressor( private val powerManager: PowerManager, private val statusBarStateController: StatusBarStateController -) : VisualInterruptionCondition(types = setOf(PEEK), reason = "not in use") { +) : VisualInterruptionCondition(types = setOf(PEEK), reason = "device not in use") { override fun shouldSuppress() = when { !powerManager.isScreenOn || statusBarStateController.isDreaming -> true @@ -144,7 +141,7 @@ class PeekDeviceNotInUseSuppressor( } class PeekOldWhenSuppressor(private val systemClock: SystemClock) : - VisualInterruptionFilter(types = setOf(PEEK), reason = "old when") { + VisualInterruptionFilter(types = setOf(PEEK), reason = "has old `when`") { private fun whenAge(entry: NotificationEntry) = systemClock.currentTimeMillis() - entry.sbn.notification.`when` @@ -165,21 +162,21 @@ class PeekOldWhenSuppressor(private val systemClock: SystemClock) : } class PulseEffectSuppressor() : - VisualInterruptionFilter(types = setOf(PULSE), reason = "ambient effect suppressed") { + VisualInterruptionFilter(types = setOf(PULSE), reason = "suppressed by DND") { override fun shouldSuppress(entry: NotificationEntry) = entry.shouldSuppressAmbient() } class PulseLockscreenVisibilityPrivateSuppressor() : VisualInterruptionFilter( types = setOf(PULSE), - reason = "notification hidden on lock screen by override" + reason = "hidden by lockscreen visibility override" ) { override fun shouldSuppress(entry: NotificationEntry) = entry.ranking.lockscreenVisibilityOverride == VISIBILITY_PRIVATE } class PulseLowImportanceSuppressor() : - VisualInterruptionFilter(types = setOf(PULSE), reason = "importance less than DEFAULT") { + VisualInterruptionFilter(types = setOf(PULSE), reason = "importance < DEFAULT") { override fun shouldSuppress(entry: NotificationEntry) = entry.importance < IMPORTANCE_DEFAULT } @@ -198,12 +195,12 @@ class HunJustLaunchedFsiSuppressor() : } class BubbleNotAllowedSuppressor() : - VisualInterruptionFilter(types = setOf(BUBBLE), reason = "not allowed") { + VisualInterruptionFilter(types = setOf(BUBBLE), reason = "cannot bubble") { override fun shouldSuppress(entry: NotificationEntry) = !entry.canBubble() } class BubbleNoMetadataSuppressor() : - VisualInterruptionFilter(types = setOf(BUBBLE), reason = "no bubble metadata") { + VisualInterruptionFilter(types = setOf(BUBBLE), reason = "has no or invalid bubble metadata") { private fun isValidMetadata(metadata: BubbleMetadata?) = metadata != null && (metadata.intent != null || metadata.shortcutId != null) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/FullScreenIntentDecisionProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/FullScreenIntentDecisionProvider.kt index 6af25439ecc3..b44a36724bdb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/FullScreenIntentDecisionProvider.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/FullScreenIntentDecisionProvider.kt @@ -50,19 +50,32 @@ class FullScreenIntentDecisionProvider( val shouldFsi: Boolean val wouldFsiWithoutDnd: Boolean val logReason: String + val shouldLog: Boolean + val isWarning: Boolean } private enum class DecisionImpl( override val shouldFsi: Boolean, override val logReason: String, override val wouldFsiWithoutDnd: Boolean = shouldFsi, - val supersedesDnd: Boolean = false + val supersedesDnd: Boolean = false, + override val shouldLog: Boolean = true, + override val isWarning: Boolean = false ) : Decision { - NO_FSI_NO_FULL_SCREEN_INTENT(false, "no full-screen intent", supersedesDnd = true), + NO_FSI_NO_FULL_SCREEN_INTENT( + false, + "no full-screen intent", + supersedesDnd = true, + shouldLog = false + ), NO_FSI_SHOW_STICKY_HUN(false, "full-screen intents are disabled", supersedesDnd = true), NO_FSI_NOT_IMPORTANT_ENOUGH(false, "not important enough"), - NO_FSI_SUPPRESSIVE_GROUP_ALERT_BEHAVIOR(false, "suppressive group alert behavior"), - NO_FSI_SUPPRESSIVE_BUBBLE_METADATA(false, "suppressive bubble metadata"), + NO_FSI_SUPPRESSIVE_GROUP_ALERT_BEHAVIOR( + false, + "suppressive group alert behavior", + isWarning = true + ), + NO_FSI_SUPPRESSIVE_BUBBLE_METADATA(false, "suppressive bubble metadata", isWarning = true), NO_FSI_PACKAGE_SUSPENDED(false, "package suspended"), FSI_DEVICE_NOT_INTERACTIVE(true, "device is not interactive"), FSI_DEVICE_DREAMING(true, "device is dreaming"), @@ -71,7 +84,7 @@ class FullScreenIntentDecisionProvider( FSI_KEYGUARD_OCCLUDED(true, "keyguard is occluded"), FSI_LOCKED_SHADE(true, "locked shade"), FSI_DEVICE_NOT_PROVISIONED(true, "device not provisioned"), - NO_FSI_NO_HUN_OR_KEYGUARD(false, "no HUN or keyguard"), + NO_FSI_NO_HUN_OR_KEYGUARD(false, "no HUN or keyguard", isWarning = true), NO_FSI_SUPPRESSED_BY_DND(false, "suppressed by DND", wouldFsiWithoutDnd = false), NO_FSI_SUPPRESSED_ONLY_BY_DND(false, "suppressed only by DND", wouldFsiWithoutDnd = true) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionLogger.kt new file mode 100644 index 000000000000..1470b0331359 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionLogger.kt @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2023 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.interruption + +import com.android.systemui.log.LogBuffer +import com.android.systemui.log.core.LogLevel.DEBUG +import com.android.systemui.log.core.LogLevel.INFO +import com.android.systemui.log.core.LogLevel.WARNING +import com.android.systemui.log.dagger.NotificationInterruptLog +import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.interruption.VisualInterruptionDecisionProvider.FullScreenIntentDecision +import com.android.systemui.statusbar.notification.logKey +import javax.inject.Inject + +class VisualInterruptionDecisionLogger +@Inject +constructor(@NotificationInterruptLog val buffer: LogBuffer) { + fun logHeadsUpFeatureChanged(isEnabled: Boolean) { + buffer.log( + TAG, + INFO, + { bool1 = isEnabled }, + { "HUN feature is now ${if (bool1) "enabled" else "disabled"}" } + ) + } + + fun logWillDismissAll() { + buffer.log(TAG, INFO, {}, { "dismissing all HUNs since feature was disabled" }) + } + + fun logDecision( + type: String, + entry: NotificationEntry, + decision: VisualInterruptionDecisionProvider.Decision + ) { + buffer.log( + TAG, + DEBUG, + { + str1 = type + bool1 = decision.shouldInterrupt + str2 = decision.logReason + str3 = entry.logKey + }, + { + val outcome = if (bool1) "allowed" else "suppressed" + "$str1 $outcome: $str2 (key=$str3)" + } + ) + } + + fun logFullScreenIntentDecision( + entry: NotificationEntry, + decision: FullScreenIntentDecision, + warning: Boolean + ) { + buffer.log( + TAG, + if (warning) WARNING else DEBUG, + { + bool1 = decision.shouldInterrupt + bool2 = decision.wouldInterruptWithoutDnd + str1 = decision.logReason + str2 = entry.logKey + }, + { + val outcome = + when { + bool1 -> "allowed" + bool2 -> "suppressed only by DND" + else -> "suppressed" + } + "FSI $outcome: $str1 (key=$str2)" + } + ) + } +} + +private const val TAG = "VisualInterruptionDecisionProvider" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImpl.kt index ed19be38665f..c0a1a32b1401 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImpl.kt @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.notification.interruption import android.hardware.display.AmbientDisplayConfiguration import android.os.Handler import android.os.PowerManager +import android.util.Log import com.android.internal.annotations.VisibleForTesting import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.plugins.statusbar.StatusBarStateController @@ -46,7 +47,7 @@ constructor( private val headsUpManager: HeadsUpManager, private val keyguardNotificationVisibilityProvider: KeyguardNotificationVisibilityProvider, keyguardStateController: KeyguardStateController, - private val logger: NotificationInterruptLogger, + private val logger: VisualInterruptionDecisionLogger, @Main private val mainHandler: Handler, private val powerManager: PowerManager, private val statusBarStateController: StatusBarStateController, @@ -79,8 +80,11 @@ constructor( } private class FullScreenIntentDecisionImpl( + val entry: NotificationEntry, private val fsiDecision: FullScreenIntentDecisionProvider.Decision ) : FullScreenIntentDecision { + var hasBeenLogged = false + override val shouldInterrupt get() = fsiDecision.shouldFsi @@ -89,6 +93,12 @@ constructor( override val logReason get() = fsiDecision.logReason + + val shouldLog + get() = fsiDecision.shouldLog + + val isWarning + get() = fsiDecision.isWarning } private val fullScreenIntentDecisionProvider = @@ -206,7 +216,7 @@ constructor( entry: NotificationEntry, loggable: LoggableDecision ) { - // Not yet implemented. + logger.logDecision(type.name, entry, loggable.decision) } override fun makeUnloggedFullScreenIntentDecision( @@ -217,13 +227,29 @@ constructor( val couldHeadsUp = makeUnloggedHeadsUpDecision(entry).shouldInterrupt val fsiDecision = fullScreenIntentDecisionProvider.makeFullScreenIntentDecision(entry, couldHeadsUp) - return FullScreenIntentDecisionImpl(fsiDecision) + return FullScreenIntentDecisionImpl(entry, fsiDecision) } override fun logFullScreenIntentDecision(decision: FullScreenIntentDecision) { check(started) - // Not yet implemented. + if (decision !is FullScreenIntentDecisionImpl) { + Log.wtf(TAG, "FSI decision $decision was not created by this class") + return + } + + if (decision.hasBeenLogged) { + Log.wtf(TAG, "FSI decision $decision has already been logged") + return + } + + decision.hasBeenLogged = true + + if (!decision.shouldLog) { + return + } + + logger.logFullScreenIntentDecision(decision.entry, decision, decision.isWarning) } private fun checkSuppressInterruptions(entry: NotificationEntry) = diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderWrapperTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderWrapperTest.kt index 1d2055ec2e30..e1581eade4ca 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderWrapperTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderWrapperTest.kt @@ -44,7 +44,7 @@ class NotificationInterruptStateProviderWrapperTest : VisualInterruptionDecision statusBarStateController, keyguardStateController, headsUpManager, - logger, + oldLogger, mainHandler, flags, keyguardNotificationVisibilityProvider, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImplTest.kt index 722b1704bb18..1064475c744c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImplTest.kt @@ -37,7 +37,7 @@ class VisualInterruptionDecisionProviderImplTest : VisualInterruptionDecisionPro headsUpManager, keyguardNotificationVisibilityProvider, keyguardStateController, - logger, + newLogger, mainHandler, powerManager, statusBarStateController, @@ -222,14 +222,14 @@ class VisualInterruptionDecisionProviderImplTest : VisualInterruptionDecisionPro private class TestCondition( types: Set<VisualInterruptionType>, val onShouldSuppress: () -> Boolean - ) : VisualInterruptionCondition(types = types, reason = "") { + ) : VisualInterruptionCondition(types = types, reason = "test condition") { override fun shouldSuppress(): Boolean = onShouldSuppress() } private class TestFilter( types: Set<VisualInterruptionType>, val onShouldSuppress: (NotificationEntry) -> Boolean = { true } - ) : VisualInterruptionFilter(types = types, reason = "") { + ) : VisualInterruptionFilter(types = types, reason = "test filter") { override fun shouldSuppress(entry: NotificationEntry) = onShouldSuppress(entry) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderTestBase.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderTestBase.kt index 433c406fa309..5e811561682e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderTestBase.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderTestBase.kt @@ -49,6 +49,9 @@ import android.provider.Settings.Global.HEADS_UP_OFF import android.provider.Settings.Global.HEADS_UP_ON import com.android.internal.logging.testing.UiEventLoggerFake import com.android.systemui.SysuiTestCase +import com.android.systemui.log.LogBuffer +import com.android.systemui.log.LogcatEchoTracker +import com.android.systemui.log.core.LogLevel import com.android.systemui.res.R import com.android.systemui.settings.FakeUserTracker import com.android.systemui.statusbar.FakeStatusBarStateController @@ -78,6 +81,20 @@ import org.junit.Test import org.mockito.Mockito.`when` as whenever abstract class VisualInterruptionDecisionProviderTestBase : SysuiTestCase() { + private val fakeLogBuffer = + LogBuffer( + name = "FakeLog", + maxSize = 1, + logcatEchoTracker = + object : LogcatEchoTracker { + override fun isBufferLoggable(bufferName: String, level: LogLevel): Boolean = + true + + override fun isTagLoggable(tagName: String, level: LogLevel): Boolean = true + }, + systrace = false + ) + private val leakCheck = LeakCheckedTest.SysuiLeakCheck() protected val ambientDisplayConfiguration = FakeAmbientDisplayConfiguration(context) @@ -90,8 +107,9 @@ abstract class VisualInterruptionDecisionProviderTestBase : SysuiTestCase() { protected val keyguardNotificationVisibilityProvider: KeyguardNotificationVisibilityProvider = mock() protected val keyguardStateController = FakeKeyguardStateController(leakCheck) - protected val logger: NotificationInterruptLogger = mock() protected val mainHandler = FakeHandler(Looper.getMainLooper()) + protected val newLogger = VisualInterruptionDecisionLogger(fakeLogBuffer) + protected val oldLogger = NotificationInterruptLogger(fakeLogBuffer) protected val powerManager: PowerManager = mock() protected val statusBarStateController = FakeStatusBarStateController() protected val systemClock = FakeSystemClock() @@ -119,14 +137,9 @@ abstract class VisualInterruptionDecisionProviderTestBase : SysuiTestCase() { @Before fun setUp() { - globalSettings.putInt(HEADS_UP_NOTIFICATIONS_ENABLED, HEADS_UP_ON) - val user = UserInfo(ActivityManager.getCurrentUser(), "Current user", /* flags = */ 0) userTracker.set(listOf(user), /* currentUserIndex = */ 0) - whenever(keyguardNotificationVisibilityProvider.shouldHideNotification(any())) - .thenReturn(false) - provider.start() } @@ -866,12 +879,12 @@ abstract class VisualInterruptionDecisionProviderTestBase : SysuiTestCase() { } protected fun assertShouldHeadsUp(entry: NotificationEntry) = - provider.makeUnloggedHeadsUpDecision(entry).let { + provider.makeAndLogHeadsUpDecision(entry).let { assertTrue("unexpected suppressed HUN: ${it.logReason}", it.shouldInterrupt) } protected fun assertShouldNotHeadsUp(entry: NotificationEntry) = - provider.makeUnloggedHeadsUpDecision(entry).let { + provider.makeAndLogHeadsUpDecision(entry).let { assertFalse("unexpected unsuppressed HUN: ${it.logReason}", it.shouldInterrupt) } @@ -887,6 +900,7 @@ abstract class VisualInterruptionDecisionProviderTestBase : SysuiTestCase() { protected fun assertShouldFsi(entry: NotificationEntry) = provider.makeUnloggedFullScreenIntentDecision(entry).let { + provider.logFullScreenIntentDecision(it) assertTrue("unexpected suppressed FSI: ${it.logReason}", it.shouldInterrupt) } @@ -895,6 +909,7 @@ abstract class VisualInterruptionDecisionProviderTestBase : SysuiTestCase() { expectWouldInterruptWithoutDnd: Boolean? = null ) = provider.makeUnloggedFullScreenIntentDecision(entry).let { + provider.logFullScreenIntentDecision(it) assertFalse("unexpected unsuppressed FSI: ${it.logReason}", it.shouldInterrupt) if (expectWouldInterruptWithoutDnd != null) { assertEquals( |