Merge "[Dumps] Give dumpables NORMAL or CRITICAL priority." into tm-qpr-dev
diff --git a/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt b/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt
index 609bd76..a2f65ba 100644
--- a/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt
+++ b/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt
@@ -22,7 +22,6 @@
 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 @@
     }
 
     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 @@
     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",
diff --git a/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt b/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt
index ae78089..c982131 100644
--- a/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt
@@ -40,20 +40,54 @@
     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 @@
         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 @@
     }
 
     /**
-     * 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 @@
 
 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,
+}
diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt
index 6271334..7189f00 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt
+++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebugStartable.kt
@@ -35,7 +35,7 @@
 ) : CoreStartable {
 
     init {
-        dumpManager.registerDumpable(FeatureFlagsDebug.TAG) { pw, args ->
+        dumpManager.registerCriticalDumpable(FeatureFlagsDebug.TAG) { pw, args ->
             featureFlags.dump(pw, args)
         }
     }
diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsReleaseStartable.kt b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsReleaseStartable.kt
index e7d8cc3..d088d74 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsReleaseStartable.kt
+++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsReleaseStartable.kt
@@ -29,7 +29,7 @@
 constructor(dumpManager: DumpManager, featureFlags: FeatureFlags) : CoreStartable {
 
     init {
-        dumpManager.registerDumpable(FeatureFlagsRelease.TAG) { pw, args ->
+        dumpManager.registerCriticalDumpable(FeatureFlagsRelease.TAG) { pw, args ->
             featureFlags.dump(pw, args)
         }
     }
diff --git a/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt b/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt
index 1054aa5..07820ec 100644
--- a/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt
@@ -78,7 +78,7 @@
             })
         shadeExpansionStateManager.addExpansionListener(this::onPanelExpansionChanged)
         shadeExpansionStateManager.addStateListener(this::onPanelStateChanged)
-        dumpManager.registerDumpable("ShadeTransitionController") { printWriter, _ ->
+        dumpManager.registerCriticalDumpable("ShadeTransitionController") { printWriter, _ ->
             dump(printWriter)
         }
     }
diff --git a/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt b/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt
index fde08ee..f95125f 100644
--- a/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt
@@ -70,7 +70,7 @@
                     updateResources()
                 }
             })
-        dumpManager.registerDumpable("SplitShadeOverScroller") { printWriter, _ ->
+        dumpManager.registerCriticalDumpable("SplitShadeOverScroller") { printWriter, _ ->
             dump(printWriter)
         }
     }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShadeDepthController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShadeDepthController.kt
index b5879ec..8dc7842 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShadeDepthController.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShadeDepthController.kt
@@ -304,7 +304,7 @@
     }
 
     init {
-        dumpManager.registerDumpable(javaClass.name, this)
+        dumpManager.registerCriticalDumpable(javaClass.name, this)
         if (WAKE_UP_ANIMATION_ENABLED) {
             keyguardStateController.addCallback(keyguardStateCallback)
         }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScroller.kt b/packages/SystemUI/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScroller.kt
index 13d8adb..572c0e0 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScroller.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScroller.kt
@@ -51,8 +51,8 @@
                     updateResources()
                 }
             })
-        dumpManager.registerDumpable("SplitShadeLockscreenOverScroller") { printWriter, _ ->
-            dump(printWriter)
+        dumpManager.registerCriticalDumpable("SplitShadeLockscreenOverScroller") { pw, _ ->
+            dump(pw)
         }
     }
 
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt
index 4496607..3989854 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt
@@ -62,7 +62,7 @@
     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() {
diff --git a/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt
index 65ae90b..19135d0 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt
@@ -103,8 +103,8 @@
         // 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 @@
     @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 @@
         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 @@
     @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 @@
         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)
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/dump/DumpManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/dump/DumpManagerTest.kt
new file mode 100644
index 0000000..0c5a74c
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/dump/DumpManagerTest.kt
@@ -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))
+    }
+}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationShadeDepthControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationShadeDepthControllerTest.kt
index 77b1e37..beaf300 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationShadeDepthControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationShadeDepthControllerTest.kt
@@ -131,7 +131,9 @@
 
     @Test
     fun setupListeners() {
-        verify(dumpManager).registerDumpable(anyString(), eq(notificationShadeDepthController))
+        verify(dumpManager).registerCriticalDumpable(
+            anyString(), eq(notificationShadeDepthController)
+        )
     }
 
     @Test