Closes #4299 Add Mechanism to Keep Higher Priority Breadcrumbs if Max…

components/lib/crash/src/main/java/mozilla/components/lib/crash/Breadcrumb.kt

@@ -42,7 +42,7 @@ data class Breadcrumb(
      * Date of of the crash breadcrumb.
      */
     val date: Date = Date()
-) : Parcelable {
+) : Parcelable, Comparable<Breadcrumb> {
     /**
      * Crash breadcrumb priority level.
      */
@@ -97,4 +97,12 @@ data class Breadcrumb(
          */
         USER
     }
+
+    override fun compareTo(other: Breadcrumb): Int {
+        if (this.level.ordinal == other.level.ordinal) {
+            return this.date.compareTo(other.date)
+        }
+
+        return this.level.ordinal.compareTo(other.level.ordinal)
+    }
 }

components/lib/crash/src/main/java/mozilla/components/lib/crash/BreadcrumbPriorityQueue.kt

@@ -0,0 +1,29 @@
+package mozilla.components.lib.crash
+
+import java.util.PriorityQueue
+import kotlin.collections.ArrayList
+
+internal class BreadcrumbPriorityQueue(
+    private val maxSize: Int
+) : PriorityQueue<Breadcrumb>() {
+    @Synchronized
+    override fun add(element: Breadcrumb?): Boolean {
+        val result = super.add(element)
+        if (this.size > maxSize) {
+            this.poll()
+        }
+        return result
+    }
+
+    @Synchronized
+    fun toSortedArrayList(): ArrayList<Breadcrumb> {
+        val breadcrumbsArrayList: ArrayList<Breadcrumb> = arrayListOf()
+        if (isNotEmpty()) {
+            breadcrumbsArrayList.addAll(this)
+
+            /* Sort by timestamp before reporting */
+            breadcrumbsArrayList.sortBy { it.date }
+        }
+        return breadcrumbsArrayList
+    }
+}

components/lib/crash/src/main/java/mozilla/components/lib/crash/CrashReporter.kt

