Fix/improve deep link race conditions#2998
Open
StylianosGakis wants to merge 8 commits into
Open
Conversation
Clarifies that this back-stack entry point handles in-app link taps (Compose LocalUriHandler) which join the current task, distinct from external/notification deep links. No behavior change.
External App Links and notification taps were reusing the in-app uri handler, which appends onto the live stack and never gated on auth readiness. Concurrent with SessionReconciler's setLoggedIn, this raced: the deep-linked screen sometimes landed alone, sometimes on top of Home, so system Back went nondeterministically to the app's Home or back out to the launching app. Add ExternalDeepLinkHandler, which waits for the ready signal, then lands a matched key (or a Home fallback for an unmatched own-host uri) alone via the new BackstackController.navigateToExternalDeepLink. The Home fallback matches our autoVerify'd hosts by host and path prefix, mirroring the manifest filters; foreign hosts are ignored rather than opened.
NavDisplay rendered the seeded LoginKey root for a few frames before SessionReconciler flipped it to the authenticated root. The OS splash hid this on a normal cold start, but an App Link launched into another app's task shows no splash, briefly exposing the login screen. Gate NavDisplay on the reconciler's ready signal and make reconcile() latch isReady idempotently (a config-change re-entry is now a no-op rather than dipping isReady back to false and blanking the content).
Captures the empirical finding that onNewIntent fires on a standard launchMode Activity only when a caller sets FLAG_ACTIVITY_SINGLE_TOP, so MainActivity's addOnNewIntentListener is a defensive safety net rather than a regularly-exercised path today.
ExternalDeepLinkHandler had no Compose dependency — it backed no CompositionLocal and only the deepLinkChannel collector used it. It was in HedvigApp only because the collector began life as a LaunchedEffect. Build it as a per-Activity collaborator in MainActivity (alongside androidAppHost) and collect the channel in a lifecycleScope job, next to SessionReconciler's — the per-Activity nav/auth coordinator it mirrors. HedvigApp no longer takes deepLinkChannel. The handler and its test are untouched; the UNLIMITED channel still buffers links until collection starts, so behavior is unchanged.
Comments should describe the current code, not a removed predecessor a new reader never saw. Remove the "Replaces Nav2's ... NavController" sentence from the deepLinkChannel doc and the "(Nav2 parity)" aside from navigateToInAppLink; both surrounding docs already explain the behavior in present terms.
Same cleanup as the prior commit, for comments that predate this branch: the BackstackController class doc no longer recounts the old rememberSerializable/app-singleton designs (it now justifies the retained ViewModel on its own terms), currentDestination drops the Nav2 comparison, and SessionReconciler stops noting it once lived as loose effects.
…ink-race-conditions
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.
This is an attempt to fix the race conditions around deep links by making sure reconcilation happens first, and then if a deep link is pending, it is handled appropriately, meaning taking over the entire backstack if required.
Need to test a bit more before merge
AI description of the reason the new timing corodingation which works well compared to letting the coroutines race each other: