Merge "[Dumps] Give dumpables NORMAL or CRITICAL priority." into tm-qpr-dev

This commit is contained in:
Caitlin Shkuratov
2022-11-22 20:46:25 +00:00
committed by Android (Google) Code Review
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