fix: remove same-tag notifications before showing new ones (#24406)

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
electron · Jul 3, 2020 · 6dd394a · 6dd394a
1 parent 4ace499
commit 6dd394a
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 5 deletions.
diff --git a/shell/browser/notifications/mac/cocoa_notification.mm b/shell/browser/notifications/mac/cocoa_notification.mm
@@ -111,8 +111,6 @@
 
   NotificationDismissed();
 
-  this->LogAction("dismissed");
-
   notification_.reset(nil);
 }
 
@@ -163,8 +161,9 @@
 }
 
 void CocoaNotification::LogAction(const char* action) {
-  if (getenv("ELECTRON_DEBUG_NOTIFICATIONS")) {
+  if (getenv("ELECTRON_DEBUG_NOTIFICATIONS") && notification_) {
     NSString* identifier = [notification_ valueForKey:@"identifier"];
+    DCHECK(identifier);
     LOG(INFO) << "Notification " << action << " (" << [identifier UTF8String]
               << ")";
   }

diff --git a/shell/browser/notifications/notification_presenter.cc b/shell/browser/notifications/notification_presenter.cc
@@ -37,8 +37,11 @@ void NotificationPresenter::CloseNotificationWithId(
                          [&notification_id](const Notification* n) {
                            return n->notification_id() == notification_id;
                          });
-  if (it != notifications_.end())
-    (*it)->Dismiss();
+  if (it != notifications_.end()) {
+    Notification* notification = (*it);
+    notification->Dismiss();
+    notifications_.erase(notification);
+  }
 }
 
 }  // namespace electron
diff --git a/shell/browser/notifications/platform_notification_service.cc b/shell/browser/notifications/platform_notification_service.cc
@@ -86,8 +86,19 @@ void PlatformNotificationService::DisplayNotification(
   auto* presenter = browser_client_->GetNotificationPresenter();
   if (!presenter)
     return;
+
+  // If a new notification is created with the same tag as an
+  // existing one, replace the old notification with the new one.
+  // The notification_id is generated from the tag, so the only way a
+  // notification will be closed as a result of this call is if one with
+  // the same tag is already extant.
+  //
+  // See: https://notifications.spec.whatwg.org/#showing-a-notification
+  presenter->CloseNotificationWithId(notification_id);
+
   NotificationDelegateImpl* delegate =
       new NotificationDelegateImpl(notification_id);
+
   auto notification = presenter->CreateNotification(delegate, notification_id);
   if (notification) {
     browser_client_->WebNotificationAllowed(