Merge "Don't increment procStateSeq if uid doesn't have internet permission."... (cb363ec4) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/am/ActivityManagerService.java

+35 −9

Original line number	Original line	Diff line number	Diff line
	@@ -6556,6 +6556,7 @@ public class ActivityManagerService extends IActivityManager.Stub
	if (Arrays.binarySearch(mDeviceIdleTempWhitelist, UserHandle.getAppId(proc.uid)) >= 0) {		if (Arrays.binarySearch(mDeviceIdleTempWhitelist, UserHandle.getAppId(proc.uid)) >= 0) {
	uidRec.setWhitelist = uidRec.curWhitelist = true;		uidRec.setWhitelist = uidRec.curWhitelist = true;
	}		}
			uidRec.updateHasInternetPermission();
	mActiveUids.put(proc.uid, uidRec);		mActiveUids.put(proc.uid, uidRec);
	noteUidProcessState(uidRec.uid, uidRec.curProcState);		noteUidProcessState(uidRec.uid, uidRec.curProcState);
	enqueueUidChangeLocked(uidRec, -1, UidRecord.CHANGE_ACTIVE);		enqueueUidChangeLocked(uidRec, -1, UidRecord.CHANGE_ACTIVE);
	@@ -18871,9 +18872,7 @@ public class ActivityManagerService extends IActivityManager.Stub
	}		}
	switch (action) {		switch (action) {
	case Intent.ACTION_UID_REMOVED:		case Intent.ACTION_UID_REMOVED:
	final Bundle intentExtras = intent.getExtras();		final int uid = getUidFromIntent(intent);
	final int uid = intentExtras != null
	? intentExtras.getInt(Intent.EXTRA_UID) : -1;
	if (uid >= 0) {		if (uid >= 0) {
	mBatteryStatsService.removeUid(uid);		mBatteryStatsService.removeUid(uid);
	mAppOpsService.uidRemoved(uid);		mAppOpsService.uidRemoved(uid);
	@@ -19069,6 +19068,18 @@ public class ActivityManagerService extends IActivityManager.Stub
	mHandler.sendEmptyMessage(HANDLE_TRUST_STORAGE_UPDATE_MSG);		mHandler.sendEmptyMessage(HANDLE_TRUST_STORAGE_UPDATE_MSG);
	break;		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.		// Add to the sticky list if requested.
	@@ -19335,6 +19346,18 @@ public class ActivityManagerService extends IActivityManager.Stub
	return ActivityManager.BROADCAST_SUCCESS;		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 void rotateBroadcastStatsIfNeededLocked() {
	final long now = SystemClock.elapsedRealtime();		final long now = SystemClock.elapsedRealtime();
	if (mCurBroadcastStats == null \|\|		if (mCurBroadcastStats == null \|\|
	@@ -22553,6 +22576,9 @@ public class ActivityManagerService extends IActivityManager.Stub
	if (!mInjector.isNetworkRestrictedForUid(uidRec.uid)) {		if (!mInjector.isNetworkRestrictedForUid(uidRec.uid)) {
	continue;		continue;
	}		}
			if (!UserHandle.isApp(uidRec.uid) \|\| !uidRec.hasInternetPermission) {
			continue;
			}
	// If process state is not changed, then there's nothing to do.		// If process state is not changed, then there's nothing to do.
	if (uidRec.setProcState == uidRec.curProcState) {		if (uidRec.setProcState == uidRec.curProcState) {
	continue;		continue;
	@@ -22563,7 +22589,7 @@ public class ActivityManagerService extends IActivityManager.Stub
	if (blockState == NETWORK_STATE_NO_CHANGE) {		if (blockState == NETWORK_STATE_NO_CHANGE) {
	continue;		continue;
	}		}
	synchronized (uidRec.lock) {		synchronized (uidRec.networkStateLock) {
	uidRec.curProcStateSeq = ++mProcStateSeqCounter;		uidRec.curProcStateSeq = ++mProcStateSeqCounter;
	if (blockState == NETWORK_STATE_BLOCK) {		if (blockState == NETWORK_STATE_BLOCK) {
	if (blockingUids == null) {		if (blockingUids == null) {
	@@ -22576,7 +22602,7 @@ public class ActivityManagerService extends IActivityManager.Stub
	+ " threads for uid: " + uidRec);		+ " threads for uid: " + uidRec);
	}		}
	if (uidRec.waitingForNetwork) {		if (uidRec.waitingForNetwork) {
	uidRec.lock.notifyAll();		uidRec.networkStateLock.notifyAll();
	}		}
	}		}
	}		}
	@@ -23506,7 +23532,7 @@ public class ActivityManagerService extends IActivityManager.Stub
	return;		return;
	}		}
	}		}
	synchronized (record.lock) {		synchronized (record.networkStateLock) {
	if (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) {		if (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) {
	if (DEBUG_NETWORK) {		if (DEBUG_NETWORK) {
	Slog.d(TAG_NETWORK, "procStateSeq: " + procStateSeq + " has already"		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		Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid
	+ ", procStateSeq: " + procStateSeq);		+ ", procStateSeq: " + procStateSeq);
	}		}
	record.lock.notifyAll();		record.networkStateLock.notifyAll();
	}		}
	}		}
	}		}
	@@ -23553,7 +23579,7 @@ public class ActivityManagerService extends IActivityManager.Stub
	return;		return;
	}		}
	}		}
	synchronized (record.lock) {		synchronized (record.networkStateLock) {
	if (record.lastDispatchedProcStateSeq < procStateSeq) {		if (record.lastDispatchedProcStateSeq < procStateSeq) {
	if (DEBUG_NETWORK) {		if (DEBUG_NETWORK) {
	Slog.d(TAG_NETWORK, "Uid state change for seq no. " + procStateSeq + " is not "		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();		final long startTime = SystemClock.uptimeMillis();
	record.waitingForNetwork = true;		record.waitingForNetwork = true;
	record.lock.wait(mWaitForNetworkTimeoutMs);		record.networkStateLock.wait(mWaitForNetworkTimeoutMs);
	record.waitingForNetwork = false;		record.waitingForNetwork = false;
	final long totalTime = SystemClock.uptimeMillis() - startTime;		final long totalTime = SystemClock.uptimeMillis() - startTime;
	if (totalTime >= mWaitForNetworkTimeoutMs) {		if (totalTime >= mWaitForNetworkTimeoutMs) {

services/core/java/com/android/server/am/UidRecord.java

+20 −6

Original line number	Original line	Diff line number	Diff line
	@@ -16,7 +16,9 @@

	package com.android.server.am;		package com.android.server.am;

			import android.Manifest;
	import android.app.ActivityManager;		import android.app.ActivityManager;
			import android.content.pm.PackageManager;
	import android.os.SystemClock;		import android.os.SystemClock;
	import android.os.UserHandle;		import android.os.UserHandle;
	import android.util.TimeUtils;		import android.util.TimeUtils;
	@@ -43,30 +45,37 @@ public final class UidRecord {
	* {@link ActivityManagerService#mProcStateSeqCounter}		* {@link ActivityManagerService#mProcStateSeqCounter}
	* when {@link #curProcState} changes from background to foreground or vice versa.		* when {@link #curProcState} changes from background to foreground or vice versa.
	*/		*/
	@GuardedBy("lock")		@GuardedBy("networkStateUpdate")
	long curProcStateSeq;		long curProcStateSeq;

	/**		/**
	* Last seq number for which NetworkPolicyManagerService notified ActivityManagerService that		* Last seq number for which NetworkPolicyManagerService notified ActivityManagerService that
	* network policies rules were updated.		* network policies rules were updated.
	*/		*/
	@GuardedBy("lock")		@GuardedBy("networkStateUpdate")
	long lastNetworkUpdatedProcStateSeq;		long lastNetworkUpdatedProcStateSeq;

	/**		/**
	* Last seq number for which AcitivityManagerService dispatched uid state change to		* Last seq number for which AcitivityManagerService dispatched uid state change to
	* NetworkPolicyManagerService.		* NetworkPolicyManagerService.
	*/		*/
	@GuardedBy("lock")		@GuardedBy("networkStateUpdate")
	long lastDispatchedProcStateSeq;		long lastDispatchedProcStateSeq;

	/**		/**
	* Indicates if any thread is waiting for network rules to get updated for {@link #uid}.		* Indicates if any thread is waiting for network rules to get updated for {@link #uid}.
	*/		*/
	@GuardedBy("lock")		volatile boolean waitingForNetwork;
	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_PROCSTATE = 0;
	static final int CHANGE_GONE = 1;		static final int CHANGE_GONE = 1;
	@@ -95,6 +104,11 @@ public final class UidRecord {
	curProcState = ActivityManager.PROCESS_STATE_CACHED_EMPTY;		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		* If the change being dispatched is neither CHANGE_GONE nor CHANGE_GONE_IDLE (not interested in
	* these changes), then update the {@link #lastDispatchedProcStateSeq} with		* these changes), then update the {@link #lastDispatchedProcStateSeq} with

services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java

+2 −2

Original line number	Original line	Diff line number	Diff line
	@@ -105,9 +105,9 @@ public class ActivityManagerInternalTest {
	final UidRecord record2 = addActiveUidRecord(TEST_UID2, curProcStateSeq,		final UidRecord record2 = addActiveUidRecord(TEST_UID2, curProcStateSeq,
	lastNetworkUpdatedProcStateSeq);		lastNetworkUpdatedProcStateSeq);

	final CustomThread thread1 = new CustomThread(record1.lock);		final CustomThread thread1 = new CustomThread(record1.networkStateLock);
	thread1.startAndWait("Unexpected state for " + record1);		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);		thread2.startAndWait("Unexpected state for " + record2);

	mAmi.notifyNetworkPolicyRulesUpdated(TEST_UID1, expectedProcStateSeq);		mAmi.notifyNetworkPolicyRulesUpdated(TEST_UID1, expectedProcStateSeq);

services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java

+47 −18

Original line number	Original line	Diff line number	Diff line
	@@ -105,7 +105,7 @@ import java.util.function.Function;
	public class ActivityManagerServiceTest {		public class ActivityManagerServiceTest {
	private static final String TAG = ActivityManagerServiceTest.class.getSimpleName();		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_SEQ1 = 555;
	private static final long TEST_PROC_STATE_SEQ2 = 556;		private static final long TEST_PROC_STATE_SEQ2 = 556;
	@@ -121,6 +121,7 @@ public class ActivityManagerServiceTest {
	@Mock private Context mContext;		@Mock private Context mContext;
	@Mock private AppOpsService mAppOpsService;		@Mock private AppOpsService mAppOpsService;
	@Mock private PackageManager mPackageManager;		@Mock private PackageManager mPackageManager;
			@Mock private BatteryStatsImpl mBatteryStatsImpl;

	private TestInjector mInjector;		private TestInjector mInjector;
	private ActivityManagerService mAms;		private ActivityManagerService mAms;
	@@ -149,20 +150,9 @@ public class ActivityManagerServiceTest {
	@MediumTest		@MediumTest
	@Test		@Test
	public void incrementProcStateSeqAndNotifyAppsLocked() throws Exception {		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 UidRecord uidRec = addUidRecord(TEST_UID);
	final ProcessRecord appRec = new ProcessRecord(batteryStats,		addUidRecord(TEST_UID + 1);
	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);

	// Uid state is not moving from background to foreground or vice versa.		// Uid state is not moving from background to foreground or vice versa.
	verifySeqCounterAndInteractions(uidRec,		verifySeqCounterAndInteractions(uidRec,
	@@ -235,12 +225,51 @@ public class ActivityManagerServiceTest {
	44, // exptectedCurProcStateSeq		44, // exptectedCurProcStateSeq
	-1, // expectedBlockState, -1 to verify there are no interactions with main thread.		-1, // expectedBlockState, -1 to verify there are no interactions with main thread.
	false); // expectNotify		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,		private void verifySeqCounterAndInteractions(UidRecord uidRec, int prevState, int curState,
	int expectedGlobalCounter, int expectedCurProcStateSeq, int expectedBlockState,		int expectedGlobalCounter, int expectedCurProcStateSeq, int expectedBlockState,
	boolean expectNotify) throws Exception {		boolean expectNotify) throws Exception {
	CustomThread thread = new CustomThread(uidRec.lock);		CustomThread thread = new CustomThread(uidRec.networkStateLock);
	thread.startAndWait("Unexpected state for " + uidRec);		thread.startAndWait("Unexpected state for " + uidRec);

	uidRec.setProcState = prevState;		uidRec.setProcState = prevState;
	@@ -720,7 +749,7 @@ public class ActivityManagerServiceTest {
	record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq;		record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq;
	mAms.mActiveUids.put(Process.myUid(), record);		mAms.mActiveUids.put(Process.myUid(), record);

	CustomThread thread = new CustomThread(record.lock, new Runnable() {		CustomThread thread = new CustomThread(record.networkStateLock, new Runnable() {
	@Override		@Override
	public void run() {		public void run() {
	mAms.waitForNetworkStateUpdate(procStateSeqToWait);		mAms.waitForNetworkStateUpdate(procStateSeqToWait);
	@@ -730,8 +759,8 @@ public class ActivityManagerServiceTest {
	if (expectWait) {		if (expectWait) {
	thread.startAndWait(errMsg, true);		thread.startAndWait(errMsg, true);
	thread.assertTimedWaiting(errMsg);		thread.assertTimedWaiting(errMsg);
	synchronized (record.lock) {		synchronized (record.networkStateLock) {
	record.lock.notifyAll();		record.networkStateLock.notifyAll();
	}		}
	thread.assertTerminated(errMsg);		thread.assertTerminated(errMsg);
	assertTrue(thread.mNotified);		assertTrue(thread.mNotified);