Merge "Do not notify onOpNoted if less than 5s"

3 files changed, 50 insertions, 8 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/appops/AppOpsController.java b/packages/SystemUI/src/com/android/systemui/appops/AppOpsController.java
index 4966fc62c542..2661d8913a3f 100644
--- a/packages/SystemUI/src/com/android/systemui/appops/AppOpsController.java
+++ b/packages/SystemUI/src/com/android/systemui/appops/AppOpsController.java
@@ -27,7 +27,9 @@ import java.util.List;
 public interface AppOpsController {
 
     /**
-     * Callback to notify when the state of active AppOps tracked by the controller has changed
+     * Callback to notify when the state of active AppOps tracked by the controller has changed.
+     * AppOps that are noted will not be notified every time, just when the tracked state changes
+     * between currently in use and not.
      */
     interface Callback {
         void onActiveStateChanged(int code, int uid, String packageName, boolean active);
diff --git a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
index 139bfd55eab7..bad6b5458aa3 100644
--- a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
@@ -52,6 +52,9 @@ public class AppOpsControllerImpl implements AppOpsController,
         AppOpsManager.OnOpActiveChangedInternalListener,
         AppOpsManager.OnOpNotedListener, Dumpable {
 
+    // This is the minimum time that we will keep AppOps that are noted on record. If multiple
+    // occurrences of the same (op, package, uid) happen in a shorter interval, they will not be
+    // notified to listeners.
     private static final long NOTED_OP_TIME_DELAY_MS = 5000;
     private static final String TAG = "AppOpsControllerImpl";
     private static final boolean DEBUG = false;
@@ -167,7 +170,8 @@ public class AppOpsControllerImpl implements AppOpsController,
         if (mCallbacks.isEmpty()) setListening(false);
     }
 
-    private AppOpItem getAppOpItem(List<AppOpItem> appOpList, int code, int uid,
+    // Find item number in list, only call if the list passed is locked
+    private AppOpItem getAppOpItemLocked(List<AppOpItem> appOpList, int code, int uid,
             String packageName) {
         final int itemsQ = appOpList.size();
         for (int i = 0; i < itemsQ; i++) {
@@ -182,7 +186,7 @@ public class AppOpsControllerImpl implements AppOpsController,
 
     private boolean updateActives(int code, int uid, String packageName, boolean active) {
         synchronized (mActiveItems) {
-            AppOpItem item = getAppOpItem(mActiveItems, code, uid, packageName);
+            AppOpItem item = getAppOpItemLocked(mActiveItems, code, uid, packageName);
             if (item == null && active) {
                 item = new AppOpItem(code, uid, packageName, System.currentTimeMillis());
                 mActiveItems.add(item);
@@ -200,7 +204,7 @@ public class AppOpsControllerImpl implements AppOpsController,
     private void removeNoted(int code, int uid, String packageName) {
         AppOpItem item;
         synchronized (mNotedItems) {
-            item = getAppOpItem(mNotedItems, code, uid, packageName);
+            item = getAppOpItemLocked(mNotedItems, code, uid, packageName);
             if (item == null) return;
             mNotedItems.remove(item);
             if (DEBUG) Log.w(TAG, "Removed item: " + item.toString());
@@ -208,17 +212,20 @@ public class AppOpsControllerImpl implements AppOpsController,
         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);
+            item = getAppOpItemLocked(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;
             }
         }
         mBGHandler.scheduleRemoval(item, NOTED_OP_TIME_DELAY_MS);
+        return createdNew;
     }
 
     /**
@@ -329,13 +336,15 @@ public class AppOpsControllerImpl implements AppOpsController,
             Log.w(TAG, "Op: " + code + " with result " + AppOpsManager.MODE_NAMES[result]);
         }
         if (result != AppOpsManager.MODE_ALLOWED) return;
-        addNoted(code, uid, packageName);
-        mBGHandler.post(() -> notifySuscribers(code, uid, packageName, true));
+        if (addNoted(code, uid, packageName)) {
+            mBGHandler.post(() -> notifySuscribers(code, uid, packageName, true));
+        }
     }
 
     private void notifySuscribers(int code, int uid, String packageName, boolean active) {
         if (mCallbacksByCode.containsKey(code)
                 && isUserVisible(code, uid, packageName)) {
+            if (DEBUG) Log.d(TAG, "Notifying of change in package " + packageName);
             for (Callback cb: mCallbacksByCode.get(code)) {
                 cb.onActiveStateChanged(code, uid, packageName, active);
             }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
index 26185e101346..af0ef8298a3f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
@@ -48,6 +48,8 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.util.List;
+
 @SmallTest
 @RunWith(AndroidTestingRunner.class)
 @TestableLooper.RunWithLooper
@@ -220,4 +222,33 @@ 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());
+    }
 }