fix(navigation): Defer onNewIntent navCtrl call until Compose attaches#587
Merged
Conversation
Move consumeUpgradeExtra() out of MainActivity.onNewIntent and into a Compose-level LaunchedEffect that collects a new MainActivity.warmIntents SingleEventFlow.
onNewIntent could fire before the setContent{} lambda registered the back stack with NavigationController, leaving navCtrl.goTo(Nav.Main.Upgrade) to throw IllegalStateException("NavigationController not initialized"). Same race shipped a fix for in octi (d4rken-org/octi#284).
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.
Summary
Latent twin of d4rken-org/octi#284 — same
NavigationControllerdesign (@Singleton+ nullable_backStack+ stricterror("...not initialized")), same ComposesetContent { }registration pattern, same race surface.MainActivity.onNewIntentcallsconsumeUpgradeExtra(intent), which in turn callsnavCtrl.goTo(Nav.Main.Upgrade). IfonNewIntentfires before thesetContent { }lambda has run (e.g. during activity recreation from a config change, orsingleTaskwarm intent delivery on a slow device),_backStackis stillnulland thegoTothrowsIllegalStateException.This PR mirrors the fix shipped to Octi: defer the navigation step until a Compose-side collector picks it up.
Changes
MainActivity.warmIntents: SingleEventFlow<Intent>fieldonNewIntentnow just callssetIntent(intent)andwarmIntents.tryEmit(intent)— nonavCtrlinteractionLaunchedEffect(Unit)insidesetContentextends to alsowarmIntents.collect { newIntent -> consumeUpgradeExtra(newIntent) }after handling the cold-start intentNavigationControlleris unchanged — strict contract preservedThe race window is tight (it requires
onNewIntentto fire betweensetContentreturning and the Compose lambda running), so this is a latent crash with no Play Store report yet — but the same code path crashed Octi on Android 16 Beta. Fixing here pre-emptively.Test plan
./gradlew :app:testFossDebugUnitTestpasses./gradlew :app:assembleFossDebugcompiles cleanlyEXTRA_NAVIGATE_TO_UPGRADE(e.g. viaEXTRA_UPGRADE_FOR_RESULTflow or the link that triggers it) — confirm Upgrade screen still opens