fix(android): Stop attributing Nav3 data to Activities without a SentryNavEffect

Stop misattributing nav data and spans to a Nav3 root when the user navigates to a new Activity (or ViewPager2 page) that doesn't contain a SentryNavEffect.

THE PROBLEM:

Assume Home Activity has a `SentryNavEffect` but Plain Activity doesn't.

```
 1. App goes to background:   Home ON_PAUSE ──────────────────▶ Home ON_STOP
 2. Covered by PlainActivity: Home ON_PAUSE ─▶ Plain ON_RESUME ─▶ Home ON_STOP
                                               ^^^^^^^^^^^^^^^
                                      Invisible to Home's SentryNavEffect
```

Case (1) is already handled properly today; case (2) is what this PR addresses.

With case (2), spans generated by the Plain Activity will be (wrongly) attributed to the Home Activity's nav transaction, and errors thrown by Plain Activity will contain the Home Activity's nav context (screen name + back stack).

Note that cases (1) and (2) require opposite implementations. For (1), the leaseholder should keep its data b/c it's still the last activity the user saw and the user may come back to it. For (2), the leaseholder's data no longer describes what's on the screen and it should be cleared.

THE SOLUTION:

This commit satisfies both cases by clearing Home's nav context and unbinding its nav transaction when Plain is resumed. If Home returns, its nav context and transaction (if still unfinished) is restored.

Implemented by the existing `NavLeaseCoordinator` via the concept of "lease withdrawal".

