From 8aebf3585824f5acb3eaa04926474cba11f78c96 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Mon, 26 Jun 2017 11:35:33 -0400 Subject: [PATCH] Only try to autogroup if group info changes Test: runtest systemui-notification Change-Id: I7ddf622530ee4c452353bea6751b613354917a1b Fixes: 62788068 --- .../notification/NotificationManagerService.java | 82 ++++++++++++---------- .../NotificationManagerServiceTest.java | 46 +++++++++++- 2 files changed, 89 insertions(+), 39 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index fbe6d302dd64..48b4c5724b5f 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -1136,7 +1136,7 @@ public class NotificationManagerService extends SystemService { NotificationAssistants notificationAssistants, ConditionProviders conditionProviders, ICompanionDeviceManager companionManager, SnoozeHelper snoozeHelper, NotificationUsageStats usageStats, AtomicFile policyFile, - ActivityManager activityManager) { + ActivityManager activityManager, GroupHelper groupHelper) { Resources resources = getContext().getResources(); mMaxPackageEnqueueRate = Settings.Global.getFloat(getContext().getContentResolver(), Settings.Global.MAX_NOTIFICATION_ENQUEUE_RATE, @@ -1193,35 +1193,7 @@ public class NotificationManagerService extends SystemService { } }); mSnoozeHelper = snoozeHelper; - mGroupHelper = new GroupHelper(new GroupHelper.Callback() { - @Override - public void addAutoGroup(String key) { - synchronized (mNotificationLock) { - addAutogroupKeyLocked(key); - } - mRankingHandler.requestSort(false); - } - - @Override - public void removeAutoGroup(String key) { - synchronized (mNotificationLock) { - removeAutogroupKeyLocked(key); - } - mRankingHandler.requestSort(false); - } - - @Override - public void addAutoGroupSummary(int userId, String pkg, String triggeringKey) { - createAutoGroupSummary(userId, pkg, triggeringKey); - } - - @Override - public void removeAutoGroupSummary(int userId, String pkg) { - synchronized (mNotificationLock) { - clearAutogroupSummaryLocked(userId, pkg); - } - } - }); + mGroupHelper = groupHelper; // This is a ManagedServices object that keeps track of the listeners. mListeners = notificationListeners; @@ -1337,11 +1309,44 @@ public class NotificationManagerService extends SystemService { new ConditionProviders(getContext(), mUserProfiles, AppGlobals.getPackageManager()), null, snoozeHelper, new NotificationUsageStats(getContext()), new AtomicFile(new File(systemDir, "notification_policy.xml")), - (ActivityManager) getContext().getSystemService(Context.ACTIVITY_SERVICE)); + (ActivityManager) getContext().getSystemService(Context.ACTIVITY_SERVICE), + getGroupHelper()); publishBinderService(Context.NOTIFICATION_SERVICE, mService); publishLocalService(NotificationManagerInternal.class, mInternalService); } + private GroupHelper getGroupHelper() { + return new GroupHelper(new GroupHelper.Callback() { + @Override + public void addAutoGroup(String key) { + synchronized (mNotificationLock) { + addAutogroupKeyLocked(key); + } + mRankingHandler.requestSort(false); + } + + @Override + public void removeAutoGroup(String key) { + synchronized (mNotificationLock) { + removeAutogroupKeyLocked(key); + } + mRankingHandler.requestSort(false); + } + + @Override + public void addAutoGroupSummary(int userId, String pkg, String triggeringKey) { + createAutoGroupSummary(userId, pkg, triggeringKey); + } + + @Override + public void removeAutoGroupSummary(int userId, String pkg) { + synchronized (mNotificationLock) { + clearAutogroupSummaryLocked(userId, pkg); + } + } + }); + } + private void sendRegisteredOnlyBroadcast(String action) { getContext().sendBroadcastAsUser(new Intent(action) .addFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY), UserHandle.ALL, null); @@ -3323,7 +3328,6 @@ public class NotificationManagerService extends SystemService { (r.mOriginalFlags & ~Notification.FLAG_FOREGROUND_SERVICE); mRankingHelper.sort(mNotificationList); mListeners.notifyPostedLocked(sbn, sbn /* oldSbn */); - mGroupHelper.onNotificationPosted(sbn); } }; @@ -3740,12 +3744,14 @@ public class NotificationManagerService extends SystemService { if (notification.getSmallIcon() != null) { StatusBarNotification oldSbn = (old != null) ? old.sbn : null; mListeners.notifyPostedLocked(n, oldSbn); - mHandler.post(new Runnable() { - @Override - public void run() { - mGroupHelper.onNotificationPosted(n); - } - }); + if (oldSbn == null || !Objects.equals(oldSbn.getGroup(), n.getGroup())) { + mHandler.post(new Runnable() { + @Override + public void run() { + mGroupHelper.onNotificationPosted(n); + } + }); + } } else { Slog.e(TAG, "Not posting notification without small icon: " + notification); if (old != null && !old.isCanceled) { diff --git a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java index d475d6459c72..8ff46394db14 100644 --- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -118,6 +118,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase { private ManagedServices.ManagedServiceInfo mListener; @Mock private ICompanionDeviceManager mCompanionMgr; @Mock SnoozeHelper mSnoozeHelper; + @Mock GroupHelper mGroupHelper; // Use a Testable subclass so we can simulate calls from the system without failing. private static class TestableNotificationManagerService extends NotificationManagerService { @@ -175,7 +176,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase { mNotificationManagerService.init(mTestableLooper.getLooper(), mPackageManager, mPackageManagerClient, mockLightsManager, mNotificationListeners, mNotificationAssistants, mConditionProviders, mCompanionMgr, - mSnoozeHelper, mUsageStats, mPolicyFile, mActivityManager); + mSnoozeHelper, mUsageStats, mPolicyFile, mActivityManager, mGroupHelper); } catch (SecurityException e) { if (!e.getMessage().contains("Permission Denial: not allowed to send broadcast")) { throw e; @@ -1086,4 +1087,47 @@ public class NotificationManagerServiceTest extends NotificationTestCase { verify(mNotificationAssistants, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); } + + @Test + public void testOnlyAutogroupIfGroupChanged_noPriorNoti_autogroups() throws Exception { + NotificationRecord r = generateNotificationRecord(mTestNotificationChannel, 0, null, false); + mNotificationManagerService.addEnqueuedNotification(r); + NotificationManagerService.PostNotificationRunnable runnable = + mNotificationManagerService.new PostNotificationRunnable(r.getKey()); + runnable.run(); + waitForIdle(); + + verify(mGroupHelper, times(1)).onNotificationPosted(any()); + } + + @Test + public void testOnlyAutogroupIfGroupChanged_groupChanged_autogroups() + throws Exception { + NotificationRecord r = generateNotificationRecord(mTestNotificationChannel, 0, "group", false); + mNotificationManagerService.addNotification(r); + + r = generateNotificationRecord(mTestNotificationChannel, 0, null, false); + mNotificationManagerService.addEnqueuedNotification(r); + NotificationManagerService.PostNotificationRunnable runnable = + mNotificationManagerService.new PostNotificationRunnable(r.getKey()); + runnable.run(); + waitForIdle(); + + verify(mGroupHelper, times(1)).onNotificationPosted(any()); + } + + @Test + public void testOnlyAutogroupIfGroupChanged_noGroupChanged_autogroups() + throws Exception { + NotificationRecord r = generateNotificationRecord(mTestNotificationChannel, 0, "group", false); + mNotificationManagerService.addNotification(r); + mNotificationManagerService.addEnqueuedNotification(r); + + NotificationManagerService.PostNotificationRunnable runnable = + mNotificationManagerService.new PostNotificationRunnable(r.getKey()); + runnable.run(); + waitForIdle(); + + verify(mGroupHelper, never()).onNotificationPosted(any()); + } } -- 2.11.0