Merge "Don't increment procStateSeq if uid doesn't have internet permission." into oc-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
20959b2d40
@@ -6556,6 +6556,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
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);
|
||||
@@ -18871,9 +18872,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
}
|
||||
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);
|
||||
@@ -19069,6 +19068,18 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
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.
|
||||
@@ -19335,6 +19346,18 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
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 ||
|
||||
@@ -22553,6 +22576,9 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
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;
|
||||
@@ -22563,7 +22589,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
if (blockState == NETWORK_STATE_NO_CHANGE) {
|
||||
continue;
|
||||
}
|
||||
synchronized (uidRec.lock) {
|
||||
synchronized (uidRec.networkStateLock) {
|
||||
uidRec.curProcStateSeq = ++mProcStateSeqCounter;
|
||||
if (blockState == NETWORK_STATE_BLOCK) {
|
||||
if (blockingUids == null) {
|
||||
@@ -22576,7 +22602,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
+ " threads for uid: " + uidRec);
|
||||
}
|
||||
if (uidRec.waitingForNetwork) {
|
||||
uidRec.lock.notifyAll();
|
||||
uidRec.networkStateLock.notifyAll();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -23506,7 +23532,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
return;
|
||||
}
|
||||
}
|
||||
synchronized (record.lock) {
|
||||
synchronized (record.networkStateLock) {
|
||||
if (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) {
|
||||
if (DEBUG_NETWORK) {
|
||||
Slog.d(TAG_NETWORK, "procStateSeq: " + procStateSeq + " has already"
|
||||
@@ -23528,7 +23554,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid
|
||||
+ ", procStateSeq: " + procStateSeq);
|
||||
}
|
||||
record.lock.notifyAll();
|
||||
record.networkStateLock.notifyAll();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -23553,7 +23579,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
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 "
|
||||
@@ -23587,7 +23613,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
||||
}
|
||||
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) {
|
||||
|
||||
@@ -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 @@ public final class UidRecord {
|
||||
* {@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 @@ public final class UidRecord {
|
||||
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
|
||||
|
||||
@@ -105,9 +105,9 @@ public class ActivityManagerInternalTest {
|
||||
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);
|
||||
|
||||
@@ -105,7 +105,7 @@ import java.util.function.Function;
|
||||
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 @@ public class ActivityManagerServiceTest {
|
||||
@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 @@ public class ActivityManagerServiceTest {
|
||||
@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 @@ public class ActivityManagerServiceTest {
|
||||
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 @@ public class ActivityManagerServiceTest {
|
||||
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 @@ public class ActivityManagerServiceTest {
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user