From 38fb1c763c4694bd88a0f0a00cd1791afcbe6b65 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Wed, 1 Jul 2020 20:07:02 -0700
Subject: [PATCH] fix: ensure notification close takes effect

---
 .../notifications/mac/cocoa_notification.mm       | 15 +++++++++------
 .../notifications/notification_presenter.cc       | 12 +++++++++---
 .../notifications/notification_presenter.h        |  2 +-
 .../platform_notification_service.cc              | 13 ++++++++++++-
 4 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/shell/browser/notifications/mac/cocoa_notification.mm b/shell/browser/notifications/mac/cocoa_notification.mm
index b4be60027323f..6e18a30f1fd23 100644
--- a/shell/browser/notifications/mac/cocoa_notification.mm
+++ b/shell/browser/notifications/mac/cocoa_notification.mm
@@ -105,15 +105,16 @@
 }
 
 void CocoaNotification::Dismiss() {
-  if (notification_)
+  if (notification_) {
     [NSUserNotificationCenter.defaultUserNotificationCenter
         removeDeliveredNotification:notification_];
 
-  NotificationDismissed();
+    NotificationDismissed();
 
-  this->LogAction("dismissed");
+    this->LogAction("dismissed");
 
-  notification_.reset(nil);
+    notification_.reset(nil);
+  }
 }
 
 void CocoaNotification::NotificationDisplayed() {
@@ -165,8 +166,10 @@
 void CocoaNotification::LogAction(const char* action) {
   if (getenv("ELECTRON_DEBUG_NOTIFICATIONS")) {
     NSString* identifier = [notification_ valueForKey:@"identifier"];
-    LOG(INFO) << "Notification " << action << " (" << [identifier UTF8String]
-              << ")";
+    if (identifier) {
+      LOG(INFO) << "Notification " << action << " (" << [identifier UTF8String]
+                << ")";
+    }
   }
 }
 
diff --git a/shell/browser/notifications/notification_presenter.cc b/shell/browser/notifications/notification_presenter.cc
index 891bd171f125b..f58a2ae4f2d98 100644
--- a/shell/browser/notifications/notification_presenter.cc
+++ b/shell/browser/notifications/notification_presenter.cc
@@ -32,13 +32,19 @@ void NotificationPresenter::RemoveNotification(Notification* notification) {
 }
 
 void NotificationPresenter::CloseNotificationWithId(
-    const std::string& notification_id) {
+    const std::string& notification_id,
+    bool remove) {
   auto it = std::find_if(notifications_.begin(), notifications_.end(),
                          [&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();
+
+    if (remove)
+      notifications_.erase(notification);
+  }
 }
 
 }  // namespace electron
diff --git a/shell/browser/notifications/notification_presenter.h b/shell/browser/notifications/notification_presenter.h
index 703810c117011..cdab71c36679b 100644
--- a/shell/browser/notifications/notification_presenter.h
+++ b/shell/browser/notifications/notification_presenter.h
@@ -24,7 +24,7 @@ class NotificationPresenter {
   base::WeakPtr<Notification> CreateNotification(
       NotificationDelegate* delegate,
       const std::string& notification_id);
-  void CloseNotificationWithId(const std::string& notification_id);
+  void CloseNotificationWithId(const std::string& notification_id, bool remove);
 
   std::set<Notification*> notifications() const { return notifications_; }
 
diff --git a/shell/browser/notifications/platform_notification_service.cc b/shell/browser/notifications/platform_notification_service.cc
index f6f15bfdb542c..59c356f772fc2 100644
--- 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, true);
+
   NotificationDelegateImpl* delegate =
       new NotificationDelegateImpl(notification_id);
+
   auto notification = presenter->CreateNotification(delegate, notification_id);
   if (notification) {
     browser_client_->WebNotificationAllowed(
@@ -113,7 +124,7 @@ void PlatformNotificationService::CloseNotification(
   auto* presenter = browser_client_->GetNotificationPresenter();
   if (!presenter)
     return;
-  presenter->CloseNotificationWithId(notification_id);
+  presenter->CloseNotificationWithId(notification_id, false);
 }
 
 void PlatformNotificationService::GetDisplayedNotifications(