Notification: gate changes. - #9087
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review comments remain, and the changes include coverage for the updated coordination behavior.
Review effort: Lite
Findings: None
What changed in this PR
Improves VPN startup synchronization for duplicate and conflicting start/stop requests, including explicit rejection handling and foreground-service cleanup.
Changes:
- Added rejection reasons and callbacks to
VpnStartGate. - Updated VPN service cleanup for rejected starts.
- Reported foreground promotion status from notifications.
- Added coordination and rejection tests.
| File | Description |
|---|---|
android/app/src/test/kotlin/org/getlantern/lantern/service/VpnStartGateTest.kt |
Tests duplicate and conflicting start rejection behavior. |
android/app/src/main/kotlin/org/getlantern/lantern/service/VpnStartGate.kt |
Adds rejection reasons and callback handling. |
android/app/src/main/kotlin/org/getlantern/lantern/service/LanternVpnService.kt |
Cleans up foreground state after rejected starts. |
android/app/src/main/kotlin/org/getlantern/lantern/notification/NotificationManager.kt |
Reports whether foreground promotion occurred. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
LGTM! |
* fix(android): serialize VPN startup preparation * code review updates * code review updates * code review updates * code review updates * Notification: gate changes. (#9087) Co-authored-by: atavism <atavism@users.noreply.github.com> * code review updates --------- Co-authored-by: jigar-f <132374182+jigar-f@users.noreply.github.com>
This pull request enhances the VPN startup synchronization logic to better handle duplicate and conflicting start/stop requests. It introduces explicit rejection reasons for denied VPN start attempts, allows custom handling of these rejections, and ensures proper cleanup of notifications and service state. The update also adds comprehensive tests to verify the new behavior.
Key changes include:
VPN Start/Stop Coordination Improvements:
Rejectionenum inVpnStartGateto explicitly indicate why a VPN start was rejected (eitherSTART_IN_PROGRESSorSTOPPING), and updated therunmethod to accept anonRejectedcallback for custom rejection handling. [1] [2]LanternVpnServiceto track if the service was promoted to the foreground during a start attempt, and to properly stop the notification and service if the attempt is later rejected due to a stop in progress.Notification Handling:
NotificationHelper.showStartingVPNConnectedNotificationto return a Boolean indicating if the service was promoted, allowing callers to react appropriately when a start is rejected.Testing Enhancements:
VpnStartGateTestto verify that duplicate and conflicting start attempts are rejected with the correct reasons, and that theonRejectedcallback is invoked as expected.These changes collectively improve the reliability and clarity of VPN service management, especially in edge cases involving rapid or overlapping start/stop requests.