ref(android): [Part 1] Refactor

sentry-android-navigation3/build.gradle.kts

@@ -64,6 +64,7 @@ dependencies {
   implementation(libs.androidx.lifecycle.runtime.compose)
 
   compileOnly(libs.androidx.compose.runtime)
+  compileOnly(libs.androidx.compose.ui)
 
   testImplementation(libs.androidx.compose.ui.test.junit4)
   testImplementation(libs.androidx.fragment.ktx)

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt

@@ -33,8 +33,8 @@ private const val NAVIGATION_OP: String = "navigation"
  *
  * **Entry identity determines whether the top has changed**
  *
- * Referential equality (===), not structural equality, is used to determine whether the top of the
- * incoming back stack has changed. That approach:
+ * Referential equality (===) is used to determine whether the top of the incoming back stack has
+ * changed. That approach:
  *
  * - matches the typical Nav3 SnapshotStateList, where an entry instance has a stable identity for
  *   its lifetime in the stack;
@@ -206,21 +206,21 @@ internal class BackStackObserver<T : Any>(
       scope.rotatePropagationContext()
     }
 
-    storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topEntryNormalized)
+    storePreviousTop(currentBackStack)
   }
 
   private fun handleSameTop(scope: INavScope, backStack: BackStackData<T>) {
     navContext.update(scope, backStack.capturedEntriesNormalized)
-    storeAsPreviousTop(backStack.topEntry, backStack.topEntryNormalized)
+    storePreviousTop(backStack)
   }
 
   private fun handleEmptyBackStack(scope: INavScope) {
     tearDown(scope)
   }
 
-  private fun storeAsPreviousTop(topEntry: T, topEntryNormalized: NormalizedSentryBackStackEntry) {
-    previousTop = WeakReference(topEntry)
-    previousTopNormalized = topEntryNormalized
+  private fun storePreviousTop(backStack: BackStackData<T>) {
+    previousTop = WeakReference(backStack.topEntry)
+    previousTopNormalized = backStack.topEntryNormalized
   }
 
   private fun clearPreviousTop() {

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt

@@ -1,15 +1,15 @@
 package io.sentry.compose.navigation3
 
-import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.LifecycleEventObserver
 import io.sentry.IScope
 import io.sentry.IScopes
 import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator
+import io.sentry.compose.navigation3.lifecycle.NavRootInfo
 import io.sentry.compose.navigation3.lifecycle.NavScopes
 
 /**
- * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with other
- * `BackStackObserver`s.
+ * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with those of
+ * other `BackStackObserver`s.
  *
  * Coordination is achieved via [NavScopes].
  *
@@ -35,8 +35,8 @@ internal class CoordinatedBackStackObserver<T : Any>(
   private val navScopes = NavScopes(scopes, coordinator)
   private val observer = BackStackObserver(navScopes, options, entryMapper)
 
-  fun attach(lifecycle: Lifecycle) {
-    navScopes.attach(lifecycle)
+  fun attach(rootInfo: NavRootInfo) {
+    navScopes.attach(rootInfo)
   }
 
   fun onBackStackChanged(backStack: List<T>) {
@@ -52,5 +52,24 @@ internal class CoordinatedBackStackObserver<T : Any>(
   }
 }
 
-/** The [NavLeaseCoordinator] shared by default by all [SentryNavEffect]s. */
+/**
+ * The [NavLeaseCoordinator] shared by default by all [CoordinatedBackStackObserver]s.
+ *
+ * **Lifetime**
+ *
+ * The default coordinator lives as long as the process, since lease state follows nav root
+ * lifecycles rather than the SDK's. Closing and re-initializing the SDK doesn't require a reset
+ * because:
+ *
+ * - `SentryNavEffect` resolves the current scopes on every call;
+ *
+ * - while the SDK is closed, bookkeeping continues without a scope; and
+ *
+ * - unpublishing compares by reference, so values published to an earlier scope don't affect the
+ *   current one.
+ *
+ * Limitation: A nav transaction that's still running when the SDK is closed and re-initialized can
+ * be bound to the new scope the next time it's published, although its client has been closed.
+ * Spans then attach to it, and are lost, until it times out or the next navigation finishes it.
+ */
 internal val defaultNavLeaseCoordinator: NavLeaseCoordinator = NavLeaseCoordinator()

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt

@@ -4,17 +4,19 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberUpdatedState
+import androidx.compose.ui.platform.LocalContext
 import androidx.lifecycle.compose.LocalLifecycleOwner
 import io.sentry.IScopes
 import io.sentry.ScopesAdapter
 import io.sentry.SentryOptions
 import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator
+import io.sentry.compose.navigation3.lifecycle.NavRootInfo
+import io.sentry.compose.navigation3.lifecycle.findActivity
 import org.jetbrains.annotations.ApiStatus
 
 /**
- * An effect for generating Sentry data from your Nav3 backstack. Configure it via [options] and
- * call it before you invoke your `NavDisplay`.
- *
+ * A composable effect for generating Sentry data from your Nav3 backstack. Configure it via
+ * [options] and call it before you invoke your `NavDisplay`:
  * ```kotlin
  *  @Composable
  *  fun AppNavigation() {
@@ -24,7 +26,7 @@ import org.jetbrains.annotations.ApiStatus
  *    // Place SentryNavEffect in the same composable as your NavDisplay and call
  *    // the effect first. Doing so ensures the effect's lifecycle matches your
  *    // NavDisplay, and that any Sentry data produced by your initial nav
- *    // destination get attributed to the appropriate nav transaction.
+ *    // destination are attributed to the appropriate nav transaction.
  *    SentryNavEffect(
  *      backStack = navBackStack,
  *      backStackEntryMapper = { entry ->
@@ -105,6 +107,7 @@ internal fun <T : Any> SentryNavEffect(
 ) {
   val currentBackStackEntryMapper = rememberUpdatedState(backStackEntryMapper)
   val lifecycle = LocalLifecycleOwner.current.lifecycle
+  val context = LocalContext.current
 
   val observer =
     remember(scopes, options, lifecycle, coordinator) {
@@ -116,9 +119,13 @@ internal fun <T : Any> SentryNavEffect(
       )
     }
 
-  // Register first so the initial prepared back stack update can acquire navigation ownership.
+  // Intentionally don't key on context. It's only used to find the Activity that displays the nav
+  // root, which can't change without the lifecycle changing too, and that replaces the observer.
+  // The context's identity can change more often (e.g., if the host app provides a new
+  // ContextThemeWrapper on every recomposition), and re-attaching for those changes would
+  // needlessly finish the nav transaction.
   DisposableEffect(observer) {
-    observer.attach(lifecycle)
+    observer.attach(NavRootInfo(lifecycle, context.findActivity()))
     onDispose { observer.dispose() }
   }