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

trop[bot] · codebytere · web-flow · commit 91ce51cfb332 · 2020-07-03T09:20:53.000-07:00
Co-authored-by: Shelley Vohr &lt;shelley.vohr@gmail.com&gt;
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(

Original file line number	Diff line number	Diff line change
`@@ -111,8 +111,6 @@`
`111`	`111`
`112`	`112`	`NotificationDismissed();`
`113`	`113`
`114`		`- this->LogAction("dismissed");`
`115`		`-`
`116`	`114`	`notification_.reset(nil);`
`117`	`115`	`}`
`118`	`116`
`@@ -163,8 +161,9 @@`
`163`	`161`	`}`
`164`	`162`
`165`	`163`	`void CocoaNotification::LogAction(const char* action) {`
`166`		`- if (getenv("ELECTRON_DEBUG_NOTIFICATIONS")) {`
	`164`	`+ if (getenv("ELECTRON_DEBUG_NOTIFICATIONS") && notification_) {`
`167`	`165`	`NSString* identifier = [notification_ valueForKey:@"identifier"];`
	`166`	`+ DCHECK(identifier);`
`168`	`167`	`LOG(INFO) << "Notification " << action << " (" << [identifier UTF8String]`
`169`	`168`	`<< ")";`
`170`	`169`	`}`