Merge "Prevent sending early termination of appop use" into qt-dev am: c19818819e (c866e0c5) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java

+43 −8

Original line number	Diff line number	Diff line
		@@ -195,20 +195,32 @@ public class AppOpsControllerImpl implements AppOpsController,
		mNotedItems.remove(item);
		if (DEBUG) Log.w(TAG, "Removed item: " + item.toString());
		}
		boolean active;
		// Check if the item is also active
		synchronized (mActiveItems) {
		active = getAppOpItem(mActiveItems, code, uid, packageName) != null;
		}
		if (!active) {
		notifySuscribers(code, uid, packageName, false);
		}
		}

		private void addNoted(int code, int uid, String packageName) {
		private boolean addNoted(int code, int uid, String packageName) {
		AppOpItem item;
		boolean createdNew = false;
		synchronized (mNotedItems) {
		item = getAppOpItem(mNotedItems, code, uid, packageName);
		if (item == null) {
		item = new AppOpItem(code, uid, packageName, System.currentTimeMillis());
		mNotedItems.add(item);
		if (DEBUG) Log.w(TAG, "Added item: " + item.toString());
		createdNew = true;
		}
		}
		// We should keep this so we make sure it cannot time out.
		mBGHandler.removeCallbacksAndMessages(item);
		mBGHandler.scheduleRemoval(item, NOTED_OP_TIME_DELAY_MS);
		return createdNew;
		}

		/**
		@@ -255,23 +267,46 @@ public class AppOpsControllerImpl implements AppOpsController,

		@Override
		public void onOpActiveChanged(int code, int uid, String packageName, boolean active) {
		if (updateActives(code, uid, packageName, active)) {
		notifySuscribers(code, uid, packageName, active);
		if (DEBUG) {
		Log.w(TAG, String.format("onActiveChanged(%d,%d,%s,%s", code, uid, packageName,
		Boolean.toString(active)));
		}
		boolean activeChanged = updateActives(code, uid, packageName, active);
		if (!activeChanged) return; // early return
		// Check if the item is also noted, in that case, there's no update.
		boolean alsoNoted;
		synchronized (mNotedItems) {
		alsoNoted = getAppOpItem(mNotedItems, code, uid, packageName) != null;
		}
		// If active is true, we only send the update if the op is not actively noted (already true)
		// If active is false, we only send the update if the op is not actively noted (prevent
		// early removal)
		if (!alsoNoted) {
		mBGHandler.post(() -> notifySuscribers(code, uid, packageName, active));
		}
		}

		@Override
		public void onOpNoted(int code, int uid, String packageName, int result) {
		if (DEBUG) {
		Log.w(TAG, "Op: " + code + " with result " + AppOpsManager.MODE_NAMES[result]);
		Log.w(TAG, "Noted op: " + code + " with result "
		+ AppOpsManager.MODE_NAMES[result] + " for package " + packageName);
		}
		if (result != AppOpsManager.MODE_ALLOWED) return;
		addNoted(code, uid, packageName);
		notifySuscribers(code, uid, packageName, true);
		boolean notedAdded = addNoted(code, uid, packageName);
		if (!notedAdded) return; // early return
		boolean alsoActive;
		synchronized (mActiveItems) {
		alsoActive = getAppOpItem(mActiveItems, code, uid, packageName) != null;
		}
		if (!alsoActive) {
		mBGHandler.post(() -> notifySuscribers(code, uid, packageName, true));
		}
		}

		private void notifySuscribers(int code, int uid, String packageName, boolean active) {
		if (mCallbacksByCode.containsKey(code)) {
		if (DEBUG) Log.d(TAG, "Notifying of change in package " + packageName);
		for (Callback cb: mCallbacksByCode.get(code)) {
		cb.onActiveStateChanged(code, uid, packageName, active);
		}
		@@ -295,7 +330,7 @@ public class AppOpsControllerImpl implements AppOpsController,

		}

		protected final class H extends Handler {
		protected class H extends Handler {
		H(Looper looper) {
		super(looper);
		}

packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java

+136 −2

Original line number	Diff line number	Diff line
		@@ -28,6 +28,8 @@ import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.times;
		import static org.mockito.Mockito.verify;

		import static java.lang.Thread.sleep;

		import android.app.AppOpsManager;
		import android.content.pm.PackageManager;
		import android.os.UserHandle;
		@@ -36,7 +38,6 @@ import android.testing.TestableLooper;

		import androidx.test.filters.SmallTest;

		import com.android.systemui.Dependency;
		import com.android.systemui.SysuiTestCase;

		import org.junit.Before;
		@@ -45,6 +46,8 @@ import org.junit.runner.RunWith;
		import org.mockito.Mock;
		import org.mockito.MockitoAnnotations;

		import java.util.List;

		@SmallTest
		@RunWith(AndroidTestingRunner.class)
		@TestableLooper.RunWithLooper
		@@ -63,14 +66,16 @@ public class AppOpsControllerTest extends SysuiTestCase {
		private AppOpsControllerImpl.H mMockHandler;

		private AppOpsControllerImpl mController;
		private TestableLooper mTestableLooper;

		@Before
		public void setUp() {
		MockitoAnnotations.initMocks(this);
		mTestableLooper = TestableLooper.get(this);

		getContext().addMockSystemService(AppOpsManager.class, mAppOpsManager);

		mController = new AppOpsControllerImpl(mContext, Dependency.get(Dependency.BG_LOOPER));
		mController = new AppOpsControllerImpl(mContext, mTestableLooper.getLooper());
		}

		@Test
		@@ -94,6 +99,7 @@ public class AppOpsControllerTest extends SysuiTestCase {
		AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);
		mTestableLooper.processAllMessages();
		verify(mCallback).onActiveStateChanged(AppOpsManager.OP_RECORD_AUDIO,
		TEST_UID, TEST_PACKAGE_NAME, true);
		}
		@@ -103,6 +109,7 @@ public class AppOpsControllerTest extends SysuiTestCase {
		mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback);
		mController.onOpActiveChanged(
		AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
		mTestableLooper.processAllMessages();
		verify(mCallback, never()).onActiveStateChanged(
		anyInt(), anyInt(), anyString(), anyBoolean());
		}
		@@ -113,6 +120,7 @@ public class AppOpsControllerTest extends SysuiTestCase {
		mController.removeCallback(new int[]{AppOpsManager.OP_RECORD_AUDIO}, mCallback);
		mController.onOpActiveChanged(
		AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
		mTestableLooper.processAllMessages();
		verify(mCallback, never()).onActiveStateChanged(
		anyInt(), anyInt(), anyString(), anyBoolean());
		}
		@@ -123,6 +131,7 @@ public class AppOpsControllerTest extends SysuiTestCase {
		mController.removeCallback(new int[]{AppOpsManager.OP_CAMERA}, mCallback);
		mController.onOpActiveChanged(
		AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
		mTestableLooper.processAllMessages();
		verify(mCallback).onActiveStateChanged(AppOpsManager.OP_RECORD_AUDIO,
		TEST_UID, TEST_PACKAGE_NAME, true);
		}
		@@ -185,4 +194,129 @@ public class AppOpsControllerTest extends SysuiTestCase {
		verify(mMockHandler).removeCallbacksAndMessages(null);
		assertTrue(mController.getActiveAppOps().isEmpty());
		}

		@Test
		public void noDoubleUpdateOnOpNoted() {
		mController.setBGHandler(mMockHandler);

		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);
		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);

		// Only one post to notify subscribers
		verify(mMockHandler, times(1)).post(any());

		List<AppOpItem> list = mController.getActiveAppOps();
		assertEquals(1, list.size());
		}

		@Test
		public void onDoubleOPNoted_scheduleTwiceForRemoval() {
		mController.setBGHandler(mMockHandler);

		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);
		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);

		// Only one post to notify subscribers
		verify(mMockHandler, times(2)).scheduleRemoval(any(), anyLong());
		}

		@Test
		public void testActiveOpNotRemovedAfterNoted() throws InterruptedException {
		// Replaces the timeout delay with 5 ms
		AppOpsControllerImpl.H testHandler = mController.new H(mTestableLooper.getLooper()) {
		@Override
		public void scheduleRemoval(AppOpItem item, long timeToRemoval) {
		super.scheduleRemoval(item, 5L);
		}
		};

		mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback);
		mController.setBGHandler(testHandler);

		mController.onOpActiveChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);

		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);

		mTestableLooper.processAllMessages();
		List<AppOpItem> list = mController.getActiveAppOps();
		verify(mCallback).onActiveStateChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);

		// Duplicates are not removed between active and noted
		assertEquals(2, list.size());

		sleep(10L);

		mTestableLooper.processAllMessages();

		verify(mCallback, never()).onActiveStateChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, false);
		list = mController.getActiveAppOps();
		assertEquals(1, list.size());
		}

		@Test
		public void testNotedNotRemovedAfterActive() {
		mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback);

		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);

		mController.onOpActiveChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);

		mTestableLooper.processAllMessages();
		List<AppOpItem> list = mController.getActiveAppOps();
		verify(mCallback).onActiveStateChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);

		// Duplicates are not removed between active and noted
		assertEquals(2, list.size());

		mController.onOpActiveChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, false);

		mTestableLooper.processAllMessages();

		verify(mCallback, never()).onActiveStateChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, false);
		list = mController.getActiveAppOps();
		assertEquals(1, list.size());
		}

		@Test
		public void testNotedAndActiveOnlyOneCall() {
		mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback);

		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);

		mController.onOpActiveChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);

		mTestableLooper.processAllMessages();
		verify(mCallback).onActiveStateChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);
		}

		@Test
		public void testActiveAndNotedOnlyOneCall() {
		mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback);

		mController.onOpActiveChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);

		mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
		AppOpsManager.MODE_ALLOWED);

		mTestableLooper.processAllMessages();
		verify(mCallback).onActiveStateChanged(
		AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true);
		}
		}