[Dumps] Give dumpables NORMAL or CRITICAL priority.

Previously, dumpables were always dumped in the critical section. Now,
dumpables can be in the normal OR critical section.

Also adds DumpManagerTest, with some tests being eeriely similar to
DumpHandlerTest but also some new tests!

Bug: 259973758
Test: `adb shell dumpsys statusbar --proto` -> dumps protos
Test: `adb shell dumpsys activity service
com.android.systemui/.SystemUIService` -> dumps everything
Test: `adb shell dumpsys activity service
com.android.systemui/.SystemUIService {InsertDumpableNameHere}` -> dumps
just that dumpable
Test: `adb shell dumpsys activity service
com.android.systemui/.SystemUIService {InsertBufferNameHere}` -> dumps
just that buffer
Test: atest DumpManagerTest
Test: atest DumpHandlerTest

Change-Id: I1a8506b0ffd2134647d361d88885d0cc033610c4
This commit is contained in:
Caitlin Shkuratov
2022-11-21 18:49:58 +00:00
parent d41f29df84
commit 7759669c31
12 changed files with 330 additions and 37 deletions

View File

@@ -22,7 +22,6 @@ import android.os.Trace
import com.android.systemui.CoreStartable
import com.android.systemui.R
import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_CRITICAL
import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_HIGH
import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_NORMAL
import com.android.systemui.dump.nano.SystemUIProtoDump
import com.android.systemui.plugins.log.LogBuffer
@@ -148,12 +147,12 @@ class DumpHandler @Inject constructor(
}
private fun dumpCritical(pw: PrintWriter, args: ParsedArgs) {
dumpManager.dumpDumpables(pw, args.rawArgs)
dumpManager.dumpCritical(pw, args.rawArgs)
dumpConfig(pw)
}
private fun dumpNormal(pw: PrintWriter, args: ParsedArgs) {
dumpManager.dumpBuffers(pw, args.tailLength)
dumpManager.dumpNormal(pw, args.rawArgs, args.tailLength)
logBufferEulogizer.readEulogyIfPresent(pw)
}
@@ -349,14 +348,12 @@ class DumpHandler @Inject constructor(
companion object {
const val PRIORITY_ARG = "--dump-priority"
const val PRIORITY_ARG_CRITICAL = "CRITICAL"
const val PRIORITY_ARG_HIGH = "HIGH"
const val PRIORITY_ARG_NORMAL = "NORMAL"
const val PROTO = "--proto"
}
}
private val PRIORITY_OPTIONS =
arrayOf(PRIORITY_ARG_CRITICAL, PRIORITY_ARG_HIGH, PRIORITY_ARG_NORMAL)
private val PRIORITY_OPTIONS = arrayOf(PRIORITY_ARG_CRITICAL, PRIORITY_ARG_NORMAL)
private val COMMANDS = arrayOf(
"bugreport-critical",

View File

@@ -40,20 +40,54 @@ open class DumpManager @Inject constructor() {
private val buffers: MutableMap<String, RegisteredDumpable<LogBuffer>> = ArrayMap()
/**
* Register a dumpable to be called during a bug report. The dumpable will be called during the
* CRITICAL section of the bug report, so don't dump an excessive amount of stuff here.
* Registers a dumpable to be called during the CRITICAL section of the bug report.
*
* The CRITICAL section gets very high priority during a dump, but also a very limited amount of
* time to do the dumping. So, please don't dump an excessive amount of stuff using CRITICAL.
*
* See [registerDumpable].
*/
fun registerCriticalDumpable(name: String, module: Dumpable) {
registerDumpable(name, module, DumpPriority.CRITICAL)
}
/**
* Registers a dumpable to be called during the NORMAL section of the bug report.
*
* The NORMAL section gets a lower priority during a dump, but also more time. This should be
* used by [LogBuffer] instances, [ProtoDumpable] instances, and any [Dumpable] instances that
* dump a lot of information.
*/
fun registerNormalDumpable(name: String, module: Dumpable) {
registerDumpable(name, module, DumpPriority.NORMAL)
}
/**
* Register a dumpable to be called during a bug report.
*
* @param name The name to register the dumpable under. This is typically the qualified class
* name of the thing being dumped (getClass().getName()), but can be anything as long as it
* doesn't clash with an existing registration.
* @param priority the priority level of this dumpable, which affects at what point in the bug
* report this gets dump. By default, the dumpable will be called during the CRITICAL section of
* the bug report, so don't dump an excessive amount of stuff here.
*
* TODO(b/259973758): Replace all calls to this method with calls to [registerCriticalDumpable]
* or [registerNormalDumpable] instead.
*/
@Synchronized
fun registerDumpable(name: String, module: Dumpable) {
@JvmOverloads
@Deprecated("Use registerCriticalDumpable or registerNormalDumpable instead")
fun registerDumpable(
name: String,
module: Dumpable,
priority: DumpPriority = DumpPriority.CRITICAL,
) {
if (!canAssignToNameLocked(name, module)) {
throw IllegalArgumentException("'$name' is already registered")
}
dumpables[name] = RegisteredDumpable(name, module)
dumpables[name] = RegisteredDumpable(name, module, priority)
}
/**
@@ -81,7 +115,10 @@ open class DumpManager @Inject constructor() {
if (!canAssignToNameLocked(name, buffer)) {
throw IllegalArgumentException("'$name' is already registered")
}
buffers[name] = RegisteredDumpable(name, buffer)
// All buffers must be priority NORMAL, not CRITICAL, because they often contain a lot of
// data.
buffers[name] = RegisteredDumpable(name, buffer, DumpPriority.NORMAL)
}
/**
@@ -140,7 +177,35 @@ open class DumpManager @Inject constructor() {
}
/**
* Dumps all registered dumpables to [pw]
* Dumps all registered dumpables with critical priority to [pw]
*/
@Synchronized
fun dumpCritical(pw: PrintWriter, args: Array<String>) {
for (dumpable in dumpables.values) {
if (dumpable.priority == DumpPriority.CRITICAL) {
dumpDumpable(dumpable, pw, args)
}
}
}
/**
* To [pw], dumps (1) all registered dumpables with normal priority; and (2) all [LogBuffer]s.
*/
@Synchronized
fun dumpNormal(pw: PrintWriter, args: Array<String>, tailLength: Int = 0) {
for (dumpable in dumpables.values) {
if (dumpable.priority == DumpPriority.NORMAL) {
dumpDumpable(dumpable, pw, args)
}
}
for (buffer in buffers.values) {
dumpBuffer(buffer, pw, tailLength)
}
}
/**
* Dump all the instances of [Dumpable].
*/
@Synchronized
fun dumpDumpables(pw: PrintWriter, args: Array<String>) {
@@ -232,7 +297,15 @@ open class DumpManager @Inject constructor() {
private data class RegisteredDumpable<T>(
val name: String,
val dumpable: T
val dumpable: T,
val priority: DumpPriority,
)
private const val TAG = "DumpManager"
/**
* The priority level for a given dumpable, which affects at what point in the bug report this gets
* dumped.
*/
enum class DumpPriority {
CRITICAL,
NORMAL,
}

View File

@@ -35,7 +35,7 @@ constructor(
) : CoreStartable {
init {
dumpManager.registerDumpable(FeatureFlagsDebug.TAG) { pw, args ->
dumpManager.registerCriticalDumpable(FeatureFlagsDebug.TAG) { pw, args ->
featureFlags.dump(pw, args)
}
}

View File

@@ -29,7 +29,7 @@ class FeatureFlagsReleaseStartable
constructor(dumpManager: DumpManager, featureFlags: FeatureFlags) : CoreStartable {
init {
dumpManager.registerDumpable(FeatureFlagsRelease.TAG) { pw, args ->
dumpManager.registerCriticalDumpable(FeatureFlagsRelease.TAG) { pw, args ->
featureFlags.dump(pw, args)
}
}

View File

@@ -78,7 +78,7 @@ constructor(
})
shadeExpansionStateManager.addExpansionListener(this::onPanelExpansionChanged)
shadeExpansionStateManager.addStateListener(this::onPanelStateChanged)
dumpManager.registerDumpable("ShadeTransitionController") { printWriter, _ ->
dumpManager.registerCriticalDumpable("ShadeTransitionController") { printWriter, _ ->
dump(printWriter)
}
}

View File

@@ -70,7 +70,7 @@ constructor(
updateResources()
}
})
dumpManager.registerDumpable("SplitShadeOverScroller") { printWriter, _ ->
dumpManager.registerCriticalDumpable("SplitShadeOverScroller") { printWriter, _ ->
dump(printWriter)
}
}

View File

@@ -304,7 +304,7 @@ class NotificationShadeDepthController @Inject constructor(
}
init {
dumpManager.registerDumpable(javaClass.name, this)
dumpManager.registerCriticalDumpable(javaClass.name, this)
if (WAKE_UP_ANIMATION_ENABLED) {
keyguardStateController.addCallback(keyguardStateCallback)
}

View File

@@ -51,8 +51,8 @@ constructor(
updateResources()
}
})
dumpManager.registerDumpable("SplitShadeLockscreenOverScroller") { printWriter, _ ->
dump(printWriter)
dumpManager.registerCriticalDumpable("SplitShadeLockscreenOverScroller") { pw, _ ->
dump(pw)
}
}

View File

@@ -62,7 +62,7 @@ constructor(
private var statusBarBoundsProvider: StatusBarBoundsProvider? = null
override fun start() {
dumpManager.registerDumpable(javaClass.simpleName) { printWriter, _ -> dump(printWriter) }
dumpManager.registerCriticalDumpable(javaClass.simpleName) { pw, _ -> dump(pw) }
}
override fun stop() {

View File

@@ -103,8 +103,8 @@ class DumpHandlerTest : SysuiTestCase() {
// THEN only the requested ones have their dump() method called
verify(dumpable1).dump(pw, args)
verify(dumpable2, never()).dump(
any(PrintWriter::class.java),
any(Array<String>::class.java))
any(PrintWriter::class.java),
any(Array<String>::class.java))
verify(dumpable3).dump(pw, args)
verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())
verify(buffer2).dump(pw, 0)
@@ -126,9 +126,9 @@ class DumpHandlerTest : SysuiTestCase() {
@Test
fun testCriticalDump() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerDumpable("dumpable1", dumpable1)
dumpManager.registerDumpable("dumpable2", dumpable2)
dumpManager.registerDumpable("dumpable3", dumpable3)
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
@@ -136,10 +136,12 @@ class DumpHandlerTest : SysuiTestCase() {
val args = arrayOf("--dump-priority", "CRITICAL")
dumpHandler.dump(fd, pw, args)
// THEN all modules are dumped (but no buffers)
// THEN only critical modules are dumped (and no buffers)
verify(dumpable1).dump(pw, args)
verify(dumpable2).dump(pw, args)
verify(dumpable3).dump(pw, args)
verify(dumpable3, never()).dump(
any(PrintWriter::class.java),
any(Array<String>::class.java))
verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())
verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())
}
@@ -147,9 +149,9 @@ class DumpHandlerTest : SysuiTestCase() {
@Test
fun testNormalDump() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerDumpable("dumpable1", dumpable1)
dumpManager.registerDumpable("dumpable2", dumpable2)
dumpManager.registerDumpable("dumpable3", dumpable3)
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
@@ -157,16 +159,14 @@ class DumpHandlerTest : SysuiTestCase() {
val args = arrayOf("--dump-priority", "NORMAL")
dumpHandler.dump(fd, pw, args)
// THEN all buffers are dumped (but no modules)
// THEN the normal module and all buffers are dumped
verify(dumpable1, never()).dump(
any(PrintWriter::class.java),
any(Array<String>::class.java))
verify(dumpable2, never()).dump(
any(PrintWriter::class.java),
any(Array<String>::class.java))
verify(dumpable3, never()).dump(
any(PrintWriter::class.java),
any(Array<String>::class.java))
verify(dumpable3).dump(pw, args)
verify(buffer1).dump(pw, 0)
verify(buffer2).dump(pw, 0)
}

View File

@@ -0,0 +1,221 @@
/*
* Copyright (C) 2020 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.dump
import androidx.test.filters.SmallTest
import com.android.systemui.Dumpable
import com.android.systemui.SysuiTestCase
import com.android.systemui.plugins.log.LogBuffer
import com.android.systemui.util.mockito.any
import java.io.PrintWriter
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.Mockito.anyInt
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
@SmallTest
class DumpManagerTest : SysuiTestCase() {
@Mock private lateinit var pw: PrintWriter
@Mock private lateinit var dumpable1: Dumpable
@Mock private lateinit var dumpable2: Dumpable
@Mock private lateinit var dumpable3: Dumpable
@Mock private lateinit var buffer1: LogBuffer
@Mock private lateinit var buffer2: LogBuffer
private val dumpManager = DumpManager()
@Before
fun setUp() {
MockitoAnnotations.initMocks(this)
}
@Test
fun testDumpTarget_dumpable() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerCriticalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
// WHEN a dumpable is dumped explicitly
val args = arrayOf<String>()
dumpManager.dumpTarget("dumpable2", pw, arrayOf(), tailLength = 0)
// THEN only the requested one has their dump() method called
verify(dumpable1, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable2).dump(pw, args)
verify(dumpable3, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())
verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())
}
@Test
fun testDumpTarget_buffer() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerCriticalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
// WHEN a buffer is dumped explicitly
dumpManager.dumpTarget("buffer1", pw, arrayOf(), tailLength = 14)
// THEN only the requested one has their dump() method called
verify(dumpable1, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable2, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable2, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(buffer1).dump(pw, tailLength = 14)
verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())
}
@Test
fun testDumpableMatchingIsBasedOnEndOfTag() {
// GIVEN a dumpable registered to the manager
dumpManager.registerCriticalDumpable("com.android.foo.bar.dumpable1", dumpable1)
// WHEN that module is dumped
val args = arrayOf<String>()
dumpManager.dumpTarget("dumpable1", pw, arrayOf(), tailLength = 14)
// THEN its dump() method is called
verify(dumpable1).dump(pw, args)
}
@Test
fun testDumpDumpables() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
// WHEN a dumpable dump is requested
val args = arrayOf<String>()
dumpManager.dumpDumpables(pw, args)
// THEN all dumpables are dumped (both critical and normal) (and no dumpables)
verify(dumpable1).dump(pw, args)
verify(dumpable2).dump(pw, args)
verify(dumpable3).dump(pw, args)
verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())
verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())
}
@Test
fun testDumpBuffers() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
// WHEN a buffer dump is requested
dumpManager.dumpBuffers(pw, tailLength = 1)
// THEN all buffers are dumped (and no dumpables)
verify(dumpable1, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable2, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable3, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(buffer1).dump(pw, tailLength = 1)
verify(buffer2).dump(pw, tailLength = 1)
}
@Test
fun testCriticalDump() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
// WHEN a critical dump is requested
val args = arrayOf<String>()
dumpManager.dumpCritical(pw, args)
// THEN only critical modules are dumped (and no buffers)
verify(dumpable1).dump(pw, args)
verify(dumpable2).dump(pw, args)
verify(dumpable3, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())
verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())
}
@Test
fun testNormalDump() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.registerBuffer("buffer1", buffer1)
dumpManager.registerBuffer("buffer2", buffer2)
// WHEN a normal dump is requested
val args = arrayOf<String>()
dumpManager.dumpNormal(pw, args, tailLength = 2)
// THEN the normal module and all buffers are dumped
verify(dumpable1, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable2, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable3).dump(pw, args)
verify(buffer1).dump(pw, tailLength = 2)
verify(buffer2).dump(pw, tailLength = 2)
}
@Test
fun testUnregister() {
// GIVEN a variety of registered dumpables and buffers
dumpManager.registerCriticalDumpable("dumpable1", dumpable1)
dumpManager.registerCriticalDumpable("dumpable2", dumpable2)
dumpManager.registerNormalDumpable("dumpable3", dumpable3)
dumpManager.unregisterDumpable("dumpable2")
dumpManager.unregisterDumpable("dumpable3")
// WHEN a dumpables dump is requested
val args = arrayOf<String>()
dumpManager.dumpDumpables(pw, args)
// THEN the unregistered dumpables (both normal and critical) are not dumped
verify(dumpable1).dump(pw, args)
verify(dumpable2, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
verify(dumpable3, never())
.dump(any(PrintWriter::class.java), any(Array<String>::class.java))
}
}

View File

@@ -131,7 +131,9 @@ class NotificationShadeDepthControllerTest : SysuiTestCase() {
@Test
fun setupListeners() {
verify(dumpManager).registerDumpable(anyString(), eq(notificationShadeDepthController))
verify(dumpManager).registerCriticalDumpable(
anyString(), eq(notificationShadeDepthController)
)
}
@Test