Highlighting new leaks (#1334)

New leaks are leaks that have a leak group that doesn't exist in any prior analysis.

* The notification now reports the count of leaks, with a split of new, known, and won't fix.
* The analysis success screen shows new leaks
* Moved description prefixes to be computed rather than inserted in the DB

Fixes #1328

leakcanary-android-core/src/main/java/leakcanary/DefaultAnalysisResultListener.kt

@@ -3,9 +3,12 @@ package leakcanary
 import android.app.Application
 import android.app.PendingIntent
 import com.squareup.leakcanary.core.R
+import leakcanary.Exclusion.Status.WEAKLY_REACHABLE
+import leakcanary.Exclusion.Status.WONT_FIX_LEAK
 import leakcanary.internal.LeakCanaryUtils
 import leakcanary.internal.activity.LeakActivity
 import leakcanary.internal.activity.db.HeapAnalysisTable
+import leakcanary.internal.activity.db.LeakingInstanceTable
 import leakcanary.internal.activity.db.LeaksDbHelper
 import leakcanary.internal.activity.screen.GroupListScreen
 import leakcanary.internal.activity.screen.HeapAnalysisFailureScreen
@@ -32,20 +35,38 @@ object DefaultAnalysisResultListener : (Application, HeapAnalysis) -> Unit {
       is HeapAnalysisSuccess -> heapAnalysis.copy(heapDumpFile = movedHeapDump)
     }
 
-    val id = LeaksDbHelper(application)
+    val (id, groupProjections) = LeaksDbHelper(application)
         .writableDatabase.use { db ->
-      HeapAnalysisTable.insert(db, updatedHeapAnalysis)
+      val id = HeapAnalysisTable.insert(db, updatedHeapAnalysis)
+      id to LeakingInstanceTable.retrieveAllByHeapAnalysisId(db, id)
     }
 
-    val contentTitle = when (heapAnalysis) {
-      is HeapAnalysisFailure -> application.getString(R.string.leak_canary_analysis_failed)
-      // TODO better text and res
-      is HeapAnalysisSuccess -> "Leak analysis done"
-    }
+    val (contentTitle, screenToShow) = when (heapAnalysis) {
+      is HeapAnalysisFailure -> application.getString(
+          R.string.leak_canary_analysis_failed
+      ) to HeapAnalysisFailureScreen(id)
+      is HeapAnalysisSuccess -> {
+        var leakCount = 0
+        var newLeakCount = 0
+        var knownLeakCount = 0
+        var wontFixLeakCount = 0
+
+        for ((_, projection) in groupProjections) {
+          if (projection.exclusionStatus != WEAKLY_REACHABLE) {
+            leakCount += projection.leakCount
+            when {
+              projection.exclusionStatus == WONT_FIX_LEAK -> wontFixLeakCount += projection.leakCount
+              projection.isNew -> newLeakCount += projection.leakCount
+              else -> knownLeakCount += projection.leakCount
+            }
+          }
+        }
 
-    val screenToShow = when (heapAnalysis) {
-      is HeapAnalysisFailure -> HeapAnalysisFailureScreen(id)
-      is HeapAnalysisSuccess -> HeapAnalysisSuccessScreen(id)
+        application.getString(
+            R.string.leak_canary_analysis_success_notification, leakCount, newLeakCount,
+            knownLeakCount, wontFixLeakCount
+        ) to HeapAnalysisSuccessScreen(id)
+      }
     }
 
     val pendingIntent = LeakActivity.createPendingIntent(

leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/HeapAnalysisTable.kt

@@ -10,7 +10,6 @@ import leakcanary.HeapAnalysisFailure
 import leakcanary.HeapAnalysisSuccess
 import leakcanary.Serializables
 import leakcanary.internal.LeakCanaryUtils
-import leakcanary.internal.activity.db.LeakingInstanceTable.HeapAnalysisGroupProjection
 import leakcanary.leakingInstances
 import leakcanary.toByteArray
 import org.intellij.lang.annotations.Language
@@ -21,7 +20,7 @@ internal object HeapAnalysisTable {
   @Language("RoomSql")
   const val create = """CREATE TABLE heap_analysis
         (
-        id INTEGER PRIMARY KEY,
+        id INTEGER PRIMARY KEY AUTOINCREMENT,
         created_at_time_millis INTEGER,
         retained_instance_count INTEGER DEFAULT 0,
         exception_summary TEXT DEFAULT NULL,
@@ -64,9 +63,9 @@ internal object HeapAnalysisTable {
   inline fun <reified T : HeapAnalysis> retrieve(
     db: SQLiteDatabase,
     id: Long
-  ): Pair<T, Map<String, HeapAnalysisGroupProjection>>? {
+  ): T? {
     db.inTransaction {
-      val heapAnalysis = db.rawQuery(
+      return db.rawQuery(
           """
               SELECT
               object
@@ -84,11 +83,6 @@ internal object HeapAnalysisTable {
             } else
               null
           } ?: return null
-
-      val hashes =
-        LeakingInstanceTable.retrieveAllByHeapAnalysisId(db, id)
-
-      return heapAnalysis to hashes
     }
   }
 

leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/LeakingInstanceTable.kt

@@ -2,7 +2,7 @@ package leakcanary.internal.activity.db
 
 import android.content.ContentValues
 import android.database.sqlite.SQLiteDatabase
-import leakcanary.Exclusion.Status.WEAKLY_REACHABLE
+import leakcanary.Exclusion
 import leakcanary.Exclusion.Status.WONT_FIX_LEAK
 import leakcanary.LeakTrace
 import leakcanary.LeakTraceElement
@@ -26,6 +26,7 @@ internal object LeakingInstanceTable {
         group_hash TEXT,
         group_description TEXT,
         class_simple_name TEXT,
+        exclusion_status INTEGER,
         object BLOB
         )"""
 
@@ -49,6 +50,7 @@ internal object LeakingInstanceTable {
     values.put("group_description", leakingInstance.createGroupDescription())
     values.put("class_simple_name", leakingInstance.instanceClassName.lastSegment('.'))
     values.put("object", leakingInstance.toByteArray())
+    values.put("exclusion_status", leakingInstance.exclusionStatus?.ordinal ?: -1)
     return db.insertOrThrow("leaking_instance", null, values)
   }
 
@@ -84,14 +86,22 @@ internal object LeakingInstanceTable {
     val description: String,
     val createdAtTimeMillis: Long,
     val leakCount: Int,
-    val totalLeakCount: Int
+    val totalLeakCount: Int,
+    val isNew: Boolean,
+    val exclusionStatus: Exclusion.Status?
   )
 
   fun retrieveAllByHeapAnalysisId(
     db: SQLiteDatabase,
     heapAnalysisId: Long
   ): Map<String, HeapAnalysisGroupProjection> {
 
+    val isLatestHeapAnalysis = db.rawQuery("SELECT MAX(id) FROM heap_analysis", null)
+        .use { cursor ->
+          cursor.moveToNext()
+          cursor.getLong(0) == heapAnalysisId
+        }
+
     return db.rawQuery(
         """
           SELECT
@@ -100,6 +110,7 @@ internal object LeakingInstanceTable {
           , MAX(created_at_time_millis) as created_at_time_millis
           , SUM(CASE WHEN heap_analysis_id=$heapAnalysisId THEN 1 ELSE 0 END) as leak_count
           , COUNT(*) as total_leak_count
+          , MIN(exclusion_status) as exclusion_status
           FROM leaking_instance l
           LEFT JOIN heap_analysis h ON l.heap_analysis_id = h.id
           GROUP BY 1, 2
@@ -111,12 +122,17 @@ internal object LeakingInstanceTable {
           val projectionsByHash = linkedMapOf<String, HeapAnalysisGroupProjection>()
           while (cursor.moveToNext()) {
             val hash = cursor.getString(0)
+            val description = cursor.getString(1)
+            val createdAtTimeMillis = cursor.getLong(2)
+            val leakCount = cursor.getInt(3)
+            val totalLeakCount = cursor.getInt(4)
+            val isNew = isLatestHeapAnalysis && leakCount == totalLeakCount
+            val exclusionStatusOrdinal = cursor.getInt(5)
+            val exclusionStatus =
+              if (exclusionStatusOrdinal == -1) null else Exclusion.Status.values()[exclusionStatusOrdinal]
             val group = HeapAnalysisGroupProjection(
-                hash = hash,
-                description = cursor.getString(1),
-                createdAtTimeMillis = cursor.getLong(2),
-                leakCount = cursor.getInt(3),
-                totalLeakCount = cursor.getInt(4)
+                hash, description, createdAtTimeMillis, leakCount, totalLeakCount, isNew,
+                exclusionStatus
             )
             projectionsByHash[hash] = group
           }
@@ -184,8 +200,6 @@ internal object LeakingInstanceTable {
     )
         .use { cursor ->
           if (cursor.moveToNext()) {
-            // TODO This may crash if we can't deserialize the first entry we find.
-            // We need to either prune early, or have a better deserialization story.
             val leakingInstance = Serializables.fromByteArray<LeakingInstance>(cursor.getBlob(0))!!
             val leakTrace = leakingInstance.leakTrace
 
@@ -262,16 +276,12 @@ internal object LeakingInstanceTable {
 
   private fun LeakingInstance.createGroupDescription(): String {
     return if (exclusionStatus == WONT_FIX_LEAK) {
-      "[Won't Fix] ${leakTrace.firstElementExclusion.matching}"
+      leakTrace.firstElementExclusion.matching
     } else {
       val element = leakTrace.leakCauses.first()
       val referenceName = element.reference!!.groupingName
       val refDescription = element.simpleClassName + "." + referenceName
-      if (exclusionStatus == WEAKLY_REACHABLE) {
-        "[Weakly Reachable] $refDescription"
-      } else {
-        refDescription
-      }
+      refDescription
     }
   }
 

leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/LeaksDbHelper.kt

@@ -26,6 +26,6 @@ internal class LeaksDbHelper(context: Context) : SQLiteOpenHelper(
 
   companion object {
     // Last updated for 2.0-alpha-2
-    private const val VERSION = 4
+    private const val VERSION = 6
   }
 }

leakcanary-android-core/src/main/java/leakcanary/internal/activity/screen/HeapAnalysisFailureScreen.kt

@@ -24,13 +24,12 @@ internal class HeapAnalysisFailureScreen(
     container.inflate(R.layout.leak_canary_heap_analysis_failure_screen).apply {
       activity.title = resources.getString(R.string.leak_canary_loading_title)
       executeOnDb {
-        val pair = HeapAnalysisTable.retrieve<HeapAnalysisFailure>(db, analysisId)
-        if (pair == null) {
+        val heapAnalysis = HeapAnalysisTable.retrieve<HeapAnalysisFailure>(db, analysisId)
+        if (heapAnalysis == null) {
           updateUi {
             activity.title = resources.getString(R.string.leak_canary_analysis_deleted_title)
           }
         } else {
-          val (heapAnalysis, _) = pair
           val heapDumpFileExist = heapAnalysis.heapDumpFile.exists()
           updateUi { onFailureRetrieved(heapAnalysis, heapDumpFileExist) }
         }

leakcanary-android-core/src/main/java/leakcanary/internal/activity/screen/HeapAnalysisSuccessScreen.kt

@@ -6,12 +6,16 @@ import android.view.ViewGroup
 import android.widget.ListView
 import android.widget.TextView
 import com.squareup.leakcanary.core.R
+import leakcanary.Exclusion
+import leakcanary.Exclusion.Status.WEAKLY_REACHABLE
+import leakcanary.Exclusion.Status.WONT_FIX_LEAK
 import leakcanary.HeapAnalysisSuccess
 import leakcanary.LeakingInstance
 import leakcanary.NoPathToInstance
 import leakcanary.WeakReferenceCleared
 import leakcanary.WeakReferenceMissing
 import leakcanary.internal.activity.db.HeapAnalysisTable
+import leakcanary.internal.activity.db.LeakingInstanceTable
 import leakcanary.internal.activity.db.LeakingInstanceTable.HeapAnalysisGroupProjection
 import leakcanary.internal.activity.db.executeOnDb
 import leakcanary.internal.activity.shareHeapDump
@@ -32,13 +36,14 @@ internal class HeapAnalysisSuccessScreen(
       activity.title = resources.getString(R.string.leak_canary_loading_title)
 
       executeOnDb {
-        val pair = HeapAnalysisTable.retrieve<HeapAnalysisSuccess>(db, analysisId)
-        if (pair == null) {
+        val heapAnalysis = HeapAnalysisTable.retrieve<HeapAnalysisSuccess>(db, analysisId)
+        if (heapAnalysis == null) {
           updateUi {
             activity.title = resources.getString(R.string.leak_canary_analysis_deleted_title)
           }
         } else {
-          val (heapAnalysis, leakGroupByHash) = pair
+          val leakGroupByHash =
+            LeakingInstanceTable.retrieveAllByHeapAnalysisId(db, analysisId)
           val heapDumpFileExist = heapAnalysis.heapDumpFile.exists()
           updateUi { onSuccessRetrieved(heapAnalysis, leakGroupByHash, heapDumpFileExist) }
         }
@@ -88,14 +93,12 @@ internal class HeapAnalysisSuccessScreen(
     var noPathToInstanceCount = 0
     var weakReferenceMissingCount = 0
     retainedInstances.forEach { retainedInstance ->
-      // if a leak, add to a map of groupSha -> (description, count, total count, time)
-      // => instead of list of shas we can get the list of projections that already exists
-      // TODO If the sha doesn't exist in the map then this is a removed group. Can't happen
-      // right now as we don't allow removing groups
       when (retainedInstance) {
         is LeakingInstance -> {
           if (leakGroupByHash[retainedInstance.groupHash] == null) {
-            TODO("Removing groups is not supported yet, this should not happen yet.")
+            throw IllegalStateException(
+                "Removing groups is not supported, this should never happen."
+            )
           }
         }
         is WeakReferenceCleared -> {
@@ -115,10 +118,29 @@ internal class HeapAnalysisSuccessScreen(
     val leakGroups = leakGroupByHash.values.toList()
 
     rowList.addAll(leakGroups.map { projection ->
-      val titleText = resources.getString(
-          R.string.leak_canary_heap_analysis_success_screen_row_title, projection.leakCount,
-          projection.totalLeakCount, projection.description
-      )
+      val description = when (projection.exclusionStatus) {
+        WONT_FIX_LEAK -> {
+          "[Won't Fix] ${projection.description}"
+        }
+        WEAKLY_REACHABLE -> {
+          "[Weakly Reachable] ${projection.description}"
+        }
+        else -> {
+          projection.description
+        }
+      }
+
+      val titleText = if (projection.isNew && projection.exclusionStatus == null) {
+        resources.getString(
+            R.string.leak_canary_heap_analysis_success_screen_row_title_new, projection.leakCount,
+            description
+        )
+      } else {
+        resources.getString(
+            R.string.leak_canary_heap_analysis_success_screen_row_title, projection.leakCount,
+            projection.totalLeakCount, description
+        )
+      }
       val timeText = resources.getString(
           R.string.leak_canary_heap_analysis_success_screen_row_time_format,
           DateUtils.formatDateTime(

leakcanary-android-core/src/main/res/values/leak_canary_strings.xml

@@ -22,6 +22,7 @@
   We welcome contributions from the community, please do not hesitate to
   <a href="https://github.com/square/leakcanary/issues">report an issue</a> or open a pull request!<br>]]></string>
   <string name="leak_canary_analysis_failed">Leak analysis failed</string>
+  <string name="leak_canary_analysis_success_notification">Analysis done: %1$d leaks (%2$d new, %3$d known, %4$d won\'t fix)</string>
   <string name="leak_canary_class_has_leaked">%1$s Leaked</string>
   <string name="leak_canary_download_dump">You can download the heap dump via \"Menu > Share Heap Dump\" or \"adb pull %1$s\"</string>
   <string name="leak_canary_loading_title">Loading…</string>
@@ -61,6 +62,7 @@
   <string name="leak_canary_go_to_heap_analysis">Go to Heap Analysis</string>
   <string name="leak_canary_heap_analysis_success_screen_title">Heap Analysis (%d Retained Instances)</string>
   <string name="leak_canary_heap_analysis_success_screen_row_title">(%1$d / %2$d Total) %3$s</string>
+  <string name="leak_canary_heap_analysis_success_screen_row_title_new">[NEW] (%1$d) %2$s</string>
   <string name="leak_canary_heap_analysis_success_screen_row_time_format">Latest: %s</string>
   <string name="leak_canary_heap_analysis_list_screen_title">All Analysis</string>
   <plurals name="leak_canary_group_screen_title">