Consolidate system-app settings into one "Monitor system apps" toggle - #656
Merged
Conversation
Route+monitor+visibility for system apps were controlled by three interacting prefs: include_system_vpn (VPN routing), manage_system (tracker detection/blocking), and show_system (list/insights visibility). The user saw two switches in settings plus a filter menu. Present a single user-facing "Monitor system apps" toggle backed by include_system_vpn; it keeps manage_system and show_system in lock-step so routing, detection and visibility move together. The summary keeps the documented trade-off: off is battery-friendly, on conflicts with Android's "Block connections without VPN". Migration (ReceiverAutostart, version 2026071202): preserve the existing VPN-routing choice exactly and derive the other two internal prefs from include_system_vpn, so an install that routed system apps now also monitors them; installs that excluded them are unchanged. Part of #653 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Review findings fixed in Two cases contradicted the promised single, safe toggle:
|
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.
Part of #653
UX simplification
System-app handling was spread across three interacting preferences:
include_system_vpnServiceSinkhole.getBuilder/getBlockingBuilder(addDisallowedApplicationwhen!includeSystem && rule.system)manage_systemServiceSinkhole.shouldTrackApp(viacachedManageSystem),ActivityMain(menu/hint),ActivitySettingsshow_systemRule.getRules,ActivityMainfilter menu,InsightsDataProviderThe user saw two switches in Settings (
include_system_vpn+ its dependentmanage_system) plus a separate overflow-menu filter that flippedshow_system. The three were already partly cascaded, but the model was hard to reason about.This PR presents one user-facing toggle, "Monitor system apps", backed by
include_system_vpn. When it changes,ActivitySettingswritesmanage_systemandshow_systemto match, so routing, detection and visibility always move together. All existing read sites are untouched — they keep reading the derived prefs, which minimizes risk. Themanage_systemswitch is removed frompreferences.xml. The Apps overflow menu's separate "system apps" filter item is also removed (inActivityMain), since visibility now follows the single toggle and a second control there could desynchroniseshow_system; the "user apps" filter item remains.The toggle summary preserves and documents the trade-off: off is battery-friendly (system-app background traffic no longer wakes the VPN packet threads), on conflicts with Android's "Block connections without VPN", which must be disabled.
Migration rule
ReceiverAutostart.upgradegains a block foroldVersion < 2026071202that collapses the three prefs into the single toggle without silently enabling blocking:monitorSystem = include_system_vpn && manage_system. A user who routed system apps but had deliberately leftmanage_systemoff is not opted into tracker detection/blocking.include_system_vpn,manage_system,show_system) are then written tomonitorSystem, so routing, detection and visibility stay coherent;show_useris forced back on when the toggle ends up off.oldVersion < 2026010401) are handled before the batchededitoris committed: the earliermanage_system → include_system_vpnmigration only writes to the pending editor, so this block re-derivesinclude_system_vpnfrommanage_systemdirectly to avoid reading a stale committed value.Net effect: installs that both routed and managed system apps keep monitoring them; every other install ends up with the toggle off. A user who routed-without-managing loses system-app routing — that is the intended coherent semantics of a single toggle (routing without detection is no longer expressible), and is the only behavior change, documented here rather than applied silently.
Base
values/strings.xmlonly (setting_monitor_system/summary_monitor_system); translation dirs left to Crowdin. No per-flavourpreferences.xmloverrides exist (github/fdroid/playshare the main one). Orphanedsetting_system/summary_systemstrings are left in place to avoid churn.Measurement plan (wakeups, not throughput)
The battery cost of including system apps is wakeup frequency, not tun MB/s: background traffic from Play Services/Store/carrier apps traverses the tun and wakes the packet threads (with WG egress up, TC absorbs radio-wakeup blame roughly every ~2.5 min). Measure it as follows, holding everything else fixed (same device, same WG/DoH state, screen off, same duration):
wakeupapevents attributed to the TC uid per hour, plus wakelock time; MB/s is explicitly not the metric.read()on the tun fd) and accumulate loop time, logged through the existing stats/log path — gives a device-independent signal that isolates system-app traffic. Not added in this PR to keep it Java-only and low-risk; specified here so it can land separately if the batterystats signal is noisy.Expected result: ON shows materially more TC-attributed wakeups/wakelock time per hour than OFF, confirming routing frequency (not tunnel volume) as the cost — which is exactly why the toggle defaults off and the summary flags the battery/"Block connections without VPN" trade-off.
Verification
./gradlew :app:compileGithubDebugJavaWithJavac -q— passes./gradlew :app:testGithubDebugUnitTest— BUILD SUCCESSFUL🤖 Generated with Claude Code