From 194256a8147a619bd8dd21ff906b5b8385451432 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 05:46:27 +0100 Subject: [PATCH 1/4] fix(android): Stop attributing Nav3 data to Activities and pages without a SentryNavEffect When an Activity or ViewPager2 page without a SentryNavEffect replaced the Nav3 leaseholder, the leaseholder kept its nav context and bound nav transaction, so errors and spans from the new screen were attributed to it. The coordinator now withdraws the lease in that case: it unpublishes the leaseholder's nav state and unbinds its transaction without handing the lease to another nav root. The leaseholder republishes them when it resumes. Withdrawal is triggered when another Activity resumes while the leaseholder isn't resumed (unless that Activity is itself a nav root), or when the leaseholder pauses while its host Activity stays resumed (e.g., a ViewPager2 swipe). The host Activity comes from NavRootInfo, and a NavActivityWatcher reports Activity resumes. A nav root that isn't resumed also no longer takes the lease on its first update while its host Activity is resumed (e.g., a page preloaded next to one without an effect). --- CHANGELOG.md | 4 + .../lifecycle/NavActivityWatcher.kt | 52 ++ .../lifecycle/NavLeaseCoordinator.kt | 119 +++- .../compose/navigation3/lifecycle/README.md | 42 +- ...SentryNavEffectLifecycleIntegrationTest.kt | 399 +++++++++++- .../navigation3/SentryNavEffectTest.kt | 33 +- .../lifecycle/NavLeaseCoordinatorTest.kt | 566 +++++++++++++++++- .../src/main/AndroidManifest.xml | 5 + .../navigation/nav3/Nav3MultiFragment.kt | 17 +- .../navigation/nav3/Nav3PlainScreens.kt | 206 +++++++ .../navigation/nav3/Nav3RouteScreens.kt | 1 + .../android/navigation/nav3/Nav3Routes.kt | 4 +- .../navigation/nav3/Nav3TransactionHistory.kt | 16 + 13 files changed, 1419 insertions(+), 45 deletions(-) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavActivityWatcher.kt create mode 100644 sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3PlainScreens.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index ff1ad7074b6..9b82a501d4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ - Nav3: Returning to an Activity or Fragment with its own `SentryNavEffect` within the app (e.g., closing the Activity that covered it, swiping back to its ViewPager2 page, or dismissing a dialog over it) now starts a new navigation transaction for the current destination if the previous one finished, so spans produced upon return can now be consistently captured ([#6482](https://github.com/getsentry/sentry-java/pull/6482)) +### Fixes + +- Nav3: Stop attributing errors and spans from an Activity or ViewPager2 page without a `SentryNavEffect` to the most recently visited Nav3 root ([#XXXX](https://github.com/getsentry/sentry-java/pull/XXXX)) + ## 8.63.0 ### Features diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavActivityWatcher.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavActivityWatcher.kt new file mode 100644 index 00000000000..0810c41f27a --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavActivityWatcher.kt @@ -0,0 +1,52 @@ +package io.sentry.compose.navigation3.lifecycle + +import android.app.Activity +import android.app.Application +import android.app.Application.ActivityLifecycleCallbacks +import android.os.Bundle + +/** + * Watches every Activity of the host application and reports the Activity lifecycle events that + * [NavLeaseCoordinator] acts on. + * + * - [onActivityResumed] is called when any Activity resumes, including Activities without a + * `SentryNavEffect`. + * + * **Lifetime** + * + * The callbacks are registered at most once per watcher and never unregistered. Like + * `defaultNavLeaseCoordinator`, which owns the watcher used in practice, they're meant to live as + * long as the process. + */ +internal class NavActivityWatcher(private val onActivityResumed: (activity: Activity) -> Unit) { + + private var isWatching = false + + /** Starts watching the Activities of [application]. Does nothing if already watching. */ + fun watch(application: Application) { + if (isWatching) { + return + } + isWatching = true + application.registerActivityLifecycleCallbacks(Callbacks()) + } + + private inner class Callbacks : ActivityLifecycleCallbacks { + + override fun onActivityResumed(activity: Activity) { + this@NavActivityWatcher.onActivityResumed(activity) + } + + override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) = Unit + + override fun onActivityStarted(activity: Activity) = Unit + + override fun onActivityPaused(activity: Activity) = Unit + + override fun onActivityStopped(activity: Activity) = Unit + + override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) = Unit + + override fun onActivityDestroyed(activity: Activity) = Unit + } +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt index 492a7c35526..78a20008e6c 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt @@ -1,5 +1,6 @@ package io.sentry.compose.navigation3.lifecycle +import android.app.Activity import androidx.lifecycle.Lifecycle import androidx.lifecycle.Lifecycle.Event.ON_DESTROY import androidx.lifecycle.Lifecycle.Event.ON_PAUSE @@ -10,6 +11,7 @@ import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.LifecycleOwner import io.sentry.IScope import io.sentry.IScopes +import io.sentry.SentryLevel /** * Manages a lease in order to determine which [NavScopes] instance can bind nav transactions and @@ -53,6 +55,12 @@ internal class NavLeaseCoordinator { */ private var resumeOrder: Long = 0 + /** + * Reports the host application's Activities to this coordinator so that it can detect whether a + * leaseholder is covered by a non-candidate root. + */ + private val activityWatcher = NavActivityWatcher(onActivityResumed = ::onAnyActivityResumed) + /** * Registers a [nav root][rootInfo] as a lease candidate until disposal or lifecycle destruction. * @@ -70,11 +78,13 @@ internal class NavLeaseCoordinator { return null } + rootInfo.activity?.application?.let { activityWatcher.watch(it) } + val initialResumeOrder = if (lifecycleState == Lifecycle.State.RESUMED) ++resumeOrder else 0 val candidate = Candidate(rootInfo, lifecycleState, initialResumeOrder, publisher, scopes) - // Only list the candidate once addObserver() succeeds, so a failure can't leave behind a - // candidate that never observes its nav root's lifecycle. + // Only list the candidate once addObserver() succeeds, so a failure can't leak the host + // Activity. lifecycle.addObserver(candidate) candidates += candidate return candidate @@ -93,15 +103,15 @@ internal class NavLeaseCoordinator { val holder = leaseHolder - // A nav root that isn't resumed doesn't take the lease from a resumed leaseholder, as it isn't - // the one the user is interacting with (e.g., a preloaded ViewPager2 page next to the current - // page). Instead, it acquires the lease when it resumes. - val wouldDisplaceResumedHolder = - holder != null && - holder.state == Lifecycle.State.RESUMED && - candidate.state != Lifecycle.State.RESUMED + // A nav root that isn't resumed while either the leaseholder or its own host Activity is + // resumed isn't the one the user is interacting with (e.g., a preloaded ViewPager2 page next to + // the current page, which may be a page without a SentryNavEffect and so never a leaseholder). + // Instead, it acquires the lease when it resumes. + val isBesideResumedContent = + candidate.state != Lifecycle.State.RESUMED && + (holder?.state == Lifecycle.State.RESUMED || candidate.isHostActivityResumed) - if (candidate.canAcquireLeaseOnFirstUpdate && !wouldDisplaceResumedHolder) { + if (candidate.canAcquireLeaseOnFirstUpdate && !isBesideResumedContent) { transferLeaseTo(candidate, scope) } } @@ -136,6 +146,32 @@ internal class NavLeaseCoordinator { } } + /** + * Releases the lease without handing it off and unpublishes its nav state from [scope]. The + * candidate retains staged state for a later return. Without a scope, state remains published. + * + * Logs [withdrawalReason]. + */ + private fun withdrawLease(scope: IScope?, withdrawalReason: String) { + val holder = leaseHolder ?: return + + leaseHolder = null + scope + ?.options + ?.logger + ?.log( + SentryLevel.DEBUG, + "Withdrew the Nav3 lease from a nav root hosted by %s: %s.", + holder.hostActivity?.javaClass?.name, + withdrawalReason, + ) + if (scope != null && lastPublisher === holder) { + // Clear the last publisher before calling into the scope (see the class's exception policy). + lastPublisher = null + holder.publisher.unpublishFrom(scope) + } + } + /** * The eligible candidate most recently observed to be in a resumed state, other than [excluding]. * @@ -176,7 +212,8 @@ internal class NavLeaseCoordinator { * Records that [candidate] paused. * * If [candidate] holds the lease when it pauses, hands it to another resumed nav root (e.g., a - * Fragment whose dialog is being dismissed); otherwise the paused nav root keeps the lease. + * Fragment whose dialog is being dismissed). If none exists, withdraws it when the host Activity + * remains resumed; otherwise the paused nav root keeps the lease. */ private fun pauseCandidate(candidate: Candidate, scope: IScope?) { candidate.pause() @@ -185,8 +222,14 @@ internal class NavLeaseCoordinator { // window or picture-in-picture mode), and any other paused nav root is beneath it. A displaced // nav root will acquire the lease again when it resumes. if (leaseHolder === candidate) { - mostRecentlyResumedCandidate(excluding = candidate, resumedOnly = true)?.let { next -> - handOffLease(next, scope) + val next = mostRecentlyResumedCandidate(excluding = candidate, resumedOnly = true) + when { + next != null -> handOffLease(next, scope) + candidate.isHostActivityResumed -> + withdrawLease( + scope, + withdrawalReason = "the nav root paused while its host Activity stayed resumed", + ) } } } @@ -242,6 +285,44 @@ internal class NavLeaseCoordinator { } } + /** + * Whether [activity] owns a candidate's nav root that + * [can receive the lease][Candidate.canReceiveLease]. + * + * A returning Activity root takes the lease on resume and should not trigger withdrawal first. + */ + private fun isActivityNavRoot(activity: Activity): Boolean = candidates.any { + it.canReceiveLease && it.isActivityNavRoot(activity) + } + + /** Withdraws the lease when the resumed [activity] appears to cover a non-resumed nav root. */ + private fun onAnyActivityResumed(activity: Activity) { + // Assume the resumed `activity` is covering the current leaseholder if i) the leaseholder + // itself isn't resumed, ii) `activity` isn't the leaseholder's host Activity, and iii) + // `activity` isn't itself a nav root (see [isActivityNavRoot]). (i) checks the leaseholder's + // own + // state rather than its host Activity's, as they differ for Fragment nav roots: a Fragment + // leaseholder that isn't resumed isn't what the user is interacting with, even if its host + // Activity is resumed. + val holder = leaseHolder ?: return + val hostActivity = holder.hostActivity ?: return + if ( + hostActivity === activity || + holder.state == Lifecycle.State.RESUMED || + isActivityNavRoot(activity) + ) { + return + } + + holder.withScopeOrNull { scope -> + withdrawLease( + scope, + withdrawalReason = + "${activity.javaClass.name} was resumed while the nav root wasn't resumed", + ) + } + } + /** * A [nav root][rootInfo] eligible to hold the lease managed by [NavLeaseCoordinator]. * @@ -255,6 +336,12 @@ internal class NavLeaseCoordinator { private val scopes: IScopes, ) : NavLeaseRegistration, LifecycleEventObserver { + val hostActivity: Activity? = rootInfo.activity + + /** Whether the host Activity is resumed; false when its lifecycle is unavailable. */ + val isHostActivityResumed: Boolean + get() = (hostActivity as? LifecycleOwner)?.lifecycle?.currentState == Lifecycle.State.RESUMED + /** Most recent resume order; zero until this candidate first resumes. */ var resumeOrder: Long = initialResumeOrder private set @@ -353,7 +440,7 @@ internal class NavLeaseCoordinator { * Runs [block] with the scope, or with `null` if [IScopes.configureScope] skips its callback. * Does not rerun a callback that throws. */ - private fun withScopeOrNull(block: (IScope?) -> Unit) { + fun withScopeOrNull(block: (IScope?) -> Unit) { var callbackInvoked = false scopes.configureScope { scope -> callbackInvoked = true @@ -364,6 +451,10 @@ internal class NavLeaseCoordinator { block(null) } } + + /** Whether [activity] itself owns this nav root, rather than a Fragment it hosts. */ + fun isActivityNavRoot(activity: Activity): Boolean = + hostActivity === activity && (activity as? LifecycleOwner)?.lifecycle === lifecycle } } diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md index c6aa677e7ce..2b496552c93 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md @@ -56,8 +56,9 @@ A displaced nav root remains eligible for lease restoration until it stops (`ON_ root's nav data is reinstated, and so is its nav transaction if unfinished. Otherwise, the nav root gets a new nav transaction (see [below](#returning-to-a-nav-root)). -> † The exception is an incoming nav root that isn't resumed while the leaseholder is, e.g., a -preloaded ViewPager2 Fragment page next to the current one. The user isn't interacting with such a +> † The exception is an incoming nav root that isn't resumed while the leaseholder or the incoming +nav root's host Activity is, e.g., a preloaded ViewPager2 Fragment page next to the current one, +whether or not the current page has a `SentryNavEffect`. The user isn't interacting with such a nav root yet, so it acquires the lease when it resumes instead. The following diagram illustrates how handoff works: @@ -144,6 +145,29 @@ Alternative at rollback point: B disposes before A stops Finish txB if unfinished │ ``` +### Non-candidate roots and lease withdrawal + +An Activity or Fragment without a `SentryNavEffect` (aka, a "non-candidate root") can't acquire the lease. When one replaces the +leaseholder, the coordinator "withdraws" the lease instead, i.e., it unpublishes the leaseholder's nav state without handing off the lease, +leaving the transaction unbound but running until the leaseholder resumes. + +Non-candidate roots never register, so the coordinator infers their presence from either of two signals: + +- another Activity resumes while the leaseholder isn't resumed, unless the resumed Activity is + itself a nav root, which takes the lease when it resumes; or + +- the leaseholder pauses while its host Activity stays resumed (e.g., a ViewPager2 swipe). (Note + that an Activity pauses before its Fragments do when the Activity is backgrounded or covered.) + +**Limitations**: Leaseholders whose host Activity is unknown are never withdrawn. Fragments that cover the leaseholder without pausing it +(e.g., dialogs) go undetected. These two signals are deliberately the only ones: other surfaces that cover a leaseholder without pausing it +(e.g., bottom sheets or Compose dialogs) aren't detected, and shouldn't be by adding signals here. + +**Limitation (before the covering Activity resumes)**: The lease is withdrawn only once the covering Activity resumes. Until then (i.e., +during its `onCreate` and `onStart`), errors and spans the covering Activity produces are still attributed to the leaseholder. For the same +reason, the covering Activity's `ui.load` transaction isn't bound to the scope: `ActivityLifecycleIntegration` tries to bind it when the +Activity is created, while the leaseholder's nav transaction still holds the scope's transaction slot. + ### Returning to a nav root A nav root is "returned to" when its candidate acquires the lease at any time other than on its first back stack update, e.g., when the @@ -167,3 +191,17 @@ gets a transaction on its first back stack update. See [#6454](https://github.co Bringing the app back to the foreground isn't a return either: the leaseholder keeps the lease while the app is in the background (see `stopCandidate`), so it starts no new nav transaction when it resumes. This matches the rest of the SDK, which starts no transaction when an existing Activity comes back to the foreground. + +### Android ordering assumptions + +`NavLeaseCoordinator` relies on the following sequences, which Android doesn't document but AOSP and androidx have historically upheld: + +- An Activity's `onActivityResumed` callbacks run before its `Lifecycle` dispatches + `ON_RESUME` (see `isActivityNavRoot`). + +- An Activity's `Lifecycle` dispatches `ON_PAUSE` before its Fragments' + lifecycles do. androidx's `ReportFragment` and `FragmentActivity` ensure this, not the + platform. + +Going forward, if an order changes, nav data may be attributed to the wrong nav root, or a nav transaction may go unbound, but nothing would +throw. diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt index c30f296fd39..2a9138b93e5 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt @@ -41,7 +41,6 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.robolectric.Robolectric import org.robolectric.Shadows.shadowOf -import org.robolectric.android.controller.ActivityController import org.robolectric.annotation.Config /** @@ -58,6 +57,7 @@ import org.robolectric.annotation.Config */ @RunWith(AndroidJUnit4::class) @Config(sdk = [30]) +@Suppress("LargeClass") class SentryNavEffectLifecycleIntegrationTest { @After @@ -399,6 +399,68 @@ class SentryNavEffectLifecycleIntegrationTest { } // ViewPager2 pages + + @Test + @Config(sdk = [30]) + fun `backgrounding a view pager activity above API 29 keeps the current page's nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + + activity.pause() + activity.stop() + fixture.idle() + + assertThat(fixture.events).containsAtLeast("ViewPager:ON_PAUSE", "Home:view:ON_PAUSE").inOrder() + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + @Config(sdk = [29]) + fun `backgrounding a view pager activity at API 29 keeps the current page's nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + + activity.pause() + activity.stop() + fixture.idle() + + assertThat(fixture.events).containsAtLeast("ViewPager:ON_PAUSE", "Home:view:ON_PAUSE").inOrder() + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) + assertThat(homeTransaction.isFinished).isFalse() + } + + /** + * Below API 29, `ComponentActivity` reports that it paused from `ReportFragment` rather than from + * `onActivityPrePaused`, which must still precede its Fragments pausing. + */ + @Test + @Config(sdk = [28]) + fun `backgrounding a view pager activity below API 29 keeps the current page's nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + + activity.pause() + activity.stop() + fixture.idle() + + assertThat(fixture.events).containsAtLeast("ViewPager:ON_PAUSE", "Home:view:ON_PAUSE").inOrder() + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) + assertThat(homeTransaction.isFinished).isFalse() + } + @Test fun `a preloaded view pager page does not displace the current page's nav data and transaction`() { val fixture = Fixture.create() @@ -637,10 +699,136 @@ class SentryNavEffectLifecycleIntegrationTest { .inOrder() } - // Screens without an effect + // Nav roots without a SentryNavEffect @Test - fun `an activity without an effect keeps the outgoing activity's nav data and transaction on the scope`() { + fun `swiping to a view pager page without an effect clears the outgoing page's nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(PlainPageViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + fixture.events.clear() + + activity.get().viewPager.setCurrentItem(1, false) + fixture.idle() + + assertThat(fixture.events) + .containsExactly("Home:view:ON_PAUSE", "Plain:view:ON_RESUME") + .inOrder() + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + fun `swiping back to a view pager page from one without an effect restores its nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(PlainPageViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + activity.get().viewPager.setCurrentItem(1, false) + fixture.idle() + + activity.get().viewPager.setCurrentItem(0, false) + fixture.idle() + + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + fun `a view pager page preloaded next to a current page without an effect publishes no nav data or transaction`() { + val fixture = Fixture.create() + + Robolectric.buildActivity(PlainInitialPageViewPagerActivity::class.java).setup() + fixture.idle() + + assertThat(fixture.events) + .containsAtLeast("Plain:view:ON_RESUME", "Home:composed(CREATED)", "Home:view:ON_START") + assertThat(fixture.events).doesNotContain("Home:view:ON_RESUME") + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `swiping from a view pager page without an effect to a preloaded page publishes its nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(PlainInitialPageViewPagerActivity::class.java).setup() + fixture.idle() + + activity.get().viewPager.setCurrentItem(0, false) + fixture.idle() + + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + } + + @Test + fun `recreating a view pager activity on a page without an effect publishes no nav data or transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(PlainPageViewPagerActivity::class.java).setup() + fixture.idle() + activity.get().viewPager.setCurrentItem(1, false) + fixture.idle() + + activity.recreate() + fixture.idle() + + check(activity.get().viewPager.currentItem == 1) + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `restoring a view pager on a page without an effect after process death publishes no nav data`() { + val savedInstanceState = Bundle() + Fixture.create().run { + val activity = Robolectric.buildActivity(PlainPageViewPagerActivity::class.java).setup() + idle() + activity.get().viewPager.setCurrentItem(1, false) + idle() + activity.pause().stop().saveInstanceState(savedInstanceState).destroy() + idle() + } + + // A new process starts with a new scope and coordinator. + val fixture = Fixture.create() + val activity = + Robolectric.buildActivity(PlainPageViewPagerActivity::class.java).setup(savedInstanceState) + fixture.idle() + + check(activity.get().viewPager.currentItem == 1) + assertThat(fixture.events).contains("Home:composed(STARTED)") + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `recreating a view pager activity on a page with an effect publishes that page's nav data and transaction`() { + val fixture = Fixture.create() + val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() + fixture.idle() + activity.get().viewPager.setCurrentItem(1, false) + fixture.idle() + + activity.recreate() + fixture.idle() + + check(activity.get().viewPager.currentItem == 1) + assertThat(fixture.scope.screen).isEqualTo("/Checkout") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Checkout") + assertThat(fixture.scope.transaction?.name).isEqualTo("/Checkout") + } + + @Test + fun `an activity without an effect clears a covered activity's nav data and unbinds its unfinished transaction`() { val fixture = Fixture.create() val home = Robolectric.buildActivity(HomeActivity::class.java).setup() fixture.idle() @@ -655,6 +843,168 @@ class SentryNavEffectLifecycleIntegrationTest { assertThat(fixture.events) .containsAtLeast("Home:ON_PAUSE", "Plain:ON_RESUME", "Home:ON_STOP") .inOrder() + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + fun `an activity without an effect clears the nav data and transaction of an activity it only pauses`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + + home.pause() + Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + + assertThat(fixture.events).doesNotContain("Home:ON_STOP") + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + fun `going back to an activity covered by one without an effect restores its nav data and unfinished transaction`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + home.pause() + val plain = Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + home.stop() + fixture.idle() + + plain.pause() + home.restart() + home.resume() + fixture.idle() + plain.stop() + plain.destroy() + fixture.idle() + + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + fun `going back to an activity covered by one without an effect starts a new nav transaction if its own finished`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + home.pause() + val plain = Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + home.stop() + fixture.idle() + homeTransaction.finish() + + plain.pause() + home.restart() + home.resume() + fixture.idle() + plain.stop() + plain.destroy() + fixture.idle() + + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.transaction).isNotSameInstanceAs(homeTransaction) + assertThat(fixture.scope.transaction?.isFinished).isFalse() + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }).containsExactly("/Home") + } + + @Test + fun `navigating while covered by an activity without an effect publishes no nav data or transaction`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + home.pause() + Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + + fixture.backStack("Home") += "Profile" + fixture.idle() + + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }) + .containsExactly("/Home", "/Profile") + .inOrder() + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `going back to an activity that navigated while covered publishes the destination's nav data and transaction`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + home.pause() + val plain = Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + fixture.backStack("Home") += "Profile" + fixture.idle() + + plain.pause() + home.resume() + fixture.idle() + + assertThat(homeTransaction.isFinished).isTrue() + assertThat(fixture.scope.screen).isEqualTo("/Profile") + assertThat(fixture.scope.navigationEntries()).containsExactly("/Profile", "/Home").inOrder() + assertThat(fixture.scope.transaction?.name).isEqualTo("/Profile") + assertThat(fixture.scope.transaction?.isFinished).isFalse() + } + + @Test + fun `an activity without an effect removes a covered fragment's nav data and transaction from the scope`() { + val fixture = Fixture.create() + val host = Robolectric.buildActivity(FragmentHostActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + + host.pause() + Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + host.stop() + fixture.idle() + + assertThat(fixture.events).containsAtLeast("Home:view:ON_PAUSE", "Plain:ON_RESUME").inOrder() + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.navigationEntries()).isNull() + assertThat(fixture.scope.transaction).isNull() + assertThat(homeTransaction.isFinished).isFalse() + } + + @Test + fun `going back to a fragment covered by an activity without an effect restores its nav data and transaction`() { + val fixture = Fixture.create() + val host = Robolectric.buildActivity(FragmentHostActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + host.pause() + val plain = Robolectric.buildActivity(PlainActivity::class.java).setup() + fixture.idle() + host.stop() + fixture.idle() + + plain.pause() + host.restart() + host.resume() + fixture.idle() + plain.stop() + plain.destroy() + fixture.idle() + assertThat(fixture.scope.screen).isEqualTo("/Home") assertThat(fixture.scope.navigationEntries()).containsExactly("/Home") assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) @@ -759,10 +1109,13 @@ class SentryNavEffectLifecycleIntegrationTest { } } + /** A [NavEffectActivity] whose SentryNavEffect observes the "Home" back stack. */ internal class HomeActivity : NavEffectActivity("Home") + /** A [NavEffectActivity] whose SentryNavEffect observes the "Checkout" back stack. */ internal class CheckoutActivity : NavEffectActivity("Checkout") + /** An Activity whose content has no SentryNavEffect. */ internal class PlainActivity : FragmentActivity() { override fun onCreate(savedInstanceState: Bundle?) { @@ -772,6 +1125,7 @@ class SentryNavEffectLifecycleIntegrationTest { } } + /** An Activity with no SentryNavEffect of its own, hosting a [HomeFragment] that has one. */ internal class FragmentHostActivity : FragmentActivity() { override fun onCreate(savedInstanceState: Bundle?) { @@ -787,7 +1141,15 @@ class SentryNavEffectLifecycleIntegrationTest { } } - internal class ViewPagerActivity : FragmentActivity() { + /** + * An Activity without a SentryNavEffect of its own whose ViewPager2 pages are a [HomeFragment] + * and, by default, a [CheckoutFragment], each with a SentryNavEffect. The page at [initialPage] + * is current when the Activity is created. + */ + internal open class ViewPagerActivity( + private val createSecondPage: () -> Fragment = ::CheckoutFragment, + private val initialPage: Int = 0, + ) : FragmentActivity() { lateinit var viewPager: ViewPager2 @@ -796,23 +1158,43 @@ class SentryNavEffectLifecycleIntegrationTest { lifecycle.addObserver(Fixture.require().eventRecorder("ViewPager")) viewPager = ViewPager2(this).apply { + // An ID lets the ViewPager2 save and restore its current page and pages. + id = VIEW_PAGER_ID // Preload the page next to the current one. FragmentStateAdapter caps a preloaded page // at STARTED and resumes only the current page. offscreenPageLimit = 1 - adapter = PagesAdapter(this@ViewPagerActivity) + adapter = PagesAdapter(this@ViewPagerActivity, createSecondPage) + setCurrentItem(initialPage, false) } setContentView(viewPager) } - private class PagesAdapter(activity: FragmentActivity) : FragmentStateAdapter(activity) { + private companion object { + const val VIEW_PAGER_ID = 0x7f0b0001 + } + + private class PagesAdapter( + activity: FragmentActivity, + private val createSecondPage: () -> Fragment, + ) : FragmentStateAdapter(activity) { override fun getItemCount(): Int = 2 override fun createFragment(position: Int): Fragment = - if (position == 0) HomeFragment() else CheckoutFragment() + if (position == 0) HomeFragment() else createSecondPage() } } + /** A [ViewPagerActivity] whose second page has no SentryNavEffect. */ + internal class PlainPageViewPagerActivity : ViewPagerActivity(::PlainFragment) + + /** + * Like [PlainPageViewPagerActivity], but its current page is initially the one without a + * SentryNavEffect, e.g., as after rotating the device on that page. + */ + internal class PlainInitialPageViewPagerActivity : + ViewPagerActivity(::PlainFragment, initialPage = 1) + internal open class NavEffectFragment(private val route: String) : Fragment() { override fun onCreateView( @@ -830,6 +1212,7 @@ class SentryNavEffectLifecycleIntegrationTest { internal class CheckoutFragment : NavEffectFragment("Checkout") + /** A Fragment whose content has no SentryNavEffect. */ internal class PlainFragment : Fragment() { override fun onCreateView( @@ -927,6 +1310,8 @@ class SentryNavEffectLifecycleIntegrationTest { PlainActivity::class.java, FragmentHostActivity::class.java, ViewPagerActivity::class.java, + PlainPageViewPagerActivity::class.java, + PlainInitialPageViewPagerActivity::class.java, ) .forEach { activityClass: Class -> packageManager.addActivityIfNotPresent( diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt index 0ce82b832a6..7a0ed005f8e 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt @@ -3,11 +3,13 @@ package io.sentry.compose.navigation3 import android.app.Application import android.content.ComponentName import androidx.activity.ComponentActivity +import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner @@ -576,7 +578,7 @@ class SentryNavEffectTest { val owner = NavTestLifecycle() val options = SentryNavOptions() composeRule.setContent { - CompositionLocalProvider(LocalLifecycleOwner provides owner) { + WithoutHostActivity(owner) { SentryNavEffect( backStack = listOf("Home"), backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, @@ -605,7 +607,7 @@ class SentryNavEffectTest { val backStack = mutableStateListOf("Home") val options = SentryNavOptions() composeRule.setContent { - CompositionLocalProvider(LocalLifecycleOwner provides owner) { + WithoutHostActivity(owner) { SentryNavEffect( backStack = backStack, backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, @@ -638,11 +640,11 @@ class SentryNavEffectTest { val options = SentryNavOptions() val mapper = BackStackEntryMapper { SentryBackStackEntry(it) } composeRule.setContent { - CompositionLocalProvider(LocalLifecycleOwner provides ownerA) { + WithoutHostActivity(ownerA) { SentryNavEffect(backStackA, mapper, options, fixture.scopes, fixture.coordinator) } if (showB.value) { - CompositionLocalProvider(LocalLifecycleOwner provides ownerB) { + WithoutHostActivity(ownerB) { SentryNavEffect(listOf("Picker"), mapper, options, fixture.scopes, fixture.coordinator) } } @@ -692,11 +694,11 @@ class SentryNavEffectTest { val options = SentryNavOptions() val mapper = BackStackEntryMapper { SentryBackStackEntry(it) } composeRule.setContent { - CompositionLocalProvider(LocalLifecycleOwner provides beneath) { + WithoutHostActivity(beneath) { SentryNavEffect(listOf("Products"), mapper, options, fixture.scopes, fixture.coordinator) } if (showTranslucent.value) { - CompositionLocalProvider(LocalLifecycleOwner provides translucent) { + WithoutHostActivity(translucent) { SentryNavEffect(listOf("Checkout"), mapper, options, fixture.scopes, fixture.coordinator) } } @@ -734,7 +736,7 @@ class SentryNavEffectTest { val owner = NavTestLifecycle(Lifecycle.State.RESUMED) val options = SentryNavOptions() composeRule.setContent { - CompositionLocalProvider(LocalLifecycleOwner provides owner) { + WithoutHostActivity(owner) { SentryNavEffect( backStack = listOf("Home"), backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, @@ -766,7 +768,7 @@ class SentryNavEffectTest { val owner = mutableStateOf(originalOwner) val options = SentryNavOptions() composeRule.setContent { - CompositionLocalProvider(LocalLifecycleOwner provides owner.value) { + WithoutHostActivity(owner.value) { SentryNavEffect( backStack = listOf("Home"), backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, @@ -794,6 +796,21 @@ class SentryNavEffectTest { } } + /** + * Provides [owner] to [content] without a host Activity. Useful for tests whose owners stand in + * for Activities. Otherwise, the compose rule's Activity, which stays resumed, would be treated + * as the owner's host. Pausing the owner would then withdraw the lease, and an owner that isn't + * resumed yet couldn't acquire it on its first back stack update. + */ + @Composable + private fun WithoutHostActivity(owner: LifecycleOwner, content: @Composable () -> Unit) { + CompositionLocalProvider( + LocalLifecycleOwner provides owner, + LocalContext provides LocalContext.current.applicationContext, + content = content, + ) + } + private fun IScope.navigationBackStack(): List>? { val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt index 20acfc4078b..30a052a2b66 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt @@ -1,5 +1,8 @@ package io.sentry.compose.navigation3.lifecycle +import android.app.Activity +import android.app.Application +import android.app.Application.ActivityLifecycleCallbacks import androidx.lifecycle.Lifecycle import androidx.lifecycle.Lifecycle.State.CREATED import androidx.lifecycle.Lifecycle.State.DESTROYED @@ -8,16 +11,20 @@ import androidx.lifecycle.Lifecycle.State.STARTED import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry import com.google.common.truth.Truth.assertThat +import io.sentry.ILogger import io.sentry.IScope import io.sentry.IScopes import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryLevel import io.sentry.SentryOptions import org.junit.Assert.assertThrows import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever @Suppress("LargeClass") @@ -54,6 +61,44 @@ class NavLeaseCoordinatorTest { assertThat(second.hasLease).isTrue() } + @Test + fun `register() - registers activity lifecycle callbacks with the host activity's application`() { + val fixture = Fixture.create() + + fixture.register("owner", TestOwner(CREATED), host = fixture.hostActivity) + + assertThat(fixture.registeredActivityCallbacks).hasSize(1) + } + + @Test + fun `register() - registers activity lifecycle callbacks only once for several hosted candidates`() { + val fixture = Fixture.create() + fixture.register("first", TestOwner(CREATED), host = fixture.hostActivity) + + fixture.register("second", TestOwner(CREATED), host = fixture.coveringActivity) + + assertThat(fixture.registeredActivityCallbacks).hasSize(1) + } + + @Test + fun `register() - doesn't register activity lifecycle callbacks without a host activity`() { + val fixture = Fixture.create() + + fixture.register("owner", TestOwner(CREATED)) + + assertThat(fixture.registeredActivityCallbacks).isEmpty() + } + + @Test + fun `register() - registers activity lifecycle callbacks for a later host activity if an earlier one has no app`() { + val fixture = Fixture.create() + fixture.register("detached", TestOwner(CREATED), host = fixture.activity(application = null)) + + fixture.register("owner", TestOwner(CREATED), host = fixture.hostActivity) + + assertThat(fixture.registeredActivityCallbacks).hasSize(1) + } + // endregion // region acquiring the nav lease on the first back stack update @@ -218,6 +263,80 @@ class NavLeaseCoordinatorTest { assertThat(registration.hasLease).isTrue() } + @Test + fun `a non-resumed owner doesn't acquire the nav lease on its first update while its host activity is resumed`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val registration = fixture.register("owner", TestOwner(STARTED), host = host) + + registration.onBackStackUpdate() + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `an owner kept from the nav lease by its resumed host activity acquires it when it resumes`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val owner = TestOwner(STARTED) + val registration = fixture.acquireLease("owner", owner, host = host) + + owner.lifecycle.currentState = RESUMED + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") + } + + @Test + fun `a non-resumed owner doesn't take the nav lease from a paused leaseholder while its host activity is resumed`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val incumbentOwner = TestOwner(RESUMED) + val incumbent = fixture.acquireLease("incumbent", incumbentOwner) + incumbentOwner.lifecycle.currentState = STARTED + + fixture.acquireLease("owner", TestOwner(STARTED), host = host) + + assertThat(incumbent.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent") + } + + @Test + fun `a non-resumed owner acquires the nav lease on its first update while its host activity isn't resumed`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(STARTED).lifecycle) + val registration = fixture.register("owner", TestOwner(STARTED), host = host) + + registration.onBackStackUpdate() + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") + } + + @Test + fun `a non-resumed owner acquires the nav lease on its first update if its host activity isn't a lifecycle owner`() { + val fixture = Fixture.create() + val registration = fixture.register("owner", TestOwner(STARTED), host = fixture.hostActivity) + + registration.onBackStackUpdate() + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") + } + + @Test + fun `a resumed owner acquires the nav lease on its first back stack update while its host activity is resumed`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val registration = fixture.register("owner", TestOwner(RESUMED), host = host) + + registration.onBackStackUpdate() + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") + } + @Test fun `acquiring the nav lease unpublishes the previous publisher's nav state before publishing its own`() { val fixture = Fixture.create() @@ -622,6 +741,392 @@ class NavLeaseCoordinatorTest { // endregion + // region lease withdrawal when another Activity covers the leaseholder + + @Test + fun `another activity resuming while the leaseholder is paused withdraws the nav lease`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `another activity resuming while the leaseholder is stopped withdraws the nav lease`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = CREATED + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `another activity resuming while the leaseholder is resumed keeps the nav lease`() { + val fixture = Fixture.create() + val registration = + fixture.acquireLease("owner", TestOwner(RESUMED), host = fixture.hostActivity) + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `another activity resuming while the leaseholder is paused logs the withdrawal`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + verify(fixture.logger) + .log( + SentryLevel.DEBUG, + "Withdrew the Nav3 lease from a nav root hosted by %s: %s.", + fixture.hostActivity.javaClass.name, + "${fixture.coveringActivity.javaClass.name} was resumed while the nav root wasn't resumed", + ) + } + + @Test + fun `another activity resuming while the leaseholder is resumed logs nothing`() { + val fixture = Fixture.create() + fixture.acquireLease("owner", TestOwner(RESUMED), host = fixture.hostActivity) + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + verifyNoInteractions(fixture.logger) + } + + @Test + fun `an activity nav root resuming while the leaseholder is paused takes the nav lease without a withdrawal`() { + val fixture = Fixture.create() + val returnedTo = TestOwner(RESUMED) + val returnedToActivity = fixture.lifecycleOwnerActivity(returnedTo.lifecycle) + val returnedToRegistration = + fixture.acquireLease("returnedTo", returnedTo, host = returnedToActivity) + returnedTo.lifecycle.currentState = STARTED + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + returnedTo.lifecycle.currentState = CREATED + owner.lifecycle.currentState = STARTED + + // Android calls back onActivityResumed before dispatching ON_RESUME to the Activity's + // lifecycle. + fixture.activityCallbacks.onActivityResumed(returnedToActivity) + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).containsExactly("returnedTo") + verifyNoInteractions(fixture.logger) + + returnedTo.lifecycle.currentState = RESUMED + + assertThat(returnedToRegistration.hasLease).isTrue() + assertThat(fixture.unpublished).containsExactly("returnedTo", "owner").inOrder() + verifyNoInteractions(fixture.logger) + } + + @Test + fun `an activity nav root that hasn't applied a back stack update resuming withdraws the nav lease`() { + val fixture = Fixture.create() + val returnedTo = TestOwner(CREATED) + val returnedToActivity = fixture.lifecycleOwnerActivity(returnedTo.lifecycle) + fixture.register("returnedTo", returnedTo, host = returnedToActivity) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(returnedToActivity) + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `an activity hosting a fragment nav root resuming while the leaseholder is paused withdraws the nav lease`() { + val fixture = Fixture.create() + val activityOwner = TestOwner(RESUMED) + val hostingActivity = fixture.lifecycleOwnerActivity(activityOwner.lifecycle) + // A Fragment page that isn't resumed applies its first update without acquiring the lease. + fixture.acquireLease("page", TestOwner(STARTED), host = hostingActivity) + activityOwner.lifecycle.currentState = STARTED + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(hostingActivity) + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `the leaseholder's host activity resuming keeps the nav lease`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(fixture.hostActivity) + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `another activity resuming keeps the nav lease of a leaseholder whose host activity is unknown`() { + val fixture = Fixture.create() + fixture.register("hosted", TestOwner(CREATED), host = fixture.hostActivity) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `another activity resuming without a leaseholder doesn't unpublish any nav state`() { + val fixture = Fixture.create() + val registration = fixture.register("owner", TestOwner(RESUMED), host = fixture.hostActivity) + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `withdrawing the nav lease doesn't hand it to another paused owner`() { + val fixture = Fixture.create() + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner, host = fixture.hostActivity) + previousOwner.lifecycle.currentState = STARTED + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + assertThat(registration.hasLease).isFalse() + assertThat(previous.hasLease).isFalse() + assertThat(fixture.published).containsExactly("previous", "owner").inOrder() + } + + @Test + fun `a withdrawn owner reacquires the nav lease and republishes its nav state when it resumes`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + owner.lifecycle.currentState = RESUMED + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner", "owner") + } + + @Test + fun `an owner acquiring the nav lease after a withdrawal doesn't unpublish the withdrawn owner's nav state again`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + + val covering = + fixture.acquireLease("covering", TestOwner(CREATED), host = fixture.coveringActivity) + + assertThat(covering.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner", "covering").inOrder() + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `withdrawing the nav lease without a scope releases it but leaves its nav state published`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + + fixture.withoutScope { + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + } + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `acquiring the nav lease after a withdrawal without a scope unpublishes the withdrawn owner's nav state`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = fixture.hostActivity) + owner.lifecycle.currentState = STARTED + fixture.withoutScope { + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + } + + fixture.acquireLease("covering", TestOwner(CREATED), host = fixture.coveringActivity) + + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `if unpublishing throws during a withdrawal, the lease is released and the nav state isn't unpublished again`() { + val fixture = Fixture.create() + val failure = IllegalStateException("unpublish failed") + var unpublishAttempts = 0 + val owner = TestOwner(RESUMED) + val registration = + fixture.acquireLease( + "owner", + owner, + publisher = + fixture.publisher( + "owner", + onUnpublishFrom = { + unpublishAttempts++ + throw failure + }, + ), + host = fixture.hostActivity, + ) + owner.lifecycle.currentState = STARTED + + val thrown = + assertThrows(IllegalStateException::class.java) { + fixture.activityCallbacks.onActivityResumed(fixture.coveringActivity) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(registration.hasLease).isFalse() + + val covering = fixture.acquireLease("covering", TestOwner(CREATED)) + + assertThat(covering.hasLease).isTrue() + assertThat(unpublishAttempts).isEqualTo(1) + } + + // endregion + + // region lease withdrawal when the leaseholder pauses within a resumed Activity + + @Test + fun `pausing the leaseholder while its host activity is resumed withdraws the lease and unpublishes nav state`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = host) + + owner.lifecycle.currentState = STARTED + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + } + + @Test + fun `pausing the leaseholder while its host activity is resumed logs the withdrawal`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = host) + + owner.lifecycle.currentState = STARTED + + verify(fixture.logger) + .log( + SentryLevel.DEBUG, + "Withdrew the Nav3 lease from a nav root hosted by %s: %s.", + host.javaClass.name, + "the nav root paused while its host Activity stayed resumed", + ) + } + + @Test + fun `pausing the leaseholder while its host activity is paused logs nothing`() { + val fixture = Fixture.create() + val hostOwner = TestOwner(RESUMED) + val host = fixture.lifecycleOwnerActivity(hostOwner.lifecycle) + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = host) + hostOwner.lifecycle.currentState = STARTED + + owner.lifecycle.currentState = STARTED + + verifyNoInteractions(fixture.logger) + } + + @Test + fun `pausing the leaseholder while its host activity is paused keeps the nav lease`() { + val fixture = Fixture.create() + val hostOwner = TestOwner(RESUMED) + val host = fixture.lifecycleOwnerActivity(hostOwner.lifecycle) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = host) + hostOwner.lifecycle.currentState = STARTED + + owner.lifecycle.currentState = STARTED + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `pausing a leaseholder that observes its host activity's own lifecycle keeps the nav lease`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val host = fixture.lifecycleOwnerActivity(owner.lifecycle) + val registration = fixture.acquireLease("owner", owner, host = host) + + owner.lifecycle.currentState = STARTED + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `pausing the leaseholder keeps the nav lease if its host activity isn't a lifecycle owner`() { + val fixture = Fixture.create() + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = fixture.hostActivity) + + owner.lifecycle.currentState = STARTED + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() + } + + @Test + fun `pausing the leaseholder within its resumed host activity hands the nav lease to a resumed owner`() { + val fixture = Fixture.create() + val host = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val other = fixture.acquireLease("other", TestOwner(RESUMED), host = host) + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = host) + + owner.lifecycle.currentState = STARTED + + assertThat(other.hasLease).isTrue() + assertThat(fixture.published).containsExactly("other", "owner", "other").inOrder() + } + + // endregion + // region unregistering @Test @@ -860,13 +1365,24 @@ class NavLeaseCoordinatorTest { private class Fixture private constructor() { val navLeaseCoordinator = NavLeaseCoordinator() - val scope = Scope(SentryOptions()) + + val logger = mock() + + // Shared by scope and scopes, as in production. + private val options = + SentryOptions().apply { + setLogger(this@Fixture.logger) + isDebug = true + } + + val scope = Scope(options) /** Whether [scopes] runs `configureScope` callbacks; when false, it skips them. */ private var providesScope = true val scopes = mock().also { scopes -> + whenever(scopes.options).thenReturn(options) doAnswer { if (providesScope) { (it.arguments[0] as ScopeCallback).run(scope) @@ -903,29 +1419,67 @@ class NavLeaseCoordinatorTest { onUnregistered: (IScope?) -> Unit = { unregistered += name to it }, ): INavStatePublisher = FakeNavStatePublisher(onPublishTo, onUnpublishFrom, onUnregistered) + /** The application of [hostActivity] and [coveringActivity]. */ + val application = mock() + + /** Activity lifecycle callbacks registered with [application], in registration order. */ + val registeredActivityCallbacks = mutableListOf() + + /** The activity lifecycle callbacks the coordinator registered with [application]. */ + val activityCallbacks: ActivityLifecycleCallbacks + get() = registeredActivityCallbacks.single() + + /** An Activity of [application] that hosts candidates. */ + val hostActivity: Activity = activity(application) + + /** Another Activity of [application], which can cover [hostActivity]. */ + val coveringActivity: Activity = activity(application) + + init { + doAnswer { + registeredActivityCallbacks += it.arguments[0] as ActivityLifecycleCallbacks + null + } + .whenever(application) + .registerActivityLifecycleCallbacks(any()) + } + + /** An Activity whose application is [application], or that has none given `null`. */ + fun activity(application: Application?): Activity = + mock().also { whenever(it.application).thenReturn(application) } + + /** An Activity of [application] that's a [LifecycleOwner] of [lifecycle]. */ + fun lifecycleOwnerActivity(lifecycle: Lifecycle): Activity = + mock(extraInterfaces = arrayOf(LifecycleOwner::class)).also { + whenever(it.application).thenReturn(application) + whenever((it as LifecycleOwner).lifecycle).thenReturn(lifecycle) + } + /** * Registers a candidate with the publisher named [name] for a nav root whose lifecycle is - * [owner]'s. + * [owner]'s and whose Activity is [host]. */ fun register( name: String, owner: TestOwner, publisher: INavStatePublisher = publisher(name), + host: Activity? = null, ): NavLeaseRegistration { owners += owner - val rootInfo = NavRootInfo(owner.lifecycle, activity = null) + val rootInfo = NavRootInfo(owner.lifecycle, activity = host) return checkNotNull(navLeaseCoordinator.register(scopes, rootInfo, publisher)) } /** - * Registers a candidate for [owner]'s lifecycle and applies its first back stack update, which - * acquires the nav lease if the owner's lifecycle permits it. + * Registers a candidate for [owner]'s lifecycle, hosted by [host], and applies its first back + * stack update, which acquires the nav lease if the owner's lifecycle permits it. */ fun acquireLease( name: String, owner: TestOwner, publisher: INavStatePublisher = publisher(name), - ): NavLeaseRegistration = register(name, owner, publisher).also { it.onBackStackUpdate() } + host: Activity? = null, + ): NavLeaseRegistration = register(name, owner, publisher, host).also { it.onBackStackUpdate() } /** Runs [block] while `configureScope` skips its callbacks, e.g., as if the SDK is disabled. */ fun withoutScope(block: () -> Unit) { diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index da8b00d97a0..d06b6f470d1 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -141,6 +141,11 @@ android:exported="false" android:theme="@style/AppTheme.Main.Translucent" /> + + Unit) { modifier = Modifier.testTag(nav3TestTag("multi_fragment_current_page")), ) Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - Nav3RootFragment.pageRoots.indices.forEach { index -> + (0 until PAGER_PAGE_COUNT).forEach { index -> OutlinedButton( onClick = { onSelectPage(index) }, modifier = Modifier.testTag(nav3TestTag("multi_fragment_page_$index")), @@ -447,5 +450,7 @@ private fun PagerHeader(currentPage: Int, onSelectPage: (Int) -> Unit) { } } +private const val PAGER_PAGE_COUNT = 4 + private const val MATCH_PARENT = ViewGroup.LayoutParams.MATCH_PARENT private const val WRAP_CONTENT = ViewGroup.LayoutParams.WRAP_CONTENT diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3PlainScreens.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3PlainScreens.kt new file mode 100644 index 00000000000..6d64c7da45f --- /dev/null +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3PlainScreens.kt @@ -0,0 +1,206 @@ +package io.sentry.samples.android.navigation.nav3 + +import android.content.Intent +import android.os.Bundle +import android.util.Log +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import androidx.activity.ComponentActivity +import androidx.activity.compose.setContent +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.safeDrawingPadding +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.ViewCompositionStrategy +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.testTagsAsResourceId +import androidx.compose.ui.unit.dp +import androidx.fragment.app.Fragment +import io.sentry.ISpan +import io.sentry.Sentry +import io.sentry.samples.android.navigation.common.NavigationSampleConfigSnapshot +import io.sentry.samples.android.navigation.common.applyToCurrentOptions +import io.sentry.samples.android.navigation.common.currentNavigationSampleConfigSnapshot +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** Intentionally has no NavDisplay or SentryNavEffect; launch normally to stay in the same task. */ +class Nav3PlainActivity : ComponentActivity() { + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + // The system can restore this Activity without recreating the Nav3 Activity that set these. + NavigationSampleConfigSnapshot( + enableScreenTracking = intent.getBooleanExtra(EXTRA_ENABLE_SCREEN_TRACKING, true), + enableUserInteractionTransactions = + intent.getBooleanExtra(EXTRA_ENABLE_USER_INTERACTION_TRANSACTIONS, false), + enableUserInteractionBreadcrumbs = + intent.getBooleanExtra(EXTRA_ENABLE_USER_INTERACTION_BREADCRUMBS, false), + ) + .applyToCurrentOptions() + setContent { + Nav3SampleTheme { + PlainNav3Screen("Plain Activity", onBack = ::finish) + // Let ui.load finish normally rather than wait for the sample's TTFD timeout. + LaunchedEffect(Unit) { + reportFullyDrawn() + Sentry.reportFullyDisplayed() + } + } + } + } +} + +/** A real non-candidate pager page: no navigation integration is composed in this Fragment. */ +class Nav3PlainFragment : Fragment() { + + override fun onCreateView( + inflater: LayoutInflater, + container: ViewGroup?, + savedInstanceState: Bundle?, + ): View = + ComposeView(requireContext()).apply { + setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) + setContent { Nav3SampleTheme { PlainNav3Screen("Plain page (3)") } } + } +} + +@Composable +internal fun Nav3OpenPlainActivityButton(modifier: Modifier = Modifier) { + val context = LocalContext.current + Nav3SentryButton( + label = "Plain Activity", + onClick = { + val options = currentNavigationSampleConfigSnapshot() + context.startActivity( + Intent(context, Nav3PlainActivity::class.java) + .putExtra(EXTRA_ENABLE_SCREEN_TRACKING, options.enableScreenTracking) + .putExtra( + EXTRA_ENABLE_USER_INTERACTION_TRANSACTIONS, + options.enableUserInteractionTransactions, + ) + .putExtra( + EXTRA_ENABLE_USER_INTERACTION_BREADCRUMBS, + options.enableUserInteractionBreadcrumbs, + ) + ) + }, + modifier = modifier, + ) +} + +/** + * A screen without a SentryNavEffect, for checking manually that no nav data or nav transaction + * from a previously shown Nav3 root is attributed to it. + */ +@OptIn(ExperimentalComposeUiApi::class) +@Composable +private fun PlainNav3Screen(title: String, onBack: (() -> Unit)? = null) { + val coroutineScope = rememberCoroutineScope() + var snapshot by remember { mutableStateOf(PlainScreenScopeProbe.snapshot()) } + var result by remember { mutableStateOf("") } + LaunchedEffect(Unit) { + while (true) { + snapshot = PlainScreenScopeProbe.snapshot() + delay(SNAPSHOT_REFRESH_MILLIS) + } + } + Column( + Modifier.fillMaxSize().safeDrawingPadding().padding(16.dp).semantics { + testTagsAsResourceId = true + } + ) { + Text(title, modifier = Modifier.testTag(nav3TestTag("plain_title"))) + Text( + "This screen has no SentryNavEffect. The previous root's Navigation context should be absent." + ) + Text(snapshot, modifier = Modifier.testTag(nav3TestTag("plain_scope_state"))) + Text(result, modifier = Modifier.testTag(nav3TestTag("plain_result"))) + TextButton( + onClick = { coroutineScope.launch { result = PlainScreenScopeProbe.emitSpan() } }, + modifier = Modifier.testTag(nav3TestTag("plain_emit_span")), + ) { + Text("Emit span") + } + TextButton( + onClick = { result = PlainScreenScopeProbe.captureEvent() }, + modifier = Modifier.testTag(nav3TestTag("plain_capture_event")), + ) { + Text("Capture event") + } + if (onBack != null) { + TextButton(onClick = onBack, modifier = Modifier.testTag(nav3TestTag("plain_back"))) { + Text("Back") + } + } + } +} + +/** Reads and exercises the scope from a plain screen. Main-thread only. */ +private object PlainScreenScopeProbe { + + fun snapshot(): String { + var state = "" + Sentry.configureScope { scope -> + val transaction = scope.transaction + state = + "Transaction: ${transaction?.name} (${transaction?.operation})\n" + + "Screen: ${scope.screen}\n" + + "Navigation: ${scope.contexts["Navigation"]}" + } + return state + } + + suspend fun emitSpan(): String { + val parent = Sentry.getSpan() + val span = parent?.startChild("test.navigation.manual", "Nav3 plain screen span") + // The span must finish even if the screen disappears during the delay. + try { + delay(EMITTED_SPAN_DURATION_MILLIS) + } finally { + span?.finish() + } + return record("Emitted span; parent=${parent?.spanContext?.spanId}; ${traceUrl(parent)}") + } + + fun captureEvent(): String { + val id = Sentry.captureMessage("Nav3 plain screen event") + return record("Event: ${sentryEventSearchUrl(dsn, id.toString()) ?: id}") + } + + private fun traceUrl(transaction: ISpan?): String { + val traceId = transaction?.spanContext?.traceId?.toString() ?: return "No parent transaction" + return sentryTraceUrl(dsn, traceId) ?: "Trace: $traceId" + } + + private val dsn: String? + get() = Sentry.getCurrentScopes().options.dsn + + private fun record(message: String): String { + Log.i("Nav3PlainScreen", "$message\n${snapshot()}") + return message + } +} + +private const val SNAPSHOT_REFRESH_MILLIS = 250L +private const val EMITTED_SPAN_DURATION_MILLIS = 300L +private const val EXTRA_ENABLE_SCREEN_TRACKING = "nav3_plain.enable_screen_tracking" +private const val EXTRA_ENABLE_USER_INTERACTION_TRANSACTIONS = + "nav3_plain.enable_user_interaction_transactions" +private const val EXTRA_ENABLE_USER_INTERACTION_BREADCRUMBS = + "nav3_plain.enable_user_interaction_breadcrumbs" diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt index a67ed6d9d4e..92a7cbbd6bb 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt @@ -217,6 +217,7 @@ internal fun SentryControls( onClick = onCaptureException, modifier = Modifier.weight(1f), ) + Nav3OpenPlainActivityButton(modifier = Modifier.weight(1f)) Nav3SentryButton( label = "Crash App", onClick = onCrashApp, diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt index 70b5232c533..8dc18c45941 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt @@ -56,8 +56,8 @@ internal fun Nav3Route.routeSpec(): RouteSpec = routeName = Nav3Route.ViewPager2.routeName, title = "ViewPager2", description = - "Each page is a Fragment with its own SentryNavEffect and NavDisplay. Neighboring " + - "pages are preloaded at STARTED. Back returns to the Products Fragment underneath.", + "Pages 0–2 each have a SentryNavEffect and NavDisplay. Page 3 has no navigation " + + "integration. Neighbors are preloaded at STARTED. Back returns to Products.", ) Nav3Route.DeepLink -> RouteSpec( diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt index 21d99a50b2c..c443c9a8bd5 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt @@ -187,6 +187,22 @@ private fun sentryTransactionUrl( "×tamp=${timestampSeconds.toLong()}" } +/** The URL of trace [traceId] in the project [dsn] reports to, or `null` if `dsn` names none. */ +internal fun sentryTraceUrl(dsn: String?, traceId: String): String? { + val projectId = dsn?.projectIdFromDsn() ?: return null + return "https://$SENTRY_SAMPLE_ORG_SLUG.sentry.io/explore/traces/trace/$traceId/" + + "?project=$projectId" +} + +/** + * The URL of an issue search for event [eventId] in the project [dsn] reports to, or `null` if + * `dsn` names none. + */ +internal fun sentryEventSearchUrl(dsn: String?, eventId: String): String? { + val projectId = dsn?.projectIdFromDsn() ?: return null + return "https://$SENTRY_SAMPLE_ORG_SLUG.sentry.io/issues/?project=$projectId&query=$eventId" +} + private fun String.projectIdFromDsn(): String? = substringBefore('?').substringBefore('#').trimEnd('/').substringAfterLast('/').takeIf { projectId -> From fab87b53428e550a6c715bd19aebb17b77495ed3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 05:47:08 +0100 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b82a501d4e..d6df0298302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ ### Fixes -- Nav3: Stop attributing errors and spans from an Activity or ViewPager2 page without a `SentryNavEffect` to the most recently visited Nav3 root ([#XXXX](https://github.com/getsentry/sentry-java/pull/XXXX)) +- Nav3: Stop attributing errors and spans from an Activity or ViewPager2 page without a `SentryNavEffect` to the most recently visited Nav3 root ([#6483](https://github.com/getsentry/sentry-java/pull/6483)) ## 8.63.0 From ef09613afbc9c2ff88c94653c4a65e6c3ee288f6 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 06:00:29 +0100 Subject: [PATCH 3/4] ref(android): Fix comment wrapping in NavLeaseCoordinator --- .../compose/navigation3/lifecycle/NavLeaseCoordinator.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt index 78a20008e6c..7851d757ffd 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt @@ -300,8 +300,7 @@ internal class NavLeaseCoordinator { // Assume the resumed `activity` is covering the current leaseholder if i) the leaseholder // itself isn't resumed, ii) `activity` isn't the leaseholder's host Activity, and iii) // `activity` isn't itself a nav root (see [isActivityNavRoot]). (i) checks the leaseholder's - // own - // state rather than its host Activity's, as they differ for Fragment nav roots: a Fragment + // own state rather than its host Activity's, as they differ for Fragment nav roots: a Fragment // leaseholder that isn't resumed isn't what the user is interacting with, even if its host // Activity is resumed. val holder = leaseHolder ?: return From 274e94e688174bdee81cf6fd9d036e10df4ef85e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 12:55:16 +0100 Subject: [PATCH 4/4] fix(android): Don't hand the Nav3 lease to a sibling nav root while its Activity pauses An Activity pauses before its Fragments, so when a Fragment leaseholder paused with its host, the lease went to a sibling nav root that was about to pause too. With nav transactions restarting on return, that started a navigation transaction as the app was backgrounded or covered (e.g., a DialogFragment with its own SentryNavEffect over a Fragment nav root). --- .../lifecycle/NavLeaseCoordinator.kt | 29 +++++++++++++--- .../lifecycle/NavLeaseCoordinatorTest.kt | 34 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt index 7851d757ffd..e2791e31415 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt @@ -176,15 +176,18 @@ internal class NavLeaseCoordinator { * The eligible candidate most recently observed to be in a resumed state, other than [excluding]. * * If [resumedOnly] is set to `true`, only candidates that are resumed right now are considered; - * if `false`, started and paused candidates are also included. + * if `false`, started and paused candidates are also included. Candidates hosted by + * [excludingHostedBy] are skipped. */ private fun mostRecentlyResumedCandidate( excluding: Candidate? = null, resumedOnly: Boolean = false, + excludingHostedBy: Activity? = null, ): Candidate? = candidates .filter { it !== excluding && + (excludingHostedBy == null || it.hostActivity !== excludingHostedBy) && it.canReceiveLease && it.state.isAtLeast(Lifecycle.State.STARTED) && it.resumeOrder > 0 && @@ -212,8 +215,9 @@ internal class NavLeaseCoordinator { * Records that [candidate] paused. * * If [candidate] holds the lease when it pauses, hands it to another resumed nav root (e.g., a - * Fragment whose dialog is being dismissed). If none exists, withdraws it when the host Activity - * remains resumed; otherwise the paused nav root keeps the lease. + * Fragment whose dialog is being dismissed), unless that nav root shares [candidate]'s pausing + * host Activity. If none exists, withdraws it when the host Activity remains resumed; otherwise + * the paused nav root keeps the lease. */ private fun pauseCandidate(candidate: Candidate, scope: IScope?) { candidate.pause() @@ -222,7 +226,15 @@ internal class NavLeaseCoordinator { // window or picture-in-picture mode), and any other paused nav root is beneath it. A displaced // nav root will acquire the lease again when it resumes. if (leaseHolder === candidate) { - val next = mostRecentlyResumedCandidate(excluding = candidate, resumedOnly = true) + // An Activity pauses before its Fragments, so when the host Activity is pausing, its other + // nav roots are still resumed but about to pause too. Handing them the lease would publish + // their nav state, and start a nav transaction, just as the app is backgrounded or covered. + val next = + mostRecentlyResumedCandidate( + excluding = candidate, + resumedOnly = true, + excludingHostedBy = candidate.hostActivity.takeIf { candidate.isHostActivityPaused }, + ) when { next != null -> handOffLease(next, scope) candidate.isHostActivityResumed -> @@ -339,7 +351,14 @@ internal class NavLeaseCoordinator { /** Whether the host Activity is resumed; false when its lifecycle is unavailable. */ val isHostActivityResumed: Boolean - get() = (hostActivity as? LifecycleOwner)?.lifecycle?.currentState == Lifecycle.State.RESUMED + get() = hostActivityState == Lifecycle.State.RESUMED + + /** Whether the host Activity isn't resumed; false when its lifecycle is unavailable. */ + val isHostActivityPaused: Boolean + get() = hostActivityState.let { it != null && it != Lifecycle.State.RESUMED } + + private val hostActivityState: Lifecycle.State? + get() = (hostActivity as? LifecycleOwner)?.lifecycle?.currentState /** Most recent resume order; zero until this candidate first resumes. */ var resumeOrder: Long = initialResumeOrder diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt index 30a052a2b66..ae13a38d279 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt @@ -1125,6 +1125,40 @@ class NavLeaseCoordinatorTest { assertThat(fixture.published).containsExactly("other", "owner", "other").inOrder() } + @Test + fun `pausing the leaseholder while its host activity is paused keeps the nav lease from a resumed owner in that activity`() { + val fixture = Fixture.create() + val hostOwner = TestOwner(RESUMED) + val host = fixture.lifecycleOwnerActivity(hostOwner.lifecycle) + val other = fixture.acquireLease("other", TestOwner(RESUMED), host = host) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner, host = host) + hostOwner.lifecycle.currentState = STARTED + + owner.lifecycle.currentState = STARTED + + assertThat(registration.hasLease).isTrue() + assertThat(other.hasLease).isFalse() + assertThat(fixture.published).containsExactly("other", "owner").inOrder() + } + + @Test + fun `pausing the leaseholder while its host activity is paused hands the nav lease to a resumed owner in another activity`() { + val fixture = Fixture.create() + val otherHost = fixture.lifecycleOwnerActivity(TestOwner(RESUMED).lifecycle) + val other = fixture.acquireLease("other", TestOwner(RESUMED), host = otherHost) + val hostOwner = TestOwner(RESUMED) + val host = fixture.lifecycleOwnerActivity(hostOwner.lifecycle) + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner, host = host) + hostOwner.lifecycle.currentState = STARTED + + owner.lifecycle.currentState = STARTED + + assertThat(other.hasLease).isTrue() + assertThat(fixture.published).containsExactly("other", "owner", "other").inOrder() + } + // endregion // region unregistering