Merge "Add restriction grace period for sensitive process-states" into main

3 files changed, 202 insertions, 30 deletions

diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index a26ac615ba40..b1faa7fe60bd 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -28,6 +28,7 @@ import static android.Manifest.permission.READ_PHONE_STATE;
 import static android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_POWER_RESTRICTED_NETWORK;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_USER_RESTRICTED_NETWORK;
+import static android.app.ActivityManager.PROCESS_STATE_LAST_ACTIVITY;
 import static android.app.ActivityManager.PROCESS_STATE_UNKNOWN;
 import static android.app.ActivityManager.isProcStateConsideredInteraction;
 import static android.app.ActivityManager.printCapabilitiesSummary;
@@ -523,6 +524,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
      */
     private boolean mUseMeteredFirewallChains;
 
+    /**
+     * Whether or not sensitive process states and non-sensitive process-states have different
+     * delays before network is blocked after transitioning to background.
+     */
+    private boolean mUseDifferentDelaysForBackgroundChain;
+
     // See main javadoc for instructions on how to use these locks.
     final Object mUidRulesFirstLock = new Object();
     final Object mNetworkPoliciesSecondLock = new Object();
@@ -552,10 +559,43 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
      * {@link NetworkPolicyManager#BACKGROUND_THRESHOLD_STATE} will lose network access.
      * The delay is meant to prevent churn due to quick process-state changes.
      * Note that there is no delay while granting network access.
+     *
+     * This is only used when the flag {@link #mUseDifferentDelaysForBackgroundChain} is disabled.
      */
     @VisibleForTesting
     long mBackgroundRestrictionDelayMs = TimeUnit.SECONDS.toMillis(5);
 
+    /**
+     * Short delay after which a uid going into a process state having priority equal to
+     * {@link NetworkPolicyManager#BACKGROUND_THRESHOLD_STATE} or lower will lose network access.
+     *
+     * This will apply to apps that should be fine with losing network access immediately.
+     * It is only meant as a debounce to prevent churn due to quick process-state changes.
+     * Note that there is no delay while granting network access.
+     *
+     * This is only used when the flag {@link #mUseDifferentDelaysForBackgroundChain} is enabled.
+     */
+    @VisibleForTesting
+    long mBackgroundRestrictionShortDelayMs = TimeUnit.SECONDS.toMillis(2);
+
+    /**
+     * Long delay after which a uid going into a process state having priority equal to
+     * {@link NetworkPolicyManager#BACKGROUND_THRESHOLD_STATE} or lower will lose network access.
+     *
+     * Unlike {@link #mBackgroundRestrictionShortDelayMs}, this is meant to be applied to apps
+     * in sensitive proc-states like {@link ActivityManager#PROCESS_STATE_TOP_SLEEPING} and
+     * {@link ActivityManager#PROCESS_STATE_LAST_ACTIVITY}, where the user may switch to this app
+     * before this period and any latency in granting network access before resuming app activities
+     * may degrade experience.
+     *
+     * This is only used when the flag {@link #mUseDifferentDelaysForBackgroundChain} is enabled.
+     */
+    @VisibleForTesting
+    long mBackgroundRestrictionLongDelayMs = TimeUnit.SECONDS.toMillis(20);
+
+    @GuardedBy("mUidRulesFirstLock")
+    private long mNextProcessBackgroundUidsTime = Long.MAX_VALUE;
+
     /** Defined network policies. */
     @GuardedBy("mNetworkPoliciesSecondLock")
     final ArrayMap<NetworkTemplate, NetworkPolicy> mNetworkPolicy = new ArrayMap<>();
@@ -1007,6 +1047,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             mActivityManagerInternal = LocalServices.getService(ActivityManagerInternal.class);
 
             mUseMeteredFirewallChains = Flags.useMeteredFirewallChains();
+            mUseDifferentDelaysForBackgroundChain = Flags.useDifferentDelaysForBackgroundChain();
 
             synchronized (mUidRulesFirstLock) {
                 synchronized (mNetworkPoliciesSecondLock) {
@@ -1241,11 +1282,21 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 // different chains may change.
                 return true;
             }
-            if (mBackgroundNetworkRestricted && (previousProcState >= BACKGROUND_THRESHOLD_STATE)
+            if (mBackgroundNetworkRestricted) {
+                if ((previousProcState >= BACKGROUND_THRESHOLD_STATE)
                     != (newProcState >= BACKGROUND_THRESHOLD_STATE)) {
-                // Proc-state change crossed BACKGROUND_THRESHOLD_STATE: Network rules for the
-                // BACKGROUND chain may change.
-                return true;
+                    // Proc-state change crossed BACKGROUND_THRESHOLD_STATE: The network rules will
+                    // need to be re-evaluated for the background chain.
+                    return true;
+                }
+                if (mUseDifferentDelaysForBackgroundChain
+                        && newProcState >= BACKGROUND_THRESHOLD_STATE
+                        && getBackgroundTransitioningDelay(newProcState)
+                        < getBackgroundTransitioningDelay(previousProcState)) {
+                    // The old and new proc-state both are in the blocked state but the background
+                    // transition delay is reduced, so we may have to update the rules sooner.
+                    return true;
+                }
             }
             final int networkCapabilities = PROCESS_CAPABILITY_POWER_RESTRICTED_NETWORK
                     | PROCESS_CAPABILITY_USER_RESTRICTED_NETWORK;
@@ -4045,6 +4096,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                         + mBackgroundNetworkRestricted);
                 fout.println(Flags.FLAG_USE_METERED_FIREWALL_CHAINS + ": "
                         + mUseMeteredFirewallChains);
+                fout.println(Flags.FLAG_USE_DIFFERENT_DELAYS_FOR_BACKGROUND_CHAIN + ": "
+                        + mUseDifferentDelaysForBackgroundChain);
 
                 fout.println();
                 fout.println("mRestrictBackgroundLowPowerMode: " + mRestrictBackgroundLowPowerMode);
@@ -4188,20 +4241,34 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                     fout.decreaseIndent();
                 }
 
-                size = mBackgroundTransitioningUids.size();
-                if (size > 0) {
-                    final long nowUptime = SystemClock.uptimeMillis();
-                    fout.println("Uids transitioning to background:");
-                    fout.increaseIndent();
-                    for (int i = 0; i < size; i++) {
-                        fout.print("UID=");
-                        fout.print(mBackgroundTransitioningUids.keyAt(i));
-                        fout.print(", ");
-                        TimeUtils.formatDuration(mBackgroundTransitioningUids.valueAt(i), nowUptime,
-                                fout);
+                if (mBackgroundNetworkRestricted) {
+                    fout.println();
+                    if (mUseDifferentDelaysForBackgroundChain) {
+                        fout.print("Background restrictions short delay: ");
+                        TimeUtils.formatDuration(mBackgroundRestrictionShortDelayMs, fout);
+                        fout.println();
+
+                        fout.print("Background restrictions long delay: ");
+                        TimeUtils.formatDuration(mBackgroundRestrictionLongDelayMs, fout);
                         fout.println();
                     }
-                    fout.decreaseIndent();
+
+                    size = mBackgroundTransitioningUids.size();
+                    if (size > 0) {
+                        final long nowUptime = SystemClock.uptimeMillis();
+                        fout.println("Uids transitioning to background:");
+                        fout.increaseIndent();
+                        for (int i = 0; i < size; i++) {
+                            fout.print("UID=");
+                            fout.print(mBackgroundTransitioningUids.keyAt(i));
+                            fout.print(", ");
+                            TimeUtils.formatDuration(mBackgroundTransitioningUids.valueAt(i),
+                                    nowUptime, fout);
+                            fout.println();
+                        }
+                        fout.decreaseIndent();
+                    }
+                    fout.println();
                 }
 
                 final SparseBooleanArray knownUids = new SparseBooleanArray();
@@ -4331,6 +4398,15 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 || isProcStateAllowedNetworkWhileBackground(mUidState.get(uid));
     }
 
+    private long getBackgroundTransitioningDelay(int procState) {
+        if (mUseDifferentDelaysForBackgroundChain) {
+            return procState <= PROCESS_STATE_LAST_ACTIVITY ? mBackgroundRestrictionLongDelayMs
+                    : mBackgroundRestrictionShortDelayMs;
+        } else {
+            return mBackgroundRestrictionDelayMs;
+        }
+    }
+
     /**
      * Process state of UID changed; if needed, will trigger
      * {@link #updateRulesForDataUsageRestrictionsUL(int)} and
@@ -4381,19 +4457,41 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                         mBackgroundTransitioningUids.delete(uid);
                         updateRuleForBackgroundUL(uid);
                         updatePowerRestrictionRules = true;
-                    } else if (wasAllowed && !isAllowed) {
+                    } else if (!isAllowed) {
+                        final int transitionIdx = mBackgroundTransitioningUids.indexOfKey(uid);
                         final long completionTimeMs = SystemClock.uptimeMillis()
-                                + mBackgroundRestrictionDelayMs;
-                        if (mBackgroundTransitioningUids.indexOfKey(uid) < 0) {
-                            // This is just a defensive check in case the upstream code ever makes
-                            // multiple calls for the same process state change.
-                            mBackgroundTransitioningUids.put(uid, completionTimeMs);
+                                + getBackgroundTransitioningDelay(procState);
+                        boolean completionTimeUpdated = false;
+                        if (wasAllowed) {
+                            // Rules need to transition from allowed to blocked after the respective
+                            // delay.
+                            if (transitionIdx < 0) {
+                                // This is just a defensive check in case the upstream code ever
+                                // makes multiple calls for the same process state change.
+                                mBackgroundTransitioningUids.put(uid, completionTimeMs);
+                                completionTimeUpdated = true;
+                            }
+                        } else if (mUseDifferentDelaysForBackgroundChain) {
+                            // wasAllowed was false, but the transition delay may have reduced.
+                            // Currently, this can happen when the uid transitions from
+                            // LAST_ACTIVITY to CACHED_ACTIVITY, for example.
+                            if (transitionIdx >= 0
+                                    && completionTimeMs < mBackgroundTransitioningUids.valueAt(
+                                    transitionIdx)) {
+                                mBackgroundTransitioningUids.setValueAt(transitionIdx,
+                                        completionTimeMs);
+                                completionTimeUpdated = true;
+                            }
                         }
-                        if (!mHandler.hasMessages(MSG_PROCESS_BACKGROUND_TRANSITIONING_UIDS)) {
-                            // Many uids may be in this "transitioning" state at the same time, so
-                            // using one message at a time to avoid congestion in the MessageQueue.
+                        if (completionTimeUpdated
+                                && completionTimeMs < mNextProcessBackgroundUidsTime) {
+                            // Many uids may be in this "transitioning" state at the same time,
+                            // so we always keep one message to process transition completion at
+                            // the earliest time.
+                            mHandler.removeMessages(MSG_PROCESS_BACKGROUND_TRANSITIONING_UIDS);
                             mHandler.sendEmptyMessageAtTime(
                                     MSG_PROCESS_BACKGROUND_TRANSITIONING_UIDS, completionTimeMs);
+                            mNextProcessBackgroundUidsTime = completionTimeMs;
                         }
                     }
                 }
@@ -5744,10 +5842,11 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                             updateRuleForBackgroundUL(uid);
                             updateRulesForPowerRestrictionsUL(uid, false);
                         }
-                    }
-                    if (nextCheckTime < Long.MAX_VALUE) {
-                        mHandler.sendEmptyMessageAtTime(MSG_PROCESS_BACKGROUND_TRANSITIONING_UIDS,
-                                nextCheckTime);
+                        mNextProcessBackgroundUidsTime = nextCheckTime;
+                        if (nextCheckTime < Long.MAX_VALUE) {
+                            mHandler.sendEmptyMessageAtTime(
+                                    MSG_PROCESS_BACKGROUND_TRANSITIONING_UIDS, nextCheckTime);
+                        }
                     }
                     return true;
                 }
diff --git a/services/core/java/com/android/server/net/flags.aconfig b/services/core/java/com/android/server/net/flags.aconfig
index e986dd81b94b..586baf022897 100644
--- a/services/core/java/com/android/server/net/flags.aconfig
+++ b/services/core/java/com/android/server/net/flags.aconfig
@@ -17,3 +17,13 @@ flag {
       purpose: PURPOSE_BUGFIX
     }
 }
+
+flag {
+    name: "use_different_delays_for_background_chain"
+    namespace: "backstage_power"
+    description: "Grant longer grace periods for sensitive process-states before blocking network using the background chain"
+    bug: "323963467"
+    metadata {
+      purpose: PURPOSE_BUGFIX
+    }
+}
diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
index bd30ef583903..7be457ee2565 100644
--- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
@@ -18,11 +18,13 @@ package com.android.server.net;
 
 import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS;
 import static android.Manifest.permission.NETWORK_STACK;
+import static android.app.ActivityManager.MAX_PROCESS_STATE;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_ALL;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_NONE;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_POWER_RESTRICTED_NETWORK;
 import static android.app.ActivityManager.PROCESS_CAPABILITY_USER_RESTRICTED_NETWORK;
 import static android.app.ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND;
+import static android.app.ActivityManager.PROCESS_STATE_LAST_ACTIVITY;
 import static android.app.ActivityManager.PROCESS_STATE_SERVICE;
 import static android.app.ActivityManager.PROCESS_STATE_TOP;
 import static android.net.ConnectivityManager.BLOCKED_METERED_REASON_DATA_SAVER;
@@ -165,9 +167,11 @@ import android.os.PowerManagerInternal;
 import android.os.PowerSaveState;
 import android.os.RemoteException;
 import android.os.SimpleClock;
+import android.os.SystemClock;
 import android.os.UserHandle;
 import android.os.UserManager;
 import android.platform.test.annotations.Presubmit;
+import android.platform.test.annotations.RequiresFlagsDisabled;
 import android.platform.test.annotations.RequiresFlagsEnabled;
 import android.platform.test.flag.junit.CheckFlagsRule;
 import android.platform.test.flag.junit.DeviceFlagsValueProvider;
@@ -2158,7 +2162,8 @@ public class NetworkPolicyManagerServiceTest {
 
     @Test
     @RequiresFlagsEnabled(Flags.FLAG_NETWORK_BLOCKED_FOR_TOP_SLEEPING_AND_ABOVE)
-    public void testBackgroundChainOnProcStateChange() throws Exception {
+    @RequiresFlagsDisabled(Flags.FLAG_USE_DIFFERENT_DELAYS_FOR_BACKGROUND_CHAIN)
+    public void testBackgroundChainOnProcStateChangeSameDelay() throws Exception {
         // initialization calls setFirewallChainEnabled, so we want to reset the invocations.
         clearInvocations(mNetworkManager);
 
@@ -2186,6 +2191,59 @@ public class NetworkPolicyManagerServiceTest {
     }
 
     @Test
+    @RequiresFlagsEnabled({
+            Flags.FLAG_NETWORK_BLOCKED_FOR_TOP_SLEEPING_AND_ABOVE,
+            Flags.FLAG_USE_DIFFERENT_DELAYS_FOR_BACKGROUND_CHAIN
+    })
+    public void testBackgroundChainOnProcStateChangeDifferentDelays() throws Exception {
+        // The app will be blocked when there is no prior proc-state.
+        assertTrue(mService.isUidNetworkingBlocked(UID_A, false));
+
+        // Tweak delays to avoid waiting too long in tests.
+        mService.mBackgroundRestrictionShortDelayMs = 50;
+        mService.mBackgroundRestrictionLongDelayMs = 1000;
+
+        int procStateSeq = 231; // Any arbitrary starting sequence.
+        for (int ps = BACKGROUND_THRESHOLD_STATE; ps <= MAX_PROCESS_STATE; ps++) {
+            clearInvocations(mNetworkManager);
+
+            // Make sure app is in correct process-state to access network.
+            callAndWaitOnUidStateChanged(UID_A, BACKGROUND_THRESHOLD_STATE - 1, procStateSeq++);
+            verify(mNetworkManager).setFirewallUidRule(FIREWALL_CHAIN_BACKGROUND, UID_A,
+                    FIREWALL_RULE_ALLOW);
+            assertFalse(mService.isUidNetworkingBlocked(UID_A, false));
+
+            // Now put the app into the background and test that it eventually loses network.
+            callAndWaitOnUidStateChanged(UID_A, ps, procStateSeq++);
+
+            final long uidStateChangeTime = SystemClock.uptimeMillis();
+            if (ps <= PROCESS_STATE_LAST_ACTIVITY) {
+                // Verify that the app is blocked after long delay but not after short delay.
+                waitForDelayedMessageOnHandler(mService.mBackgroundRestrictionShortDelayMs + 1);
+                verify(mNetworkManager, never()).setFirewallUidRule(FIREWALL_CHAIN_BACKGROUND,
+                        UID_A, FIREWALL_RULE_DEFAULT);
+                assertFalse(mService.isUidNetworkingBlocked(UID_A, false));
+
+                final long timeUntilLongDelay = uidStateChangeTime
+                        + mService.mBackgroundRestrictionLongDelayMs - SystemClock.uptimeMillis();
+                assertTrue("No time left to verify long delay in background transition",
+                        timeUntilLongDelay >= 0);
+
+                waitForDelayedMessageOnHandler(timeUntilLongDelay + 1);
+                verify(mNetworkManager).setFirewallUidRule(FIREWALL_CHAIN_BACKGROUND, UID_A,
+                        FIREWALL_RULE_DEFAULT);
+                assertTrue(mService.isUidNetworkingBlocked(UID_A, false));
+            } else {
+                // Verify that the app is blocked after short delay.
+                waitForDelayedMessageOnHandler(mService.mBackgroundRestrictionShortDelayMs + 1);
+                verify(mNetworkManager).setFirewallUidRule(FIREWALL_CHAIN_BACKGROUND, UID_A,
+                        FIREWALL_RULE_DEFAULT);
+                assertTrue(mService.isUidNetworkingBlocked(UID_A, false));
+            }
+        }
+    }
+
+    @Test
     @RequiresFlagsEnabled(Flags.FLAG_NETWORK_BLOCKED_FOR_TOP_SLEEPING_AND_ABOVE)
     public void testBackgroundChainOnAllowlistChange() throws Exception {
         // initialization calls setFirewallChainEnabled, so we want to reset the invocations.
@@ -2881,6 +2939,11 @@ public class NetworkPolicyManagerServiceTest {
         }
     }
 
+    /**
+     * This posts a blocking message to the service handler with the given delayMs and waits for it
+     * to complete. This ensures that all messages posted before the given delayMs will also
+     * have been executed before this method returns and can be verified in subsequent code.
+     */
     private void waitForDelayedMessageOnHandler(long delayMs) throws InterruptedException {
         final CountDownLatch latch = new CountDownLatch(1);
         mService.getHandlerForTesting().postDelayed(latch::countDown, delayMs);