BraveVPNService.onDestroy: STOP_FOREGROUND_REMOVE so notification clears on stop#2695
Open
dsremo wants to merge 1 commit into
Open
BraveVPNService.onDestroy: STOP_FOREGROUND_REMOVE so notification clears on stop#2695dsremo wants to merge 1 commit into
dsremo wants to merge 1 commit into
Conversation
…EMOVE The current `stopForeground(STOP_FOREGROUND_DETACH)` only strips the service's foreground status while leaving the persistent VPN notification visible. After the user taps Stop (or the service is otherwise destroyed), the "RethinkDNS protected" notification lingers until the user swipes it away manually or the service is re-created. `STOP_FOREGROUND_REMOVE` removes the notification along with the foreground flag, which matches the actual lifecycle intent at this code point — onDestroy is the final cleanup, not a "keep showing the user we're still around" stop. Single-line change. ServiceCompat.stopForeground is API-compatible across all the minSdk range this app supports.
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.
Problem
BraveVPNService.onDestroy()currently calls:```kotlin
ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_DETACH)
```
STOP_FOREGROUND_DETACHstrips the service's foreground status while leaving the persistent notification visible. After the user taps Stop (or the service is destroyed via any other path), the "RethinkDNS protected" notification lingers until the user either manually swipes it away or the service is re-created. This is confusing UX: the user has just stopped the VPN, yet sees what looks like an "active" notification.Fix
Switch to
STOP_FOREGROUND_REMOVE:```kotlin
ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE)
```
REMOVEclears the notification along with the foreground flag — which is whatonDestroy()actually wants: this is the final cleanup, not a "keep showing the user we're still around" stop.Why this is safe
ServiceCompat.stopForegroundis the AndroidX shim that handles the API-level differences. BothSTOP_FOREGROUND_REMOVEandSTOP_FOREGROUND_DETACHare supported on all API levels this app targets.onDestroy()— the service is going away anyway. Whether the foreground notification is detached-and-left-visible vs detached-and-removed is purely a UI-cleanup choice; service lifecycle is unchanged.stopVpnAdapterand related cleanup earlier inonDestroy.Tested