Don't increment procStateSeq if uid doesn't have internet permission.
Bug: 27803922
Test: runtest -c com.android.server.am.ActivityManagerServiceTest frameworks-services
and manual
Change-Id: I02bfd86837813cc79f37301d91489485c6a1d056
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 8cb0eee..1d60906 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -6552,6 +6552,7 @@
if (Arrays.binarySearch(mDeviceIdleTempWhitelist, UserHandle.getAppId(proc.uid)) >= 0) {
uidRec.setWhitelist = uidRec.curWhitelist = true;
}
+ uidRec.updateHasInternetPermission();
mActiveUids.put(proc.uid, uidRec);
noteUidProcessState(uidRec.uid, uidRec.curProcState);
enqueueUidChangeLocked(uidRec, -1, UidRecord.CHANGE_ACTIVE);
@@ -18865,9 +18866,7 @@
}
switch (action) {
case Intent.ACTION_UID_REMOVED:
- final Bundle intentExtras = intent.getExtras();
- final int uid = intentExtras != null
- ? intentExtras.getInt(Intent.EXTRA_UID) : -1;
+ final int uid = getUidFromIntent(intent);
if (uid >= 0) {
mBatteryStatsService.removeUid(uid);
mAppOpsService.uidRemoved(uid);
@@ -19063,6 +19062,18 @@
mHandler.sendEmptyMessage(HANDLE_TRUST_STORAGE_UPDATE_MSG);
break;
}
+
+ if (Intent.ACTION_PACKAGE_ADDED.equals(action) ||
+ Intent.ACTION_PACKAGE_REMOVED.equals(action) ||
+ Intent.ACTION_PACKAGE_REPLACED.equals(action)) {
+ final int uid = getUidFromIntent(intent);
+ if (uid != -1) {
+ final UidRecord uidRec = mActiveUids.get(uid);
+ if (uidRec != null) {
+ uidRec.updateHasInternetPermission();
+ }
+ }
+ }
}
// Add to the sticky list if requested.
@@ -19329,6 +19340,18 @@
return ActivityManager.BROADCAST_SUCCESS;
}
+ /**
+ * @return uid from the extra field {@link Intent#EXTRA_UID} if present, Otherwise -1
+ */
+ private int getUidFromIntent(Intent intent) {
+ if (intent == null) {
+ return -1;
+ }
+ final Bundle intentExtras = intent.getExtras();
+ return intent.hasExtra(Intent.EXTRA_UID)
+ ? intentExtras.getInt(Intent.EXTRA_UID) : -1;
+ }
+
final void rotateBroadcastStatsIfNeededLocked() {
final long now = SystemClock.elapsedRealtime();
if (mCurBroadcastStats == null ||
@@ -22547,6 +22570,9 @@
if (!mInjector.isNetworkRestrictedForUid(uidRec.uid)) {
continue;
}
+ if (!UserHandle.isApp(uidRec.uid) || !uidRec.hasInternetPermission) {
+ continue;
+ }
// If process state is not changed, then there's nothing to do.
if (uidRec.setProcState == uidRec.curProcState) {
continue;
@@ -22557,7 +22583,7 @@
if (blockState == NETWORK_STATE_NO_CHANGE) {
continue;
}
- synchronized (uidRec.lock) {
+ synchronized (uidRec.networkStateLock) {
uidRec.curProcStateSeq = ++mProcStateSeqCounter;
if (blockState == NETWORK_STATE_BLOCK) {
if (blockingUids == null) {
@@ -22570,7 +22596,7 @@
+ " threads for uid: " + uidRec);
}
if (uidRec.waitingForNetwork) {
- uidRec.lock.notifyAll();
+ uidRec.networkStateLock.notifyAll();
}
}
}
@@ -23500,7 +23526,7 @@
return;
}
}
- synchronized (record.lock) {
+ synchronized (record.networkStateLock) {
if (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) {
if (DEBUG_NETWORK) {
Slog.d(TAG_NETWORK, "procStateSeq: " + procStateSeq + " has already"
@@ -23522,7 +23548,7 @@
Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid
+ ", procStateSeq: " + procStateSeq);
}
- record.lock.notifyAll();
+ record.networkStateLock.notifyAll();
}
}
}
@@ -23547,7 +23573,7 @@
return;
}
}
- synchronized (record.lock) {
+ synchronized (record.networkStateLock) {
if (record.lastDispatchedProcStateSeq < procStateSeq) {
if (DEBUG_NETWORK) {
Slog.d(TAG_NETWORK, "Uid state change for seq no. " + procStateSeq + " is not "
@@ -23581,7 +23607,7 @@
}
final long startTime = SystemClock.uptimeMillis();
record.waitingForNetwork = true;
- record.lock.wait(mWaitForNetworkTimeoutMs);
+ record.networkStateLock.wait(mWaitForNetworkTimeoutMs);
record.waitingForNetwork = false;
final long totalTime = SystemClock.uptimeMillis() - startTime;
if (totalTime >= mWaitForNetworkTimeoutMs) {
diff --git a/services/core/java/com/android/server/am/UidRecord.java b/services/core/java/com/android/server/am/UidRecord.java
index 67b80f6..c0fb77f 100644
--- a/services/core/java/com/android/server/am/UidRecord.java
+++ b/services/core/java/com/android/server/am/UidRecord.java
@@ -16,7 +16,9 @@
package com.android.server.am;
+import android.Manifest;
import android.app.ActivityManager;
+import android.content.pm.PackageManager;
import android.os.SystemClock;
import android.os.UserHandle;
import android.util.TimeUtils;
@@ -43,30 +45,37 @@
* {@link ActivityManagerService#mProcStateSeqCounter}
* when {@link #curProcState} changes from background to foreground or vice versa.
*/
- @GuardedBy("lock")
+ @GuardedBy("networkStateUpdate")
long curProcStateSeq;
/**
* Last seq number for which NetworkPolicyManagerService notified ActivityManagerService that
* network policies rules were updated.
*/
- @GuardedBy("lock")
+ @GuardedBy("networkStateUpdate")
long lastNetworkUpdatedProcStateSeq;
/**
* Last seq number for which AcitivityManagerService dispatched uid state change to
* NetworkPolicyManagerService.
*/
- @GuardedBy("lock")
+ @GuardedBy("networkStateUpdate")
long lastDispatchedProcStateSeq;
/**
* Indicates if any thread is waiting for network rules to get updated for {@link #uid}.
*/
- @GuardedBy("lock")
- boolean waitingForNetwork;
+ volatile boolean waitingForNetwork;
- final Object lock = new Object();
+ /**
+ * Indicates whether this uid has internet permission or not.
+ */
+ volatile boolean hasInternetPermission;
+
+ /**
+ * This object is used for waiting for the network state to get updated.
+ */
+ final Object networkStateLock = new Object();
static final int CHANGE_PROCSTATE = 0;
static final int CHANGE_GONE = 1;
@@ -95,6 +104,11 @@
curProcState = ActivityManager.PROCESS_STATE_CACHED_EMPTY;
}
+ public void updateHasInternetPermission() {
+ hasInternetPermission = ActivityManager.checkUidPermission(Manifest.permission.INTERNET,
+ uid) == PackageManager.PERMISSION_GRANTED;
+ }
+
/**
* If the change being dispatched is neither CHANGE_GONE nor CHANGE_GONE_IDLE (not interested in
* these changes), then update the {@link #lastDispatchedProcStateSeq} with
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 e7c91c0..bce87dc 100644
--- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java
@@ -105,9 +105,9 @@
final UidRecord record2 = addActiveUidRecord(TEST_UID2, curProcStateSeq,
lastNetworkUpdatedProcStateSeq);
- final CustomThread thread1 = new CustomThread(record1.lock);
+ final CustomThread thread1 = new CustomThread(record1.networkStateLock);
thread1.startAndWait("Unexpected state for " + record1);
- final CustomThread thread2 = new CustomThread(record2.lock);
+ final CustomThread thread2 = new CustomThread(record2.networkStateLock);
thread2.startAndWait("Unexpected state for " + record2);
mAmi.notifyNetworkPolicyRulesUpdated(TEST_UID1, expectedProcStateSeq);
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 1e038df..092c60b 100644
--- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java
@@ -105,7 +105,7 @@
public class ActivityManagerServiceTest {
private static final String TAG = ActivityManagerServiceTest.class.getSimpleName();
- private static final int TEST_UID = 111;
+ private static final int TEST_UID = 11111;
private static final long TEST_PROC_STATE_SEQ1 = 555;
private static final long TEST_PROC_STATE_SEQ2 = 556;
@@ -121,6 +121,7 @@
@Mock private Context mContext;
@Mock private AppOpsService mAppOpsService;
@Mock private PackageManager mPackageManager;
+ @Mock private BatteryStatsImpl mBatteryStatsImpl;
private TestInjector mInjector;
private ActivityManagerService mAms;
@@ -149,20 +150,9 @@
@MediumTest
@Test
public void incrementProcStateSeqAndNotifyAppsLocked() throws Exception {
- final UidRecord uidRec = new UidRecord(TEST_UID);
- uidRec.waitingForNetwork = true;
- mAms.mActiveUids.put(TEST_UID, uidRec);
- final BatteryStatsImpl batteryStats = Mockito.mock(BatteryStatsImpl.class);
- final ProcessRecord appRec = new ProcessRecord(batteryStats,
- new ApplicationInfo(), TAG, TEST_UID);
- appRec.thread = Mockito.mock(IApplicationThread.class);
- mAms.mLruProcesses.add(appRec);
-
- final ProcessRecord appRec2 = new ProcessRecord(batteryStats,
- new ApplicationInfo(), TAG, TEST_UID + 1);
- appRec2.thread = Mockito.mock(IApplicationThread.class);
- mAms.mLruProcesses.add(appRec2);
+ final UidRecord uidRec = addUidRecord(TEST_UID);
+ addUidRecord(TEST_UID + 1);
// Uid state is not moving from background to foreground or vice versa.
verifySeqCounterAndInteractions(uidRec,
@@ -235,12 +225,51 @@
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);
+ uidRec.waitingForNetwork = true;
+ uidRec.hasInternetPermission = true;
+ mAms.mActiveUids.put(uid, uidRec);
+
+ final ProcessRecord appRec = new ProcessRecord(mBatteryStatsImpl,
+ new ApplicationInfo(), TAG, uid);
+ appRec.thread = Mockito.mock(IApplicationThread.class);
+ mAms.mLruProcesses.add(appRec);
+
+ return uidRec;
}
private void verifySeqCounterAndInteractions(UidRecord uidRec, int prevState, int curState,
int expectedGlobalCounter, int expectedCurProcStateSeq, int expectedBlockState,
boolean expectNotify) throws Exception {
- CustomThread thread = new CustomThread(uidRec.lock);
+ CustomThread thread = new CustomThread(uidRec.networkStateLock);
thread.startAndWait("Unexpected state for " + uidRec);
uidRec.setProcState = prevState;
@@ -720,7 +749,7 @@
record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq;
mAms.mActiveUids.put(Process.myUid(), record);
- CustomThread thread = new CustomThread(record.lock, new Runnable() {
+ CustomThread thread = new CustomThread(record.networkStateLock, new Runnable() {
@Override
public void run() {
mAms.waitForNetworkStateUpdate(procStateSeqToWait);
@@ -730,8 +759,8 @@
if (expectWait) {
thread.startAndWait(errMsg, true);
thread.assertTimedWaiting(errMsg);
- synchronized (record.lock) {
- record.lock.notifyAll();
+ synchronized (record.networkStateLock) {
+ record.networkStateLock.notifyAll();
}
thread.assertTerminated(errMsg);
assertTrue(thread.mNotified);