Merge "Minor refactor that moves the code for dumping LogMessage to an extension method for LogMessage." into tm-qpr-dev

packages/SystemUI/src/com/android/systemui/log/LogBuffer.kt

@@ -22,17 +22,11 @@ import com.android.systemui.log.dagger.LogModule
 import com.android.systemui.util.collection.RingBuffer
 import com.google.errorprone.annotations.CompileTimeConstant
 import java.io.PrintWriter
-import java.text.SimpleDateFormat
-import java.util.Arrays.stream
-import java.util.Locale
 import java.util.concurrent.ArrayBlockingQueue
 import java.util.concurrent.BlockingQueue
 import kotlin.concurrent.thread
 import kotlin.math.max
 
-const val UNBOUNDED_STACK_TRACE = -1
-const val NESTED_TRACE_DEPTH = 10
-
 /**
  * A simple ring buffer of recyclable log messages
  *
@@ -74,18 +68,12 @@ const val NESTED_TRACE_DEPTH = 10
  * @param maxSize The maximum number of messages to keep in memory at any one time. Buffers start
  * out empty and grow up to [maxSize] as new messages are logged. Once the buffer's size reaches
  * the maximum, it behaves like a ring buffer.
- * @param rootStackTraceDepth The number of stack trace elements to be logged for an exception when
- * the logBuffer is dumped. Defaulted to -1 [UNBOUNDED_STACK_TRACE] to print the entire stack trace.
- * @param nestedStackTraceDepth The number of stack trace elements to be logged for any nested
- * exceptions present in [Throwable.cause] or [Throwable.suppressedExceptions].
  */
 class LogBuffer @JvmOverloads constructor(
     private val name: String,
     private val maxSize: Int,
     private val logcatEchoTracker: LogcatEchoTracker,
     private val systrace: Boolean = true,
-    private val rootStackTraceDepth: Int = UNBOUNDED_STACK_TRACE,
-    private val nestedStackTraceDepth: Int = NESTED_TRACE_DEPTH,
 ) {
     private val buffer = RingBuffer(maxSize) { LogMessageImpl.create() }
 
@@ -236,7 +224,7 @@ class LogBuffer @JvmOverloads constructor(
         val iterationStart = if (tailLength <= 0) { 0 } else { max(0, buffer.size - tailLength) }
 
         for (i in iterationStart until buffer.size) {
-            dumpMessage(buffer[i], pw)
+            buffer[i].dump(pw)
         }
     }
 
@@ -264,76 +252,6 @@ class LogBuffer @JvmOverloads constructor(
         }
     }
 
