From a54e9fab9262b8a885c6246caa0a9d25b67ec533 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Thu, 18 Apr 2019 13:26:18 -0700 Subject: [PATCH] Let NoMan know when something is no longer a bubble This is needed for some of the notification group summary stuff to work. More importantly, this is needed so that we can properly report to apps if their notification is bubbled or not, e.g. if the user dismisses the bubble & the notification is in the shade, that shouldn't be reported as 'FLAG_BUBBLE' as that notification is not actually being shown as a bubble. * Adds onBubbleNotificationChanged to NotificationDelegate to pipe through changes in bubble state, currently we only ever change it to 'false' but this CL includes ability to flip it to 'true' (and also checks if the notif should actually be able to bubble) * Factors code that indicates something is approved to bubble into own method * Adds way to set BubbleMetadata on a notification (hidden !!) Bug: 130250809 Test: atest NotificationManagerServiceTest Change-Id: I8df4cc1231ed5d078ce4d50a70d2631f82fd2306 --- .../internal/statusbar/IStatusBarService.aidl | 1 + .../systemui/bubbles/BubbleController.java | 27 +++-- .../notification/NotificationDelegate.java | 1 + .../NotificationManagerService.java | 38 +++++- .../statusbar/StatusBarManagerService.java | 11 ++ .../NotificationManagerServiceTest.java | 109 +++++++++++++++++- 6 files changed, 170 insertions(+), 17 deletions(-) diff --git a/core/java/com/android/internal/statusbar/IStatusBarService.aidl b/core/java/com/android/internal/statusbar/IStatusBarService.aidl index 8f8e4d8dc1087..f22b6cd3d601c 100644 --- a/core/java/com/android/internal/statusbar/IStatusBarService.aidl +++ b/core/java/com/android/internal/statusbar/IStatusBarService.aidl @@ -76,6 +76,7 @@ interface IStatusBarService in int notificationLocation, boolean modifiedBeforeSending); void onNotificationSettingsViewed(String key); void setSystemUiVisibility(int displayId, int vis, int mask, String cause); + void onNotificationBubbleChanged(String key, boolean isBubble); void onGlobalActionsShown(); void onGlobalActionsHidden(); diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java index 744f88d19ba35..ef383add644ed 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java @@ -32,7 +32,6 @@ import android.app.ActivityManager; import android.app.ActivityManager.RunningTaskInfo; import android.app.ActivityTaskManager; import android.app.IActivityTaskManager; -import android.app.INotificationManager; import android.app.Notification; import android.content.Context; import android.content.pm.ParceledListSlice; @@ -52,6 +51,7 @@ import androidx.annotation.IntDef; import androidx.annotation.MainThread; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.statusbar.IStatusBarService; import com.android.internal.statusbar.NotificationVisibility; import com.android.systemui.Dependency; import com.android.systemui.R; @@ -131,8 +131,7 @@ public class BubbleController implements ConfigurationController.ConfigurationLi private StatusBarStateListener mStatusBarStateListener; private final NotificationInterruptionStateProvider mNotificationInterruptionStateProvider; - - private INotificationManager mNotificationManagerService; + private IStatusBarService mBarService; // Used for determining view rect for touch interaction private Rect mTempRect = new Rect(); @@ -207,13 +206,6 @@ public class BubbleController implements ConfigurationController.ConfigurationLi mNotificationEntryManager = Dependency.get(NotificationEntryManager.class); mNotificationEntryManager.addNotificationEntryListener(mEntryListener); - try { - mNotificationManagerService = INotificationManager.Stub.asInterface( - ServiceManager.getServiceOrThrow(Context.NOTIFICATION_SERVICE)); - } catch (ServiceManager.ServiceNotFoundException e) { - e.printStackTrace(); - } - mStatusBarWindowController = statusBarWindowController; mStatusBarStateListener = new StatusBarStateListener(); Dependency.get(StatusBarStateController.class).addCallback(mStatusBarStateListener); @@ -231,6 +223,9 @@ public class BubbleController implements ConfigurationController.ConfigurationLi mBubbleData = data; mBubbleData.setListener(mBubbleDataListener); mSurfaceSynchronizer = synchronizer; + + mBarService = IStatusBarService.Stub.asInterface( + ServiceManager.getService(Context.STATUS_BAR_SERVICE)); } /** @@ -462,6 +457,18 @@ public class BubbleController implements ConfigurationController.ConfigurationLi if (mStackView != null) { mStackView.removeBubble(bubble); } + if (!bubble.entry.showInShadeWhenBubble()) { + // The notification is gone & bubble is gone, time to actually remove it + mNotificationEntryManager.performRemoveNotification(bubble.entry.notification); + } else { + // The notification is still in the shade but we've removed the bubble so + // lets make sure NoMan knows it's not a bubble anymore + try { + mBarService.onNotificationBubbleChanged(bubble.getKey(), false /* isBubble */); + } catch (RemoteException e) { + // Bad things have happened + } + } } public void onBubbleUpdated(Bubble bubble) { diff --git a/services/core/java/com/android/server/notification/NotificationDelegate.java b/services/core/java/com/android/server/notification/NotificationDelegate.java index b85abd98b00f7..61be1f5e559b9 100644 --- a/services/core/java/com/android/server/notification/NotificationDelegate.java +++ b/services/core/java/com/android/server/notification/NotificationDelegate.java @@ -46,6 +46,7 @@ public interface NotificationDelegate { int notificationLocation); void onNotificationDirectReplied(String key); void onNotificationSettingsViewed(String key); + void onNotificationBubbleChanged(String key, boolean isBubble); /** * Notifies that smart replies and actions have been added to the UI. diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index e5ecd49c67da7..9fc30ebc8cc7c 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -1020,6 +1020,24 @@ public class NotificationManagerService extends SystemService { } } } + + @Override + public void onNotificationBubbleChanged(String key, boolean isBubble) { + synchronized (mNotificationLock) { + NotificationRecord r = mNotificationsByKey.get(key); + if (r != null) { + final StatusBarNotification n = r.sbn; + final int callingUid = n.getUid(); + final String pkg = n.getPackageName(); + if (isBubble && isNotificationAppropriateToBubble(r, pkg, callingUid, + null /* oldEntry */)) { + r.getNotification().flags |= FLAG_BUBBLE; + } else { + r.getNotification().flags &= ~FLAG_BUBBLE; + } + } + } + } }; @VisibleForTesting @@ -4782,6 +4800,19 @@ public class NotificationManagerService extends SystemService { private void flagNotificationForBubbles(NotificationRecord r, String pkg, int userId, NotificationRecord oldRecord) { Notification notification = r.getNotification(); + if (isNotificationAppropriateToBubble(r, pkg, userId, oldRecord)) { + notification.flags |= FLAG_BUBBLE; + } else { + notification.flags &= ~FLAG_BUBBLE; + } + } + + /** + * @return whether the provided notification record is allowed to be represented as a bubble. + */ + private boolean isNotificationAppropriateToBubble(NotificationRecord r, String pkg, int userId, + NotificationRecord oldRecord) { + Notification notification = r.getNotification(); // Does the app want to bubble & have permission to bubble? boolean canBubble = notification.getBubbleMetadata() != null @@ -4807,12 +4838,7 @@ public class NotificationManagerService extends SystemService { // OR something that was previously a bubble & still exists boolean bubbleUpdate = oldRecord != null && (oldRecord.getNotification().flags & FLAG_BUBBLE) != 0; - - if (canBubble && (notificationAppropriateToBubble || appIsForeground || bubbleUpdate)) { - notification.flags |= FLAG_BUBBLE; - } else { - notification.flags &= ~FLAG_BUBBLE; - } + return canBubble && (notificationAppropriateToBubble || appIsForeground || bubbleUpdate); } private void doChannelWarningToast(CharSequence toastText) { diff --git a/services/core/java/com/android/server/statusbar/StatusBarManagerService.java b/services/core/java/com/android/server/statusbar/StatusBarManagerService.java index 9cbf00b85e8bc..af009ecb8c3eb 100644 --- a/services/core/java/com/android/server/statusbar/StatusBarManagerService.java +++ b/services/core/java/com/android/server/statusbar/StatusBarManagerService.java @@ -1313,6 +1313,17 @@ public class StatusBarManagerService extends IStatusBarService.Stub implements D } } + @Override + public void onNotificationBubbleChanged(String key, boolean isBubble) { + enforceStatusBarService(); + long identity = Binder.clearCallingIdentity(); + try { + mNotificationDelegate.onNotificationBubbleChanged(key, isBubble); + } finally { + Binder.restoreCallingIdentity(identity); + } + } + @Override public void onShellCommand(FileDescriptor in, FileDescriptor out, FileDescriptor err, String[] args, ShellCallback callback, ResultReceiver resultReceiver) { 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 2d101dd87a0ff..34bb0a89227a1 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -71,7 +71,6 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import android.app.Activity; import android.app.ActivityManager; import android.app.AppOpsManager; import android.app.AutomaticZenRule; @@ -5047,4 +5046,112 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.areBubblesAllowedForPackage(mContext.getPackageName(), mUid + UserHandle.PER_USER_RANGE); } + + @Test + public void testNotificationBubbleChanged_false() throws Exception { + // Bubbles are allowed! + mService.setPreferencesHelper(mPreferencesHelper); + when(mPreferencesHelper.areBubblesAllowed(anyString(), anyInt())).thenReturn(true); + when(mPreferencesHelper.getNotificationChannel( + anyString(), anyInt(), anyString(), anyBoolean())).thenReturn( + mTestNotificationChannel); + when(mPreferencesHelper.getImportance(anyString(), anyInt())).thenReturn( + mTestNotificationChannel.getImportance()); + + // Notif with bubble metadata but not our other misc requirements + NotificationRecord nr = generateNotificationRecord(mTestNotificationChannel, + null /* tvExtender */, true /* isBubble */); + + // Say we're foreground + when(mActivityManager.getPackageImportance(nr.sbn.getPackageName())).thenReturn( + IMPORTANCE_FOREGROUND); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nr.sbn.getId(), nr.sbn.getNotification(), nr.sbn.getUserId()); + waitForIdle(); + + // First we were a bubble + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsBefore.length); + assertTrue((notifsBefore[0].getNotification().flags & FLAG_BUBBLE) != 0); + + // Notify we're not a bubble + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), false); + waitForIdle(); + + // Now we are not a bubble + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + assertEquals((notifsAfter[0].getNotification().flags & FLAG_BUBBLE), 0); + } + + @Test + public void testNotificationBubbleChanged_true() throws Exception { + // Bubbles are allowed! + mService.setPreferencesHelper(mPreferencesHelper); + when(mPreferencesHelper.areBubblesAllowed(anyString(), anyInt())).thenReturn(true); + when(mPreferencesHelper.getNotificationChannel( + anyString(), anyInt(), anyString(), anyBoolean())).thenReturn( + mTestNotificationChannel); + when(mPreferencesHelper.getImportance(anyString(), anyInt())).thenReturn( + mTestNotificationChannel.getImportance()); + + // Plain notification that has bubble metadata + NotificationRecord nr = generateNotificationRecord(mTestNotificationChannel, + null /* tvExtender */, true /* isBubble */); + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nr.sbn.getId(), nr.sbn.getNotification(), nr.sbn.getUserId()); + waitForIdle(); + + // Would be a normal notification because wouldn't have met requirements to bubble + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsBefore.length); + assertEquals((notifsBefore[0].getNotification().flags & FLAG_BUBBLE), 0); + + // Make the package foreground so that we're allowed to be a bubble + when(mActivityManager.getPackageImportance(nr.sbn.getPackageName())).thenReturn( + IMPORTANCE_FOREGROUND); + + // Notify we are now a bubble + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), true); + waitForIdle(); + + // Make sure we are a bubble + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + assertTrue((notifsAfter[0].getNotification().flags & FLAG_BUBBLE) != 0); + } + + @Test + public void testNotificationBubbleChanged_true_notAllowed() throws Exception { + // Bubbles are allowed! + mService.setPreferencesHelper(mPreferencesHelper); + when(mPreferencesHelper.areBubblesAllowed(anyString(), anyInt())).thenReturn(true); + when(mPreferencesHelper.getNotificationChannel( + anyString(), anyInt(), anyString(), anyBoolean())).thenReturn( + mTestNotificationChannel); + when(mPreferencesHelper.getImportance(anyString(), anyInt())).thenReturn( + mTestNotificationChannel.getImportance()); + + // Notif that is not a bubble + NotificationRecord nr = generateNotificationRecord(mTestNotificationChannel, + null /* tvExtender */, true /* isBubble */); + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nr.sbn.getId(), nr.sbn.getNotification(), nr.sbn.getUserId()); + waitForIdle(); + + // Would be a normal notification because wouldn't have met requirements to bubble + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsBefore.length); + assertEquals((notifsBefore[0].getNotification().flags & FLAG_BUBBLE), 0); + + // Notify we are now a bubble + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), true); + waitForIdle(); + + // We still wouldn't be a bubble because the notification didn't meet requirements + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + assertEquals((notifsAfter[0].getNotification().flags & FLAG_BUBBLE), 0); + } }