@@ -49,7 +49,7 @@ class CrashReporter(
     private val nonFatalCrashIntent: PendingIntent? = null
 ) {
     internal val logger = Logger("mozac/CrashReporter")
-    internal val crashBreadcrumbs = arrayListOf<Breadcrumb>()
+    internal val crashBreadcrumbs = BreadcrumbPriorityQueue(BREADCRUMB_MAX_NUM)
 
     init {
         if (services.isEmpty()) {
@@ -96,9 +96,6 @@ class CrashReporter(
      * ```
      */
     fun recordCrashBreadcrumb(breadcrumb: Breadcrumb) {
-        if (crashBreadcrumbs.size >= BREADCRUMB_MAX_NUM) {
-            crashBreadcrumbs.removeAt(0)
-        }
         crashBreadcrumbs.add(breadcrumb)
     }
 

components/lib/crash/src/main/java/mozilla/components/lib/crash/handler/ExceptionHandler.kt

@@ -26,9 +26,8 @@ class ExceptionHandler(
 
         try {
             crashing = true
-
             crashReporter.onCrash(context, Crash.UncaughtExceptionCrash(throwable,
-                    crashReporter.crashBreadcrumbs))
+                    crashReporter.crashBreadcrumbs.toSortedArrayList()))
 
             defaultExceptionHandler?.uncaughtException(thread, throwable)
         } finally {

components/lib/crash/src/test/java/mozilla/components/lib/crash/BreadcrumbPriorityQueueTest.kt

@@ -0,0 +1,121 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.lib.crash
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.lang.Thread.sleep
+
+@RunWith(AndroidJUnit4::class)
+class BreadcrumbPriorityQueueTest {
+
+    @Test
+    fun `Breadcrumb priority queue stores only max number of breadcrumbs`() {
+        val testMessage = "test_Message"
+        val testData = hashMapOf("1" to "one", "2" to "two")
+        val testCategory = "testing_category"
+        val testLevel = Breadcrumb.Level.CRITICAL
+        val testType = Breadcrumb.Type.USER
+        var crashBreadcrumbs = BreadcrumbPriorityQueue(5)
+        repeat(10) {
+            crashBreadcrumbs.add(Breadcrumb(testMessage, testData, testCategory, testLevel, testType))
+        }
+        assertEquals(crashBreadcrumbs.size, 5)
+
+        crashBreadcrumbs = BreadcrumbPriorityQueue(10)
+        repeat(15) {
+            crashBreadcrumbs.add(Breadcrumb(testMessage, testData, testCategory, testLevel, testType))
+        }
+        assertEquals(crashBreadcrumbs.size, 10)
+    }
+
+    @Test
+    fun `Breadcrumb priority queue stores the correct priority`() {
+        val testMessage = "test_Message"
+        val testData = hashMapOf("1" to "one", "2" to "two")
+        val testCategory = "testing_category"
+        val testType = Breadcrumb.Type.USER
+        val maxNum = 5
+        var crashBreadcrumbs = BreadcrumbPriorityQueue(maxNum)
+
+        repeat(maxNum) {
+            crashBreadcrumbs.add(
+                    Breadcrumb(testMessage, testData, testCategory, Breadcrumb.Level.DEBUG, testType)
+            )
+        }
+        assertEquals(crashBreadcrumbs.size, maxNum)
+
+        repeat(maxNum) {
+            crashBreadcrumbs.add(
+                    Breadcrumb(testMessage, testData, testCategory, Breadcrumb.Level.INFO, testType)
+            )
+        }
+
+        for (i in 0 until maxNum) {
+            assertEquals(crashBreadcrumbs.elementAt(i).level, Breadcrumb.Level.INFO)
+        }
+
+        repeat(maxNum) {
+            crashBreadcrumbs.add(
+                    Breadcrumb(testMessage, testData, testCategory, Breadcrumb.Level.DEBUG, testType)
+            )
+        }
+
+        for (i in 0 until maxNum) {
+            assertEquals(crashBreadcrumbs.elementAt(i).level, Breadcrumb.Level.INFO)
+        }
+
+        repeat(maxNum) {
+            crashBreadcrumbs.add(
+                    Breadcrumb(testMessage, testData, testCategory, Breadcrumb.Level.WARNING, testType)
+            )
+        }
+
+        for (i in 0 until maxNum) {
+            assertEquals(crashBreadcrumbs.elementAt(i).level, Breadcrumb.Level.WARNING)
+        }
+    }
+
+    @Test
+    fun `Breadcrumb priority queue output list result is sorted by time`() {
+        val testMessage = "test_Message"
+        val testData = hashMapOf("1" to "one", "2" to "two")
+        val testCategory = "testing_category"
+        val testType = Breadcrumb.Type.USER
+        val maxNum = 10
+        var crashBreadcrumbs = BreadcrumbPriorityQueue(maxNum)
+
+        repeat(maxNum) {
+            crashBreadcrumbs.add(
+                    Breadcrumb(testMessage, testData, testCategory, Breadcrumb.Level.DEBUG, testType)
+            )
+            sleep(100) /* make sure time elapsed */
+        }
+
+        var result = crashBreadcrumbs.toSortedArrayList()
+        var time = result[0].date
+        for (i in 1 until result.size) {
+            assertTrue(time.before(result[i].date))
+            time = result[i].date
+        }
+
+        repeat(maxNum / 2) {
+            crashBreadcrumbs.add(
+                    Breadcrumb(testMessage, testData, testCategory, Breadcrumb.Level.INFO, testType)
+            )
+            sleep(100) /* make sure time elapsed */
+        }
+
+        result = crashBreadcrumbs.toSortedArrayList()
+        time = result[0].date
+        for (i in 1 until result.size) {
+            assertTrue(time.before(result[i].date))
+            time = result[i].date
+        }
+    }
+}

components/lib/crash/src/test/java/mozilla/components/lib/crash/BreadcrumbTest.kt

@@ -5,7 +5,7 @@
 package mozilla.components.lib.crash
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import mozilla.components.lib.crash.service.CrashReporterService
+import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
@@ -33,29 +33,21 @@ class BreadcrumbTest {
         val testLevel = Breadcrumb.Level.CRITICAL
         val testType = Breadcrumb.Type.USER
 
-        val service = object : CrashReporterService {
-            override fun report(crash: Crash.UncaughtExceptionCrash) {
-            }
-
-            override fun report(crash: Crash.NativeCodeCrash) {
-            }
-        }
-
         val reporter = spy(CrashReporter(
-                services = listOf(service),
+                services = listOf(mock()),
                 shouldPrompt = CrashReporter.Prompt.NEVER
         ).install(testContext))
 
         reporter.recordCrashBreadcrumb(
                 Breadcrumb(testMessage, testData, testCategory, testLevel, testType)
         )
 
-        assertEquals(reporter.crashBreadcrumbs[0].message, testMessage)
-        assertEquals(reporter.crashBreadcrumbs[0].data, testData)
-        assertEquals(reporter.crashBreadcrumbs[0].category, testCategory)
-        assertEquals(reporter.crashBreadcrumbs[0].level, testLevel)
-        assertEquals(reporter.crashBreadcrumbs[0].type, testType)
-        assertNotNull(reporter.crashBreadcrumbs[0].date)
+        assertEquals(reporter.crashBreadcrumbs.elementAt(0).message, testMessage)
+        assertEquals(reporter.crashBreadcrumbs.elementAt(0).data, testData)
+        assertEquals(reporter.crashBreadcrumbs.elementAt(0).category, testCategory)
+        assertEquals(reporter.crashBreadcrumbs.elementAt(0).level, testLevel)
+        assertEquals(reporter.crashBreadcrumbs.elementAt(0).type, testType)
+        assertNotNull(reporter.crashBreadcrumbs.elementAt(0).date)
     }
 
     @Test
@@ -66,16 +58,8 @@ class BreadcrumbTest {
         val testLevel = Breadcrumb.Level.CRITICAL
         val testType = Breadcrumb.Type.USER
 
-        val service = object : CrashReporterService {
-            override fun report(crash: Crash.UncaughtExceptionCrash) {
-            }
-
-            override fun report(crash: Crash.NativeCodeCrash) {
-            }
-        }
-
         val reporter = spy(CrashReporter(
-                services = listOf(service),
+                services = listOf(mock()),
                 shouldPrompt = CrashReporter.Prompt.NEVER
         ).install(testContext))
 
@@ -95,45 +79,6 @@ class BreadcrumbTest {
         assertEquals(reporter.crashBreadcrumbs.size, 3)
     }
 
-    @Test
-    fun `Reporter stores only max number of breadcrumbs`() {
-        val testMessage = "test_Message"
-        val testData = hashMapOf("1" to "one", "2" to "two")
-        val testCategory = "testing_category"
-        val testLevel = Breadcrumb.Level.CRITICAL
-        val testType = Breadcrumb.Type.USER
-
-        val service = object : CrashReporterService {
-            override fun report(crash: Crash.UncaughtExceptionCrash) {
-            }
-
-            override fun report(crash: Crash.NativeCodeCrash) {
-            }
-        }
-
-        val reporter = spy(CrashReporter(
-                services = listOf(service),
-                shouldPrompt = CrashReporter.Prompt.NEVER
-        ).install(testContext))
-
-        repeat(CrashReporter.BREADCRUMB_MAX_NUM) {
-            reporter.recordCrashBreadcrumb(
-                    Breadcrumb(testMessage, testData, testCategory, testLevel, testType)
-            )
-        }
-        assertEquals(reporter.crashBreadcrumbs.size, CrashReporter.BREADCRUMB_MAX_NUM)
-
-        reporter.recordCrashBreadcrumb(
-                Breadcrumb(testMessage, testData, testCategory, testLevel, testType)
-        )
-        assertEquals(reporter.crashBreadcrumbs.size, CrashReporter.BREADCRUMB_MAX_NUM)
-
-        reporter.recordCrashBreadcrumb(
-                Breadcrumb(testMessage, testData, testCategory, testLevel, testType)
-        )
-        assertEquals(reporter.crashBreadcrumbs.size, CrashReporter.BREADCRUMB_MAX_NUM)
-    }
-
     @Test
     fun `RecordBreadCrumb stores correct date`() {
         val testMessage = "test_Message"
@@ -142,16 +87,8 @@ class BreadcrumbTest {
         val testLevel = Breadcrumb.Level.CRITICAL
         val testType = Breadcrumb.Type.USER
 
-        val service = object : CrashReporterService {
-            override fun report(crash: Crash.UncaughtExceptionCrash) {
-            }
-
-            override fun report(crash: Crash.NativeCodeCrash) {
-            }
-        }
-
         val reporter = spy(CrashReporter(
-                services = listOf(service),
+                services = listOf(mock()),
                 shouldPrompt = CrashReporter.Prompt.NEVER
         ).install(testContext))
 
@@ -163,13 +100,13 @@ class BreadcrumbTest {
         sleep(100) /* make sure time elapsed */
         val afterDate = Date()
 
-        assertTrue(reporter.crashBreadcrumbs[0].date.after(beginDate))
-        assertTrue(reporter.crashBreadcrumbs[0].date.before(afterDate))
+        assertTrue(reporter.crashBreadcrumbs.elementAt(0).date.after(beginDate))
+        assertTrue(reporter.crashBreadcrumbs.elementAt(0).date.before(afterDate))
 
         val date = Date()
         reporter.recordCrashBreadcrumb(
                 Breadcrumb(testMessage, testData, testCategory, testLevel, testType, date)
         )
-        assertTrue(reporter.crashBreadcrumbs[1].date.compareTo(date) == 0)
+        assertTrue(reporter.crashBreadcrumbs.elementAt(1).date.compareTo(date) == 0)
     }
 }

components/lib/crash/src/test/java/mozilla/components/lib/crash/handler/ExceptionHandlerTest.kt

@@ -62,9 +62,20 @@ class ExceptionHandlerTest {
     fun `ExceptionHandler invokes default exception handler`() {
         val defaultExceptionHandler: Thread.UncaughtExceptionHandler = mock()
 
+        val crashReporter = CrashReporter(
+                shouldPrompt = CrashReporter.Prompt.NEVER,
+                services = listOf(object : CrashReporterService {
+                    override fun report(crash: Crash.UncaughtExceptionCrash) {
+                    }
+
+                    override fun report(crash: Crash.NativeCodeCrash) {
+                    }
+                })
+        ).install(testContext)
+
         val handler = ExceptionHandler(
             testContext,
-            mock(),
+            crashReporter,
             defaultExceptionHandler
         )
 

components/lib/crash/src/test/java/mozilla/components/lib/crash/service/SentryServiceTest.kt

@@ -185,12 +185,14 @@ class SentryServiceTest {
         reporter.recordCrashBreadcrumb(
                 Breadcrumb(testMessage, testData, testCategory, testLevel, testType)
         )
+        var crashBreadCrumbs = arrayListOf<Breadcrumb>()
+        crashBreadCrumbs.addAll(reporter.crashBreadcrumbs)
         val nativeCrash = Crash.NativeCodeCrash(
                 "dump.path",
                 true,
                 "extras.path",
                 isFatal = false,
-                breadcrumbs = reporter.crashBreadcrumbs)
+                breadcrumbs = crashBreadCrumbs)
 
         service.report(nativeCrash)
         verify(clientContext).recordBreadcrumb(any())