Clear deprecated API usage (onBackPressed, getParcelable, launchWhen*) + up-button guard fix#55
Merged
Merged
Conversation
…chWhen*)
Tier-1 deprecation cleanup, no behavior change except a bug fix noted below:
- onBackPressed() overrides -> OnBackPressedDispatcher.addCallback (ProfileSettings,
ConfigEdit, RouteSettings, GroupSettings); AssetsActivity's redundant
onBackPressed{finish()} removed (default back already finishes).
- onSupportNavigateUp() now routes through onBackPressedDispatcher.onBackPressed() so the
action-bar up button honors the unsaved-changes guard too (previously it called finish()
directly and could silently discard edits) — fixes a real data-loss path.
- intent/bundle getParcelable<T> -> IntentCompat/BundleCompat.getParcelable(..., Class)
(ProfileSelectActivity, ConfigurationFragment) — typed, non-deprecated on API 33+.
- launchWhenStarted/launchWhenCreated -> lifecycleScope.launch (one-shot jobs: ServiceButton
delayed animation, AppListActivity loader); whenStarted -> withStarted (StatsBar).
Verified on-device: clean back finishes; back/up with unsaved changes both show the save
dialog; no crashes.
📝 WalkthroughWalkthroughMultiple Android API deprecation fixes across the app: four activities ( ChangesAndroid API Modernization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/io/nekohasekai/sagernet/widget/ServiceButton.kt`:
- Line 24: The lifecycle-aware behavior was lost when replacing
`launchWhenStarted` with plain `lifecycleScope.launch` in ServiceButton.kt,
allowing the delayed coroutine to execute and call `show()` even when the
activity is stopped. Locate the coroutine block in ServiceButton.kt that calls
`show()` and wrap the final UI mutation with `withStarted` to restore the
lifecycle gate, ensuring the UI update only happens when the activity is in the
STARTED state, consistent with the pattern already used in StatsBar.kt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4038ec58-1aa4-4104-aa6f-0afba40cbe5f
📒 Files selected for processing (10)
app/src/main/java/io/nekohasekai/sagernet/ui/AppListActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/AssetsActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.ktapp/src/main/java/io/nekohasekai/sagernet/ui/GroupSettingsActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/ProfileSelectActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/RouteSettingsActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/profile/ConfigEditActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/profile/ProfileSettingsActivity.ktapp/src/main/java/io/nekohasekai/sagernet/widget/ServiceButton.ktapp/src/main/java/io/nekohasekai/sagernet/widget/StatsBar.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/io/nekohasekai/sagernet/ui/AssetsActivity.kt
Plain lifecycleScope.launch (replacing launchWhenStarted) could run the delayed progress show() while the activity is stopped. Gate the UI mutation with withStarted to preserve the lifecycle-aware behavior (matches StatsBar).
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
1 participant
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
Tier-1 deprecation cleanup (from the modernization sweep). Clears compile-deprecation debt with
no behavior change — plus one real bug fix surfaced during review.
Changes
onBackPressed()→OnBackPressedDispatcher(API 33 deprecation): ProfileSettings,ConfigEdit, RouteSettings, GroupSettings now register an
OnBackPressedCallbackthat keeps theunsaved-changes guard. AssetsActivity's redundant
onBackPressed { finish() }removed (defaultback already finishes a leaf activity). Matches the existing
MainActivitypattern.onSupportNavigateUp()routedthrough
onBackPressedDispatcher.onBackPressed()instead of callingfinish()directly.Previously the action-bar up/close button could silently discard edits while the system back
button was protected — a real data-loss path (flagged by CodeRabbit).
getParcelable<T>→IntentCompat/BundleCompat.getParcelable(..., Class)(API 33deprecation): ProfileSelectActivity, ConfigurationFragment.
launchWhenStarted/launchWhenCreated→lifecycleScope.launchfor the one-shot jobs(ServiceButton delayed animation, AppListActivity loader);
whenStarted→withStarted(StatsBar). All one-shot, so plain launch / withStarted are the correct non-deprecated forms.
Testing
changes both show the "Changes not saved" dialog; "NO" discards and returns to the list; no
crashes.
Notes
com.github.shadowsocks.plugin...AlertDialogFragment) still usesgetParcelableon erased generic type params (Arg/Ret), where aBundleCompatmigration can't supply the required
Class<T>without changing the vendored API — left as-is.Greptile Summary
This PR clears four categories of Android API deprecation debt (API 33:
onBackPressed,getParcelable,launchWhenStarted/launchWhenCreated,whenStarted) with no behavior change, and fixes a real data-loss bug where the action-bar up button bypassed the unsaved-changes guard.onBackPressed()overrides replaced withOnBackPressedCallbackregistered inonCreate;onSupportNavigateUp()now routes throughonBackPressedDispatcher.onBackPressed()instead of callingfinish()directly, so both the system back gesture and the toolbar close/up button invoke the same unsaved-changes dialog.Intent.getParcelableExtra→IntentCompat.getParcelableExtraandBundle.getParcelable→BundleCompat.getParcelable, supplying the requiredClass<T>token for API 33+ correctness.launchWhenCreated/launchWhenStarted→lifecycleScope.launch;whenStarted→withStarted;ServiceButtonwraps the deferred UI mutation inwithStarted { }to preserve the lifecycle gate without the deprecated API.Confidence Score: 5/5
Safe to merge. All changes are mechanical API replacements with equivalent semantics, and the one behavioral change (up-button guard) is a genuine bug fix verified on-device.
Every changed code path was traced end-to-end: the OnBackPressedCallback correctly replaces the removed overrides, finish() in the !needSave() branch is safe because none of these activities use addToBackStack(), withStarted gating in ServiceButton preserves the original lifecycle intent, and the Compat Parcelable calls supply the required Class token. No new failure modes were introduced.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant UpButton as Up / Back Button participant Dispatcher as OnBackPressedDispatcher participant Callback as OnBackPressedCallback participant Dialog as UnsavedChangesDialog participant Activity Note over UpButton,Activity: NEW behavior (both paths unified) User->>UpButton: press back OR tap up button UpButton->>Dispatcher: onBackPressed() Dispatcher->>Callback: invoke() alt "needSave() == true" Callback->>Dialog: show() alt User clicks YES Dialog->>Activity: saveAndExit() else User clicks NO Dialog->>Activity: finish() else User clicks Cancel Dialog-->>User: dismiss (stay) end else "needSave() == false" Callback->>Activity: finish() end Note over UpButton,Activity: OLD behavior (bug — up button bypassed guard) User->>UpButton: tap up button UpButton->>Activity: onSupportNavigateUp() Activity->>Activity: finish() guard skipped!%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant UpButton as Up / Back Button participant Dispatcher as OnBackPressedDispatcher participant Callback as OnBackPressedCallback participant Dialog as UnsavedChangesDialog participant Activity Note over UpButton,Activity: NEW behavior (both paths unified) User->>UpButton: press back OR tap up button UpButton->>Dispatcher: onBackPressed() Dispatcher->>Callback: invoke() alt "needSave() == true" Callback->>Dialog: show() alt User clicks YES Dialog->>Activity: saveAndExit() else User clicks NO Dialog->>Activity: finish() else User clicks Cancel Dialog-->>User: dismiss (stay) end else "needSave() == false" Callback->>Activity: finish() end Note over UpButton,Activity: OLD behavior (bug — up button bypassed guard) User->>UpButton: tap up button UpButton->>Activity: onSupportNavigateUp() Activity->>Activity: finish() guard skipped!Comments Outside Diff (1)
app/src/main/java/io/nekohasekai/sagernet/widget/ServiceButton.kt, line 65-73 (link)launchWhenStartedwould pause the delay coroutine when the lifecycle drops below STARTED (e.g., screen off mid-connect), then resume it when the lifecycle returns to STARTED. With plainlaunch, thedelay()ticks down regardless of lifecycle state, soisIndeterminate = true; show()can be called on the progress indicator while the activity is stopped. In most cases the brief (< 2 s) window and thedelayedAnimation?.cancel()call on state changes make this harmless, but if the user backgrounds the app during the connecting state and the delay expires while stopped, the progress indicator would attempt to show on an invisible view. Consider usinglifecycleScope.launch { withStarted { ... } }as a drop-in that re-attaches the lifecycle gate without the deprecated API.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (2): Last reviewed commit: "review: keep STARTED gate on delayed con..." | Re-trigger Greptile