From efa4f742360efc714082d04d7224aae7058e186e Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Mon, 4 Apr 2022 16:08:00 -0700 Subject: [PATCH] Ignore older UID state change callbacks in NPMS. Since the onUidStateChanged() callbacks will be happening from more than one thread, we need to make sure we only handle the latest uid state change callbacks and ignore any older callbacks that could override the latest uid states in NPMS. In order to this, every uid state change will be associated with a unique seq number. Also, remove the UidRecord.lastDispatchedProcStateSeq. This was added as a safety measure to make sure we don't unnecessarily end up waiting but it isn't necessary and if we need to keep it, we need to add a way to access it without holding global AMS lock, which isn't worth the complexity. Bug: 226299593 Test: atest tests/cts/hostside/src/com/android/cts/net/HostsideRestrictBackgroundNetworkTests.java Change-Id: I631f5e01f6627916a96c930c22849a1d11eab636 Merged-In: I631f5e01f6627916a96c930c22849a1d11eab636 --- .../android/net/NetworkPolicyManager.java | 6 +- .../server/am/ActivityManagerService.java | 39 +---- .../com/android/server/am/ProcessList.java | 17 ++- .../server/am/UidObserverController.java | 1 - .../java/com/android/server/am/UidRecord.java | 26 +--- .../net/NetworkPolicyManagerService.java | 56 ++++--- .../am/ActivityManagerInternalTest.java | 16 +- .../server/am/ActivityManagerServiceTest.java | 141 +++--------------- .../net/NetworkPolicyManagerServiceTest.java | 11 +- 9 files changed, 94 insertions(+), 219 deletions(-) diff --git a/core/java/android/net/NetworkPolicyManager.java b/core/java/android/net/NetworkPolicyManager.java index 2c2a703b16e24..9341105675a2e 100644 --- a/core/java/android/net/NetworkPolicyManager.java +++ b/core/java/android/net/NetworkPolicyManager.java @@ -817,11 +817,13 @@ public class NetworkPolicyManager { public static final class UidState { public int uid; public int procState; + public long procStateSeq; public int capability; - public UidState(int uid, int procState, int capability) { + public UidState(int uid, int procState, long procStateSeq, int capability) { this.uid = uid; this.procState = procState; + this.procStateSeq = procStateSeq; this.capability = capability; } @@ -830,6 +832,8 @@ public class NetworkPolicyManager { final StringBuilder sb = new StringBuilder(); sb.append("{procState="); sb.append(procStateToString(procState)); + sb.append(",seq="); + sb.append(procStateSeq); sb.append(",cap="); ActivityManager.printCapabilitiesSummary(sb, capability); sb.append("}"); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 3cbe4e3e9f684..cafda0e2599ed 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -14449,7 +14449,6 @@ public class ActivityManagerService extends IActivityManager.Stub uid, change, procState, procStateSeq, capability, ephemeral); if (uidRec != null) { uidRec.setLastReportedChange(enqueuedChange); - uidRec.updateLastDispatchedProcStateSeq(enqueuedChange); } // Directly update the power manager, since we sit on top of it and it is critical @@ -15587,18 +15586,13 @@ public class ActivityManagerService extends IActivityManager.Stub return; } record.lastNetworkUpdatedProcStateSeq = procStateSeq; - if (record.curProcStateSeq > procStateSeq) { - if (DEBUG_NETWORK) { - Slog.d(TAG_NETWORK, "No need to handle older seq no., Uid: " + uid - + ", curProcstateSeq: " + record.curProcStateSeq - + ", procStateSeq: " + procStateSeq); - } - return; - } - if (record.waitingForNetwork) { + if (record.procStateSeqWaitingForNetwork != 0 + && procStateSeq >= record.procStateSeqWaitingForNetwork) { if (DEBUG_NETWORK) { Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid - + ", procStateSeq: " + procStateSeq); + + ", procStateSeq: " + procStateSeq + + ", procStateSeqWaitingForNetwork: " + + record.procStateSeqWaitingForNetwork); } record.networkStateLock.notifyAll(); } @@ -16313,7 +16307,7 @@ public class ActivityManagerService extends IActivityManager.Stub if (mNetworkPolicyUidObserver != null) { try { mNetworkPolicyUidObserver.onUidStateChanged(uid, PROCESS_STATE_TOP, - mProcessList.getProcStateSeqCounter(), PROCESS_CAPABILITY_ALL); + mProcessList.getNextProcStateSeq(), PROCESS_CAPABILITY_ALL); } catch (RemoteException e) { // Should not happen; call is within the same process } @@ -16551,23 +16545,6 @@ public class ActivityManagerService extends IActivityManager.Stub } } synchronized (record.networkStateLock) { - if (record.lastDispatchedProcStateSeq < procStateSeq) { - if (DEBUG_NETWORK) { - Slog.d(TAG_NETWORK, "Uid state change for seq no. " + procStateSeq + " is not " - + "dispatched to NPMS yet, so don't wait. Uid: " + callingUid - + " lastProcStateSeqDispatchedToObservers: " - + record.lastDispatchedProcStateSeq); - } - return; - } - if (record.curProcStateSeq > procStateSeq) { - if (DEBUG_NETWORK) { - Slog.d(TAG_NETWORK, "Ignore the wait requests for older seq numbers. Uid: " - + callingUid + ", curProcStateSeq: " + record.curProcStateSeq - + ", procStateSeq: " + procStateSeq); - } - return; - } if (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) { if (DEBUG_NETWORK) { Slog.d(TAG_NETWORK, "Network rules have been already updated for seq no. " @@ -16583,9 +16560,9 @@ public class ActivityManagerService extends IActivityManager.Stub + " Uid: " + callingUid + " procStateSeq: " + procStateSeq); } final long startTime = SystemClock.uptimeMillis(); - record.waitingForNetwork = true; + record.procStateSeqWaitingForNetwork = procStateSeq; record.networkStateLock.wait(mWaitForNetworkTimeoutMs); - record.waitingForNetwork = false; + record.procStateSeqWaitingForNetwork = 0; final long totalTime = SystemClock.uptimeMillis() - startTime; if (totalTime >= mWaitForNetworkTimeoutMs || DEBUG_NETWORK) { Slog.w(TAG_NETWORK, "Total time waited for network rules to get updated: " diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 0f645ebc689be..dc80c4b332975 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -4877,13 +4877,17 @@ public final class ProcessList { } /** - * Checks if any uid is coming from background to foreground or vice versa and if so, increments - * the {@link UidRecord#curProcStateSeq} corresponding to that uid using global seq counter - * {@link ProcessList#mProcStateSeqCounter} and notifies the app if it needs to block. + * Increments the {@link UidRecord#curProcStateSeq} for all uids using global seq counter + * {@link ProcessList#mProcStateSeqCounter} and checks if any uid is coming + * from background to foreground or vice versa and if so, notifies the app if it needs to block. */ @VisibleForTesting @GuardedBy(anyOf = {"mService", "mProcLock"}) void incrementProcStateSeqAndNotifyAppsLOSP(ActiveUids activeUids) { + for (int i = activeUids.size() - 1; i >= 0; --i) { + final UidRecord uidRec = activeUids.valueAt(i); + uidRec.curProcStateSeq = getNextProcStateSeq(); + } if (mService.mWaitForNetworkTimeoutMs <= 0) { return; } @@ -4910,7 +4914,6 @@ public final class ProcessList { continue; } synchronized (uidRec.networkStateLock) { - uidRec.curProcStateSeq = ++mProcStateSeqCounter; // TODO: use method if (blockState == NETWORK_STATE_BLOCK) { if (blockingUids == null) { blockingUids = new ArrayList<>(); @@ -4921,7 +4924,7 @@ public final class ProcessList { Slog.d(TAG_NETWORK, "uid going to background, notifying all blocking" + " threads for uid: " + uidRec); } - if (uidRec.waitingForNetwork) { + if (uidRec.procStateSeqWaitingForNetwork != 0) { uidRec.networkStateLock.notifyAll(); } } @@ -4955,8 +4958,8 @@ public final class ProcessList { } } - long getProcStateSeqCounter() { - return mProcStateSeqCounter; + long getNextProcStateSeq() { + return ++mProcStateSeqCounter; } /** diff --git a/services/core/java/com/android/server/am/UidObserverController.java b/services/core/java/com/android/server/am/UidObserverController.java index c1bfe25618b20..67b9ae67bbd9a 100644 --- a/services/core/java/com/android/server/am/UidObserverController.java +++ b/services/core/java/com/android/server/am/UidObserverController.java @@ -219,7 +219,6 @@ public class UidObserverController { validateUid.setCurProcState(item.procState); validateUid.setSetCapability(item.capability); validateUid.setCurCapability(item.capability); - validateUid.lastDispatchedProcStateSeq = item.procStateSeq; } } } diff --git a/services/core/java/com/android/server/am/UidRecord.java b/services/core/java/com/android/server/am/UidRecord.java index 4ba59faea9ec2..2ec08a45236fa 100644 --- a/services/core/java/com/android/server/am/UidRecord.java +++ b/services/core/java/com/android/server/am/UidRecord.java @@ -95,16 +95,9 @@ public final class UidRecord { long lastNetworkUpdatedProcStateSeq; /** - * Last seq number for which AcitivityManagerService dispatched uid state change to - * NetworkPolicyManagerService. + * Indicates if any thread is waiting for network rules to get updated for {@link #mUid}. */ - @GuardedBy("networkStateUpdate") - long lastDispatchedProcStateSeq; - - /** - * Indicates if any thread is waiting for network rules to get updated for {@link #uid}. - */ - volatile boolean waitingForNetwork; + volatile long procStateSeqWaitingForNetwork; /** * Indicates whether this uid has internet permission or not. @@ -309,18 +302,6 @@ public final class UidRecord { mUid) == PackageManager.PERMISSION_GRANTED; } - /** - * If the change being dispatched is not CHANGE_GONE (not interested in - * these changes), then update the {@link #lastDispatchedProcStateSeq} with - * {@link #curProcStateSeq}. - */ - public void updateLastDispatchedProcStateSeq(int changeToDispatch) { - if ((changeToDispatch & CHANGE_GONE) == 0) { - lastDispatchedProcStateSeq = curProcStateSeq; - } - } - - void dumpDebug(ProtoOutputStream proto, long fieldId) { long token = proto.start(fieldId); proto.write(UidRecordProto.UID, mUid); @@ -341,7 +322,6 @@ public final class UidRecord { proto.write(UidRecordProto.ProcStateSequence.CURURENT, curProcStateSeq); proto.write(UidRecordProto.ProcStateSequence.LAST_NETWORK_UPDATED, lastNetworkUpdatedProcStateSeq); - proto.write(UidRecordProto.ProcStateSequence.LAST_DISPATCHED, lastDispatchedProcStateSeq); proto.end(seqToken); proto.end(token); @@ -412,8 +392,6 @@ public final class UidRecord { sb.append(curProcStateSeq); sb.append(","); sb.append(lastNetworkUpdatedProcStateSeq); - sb.append(","); - sb.append(lastDispatchedProcStateSeq); sb.append(")}"); return sb.toString(); } diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index c804c9b234e30..66db72117585b 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_STATE_UNKNOWN; import static android.app.ActivityManager.isProcStateConsideredInteraction; +import static android.app.ActivityManager.procStateToString; import static android.app.PendingIntent.FLAG_IMMUTABLE; import static android.app.PendingIntent.FLAG_UPDATE_CURRENT; import static android.content.Intent.ACTION_PACKAGE_ADDED; @@ -1123,7 +1124,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { callbackInfo = new UidStateCallbackInfo(); mUidStateCallbackInfos.put(uid, callbackInfo); } - callbackInfo.update(uid, procState, procStateSeq, capability); + if (callbackInfo.procStateSeq == -1 || procStateSeq > callbackInfo.procStateSeq) { + callbackInfo.update(uid, procState, procStateSeq, capability); + } if (!callbackInfo.isPending) { mUidEventHandler.obtainMessage(UID_MSG_STATE_CHANGED, callbackInfo) .sendToTarget(); @@ -1147,8 +1150,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { private static final class UidStateCallbackInfo { public int uid; - public int procState; - public long procStateSeq; + public int procState = ActivityManager.PROCESS_STATE_NONEXISTENT; + public long procStateSeq = -1; @ProcessCapability public int capability; public boolean isPending; @@ -4058,13 +4061,22 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { * {@link #updateRulesForPowerRestrictionsUL(int)}. Returns true if the state was updated. */ @GuardedBy("mUidRulesFirstLock") - private boolean updateUidStateUL(int uid, int procState, @ProcessCapability int capability) { + private boolean updateUidStateUL(int uid, int procState, long procStateSeq, + @ProcessCapability int capability) { Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "updateUidStateUL"); try { final UidState oldUidState = mUidState.get(uid); + if (oldUidState != null && procStateSeq < oldUidState.procStateSeq) { + if (LOGV) { + Slog.v(TAG, "Ignoring older uid state updates; uid=" + uid + + ",procState=" + procStateToString(procState) + ",seq=" + procStateSeq + + ",cap=" + capability + ",oldUidState=" + oldUidState); + } + return false; + } if (oldUidState == null || oldUidState.procState != procState || oldUidState.capability != capability) { - final UidState newUidState = new UidState(uid, procState, capability); + final UidState newUidState = new UidState(uid, procState, procStateSeq, capability); // state changed, push updated rules mUidState.put(uid, newUidState); updateRestrictBackgroundRulesOnUidStatusChangedUL(uid, oldUidState, newUidState); @@ -4811,6 +4823,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { synchronized (mUidBlockedState) { mUidBlockedState.delete(uid); } + mUidState.delete(uid); mUidPolicy.delete(uid); mUidFirewallStandbyRules.delete(uid); mUidFirewallDozableRules.delete(uid); @@ -5452,21 +5465,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { public boolean handleMessage(Message msg) { switch (msg.what) { case UID_MSG_STATE_CHANGED: { - final UidStateCallbackInfo uidStateCallbackInfo = - (UidStateCallbackInfo) msg.obj; - final int uid; - final int procState; - final long procStateSeq; - final int capability; - synchronized (mUidStateCallbackInfos) { - uid = uidStateCallbackInfo.uid; - procState = uidStateCallbackInfo.procState; - procStateSeq = uidStateCallbackInfo.procStateSeq; - capability = uidStateCallbackInfo.capability; - uidStateCallbackInfo.isPending = false; - } - - handleUidChanged(uid, procState, procStateSeq, capability); + handleUidChanged((UidStateCallbackInfo) msg.obj); return true; } case UID_MSG_GONE: { @@ -5481,17 +5480,28 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { } }; - void handleUidChanged(int uid, int procState, long procStateSeq, - @ProcessCapability int capability) { + void handleUidChanged(@NonNull UidStateCallbackInfo uidStateCallbackInfo) { Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "onUidStateChanged"); try { boolean updated; + final int uid; + final int procState; + final long procStateSeq; + final int capability; synchronized (mUidRulesFirstLock) { + synchronized (mUidStateCallbackInfos) { + uid = uidStateCallbackInfo.uid; + procState = uidStateCallbackInfo.procState; + procStateSeq = uidStateCallbackInfo.procStateSeq; + capability = uidStateCallbackInfo.capability; + uidStateCallbackInfo.isPending = false; + } + // We received a uid state change callback, add it to the history so that it // will be useful for debugging. mLogger.uidStateChanged(uid, procState, procStateSeq, capability); // Now update the network policy rules as per the updated uid state. - updated = updateUidStateUL(uid, procState, capability); + updated = updateUidStateUL(uid, procState, procStateSeq, capability); // Updating the network rules is done, so notify AMS about this. mActivityManagerInternal.notifyNetworkPolicyRulesUpdated(uid, procStateSeq); } diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java index 8c21a39c20d1e..16406bcb9a1f5 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java @@ -129,7 +129,7 @@ public class ActivityManagerInternalTest { thread2.assertWaiting("Unexpected state for " + record2); thread2.interrupt(); - mAms.mProcessList.mActiveUids.clear(); + clearActiveUids(); } private UidRecord addActiveUidRecord(int uid, long curProcStateSeq, @@ -137,11 +137,21 @@ public class ActivityManagerInternalTest { final UidRecord record = new UidRecord(uid, mAms); record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq; record.curProcStateSeq = curProcStateSeq; - record.waitingForNetwork = true; - mAms.mProcessList.mActiveUids.put(uid, record); + record.procStateSeqWaitingForNetwork = 1; + addActiveUidRecord(uid, record); return record; } + @SuppressWarnings("GuardedBy") + private void addActiveUidRecord(int uid, UidRecord record) { + mAms.mProcessList.mActiveUids.put(uid, record); + } + + @SuppressWarnings("GuardedBy") + private void clearActiveUids() { + mAms.mProcessList.mActiveUids.clear(); + } + static class CustomThread extends Thread { private static final long WAIT_TIMEOUT_MS = 1000; private static final long WAIT_INTERVAL_MS = 100; diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java index 12e0d8bc5d626..968a3e2c354eb 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java @@ -191,8 +191,6 @@ public class ActivityManagerServiceTest { verifySeqCounterAndInteractions(uidRec, PROCESS_STATE_TOP, // prevState PROCESS_STATE_TOP, // curState - 0, // expectedGlobalCounter - 0, // exptectedCurProcStateSeq NETWORK_STATE_NO_CHANGE, // expectedBlockState false); // expectNotify @@ -200,8 +198,6 @@ public class ActivityManagerServiceTest { verifySeqCounterAndInteractions(uidRec, PROCESS_STATE_FOREGROUND_SERVICE, // prevState PROCESS_STATE_SERVICE, // curState - 1, // expectedGlobalCounter - 1, // exptectedCurProcStateSeq NETWORK_STATE_UNBLOCK, // expectedBlockState true); // expectNotify @@ -213,8 +209,6 @@ public class ActivityManagerServiceTest { verifySeqCounterAndInteractions(uidRec, PROCESS_STATE_TRANSIENT_BACKGROUND, // prevState PROCESS_STATE_IMPORTANT_BACKGROUND, // curState - 42, // expectedGlobalCounter - 1, // exptectedCurProcStateSeq NETWORK_STATE_NO_CHANGE, // expectedBlockState false); // expectNotify @@ -222,73 +216,22 @@ public class ActivityManagerServiceTest { verifySeqCounterAndInteractions(uidRec, PROCESS_STATE_LAST_ACTIVITY, // prevState PROCESS_STATE_TOP, // curState - 43, // expectedGlobalCounter - 43, // exptectedCurProcStateSeq NETWORK_STATE_BLOCK, // expectedBlockState false); // expectNotify // verify waiting threads are not notified. - uidRec.waitingForNetwork = false; + uidRec.procStateSeqWaitingForNetwork = 0; // Uid state is moving from foreground to background. verifySeqCounterAndInteractions(uidRec, PROCESS_STATE_FOREGROUND_SERVICE, // prevState PROCESS_STATE_SERVICE, // curState - 44, // expectedGlobalCounter - 44, // exptectedCurProcStateSeq NETWORK_STATE_UNBLOCK, // expectedBlockState false); // expectNotify - - // Verify when uid is not restricted, procStateSeq is not incremented. - uidRec.waitingForNetwork = true; - mInjector.setNetworkRestrictedForUid(false); - verifySeqCounterAndInteractions(uidRec, - PROCESS_STATE_IMPORTANT_BACKGROUND, // prevState - PROCESS_STATE_TOP, // curState - 44, // expectedGlobalCounter - 44, // exptectedCurProcStateSeq - -1, // expectedBlockState, -1 to verify there are no interactions with main thread. - false); // expectNotify - - // Verify when waitForNetworkTimeout is 0, then procStateSeq is not incremented. - mAms.mWaitForNetworkTimeoutMs = 0; - mInjector.setNetworkRestrictedForUid(true); - verifySeqCounterAndInteractions(uidRec, - PROCESS_STATE_TOP, // prevState - PROCESS_STATE_IMPORTANT_BACKGROUND, // curState - 44, // expectedGlobalCounter - 44, // exptectedCurProcStateSeq - -1, // expectedBlockState, -1 to verify there are no interactions with main thread. - false); // expectNotify - - // Verify when the uid doesn't have internet permission, then procStateSeq is not - // incremented. - uidRec.hasInternetPermission = false; - mAms.mWaitForNetworkTimeoutMs = 111; - mInjector.setNetworkRestrictedForUid(true); - verifySeqCounterAndInteractions(uidRec, - PROCESS_STATE_CACHED_ACTIVITY, // prevState - PROCESS_STATE_FOREGROUND_SERVICE, // curState - 44, // expectedGlobalCounter - 44, // exptectedCurProcStateSeq - -1, // expectedBlockState, -1 to verify there are no interactions with main thread. - false); // expectNotify - - // Verify procStateSeq is not incremented when the uid is not an application, regardless - // of the process state. - final int notAppUid = 111; - final UidRecord uidRec2 = addUidRecord(notAppUid); - verifySeqCounterAndInteractions(uidRec2, - PROCESS_STATE_CACHED_EMPTY, // prevState - PROCESS_STATE_TOP, // curState - 44, // expectedGlobalCounter - 0, // exptectedCurProcStateSeq - -1, // expectedBlockState, -1 to verify there are no interactions with main thread. - false); // expectNotify } private UidRecord addUidRecord(int uid) { final UidRecord uidRec = new UidRecord(uid, mAms); - uidRec.waitingForNetwork = true; + uidRec.procStateSeqWaitingForNetwork = 1; uidRec.hasInternetPermission = true; mAms.mProcessList.mActiveUids.put(uid, uidRec); @@ -305,18 +248,26 @@ public class ActivityManagerServiceTest { @SuppressWarnings("GuardedBy") private void verifySeqCounterAndInteractions(UidRecord uidRec, int prevState, int curState, - int expectedGlobalCounter, int expectedCurProcStateSeq, int expectedBlockState, - boolean expectNotify) throws Exception { + int expectedBlockState, boolean expectNotify) throws Exception { CustomThread thread = new CustomThread(uidRec.networkStateLock); thread.startAndWait("Unexpected state for " + uidRec); uidRec.setSetProcState(prevState); uidRec.setCurProcState(curState); + final long beforeProcStateSeq = mAms.mProcessList.mProcStateSeqCounter; + mAms.mProcessList.incrementProcStateSeqAndNotifyAppsLOSP(mAms.mProcessList.mActiveUids); - // @SuppressWarnings("GuardedBy") - assertEquals(expectedGlobalCounter, mAms.mProcessList.mProcStateSeqCounter); - assertEquals(expectedCurProcStateSeq, uidRec.curProcStateSeq); + final long afterProcStateSeq = beforeProcStateSeq + + mAms.mProcessList.mActiveUids.size(); + assertEquals("beforeProcStateSeq=" + beforeProcStateSeq + + ",activeUids.size=" + mAms.mProcessList.mActiveUids.size(), + afterProcStateSeq, mAms.mProcessList.mProcStateSeqCounter); + assertTrue("beforeProcStateSeq=" + beforeProcStateSeq + + ",afterProcStateSeq=" + afterProcStateSeq + + ",uidCurProcStateSeq=" + uidRec.curProcStateSeq, + uidRec.curProcStateSeq > beforeProcStateSeq + && uidRec.curProcStateSeq <= afterProcStateSeq); for (int i = mAms.mProcessList.getLruSizeLOSP() - 1; i >= 0; --i) { final ProcessRecord app = mAms.mProcessList.getLruProcessesLOSP().get(i); @@ -814,49 +765,12 @@ public class ActivityManagerServiceTest { 0, mAms.mUidObserverController.getValidateUidsForTest().size()); } - @Test - public void testEnqueueUidChangeLocked_procStateSeqUpdated() { - final UidRecord uidRecord = new UidRecord(TEST_UID, mAms); - uidRecord.curProcStateSeq = TEST_PROC_STATE_SEQ1; - - // Verify with no pending changes for TEST_UID. - verifyLastProcStateSeqUpdated(uidRecord, -1, TEST_PROC_STATE_SEQ1); - - // Add a pending change for TEST_UID and verify enqueueUidChangeLocked still works as - // expected. - uidRecord.curProcStateSeq = TEST_PROC_STATE_SEQ2; - verifyLastProcStateSeqUpdated(uidRecord, -1, TEST_PROC_STATE_SEQ2); - } - @Test public void testEnqueueUidChangeLocked_nullUidRecord() { // Use "null" uidRecord to make sure there is no crash. mAms.enqueueUidChangeLocked(null, TEST_UID, UidRecord.CHANGE_ACTIVE); } - private void verifyLastProcStateSeqUpdated(UidRecord uidRecord, int uid, long curProcstateSeq) { - // Test enqueueUidChangeLocked with every UidRecord.CHANGE_* - for (int i = 0; i < UID_RECORD_CHANGES.length; ++i) { - final int changeToDispatch = UID_RECORD_CHANGES[i]; - // Reset lastProcStateSeqDispatchToObservers after every test. - uidRecord.lastDispatchedProcStateSeq = 0; - mAms.enqueueUidChangeLocked(uidRecord, uid, changeToDispatch); - // Verify there is no effect on curProcStateSeq. - assertEquals(curProcstateSeq, uidRecord.curProcStateSeq); - if ((changeToDispatch & UidRecord.CHANGE_GONE) != 0) { - // Since the change is CHANGE_GONE or CHANGE_GONE_IDLE, verify that - // lastProcStateSeqDispatchedToObservers is not updated. - assertNotEquals(uidRecord.curProcStateSeq, - uidRecord.lastDispatchedProcStateSeq); - } else { - // Since the change is neither CHANGE_GONE nor CHANGE_GONE_IDLE, verify that - // lastProcStateSeqDispatchedToObservers has been updated to curProcStateSeq. - assertEquals(uidRecord.curProcStateSeq, - uidRecord.lastDispatchedProcStateSeq); - } - } - } - @MediumTest @Test public void testEnqueueUidChangeLocked_dispatchUidsChanged() { @@ -893,29 +807,10 @@ public class ActivityManagerServiceTest { // Check there is no crash when there is no UidRecord for myUid mAms.waitForNetworkStateUpdate(TEST_PROC_STATE_SEQ1); - // Verify there is no waiting when UidRecord.curProcStateSeq is greater than - // the procStateSeq in the request to wait. - verifyWaitingForNetworkStateUpdate( - TEST_PROC_STATE_SEQ1, // curProcStateSeq - TEST_PROC_STATE_SEQ1, // lastDsipatchedProcStateSeq - TEST_PROC_STATE_SEQ1 - 4, // lastNetworkUpdatedProcStateSeq - TEST_PROC_STATE_SEQ1 - 2, // procStateSeqToWait - false); // expectWait - - // Verify there is no waiting when the procStateSeq in the request to wait is - // not dispatched to NPMS. - verifyWaitingForNetworkStateUpdate( - TEST_PROC_STATE_SEQ1, // curProcStateSeq - TEST_PROC_STATE_SEQ1 - 1, // lastDsipatchedProcStateSeq - TEST_PROC_STATE_SEQ1 - 1, // lastNetworkUpdatedProcStateSeq - TEST_PROC_STATE_SEQ1, // procStateSeqToWait - false); // expectWait - // Verify there is not waiting when the procStateSeq in the request already has // an updated network state. verifyWaitingForNetworkStateUpdate( TEST_PROC_STATE_SEQ1, // curProcStateSeq - TEST_PROC_STATE_SEQ1, // lastDsipatchedProcStateSeq TEST_PROC_STATE_SEQ1, // lastNetworkUpdatedProcStateSeq TEST_PROC_STATE_SEQ1, // procStateSeqToWait false); // expectWait @@ -923,18 +818,16 @@ public class ActivityManagerServiceTest { // Verify waiting for network works verifyWaitingForNetworkStateUpdate( TEST_PROC_STATE_SEQ1, // curProcStateSeq - TEST_PROC_STATE_SEQ1, // lastDsipatchedProcStateSeq TEST_PROC_STATE_SEQ1 - 1, // lastNetworkUpdatedProcStateSeq TEST_PROC_STATE_SEQ1, // procStateSeqToWait true); // expectWait } private void verifyWaitingForNetworkStateUpdate(long curProcStateSeq, - long lastDispatchedProcStateSeq, long lastNetworkUpdatedProcStateSeq, + long lastNetworkUpdatedProcStateSeq, final long procStateSeqToWait, boolean expectWait) throws Exception { final UidRecord record = new UidRecord(Process.myUid(), mAms); record.curProcStateSeq = curProcStateSeq; - record.lastDispatchedProcStateSeq = lastDispatchedProcStateSeq; record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq; mAms.mProcessList.mActiveUids.put(Process.myUid(), record); @@ -953,7 +846,7 @@ public class ActivityManagerServiceTest { } thread.assertTerminated(errMsg); assertTrue(thread.mNotified); - assertFalse(record.waitingForNetwork); + assertEquals(0, record.procStateSeqWaitingForNetwork); } else { thread.start(); thread.assertTerminated(errMsg); 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 6c7f87244cfe2..0f2fe4434bfa1 100644 --- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java @@ -991,19 +991,20 @@ public class NetworkPolicyManagerServiceTest { @Test public void testUidForeground() throws Exception { // push all uids into background - callOnUidStateChanged(UID_A, ActivityManager.PROCESS_STATE_SERVICE, 0); - callOnUidStateChanged(UID_B, ActivityManager.PROCESS_STATE_SERVICE, 0); + long procStateSeq = 0; + callOnUidStateChanged(UID_A, ActivityManager.PROCESS_STATE_SERVICE, procStateSeq++); + callOnUidStateChanged(UID_B, ActivityManager.PROCESS_STATE_SERVICE, procStateSeq++); assertFalse(mService.isUidForeground(UID_A)); assertFalse(mService.isUidForeground(UID_B)); // push one of the uids into foreground - callOnUidStateChanged(UID_A, ActivityManager.PROCESS_STATE_TOP, 0); + callOnUidStateChanged(UID_A, ActivityManager.PROCESS_STATE_TOP, procStateSeq++); assertTrue(mService.isUidForeground(UID_A)); assertFalse(mService.isUidForeground(UID_B)); // and swap another uid into foreground - callOnUidStateChanged(UID_A, ActivityManager.PROCESS_STATE_SERVICE, 0); - callOnUidStateChanged(UID_B, ActivityManager.PROCESS_STATE_TOP, 0); + callOnUidStateChanged(UID_A, ActivityManager.PROCESS_STATE_SERVICE, procStateSeq++); + callOnUidStateChanged(UID_B, ActivityManager.PROCESS_STATE_TOP, procStateSeq++); assertFalse(mService.isUidForeground(UID_A)); assertTrue(mService.isUidForeground(UID_B)); }