RUMM-3475: Fix memory leak in JankStats usage

DataDog · Jul 24, 2023 · c20c477 · c20c477
1 parent 01ece69
commit c20c477
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 3 deletions.
diff --git a/detekt_custom.yml b/detekt_custom.yml
@@ -750,6 +750,7 @@ datadog:
       - "kotlin.collections.MutableList.forEachIndexed(kotlin.Function2)"
       - "kotlin.collections.MutableList.isEmpty()"
       - "kotlin.collections.MutableList.isNotEmpty()"
+      - "kotlin.collections.MutableList.isNullOrEmpty()"
       - "kotlin.collections.MutableList.iterator()"
       - "kotlin.collections.MutableList.joinToString(kotlin.CharSequence, kotlin.CharSequence, kotlin.CharSequence, kotlin.Int, kotlin.CharSequence, kotlin.Function1?)"
       - "kotlin.collections.MutableList.remove(java.io.File)"

diff --git a/...main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt b/...main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt
@@ -27,8 +27,8 @@ internal class JankStatsActivityLifecycleListener(
     private val jankStatsProvider: JankStatsProvider = JankStatsProvider.DEFAULT
 ) : ActivityLifecycleCallbacks, JankStats.OnFrameListener {
 
-    private val activeWindowsListener = WeakHashMap<Window, JankStats>()
-    private val activeActivities = WeakHashMap<Window, MutableList<WeakReference<Activity>>>()
+    internal val activeWindowsListener = WeakHashMap<Window, JankStats>()
+    internal val activeActivities = WeakHashMap<Window, MutableList<WeakReference<Activity>>>()
 
     // region ActivityLifecycleCallbacks
     @MainThread
@@ -106,6 +106,10 @@ internal class JankStatsActivityLifecycleListener(
 
     @MainThread
     override fun onActivityDestroyed(activity: Activity) {
+        if (activeActivities[activity.window].isNullOrEmpty()) {
+            activeWindowsListener.remove(activity.window)
+            activeActivities.remove(activity.window)
+        }
     }
 
     // endregion

diff --git a/.../kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt b/.../kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt
@@ -18,6 +18,7 @@ import fr.xgouchet.elmyr.annotation.BoolForgery
 import fr.xgouchet.elmyr.annotation.LongForgery
 import fr.xgouchet.elmyr.junit5.ForgeConfiguration
 import fr.xgouchet.elmyr.junit5.ForgeExtension
+import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
@@ -142,7 +143,40 @@ internal class JankStatsActivityLifecycleListenerTest {
     }
 
     @Test
-    fun `𝕄 foNothing 𝕎 onActivityStopped() {}`() {
+    fun `𝕄 remove window 𝕎 onActivityDestroyed() { no more activities for window }`() {
+        // Given
+
+        // When
+        testedJankListener.onActivityStarted(mockActivity)
+        testedJankListener.onActivityStopped(mockActivity)
+        testedJankListener.onActivityDestroyed(mockActivity)
+
+        // Then
+        assertThat(testedJankListener.activeActivities).isEmpty()
+        assertThat(testedJankListener.activeWindowsListener).isEmpty()
+    }
+
+    @Test
+    fun `𝕄 not remove window 𝕎 onActivityDestroyed() { there are activities for window }`() {
+        // Given
+        val anotherActivity = mock<Activity>().apply {
+            whenever(window) doReturn mockWindow
+            whenever(display) doReturn mockDisplay
+        }
+
+        // When
+        testedJankListener.onActivityStarted(mockActivity)
+        testedJankListener.onActivityStopped(mockActivity)
+        testedJankListener.onActivityStarted(anotherActivity)
+        testedJankListener.onActivityDestroyed(mockActivity)
+
+        // Then
+        assertThat(testedJankListener.activeActivities).isNotEmpty
+        assertThat(testedJankListener.activeWindowsListener).isNotEmpty
+    }
+
+    @Test
+    fun `𝕄 do nothing 𝕎 onActivityStopped() {}`() {
         // Given
 
         // When