Implement the ability to close alerts programatically

chrome/browser/notifications/notification_platform_bridge_mac.mm

Index: chrome/browser/notifications/notification_platform_bridge_mac.mm
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac.mm b/chrome/browser/notifications/notification_platform_bridge_mac.mm
index 6b3c20dc60e789395257adb0ddd1f115608ac000..903b7c325be9edc9219d0cce58ba992d43f14802 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -114,8 +114,21 @@ NotificationPlatformBridge* NotificationPlatformBridge::Create() {
 // Interface to communicate with the Alert XPC service.
 @interface NotificationRemoteDispatcher : NSObject
 
+// Deliver a notification to the XPC service to be displayed as an alert.
 - (void)dispatchNotification:(NSDictionary*)data;
 
+// CLose a notification for a given |notificationId| and |profileId|.
+- (void)closeNotificationWithId:(NSString*)notificationId
+                  withProfileId:(NSString*)profileId;
+
+// Close all notifications.
+- (void)closeAllNotifications;
+
+// Return true if an alert with the given |notificationId| and |profileId|
+// is being displayed. This is based on cached results so it might not
+// be totally accurate.
+- (BOOL)isAlertDisplayed:(NSString*)notificationId
+             withProfile:(NSString*)profileId;
 @end
 
 // /////////////////////////////////////////////////////////////////////////////
@@ -137,9 +150,11 @@ NotificationPlatformBridgeMac::NotificationPlatformBridgeMac(
 NotificationPlatformBridgeMac::~NotificationPlatformBridgeMac() {
   [notification_center_ setDelegate:nil];
 
-  // TODO(miguelg) remove only alerts shown by the XPC service.
   // TODO(miguelg) do not remove banners if possible.
   [notification_center_ removeAllDeliveredNotifications];
+#if BUILDFLAG(ENABLE_XPC_NOTIFICATIONS)
+  [notification_remote_dispatcher_ closeAllNotifications];
+#endif  // BUILDFLAG(ENABLE_XPC_NOTIFICATIONS)
 }
 
 void NotificationPlatformBridgeMac::Display(
@@ -231,8 +246,17 @@ void NotificationPlatformBridgeMac::Display(
 void NotificationPlatformBridgeMac::Close(const std::string& profile_id,
                                           const std::string& notification_id) {
   NSString* candidate_id = base::SysUTF8ToNSString(notification_id);
-
   NSString* current_profile_id = base::SysUTF8ToNSString(profile_id);
+#if BUILDFLAG(ENABLE_XPC_NOTIFICATIONS)
+  if ([notification_remote_dispatcher_ isAlertDisplayed:candidate_id
+                                            withProfile:current_profile_id]) {
+    [notification_remote_dispatcher_
+        closeNotificationWithId:candidate_id
+                  withProfileId:current_profile_id];
+    return;
+  }
+#endif  // ENABLE_XPC_NOTIFICATIONS
+
   for (NSUserNotification* toast in
        [notification_center_ deliveredNotifications]) {
     NSString* toast_id =
@@ -403,10 +427,12 @@ bool NotificationPlatformBridgeMac::VerifyNotificationData(
 @implementation NotificationRemoteDispatcher {
   // The connection to the XPC server in charge of delivering alerts.
   base::scoped_nsobject<NSXPCConnection> xpcConnection_;
+  base::scoped_nsobject<NSMutableSet> alertsDisplayed_;

[Peter Beverloo, 2016/10/11 14:37:49]

Instead of caching which alerts we still think are opened, why not try to close
a banner first (as you do in NotificationPlatformBridgeMac::Close), and always
fall back to the NotificationRemoteDispatcher if that failed?

It looks like that would remove the majority of complexity here.

[Miguel Garcia, 2016/10/12 11:30:21]

Given you both had concerns on the cache approach and the fact that your
proposal certainly works I went ahead and implemented that instead. The cache is
removed now.

On 2016/10/11 14:37:49, Peter Beverloo wrote:

 }
 
 - (instancetype)init {
   if ((self = [super init])) {
+    alertsDisplayed_.reset([[NSMutableSet alloc] init]);
     xpcConnection_.reset([[NSXPCConnection alloc]
         initWithServiceName:
             [NSString
@@ -438,9 +464,44 @@ bool NotificationPlatformBridgeMac::VerifyNotificationData(
 }
 
 - (void)dispatchNotification:(NSDictionary*)data {
+  NSString* notificationId =
+      [data objectForKey:notification_constants::kNotificationId];
+  NSString* profileId =
+      [data objectForKey:notification_constants::kNotificationProfileId];
+  NSString* alertKey = [self createAlertKeyWithNotification:notificationId
+                                                withProfile:profileId];
+  [alertsDisplayed_ addObject:alertKey];
   [[xpcConnection_ remoteObjectProxy] deliverNotification:data];
 }
 
+- (void)closeNotificationWithId:(NSString*)notificationId
+                  withProfileId:(NSString*)profileId {
+  NSString* alertKey = [self createAlertKeyWithNotification:notificationId
+                                                withProfile:profileId];
+  [alertsDisplayed_ removeObject:alertKey];
+  [[xpcConnection_ remoteObjectProxy] closeNotificationWithId:notificationId
+                                                withProfileId:profileId];
+}
+
+- (void)closeAllNotifications {
+  [alertsDisplayed_ removeAllObjects];
+  [[xpcConnection_ remoteObjectProxy] closeAllNotifications];
+}
+
+- (BOOL)isAlertDisplayed:(NSString*)notificationId
+             withProfile:(NSString*)profileId {
+  NSString* alertKey = [self createAlertKeyWithNotification:notificationId
+                                                withProfile:profileId];
+  return [alertsDisplayed_ containsObject:alertKey];
+}
+
+// Creates a unique identifier for the notification. Only used for internal
+// tracking.
+- (NSString*)createAlertKeyWithNotification:(NSString*)notificationId
+                                withProfile:(NSString*)profileId {
+  return [NSString stringWithFormat:@"%@:%@", notificationId, profileId];
+}
+
 // NotificationReply implementation
 - (void)notificationClick:(NSDictionary*)notificationResponseData {
   NotificationPlatformBridgeMac::ProcessNotificationResponse(