Fix formatting and make DumpHeapResult a sealed class

leakcanary-android-core/src/main/java/leakcanary/internal/AndroidHeapDumper.kt

@@ -41,15 +41,15 @@ internal class AndroidHeapDumper(
   private val context: Context = context.applicationContext
   private val mainHandler: Handler = Handler(Looper.getMainLooper())
 
-  override fun dumpHeap(): DumpHeapResult? {
-    val heapDumpFile = leakDirectoryProvider.newHeapDumpFile() ?: return null
+  override fun dumpHeap(): DumpHeapResult {
+    val heapDumpFile = leakDirectoryProvider.newHeapDumpFile() ?: return NoHeapDump
 
     val waitingForToast = FutureResult<Toast?>()
     showToast(waitingForToast)
 
     if (!waitingForToast.wait(5, SECONDS)) {
       SharkLog.d { "Did not dump heap, too much time waiting for Toast." }
-      return null
+      return NoHeapDump
     }
 
     val notificationManager =
@@ -70,14 +70,14 @@ internal class AndroidHeapDumper(
       }
       if (heapDumpFile.length() == 0L) {
         SharkLog.d { "Dumped heap file is 0 byte length" }
-        null
+        NoHeapDump
       } else {
-        DumpHeapResult(file = heapDumpFile, durationMillis = durationMillis)
+        HeapDump(file = heapDumpFile, durationMillis = durationMillis)
       }
     } catch (e: Exception) {
       SharkLog.d(e) { "Could not dump heap" }
       // Abort heap dump
-      null
+      NoHeapDump
     } finally {
       cancelToast(toast)
       notificationManager.cancel(R.id.leak_canary_notification_dumping_heap)

leakcanary-android-core/src/main/java/leakcanary/internal/HeapAnalyzerService.kt

@@ -25,6 +25,7 @@ import leakcanary.LeakCanary.Config
 import shark.HeapAnalysis
 import shark.HeapAnalysisException
 import shark.HeapAnalysisFailure
+import shark.HeapAnalysisSuccess
 import shark.HeapAnalyzer
 import shark.OnAnalysisProgressListener
 import shark.OnAnalysisProgressListener.Step.REPORTING_HEAP_ANALYSIS
@@ -52,21 +53,24 @@ internal class HeapAnalyzerService : ForegroundService(
     // Since we're running in the main process we should be careful not to impact it.
     Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND)
     val heapDumpFile = intent.getSerializableExtra(HEAPDUMP_FILE_EXTRA) as File
-    val heapDumpDurationMills = intent.getLongExtra(HEAPDUMP_DURATION, -1)
+    val heapDumpDurationMillis = intent.getLongExtra(HEAPDUMP_DURATION_MILLIS, -1)
 
     val config = LeakCanary.config
     val heapAnalysis = if (heapDumpFile.exists()) {
-      analyzeHeap(heapDumpFile, heapDumpDurationMills, config)
+      analyzeHeap(heapDumpFile, config)
     } else {
-      missingFileFailure(heapDumpFile, heapDumpDurationMills)
+      missingFileFailure(heapDumpFile)
+    }
+    val fullHeapAnalysis = when (heapAnalysis) {
+      is HeapAnalysisSuccess -> heapAnalysis.copy(dumpDurationMillis = heapDumpDurationMillis)
+      is HeapAnalysisFailure -> heapAnalysis.copy(dumpDurationMillis = heapDumpDurationMillis)
     }
     onAnalysisProgress(REPORTING_HEAP_ANALYSIS)
-    config.onHeapAnalyzedListener.onHeapAnalyzed(heapAnalysis)
+    config.onHeapAnalyzedListener.onHeapAnalyzed(fullHeapAnalysis)
   }
 
   private fun analyzeHeap(
     heapDumpFile: File,
-    heapDumpDurationMillis: Long,
     config: Config
   ): HeapAnalysis {
     val heapAnalyzer = HeapAnalyzer(this)
@@ -83,14 +87,12 @@ internal class HeapAnalyzerService : ForegroundService(
         computeRetainedHeapSize = config.computeRetainedHeapSize,
         objectInspectors = config.objectInspectors,
         metadataExtractor = config.metadataExtractor,
-        proguardMapping = proguardMappingReader?.readProguardMapping(),
-        heapDumpDurationMillis = heapDumpDurationMillis
+        proguardMapping = proguardMappingReader?.readProguardMapping()
     )
   }
 
   private fun missingFileFailure(
-      heapDumpFile: File,
-      heapDumpDurationMillis: Long
+      heapDumpFile: File
   ): HeapAnalysisFailure {
     val deletedReason = LeakDirectoryProvider.hprofDeleteReason(heapDumpFile)
     val exception = IllegalStateException(
@@ -99,7 +101,6 @@ internal class HeapAnalyzerService : ForegroundService(
     return HeapAnalysisFailure(
         heapDumpFile = heapDumpFile,
         createdAtTimeMillis = System.currentTimeMillis(),
-        dumpDurationMillis = heapDumpDurationMillis,
         analysisDurationMillis = 0,
         exception = HeapAnalysisException(exception)
     )
@@ -117,18 +118,18 @@ internal class HeapAnalyzerService : ForegroundService(
 
   companion object {
     private const val HEAPDUMP_FILE_EXTRA = "HEAPDUMP_FILE_EXTRA"
-    private const val HEAPDUMP_DURATION = "HEAPDUMP_DURATION"
+    private const val HEAPDUMP_DURATION_MILLIS = "HEAPDUMP_DURATION_MILLIS"
     private const val PROGUARD_MAPPING_FILE_NAME = "leakCanaryObfuscationMapping.txt"
 
     fun runAnalysis(
       context: Context,
       heapDumpFile: File,
-      heapDumpDurationMills: Long? = null
+      heapDumpDurationMillis: Long? = null
     ) {
       val intent = Intent(context, HeapAnalyzerService::class.java)
       intent.putExtra(HEAPDUMP_FILE_EXTRA, heapDumpFile)
-      heapDumpDurationMills?.let {
-        intent.putExtra(HEAPDUMP_DURATION, heapDumpDurationMills)
+      heapDumpDurationMillis?.let {
+        intent.putExtra(HEAPDUMP_DURATION_MILLIS, heapDumpDurationMillis)
       }
       startForegroundService(context, intent)
     }

leakcanary-android-core/src/main/java/leakcanary/internal/HeapDumpTrigger.kt

@@ -156,32 +156,34 @@ internal class HeapDumpTrigger(
     saveResourceIdNamesToMemory()
     val heapDumpUptimeMillis = SystemClock.uptimeMillis()
     KeyedWeakReference.heapDumpUptimeMillis = heapDumpUptimeMillis
-    val heapDumpResult = heapDumper.dumpHeap()
-    if (heapDumpResult == null) {
-      if (retry) {
-        SharkLog.d { "Failed to dump heap, will retry in $WAIT_AFTER_DUMP_FAILED_MILLIS ms" }
-        scheduleRetainedObjectCheck(
-            delayMillis = WAIT_AFTER_DUMP_FAILED_MILLIS
+    when (val heapDumpResult = heapDumper.dumpHeap()) {
+      is NoHeapDump -> {
+        if (retry) {
+          SharkLog.d { "Failed to dump heap, will retry in $WAIT_AFTER_DUMP_FAILED_MILLIS ms" }
+          scheduleRetainedObjectCheck(
+              delayMillis = WAIT_AFTER_DUMP_FAILED_MILLIS
+          )
+        } else {
+          SharkLog.d { "Failed to dump heap, will not automatically retry" }
+        }
+        showRetainedCountNotification(
+            objectCount = retainedReferenceCount,
+            contentText = application.getString(
+                R.string.leak_canary_notification_retained_dump_failed
+            )
+        )
+      }
+      is HeapDump -> {
+        lastDisplayedRetainedObjectCount = 0
+        lastHeapDumpUptimeMillis = SystemClock.uptimeMillis()
+        objectWatcher.clearObjectsWatchedBefore(heapDumpUptimeMillis)
+        HeapAnalyzerService.runAnalysis(
+            context = application,
+            heapDumpFile = heapDumpResult.file,
+            heapDumpDurationMillis = heapDumpResult.durationMillis
         )
-      } else {
-        SharkLog.d { "Failed to dump heap, will not automatically retry" }
       }
-      showRetainedCountNotification(
-          objectCount = retainedReferenceCount,
-          contentText = application.getString(
-              R.string.leak_canary_notification_retained_dump_failed
-          )
-      )
-      return
     }
-    lastDisplayedRetainedObjectCount = 0
-    lastHeapDumpUptimeMillis = SystemClock.uptimeMillis()
-    objectWatcher.clearObjectsWatchedBefore(heapDumpUptimeMillis)
-    HeapAnalyzerService.runAnalysis(
-        context = application,
-        heapDumpFile = heapDumpResult.file,
-        heapDumpDurationMills = heapDumpResult.durationMillis
-    )
   }
 
   private fun saveResourceIdNamesToMemory() {

leakcanary-android-core/src/main/java/leakcanary/internal/HeapDumper.kt

@@ -24,12 +24,13 @@ internal interface HeapDumper {
    * @return a [File] referencing the dumped heap, or [.RETRY_LATER] if the heap could
    * not be dumped.
    */
-  fun dumpHeap(): DumpHeapResult?
+  fun dumpHeap(): DumpHeapResult
 
 }
 
 /** Dump heap result holding the file and the dump heap duration */
-data class DumpHeapResult(
-    val file: File,
-    val durationMillis: Long
-)
+internal sealed class DumpHeapResult
+
+internal data class HeapDump(val file: File, val durationMillis: Long): DumpHeapResult()
+internal object NoHeapDump: DumpHeapResult()
+

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

@@ -104,7 +104,7 @@ internal class HeapDumpScreen(
           val dumpDurationMillis = if (heapAnalysis.dumpDurationMillis > -1) {
             "${heapAnalysis.dumpDurationMillis} ms"
           } else {
-            "UNKNOWN"
+            "Unknown"
           }
           val titleText = explore + shareAnalysis + shareFile +
               (heapAnalysis.metadata + mapOf(

leakcanary-android-instrumentation/src/main/java/leakcanary/InstrumentationLeakDetector.kt

@@ -25,6 +25,7 @@ import org.junit.runner.notification.RunListener
 import shark.HeapAnalysis
 import shark.HeapAnalysisException
 import shark.HeapAnalysisFailure
+import shark.HeapAnalysisSuccess
 import shark.HeapAnalyzer
 import shark.SharkLog
 import java.io.File
@@ -156,7 +157,7 @@ class InstrumentationLeakDetector {
           HeapAnalysisFailure(
               heapDumpFile = heapDumpFile,
               createdAtTimeMillis = System.currentTimeMillis(),
-              dumpDurationMillis = -1,
+              dumpDurationMillis = HeapAnalysis.DUMP_DURATION_UNKNOWN,
               analysisDurationMillis = 0,
               exception = HeapAnalysisException(exception)
           )
@@ -172,15 +173,17 @@ class InstrumentationLeakDetector {
     SystemClock.sleep(2000)
 
     val heapAnalyzer = HeapAnalyzer(listener)
-    val heapAnalysis = heapAnalyzer.analyze(
+    val fullHeapAnalysis = when (val heapAnalysis = heapAnalyzer.analyze(
         heapDumpFile = heapDumpFile,
-        heapDumpDurationMillis = heapDumpDurationMillis,
         leakingObjectFinder = config.leakingObjectFinder,
         referenceMatchers = config.referenceMatchers,
         computeRetainedHeapSize = config.computeRetainedHeapSize,
         objectInspectors = config.objectInspectors
-    )
-    return AnalysisPerformed(heapAnalysis)
+    )) {
+      is HeapAnalysisSuccess -> heapAnalysis.copy(dumpDurationMillis = heapDumpDurationMillis)
+      is HeapAnalysisFailure -> heapAnalysis.copy(dumpDurationMillis = heapDumpDurationMillis)
+    }
+    return AnalysisPerformed(fullHeapAnalysis)
   }
 
   companion object {

shark/src/main/java/shark/HeapAnalysis.kt

@@ -32,21 +32,22 @@ sealed class HeapAnalysis : Serializable {
 
   companion object {
     private const val serialVersionUID: Long = -8657286725869987172
+    const val DUMP_DURATION_UNKNOWN: Long = -1
   }
 }
 
 /**
  * The analysis performed by [HeapAnalyzer] did not complete successfully.
  */
 data class HeapAnalysisFailure(
-    override val heapDumpFile: File,
-    override val createdAtTimeMillis: Long,
-    override val dumpDurationMillis: Long,
-    override val analysisDurationMillis: Long,
-    /**
-     * An exception wrapping the actual exception that was thrown.
-     */
-    val exception: HeapAnalysisException
+  override val heapDumpFile: File,
+  override val createdAtTimeMillis: Long,
+  override val dumpDurationMillis: Long = DUMP_DURATION_UNKNOWN,
+  override val analysisDurationMillis: Long,
+  /**
+   * An exception wrapping the actual exception that was thrown.
+   */
+  val exception: HeapAnalysisException
 ) : HeapAnalysis() {
 
   override fun toString(): String {
@@ -79,16 +80,16 @@ Heap dump timestamp: $createdAtTimeMillis
  * The result of a successful heap analysis performed by [HeapAnalyzer].
  */
 data class HeapAnalysisSuccess(
-    override val heapDumpFile: File,
-    override val createdAtTimeMillis: Long,
-    override val dumpDurationMillis: Long,
-    override val analysisDurationMillis: Long,
-    val metadata: Map<String, String>,
-    /**
+  override val heapDumpFile: File,
+  override val createdAtTimeMillis: Long,
+  override val dumpDurationMillis: Long = DUMP_DURATION_UNKNOWN,
+  override val analysisDurationMillis: Long,
+  val metadata: Map<String, String>,
+  /**
    * The list of [ApplicationLeak] found in the heap dump by [HeapAnalyzer].
    */
   val applicationLeaks: List<ApplicationLeak>,
-    /**
+  /**
    * The list of [LibraryLeak] found in the heap dump by [HeapAnalyzer].
    */
   val libraryLeaks: List<LibraryLeak>
@@ -127,7 +128,7 @@ ${if (metadata.isNotEmpty()) "\n" + metadata.map { "${it.key}: ${it.value}" }.jo
 Analysis duration: $analysisDurationMillis ms
 Heap dump file path: ${heapDumpFile.absolutePath}
 Heap dump timestamp: $createdAtTimeMillis
-Heap dump duration: ${if (dumpDurationMillis > -1) "$dumpDurationMillis ms" else "UNKNOWN"}
+Heap dump duration: ${if (dumpDurationMillis != DUMP_DURATION_UNKNOWN) "$dumpDurationMillis ms" else "Unknown"}
 ===================================="""
   }
 
@@ -162,7 +163,6 @@ Heap dump duration: ${if (dumpDurationMillis > -1) "$dumpDurationMillis ms" else
       return HeapAnalysisSuccess(
           heapDumpFile = fromV20.heapDumpFile,
           createdAtTimeMillis = fromV20.createdAtTimeMillis,
-          dumpDurationMillis = fromV20.dumpDurationMillis,
           analysisDurationMillis = fromV20.analysisDurationMillis,
           metadata = fromV20.metadata,
           applicationLeaks = applicationLeaks,
@@ -247,6 +247,7 @@ ${super.toString()}
 
   /** This field is kept to support backward compatible deserialization. */
   private val leakTrace: LeakTrace? = null
+
   /** This field is kept to support backward compatible deserialization. */
   private val retainedHeapByteSize: Int? = null
 
@@ -282,6 +283,7 @@ data class ApplicationLeak(
 
   /** This field is kept to support backward compatible deserialization. */
   private val leakTrace: LeakTrace? = null
+
   /** This field is kept to support backward compatible deserialization. */
   private val retainedHeapByteSize: Int? = null