From abdba4d1d7670ad4dde3e46fd836c9e62b98e527 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 05:29:59 +0100 Subject: [PATCH] ref(android): Introduce NavRootInfo and move the Nav3 lease policy to the README Prepares the Nav3 lifecycle stack without changing behavior. SentryNavEffect now attaches a NavRootInfo (the nav root's lifecycle plus the Activity that displays it), which later lease withdrawal needs. Renames LeasedNavState to INavStatePublisher, publisher to lastPublisher, and hasAppliedInitialBackStackUpdate to hasHandledFirstUpdate/canReceiveLease, and says "nav root" rather than "owner" throughout. The lease policy (tradeoff, handoff, diagram) moves from the NavLeaseCoordinator KDoc to the package README, so later changes to the policy are reviewed as prose. --- sentry-android-navigation3/build.gradle.kts | 1 + .../compose/navigation3/BackStackObserver.kt | 14 +- .../CoordinatedBackStackObserver.kt | 31 +- .../compose/navigation3/SentryNavEffect.kt | 19 +- .../lifecycle/NavLeaseCoordinator.kt | 439 ++++++------------ .../navigation3/lifecycle/NavRootInfo.kt | 39 ++ .../navigation3/lifecycle/NavScopes.kt | 26 +- .../compose/navigation3/lifecycle/README.md | 136 +++++- .../navigation3/BackStackObserverTest.kt | 3 +- .../CoordinatedBackStackObserverTest.kt | 18 +- .../navigation3/lifecycle/FindActivityTest.kt | 56 +++ .../lifecycle/NavLeaseCoordinatorTest.kt | 46 +- .../navigation3/lifecycle/NavScopeTest.kt | 4 +- .../lifecycle/NavScopesIntegrationTest.kt | 4 +- .../navigation3/lifecycle/NavScopesTest.kt | 58 +-- 15 files changed, 507 insertions(+), 387 deletions(-) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavRootInfo.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/FindActivityTest.kt diff --git a/sentry-android-navigation3/build.gradle.kts b/sentry-android-navigation3/build.gradle.kts index 95462ad61f1..7ad51b43904 100644 --- a/sentry-android-navigation3/build.gradle.kts +++ b/sentry-android-navigation3/build.gradle.kts @@ -64,6 +64,7 @@ dependencies { implementation(libs.androidx.lifecycle.runtime.compose) compileOnly(libs.androidx.compose.runtime) + compileOnly(libs.androidx.compose.ui) testImplementation(libs.androidx.compose.ui.test.junit4) testImplementation(libs.androidx.fragment.ktx) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 2c4fd1fb14d..0becec8b705 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -33,8 +33,8 @@ private const val NAVIGATION_OP: String = "navigation" * * **Entry identity determines whether the top has changed** * - * Referential equality (===), not structural equality, is used to determine whether the top of the - * incoming back stack has changed. That approach: + * Referential equality (===) is used to determine whether the top of the incoming back stack has + * changed. That approach: * * - matches the typical Nav3 SnapshotStateList, where an entry instance has a stable identity for * its lifetime in the stack; @@ -206,21 +206,21 @@ internal class BackStackObserver( scope.rotatePropagationContext() } - storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topEntryNormalized) + storePreviousTop(currentBackStack) } private fun handleSameTop(scope: INavScope, backStack: BackStackData) { navContext.update(scope, backStack.capturedEntriesNormalized) - storeAsPreviousTop(backStack.topEntry, backStack.topEntryNormalized) + storePreviousTop(backStack) } private fun handleEmptyBackStack(scope: INavScope) { tearDown(scope) } - private fun storeAsPreviousTop(topEntry: T, topEntryNormalized: NormalizedSentryBackStackEntry) { - previousTop = WeakReference(topEntry) - previousTopNormalized = topEntryNormalized + private fun storePreviousTop(backStack: BackStackData) { + previousTop = WeakReference(backStack.topEntry) + previousTopNormalized = backStack.topEntryNormalized } private fun clearPreviousTop() { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt index a34a214decc..a01e658e938 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt @@ -1,15 +1,15 @@ package io.sentry.compose.navigation3 -import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleEventObserver import io.sentry.IScope import io.sentry.IScopes import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator +import io.sentry.compose.navigation3.lifecycle.NavRootInfo import io.sentry.compose.navigation3.lifecycle.NavScopes /** - * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with other - * `BackStackObserver`s. + * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with those of + * other `BackStackObserver`s. * * Coordination is achieved via [NavScopes]. * @@ -35,8 +35,8 @@ internal class CoordinatedBackStackObserver( 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) { @@ -52,5 +52,24 @@ internal class CoordinatedBackStackObserver( } } -/** The [NavLeaseCoordinator] shared by default by all [SentryNavEffect]s. */ +/** + * The [NavLeaseCoordinator] shared by default by all [CoordinatedBackStackObserver]s. + * + * **Lifetime** + * + * The default coordinator lives as long as the process, since lease state follows nav root + * lifecycles rather than the SDK's. Closing and re-initializing the SDK doesn't require a reset + * because: + * + * - `SentryNavEffect` resolves the current scopes on every call; + * + * - while the SDK is closed, bookkeeping continues without a scope; and + * + * - unpublishing compares by reference, so values published to an earlier scope don't affect the + * current one. + * + * Limitation: A nav transaction that's still running when the SDK is closed and re-initialized can + * be bound to the new scope the next time it's published, although its client has been closed. + * Spans then attach to it, and are lost, until it times out or the next navigation finishes it. + */ internal val defaultNavLeaseCoordinator: NavLeaseCoordinator = NavLeaseCoordinator() diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt index c666122efc7..41e2bda95cb 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt @@ -4,17 +4,19 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.ui.platform.LocalContext import androidx.lifecycle.compose.LocalLifecycleOwner import io.sentry.IScopes import io.sentry.ScopesAdapter import io.sentry.SentryOptions import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator +import io.sentry.compose.navigation3.lifecycle.NavRootInfo +import io.sentry.compose.navigation3.lifecycle.findActivity import org.jetbrains.annotations.ApiStatus /** - * An effect for generating Sentry data from your Nav3 backstack. Configure it via [options] and - * call it before you invoke your `NavDisplay`. - * + * A composable effect for generating Sentry data from your Nav3 backstack. Configure it via + * [options] and call it before you invoke your `NavDisplay`: * ```kotlin * @Composable * fun AppNavigation() { @@ -24,7 +26,7 @@ import org.jetbrains.annotations.ApiStatus * // Place SentryNavEffect in the same composable as your NavDisplay and call * // the effect first. Doing so ensures the effect's lifecycle matches your * // NavDisplay, and that any Sentry data produced by your initial nav - * // destination get attributed to the appropriate nav transaction. + * // destination are attributed to the appropriate nav transaction. * SentryNavEffect( * backStack = navBackStack, * backStackEntryMapper = { entry -> @@ -105,6 +107,7 @@ internal fun 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 SentryNavEffect( ) } - // Register first so the initial prepared back stack update can acquire navigation ownership. + // Intentionally don't key on context. It's only used to find the Activity that displays the nav + // root, which can't change without the lifecycle changing too, and that replaces the observer. + // The context's identity can change more often (e.g., if the host app provides a new + // ContextThemeWrapper on every recomposition), and re-attaching for those changes would + // needlessly finish the nav transaction. DisposableEffect(observer) { - observer.attach(lifecycle) + observer.attach(NavRootInfo(lifecycle, context.findActivity())) onDispose { observer.dispose() } } 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 15e3706ad4c..492a7c35526 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 @@ -15,205 +15,85 @@ import io.sentry.IScopes * Manages a lease in order to determine which [NavScopes] instance can bind nav transactions and * publish nav data to a shared [IScope]. * - * `NavScopes` instances register as [candidates][Candidate] for lease ownership via [register]. - * Each candidate observes its owner's lifecycle until it's disposed. + * `NavScopes` instances register as [candidates][Candidate] for lease ownership via [register], + * each on behalf of its [nav root][NavRootInfo]. Each candidate observes its nav root's lifecycle + * until it's disposed. * - * Note: "Owner" below refers to the `LifecycleOwner` whose lifecycle the candidate observes. - * - * **A fundamental tradeoff** - * - * During `LifecycleOwner` overlap, the nav transaction and scoped nav data can reflect - * **_either_**: - * - * 1. the outgoing owner that's still visible, or - * 2. the incoming owner that may crash before it becomes visible, - * - * but not both. This class opts for (2) so that the incoming destination populates crash context - * and any work performed is associated with it† – the idea being that start-up logic is more likely - * to cause a crash or produce interesting spans than an outgoing destination's steady state. - * - * † Associating work with the incoming transaction means spans produced by the outgoing owner may - * appear as if they were produced by the incoming owner (or vice versa, were our policy reversed). - * - * **Lease handoff policy** - * - * The lease is acquired preemptively by an incoming owner's `NavScopes` instance when it applies - * its first [back stack update][onBackStackUpdate].† - * - * A displaced owner remains eligible for lease restoration until it - * [stops][Lifecycle.Event.ON_STOP]. It gets the lease back when it - * [resumes][Lifecycle.Event.ON_RESUME], or sooner if the new owner leaves or pauses while the - * displaced owner is still visible. If restored, the displaced owner's nav data and unfinished - * transaction are reinstated. - * - * † There are some exceptions, e.g., the leaseholder is resumed and the incoming owner isn't (e.g., - * a preloaded ViewPager2 Fragment page next to the current one). Such an owner acquires the lease - * when it resumes instead. - * - * The following diagram illustrates how handoff works: - * ``` - * Time - * │ - * ▼ - * - * NavScopes A + Lifecycle A Shared state NavScopes B + Lifecycle B - * ───────────────────────── ──────────── ───────────────────────── - * - * attach() Navigation lease: none - * │ - * Initial back stack update - * │ - * ├── acquire lease ────────► Navigation lease: A - * ├── create & bind txA ────► Scope transaction: txA - * ├── publish nav data ─────► Published data: A - * │ - * ON_RESUME A is eligible for restoration - * │ - * ON_PAUSE - * │ - * ├────────────────────────► Navigation lease: A (no other resumed owner) - * │ A remains eligible for restoration - * │ - * │ attach() - * │ │ - * │ Navigation lease: A ◄────────────────────┤ - * │ Scope transaction: txA │ - * │ Published data: A │ - * │ B registered, awaiting initial back │ - * │ stack update │ - * │ │ - * │ Initial back stack update - * │ │ - * │ Suspend & unbind txA ◄───────────────────┤ - * │ Navigation lease: B │ - * │ Published data: B │ - * │ Scope transaction: txB │ - * │ (rollback point) │ - * │ │ - * │ ON_RESUME - * │ │ - * │ B eligible for restoration ◄─────────────┤ - * │ │ - * ON_STOP │ - * │ │ - * ├── retain txA; timers continue │ - * ├────────────────────────► A no longer eligible for restoration │ - * │ │ - * ON_DESTROY │ - * │ │ - * ├── finish retained txA if unfinished │ - * └────────────────────────► A no longer tracked │ - * ON_PAUSE - * │ - * Navigation lease: B (no other resumed ◄─┤ - * owner) │ - * │ - * ON_STOP - * │ - * Navigation lease: B (no other resumed ◄─┤ - * owner); B keeps recording navigation │ - * B no longer eligible for restoration │ - * │ - * ON_DESTROY - * │ - * Finish txB if unfinished; ◄──────────────┤ - * unbind if still bound │ - * Clear B's remaining screen/back stack │ - * Retain recorded nav events │ - * Navigation lease: none │ - * ▼ - * - * Alternative at rollback point: B disposes before A stops - * Unbind txB ◄─────────────── dispose B ───┤ - * Navigation lease: A │ - * Republish A's staged data │ - * Restore txA if unfinished and scope free │ - * Finish txB if unfinished │ - * ``` - * - * **Nav transactions follow navigation, not the lease** - * - * A candidate gets a nav transaction for a destination it navigates to, never merely for acquiring - * the lease. A destination reached without the lease gets its transaction when the lease is next - * acquired, if it can start then; otherwise it gets none. On reacquiring the lease without having - * navigated in the meantime, a candidate only gets its unfinished transaction back, if any. - * - * Hence the asymmetry for, e.g., ViewPager2 pages: the first time the user swipes to a preloaded - * page, it gets the transaction for the destination it reached while offscreen, but swiping back to - * a page whose transaction has finished starts none. + * See the package README for the lease policy this class enforces. * * **Exception policy** * - * Methods in this class with an `IScope` parameter can throw arbitrary exceptions†, which propagate - * to the caller. Each method updates its own bookkeeping before calling into the scope or a - * candidate's [LeasedNavState], so the coordinator remains usable if the exception is caught. - * Candidates call these methods from [configureScope][IScopes.configureScope] callbacks, whose - * error handling catches the exceptions. + * Methods in this class with an `IScope` parameter can throw arbitrary exceptions, which propagate + * to the caller. For instance, `IScope` can throw arbitrary exceptions when scope mutators notify + * registered observers – including the host app – in an unguarded loop, or when starting a nav + * transaction triggers the host app's callbacks. * - * † E.g., `IScope` can throw arbitrary exceptions when scope mutators notify registered observers – - * including the host app – in an unguarded loop, or when starting a nav transaction triggers the - * host app's callbacks. + * Each method updates its own bookkeeping so that the coordinator remains usable if the exception + * is caught by the caller. * * **Thread safety** * - * This class is ***not*** thread-safe. All `NavScopes` instances using this coordinator must - * confine their access to the same thread. + * This class is ***not*** thread-safe. All callers should confine their access to the same thread. */ @Suppress("TooManyFunctions") internal class NavLeaseCoordinator { + /** A list of all possible leaseholders. */ private val candidates = mutableListOf() - /** Candidate whose nav state was most recently published to the shared scope. */ - private var publisher: Candidate? = null - + /** The actual leaseholder. */ private var leaseHolder: Candidate? = null + /** The last candidate to publish nav state, even if it no longer holds the lease. */ + private var lastPublisher: Candidate? = null + /** - * Counter that orders candidates by when this coordinator most recently observed them in resumed - * state. + * Counter that lets us order candidates by when this coordinator most recently observed them in + * resumed state. */ private var resumeOrder: Long = 0 /** - * Registers a candidate for the nav lease that observes [lifecycle] until it's - * [disposed][NavLeaseRegistration.dispose] or `lifecycle` is destroyed. Returns `null` if - * `lifecycle` is already [destroyed][Lifecycle.State.DESTROYED]. + * Registers a [nav root][rootInfo] as a lease candidate until disposal or lifecycle destruction. + * + * Returns `null` if the lifecycle is already destroyed. */ fun register( - lifecycle: Lifecycle, scopes: IScopes, - navState: LeasedNavState, + rootInfo: NavRootInfo, + publisher: INavStatePublisher, ): NavLeaseRegistration? { + val lifecycle = rootInfo.lifecycle val lifecycleState = lifecycle.currentState + if (lifecycleState == Lifecycle.State.DESTROYED) { return null } val initialResumeOrder = if (lifecycleState == Lifecycle.State.RESUMED) ++resumeOrder else 0 - val candidate = Candidate(lifecycle, scopes, navState, lifecycleState, initialResumeOrder) - candidates += candidate + 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. lifecycle.addObserver(candidate) + candidates += candidate return candidate } /** - * Records that [candidate] is about to apply a back-stack update and acquires the nav lease if - * the owner's lifecycle permits it. - * - * Later calls for the same [candidate] do nothing. + * Gives [candidate] one chance to acquire the lease on its first back-stack update, and no-ops + * otherwise. */ private fun onBackStackUpdate(candidate: Candidate, scope: IScope) { - if (candidate.hasAppliedInitialBackStackUpdate) { + if (candidate.hasHandledFirstUpdate) { return } - candidate.hasAppliedInitialBackStackUpdate = true + candidate.hasHandledFirstUpdate = true val holder = leaseHolder - // An owner that isn't resumed doesn't take the lease from a resumed leaseholder, as it isn't + // 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 = @@ -226,18 +106,68 @@ internal class NavLeaseCoordinator { } } + /** Hands the lease to [next], or releases it if [scope] is unavailable. */ + private fun handOffLease(next: Candidate?, scope: IScope?) { + if (scope != null) { + transferLeaseTo(next, scope) + } else { + leaseHolder = null + } + } + + /** Hands the lease to [candidate], or releases it given `null`. */ + private fun transferLeaseTo(candidate: Candidate?, scope: IScope) { + if (leaseHolder === candidate) { + return + } + + val previous = lastPublisher + leaseHolder = candidate + if (candidate != null) { + // Record the candidate as the last publisher before calling into the scope: it owns the + // scope's nav state from the moment it takes the lease. If unpublishing the previous nav + // root's state then throws, the next handoff still unpublishes the candidate's state rather + // than never unpublishing it. + lastPublisher = candidate + if (previous != null && previous !== candidate) { + previous.publisher.unpublishFrom(scope) + } + candidate.publisher.publishTo(scope) + } + } + + /** + * 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. + */ + private fun mostRecentlyResumedCandidate( + excluding: Candidate? = null, + resumedOnly: Boolean = false, + ): Candidate? = + candidates + .filter { + it !== excluding && + it.canReceiveLease && + it.state.isAtLeast(Lifecycle.State.STARTED) && + it.resumeOrder > 0 && + (!resumedOnly || it.state == Lifecycle.State.RESUMED) + } + .maxByOrNull { it.resumeOrder } + /** * Records that [candidate] resumed. * - * Restores the lease to `candidate` if it already applied a back stack update and [scope] is - * non-null. + * Gives [candidate] the lease if its back stack has had an initial update and [scope] is + * available. */ private fun resumeCandidate(candidate: Candidate, scope: IScope?) { // A null scope (e.g., the SDK is disabled) still records the lifecycle transition so that the // candidate remains eligible for fallback once the scope becomes available. candidate.resume(++resumeOrder) - if (scope != null && candidate.hasAppliedInitialBackStackUpdate) { + if (scope != null && candidate.canReceiveLease) { transferLeaseTo(candidate, scope) } } @@ -245,15 +175,15 @@ internal class NavLeaseCoordinator { /** * Records that [candidate] paused. * - * Hands the lease to another owner only if that owner is resumed (e.g., a Fragment whose dialog - * is being dismissed); otherwise keeps it. + * 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. */ private fun pauseCandidate(candidate: Candidate, scope: IScope?) { candidate.pause() - // A paused leaseholder may still be the topmost visible owner (e.g., beneath a translucent - // window or picture-in-picture mode), and any other paused owner is beneath it. A displaced - // owner will acquire the lease again when it resumes. + // A paused leaseholder may still be the topmost visible nav root (e.g., beneath a translucent + // 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) @@ -264,93 +194,35 @@ internal class NavLeaseCoordinator { /** * Records that [candidate] stopped. * - * As with pausing, hands the lease to another owner only if that owner is resumed; otherwise the - * stopped owner keeps it. Keeping it lets a lone owner go on recording navigation that happens - * while it isn't resumed (e.g., while the app is in the background, or after it restarts but - * before it resumes), including starting nav transactions. + * If [candidate] holds the lease when it stops, hands it to another resumed nav root or keeps it + * so a lone nav root can record navigation while the app is backgrounded. */ private fun stopCandidate(candidate: Candidate, scope: IScope?) { candidate.stop() - // A paused owner doesn't get the lease. When the leaseholder stops before it, the app is - // typically going to the background (e.g., a translucent leaseholder stops before the owner - // beneath it), and that owner isn't what the user last saw. A paused owner that comes back to - // the foreground will acquire the lease when it resumes. + // A paused nav root doesn't get the lease. When the leaseholder stops before it, the app is + // typically going to the background (e.g., a translucent leaseholder stops before the nav root + // beneath it), and that nav root isn't what the user last saw. A paused nav root that comes + // back to the foreground will acquire the lease when it resumes. if (leaseHolder === candidate) { mostRecentlyResumedCandidate(resumedOnly = true)?.let { next -> handOffLease(next, scope) } } } /** - * 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 considered. - */ - private fun mostRecentlyResumedCandidate( - excluding: Candidate? = null, - resumedOnly: Boolean = false, - ): Candidate? = - candidates - .filter { - it !== excluding && - it.hasAppliedInitialBackStackUpdate && - it.state.isAtLeast(Lifecycle.State.STARTED) && - it.resumeOrder > 0 && - (!resumedOnly || it.state == Lifecycle.State.RESUMED) - } - .maxByOrNull { it.resumeOrder } - - /** - * Hands the current lease to [next]. Without a [scope], the lease is released instead, as [next] - * could not publish its staged data. - */ - private fun handOffLease(next: Candidate?, scope: IScope?) { - if (scope != null) { - transferLeaseTo(next, scope) - } else { - leaseHolder = null - } - } - - /** Hands the lease to [candidate], or releases it given `null`. */ - private fun transferLeaseTo(candidate: Candidate?, scope: IScope) { - if (leaseHolder === candidate) { - return - } - - val previousPublisher = publisher - leaseHolder = candidate - if (candidate != null) { - // Record the candidate as the publisher before calling into the scope: it owns the scope's - // nav state from the moment it takes the lease. If unpublishing the previous owner's state - // then throws, the next handoff still unpublishes the candidate's state rather than never - // unpublishing it. - publisher = candidate - if (previousPublisher != null && previousPublisher !== candidate) { - previousPublisher.navState.unpublishFrom(scope) - } - candidate.navState.publishTo(scope) - } - } - - /** - * Stops tracking [candidate] (e.g., because it was destroyed). - * - * Unpublishes `candidate`'s nav state from [scope] if it's the most recently published. If - * `candidate` holds the lease, hands it to the eligible candidate most recently observed resumed, - * even if that candidate is now only started or paused, or releases it if there's none. + * Removes [candidate], unpublishes its state if needed, and hands off its lease to the latest + * eligible candidate. Without [scope], releases the lease without unpublishing. */ private fun unregisterCandidate(candidate: Candidate, scope: IScope?) { - val wasPublisher = publisher === candidate + val wasLastPublisher = lastPublisher === candidate val wasLeaseHolder = leaseHolder === candidate // Remove candidate before any scope calls to ensure it's released even if cleanup or handoff // throws. candidates.remove(candidate) - if (wasPublisher) { - publisher = null + if (wasLastPublisher) { + lastPublisher = null } if (wasLeaseHolder) { leaseHolder = null @@ -362,8 +234,8 @@ internal class NavLeaseCoordinator { return } - if (wasPublisher) { - candidate.navState.unpublishFrom(scope) + if (wasLastPublisher) { + candidate.publisher.unpublishFrom(scope) } if (wasLeaseHolder) { transferLeaseTo(mostRecentlyResumedCandidate(), scope) @@ -371,44 +243,55 @@ internal class NavLeaseCoordinator { } /** - * A [LifecycleOwner] registered as a candidate for lease ownership. Relays its owner's lifecycle - * events to the coordinator until it's [disposed][dispose]. + * A [nav root][rootInfo] eligible to hold the lease managed by [NavLeaseCoordinator]. + * + * [Disposed][dispose] upon the nav root's destruction. */ private inner class Candidate( - private val lifecycle: Lifecycle, - private val scopes: IScopes, - val navState: LeasedNavState, + rootInfo: NavRootInfo, initialState: Lifecycle.State, initialResumeOrder: Long, + val publisher: INavStatePublisher, + private val scopes: IScopes, ) : NavLeaseRegistration, LifecycleEventObserver { - /** The candidate's lifecycle state, as last observed. */ + /** Most recent resume order; zero until this candidate first resumes. */ + var resumeOrder: Long = initialResumeOrder + private set + + /** The candidate's last observed lifecycle state. */ var state: Lifecycle.State = initialState private set + /** Whether this candidate has handled its first back-stack update. */ + var hasHandledFirstUpdate: Boolean = false + /** - * Whether the candidate has been observed STOPPED since - * [registration][NavLeaseCoordinator.register]. + * Whether the first back-stack update may take the lease: the nav root is resumed or has not + * yet resumed or stopped since registration. */ - private var wasStopped: Boolean = false + val canAcquireLeaseOnFirstUpdate: Boolean + get() = state == Lifecycle.State.RESUMED || (resumeOrder == 0L && !wasStopped) /** - * See [NavLeaseCoordinator.resumeOrder]. Non-zero once the candidate has been observed RESUMED - * since [registration][NavLeaseCoordinator.register]. + * Whether this candidate can acquire the lease on a lifecycle change, i.e., when it resumes or + * as a fallback. + * + * False until its first back stack update is handled, as it has no nav state to publish before + * then. */ - var resumeOrder: Long = initialResumeOrder - private set + val canReceiveLease: Boolean + get() = hasHandledFirstUpdate - var hasAppliedInitialBackStackUpdate: Boolean = false + override val hasLease: Boolean + get() = leaseHolder === this - /** - * Whether the candidate's lifecycle permits its first back stack update since - * [registration][NavLeaseCoordinator.register] to acquire the nav lease: it's resumed, or it - * has neither resumed nor stopped since registration (i.e., it's still coming up for the first - * time). - */ - val canAcquireLeaseOnFirstUpdate: Boolean - get() = state == Lifecycle.State.RESUMED || (resumeOrder == 0L && !wasStopped) + private val lifecycle: Lifecycle = rootInfo.lifecycle + + /** Whether this candidate has stopped since registration. */ + private var wasStopped: Boolean = false + + private var isDisposed = false fun start() { // Attaching to an already-resumed lifecycle replays ON_START before ON_RESUME, so only move @@ -432,11 +315,6 @@ internal class NavLeaseCoordinator { wasStopped = true } - private var isDisposed = false - - override val hasLease: Boolean - get() = leaseHolder === this - override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { when (event) { ON_START -> start() @@ -466,17 +344,14 @@ internal class NavLeaseCoordinator { try { unregisterCandidate(this, scope) } finally { - navState.onUnregistered(scope) + publisher.onUnregistered(scope) } } } /** - * Runs [block] with the real scope, or with `null` if [configureScope][IScopes.configureScope] - * skips its callback (e.g., the SDK is disabled), so coordinator bookkeeping still happens. - * - * A callback that ran but threw is not retried, as retrying could let its exception escape into - * the host app. + * 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) { var callbackInvoked = false @@ -502,41 +377,25 @@ internal interface NavLeaseRegistration { /** Whether the candidate holds the nav lease. */ val hasLease: Boolean - /** - * Announces that the candidate is about to apply a back stack update, so it can acquire the nav - * lease if it's eligible to. Does nothing once the candidate is unregistered. - */ + /** Gives the candidate a chance to acquire the lease before its first back-stack update. */ fun onBackStackUpdate() /** - * Stops observing the candidate's lifecycle and unregisters it: releases the lease, unpublishes - * the candidate's nav state, and then calls [LeasedNavState.onUnregistered], even if - * unregistering throws. Does nothing once the candidate is unregistered. + * Unregisters the candidate, releases its lease, and unpublishes its nav state. Calls + * [INavStatePublisher.onUnregistered] even if cleanup throws. Repeated calls do nothing. */ fun dispose() } -/** - * The nav state a [NavLeaseCoordinator] candidate publishes to the shared [IScope] while it holds - * the nav lease. - * - * Nav transactions and nav data each occupy a single, mutually-exclusive slot in the `IScope`, so - * only one candidate's nav state can be published at a time. Unpublishing nav state removes it from - * the scope but keeps it staged, so the candidate can publish it again if it regains the lease. - */ -internal interface LeasedNavState { +/** Publishes a [NavLeaseCoordinator.Candidate]'s nav state to the shared [IScope]. */ +internal interface INavStatePublisher { - /** Publishes staged nav state to [scope]. */ + /** Publishes the candidate's staged nav state to [scope]. */ fun publishTo(scope: IScope) - /** Removes published nav state from [scope], keeping it staged so it can be republished. */ + /** Removes published nav state from [scope], retaining it for a later lease acquisition. */ fun unpublishFrom(scope: IScope) - /** - * Called when the lease candidate is unregistered, after its nav state has been unpublished. - * - * The provided [scope] is the scope the candidate was unregistered against, or `null` if there - * was none (e.g., the SDK is disabled). - */ + /** Called after unregistering, with the affected [scope] or `null` if none was available. */ fun onUnregistered(scope: IScope?) } diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavRootInfo.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavRootInfo.kt new file mode 100644 index 00000000000..01f64bef0d4 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavRootInfo.kt @@ -0,0 +1,39 @@ +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 Activity that displays the nav root's content, or `null` if it's unknown. + * + * If the nav root is itself an Activity, this is the nav root. If the nav root is a Fragment, + * this is the Activity that hosts the Fragment. + */ + val activity: Activity?, +) + +/** + * The Activity the receiver belongs to, found by unwrapping [ContextWrapper]s (e.g., the + * `ContextThemeWrapper` of a dialog), or `null` if there is none. + */ +internal fun Context.findActivity(): Activity? { + var context: Context? = this + while (context is ContextWrapper) { + if (context is Activity) { + return context + } + context = context.baseContext + } + return null +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt index 37a796a58c6..8c1c6d32b7e 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.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) + * ↓ owns / 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, described by + * [rootInfo]. * - * Attaching to a second, different lifecycle no-ops. + * No-ops if the caller tries to attach more than once or if the nav root has already been + * destroyed. */ - 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 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 e003af99061..d3e1a942bdf 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 @@ -3,10 +3,6 @@ Package contains code for coordinating multiple `SentryNavEffect`s and their access to a shared `IScope`, so that overlapping effects (e.g., during Activity or Fragment transitions) don't clobber one another's screen names, nav context, or nav transactions. -See the `NavLeaseCoordinator` KDoc for the lease policy that decides which effect may write to the scope. - -## Encapsulation policy - This package is meant to encapsulate knowledge about the possibility of multiple `SentryNavEffect`s and the shared mutable state issues it raises. Addressing those concerns is complicated. Confining them here makes them easier to reason about and easier to swap for alternative logic should we someday need to. @@ -16,10 +12,6 @@ logic should we someday need to. Support for multiple `SentryNavEffect`s is restricted to the scenarios documented in the [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. - Concretely, a host app can safely use more than one `SentryNavEffect` only if: 1. each `SentryNavEffect` is called in the same composable as the `NavDisplay` it observes; @@ -28,7 +20,133 @@ Concretely, a host app can safely use more than one `SentryNavEffect` only if: not alongside one in the Activity hosting them); and 4. the user can interact with only one of those Activities or Fragments at a time. -† Meant to ensure each effect's `LocalLifecycleOwner` is an Activity or a Fragment and not a `NavDisplay`-provided owner. `NavDisplay` +> † Meant to ensure each effect's `LocalLifecycleOwner` is an Activity or a Fragment and not a `NavDisplay`-provided owner. `NavDisplay` provides its own `LocalLifecycleOwner` to the entries and scenes it renders. That owner drops to STARTED state during transitions and while overlays are shown. An effect called inside a `NavDisplay` would therefore (wrongly) gain and lose the nav lease `NavLeaseCoordinator` manages as navigation animates, rather than as its Activity or Fragment comes and goes. + +## Lease policy + +`NavLeaseCoordinator` implements the lease policy that supports the scenarios from the Navigation for Android docs. The policy assumes that, +at any moment, the user is interacting with at most one nav root in the host app, and that the nav root being interacted with is the one +that was most recently resumed. (See the `NavRootInfo` KDoc for a definition of "nav root".) + +### A fundamental tradeoff + +While nav roots overlap (e.g., during an Activity transition), the nav transaction and scoped nav data can reflect **_either_**: + +1. the outgoing nav root that's still visible, or +2. the incoming nav root that may crash before it becomes visible, + +but not both. `NavLeaseCoordinator` opts for (2) so that the incoming destination populates crash context and any work performed is +associated with it† – the idea being that start-up logic is more likely to cause a crash or produce interesting spans than an outgoing +destination's steady state. + +> † Associating work with the incoming transaction means spans produced by the outgoing nav root +may appear as if they were produced by the incoming nav root (or vice versa, were our policy +reversed). + +### Lease handoff policy + +The lease is acquired preemptively by an incoming nav root's `NavScopes` instance when it applies its first back stack update +(`onBackStackUpdate`).† + +A displaced nav root remains eligible for lease restoration until it stops (`ON_STOP`). It gets the lease back when it resumes +(`ON_RESUME`), or sooner if the new nav root leaves or pauses while the displaced nav root is still visible. If restored, the displaced nav +root's nav data and unfinished transaction are reinstated. + +> † 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 +nav root yet, so it acquires the lease when it resumes instead. + +The following diagram illustrates how handoff works: + +``` + Time + │ + ▼ + +Nav root A Shared IScope state Nav root B +────────── ─────────────────── ────────── + +attach() Navigation lease: none + │ +Initial back stack update + │ + ├── acquire lease ────────► Navigation lease: A + ├── create & bind txA ────► Scope transaction: txA + ├── publish nav data ─────► Published data: A + │ +ON_RESUME A is eligible for restoration + │ +ON_PAUSE + │ + ├────────────────────────► Navigation lease: A (no other resumed nav root) + │ A remains eligible for restoration + │ + │ attach() + │ │ + │ Navigation lease: A ◄────────────────────┤ + │ Scope transaction: txA │ + │ Published data: A │ + │ B registered, awaiting initial back │ + │ stack update │ + │ │ + │ Initial back stack update + │ │ + │ Suspend & unbind txA ◄───────────────────┤ + │ Navigation lease: B │ + │ Published data: B │ + │ Scope transaction: txB │ + │ (rollback point) │ + │ │ + │ ON_RESUME + │ │ + │ B eligible for restoration ◄─────────────┤ + │ │ +ON_STOP │ + │ │ + ├── retain txA; timers continue │ + ├────────────────────────► A no longer eligible for restoration │ + │ │ +ON_DESTROY │ + │ │ + ├── finish retained txA if unfinished │ + └────────────────────────► A no longer tracked │ + ON_PAUSE + │ + Navigation lease: B (no other resumed ◄─┤ + nav root) │ + │ + ON_STOP + │ + Navigation lease: B (no other resumed ◄─┤ + nav root); B keeps recording │ + navigation │ + B no longer eligible for restoration │ + │ + ON_DESTROY + │ + Finish txB if unfinished; ◄──────────────┤ + unbind if still bound │ + Clear B's remaining screen/back stack │ + Retain recorded nav events │ + Navigation lease: none │ + ▼ + +Alternative at rollback point: B disposes before A stops + Unbind txB ◄─────────────── dispose B ───┤ + Navigation lease: A │ + Republish A's staged data │ + Restore txA if unfinished and scope free │ + Finish txB if unfinished │ +``` + +### Nav transactions follow navigation, not the lease + +A nav root gets a nav transaction for a destination it navigates to, never merely for acquiring the lease. A destination reached without +the lease gets its transaction when the lease is next acquired, if it can start then; otherwise it gets none. On reacquiring the lease +without having navigated in the meantime, a nav root only gets its unfinished transaction back, if any. + +Hence the asymmetry for, e.g., ViewPager2 pages: the first time the user swipes to a preloaded page, it gets the transaction for the +destination it reached while offscreen, but swiping back to a page whose transaction has finished starts none. diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index d331afabe69..ef79931174f 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -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, activity = null)) // The sole resumed owner's first update acquires the lease, which it then keeps. onBackStackUpdate() } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt index cf653c23e31..dea07db5481 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.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, activity = 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, activity = null)) } /** * Owners of the lifecycles created by [resumedLifecycle]. `LifecycleRegistry` holds its owner @@ -397,8 +405,8 @@ class CoordinatedBackStackObserverTest { .lifecycle } - /** Nav state for a competing owner that has nothing to publish. */ - private object NoOpLeasedNavState : LeasedNavState { + /** Nav state publisher for a competing nav root that has nothing to publish. */ + private object NoOpNavStatePublisher : INavStatePublisher { override fun publishTo(scope: IScope) = Unit override fun unpublishFrom(scope: IScope) = Unit diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/FindActivityTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/FindActivityTest.kt new file mode 100644 index 00000000000..1c8853155d1 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/FindActivityTest.kt @@ -0,0 +1,56 @@ +package io.sentry.compose.navigation3.lifecycle + +import android.app.Activity +import android.content.Context +import android.content.ContextWrapper +import android.view.ContextThemeWrapper +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.mock +import org.robolectric.Robolectric +import org.robolectric.annotation.Config + +@RunWith(AndroidJUnit4::class) +@Config(sdk = [30]) +class FindActivityTest { + + @Test + fun `findActivity() - returns the context when the context is an activity`() { + val activity = Robolectric.buildActivity(Activity::class.java).get() + + val found = activity.findActivity() + + assertThat(found).isSameInstanceAs(activity) + } + + @Test + fun `findActivity() - returns the wrapped activity when the context wraps an activity`() { + val activity = Robolectric.buildActivity(Activity::class.java).get() + val context = ContextWrapper(ContextThemeWrapper(activity, 0)) + + val found = context.findActivity() + + assertThat(found).isSameInstanceAs(activity) + } + + @Test + fun `findActivity() - returns null when the context wraps no activity`() { + val context: Context = ApplicationProvider.getApplicationContext() + + val found = context.findActivity() + + assertThat(found).isNull() + } + + @Test + fun `findActivity() - returns null when the context isn't a context wrapper`() { + val context = mock() + + val found = context.findActivity() + + assertThat(found).isNull() + } +} 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 9a521312f68..20acfc4078b 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 @@ -32,7 +32,11 @@ class NavLeaseCoordinatorTest { val owner = TestOwner(DESTROYED) val registration = - navLeaseCoordinator.register(owner.lifecycle, fixture.scopes, fixture.navState("owner")) + navLeaseCoordinator.register( + fixture.scopes, + NavRootInfo(owner.lifecycle, activity = null), + fixture.publisher("owner"), + ) assertThat(registration).isNull() assertThat(owner.lifecycle.observerCount).isEqualTo(0) @@ -221,8 +225,8 @@ class NavLeaseCoordinatorTest { fixture.acquireLease( "previous", TestOwner(RESUMED), - navState = - fixture.navState( + publisher = + fixture.publisher( "previous", onPublishTo = { calls += "publish previous" }, onUnpublishFrom = { calls += "unpublish previous" }, @@ -232,7 +236,7 @@ class NavLeaseCoordinatorTest { fixture.register( "incoming", TestOwner(RESUMED), - fixture.navState( + fixture.publisher( "incoming", onPublishTo = { calls += "publish incoming" }, onUnpublishFrom = { calls += "unpublish incoming" }, @@ -288,7 +292,7 @@ class NavLeaseCoordinatorTest { fixture.acquireLease( "first", TestOwner(RESUMED), - navState = fixture.navState("first", onUnpublishFrom = { throw failure }), + publisher = fixture.publisher("first", onUnpublishFrom = { throw failure }), ) val second = fixture.register("second", TestOwner(RESUMED)) @@ -718,7 +722,7 @@ class NavLeaseCoordinatorTest { fixture.acquireLease( "owner", owner, - navState = fixture.navState("owner", onUnpublishFrom = { throw failure }), + publisher = fixture.publisher("owner", onUnpublishFrom = { throw failure }), ) val thrown = assertThrows(IllegalStateException::class.java) { registration.dispose() } @@ -755,8 +759,8 @@ class NavLeaseCoordinatorTest { fixture.acquireLease( "owner", TestOwner(RESUMED), - navState = - fixture.navState( + publisher = + fixture.publisher( "owner", onUnpublishFrom = { calls += "unpublish" }, onUnregistered = { calls += "unregistered" }, @@ -796,7 +800,7 @@ class NavLeaseCoordinatorTest { fixture.acquireLease( "owner", TestOwner(RESUMED), - navState = fixture.navState("owner", onUnpublishFrom = { throw failure }), + publisher = fixture.publisher("owner", onUnpublishFrom = { throw failure }), ) val thrown = assertThrows(IllegalStateException::class.java) { registration.dispose() } @@ -889,24 +893,28 @@ class NavLeaseCoordinatorTest { private val owners = mutableListOf() /** - * Nav state named [name] that, by default, records its publication in [published], its + * Publisher named [name] that, by default, records its publication in [published], its * unpublishing in [unpublished], and its unregistration in [unregistered]. */ - fun navState( + fun publisher( name: String, onPublishTo: (IScope) -> Unit = { published += name }, onUnpublishFrom: (IScope) -> Unit = { unpublished += name }, onUnregistered: (IScope?) -> Unit = { unregistered += name to it }, - ): LeasedNavState = FakeLeasedNavState(onPublishTo, onUnpublishFrom, onUnregistered) + ): INavStatePublisher = FakeNavStatePublisher(onPublishTo, onUnpublishFrom, onUnregistered) - /** Registers a candidate for [owner]'s lifecycle with the nav state named [name]. */ + /** + * Registers a candidate with the publisher named [name] for a nav root whose lifecycle is + * [owner]'s. + */ fun register( name: String, owner: TestOwner, - navState: LeasedNavState = navState(name), + publisher: INavStatePublisher = publisher(name), ): NavLeaseRegistration { owners += owner - return checkNotNull(navLeaseCoordinator.register(owner.lifecycle, scopes, navState)) + val rootInfo = NavRootInfo(owner.lifecycle, activity = null) + return checkNotNull(navLeaseCoordinator.register(scopes, rootInfo, publisher)) } /** @@ -916,8 +924,8 @@ class NavLeaseCoordinatorTest { fun acquireLease( name: String, owner: TestOwner, - navState: LeasedNavState = navState(name), - ): NavLeaseRegistration = register(name, owner, navState).also { it.onBackStackUpdate() } + publisher: INavStatePublisher = publisher(name), + ): NavLeaseRegistration = register(name, owner, publisher).also { it.onBackStackUpdate() } /** Runs [block] while `configureScope` skips its callbacks, e.g., as if the SDK is disabled. */ fun withoutScope(block: () -> Unit) { @@ -934,11 +942,11 @@ class NavLeaseCoordinatorTest { } } - private class FakeLeasedNavState( + private class FakeNavStatePublisher( private val onPublishTo: (IScope) -> Unit, private val onUnpublishFrom: (IScope) -> Unit, private val onUnregistered: (IScope?) -> Unit, - ) : LeasedNavState { + ) : INavStatePublisher { override fun publishTo(scope: IScope) = onPublishTo(scope) override fun unpublishFrom(scope: IScope) = onUnpublishFrom(scope) diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt index 57be6d68d5d..ad2c7b9ae40 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt @@ -477,7 +477,9 @@ class NavScopeTest { private fun resumedNavScopes(): NavScopes = NavScopes(scopes, coordinator).also { - it.attach(TestLifecycleOwner().also(lifecycleOwners::add).lifecycle) + it.attach( + NavRootInfo(TestLifecycleOwner().also(lifecycleOwners::add).lifecycle, activity = null) + ) } companion object { diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt index 4f01197a0f7..fe20e834ae8 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt @@ -24,11 +24,11 @@ class NavScopesIntegrationTest { val fixture = Fixture.create() val incumbentOwner = TestLifecycleOwner(Lifecycle.State.RESUMED) val incumbent = fixture.navScopes() - incumbent.attach(incumbentOwner.lifecycle) + incumbent.attach(NavRootInfo(incumbentOwner.lifecycle, activity = null)) incumbent.onBackStackUpdate() val incomingOwner = TestLifecycleOwner(Lifecycle.State.STARTED) val incoming = fixture.navScopes() - incoming.attach(incomingOwner.lifecycle) + incoming.attach(NavRootInfo(incomingOwner.lifecycle, activity = null)) incoming.onBackStackUpdate() val contextBeforeResume = fixture.scope.propagationContext val contextsAtSampling = mutableListOf() diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt index c865e8e91e6..a50ec5445c0 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt @@ -36,7 +36,7 @@ class NavScopesTest { fun `attach() - lets the first back stack update acquire the nav lease`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle()) + navScopes.attach(NavRootInfo(fixture.lifecycle(), activity = null)) navScopes.applyFirstBackStackUpdate() @@ -48,9 +48,9 @@ class NavScopesTest { val fixture = Fixture.create() val navScopes = fixture.navScopes() val lifecycle = fixture.lifecycle() - navScopes.attach(lifecycle) + navScopes.attach(NavRootInfo(lifecycle, activity = null)) - navScopes.attach(lifecycle) + navScopes.attach(NavRootInfo(lifecycle, activity = null)) assertThat(lifecycle.observerCount).isEqualTo(1) } @@ -61,9 +61,9 @@ class NavScopesTest { val navScopes = fixture.navScopes() val first = TestLifecycleOwner() val second = TestLifecycleOwner() - navScopes.attach(first.lifecycle) + navScopes.attach(NavRootInfo(first.lifecycle, activity = null)) - navScopes.attach(second.lifecycle) + navScopes.attach(NavRootInfo(second.lifecycle, activity = null)) assertThat(first.lifecycle.observerCount).isEqualTo(1) assertThat(second.lifecycle.observerCount).isEqualTo(0) @@ -75,7 +75,7 @@ class NavScopesTest { val navScopes = fixture.navScopes() val lifecycle = fixture.lifecycle(Lifecycle.State.DESTROYED) - navScopes.attach(lifecycle) + navScopes.attach(NavRootInfo(lifecycle, activity = null)) navScopes.applyFirstBackStackUpdate() assertThat(fixture.hasLease(navScopes)).isFalse() @@ -86,12 +86,12 @@ class NavScopesTest { val fixture = Fixture.create() val navScopes = fixture.navScopes() val first = fixture.lifecycle(Lifecycle.State.RESUMED) - navScopes.attach(first) + navScopes.attach(NavRootInfo(first, activity = null)) navScopes.applyFirstBackStackUpdate() first.currentState = Lifecycle.State.DESTROYED val second = fixture.lifecycle(Lifecycle.State.RESUMED) - navScopes.attach(second) + navScopes.attach(NavRootInfo(second, activity = null)) navScopes.applyFirstBackStackUpdate() assertThat(second.observerCount).isEqualTo(1) @@ -113,7 +113,7 @@ class NavScopesTest { val fixture = Fixture.create() val navScopes = fixture.navScopes() val lifecycle = fixture.lifecycle(Lifecycle.State.RESUMED) - navScopes.attach(lifecycle) + navScopes.attach(NavRootInfo(lifecycle, activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.startNavTransaction("/Home") val started = fixture.startedTransaction("/Home") @@ -129,7 +129,7 @@ class NavScopesTest { val fixture = Fixture.create() val navScopes = fixture.navScopes() val lifecycle = fixture.lifecycle(Lifecycle.State.RESUMED) - navScopes.attach(lifecycle) + navScopes.attach(NavRootInfo(lifecycle, activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.startNavTransaction("/Home") val started = fixture.startedTransaction("/Home") @@ -144,10 +144,10 @@ class NavScopesTest { fun `addBreadcrumb() - records breadcrumbs immediately without the nav lease`() { val fixture = Fixture.create() val leaseholder = fixture.navScopes() - leaseholder.attach(TestLifecycleOwner().lifecycle) + leaseholder.attach(NavRootInfo(TestLifecycleOwner().lifecycle, activity = null)) leaseholder.applyFirstBackStackUpdate() val other = fixture.navScopes() - other.attach(TestLifecycleOwner().lifecycle) + other.attach(NavRootInfo(TestLifecycleOwner().lifecycle, activity = null)) val breadcrumb = Breadcrumb() val hint = Hint() @@ -197,7 +197,7 @@ class NavScopesTest { fun `dispose() - doesn't finish the nav transaction without a scope`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.startNavTransaction("/Home") val started = fixture.startedTransaction("/Home") @@ -215,7 +215,7 @@ class NavScopesTest { navScopes.startNavTransaction("/Home") navScopes.dispose() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() assertThat(fixture.hasLease(navScopes)).isTrue() @@ -226,7 +226,7 @@ class NavScopesTest { fun `startNavTransaction() - starts and binds a nav transaction with the nav lease`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() val transaction = navScopes.startNavTransaction("/Home") @@ -240,7 +240,7 @@ class NavScopesTest { fun `startNavTransaction() - returns null when the delegate declines with the nav lease`() { val fixture = Fixture.create(declinesTransactions = true) val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() val transaction = navScopes.startNavTransaction("/Home") @@ -252,7 +252,7 @@ class NavScopesTest { fun `startNavTransaction() - logs the route when another transaction blocks the start`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() fixture.scope.transaction = mock() @@ -293,7 +293,7 @@ class NavScopesTest { fun `startNavTransaction() - starts no nav transaction after dispose`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle()) + navScopes.attach(NavRootInfo(fixture.lifecycle(), activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.dispose() @@ -306,7 +306,7 @@ class NavScopesTest { fun `finishNavTransaction() - finishes the started nav transaction`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.startNavTransaction("/Home") val started = fixture.startedTransaction("/Home") @@ -320,7 +320,7 @@ class NavScopesTest { fun `finishNavTransaction() - finishes the started nav transaction only once`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.startNavTransaction("/Home") val started = fixture.startedTransaction("/Home") @@ -335,7 +335,7 @@ class NavScopesTest { fun `finishNavTransaction() - does nothing without a nav transaction`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() val hostTransaction = mock() fixture.scope.transaction = hostTransaction @@ -350,7 +350,7 @@ class NavScopesTest { fun `a nav transaction starts and is bound after the previous one is finished`() { val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() navScopes.startNavTransaction("/Home") navScopes.finishNavTransaction() @@ -454,11 +454,11 @@ class NavScopesTest { fun `a preloaded owner's deferred transaction starts when the owner resumes`() { val fixture = Fixture.create() val current = fixture.navScopes() - current.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + current.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) current.applyFirstBackStackUpdate() val preloaded = fixture.navScopes() val preloadedLifecycle = fixture.lifecycle(Lifecycle.State.STARTED) - preloaded.attach(preloadedLifecycle) + preloaded.attach(NavRootInfo(preloadedLifecycle, activity = null)) preloaded.applyFirstBackStackUpdate() preloaded.startNavTransaction("/PageB") check(!fixture.hasLease(preloaded)) @@ -476,7 +476,7 @@ class NavScopesTest { fixture.incoming.dispose() val started = fixture.startedTransaction("/Result") val nextIncoming = fixture.navScopes() - nextIncoming.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + nextIncoming.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) nextIncoming.applyFirstBackStackUpdate() check(fixture.scope.transaction == null) @@ -493,7 +493,7 @@ class NavScopesTest { covered.startNavTransaction("/Result") fixture.incoming.dispose() val nextIncoming = fixture.navScopes() - nextIncoming.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + nextIncoming.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) nextIncoming.applyFirstBackStackUpdate() nextIncoming.dispose() @@ -614,7 +614,7 @@ class NavScopesTest { check(fixture.startedTransactions.isEmpty()) observerThrows = false val nextIncoming = fixture.navScopes() - nextIncoming.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + nextIncoming.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) nextIncoming.applyFirstBackStackUpdate() check(!fixture.hasLease(covered)) @@ -743,7 +743,7 @@ class NavScopesTest { */ fun coveredNavScopes(startedRoute: String? = null): NavScopes { val covered = navScopes() - covered.attach(lifecycle(Lifecycle.State.RESUMED)) + covered.attach(NavRootInfo(lifecycle(Lifecycle.State.RESUMED), activity = null)) covered.onBackStackUpdate() if (startedRoute != null) { covered.configureScope { @@ -751,7 +751,7 @@ class NavScopesTest { } } incoming = navScopes() - incoming.attach(lifecycle(Lifecycle.State.RESUMED)) + incoming.attach(NavRootInfo(lifecycle(Lifecycle.State.RESUMED), activity = null)) incoming.onBackStackUpdate() check(!hasLease(covered)) return covered