Always process state changes below TOP_THRESHOLD

Sometimes, we deliberately send a process-state TOP change before the
normal oom adjuster update gets triggered for resuming activities. In
this case, there may be a duplicate notification for the same
process-state change with a newer procStateSeq. We need to ensure we
always process these notifications and notify activity manager on
completion.

These are redundant notifications but should be rare and cheap enough to
always process - even if they do not end up changing any of the firewall
state.

Also modifying the log in ActivityManager to report a wtf whenever the
wait timeouts to help emphasize this discrepancy.

Test: atest FrameworksServicesTests:NetworkPolicyManagerServiceTest

Fixes: 327303931
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:28afbd2a2183162a38d261c79abdfb50be65e896)
Merged-In: I4884072bd6d7820acee67851d7504886564e2348
Change-Id: I4884072bd6d7820acee67851d7504886564e2348

3 files changed, 31 insertions, 16 deletions

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 2ee39c577977..09c1dea2f65a 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -19423,7 +19423,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                 record.procStateSeqWaitingForNetwork = 0;
                 final long totalTime = SystemClock.uptimeMillis() - startTime;
                 if (totalTime >= mConstants.mNetworkAccessTimeoutMs || DEBUG_NETWORK) {
-                    Slog.w(TAG_NETWORK, "Total time waited for network rules to get updated: "
+                    Slog.wtf(TAG_NETWORK, "Total time waited for network rules to get updated: "
                             + totalTime + ". Uid: " + callingUid + " procStateSeq: "
                             + procStateSeq + " UidRec: " + record
                             + " validateUidRec: "
diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index 1d35244059a9..c6fca9b76f8e 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -1214,16 +1214,14 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 return false;
             }
             final int previousProcState = previousInfo.procState;
-            if (mBackgroundNetworkRestricted && (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;
-            }
             if ((previousProcState <= TOP_THRESHOLD_STATE)
-                    != (newProcState <= TOP_THRESHOLD_STATE)) {
-                // Proc-state change crossed TOP_THRESHOLD_STATE: Network rules for the
-                // LOW_POWER_STANDBY chain may change.
+                    || (newProcState <= TOP_THRESHOLD_STATE)) {
+                // If the proc-state change crossed TOP_THRESHOLD_STATE, network rules for the
+                // LOW_POWER_STANDBY chain may change, so we need to evaluate the transition.
+                // In addition, we always process changes when the new process state is
+                // TOP_THRESHOLD_STATE or below, to avoid situations where the TOP app ends up
+                // waiting for NPMS to finish processing newProcStateSeq, even when it was
+                // redundant (b/327303931).
                 return true;
             }
             if ((previousProcState <= FOREGROUND_THRESHOLD_STATE)
@@ -1232,6 +1230,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 // different chains may change.
                 return true;
             }
+            if (mBackgroundNetworkRestricted && (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;
+            }
             final int networkCapabilities = PROCESS_CAPABILITY_POWER_RESTRICTED_NETWORK
                     | PROCESS_CAPABILITY_USER_RESTRICTED_NETWORK;
             if ((previousInfo.capability & networkCapabilities)
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 a5f7963b9c96..bd30ef583903 100644
--- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
@@ -2315,10 +2315,11 @@ public class NetworkPolicyManagerServiceTest {
         }
         waitForUidEventHandlerIdle();
         try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
-            // Doesn't cross any other threshold.
+            // Doesn't cross any threshold, but changes below TOP_THRESHOLD_STATE should always
+            // be processed
             callOnUidStatechanged(UID_A, TOP_THRESHOLD_STATE - 1, testProcStateSeq++,
                     PROCESS_CAPABILITY_NONE);
-            assertFalse(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
+            assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
         }
         waitForUidEventHandlerIdle();
     }
@@ -2349,21 +2350,21 @@ public class NetworkPolicyManagerServiceTest {
         int testProcStateSeq = 0;
         try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
             // First callback for uid.
-            callOnUidStatechanged(UID_A, TOP_THRESHOLD_STATE, testProcStateSeq++,
+            callOnUidStatechanged(UID_A, FOREGROUND_THRESHOLD_STATE, testProcStateSeq++,
                     PROCESS_CAPABILITY_NONE);
             assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
         }
         waitForUidEventHandlerIdle();
         try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
             // The same process-state with one network capability added.
-            callOnUidStatechanged(UID_A, TOP_THRESHOLD_STATE, testProcStateSeq++,
+            callOnUidStatechanged(UID_A, FOREGROUND_THRESHOLD_STATE, testProcStateSeq++,
                     PROCESS_CAPABILITY_USER_RESTRICTED_NETWORK);
             assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
         }
         waitForUidEventHandlerIdle();
         try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
             // The same process-state with another network capability added.
-            callOnUidStatechanged(UID_A, TOP_THRESHOLD_STATE, testProcStateSeq++,
+            callOnUidStatechanged(UID_A, FOREGROUND_THRESHOLD_STATE, testProcStateSeq++,
                     PROCESS_CAPABILITY_POWER_RESTRICTED_NETWORK
                             | PROCESS_CAPABILITY_USER_RESTRICTED_NETWORK);
             assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
@@ -2371,11 +2372,21 @@ public class NetworkPolicyManagerServiceTest {
         waitForUidEventHandlerIdle();
         try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
             // The same process-state with all capabilities, but no change in network capabilities.
-            callOnUidStatechanged(UID_A, TOP_THRESHOLD_STATE, testProcStateSeq++,
+            callOnUidStatechanged(UID_A, FOREGROUND_THRESHOLD_STATE, testProcStateSeq++,
                     PROCESS_CAPABILITY_ALL);
             assertFalse(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
         }
         waitForUidEventHandlerIdle();
+
+        callAndWaitOnUidStateChanged(UID_A, TOP_THRESHOLD_STATE, testProcStateSeq++,
+                PROCESS_CAPABILITY_ALL);
+        try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
+            // No change in capabilities, but TOP_THRESHOLD_STATE change should always be processed.
+            callOnUidStatechanged(UID_A, TOP_THRESHOLD_STATE, testProcStateSeq++,
+                    PROCESS_CAPABILITY_ALL);
+            assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
+        }
+        waitForUidEventHandlerIdle();
     }
 
     @Test