From e5c6045070adcac613f4581c9ac010d4313423be Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Mon, 30 Apr 2018 14:41:36 -0400 Subject: [PATCH] Update logic for interruptive notifications - Only count visual changes for non-foreground service notifications, because users consider the notification to be one 'session' - Don't count every remoteviews update, but those where the layoutid or sequence number has changed. Bug: 78643290 Test: runtest systemui-notification Change-Id: I49483d26ebe63329ef2d6d3f10dd730c310fcf2a --- core/java/android/app/Notification.java | 35 +++++++- .../NotificationManagerService.java | 30 ++++--- .../NotificationManagerServiceTest.java | 47 ++++++++--- .../server/notification/NotificationTest.java | 84 +++++++++++++++++++ 4 files changed, 171 insertions(+), 25 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index c5b80196cf266..91715467b60b2 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -2722,7 +2722,40 @@ public class Notification implements Parcelable * @hide */ public static boolean areRemoteViewsChanged(Builder first, Builder second) { - return !first.usesStandardHeader() || !second.usesStandardHeader(); + if (!Objects.equals(first.usesStandardHeader(), second.usesStandardHeader())) { + return true; + } + + if (areRemoteViewsChanged(first.mN.contentView, second.mN.contentView)) { + return true; + } + if (areRemoteViewsChanged(first.mN.bigContentView, second.mN.bigContentView)) { + return true; + } + if (areRemoteViewsChanged(first.mN.headsUpContentView, second.mN.headsUpContentView)) { + return true; + } + + return false; + } + + private static boolean areRemoteViewsChanged(RemoteViews first, RemoteViews second) { + if (first == null && second == null) { + return false; + } + if (first == null && second != null || first != null && second == null) { + return true; + } + + if (!Objects.equals(first.getLayoutId(), second.getLayoutId())) { + return true; + } + + if (!Objects.equals(first.getSequenceNumber(), second.getSequenceNumber())) { + return true; + } + + return false; } /** diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index ec3949f9a4414..627e34770b7c5 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -16,6 +16,7 @@ package com.android.server.notification; +import static android.app.Notification.FLAG_FOREGROUND_SERVICE; import static android.app.NotificationManager.ACTION_APP_BLOCK_STATE_CHANGED; import static android.app.NotificationManager.ACTION_NOTIFICATION_CHANNEL_BLOCK_STATE_CHANGED; import static android.app.NotificationManager.ACTION_NOTIFICATION_CHANNEL_GROUP_BLOCK_STATE_CHANGED; @@ -710,7 +711,7 @@ public class NotificationManagerService extends SystemService { StatusBarNotification sbn = r.sbn; cancelNotification(callingUid, callingPid, sbn.getPackageName(), sbn.getTag(), sbn.getId(), Notification.FLAG_AUTO_CANCEL, - Notification.FLAG_FOREGROUND_SERVICE, false, r.getUserId(), + FLAG_FOREGROUND_SERVICE, false, r.getUserId(), REASON_CLICK, nv.rank, nv.count, null); nv.recycle(); reportUserInteraction(r); @@ -754,7 +755,7 @@ public class NotificationManagerService extends SystemService { } } cancelNotification(callingUid, callingPid, pkg, tag, id, 0, - Notification.FLAG_ONGOING_EVENT | Notification.FLAG_FOREGROUND_SERVICE, + Notification.FLAG_ONGOING_EVENT | FLAG_FOREGROUND_SERVICE, true, userId, REASON_CANCEL, nv.rank, nv.count,null); nv.recycle(); } @@ -985,7 +986,7 @@ public class NotificationManagerService extends SystemService { cancelNotification(record.sbn.getUid(), record.sbn.getInitialPid(), record.sbn.getPackageName(), record.sbn.getTag(), record.sbn.getId(), 0, - Notification.FLAG_FOREGROUND_SERVICE, true, record.getUserId(), + FLAG_FOREGROUND_SERVICE, true, record.getUserId(), REASON_TIMEOUT, null); } } @@ -2084,7 +2085,7 @@ public class NotificationManagerService extends SystemService { // Don't allow client applications to cancel foreground service notis or autobundled // summaries. final int mustNotHaveFlags = isCallingUidSystem() ? 0 : - (Notification.FLAG_FOREGROUND_SERVICE | Notification.FLAG_AUTOGROUP_SUMMARY); + (FLAG_FOREGROUND_SERVICE | Notification.FLAG_AUTOGROUP_SUMMARY); cancelNotification(Binder.getCallingUid(), Binder.getCallingPid(), pkg, tag, id, 0, mustNotHaveFlags, false, userId, REASON_APP_CANCEL, null); } @@ -2099,7 +2100,7 @@ public class NotificationManagerService extends SystemService { // Calling from user space, don't allow the canceling of actively // running foreground services. cancelAllNotificationsInt(Binder.getCallingUid(), Binder.getCallingPid(), - pkg, null, 0, Notification.FLAG_FOREGROUND_SERVICE, true, userId, + pkg, null, 0, FLAG_FOREGROUND_SERVICE, true, userId, REASON_APP_CANCEL_ALL, null); } @@ -2685,7 +2686,7 @@ public class NotificationManagerService extends SystemService { private void cancelNotificationFromListenerLocked(ManagedServiceInfo info, int callingUid, int callingPid, String pkg, String tag, int id, int userId) { cancelNotification(callingUid, callingPid, pkg, tag, id, 0, - Notification.FLAG_ONGOING_EVENT | Notification.FLAG_FOREGROUND_SERVICE, + Notification.FLAG_ONGOING_EVENT | FLAG_FOREGROUND_SERVICE, true, userId, REASON_LISTENER_CANCEL, info); } @@ -3962,7 +3963,7 @@ public class NotificationManagerService extends SystemService { // FLAG_FOREGROUND_SERVICE, we have to revert to the flags we received // initially *and* force remove FLAG_FOREGROUND_SERVICE. sbn.getNotification().flags = - (r.mOriginalFlags & ~Notification.FLAG_FOREGROUND_SERVICE); + (r.mOriginalFlags & ~FLAG_FOREGROUND_SERVICE); mRankingHelper.sort(mNotificationList); mListeners.notifyPostedLocked(r, r); } @@ -4048,7 +4049,7 @@ public class NotificationManagerService extends SystemService { final NotificationRecord r = new NotificationRecord(getContext(), n, channel); r.setIsAppImportanceLocked(mRankingHelper.getIsAppImportanceLocked(pkg, callingUid)); - if ((notification.flags & Notification.FLAG_FOREGROUND_SERVICE) != 0 + if ((notification.flags & FLAG_FOREGROUND_SERVICE) != 0 && (channel.getUserLockedFields() & NotificationChannel.USER_LOCKED_IMPORTANCE) == 0 && (r.getImportance() == IMPORTANCE_MIN || r.getImportance() == IMPORTANCE_NONE)) { // Increase the importance of foreground service notifications unless the user had an @@ -4429,7 +4430,7 @@ public class NotificationManagerService extends SystemService { mUsageStats.registerUpdatedByApp(r, old); // Make sure we don't lose the foreground service state. notification.flags |= - old.getNotification().flags & Notification.FLAG_FOREGROUND_SERVICE; + old.getNotification().flags & FLAG_FOREGROUND_SERVICE; r.isUpdate = true; r.setInterruptive(isVisuallyInterruptive(old, r)); } @@ -4438,7 +4439,7 @@ public class NotificationManagerService extends SystemService { // Ensure if this is a foreground service that the proper additional // flags are set. - if ((notification.flags & Notification.FLAG_FOREGROUND_SERVICE) != 0) { + if ((notification.flags & FLAG_FOREGROUND_SERVICE) != 0) { notification.flags |= Notification.FLAG_ONGOING_EVENT | Notification.FLAG_NO_CLEAR; } @@ -4506,6 +4507,13 @@ public class NotificationManagerService extends SystemService { if (oldN.extras == null || newN.extras == null) { return false; } + + // Ignore visual interruptions from foreground services because users + // consider them one 'session'. Count them for everything else. + if (r != null && (r.sbn.getNotification().flags & FLAG_FOREGROUND_SERVICE) != 0) { + return false; + } + if (!Objects.equals(oldN.extras.get(Notification.EXTRA_TITLE), newN.extras.get(Notification.EXTRA_TITLE))) { return true; @@ -5805,7 +5813,7 @@ public class NotificationManagerService extends SystemService { final StatusBarNotification childSbn = childR.sbn; if ((childSbn.isGroup() && !childSbn.getNotification().isGroupSummary()) && childR.getGroupKey().equals(parentNotification.getGroupKey()) - && (childR.getFlags() & Notification.FLAG_FOREGROUND_SERVICE) == 0 + && (childR.getFlags() & FLAG_FOREGROUND_SERVICE) == 0 && (flagChecker == null || flagChecker.apply(childR.getFlags()))) { EventLogTags.writeNotificationCancel(callingUid, callingPid, pkg, childSbn.getId(), childSbn.getTag(), userId, 0, 0, reason, listenerName); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index eb1c9975cb7e1..72dd8d006c14b 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -16,6 +16,7 @@ package com.android.server.notification; +import static android.app.Notification.FLAG_FOREGROUND_SERVICE; import static android.app.NotificationManager.EXTRA_BLOCKED_STATE; import static android.app.NotificationManager.IMPORTANCE_HIGH; import static android.app.NotificationManager.IMPORTANCE_LOW; @@ -528,7 +529,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { PKG, new ParceledListSlice(Arrays.asList(channel))); final StatusBarNotification sbn = generateNotificationRecord(channel).sbn; - sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + sbn.getNotification().flags |= FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", sbn.getId(), sbn.getNotification(), sbn.getUserId()); waitForIdle(); @@ -557,7 +558,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.getNotificationChannel(PKG, channel.getId()).getImportance()); final StatusBarNotification sbn = generateNotificationRecord(channel).sbn; - sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + sbn.getNotification().flags |= FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", sbn.getId(), sbn.getNotification(), sbn.getUserId()); waitForIdle(); @@ -601,7 +602,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.setNotificationsEnabledForPackage(PKG, mUid, false); final StatusBarNotification sbn = generateNotificationRecord(null).sbn; - sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + sbn.getNotification().flags |= FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", sbn.getId(), sbn.getNotification(), sbn.getUserId()); waitForIdle(); @@ -759,7 +760,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testCancelAllNotifications_IgnoreForegroundService() throws Exception { final StatusBarNotification sbn = generateNotificationRecord(null).sbn; - sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + sbn.getNotification().flags |= FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", sbn.getId(), sbn.getNotification(), sbn.getUserId()); mBinderService.cancelAllNotifications(PKG, sbn.getUserId()); @@ -773,7 +774,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testCancelAllNotifications_IgnoreOtherPackages() throws Exception { final StatusBarNotification sbn = generateNotificationRecord(null).sbn; - sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + sbn.getNotification().flags |= FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", sbn.getId(), sbn.getNotification(), sbn.getUserId()); mBinderService.cancelAllNotifications("other_pkg_name", sbn.getUserId()); @@ -901,7 +902,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testRemoveForegroundServiceFlag_ImmediatelyAfterEnqueue() throws Exception { final StatusBarNotification sbn = generateNotificationRecord(null).sbn; - sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + sbn.getNotification().flags |= FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", null, sbn.getId(), sbn.getNotification(), sbn.getUserId()); mInternalService.removeForegroundServiceFlagFromNotification(PKG, sbn.getId(), @@ -909,14 +910,14 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { waitForIdle(); StatusBarNotification[] notifs = mBinderService.getActiveNotifications(sbn.getPackageName()); - assertEquals(0, notifs[0].getNotification().flags & Notification.FLAG_FOREGROUND_SERVICE); + assertEquals(0, notifs[0].getNotification().flags & FLAG_FOREGROUND_SERVICE); } @Test public void testCancelAfterSecondEnqueueDoesNotSpecifyForegroundFlag() throws Exception { final StatusBarNotification sbn = generateNotificationRecord(null).sbn; sbn.getNotification().flags = - Notification.FLAG_ONGOING_EVENT | Notification.FLAG_FOREGROUND_SERVICE; + Notification.FLAG_ONGOING_EVENT | FLAG_FOREGROUND_SERVICE; mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", sbn.getId(), sbn.getNotification(), sbn.getUserId()); sbn.getNotification().flags = Notification.FLAG_ONGOING_EVENT; @@ -937,7 +938,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mTestNotificationChannel, 2, "group", false); final NotificationRecord child2 = generateNotificationRecord( mTestNotificationChannel, 3, "group", false); - child2.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + child2.getNotification().flags |= FLAG_FOREGROUND_SERVICE; final NotificationRecord newGroup = generateNotificationRecord( mTestNotificationChannel, 4, "group2", false); mService.addNotification(parent); @@ -960,7 +961,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mTestNotificationChannel, 2, "group", false); final NotificationRecord child2 = generateNotificationRecord( mTestNotificationChannel, 3, "group", false); - child2.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + child2.getNotification().flags |= FLAG_FOREGROUND_SERVICE; final NotificationRecord newGroup = generateNotificationRecord( mTestNotificationChannel, 4, "group2", false); mService.addNotification(parent); @@ -984,7 +985,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mTestNotificationChannel, 2, "group", false); final NotificationRecord child2 = generateNotificationRecord( mTestNotificationChannel, 3, "group", false); - child2.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; + child2.getNotification().flags |= FLAG_FOREGROUND_SERVICE; final NotificationRecord newGroup = generateNotificationRecord( mTestNotificationChannel, 4, "group2", false); mService.addNotification(parent); @@ -2257,7 +2258,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { NotificationChannel.DEFAULT_CHANNEL_ID) .setContentTitle("foo") .setSmallIcon(android.R.drawable.sym_def_app_icon) - .setFlag(Notification.FLAG_FOREGROUND_SERVICE, true) + .setFlag(FLAG_FOREGROUND_SERVICE, true) .setPriority(Notification.PRIORITY_MIN); StatusBarNotification sbn = new StatusBarNotification(preOPkg, preOPkg, 9, "tag", preOUid, @@ -2272,7 +2273,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { nb = new Notification.Builder(mContext) .setContentTitle("foo") .setSmallIcon(android.R.drawable.sym_def_app_icon) - .setFlag(Notification.FLAG_FOREGROUND_SERVICE, true) + .setFlag(FLAG_FOREGROUND_SERVICE, true) .setPriority(Notification.PRIORITY_MIN); sbn = new StatusBarNotification(preOPkg, preOPkg, 9, "tag", preOUid, @@ -2669,6 +2670,26 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(expected, actual); } + @Test + public void testVisualDifference_foreground() { + Notification.Builder nb1 = new Notification.Builder(mContext, "") + .setContentTitle("foo"); + StatusBarNotification sbn1 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0, + nb1.build(), new UserHandle(mUid), null, 0); + NotificationRecord r1 = + new NotificationRecord(mContext, sbn1, mock(NotificationChannel.class)); + + Notification.Builder nb2 = new Notification.Builder(mContext, "") + .setFlag(FLAG_FOREGROUND_SERVICE, true) + .setContentTitle("bar"); + StatusBarNotification sbn2 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0, + nb2.build(), new UserHandle(mUid), null, 0); + NotificationRecord r2 = + new NotificationRecord(mContext, sbn2, mock(NotificationChannel.class)); + + assertFalse(mService.isVisuallyInterruptive(r1, r2)); + } + @Test public void testVisualDifference_diffTitle() { Notification.Builder nb1 = new Notification.Builder(mContext, "") diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationTest.java index d846d213c5b68..36ec2210942d0 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationTest.java @@ -335,6 +335,90 @@ public class NotificationTest extends UiServiceTestCase { assertFalse(Notification.areRemoteViewsChanged(n1, n2)); } + @Test + public void testRemoteViews_layoutChange() { + RemoteViews a = mock(RemoteViews.class); + when(a.getLayoutId()).thenReturn(234); + RemoteViews b = mock(RemoteViews.class); + when(b.getLayoutId()).thenReturn(189); + + Notification.Builder n1 = new Notification.Builder(mContext, "test").setContent(a); + Notification.Builder n2 = new Notification.Builder(mContext, "test").setContent(b); + assertTrue(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomBigContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomBigContentView(b); + assertTrue(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(b); + assertTrue(Notification.areRemoteViewsChanged(n1, n2)); + } + + @Test + public void testRemoteViews_layoutSame() { + RemoteViews a = mock(RemoteViews.class); + when(a.getLayoutId()).thenReturn(234); + RemoteViews b = mock(RemoteViews.class); + when(b.getLayoutId()).thenReturn(234); + + Notification.Builder n1 = new Notification.Builder(mContext, "test").setContent(a); + Notification.Builder n2 = new Notification.Builder(mContext, "test").setContent(b); + assertFalse(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomBigContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomBigContentView(b); + assertFalse(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(b); + assertFalse(Notification.areRemoteViewsChanged(n1, n2)); + } + + @Test + public void testRemoteViews_sequenceChange() { + RemoteViews a = mock(RemoteViews.class); + when(a.getLayoutId()).thenReturn(234); + when(a.getSequenceNumber()).thenReturn(1); + RemoteViews b = mock(RemoteViews.class); + when(b.getLayoutId()).thenReturn(234); + when(b.getSequenceNumber()).thenReturn(2); + + Notification.Builder n1 = new Notification.Builder(mContext, "test").setContent(a); + Notification.Builder n2 = new Notification.Builder(mContext, "test").setContent(b); + assertTrue(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomBigContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomBigContentView(b); + assertTrue(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(b); + assertTrue(Notification.areRemoteViewsChanged(n1, n2)); + } + + @Test + public void testRemoteViews_sequenceSame() { + RemoteViews a = mock(RemoteViews.class); + when(a.getLayoutId()).thenReturn(234); + when(a.getSequenceNumber()).thenReturn(1); + RemoteViews b = mock(RemoteViews.class); + when(b.getLayoutId()).thenReturn(234); + when(b.getSequenceNumber()).thenReturn(1); + + Notification.Builder n1 = new Notification.Builder(mContext, "test").setContent(a); + Notification.Builder n2 = new Notification.Builder(mContext, "test").setContent(b); + assertFalse(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomBigContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomBigContentView(b); + assertFalse(Notification.areRemoteViewsChanged(n1, n2)); + + n1 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(a); + n2 = new Notification.Builder(mContext, "test").setCustomHeadsUpContentView(b); + assertFalse(Notification.areRemoteViewsChanged(n1, n2)); + } + @Test public void testActionsDifferent_null() { Notification n1 = new Notification.Builder(mContext, "test")