Repository navigation
fix(android): Nav3 support for ViewPager2 - #6398
Open
0xadam-brown wants to merge 1 commit into
Open
0xadam-brown wants to merge 1 commit into
0xadam-brown wants to merge 1 commit into
Conversation
📲 Install BuildsAndroid
|
0xadam-brown
marked this pull request as ready for review
October 27, 2026 23:20
0xadam-brown
requested review from
a team,
markushi and
runningcode
and removed request for
a team
November 3, 2026 08:32
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fyi, I wrote the PR description below by hand. It's beefy, but it has some useful background if you want it. (No obligation. It's helpful for my future reference either way.)
PR Stack (Stage Nav3 Transactions)
📜 Description
Makes the multi-Activity and multi-Fragment support introduced by the #6301 PR stack play nicely with ViewPager2.
If you want the tl;dr, start at the "Solution" section below.
💡 Motivation and Context
Nothing is free: Supporting multi-Activity and multi-Fragment architectures
The #6301 PR stack implemented multi-Activity and multi-Fragment support for our key Nav3 integration API,
SentryNavEffect, and theBackStackObserverclass that supports it. (BackStackObserver consumes updates to host app's back stack and translates them into nav data + nav transactions.)Explicit multi-Activity/-Fragment support was needed because the initial implementation of SentryNavEffect assumed there would be one and only one effect active at a given time.** That's true for single-Activity architectures, but not for multi-Activity or multi-Fragment ones in which each Activity or Fragment has its own nav subgraph and therefore its own SentryNavEffect. (E.g., if Activity A and Activity B each have their own SentryNavEffect, those effects will be briefly active at the same time when the user navigates from Activity A to B, or vice versa.) Absent the changes in #6301, multi-Activity / - Fragment architectures could generate stale or misleading nav data.
** We assumed that only one SentryNavEffect would be active at a time in the interest of releasing core Nav3 support more quickly, and we documented the limitation instead. The Sentry Android SDK has a long-standing policy of sharing a single
IScopesinstance throughout the SDK with a single underlyingIScope, which is used to accumulate Sentry data and hold the currently active transaction. Assuming SentryNavEffect had exclusive access to that IScope simplified our initial implementation considerably. (Changing that policy was well outside the bounds of our Nav3 work, of course.) #6301 was all about breaking that assumption.Almost there: The original approach to multi-Activity / -Fragment support
The trick of #6301 was to ensure that only one SentryNavEffect at a time could write to the shared IScope instance. It did so by replacing the IScopes object used by BackStackObserver with a new
NavScopesfacade that lets us swizzle the destination of nav data updates made by BackStackObserver. Updates to the underlyingIScopewere published directly whenever a given SentryNavEffect <> BackStackObserver <> NavScopes tuple had an exclusive lease; otherwise, they were shunted to a staging area.Staged updates were published later if the SentryNavEffect tuple eventually acquired the lease. The lease could be acquired whenever both i) the back stack tracked by the SentryNavEffect was initialized or updated, and ii) the lease wasn't currently held by another SentryNavEffect tuple in a resumed / visible state. (If one or both conditions failed, the requesting SentryNavEffect would be denied the lease and would continue to write to its staging area. Note, as well, that in most cases satisfying both conditions implies that the requesting SentryNavEffect is itself resumed / visible. That'll matter for the discussion below.)
Combined, those^^ updates let #6301 separate navigation timing (in the form of host back stack updates) from publication of nav data. And that, in turn, saved us from having to drop nav data or let it become stale as the user switched between Activities or Fragments with their own nav subgraphs. Hooray.
The next hurdle: ViewPager2
...but it turns out we'd neglected an important use case, viz., ViewPager2.
The approach in #6301 was smart about staging nav data (ie, screen name + nav context), but it didn't do the same for nav transactions. A new nav transaction could be started only if the lease was owned at the time of navigation, where "navigation" == initialization or modification of the nav back stack. Because the lease had to be owned, that meant that the Activity or Fragment hosting the SentryNavEffect requesting the lease had to be in a resumed state.
That assumption made sense for most use cases, because usually initialization of an Activity or Fragment's nav back stack coincides with the Activity or Fragment being visible and therefore being able to preempt the lease from the previous leaseholder. But the assumption didn't pan out for ViewPager2, because ViewPager2 preloads neighboring pages. That means it initializes the nav back stack for Fragments that aren't in a resumed state and therefore can't preempt the lease, causing them to drop their initial nav transaction and any spans created by the initial destination screen. Sigh.
More details about ViewPager2
Unlike most Activity and Fragment architectures, ViewPager2 creates neighboring pages before a user navigates to them (and therefore before they become visible). When ViewPager2 creates a page, it populates the page's SentryNavEffect with an initial nav back stack. (Note that each page gets its own nav back stack, and there's no nav back stack that tracks movements between the ViewPager2 pages.)
.
Pre-population of a page's nav back stack is first chance SentryNavEffect has to try to claim the lease and start a nav transaction. If the page is currently visible, the lease will be acquired, the nav transaction will be published, and all will be well. But if the page is pre-loaded before it's visible, the initial attempt to acquire the lease will fail and the nav transaction will be dropped (at least under the approach from #6301). The user's later swiping to the pre-loaded page won't modify that page's back stack, and so no additional request will be made to start a nav transaction. Oof.
The solution
The solution this PR stack implements is to create a mechanism similar to the data staging from #6301, but for transactions. Doing so lets us separate back stack initialization observed during preloading vs attempted lease acquisition when a ViewPager2 page becomes visible.
In particular, the new
StagedNavTransactionclass tracks requests to start nav transactions when a SentryNavEffect is created even if it doesn't currently have the lease. If the lease is later acquired and the transaction slot is available on the shared IScope, the staged transaction is started, which keeps ViewPager2 happy.Before and after
Returning to a previously active page follows the same rule before and after this PR stack:
A swipe alone creates no new navigation request. The stack lets a swipe fulfill a request recorded earlier, during preloading or navigation without the lease.
As shown in the final diagram above, this PR stack implements an approach that starts a nav transaction the first time the user swipes to a ViewPager2 page, but not upon swiping back between already visited pages. (Note: Previous transactions are kept in a idling state, so they can be resumed, but if they expire they won't be restarted.)**
That's contrary to what I had originally proposed here:
In the end I deviated b/c:
ui.loadtransactions;ui.loadtransactions, we should consider whether we're okay diverging for all Activities and Fragments and not just in the case of ViewPager2.If folks prefer, we could consider an across-the-board policy that starts a new transaction whenever the SentryNavEffect root changes (and not just when SentryNavEffect's back stack is initialized or updated). The details are tracked under #6454.
** This isn't as bad as it sounds! First, if the ViewPager destroys a previously visited page, then a new transaction will be started for it the next time the user swipes back to it. Second, usually each ViewPager2 page will be the root of a composable nav subgraph. So long as the user navigates inside that page, new transactions will be started. It's just when swiping the ViewPager between already created pages that no transaction is restarted, since the ViewPager2 is bringing existing Fragments back into resumed state without modifying their nav back stacks.
More details
The BackStackObserver associated with each SentryNavEffect ensures that only nav back stack updates trigger requests to start nav transactions. Creating an Activity or Fragment (or ViewPager2 Fragment) produces a nav back stack "update" when the nav back stack is initialized; resuming a non-resumed Activity or Fragment does not. That leads to two issues:Both #6301 and this PR stack re-publish nav data when an existing Activity or Fragment is resumed so that IScope data doesn't reflect the stale outgoing Activity or Fragment's nav destination. Both also rebind the incoming Activity's or Fragment's transaction. If that transaction hasn't timed out, any spans created by the incoming Activity or Fragment will be parented under the resumed nav transaction.
The question the "across-the-board policy" mentioned above raises is whether we should create a new transaction instead of rebinding the incoming Activity or Fragment's existing transaction. The latter may have expired, whereas the former is guaranteed to be fresh. The downside of a new transaction is that it creates a discontinuous span context.
addresses: JAVA-785
💚 How did you test it?
See the stack PRs.
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
#skip-changelog