[compose] Fix Composition leak in ViewAnnotation cleanup (#12389)

GitOrigin-RevId: a9235d4869f772c24c5d75d79d104a6d99f3ff59

Author: pengdev

CHANGELOG.md

@@ -10,6 +10,7 @@ Mapbox welcomes participation and contributions from everyone.
 ## Bug fixes 🐞
 * Fix attribution links accepting non-HTTP URI schemes.
 * Fix a potential file descriptor leak that could cause resource exhaustion.
+* [compose] Fixed Composition leak in ViewAnnotation when annotations are removed from the map.
 
 
 ## Dependencies

extension-compose/build.gradle.kts

@@ -15,6 +15,16 @@ android {
     targetSdk = libs.versions.androidTargetSdkVersion.get().toInt()
     unitTests.apply {
       isIncludeAndroidResources = true
+      all {
+        // JDK 17.0.7+ added StackTraceElement.HashedModules which Robolectric's
+        // SandboxClassLoader tries to instrument. Without --add-opens the module
+        // system blocks reflective access, crashing the test worker with
+        // NoClassDefFoundError: Could not initialize class
+        // java.lang.StackTraceElement$HashedModules
+        it.jvmArgs(
+          "--add-opens", "java.base/java.lang=ALL-UNNAMED",
+        )
+      }
     }
   }
 
@@ -85,6 +95,7 @@ dependencies {
     testImplementation(libs.bundles.base.dependenciesTests)
     testImplementation(project(":maps-sdk"))
     testImplementation(libs.junit)
+    testImplementation(libs.asyncInflater)
   }
 }
 

extension-compose/src/main/java/com/mapbox/maps/extension/compose/annotation/ViewAnnotation.kt

@@ -140,6 +140,11 @@ internal class ViewAnnotationNode(
       }
       removeViewAnnotation(view = view)
     }
+    // Explicitly dispose the Composition to avoid leaking it until the Activity is destroyed.
+    // The ViewCompositionStrategy is set to DisposeOnViewTreeLifecycleDestroyed to survive
+    // temporary detach/reattach cycles in ViewAnnotationManagerImpl, so we need to manually
+    // dispose when the annotation is actually removed.
+    (view as? ComposeView)?.disposeComposition()
     updatedListener = null
   }
 }

extension-compose/src/test/java/com/mapbox/maps/extension/compose/annotation/ViewAnnotationNodeTest.kt

@@ -0,0 +1,168 @@
+package com.mapbox.maps.extension.compose.annotation
+
+import android.view.ViewPropertyAnimator
+import androidx.compose.ui.platform.ComposeView
+import androidx.test.core.app.ApplicationProvider
+import com.mapbox.maps.extension.compose.R
+import com.mapbox.maps.extension.compose.ShadowLogConfiguration
+import com.mapbox.maps.extension.compose.internal.MapNode
+import com.mapbox.maps.viewannotation.OnViewAnnotationUpdatedListener
+import com.mapbox.maps.viewannotation.ViewAnnotationManager
+import io.mockk.CapturingSlot
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.slot
+import io.mockk.spyk
+import io.mockk.unmockkAll
+import io.mockk.verify
+import org.junit.After
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+@Config(shadows = [ShadowLogConfiguration::class])
+@RunWith(RobolectricTestRunner::class)
+internal class ViewAnnotationNodeTest {
+
+  private lateinit var viewAnnotationManager: ViewAnnotationManager
+  private lateinit var composeView: ComposeView
+  private lateinit var parentNode: MapNode
+
+  @Before
+  fun setup() {
+    viewAnnotationManager = mockk(relaxed = true)
+    composeView = spyk(ComposeView(ApplicationProvider.getApplicationContext()))
+    parentNode = mockk(relaxed = true)
+  }
+
+  @After
+  fun tearDown() {
+    unmockkAll()
+  }
+
+  @Test
+  fun `onRemoved disposes composition and removes view annotation`() {
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = null,
+    )
+
+    node.onRemoved(parentNode)
+
+    verify { composeView.disposeComposition() }
+    verify { viewAnnotationManager.removeViewAnnotation(view = composeView) }
+  }
+
+  @Test
+  fun `onRemoved nulls updatedListener`() {
+    val listener = mockk<OnViewAnnotationUpdatedListener>()
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = listener,
+    )
+
+    node.onAttached(parentNode)
+    node.onRemoved(parentNode)
+
+    assertNull(node.updatedListener)
+  }
+
+  @Test
+  fun `onRemoved removes internal update listener when observing`() {
+    val listener = mockk<OnViewAnnotationUpdatedListener>()
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = listener,
+    )
+
+    node.onAttached(parentNode)
+    node.onRemoved(parentNode)
+
+    verify { viewAnnotationManager.removeOnViewAnnotationUpdatedListener(any()) }
+  }
+
+  @Test
+  fun `onRemoved skips removing internal update listener when not observing`() {
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = null,
+    )
+
+    node.onRemoved(parentNode)
+
+    verify(exactly = 0) { viewAnnotationManager.removeOnViewAnnotationUpdatedListener(any()) }
+  }
+
+  @Test
+  fun `onClear disposes composition and removes view annotation`() {
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = null,
+    )
+
+    node.onClear()
+
+    verify { composeView.disposeComposition() }
+    verify { viewAnnotationManager.removeViewAnnotation(view = composeView) }
+  }
+
+  @Test
+  fun `onRemoved with disappear animation defers cleanup`() {
+    setupDisappearAnimation()
+
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = null,
+    )
+
+    node.onRemoved(parentNode)
+
+    verify(exactly = 0) { viewAnnotationManager.removeViewAnnotation(view = composeView) }
+    verify(exactly = 0) { composeView.disposeComposition() }
+  }
+
+  @Test
+  fun `onRemoved with disappear animation cleans up after animation ends`() {
+    val endActionSlot = slot<Runnable>()
+    setupDisappearAnimation(endActionSlot)
+
+    val node = ViewAnnotationNode(
+      viewAnnotationManager = viewAnnotationManager,
+      view = composeView,
+      updatedListener = null,
+    )
+
+    node.onRemoved(parentNode)
+    endActionSlot.captured.run()
+
+    verify { viewAnnotationManager.removeViewAnnotation(view = composeView) }
+    verify { composeView.disposeComposition() }
+  }
+
+  private fun setupDisappearAnimation(
+    endActionSlot: CapturingSlot<Runnable>? = null,
+  ): ViewPropertyAnimator {
+    val animationConfig = MarkerAnimationConfig(
+      listOf(MarkerAnimationEffect.Effect.Fade(from = 1f, to = 0f))
+    )
+    composeView.setTag(R.id.markerDisappearAnimation, animationConfig)
+
+    val animator = mockk<ViewPropertyAnimator>(relaxed = true)
+    every { composeView.animate() } returns animator
+    every { animator.alpha(any()) } returns animator
+    every { animator.setDuration(any()) } returns animator
+    if (endActionSlot != null) {
+      every { animator.withEndAction(capture(endActionSlot)) } returns animator
+    }
+    return animator
+  }
+}