(Note: Fragments not used as ViewPager2 pages don't have the problem discussed above because the Home fragment's view would be destroyed when the Plain fragment showed up, meaning the leaseholder's data would be cleared in case (2).)

Author: 0xadam-brown

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixes
 
+- Stop attributing errors and spans from an Activity or ViewPager2 page without a `SentryNavEffect` to the most recently visited Navigation 3 root ([#6457](https://github.com/getsentry/sentry-java/pull/6457))
 - Fix missing Navigation 3 transactions for preloaded ViewPager2 pages and destinations reached while another Activity or Fragment owns navigation tracking ([#6400](https://github.com/getsentry/sentry-java/pull/6400))
 - Keep UI profiling running when `startProfiler` is called before a requested stop takes effect at the end of the current chunk ([#6291](https://github.com/getsentry/sentry-java/pull/6291))
   - Before, the start call was ignored and the profiler still stopped at the end of the chunk, even in `TRACE` mode while sampled spans were active. On API 34 and lower, every later profile also stopped after one chunk.

sentry-android-navigation3/build.gradle.kts

@@ -63,6 +63,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/CoordinatedBackStackObserver.kt

@@ -1,10 +1,10 @@
 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
 
 /**
@@ -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>) {

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

@@ -4,11 +4,13 @@ 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 org.jetbrains.annotations.ApiStatus
 
 /**
@@ -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. attach() only uses it to find where the nav root is
+  // displayed, 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))
     onDispose { observer.dispose() }
   }
 

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

@@ -0,0 +1,45 @@
+package io.sentry.compose.navigation3.lifecycle
+
+import android.app.Activity
+import android.content.Context
+import android.content.ContextWrapper
+import androidx.lifecycle.Lifecycle
+
+/**
+ * Describes the nav root of a `SentryNavEffect`.
+ *
+ * A **nav root** is the Activity or Fragment whose content calls a `SentryNavEffect` and owns the
+ * back stack the effect observes.
+ */
+internal class NavRootInfo(
+  /** The nav root's lifecycle. */
+  val lifecycle: Lifecycle,
+  /** The context the nav root's content is displayed in, if known. */
+  val context: Context?,
+) {
+
+  /**
+   * The Activity that displays the nav root's content, or `null` if [context] is unknown or doesn't
+   * belong to an Activity.
+   *
+   * If the nav root is itself an Activity, this will be the nav root itself. If the nav root is a
+   * Fragment, this will be the Activity that hosts that Fragment.
+   */
+  val activity: Activity?
+    get() = context?.findActivity()
+
+  /**
+   * The Activity the receiver belongs to, found by unwrapping [ContextWrapper]s (e.g., the
+   * `ContextThemeWrapper` of a dialog), or `null` if there's none.
+   */
+  private fun Context.findActivity(): Activity? {
+    var context: Context? = this
+    while (context is ContextWrapper) {
+      if (context is Activity) {
+        return context
+      }
+      context = context.baseContext
+    }
+    return null
+  }
+}

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

@@ -24,8 +24,8 @@ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation"
  * Needed because all `SentryNavEffect`s mutate a single [IScope] instance by default and will
  * clobber or interleave one another's updates unless properly managed:
  * ```
- * LifecycleOwner (e.g., Activity or Fragment)
- *       ↓ owns
+ * Nav root (Activity or Fragment)
+ *       ↓ calls
  *  SentryNavEffect
  *       ↓
  *   CoordinatedBackStackObserver (wrapper)
@@ -56,7 +56,7 @@ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation"
  * 2. Pass the same [NavLeaseCoordinator] instance to all `NavScopes` whose effects must be
  *    coordinated (i.e., that target the same `IScope`).
  *
- * 3. Call [attach] with the lifecycle that manages the effect before the first back stack update.
+ * 3. Call [attach] with the effect's [nav root][NavRootInfo] before the first back stack update.
  *
  * 4. Call [onBackStackUpdate] before applying each back stack update.
  *
@@ -86,16 +86,18 @@ internal class NavScopes(
   private val navStatePublisher = NavStatePublisher()
 
   /**
-   * Registers this [NavScopes] instance with [lifecycle]. No-ops if `lifecycle` has already been
-   * attached or is [destroyed][Lifecycle.Event.ON_DESTROY].
+   * Registers this [NavScopes] instance with the lifecycle of the effect's nav root, as described
+   * by [rootInfo]. No-ops if that lifecycle has already been attached or is
+   * [destroyed][Lifecycle.Event.ON_DESTROY].
    *
    * Attaching to a second, different lifecycle no-ops.
    */
-  fun attach(lifecycle: Lifecycle) {
+  fun attach(rootInfo: NavRootInfo) {
     val currentLifecycle = attachedLifecycle
+    val incomingLifecycle = rootInfo.lifecycle
 
     if (currentLifecycle != null) {
-      if (currentLifecycle !== lifecycle) {
+      if (currentLifecycle !== incomingLifecycle) {
         delegate.options.logger.log(
           SentryLevel.WARNING,
           "NavScopes is already attached to a different lifecycle; ignoring the new lifecycle.",
@@ -106,12 +108,12 @@ internal class NavScopes(
       return
     }
 
-    if (lifecycle.currentState == Lifecycle.State.DESTROYED) {
+    if (incomingLifecycle.currentState == Lifecycle.State.DESTROYED) {
       return
     }
 
-    attachedLifecycle = lifecycle
-    registration = coordinator.register(lifecycle, delegate, navStatePublisher)
+    attachedLifecycle = incomingLifecycle
+    registration = coordinator.register(delegate, rootInfo, navStatePublisher)
   }
 
   /**
@@ -147,7 +149,7 @@ internal class NavScopes(
    * Publishes this instance's [stagedNavTransaction] and [stagedNavData] to the shared [IScope]
    * while it holds the nav lease, as directed by the [coordinator].
    */
-  private inner class NavStatePublisher : LeasedNavState {
+  private inner class NavStatePublisher : INavStatePublisher {
 
     override fun publishTo(scope: IScope) {
       // Publishing the data first ensures a staged transaction is sampled against the rotated

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

@@ -17,8 +17,8 @@ Support for multiple `SentryNavEffect`s is restricted to the scenarios documente
 [Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/#supported-setups).
 
 `NavLeaseCoordinator` implements the policy that underlies those scenarios. It assumes that, at any moment, the user is interacting with at
-most one registered `LifecycleOwner` in the host app, and that the `LifecycleOwner` being interacted with is the one that was most recently
-resumed.
+most one nav root in the host app (see `NavRootInfo` for a definition of "nav root"), and that the nav root being interacted with is the one
+that was most recently resumed.
 
 Concretely, a host app can safely use more than one `SentryNavEffect` only if:
 

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md

@@ -21,6 +21,7 @@ import io.sentry.TransactionOptions
 import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY
 import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY
 import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator
+import io.sentry.compose.navigation3.lifecycle.NavRootInfo
 import io.sentry.compose.navigation3.lifecycle.NavScopes
 import io.sentry.protocol.App
 import io.sentry.protocol.TransactionNameSource
@@ -119,7 +120,7 @@ class BackStackObserverTest {
             LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED }
         }
       return NavScopes(scopes, NavLeaseCoordinator()).apply {
-        attach(owner.lifecycle)
+        attach(NavRootInfo(owner.lifecycle, context = null))
         // The sole resumed owner's first update acquires the lease, which it then keeps.
         onBackStackUpdate()
       }

sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt

@@ -15,9 +15,10 @@ import io.sentry.SentryOptions
 import io.sentry.SentryTracer
 import io.sentry.TransactionContext
 import io.sentry.TransactionOptions
-import io.sentry.compose.navigation3.lifecycle.LeasedNavState
+import io.sentry.compose.navigation3.lifecycle.INavStatePublisher
 import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY
 import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator
+import io.sentry.compose.navigation3.lifecycle.NavRootInfo
 import org.junit.Test
 import org.mockito.kotlin.any
 import org.mockito.kotlin.doAnswer
@@ -360,7 +361,14 @@ class CoordinatedBackStackObserverTest {
     /** Registers another owner that takes the nav lease and then pauses, keeping the lease. */
     fun givenPausedLeaseholder() {
       val otherLifecycle = resumedLifecycle()
-      val other = checkNotNull(coordinator.register(otherLifecycle, scopes, NoOpLeasedNavState))
+      val other =
+        checkNotNull(
+          coordinator.register(
+            scopes,
+            NavRootInfo(otherLifecycle, context = null),
+            NoOpNavStatePublisher,
+          )
+        )
       other.onBackStackUpdate()
       otherLifecycle.currentState = Lifecycle.State.STARTED
       check(other.hasLease)
@@ -380,7 +388,7 @@ class CoordinatedBackStackObserverTest {
             },
           coordinator = coordinator,
         )
-        .apply { attach(lifecycle) }
+        .apply { attach(NavRootInfo(lifecycle, context = null)) }
 
     /**
      * Owners of the lifecycles created by [resumedLifecycle]. `LifecycleRegistry` holds its owner
@@ -398,7 +406,7 @@ class CoordinatedBackStackObserverTest {
   }
 
   /** Nav state for a competing owner that has nothing to publish. */
-  private object NoOpLeasedNavState : LeasedNavState {
+  private object NoOpNavStatePublisher : INavStatePublisher {
     override fun publishTo(scope: IScope) = Unit
 
     override fun unpublishFrom(scope: IScope) = Unit