From ec595b5b38a692c4a72b6ec3e7dd25c3d4d86b49 Mon Sep 17 00:00:00 2001 From: Tommy Webb Date: Thu, 20 Feb 2025 21:39:15 +0000 Subject: [PATCH] fixup! Fix background data clobbering other policies * Rename variables/arguments to clarify that we are working with policy flags, meaning that there can be multiple policies described by a uid policy, not just one. * When checking policies for a policy flag, do a proper flag check, rather than a direct comparison that assumes one single policy. * Only alter the displayed setting for the "Background network access" and "Unrestricted mobile data usage" toggles in response to underlying policy changes, to ensure their state represents the current reality. * Add a couple explanatory comments. Test: Manual: Install an app that does not have INTERNET permission. Open the "Unrestricted mobile data" page of Settings. Try to activate the toggle for such an app. The toggle should activate successfully without needing to be tapped twice. Issue: calyxos#2547 Change-Id: I9f2f028be4a21158a68c60982253d85586f60cdb --- .../settings/datausage/AppDataUsage.java | 8 +-- .../settings/datausage/DataSaverBackend.java | 53 ++++++++++--------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/com/android/settings/datausage/AppDataUsage.java b/src/com/android/settings/datausage/AppDataUsage.java index 3fecfb52830..26141e7e9ce 100644 --- a/src/com/android/settings/datausage/AppDataUsage.java +++ b/src/com/android/settings/datausage/AppDataUsage.java @@ -244,27 +244,23 @@ public class AppDataUsage extends DataUsageBaseFragment implements OnPreferenceC if (preference == mRestrictBackground) { mDataSaverBackend.setIsDenylisted(mAppItem.key, mPackageName, !(Boolean) newValue); updatePrefs(); - return true; } else if (preference == mRestrictAll) { setAppRestrictAll(!(Boolean) newValue); updatePrefs(); - return true; } else if (preference == mRestrictCellular) { setAppRestrictCellular(!(Boolean) newValue); updatePrefs(); - return true; } else if (preference == mRestrictVpn) { setAppRestrictVpn(!(Boolean) newValue); updatePrefs(); - return true; } else if (preference == mRestrictWifi) { setAppRestrictWifi(!(Boolean) newValue); updatePrefs(); - return true; } else if (preference == mUnrestrictedData) { mDataSaverBackend.setIsAllowlisted(mAppItem.key, mPackageName, (Boolean) newValue); - return true; + updatePrefs(); } + // updatePrefs() will alter toggle states based on the actual underlying value. return false; } diff --git a/src/com/android/settings/datausage/DataSaverBackend.java b/src/com/android/settings/datausage/DataSaverBackend.java index 4f731c71cde..cd2f466175e 100644 --- a/src/com/android/settings/datausage/DataSaverBackend.java +++ b/src/com/android/settings/datausage/DataSaverBackend.java @@ -122,57 +122,62 @@ public class DataSaverBackend { mPolicyManager.removeUidPolicy(uid, POLICY_ALLOW_METERED_BACKGROUND); } - private void loadUidPolicies(int policy) { - final int[] uidsWithPolicyArray = mPolicyManager.getUidsWithPolicy(policy); - final ArrayList uidsWithPolicy = new ArrayList<>(uidsWithPolicyArray.length); + private void loadUidPolicies(int policyFlag) { + final int[] uidsWithPolicyArray = mPolicyManager.getUidsWithPolicy(policyFlag); + final ArrayList uidsWithPolicyFlag = new ArrayList<>(uidsWithPolicyArray.length); + // Convert from int[] to a list of Integer. for (final int uid : uidsWithPolicyArray) { - uidsWithPolicy.add(uid); + uidsWithPolicyFlag.add(uid); } // Update existing cached UID policies. for (int i = 0; i < mUidPolicies.size(); i++) { final Integer cachedEntryUid = mUidPolicies.keyAt(i); - if (uidsWithPolicy.remove(cachedEntryUid)) { + if (uidsWithPolicyFlag.remove(cachedEntryUid)) { // UID had the policy. It was removed so we don't have to process it twice. - setCachedUidPolicyFlagAt(i, policy, true); + setCachedUidPolicyFlagAt(i, policyFlag, true); } else { // UID does not have the policy. - setCachedUidPolicyFlagAt(i, policy, false); + setCachedUidPolicyFlagAt(i, policyFlag, false); } } // Add policies for remaining UIDs, which did not have cached policies, so we're it. - for (final int uid : uidsWithPolicy) { - mUidPolicies.put(uid, policy); + for (final int uid : uidsWithPolicyFlag) { + mUidPolicies.put(uid, policyFlag); } } - private void setCachedUidPolicyFlag(int uid, int policy, boolean add) { + private void setCachedUidPolicyFlag(int uid, int policyFlag, boolean add) { final int index = mUidPolicies.indexOfKey(uid); if (index < 0) { if (add) { - mUidPolicies.put(uid, policy); + mUidPolicies.put(uid, policyFlag); } return; } - setCachedUidPolicyFlagAt(index, policy, add); + setCachedUidPolicyFlagAt(index, policyFlag, add); } - private void setCachedUidPolicyFlagAt(int index, int policy, boolean add) { + private void setCachedUidPolicyFlagAt(int index, int policyFlag, boolean add) { final int currentPolicy = mUidPolicies.valueAt(index); - final int newPolicy = add ? (currentPolicy | policy) : (currentPolicy & ~policy); + final int newPolicy = add ? (currentPolicy | policyFlag) : (currentPolicy & ~policyFlag); mUidPolicies.setValueAt(index, newPolicy); } - private void setUidPolicyFlag(int uid, int policy, boolean add) { + private void setUidPolicyFlag(int uid, int policyFlag, boolean add) { if (add) { - mPolicyManager.addUidPolicy(uid, policy); + mPolicyManager.addUidPolicy(uid, policyFlag); } else { - mPolicyManager.removeUidPolicy(uid, policy); + mPolicyManager.removeUidPolicy(uid, policyFlag); } - setCachedUidPolicyFlag(uid, policy, add); + setCachedUidPolicyFlag(uid, policyFlag, add); } - private boolean isUidPolicyFlagSet(int uid, int policy) { - return (mUidPolicies.get(uid, POLICY_NONE) & policy) == policy; + private boolean isUidPolicyFlagSet(int uid, int policyFlag) { + return isFlagSet(mUidPolicies.get(uid, POLICY_NONE), policyFlag); + } + + private static boolean isFlagSet(int overallPolicy, int policyFlag) { + return (overallPolicy & policyFlag) == policyFlag; } public boolean isDenylisted(int uid) { @@ -220,10 +225,10 @@ public class DataSaverBackend { mUidPolicies.put(uid, newPolicy); } - final boolean wasAllowlisted = oldPolicy == POLICY_ALLOW_METERED_BACKGROUND; - final boolean wasDenylisted = oldPolicy == POLICY_REJECT_METERED_BACKGROUND; - final boolean isAllowlisted = newPolicy == POLICY_ALLOW_METERED_BACKGROUND; - final boolean isDenylisted = newPolicy == POLICY_REJECT_METERED_BACKGROUND; + final boolean wasAllowlisted = isFlagSet(oldPolicy, POLICY_ALLOW_METERED_BACKGROUND); + final boolean wasDenylisted = isFlagSet(oldPolicy, POLICY_REJECT_METERED_BACKGROUND); + final boolean isAllowlisted = isFlagSet(newPolicy, POLICY_ALLOW_METERED_BACKGROUND); + final boolean isDenylisted = isFlagSet(newPolicy, POLICY_REJECT_METERED_BACKGROUND); if (wasAllowlisted != isAllowlisted) { handleAllowlistChanged(uid, isAllowlisted);