From 16e0f1b4c46a8a89422ad7673604b18a051878b6 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Mon, 9 May 2016 16:24:48 -0700 Subject: [PATCH] Fixed connectivity state in some power saving scenarios. NetworkPolicyManagerService (NPMS) manages 4 type of network restriction when apps are running on background: - Data Saver Mode (data usage restriction on metered-networks) - Battery Saver Mode (power restriction on all networks) - Doze Mode (power restriction on all networks) - App Idle (power restriction on all networks) These restrictions affects 2 parts of the system: - Internal framework state on NPMS which is propagated to other internal classes. - External firewall rules (managed by netd). Although each of the power-related restrictions have their own external firewall rules, internally apps are whitelisted to them through the same whitelist, and the current code is only updating the internal state (and notifying the internal listeners) when Battery Saver Mode is on. As a consequence of this problem, there are scenarios where an app correctly does not have internet access (because the firewall rules are properly set), but the NetworkInfo state returns the wrong state (like CONNECTED / CONNECTED). This CL fixes this problem by splitting the power-related logic from updateRulesForRestrictBackgroundLocked() into its own method (updateRulesForPowerRestrictionsLocked()), and making sure such method is called whenever the firewall rules are updated. Externally to this change, the CTS tests were also improved to verify the apps get the proper connection state; it can be verified by running: cts-tradefed run commandAndExit cts -m CtsHostsideNetworkTests \ -t com.android.cts.net.HostsideRestrictBackgroundNetworkTests BUG: 28521946 Change-Id: Id5187eb7a59c549ef30e2b17627ae2d734afa789 --- .../android/server/ConnectivityService.java | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 8763b93f31..53b29427b2 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -33,6 +33,7 @@ import static android.net.NetworkPolicyManager.RULE_ALLOW_METERED; import static android.net.NetworkPolicyManager.MASK_METERED_NETWORKS; import static android.net.NetworkPolicyManager.MASK_ALL_NETWORKS; import static android.net.NetworkPolicyManager.RULE_NONE; +import static android.net.NetworkPolicyManager.RULE_REJECT_ALL; import static android.net.NetworkPolicyManager.RULE_REJECT_METERED; import static android.net.NetworkPolicyManager.RULE_TEMPORARY_ALLOW_METERED; import static android.net.NetworkPolicyManager.uidRulesToString; @@ -218,9 +219,6 @@ public class ConnectivityService extends IConnectivityManager.Stub /** Flag indicating if background data is restricted. */ @GuardedBy("mRulesLock") private boolean mRestrictBackground; - /** Flag indicating if background data is restricted due to battery savings. */ - @GuardedBy("mRulesLock") - private boolean mRestrictPower; final private Context mContext; private int mNetworkPreference; @@ -669,7 +667,6 @@ public class ConnectivityService extends IConnectivityManager.Stub try { mPolicyManager.setConnectivityListener(mPolicyListener); mRestrictBackground = mPolicyManager.getRestrictBackground(); - mRestrictPower = mPolicyManager.getRestrictPower(); } catch (RemoteException e) { // ouch, no rules updates means some processes may never get network loge("unable to register INetworkPolicyListener" + e); @@ -942,13 +939,11 @@ public class ConnectivityService extends IConnectivityManager.Stub + ": " + allowed); } } - // ...then Battery Saver Mode. - if (allowed && mRestrictPower) { - allowed = (uidRules & RULE_ALLOW_ALL) != 0; + // ...then power restrictions. + if (allowed) { + allowed = (uidRules & RULE_REJECT_ALL) == 0; if (LOGD_RULES) Log.d(TAG, "allowed status for uid " + uid + " when" - + " mRestrictPower=" + mRestrictPower - + ", whitelisted=" + ((uidRules & RULE_ALLOW_ALL) != 0) - + ": " + allowed); + + " rule is " + uidRulesToString(uidRules) + ": " + allowed); } return !allowed; } @@ -1400,7 +1395,11 @@ public class ConnectivityService extends IConnectivityManager.Stub final int oldRules = mUidRules.get(uid, RULE_NONE); if (oldRules == uidRules) return; - mUidRules.put(uid, uidRules); + if (uidRules == RULE_NONE) { + mUidRules.delete(uid); + } else { + mUidRules.put(uid, uidRules); + } } // TODO: notify UID when it has requested targeted updates @@ -1438,18 +1437,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - @Override - public void onRestrictPowerChanged(boolean restrictPower) { - // caller is NPMS, since we only register with them - if (LOGD_RULES) { - log("onRestrictPowerChanged(restrictPower=" + restrictPower + ")"); - } - - synchronized (mRulesLock) { - mRestrictPower = restrictPower; - } - } - @Override public void onRestrictBackgroundWhitelistChanged(int uid, boolean whitelisted) { if (LOGD_RULES) { @@ -1891,10 +1878,6 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.println(mRestrictBackground); pw.println(); - pw.print("Restrict power: "); - pw.println(mRestrictPower); - pw.println(); - pw.println("Status for known UIDs:"); pw.increaseIndent(); final int size = mUidRules.size();