From 13f9bc83f93b014dd5c634731a20cacac3241883 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Tue, 24 Mar 2020 19:09:28 -0700 Subject: [PATCH] Make bubble flag removals from notification delegate (sysui) sticky Have a bit on NotificationRecord that indicates whether the bubble has been user dismissed (i.e. notif active without FLAG_BUBBLE). Checking this bit prevents the flag from being clobbered on subsequent ranking updates. Adds tests for the extractor applying the flag correctly based on this bit and tests for ensuring the record state is set correctly in response to onNotificationBubbleChanged & updates Test: atest BubbleExtractorTest NotificationManagerServiceTest Fixes: 152262407 Change-Id: I4bda79bd04f2d08fa3a3849f45d33d899f4c7fde --- .../server/notification/BubbleExtractor.java | 3 +- .../NotificationManagerService.java | 3 + .../notification/NotificationRecord.java | 14 ++++ .../notification/BubbleExtractorTest.java | 30 ++++++++ .../NotificationManagerServiceTest.java | 71 +++++++++++++++++++ 5 files changed, 120 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/BubbleExtractor.java b/services/core/java/com/android/server/notification/BubbleExtractor.java index 77921236e0d72..d8e699817d4da 100644 --- a/services/core/java/com/android/server/notification/BubbleExtractor.java +++ b/services/core/java/com/android/server/notification/BubbleExtractor.java @@ -76,7 +76,8 @@ public class BubbleExtractor implements NotificationSignalExtractor { record.setAllowBubble(appCanShowBubble); } } - final boolean applyFlag = mBubbleChecker.isNotificationAppropriateToBubble(record); + final boolean applyFlag = mBubbleChecker.isNotificationAppropriateToBubble(record) + && !record.isFlagBubbleRemoved(); if (applyFlag) { record.getNotification().flags |= FLAG_BUBBLE; } else { diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 35dec5a59cec0..28c1ee8d0c3c9 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -1213,10 +1213,12 @@ public class NotificationManagerService extends SystemService { // apps querying noMan will know that their notification is not showing // as a bubble. r.getNotification().flags &= ~FLAG_BUBBLE; + r.setFlagBubbleRemoved(true); } else { // Enqueue will trigger resort & if the flag is allowed to be true it'll // be applied there. r.getNotification().flags |= FLAG_ONLY_ALERT_ONCE; + r.setFlagBubbleRemoved(false); mHandler.post(new EnqueueNotificationRunnable(r.getUser().getIdentifier(), r, isAppForeground)); } @@ -5617,6 +5619,7 @@ public class NotificationManagerService extends SystemService { final NotificationRecord r = new NotificationRecord(getContext(), n, channel); r.setIsAppImportanceLocked(mPreferencesHelper.getIsAppImportanceLocked(pkg, callingUid)); r.setPostSilently(postSilently); + r.setFlagBubbleRemoved(false); if ((notification.flags & Notification.FLAG_FOREGROUND_SERVICE) != 0) { final boolean fgServiceShown = channel.isFgServiceShown(); diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index 3f24b38161c3d..54a0f9f468922 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -187,6 +187,7 @@ public final class NotificationRecord { private boolean mSuggestionsGeneratedByAssistant; private boolean mEditChoicesBeforeSending; private boolean mHasSeenSmartReplies; + private boolean mFlagBubbleRemoved; private boolean mPostSilently; /** * Whether this notification (and its channels) should be considered user locked. Used in @@ -1201,6 +1202,19 @@ public final class NotificationRecord { return stats.hasBeenVisiblyExpanded(); } + /** + * When the bubble state on a notif changes due to user action (e.g. dismiss a bubble) then + * this value is set until an update or bubble change event due to user action (e.g. create + * bubble from sysui) + **/ + public boolean isFlagBubbleRemoved() { + return mFlagBubbleRemoved; + } + + public void setFlagBubbleRemoved(boolean flagBubbleRemoved) { + mFlagBubbleRemoved = flagBubbleRemoved; + } + public void setSystemGeneratedSmartActions( ArrayList systemGeneratedSmartActions) { mSystemGeneratedSmartActions = systemGeneratedSmartActions; diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java index c7cef05ce4421..0dbbbaa9cdd69 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java @@ -47,6 +47,7 @@ import org.mockito.MockitoAnnotations; public class BubbleExtractorTest extends UiServiceTestCase { @Mock RankingConfig mConfig; + @Mock BubbleExtractor.BubbleChecker mBubbleChecker; BubbleExtractor mBubbleExtractor; private String mPkg = "com.android.server.notification"; @@ -141,4 +142,33 @@ public class BubbleExtractorTest extends UiServiceTestCase { assertFalse(r.canBubble()); } + + @Test + public void testFlagBubble_true() { + when(mConfig.bubblesEnabled()).thenReturn(true); + when(mConfig.areBubblesAllowed(mPkg, mUid)).thenReturn(true); + NotificationRecord r = getNotificationRecord(true, IMPORTANCE_UNSPECIFIED); + + mBubbleExtractor.setBubbleChecker(mBubbleChecker); + when(mBubbleChecker.isNotificationAppropriateToBubble(r)).thenReturn(true); + mBubbleExtractor.process(r); + + assertTrue(r.canBubble()); + assertTrue(r.getNotification().isBubbleNotification()); + } + + @Test + public void testFlagBubble_noFlag_previouslyRemoved() { + when(mConfig.bubblesEnabled()).thenReturn(true); + when(mConfig.areBubblesAllowed(mPkg, mUid)).thenReturn(true); + NotificationRecord r = getNotificationRecord(true, IMPORTANCE_UNSPECIFIED); + r.setFlagBubbleRemoved(true); + + mBubbleExtractor.setBubbleChecker(mBubbleChecker); + when(mBubbleChecker.isNotificationAppropriateToBubble(r)).thenReturn(true); + mBubbleExtractor.process(r); + + assertTrue(r.canBubble()); + assertFalse(r.getNotification().isBubbleNotification()); + } } 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 3c29445604356..5201ca24ac70b 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -78,6 +78,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.ActivityManager; +import android.app.ActivityManagerInternal; import android.app.AppOpsManager; import android.app.AutomaticZenRule; import android.app.IActivityManager; @@ -154,6 +155,7 @@ import com.android.internal.logging.InstanceIdSequence; import com.android.internal.logging.InstanceIdSequenceFake; import com.android.internal.statusbar.NotificationVisibility; import com.android.internal.util.FastXmlSerializer; +import com.android.server.DeviceIdleInternal; import com.android.server.LocalServices; import com.android.server.SystemService; import com.android.server.UiServiceTestCase; @@ -380,12 +382,20 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { MockitoAnnotations.initMocks(this); + DeviceIdleInternal deviceIdleInternal = mock(DeviceIdleInternal.class); + when(deviceIdleInternal.getNotificationWhitelistDuration()).thenReturn(3000L); + ActivityManagerInternal activityManagerInternal = mock(ActivityManagerInternal.class); + LocalServices.removeServiceForTest(UriGrantsManagerInternal.class); LocalServices.addService(UriGrantsManagerInternal.class, mUgmInternal); LocalServices.removeServiceForTest(WindowManagerInternal.class); LocalServices.addService(WindowManagerInternal.class, mWindowManagerInternal); LocalServices.removeServiceForTest(StatusBarManagerInternal.class); LocalServices.addService(StatusBarManagerInternal.class, mStatusBar); + LocalServices.removeServiceForTest(DeviceIdleInternal.class); + LocalServices.addService(DeviceIdleInternal.class, deviceIdleInternal); + LocalServices.removeServiceForTest(ActivityManagerInternal.class); + LocalServices.addService(ActivityManagerInternal.class, activityManagerInternal); doNothing().when(mContext).sendBroadcastAsUser(any(), any(), any()); @@ -5591,6 +5601,67 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals((notifsAfter[0].getNotification().flags & FLAG_BUBBLE), 0); } + @Test + public void testNotificationBubbleIsFlagRemoved_resetOnUpdate() throws Exception { + // Bubbles are allowed! + setUpPrefsForBubbles(PKG, mUid, true /* global */, true /* app */, true /* channel */); + // Notif with bubble metadata + NotificationRecord nr = generateMessageBubbleNotifRecord(mTestNotificationChannel, + "testNotificationBubbleIsFlagRemoved_resetOnUpdate"); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + // Flag shouldn't be modified + NotificationRecord recordToCheck = mService.getNotificationRecord(nr.getSbn().getKey()); + assertFalse(recordToCheck.isFlagBubbleRemoved()); + + // Notify we're not a bubble + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), false); + waitForIdle(); + // Flag should be modified + recordToCheck = mService.getNotificationRecord(nr.getSbn().getKey()); + assertTrue(recordToCheck.isFlagBubbleRemoved()); + + + // Update the notif + mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + // And the flag is reset + recordToCheck = mService.getNotificationRecord(nr.getSbn().getKey()); + assertFalse(recordToCheck.isFlagBubbleRemoved()); + } + + @Test + public void testNotificationBubbleIsFlagRemoved_resetOnBubbleChangedTrue() throws Exception { + // Bubbles are allowed! + setUpPrefsForBubbles(PKG, mUid, true /* global */, true /* app */, true /* channel */); + // Notif with bubble metadata + NotificationRecord nr = generateMessageBubbleNotifRecord(mTestNotificationChannel, + "testNotificationBubbleIsFlagRemoved_trueOnBubbleChangedTrue"); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + // Flag shouldn't be modified + NotificationRecord recordToCheck = mService.getNotificationRecord(nr.getSbn().getKey()); + assertFalse(recordToCheck.isFlagBubbleRemoved()); + + // Notify we're not a bubble + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), false); + waitForIdle(); + // Flag should be modified + recordToCheck = mService.getNotificationRecord(nr.getSbn().getKey()); + assertTrue(recordToCheck.isFlagBubbleRemoved()); + + // Notify we are a bubble + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), true); + waitForIdle(); + // And the flag is reset + assertFalse(recordToCheck.isFlagBubbleRemoved()); + } + @Test public void testOnBubbleNotificationSuppressionChanged() throws Exception { // Bubbles are allowed!