AndroidX Fragment leak status from lifecycle

Fixes #2565

leakcanary-android-instrumentation/src/androidTest/java/leakcanary/LifecycleLeaksTest.kt

@@ -8,10 +8,12 @@ import androidx.fragment.app.Fragment
 import androidx.lifecycle.ViewModel
 import com.squareup.leakcanary.instrumentation.test.R
 import leakcanary.TestUtils.assertLeak
+import org.assertj.core.api.Assertions.assertThat
 import org.junit.After
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
+import shark.LeakTraceObject.LeakingStatus
 
 class LifecycleLeaksTest : HasActivityTestRule<TestActivity> {
 
@@ -27,6 +29,10 @@ class LifecycleLeaksTest : HasActivityTestRule<TestActivity> {
  }
  }
 
+ class FragmentHoldingLeaky : Fragment() {
+ val leaky = Any()
+ }
+
  @get:Rule
  override val activityRule = activityTestRule<TestActivity>(
  initialTouchMode = false,
@@ -105,7 +111,46 @@ class LifecycleLeaksTest : HasActivityTestRule<TestActivity> {
  }
 
  fragment retained {
- assertLeak(Fragment::class.java)
+ val expectedLeakClass = Fragment::class.java
+ assertLeak { (heapAnalysis, leakTrace) ->
+ val className = leakTrace.leakingObject.className
+ assertThat(className)
+ .describedAs("$heapAnalysis")
+ .isEqualTo(expectedLeakClass.name)
+ assertThat(leakTrace.leakingObject.leakingStatusReason)
+ .describedAs("$heapAnalysis")
+ .contains("Fragment.mLifecycleRegistry.state is DESTROYED")
+ }
+ }
+ }
+
+ @Test
+ fun fragmentNotLeakingDetected() {
+ triggersOnActivityCreated {
+ activityRule.launchActivity(null)
+ }
+
+ getOnMainSync {
+ val fragment = FragmentHoldingLeaky()
+ activity.addFragmentNow(fragment)
+ AppWatcher.objectWatcher.expectWeaklyReachable(fragment.leaky, "leaky leaks")
+ }
+
+ assertLeak { (heapAnalysis, leakTrace) ->
+ val refToLeaky = leakTrace.referencePath.last()
+ assertThat(refToLeaky.referenceName)
+ .describedAs("$heapAnalysis")
+ .isEqualTo("leaky")
+ val fragment = refToLeaky.originObject
+ // AssertJ uses lambdas when comparing enum values, which fails on older Android versions.
+ if (fragment.leakingStatus != LeakingStatus.NOT_LEAKING) {
+ throw AssertionError(
+ "${fragment.leakingStatus} should be ${LeakingStatus.NOT_LEAKING}"
+ )
+ }
+ assertThat(fragment.leakingStatusReason).isEqualTo(
+ "Fragment.mLifecycleRegistry.state is RESUMED"
+ )
  }
  }
 

leakcanary-android-instrumentation/src/androidTest/java/leakcanary/ObjectInspectorTest.kt

@@ -41,7 +41,7 @@ class ObjectInspectorTest : HasActivityTestRule<TestActivity> {
  val leaktrace = heapAnalysis.allLeaks.single().leakTraces.single()
  val ref = leaktrace.referencePath.single { it.owningClassSimpleName == "LifecycleRegistry" }
  val lifecycleRegistry = ref.originObject
- assertThat(lifecycleRegistry.labels.single()).isEqualTo("mState = DESTROYED")
+ assertThat(lifecycleRegistry.labels.single()).isEqualTo("state = DESTROYED")
  }
  }
 }

leakcanary-android-instrumentation/src/androidTest/java/leakcanary/TestUtils.kt

@@ -2,10 +2,22 @@ package leakcanary
 
 import shark.HeapAnalysis
 import shark.HeapAnalysisSuccess