-    private fun dumpMessage(
-        message: LogMessage,
-        pw: PrintWriter
-    ) {
-        val formattedTimestamp = DATE_FORMAT.format(message.timestamp)
-        val shortLevel = message.level.shortString
-        val messageToPrint = message.messagePrinter(message)
-        val tag = message.tag
-        printLikeLogcat(pw, formattedTimestamp, shortLevel, tag, messageToPrint)
-        message.exception?.let { ex ->
-            printException(
-                pw,
-                formattedTimestamp,
-                shortLevel,
-                ex,
-                tag,
-                stackTraceDepth = rootStackTraceDepth)
-        }
-    }
-
-    private fun printException(
-            pw: PrintWriter,
-            timestamp: String,
-            level: String,
-            exception: Throwable,
-            tag: String,
-            exceptionMessagePrefix: String = "",
-            stackTraceDepth: Int = UNBOUNDED_STACK_TRACE
-    ) {
-        val message = "$exceptionMessagePrefix$exception"
-        printLikeLogcat(pw, timestamp, level, tag, message)
-        var stacktraceStream = stream(exception.stackTrace)
-        if (stackTraceDepth != UNBOUNDED_STACK_TRACE) {
-            stacktraceStream = stacktraceStream.limit(stackTraceDepth.toLong())
-        }
-        stacktraceStream.forEach { line ->
-            printLikeLogcat(pw, timestamp, level, tag, "\tat $line")
-        }
-        exception.cause?.let { cause ->
-            printException(pw, timestamp, level, cause, tag, "Caused by: ", nestedStackTraceDepth)
-        }
-        exception.suppressedExceptions.forEach { suppressed ->
-            printException(
-                pw,
-                timestamp,
-                level,
-                suppressed,
-                tag,
-                "Suppressed: ",
-                nestedStackTraceDepth
-            )
-        }
-    }
-
-    private fun printLikeLogcat(
-        pw: PrintWriter,
-        formattedTimestamp: String,
-        shortLogLevel: String,
-        tag: String,
-        message: String
-    ) {
-        pw.print(formattedTimestamp)
-        pw.print(" ")
-        pw.print(shortLogLevel)
-        pw.print(" ")
-        pw.print(tag)
-        pw.print(": ")
-        pw.println(message)
-    }
-
     private fun echo(message: LogMessage, toLogcat: Boolean, toSystrace: Boolean) {
         if (toLogcat || toSystrace) {
             val strMessage = message.messagePrinter(message)
@@ -370,5 +288,4 @@ class LogBuffer @JvmOverloads constructor(
 typealias MessageInitializer = LogMessage.() -> Unit
 
 private const val TAG = "LogBuffer"
-private val DATE_FORMAT = SimpleDateFormat("MM-dd HH:mm:ss.SSS", Locale.US)
 private val FROZEN_MESSAGE = LogMessageImpl.create()

packages/SystemUI/src/com/android/systemui/log/LogMessage.kt

@@ -16,6 +16,10 @@
 
 package com.android.systemui.log
 
+import java.io.PrintWriter
+import java.text.SimpleDateFormat
+import java.util.Locale
+
 /**
  * Generic data class for storing messages logged to a [LogBuffer]
  *
@@ -50,6 +54,17 @@ interface LogMessage {
     var bool2: Boolean
     var bool3: Boolean
     var bool4: Boolean
+
+    /**
+     * Function that dumps the [LogMessage] to the provided [writer].
+     */
+    fun dump(writer: PrintWriter) {
+        val formattedTimestamp = DATE_FORMAT.format(timestamp)
+        val shortLevel = level.shortString
+        val messageToPrint = messagePrinter(this)
+        printLikeLogcat(writer, formattedTimestamp, shortLevel, tag, messageToPrint)
+        exception?.printStackTrace(writer)
+    }
 }
 
 /**
@@ -61,3 +76,21 @@ interface LogMessage {
  * of the printer for each call, thwarting our attempts at avoiding any sort of allocation.
  */
 typealias MessagePrinter = LogMessage.() -> String
+
+private fun printLikeLogcat(
+    pw: PrintWriter,
+    formattedTimestamp: String,
+    shortLogLevel: String,
+    tag: String,
+    message: String
+) {
+    pw.print(formattedTimestamp)
+    pw.print(" ")
+    pw.print(shortLogLevel)
+    pw.print(" ")
+    pw.print(tag)
+    pw.print(": ")
+    pw.println(message)
+}
+
+private val DATE_FORMAT = SimpleDateFormat("MM-dd HH:mm:ss.SSS", Locale.US)

packages/SystemUI/tests/src/com/android/systemui/log/LogBufferTest.kt

@@ -24,16 +24,11 @@ class LogBufferTest : SysuiTestCase() {
     @Before
     fun setup() {
         outputWriter = StringWriter()
-        buffer = createBuffer(UNBOUNDED_STACK_TRACE, NESTED_TRACE_DEPTH)
+        buffer = createBuffer()
     }
 
-    private fun createBuffer(rootTraceDepth: Int, nestedTraceDepth: Int): LogBuffer {
+    private fun createBuffer(): LogBuffer {
-        return LogBuffer("TestBuffer",
+        return LogBuffer("TestBuffer", 1, logcatEchoTracker, false)
-                1,
-                logcatEchoTracker,
-                false,
-                rootStackTraceDepth = rootTraceDepth,
-                nestedStackTraceDepth = nestedTraceDepth)
     }
 
     @Test
@@ -56,95 +51,83 @@ class LogBufferTest : SysuiTestCase() {
     }
 
     @Test
-    fun dump_writesExceptionAndStacktraceLimitedToGivenDepth() {
+    fun dump_writesExceptionAndStacktrace() {
-        buffer = createBuffer(rootTraceDepth = 2, nestedTraceDepth = -1)
+        buffer = createBuffer()
-        // stack trace depth of 5
+        val exception = createTestException("Exception message", "TestClass")
-        val exception = createTestException("Exception message", "TestClass", 5)
         buffer.log("Tag", LogLevel.ERROR, { str1 = "Extra message" }, { str1!! }, exception)
 
         val dumpedString = dumpBuffer()
 
-        // logs are limited to depth 2
+        assertThat(dumpedString).contains("Extra message")
-        assertThat(dumpedString).contains("E Tag: Extra message")
+        assertThat(dumpedString).contains("java.lang.RuntimeException: Exception message")
-        assertThat(dumpedString).contains("E Tag: java.lang.RuntimeException: Exception message")
+        assertThat(dumpedString).contains("at TestClass.TestMethod(TestClass.java:1)")
-        assertThat(dumpedString).contains("E Tag: \tat TestClass.TestMethod(TestClass.java:1)")
+        assertThat(dumpedString).contains("at TestClass.TestMethod(TestClass.java:2)")
-        assertThat(dumpedString).contains("E Tag: \tat TestClass.TestMethod(TestClass.java:2)")
-        assertThat(dumpedString)
-                .doesNotContain("E Tag: \tat TestClass.TestMethod(TestClass.java:3)")
     }
 
     @Test
-    fun dump_writesCauseAndStacktraceLimitedToGivenDepth() {
+    fun dump_writesCauseAndStacktrace() {
-        buffer = createBuffer(rootTraceDepth = 0, nestedTraceDepth = 2)
+        buffer = createBuffer()
         val exception = createTestException("Exception message",
                 "TestClass",
-                1,
+                cause = createTestException("The real cause!", "TestClass"))
-                cause = createTestException("The real cause!", "TestClass", 5))
         buffer.log("Tag", LogLevel.ERROR, { str1 = "Extra message" }, { str1!! }, exception)
 
         val dumpedString = dumpBuffer()
 
-        // logs are limited to depth 2
         assertThat(dumpedString)
-                .contains("E Tag: Caused by: java.lang.RuntimeException: The real cause!")
+                .contains("Caused by: java.lang.RuntimeException: The real cause!")
-        assertThat(dumpedString).contains("E Tag: \tat TestClass.TestMethod(TestClass.java:1)")
+        assertThat(dumpedString).contains("at TestClass.TestMethod(TestClass.java:1)")
-        assertThat(dumpedString).contains("E Tag: \tat TestClass.TestMethod(TestClass.java:2)")
+        assertThat(dumpedString).contains("at TestClass.TestMethod(TestClass.java:2)")
-        assertThat(dumpedString)
-                .doesNotContain("E Tag: \tat TestClass.TestMethod(TestClass.java:3)")
     }
 
     @Test
-    fun dump_writesSuppressedExceptionAndStacktraceLimitedToGivenDepth() {
+    fun dump_writesSuppressedExceptionAndStacktrace() {
-        buffer = createBuffer(rootTraceDepth = 0, nestedTraceDepth = 2)
+        buffer = createBuffer()
         val exception = RuntimeException("Root exception message")
         exception.addSuppressed(
                 createTestException(
                         "First suppressed exception",
                         "FirstClass",
-                        5,
+                        createTestException("Cause of suppressed exp", "ThirdClass")
-                        createTestException("Cause of suppressed exp", "ThirdClass", 5)
                 ))
         exception.addSuppressed(
-                createTestException("Second suppressed exception", "SecondClass", 5))
+                createTestException("Second suppressed exception", "SecondClass"))
         buffer.log("Tag", LogLevel.ERROR, { str1 = "Extra message" }, { str1!! }, exception)
 
         val dumpedStr = dumpBuffer()
 
-        // logs are limited to depth 2
         // first suppressed exception
         assertThat(dumpedStr)
-                .contains("E Tag: Suppressed: " +
+                .contains("Suppressed: " +
                         "java.lang.RuntimeException: First suppressed exception")
-        assertThat(dumpedStr).contains("E Tag: \tat FirstClass.TestMethod(FirstClass.java:1)")
+        assertThat(dumpedStr).contains("at FirstClass.TestMethod(FirstClass.java:1)")
-        assertThat(dumpedStr).contains("E Tag: \tat FirstClass.TestMethod(FirstClass.java:2)")
+        assertThat(dumpedStr).contains("at FirstClass.TestMethod(FirstClass.java:2)")
-        assertThat(dumpedStr)
-                .doesNotContain("E Tag: \tat FirstClass.TestMethod(FirstClass.java:3)")
 
         assertThat(dumpedStr)
-                .contains("E Tag: Caused by: java.lang.RuntimeException: Cause of suppressed exp")
+                .contains("Caused by: java.lang.RuntimeException: Cause of suppressed exp")
-        assertThat(dumpedStr).contains("E Tag: \tat ThirdClass.TestMethod(ThirdClass.java:1)")
+        assertThat(dumpedStr).contains("at ThirdClass.TestMethod(ThirdClass.java:1)")
-        assertThat(dumpedStr).contains("E Tag: \tat ThirdClass.TestMethod(ThirdClass.java:2)")
+        assertThat(dumpedStr).contains("at ThirdClass.TestMethod(ThirdClass.java:2)")
-        assertThat(dumpedStr)
-                .doesNotContain("E Tag: \tat ThirdClass.TestMethod(ThirdClass.java:3)")
 
         // second suppressed exception
         assertThat(dumpedStr)
-                .contains("E Tag: Suppressed: " +
+                .contains("Suppressed: " +
                         "java.lang.RuntimeException: Second suppressed exception")
-        assertThat(dumpedStr).contains("E Tag: \tat SecondClass.TestMethod(SecondClass.java:1)")
+        assertThat(dumpedStr).contains("at SecondClass.TestMethod(SecondClass.java:1)")
-        assertThat(dumpedStr).contains("E Tag: \tat SecondClass.TestMethod(SecondClass.java:2)")
+        assertThat(dumpedStr).contains("at SecondClass.TestMethod(SecondClass.java:2)")
-        assertThat(dumpedStr)
-                .doesNotContain("E Tag: \tat SecondClass.TestMethod(SecondClass.java:3)")
     }
 
     private fun createTestException(
-        message: String,
+            message: String,
-        errorClass: String,
+            errorClass: String,
-        stackTraceLength: Int,
+            cause: Throwable? = null,
-        cause: Throwable? = null
     ): Exception {
         val exception = RuntimeException(message, cause)
-        exception.stackTrace = createStackTraceElements(errorClass, stackTraceLength)
+        exception.stackTrace = (1..5).map { lineNumber ->
+            StackTraceElement(errorClass,
+                    "TestMethod",
+                    "$errorClass.java",
+                    lineNumber)
+        }.toTypedArray()
         return exception
     }
 
@@ -152,16 +135,4 @@ class LogBufferTest : SysuiTestCase() {
         buffer.dump(PrintWriter(outputWriter), tailLength = 100)
         return outputWriter.toString()
     }
-
-    private fun createStackTraceElements(
-        errorClass: String,
-        stackTraceLength: Int
-    ): Array<StackTraceElement> {
-        return (1..stackTraceLength).map { lineNumber ->
-            StackTraceElement(errorClass,
-                    "TestMethod",
-                    "$errorClass.java",
-                    lineNumber)
-        }.toTypedArray()
-    }
 }