Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 781ba14a authored by Felipe Leme's avatar Felipe Leme
Browse files

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
parent 18e9f13b

core/java/android/net/INetworkPolicyListener.aidl

+0 −1
Original line number Diff line number Diff line
@@ -23,6 +23,5 @@ oneway interface INetworkPolicyListener { 
    void onMeteredIfacesChanged(in String[] meteredIfaces);

    void onRestrictBackgroundChanged(boolean restrictBackground);

    void onRestrictBackgroundWhitelistChanged(int uid, boolean whitelisted);

    void onRestrictPowerChanged(boolean restrictPower);



}

core/java/android/net/INetworkPolicyManager.aidl

+0 −1
Original line number Diff line number Diff line
@@ -54,7 +54,6 @@ interface INetworkPolicyManager { 
    /** Control if background data is restricted system-wide. */

    void setRestrictBackground(boolean restrictBackground);

    boolean getRestrictBackground();

    boolean getRestrictPower();



    /** Callback used to change internal state on tethering */

    void onTetheringChanged(String iface, boolean tethering);

packages/SystemUI/src/com/android/systemui/statusbar/policy/DataSaverController.java

+0 −4
Original line number Diff line number Diff line
@@ -90,10 +90,6 @@ public class DataSaverController { 
            });

        }



        @Override

        public void onRestrictPowerChanged(boolean restrictPower) {

        }



        @Override

        public void onRestrictBackgroundWhitelistChanged(int uid, boolean whitelisted) {

        }

services/core/java/com/android/server/ConnectivityService.java

+10 −27
Original line number Diff line number Diff line
@@ -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,8 +1395,12 @@ public class ConnectivityService extends IConnectivityManager.Stub 
                final int oldRules = mUidRules.get(uid, RULE_NONE);

                if (oldRules == uidRules) return;



                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();

services/core/java/com/android/server/job/controllers/ConnectivityController.java

+0 −5
Original line number Diff line number Diff line
@@ -169,11 +169,6 @@ public class ConnectivityController extends StateController implements 
            updateTrackedJobs(-1);

        }



        @Override

        public void onRestrictPowerChanged(boolean restrictPower) {

            updateTrackedJobs(-1);

        }



        @Override

        public void onRestrictBackgroundChanged(boolean restrictBackground) {

            updateTrackedJobs(-1);