Skip to content

ref(android): [Nav3 support for ViewPager2 – Part 1] Refactor .navigation3.lifecycle ahead of staging Nav3 transactions - #6399

Merged
0xadam-brown merged 1 commit into
0xadam-brown/collection/nav3-deferred-transactionsfrom
0xadam-brown/ref/nav3-lease-registration
Nov 4, 2026
Merged

ref(android): [Nav3 support for ViewPager2 – Part 1] Refactor .navigation3.lifecycle ahead of staging Nav3 transactions#6399
0xadam-brown merged 1 commit into
0xadam-brown/collection/nav3-deferred-transactionsfrom
0xadam-brown/ref/nav3-lease-registration

Conversation

@0xadam-brown

@0xadam-brown 0xadam-brown commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

PR Stack (Nav3 Deferred Transactions)

📜 Description

Refactors .navigation3.lifecycle ahead of staging nav transactions in the next PR.

Lets us separate three responsibilities:

  1. Lease coordination: NavLeaseCoordinator observes registered owners’ lifecycles and decides which SentryNavEffect may publish navigation state.
  2. State publication: Each effect’s NavScopes publishes and withdraws its navigation data and transaction together when ownership changes.
  3. Transaction management: Each effect gets a transaction handle that manages starting, finishing, binding, and restoring its transaction. The coordinator no longer retains transactions itself.

Behavior is unchanged save for one corner case: when a SentryNavEffect regains the lease, its unfinished transaction is now also re-bound if the scope holds a finished transaction. Before, it was only re-bound into an empty scope. The update matches the rule for starting a transaction, and in practice shouldn't occur, since a finished SentryTracer removes itself from the scope.

See the PR description in #6398 for more details.

💡 Motivation and Context

Splits out structural refactoring from the meaningful behavioral changes which follow.

addresses: JAVA-785

💚 How did you test it?

  • Unit tests
  • Had my clanker run thorough manual tests against the Nav3 sample app before and after.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

The next PR stages nav transactions requested without the nav lease.

#skip-changelog

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

@0xadam-brown @claude
ref(android): Give each SentryNavEffect a lease registration and a tr…
509777e 
…ansaction handle

NavLeaseCoordinator now observes each candidate's lifecycle itself, via
register(), and publishes and unpublishes its nav state through
LeasedNavState. NavScopes folds the old NavScope into an inner class
and publishes its StagedNavData and nav transaction through a single
publisher.

Each NavScopes instance now keeps its nav transaction in a
StagedNavTransaction handle, which INavScope.startNavTransaction and
finishNavTransaction drive, rather than BackStackObserver binding and
unbinding transactions itself and the coordinator retaining them.
StagedNavTransaction alone decides whether the scope's transaction slot
is free.

No behavior change: nav transactions still start only with the nav
lease and a free slot, and an owner that regains the lease still gets
its unfinished transaction back. The integration tests are unchanged.
This prepares for deferring transactions requested without the lease.

Refs JAVA-785
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 27, 2026
Merged
Merged
@sentry

sentry Bot commented Oct 27, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.62.0 (1) release

⚙️ sentry-android Build Distribution Settings

@0xadam-brown 0xadam-brown changed the title ref(android): [Nav3 Deferred Transactions 1] Give each SentryNavEffect a lease registration and a transaction handle ref(android): [Staged Nav3 Transactions 1] Give each SentryNavEffect a lease registration and a transaction handle Oct 27, 2026
@0xadam-brown 0xadam-brown added the sanity-check PR needs a lightweight review for obvious issues label Oct 27, 2026
@0xadam-brown
0xadam-brown marked this pull request as ready for review October 27, 2026 23:20
@0xadam-brown
0xadam-brown requested review from a team and adinauer as code owners October 27, 2026 23:20
@0xadam-brown 0xadam-brown changed the title ref(android): [Staged Nav3 Transactions 1] Give each SentryNavEffect a lease registration and a transaction handle ref(android): [Nav3 support for ViewPager2 1] Give each SentryNavEffect a lease registration and a transaction handle Nov 3, 2026
@0xadam-brown 0xadam-brown changed the title ref(android): [Nav3 support for ViewPager2 1] Give each SentryNavEffect a lease registration and a transaction handle ref(android): [Nav3 support for ViewPager2: Part 1] Give each SentryNavEffect a lease registration and a transaction handle Nov 3, 2026
@0xadam-brown 0xadam-brown changed the title ref(android): [Nav3 support for ViewPager2: Part 1] Give each SentryNavEffect a lease registration and a transaction handle ref(android): [Nav3 support for ViewPager2 – Part 1] Give each SentryNavEffect a lease registration and a transaction handle Nov 3, 2026
runningcode
runningcode approved these changes Nov 3, 2026
View reviewed changes

@runningcode runningcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together along with the docs and diagrams!

This was a long one so I'll just note that I didn't review the tests.

My only thought is that it seems like a lot of code for just adding ViewPager2 support :O

Comment thread ...d-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt Resolved
Comment thread ...d-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt Resolved
Comment thread ...d-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt Resolved
@0xadam-brown

0xadam-brown commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Thanks for putting this together along with the docs and diagrams!

This was a long one so I'll just note that I didn't review the tests.

My only thought is that it seems like a lot of code for just adding ViewPager2 support :O

Thanks for taking a look!

Yeah, it's a lot of code. Fwiw, this PR will pay off w/r/t the next PR in this stack. This one's a refactor that gives us better abstractions and prevents the next PR's actual changes from being more or less indecipherable. ("Indecipherable" is where my clankers' various attempts started until I led the best prototype through the refactorings here.)

@0xadam-brown
0xadam-brown merged commit 0d30809 into 0xadam-brown/collection/nav3-deferred-transactions Nov 4, 2026
43 of 44 checks passed
@0xadam-brown
0xadam-brown deleted the 0xadam-brown/ref/nav3-lease-registration branch November 4, 2026 08:26
@0xadam-brown 0xadam-brown changed the title ref(android): [Nav3 support for ViewPager2 – Part 1] Give each SentryNavEffect a lease registration and a transaction handle ref(android): [Nav3 support for ViewPager2 – Part 1] Refactor .navigation3.lifecycle ahead of staging Nav3 transactions Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@runningcode runningcode runningcode approved these changes
@adinauer adinauer Awaiting requested review from adinauer

Assignees

No one assigned

Labels

sanity-check PR needs a lightweight review for obvious issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@0xadam-brown @runningcode