Add logging to LogBuffer
Bug: 261728888
Test: atest NotificationInterruptStateProviderWrapperTest
Test: atest VisualInterruptionDecisionProviderImplTest
Flag: ACONFIG com.android.systemui.visual_interruptions_refactor DEVELOPMENT
Change-Id: Iff50fbf8d84f0166f7d7faa21a3162fafb8b979c
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 538be14..9ff416a 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 @@
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 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 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 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 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 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 6af2543..b44a367 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 @@
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 @@
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 0000000..1470b03
--- /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 ed19be3..c0a1a32 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 @@
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 @@
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 @@
}
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 @@
override val logReason
get() = fsiDecision.logReason
+
+ val shouldLog
+ get() = fsiDecision.shouldLog
+
+ val isWarning
+ get() = fsiDecision.isWarning
}
private val fullScreenIntentDecisionProvider =
@@ -206,7 +216,7 @@
entry: NotificationEntry,
loggable: LoggableDecision
) {
- // Not yet implemented.
+ logger.logDecision(type.name, entry, loggable.decision)
}
override fun makeUnloggedFullScreenIntentDecision(
@@ -217,13 +227,29 @@
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 1d2055e..e1581ea 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 @@
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 722b170..1064475 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 @@
headsUpManager,
keyguardNotificationVisibilityProvider,
keyguardStateController,
- logger,
+ newLogger,
mainHandler,
powerManager,
statusBarStateController,
@@ -222,14 +222,14 @@
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 433c406..5e81156 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_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.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 @@
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 @@
@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 @@
}
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 @@
protected fun assertShouldFsi(entry: NotificationEntry) =
provider.makeUnloggedFullScreenIntentDecision(entry).let {
+ provider.logFullScreenIntentDecision(it)
assertTrue("unexpected suppressed FSI: ${it.logReason}", it.shouldInterrupt)
}
@@ -895,6 +909,7 @@
expectWouldInterruptWithoutDnd: Boolean? = null
) =
provider.makeUnloggedFullScreenIntentDecision(entry).let {
+ provider.logFullScreenIntentDecision(it)
assertFalse("unexpected unsuppressed FSI: ${it.logReason}", it.shouldInterrupt)
if (expectWouldInterruptWithoutDnd != null) {
assertEquals(