Merge changes I89aeecb8,I49cbbd17,I823efa1b,Ic2b99e7b,I1e91a5b3 into udc-qpr-dev

* changes:
  Demote 2 Dumpables from CRITICAL for being slow
  Print when the sysui dump actually starts.
  Ensure buffers are parsed by using standard divider
  Include the dump time and warning at the end of each Dumpable dump
  Extract the SystemUI 'config' from DumpHandler to a critical Dumpable

packages/SystemUI/src/com/android/systemui/CoreStartable.java

@@ -53,6 +53,11 @@ public interface CoreStartable extends Dumpable {
     default void dump(@NonNull PrintWriter pw, @NonNull String[] args) {
     }
 
+    /** Called to determine if the dumpable should be registered as critical or normal priority */
+    default boolean isDumpCritical() {
+        return true;
+    }
+
     /** Called immediately after the system broadcasts
      * {@link android.content.Intent#ACTION_LOCKED_BOOT_COMPLETED} or during SysUI startup if the
      * property {@code sys.boot_completed} is already set to 1. The latter typically occurs when

packages/SystemUI/src/com/android/systemui/SystemUIApplication.java

@@ -254,11 +254,16 @@ public class SystemUIApplication extends Application implements
         }
 
         for (i = 0; i < mServices.length; i++) {
+            CoreStartable service = mServices[i];
             if (mBootCompleteCache.isBootComplete()) {
-                notifyBootCompleted(mServices[i]);
+                notifyBootCompleted(service);
             }
 
-            dumpManager.registerDumpable(mServices[i].getClass().getName(), mServices[i]);
+            if (service.isDumpCritical()) {
+                dumpManager.registerCriticalDumpable(service.getClass().getName(), service);
+            } else {
+                dumpManager.registerNormalDumpable(service.getClass().getName(), service);
+            }
         }
         mSysUIComponent.getInitController().executePostInitTasks();
         log.traceEnd();

packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt

@@ -16,12 +16,10 @@
 
 package com.android.systemui.dump
 
-import android.content.Context
+import android.icu.text.SimpleDateFormat
 import android.os.SystemClock
 import android.os.Trace
-import com.android.systemui.CoreStartable
 import com.android.systemui.ProtoDumpable
-import com.android.systemui.R
 import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_CRITICAL
 import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_NORMAL
 import com.android.systemui.dump.DumpsysEntry.DumpableEntry
@@ -35,8 +33,9 @@ import java.io.BufferedOutputStream
 import java.io.FileDescriptor
 import java.io.FileOutputStream
 import java.io.PrintWriter
+import java.util.Locale
 import javax.inject.Inject
-import javax.inject.Provider
+import kotlin.system.measureTimeMillis
 
 /**
  * Oversees SystemUI's output during bug reports (and dumpsys in general)
@@ -77,6 +76,7 @@ import javax.inject.Provider
  * $ <invocation> dumpables
  * $ <invocation> buffers
  * $ <invocation> tables
+ * $ <invocation> all
  *
  * # Finally, the following will simulate what we dump during the CRITICAL and NORMAL sections of a
  * # bug report:
@@ -91,10 +91,9 @@ import javax.inject.Provider
 class DumpHandler
 @Inject
 constructor(
-    private val context: Context,
     private val dumpManager: DumpManager,
     private val logBufferEulogizer: LogBufferEulogizer,
-    private val startables: MutableMap<Class<*>, Provider<CoreStartable>>,
+    private val config: SystemUIConfigDumpable,
 ) {
     /** Dump the diagnostics! Behavior can be controlled via [args]. */
     fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array<String>) {
@@ -109,6 +108,8 @@ constructor(
                 return
             }
 
+        pw.print("Dump starting: ")
+        pw.println(DATE_FORMAT.format(System.currentTimeMillis()))
         when {
             parsedArgs.dumpPriority == PRIORITY_ARG_CRITICAL -> dumpCritical(pw, parsedArgs)
             parsedArgs.dumpPriority == PRIORITY_ARG_NORMAL && !parsedArgs.proto -> {
@@ -129,6 +130,11 @@ constructor(
             "dumpables" -> dumpDumpables(pw, args)
             "buffers" -> dumpBuffers(pw, args)
             "tables" -> dumpTables(pw, args)
+            "all" -> {
+                dumpDumpables(pw, args)
+                dumpBuffers(pw, args)
+                dumpTables(pw, args)
+            }
             "config" -> dumpConfig(pw)
             "help" -> dumpHelp(pw)
             else -> {
@@ -148,7 +154,6 @@ constructor(
                 dumpDumpable(target, pw, args.rawArgs)
             }
         }
-        dumpConfig(pw)
     }
 
     private fun dumpNormal(pw: PrintWriter, args: ParsedArgs) {
@@ -251,53 +256,8 @@ constructor(
             .sortedBy { it.name }
             .minByOrNull { it.name.length }
 
-    private fun dumpDumpable(entry: DumpableEntry, pw: PrintWriter, args: Array<String>) {
-        pw.preamble(entry)
-        entry.dumpable.dump(pw, args)
-    }
-
-    private fun dumpBuffer(entry: LogBufferEntry, pw: PrintWriter, tailLength: Int) {
-        pw.preamble(entry)
-        entry.buffer.dump(pw, tailLength)
-    }
-
-    private fun dumpTableBuffer(buffer: TableLogBufferEntry, pw: PrintWriter, args: Array<String>) {
-        pw.preamble(buffer)
-        buffer.table.dump(pw, args)
-    }
-
     private fun dumpConfig(pw: PrintWriter) {
-        pw.println("SystemUiServiceComponents configuration:")
+        config.dump(pw, arrayOf())
-        pw.print("vendor component: ")
-        pw.println(context.resources.getString(R.string.config_systemUIVendorServiceComponent))
-        val services: MutableList<String> =
-            startables.keys.map({ cls: Class<*> -> cls.simpleName }).toMutableList()
-
-        services.add(context.resources.getString(R.string.config_systemUIVendorServiceComponent))
-        dumpServiceList(pw, "global", services.toTypedArray())
-        dumpServiceList(pw, "per-user", R.array.config_systemUIServiceComponentsPerUser)
-    }
-
-    private fun dumpServiceList(pw: PrintWriter, type: String, resId: Int) {
-        val services: Array<String> = context.resources.getStringArray(resId)
-        dumpServiceList(pw, type, services)
-    }
-
-    private fun dumpServiceList(pw: PrintWriter, type: String, services: Array<String>?) {
-        pw.print(type)
-        pw.print(": ")
-        if (services == null) {
-            pw.println("N/A")
-            return
-        }
-        pw.print(services.size)
-        pw.println(" services")
-        for (i in services.indices) {
-            pw.print("  ")
-            pw.print(i)
-            pw.print(": ")
-            pw.println(services[i])
-        }
     }
 
     private fun dumpHelp(pw: PrintWriter) {
@@ -426,13 +386,6 @@ constructor(
         const val DUMPSYS_DUMPABLE_DIVIDER =
             "----------------------------------------------------------------------------"
 
-        /**
-         * Important: do not change this divider without updating any bug report processing tools
-         * (e.g. ABT), since this divider is used to determine boundaries for bug report views
-         */
-        const val DUMPSYS_BUFFER_DIVIDER =
-            "============================================================================"
-
         private fun findBestTargetMatch(c: Collection<DumpsysEntry>, target: String) =
             c.asSequence().filter { it.name.endsWith(target) }.minByOrNull { it.name.length }
 
@@ -453,42 +406,71 @@ constructor(
                 is DumpableEntry,
                 is TableLogBufferEntry -> {
                     println()
-                    println(entry.name)
+                    println("${entry.name}:")
                     println(DUMPSYS_DUMPABLE_DIVIDER)
                 }
                 is LogBufferEntry -> {
                     println()
                     println()
                     println("BUFFER ${entry.name}:")
-                    println(DUMPSYS_BUFFER_DIVIDER)
+                    println(DUMPSYS_DUMPABLE_DIVIDER)
                 }
             }
 
-        /**
+        private fun PrintWriter.footer(entry: DumpsysEntry, dumpTimeMillis: Long) {
-         * Zero-arg utility to write a [DumpableEntry] to the given [PrintWriter] in a
+            if (entry !is DumpableEntry) return
-         * dumpsys-appropriate format.
+            println()
-         */
+            print(entry.priority)
-        private fun dumpDumpable(entry: DumpableEntry, pw: PrintWriter) {
+            print(" dump took ")
-            pw.preamble(entry)
+            print(dumpTimeMillis)
-            entry.dumpable.dump(pw, arrayOf())
+            print("ms -- ")
+            print(entry.name)
+            if (entry.priority == DumpPriority.CRITICAL && dumpTimeMillis > 25) {
+                print(" -- warning: individual dump time exceeds 5% of total CRITICAL dump time!")
+            }
+            println()
+        }
+
+        private inline fun PrintWriter.wrapSection(entry: DumpsysEntry, block: () -> Unit) {
+            preamble(entry)
+            val dumpTime = measureTimeMillis(block)
+            footer(entry, dumpTime)
         }
 
         /**
-         * Zero-arg utility to write a [LogBufferEntry] to the given [PrintWriter] in a
+         * Utility to write a [DumpableEntry] to the given [PrintWriter] in a
          * dumpsys-appropriate format.
          */
-        private fun dumpBuffer(entry: LogBufferEntry, pw: PrintWriter) {
+        private fun dumpDumpable(
-            pw.preamble(entry)
+                entry: DumpableEntry,
-            entry.buffer.dump(pw, 0)
+                pw: PrintWriter,
+                args: Array<String> = arrayOf(),
+        ) = pw.wrapSection(entry) {
+            entry.dumpable.dump(pw, args)
         }
 
         /**
-         * Zero-arg utility to write a [TableLogBufferEntry] to the given [PrintWriter] in a
+         * Utility to write a [LogBufferEntry] to the given [PrintWriter] in a
          * dumpsys-appropriate format.
          */
-        private fun dumpTableBuffer(entry: TableLogBufferEntry, pw: PrintWriter) {
+        private fun dumpBuffer(
-            pw.preamble(entry)
+                entry: LogBufferEntry,
-            entry.table.dump(pw, arrayOf())
+                pw: PrintWriter,
+                tailLength: Int = 0,
+        ) = pw.wrapSection(entry) {
+            entry.buffer.dump(pw, tailLength)
+        }
+
+        /**
+         * Utility to write a [TableLogBufferEntry] to the given [PrintWriter] in a
+         * dumpsys-appropriate format.
+         */
+        private fun dumpTableBuffer(
+                entry: TableLogBufferEntry,
+                pw: PrintWriter,
+                args: Array<String> = arrayOf(),
+        ) = pw.wrapSection(entry) {
+            entry.table.dump(pw, args)
         }
 
         /**
@@ -510,6 +492,7 @@ constructor(
     }
 }
 
+private val DATE_FORMAT = SimpleDateFormat("MM-dd HH:mm:ss.SSS", Locale.US)
 private val PRIORITY_OPTIONS = arrayOf(PRIORITY_ARG_CRITICAL, PRIORITY_ARG_NORMAL)
 
 private val COMMANDS =

packages/SystemUI/src/com/android/systemui/dump/SystemUIConfigDumpable.kt

@@ -0,0 +1,73 @@
+/*
+ * 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.dump
+
+import android.content.Context
+import com.android.systemui.CoreStartable
+import com.android.systemui.Dumpable
+import com.android.systemui.R
+import com.android.systemui.dagger.SysUISingleton
+import java.io.PrintWriter
+import javax.inject.Inject
+import javax.inject.Provider
+
+@SysUISingleton
+class SystemUIConfigDumpable
+@Inject
+constructor(
+    dumpManager: DumpManager,
+    private val context: Context,
+    private val startables: MutableMap<Class<*>, Provider<CoreStartable>>,
+) : Dumpable {
+    init {
+        dumpManager.registerCriticalDumpable("SystemUiServiceComponents", this)
+    }
+
+    override fun dump(pw: PrintWriter, args: Array<out String>) {
+        pw.println("SystemUiServiceComponents configuration:")
+        pw.print("vendor component: ")
+        pw.println(context.resources.getString(R.string.config_systemUIVendorServiceComponent))
+        val services: MutableList<String> =
+            startables.keys.map { cls: Class<*> -> cls.simpleName }.toMutableList()
+
+        services.add(context.resources.getString(R.string.config_systemUIVendorServiceComponent))
+        dumpServiceList(pw, "global", services.toTypedArray())
+        dumpServiceList(pw, "per-user", R.array.config_systemUIServiceComponentsPerUser)
+    }
+
+    private fun dumpServiceList(pw: PrintWriter, type: String, resId: Int) {
+        val services: Array<String> = context.resources.getStringArray(resId)
+        dumpServiceList(pw, type, services)
+    }
+
+    private fun dumpServiceList(pw: PrintWriter, type: String, services: Array<String>?) {
+        pw.print(type)
+        pw.print(": ")
+        if (services == null) {
+            pw.println("N/A")
+            return
+        }
+        pw.print(services.size)
+        pw.println(" services")
+        for (i in services.indices) {
+            pw.print("  ")
+            pw.print(i)
+            pw.print(": ")
+            pw.println(services[i])
+        }
+    }
+}

packages/SystemUI/src/com/android/systemui/statusbar/connectivity/NetworkControllerImpl.java

@@ -83,8 +83,6 @@ import com.android.systemui.statusbar.policy.DeviceProvisionedController.DeviceP
 import com.android.systemui.telephony.TelephonyListenerManager;
 import com.android.systemui.util.CarrierConfigTracker;
 
-import kotlin.Unit;
-
 import java.io.PrintWriter;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -99,6 +97,8 @@ import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
+import kotlin.Unit;
+
 /** Platform implementation of the network controller. **/
 @SysUISingleton
 public class NetworkControllerImpl extends BroadcastReceiver
@@ -465,7 +465,7 @@ public class NetworkControllerImpl extends BroadcastReceiver
 
         mDemoModeController.addCallback(this);
 
-        mDumpManager.registerDumpable(TAG, this);
+        mDumpManager.registerNormalDumpable(TAG, this);
     }
 
     private final Runnable mClearForceValidated = () -> {

packages/SystemUI/src/com/android/systemui/wmshell/WMShell.java

@@ -351,6 +351,13 @@ public final class WMShell implements
                 }, mSysUiMainExecutor);
     }
 
+    @Override
+    public boolean isDumpCritical() {
+        // Dump can't be critical because the shell has to dump on the main thread for
+        // synchronization reasons, which isn't reliably fast.
+        return false;
+    }
+
     @Override
     public void dump(PrintWriter pw, String[] args) {
         // Handle commands if provided

packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt

@@ -26,10 +26,6 @@ import com.android.systemui.log.table.TableLogBuffer
 import com.android.systemui.util.mockito.any
 import com.android.systemui.util.mockito.eq
 import com.google.common.truth.Truth.assertThat
-import java.io.FileDescriptor
-import java.io.PrintWriter
-import java.io.StringWriter
-import javax.inject.Provider
 import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
@@ -37,6 +33,10 @@ import org.mockito.Mockito.anyInt
 import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
 import org.mockito.MockitoAnnotations
+import java.io.FileDescriptor
+import java.io.PrintWriter
+import java.io.StringWriter
+import javax.inject.Provider
 
 @SmallTest
 class DumpHandlerTest : SysuiTestCase() {
@@ -79,14 +79,12 @@ class DumpHandlerTest : SysuiTestCase() {
     fun setUp() {
         MockitoAnnotations.initMocks(this)
 
-        dumpHandler = DumpHandler(
+        val config = SystemUIConfigDumpable(
-            mContext,
                 dumpManager,
-            logBufferEulogizer,
+                mContext,
-            mutableMapOf(
+                mutableMapOf(EmptyCoreStartable::class.java to Provider { EmptyCoreStartable() }),
-                EmptyCoreStartable::class.java to Provider { EmptyCoreStartable() }
-            ),
         )
+        dumpHandler = DumpHandler(dumpManager, logBufferEulogizer, config)
     }
 
     @Test