From 9159006d089ea853ae6982988cfd9f08b97bfee7 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Mon, 2 Apr 2018 16:24:11 -0400 Subject: [PATCH] Only tag foreground notifications With active permission icons Test: atest SystemUITests Change-Id: I31828f6239b1253794f9569d29dccdac5c10b0da Fixes: 75276447 --- .../systemui/ForegroundServiceController.java | 2 +- .../systemui/statusbar/AppOpsListener.java | 2 +- .../systemui/statusbar/NotificationData.java | 19 +---- .../statusbar/NotificationEntryManager.java | 24 +++++-- .../statusbar/AppOpsListenerTest.java | 2 +- .../statusbar/NotificationDataTest.java | 33 ++------- .../NotificationEntryManagerTest.java | 70 ++++++++++++++++--- 7 files changed, 92 insertions(+), 60 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java index 5a2263cf26c73..ae6ee2af29fd8 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceController.java @@ -73,7 +73,7 @@ public interface ForegroundServiceController { void onAppOpChanged(int code, int uid, String packageName, boolean active); /** - * Gets active app ops for this user and package. + * Gets active app ops for this user and package */ @Nullable ArraySet getAppOps(int userId, String packageName); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/AppOpsListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/AppOpsListener.java index 2ec78cfe93825..019c6807478b7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/AppOpsListener.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/AppOpsListener.java @@ -62,7 +62,7 @@ public class AppOpsListener implements AppOpsManager.OnOpActiveChangedListener { public void onOpActiveChanged(int code, int uid, String packageName, boolean active) { mFsc.onAppOpChanged(code, uid, packageName, active); mPresenter.getHandler().post(() -> { - mEntryManager.updateNotificationsForAppOps(code, uid, packageName, active); + mEntryManager.updateNotificationsForAppOp(code, uid, packageName, active); }); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java index 4b6ab64ab6249..ab46b39a4ca34 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java @@ -383,8 +383,6 @@ public class NotificationData { } mGroupManager.onEntryAdded(entry); - updateAppOps(entry); - updateRankingAndSort(mRankingMap); } @@ -403,25 +401,14 @@ public class NotificationData { updateRankingAndSort(ranking); } - private void updateAppOps(Entry entry) { - final int uid = entry.notification.getUid(); - final String pkg = entry.notification.getPackageName(); - ArraySet activeOps = mFsc.getAppOps(entry.notification.getUserId(), pkg); - if (activeOps != null) { - int N = activeOps.size(); - for (int i = 0; i < N; i++) { - updateAppOp(activeOps.valueAt(i), uid, pkg, true); - } - } - } - - public void updateAppOp(int appOp, int uid, String pkg, boolean showIcon) { + public void updateAppOp(int appOp, int uid, String pkg, String key, boolean showIcon) { synchronized (mEntries) { final int N = mEntries.size(); for (int i = 0; i < N; i++) { Entry entry = mEntries.valueAt(i); if (uid == entry.notification.getUid() - && pkg.equals(entry.notification.getPackageName())) { + && pkg.equals(entry.notification.getPackageName()) + && key.equals(entry.key)) { if (showIcon) { entry.mActiveAppOps.add(appOp); } else { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java index 45df4505fac2b..849cfdd9f7504 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java @@ -43,6 +43,7 @@ import android.util.Log; import android.view.View; import android.view.ViewGroup; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.logging.MetricsLogger; import com.android.internal.statusbar.IStatusBarService; import com.android.internal.util.NotificationMessagingUtil; @@ -665,6 +666,7 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. } // Add the expanded view and icon. mNotificationData.add(entry); + tagForeground(entry.notification); updateNotifications(); } @@ -726,6 +728,19 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. mPendingNotifications.put(key, shadeEntry); } + @VisibleForTesting + protected void tagForeground(StatusBarNotification notification) { + ArraySet activeOps = mForegroundServiceController.getAppOps( + notification.getUserId(), notification.getPackageName()); + if (activeOps != null) { + int N = activeOps.size(); + for (int i = 0; i < N; i++) { + updateNotificationsForAppOp(activeOps.valueAt(i), notification.getUid(), + notification.getPackageName(), true); + } + } + } + @Override public void addNotification(StatusBarNotification notification, NotificationListenerService.RankingMap ranking) { @@ -736,10 +751,11 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. } } - public void updateNotificationsForAppOps(int appOp, int uid, String pkg, boolean showIcon) { - if (mForegroundServiceController.getStandardLayoutKey( - UserHandle.getUserId(uid), pkg) != null) { - mNotificationData.updateAppOp(appOp, uid, pkg, showIcon); + public void updateNotificationsForAppOp(int appOp, int uid, String pkg, boolean showIcon) { + String foregroundKey = mForegroundServiceController.getStandardLayoutKey( + UserHandle.getUserId(uid), pkg); + if (foregroundKey != null) { + mNotificationData.updateAppOp(appOp, uid, pkg, foregroundKey, showIcon); updateNotifications(); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/AppOpsListenerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/AppOpsListenerTest.java index 2a48c4b67e0ec..dc0e2b0ef495e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/AppOpsListenerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/AppOpsListenerTest.java @@ -86,7 +86,7 @@ public class AppOpsListenerTest extends SysuiTestCase { mListener.onOpActiveChanged( AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); waitForIdleSync(mHandler); - verify(mEntryManager, times(1)).updateNotificationsForAppOps( + verify(mEntryManager, times(1)).updateNotificationsForAppOp( AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java index 5e27fde044418..5ec77acfa7283 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java @@ -139,32 +139,7 @@ public class NotificationDataTest extends SysuiTestCase { Assert.assertTrue(mRow.getEntry().channel != null); } - @Test - public void testAdd_appOpsAdded() { - ArraySet expected = new ArraySet<>(); - expected.add(3); - expected.add(235); - expected.add(1); - when(mFsc.getAppOps(mRow.getEntry().notification.getUserId(), - mRow.getEntry().notification.getPackageName())).thenReturn(expected); - mNotificationData.add(mRow.getEntry()); - assertEquals(expected.size(), - mNotificationData.get(mRow.getEntry().key).mActiveAppOps.size()); - for (int op : expected) { - assertTrue(" entry missing op " + op, - mNotificationData.get(mRow.getEntry().key).mActiveAppOps.contains(op)); - } - } - - @Test - public void testAdd_noExistingAppOps() { - when(mFsc.getAppOps(mRow.getEntry().notification.getUserId(), - mRow.getEntry().notification.getPackageName())).thenReturn(null); - - mNotificationData.add(mRow.getEntry()); - assertEquals(0, mNotificationData.get(mRow.getEntry().key).mActiveAppOps.size()); - } @Test public void testAllRelevantNotisTaggedWithAppOps() throws Exception { @@ -181,7 +156,9 @@ public class NotificationDataTest extends SysuiTestCase { for (int op : expectedOps) { mNotificationData.updateAppOp(op, NotificationTestHelper.UID, - NotificationTestHelper.PKG, true); + NotificationTestHelper.PKG, mRow.getEntry().key, true); + mNotificationData.updateAppOp(op, NotificationTestHelper.UID, + NotificationTestHelper.PKG, row2.getEntry().key, true); } for (int op : expectedOps) { assertTrue(mRow.getEntry().key + " doesn't have op " + op, @@ -205,12 +182,12 @@ public class NotificationDataTest extends SysuiTestCase { for (int op : expectedOps) { mNotificationData.updateAppOp(op, NotificationTestHelper.UID, - NotificationTestHelper.PKG, true); + NotificationTestHelper.PKG, row2.getEntry().key, true); } expectedOps.remove(OP_ACCEPT_HANDOVER); mNotificationData.updateAppOp(OP_ACCEPT_HANDOVER, NotificationTestHelper.UID, - NotificationTestHelper.PKG, false); + NotificationTestHelper.PKG, row2.getEntry().key, false); assertTrue(mRow.getEntry().key + " doesn't have op " + OP_CAMERA, mNotificationData.get(mRow.getEntry().key).mActiveAppOps.contains(OP_CAMERA)); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationEntryManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationEntryManagerTest.java index 3703d6ad2b837..7cfd7a3d18222 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationEntryManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationEntryManagerTest.java @@ -37,18 +37,15 @@ import android.app.AppOpsManager; import android.app.Notification; import android.app.NotificationManager; import android.content.Context; -import android.content.pm.ApplicationInfo; -import android.content.pm.PackageManager; -import android.os.Bundle; import android.os.Handler; import android.os.Looper; import android.os.UserHandle; import android.service.notification.NotificationListenerService; -import android.service.notification.NotificationRankingUpdate; import android.service.notification.StatusBarNotification; import android.support.test.filters.SmallTest; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; +import android.util.ArraySet; import android.widget.FrameLayout; import com.android.internal.logging.MetricsLogger; @@ -56,12 +53,13 @@ import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.ForegroundServiceController; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; -import com.android.systemui.UiOffloadThread; import com.android.systemui.statusbar.notification.VisualStabilityManager; import com.android.systemui.statusbar.phone.NotificationGroupManager; import com.android.systemui.statusbar.policy.DeviceProvisionedController; import com.android.systemui.statusbar.policy.HeadsUpManager; +import junit.framework.Assert; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -285,13 +283,12 @@ public class NotificationEntryManagerTest extends SysuiTestCase { com.android.systemui.util.Assert.isNotMainThread(); when(mForegroundServiceController.getStandardLayoutKey(anyInt(), anyString())) - .thenReturn("something"); + .thenReturn(mEntry.key); mEntry.row = mRow; mEntryManager.getNotificationData().add(mEntry); - mHandler.post(() -> { - mEntryManager.updateNotificationsForAppOps( + mEntryManager.updateNotificationsForAppOp( AppOpsManager.OP_CAMERA, mEntry.notification.getUid(), mEntry.notification.getPackageName(), true); }); @@ -309,10 +306,65 @@ public class NotificationEntryManagerTest extends SysuiTestCase { when(mForegroundServiceController.getStandardLayoutKey(anyInt(), anyString())) .thenReturn(null); mHandler.post(() -> { - mEntryManager.updateNotificationsForAppOps(AppOpsManager.OP_CAMERA, 1000, "pkg", true); + mEntryManager.updateNotificationsForAppOp(AppOpsManager.OP_CAMERA, 1000, "pkg", true); }); waitForIdleSync(mHandler); verify(mPresenter, never()).updateNotificationViews(); } + + @Test + public void testAddNotificationExistingAppOps() { + mEntry.row = mRow; + mEntryManager.getNotificationData().add(mEntry); + ArraySet expected = new ArraySet<>(); + expected.add(3); + expected.add(235); + expected.add(1); + + when(mForegroundServiceController.getAppOps(mEntry.notification.getUserId(), + mEntry.notification.getPackageName())).thenReturn(expected); + when(mForegroundServiceController.getStandardLayoutKey( + mEntry.notification.getUserId(), + mEntry.notification.getPackageName())).thenReturn(mEntry.key); + + mEntryManager.tagForeground(mEntry.notification); + + Assert.assertEquals(expected.size(), mEntry.mActiveAppOps.size()); + for (int op : expected) { + assertTrue("Entry missing op " + op, mEntry.mActiveAppOps.contains(op)); + } + } + + @Test + public void testAdd_noExistingAppOps() { + mEntry.row = mRow; + mEntryManager.getNotificationData().add(mEntry); + when(mForegroundServiceController.getStandardLayoutKey( + mEntry.notification.getUserId(), + mEntry.notification.getPackageName())).thenReturn(mEntry.key); + when(mForegroundServiceController.getAppOps(mEntry.notification.getUserId(), + mEntry.notification.getPackageName())).thenReturn(null); + + mEntryManager.tagForeground(mEntry.notification); + Assert.assertEquals(0, mEntry.mActiveAppOps.size()); + } + + @Test + public void testAdd_existingAppOpsNotForegroundNoti() { + mEntry.row = mRow; + mEntryManager.getNotificationData().add(mEntry); + ArraySet ops = new ArraySet<>(); + ops.add(3); + ops.add(235); + ops.add(1); + when(mForegroundServiceController.getAppOps(mEntry.notification.getUserId(), + mEntry.notification.getPackageName())).thenReturn(ops); + when(mForegroundServiceController.getStandardLayoutKey( + mEntry.notification.getUserId(), + mEntry.notification.getPackageName())).thenReturn("something else"); + + mEntryManager.tagForeground(mEntry.notification); + Assert.assertEquals(0, mEntry.mActiveAppOps.size()); + } }