+import shark.LeakTrace
 
 object TestUtils {
 
  fun assertLeak(expectedLeakClass: Class<*>) {
+ assertLeak { (heapAnalysis, leakTrace) ->
+ val className = leakTrace.leakingObject.className
+ if (className != expectedLeakClass.name) {
+ throw AssertionError(
+ "Expected a leak of $expectedLeakClass, not $className in $heapAnalysis"
+ )
+ }
+ }
+ }
+
+ fun assertLeak(inspectLeakTrace: (Pair<HeapAnalysisSuccess, LeakTrace>) -> Unit = {}) {
  val heapAnalysis = detectLeaks()
  val applicationLeaks = heapAnalysis.applicationLeaks
  if (applicationLeaks.size != 1) {
@@ -17,12 +29,7 @@ object TestUtils {
  val leak = applicationLeaks.first()
 
  val leakTrace = leak.leakTraces.first()
- val className = leakTrace.leakingObject.className
- if (className != expectedLeakClass.name) {
- throw AssertionError(
- "Expected a leak of $expectedLeakClass, not $className in $heapAnalysis"
- )
- }
+ inspectLeakTrace(heapAnalysis to leakTrace)
  }
 
  fun detectLeaks(): HeapAnalysisSuccess {

shark-android/src/main/java/shark/AndroidObjectInspectors.kt

@@ -15,12 +15,12 @@
  */
 package shark
 
+import java.util.EnumSet
+import kotlin.math.absoluteValue
 import shark.AndroidObjectInspectors.Companion.appDefaults
 import shark.AndroidServices.aliveAndroidServiceObjectIds
 import shark.FilteringLeakingObjectFinder.LeakingObjectFilter
 import shark.HeapObject.HeapInstance
-import java.util.EnumSet
-import kotlin.math.absoluteValue
 import shark.internal.InternalSharkCollectionsHelper
 
 /**
@@ -462,25 +462,31 @@ enum class AndroidObjectInspectors : ObjectInspector {
  override val leakingObjectFilter = { heapObject: HeapObject ->
  heapObject is HeapInstance &&
  heapObject instanceOf "androidx.fragment.app.Fragment" &&
- heapObject.getOrThrow(
- "androidx.fragment.app.Fragment", "mFragmentManager"
- ).value.isNullReference
+ heapObject["androidx.fragment.app.Fragment", "mLifecycleRegistry"]!!
+ .valueAsInstance
+  ?.lifecycleRegistryState == "DESTROYED"
  }
 
  override fun inspect(
  reporter: ObjectReporter
  ) {
  reporter.whenInstanceOf("androidx.fragment.app.Fragment") { instance ->
- val fragmentManager =
- instance.getOrThrow("androidx.fragment.app.Fragment", "mFragmentManager")
- if (fragmentManager.value.isNullReference) {
- leakingReasons += fragmentManager describedWithValue "null"
+ val lifecycleRegistryField = instance["androidx.fragment.app.Fragment", "mLifecycleRegistry"]!!
+ val lifecycleRegistry = lifecycleRegistryField.valueAsInstance
+ if (lifecycleRegistry != null) {
+ val state = lifecycleRegistry.lifecycleRegistryState
+ val reason = "Fragment.mLifecycleRegistry.state is $state"
+ if (state == "DESTROYED") {
+ leakingReasons += reason
+ } else {
+ notLeakingReasons += reason
+ }
  } else {
- notLeakingReasons += fragmentManager describedWithValue "not null"
+ labels += "Fragment.mLifecycleRegistry = null"
  }
  val mTag = instance["androidx.fragment.app.Fragment", "mTag"]?.value?.readAsJavaString()
  if (!mTag.isNullOrEmpty()) {
- labels += "Fragment.mTag=$mTag"
+ labels += "Fragment.mTag = $mTag"
  }
  }
  }
@@ -850,25 +856,16 @@ enum class AndroidObjectInspectors : ObjectInspector {
  override fun inspect(reporter: ObjectReporter) {
  reporter.whenInstanceOf("androidx.lifecycle.LifecycleRegistry") { instance ->
  val state = instance.lifecycleRegistryState
- labels += "mState = $state"
  // If state is DESTROYED, this doesn't mean the LifecycleRegistry itself is leaking.
  // Fragment.mViewLifecycleRegistry becomes DESTROYED when the fragment view is destroyed,
  // but the registry itself is still held in memory by the fragment.
  if (state != "DESTROYED") {
- notLeakingReasons += "mState is not DESTROYED"
+ notLeakingReasons += "state is $state"
+ } else {
+ labels += "state = $state"
  }
  }
  }
-
- private val HeapInstance.lifecycleRegistryState: String
- get() {
- // LifecycleRegistry was converted to Kotlin
- // https://cs.android.com/androidx/platform/frameworks/support/+/36833f9ab0c50bf449fc795e297a0e124df3356e
- val stateField = this["androidx.lifecycle.LifecycleRegistry", "state"]
- ?: this["androidx.lifecycle.LifecycleRegistry", "mState"]!!
- val state = stateField.valueAsInstance!!
- return state["java.lang.Enum", "name"]!!.value.readAsJavaString()!!
- }
  },
 
  STUB {
@@ -976,6 +973,16 @@ private fun ObjectReporter.applyFromField(
  notLeakingReasons += delegateReporter.notLeakingReasons.map { "$prefix $it" }
 }
 
+private val HeapInstance.lifecycleRegistryState: String
+ get() {
+ // LifecycleRegistry was converted to Kotlin
+ // https://cs.android.com/androidx/platform/frameworks/support/+/36833f9ab0c50bf449fc795e297a0e124df3356e
+ val stateField = this["androidx.lifecycle.LifecycleRegistry", "state"]
+ ?: this["androidx.lifecycle.LifecycleRegistry", "mState"]!!
+ val state = stateField.valueAsInstance!!
+ return state["java.lang.Enum", "name"]!!.value.readAsJavaString()!!
+}
+
 /**
  * Recursively unwraps `this` [HeapInstance] as a ContextWrapper until an Activity is found in which case it is
  * returned. Returns null if no activity was found.