Skip to content

Commit

Permalink
fix: ensure notification close takes effect
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Jul 2, 2020
1 parent c157311 commit 38fb1c7
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 11 deletions.
15 changes: 9 additions & 6 deletions shell/browser/notifications/mac/cocoa_notification.mm
Expand Up @@ -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() {
Expand Down Expand Up @@ -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]
<< ")";
}
}
}

Expand Down
12 changes: 9 additions & 3 deletions shell/browser/notifications/notification_presenter.cc
Expand Up @@ -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
2 changes: 1 addition & 1 deletion shell/browser/notifications/notification_presenter.h
Expand Up @@ -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_; }

Expand Down
13 changes: 12 additions & 1 deletion shell/browser/notifications/platform_notification_service.cc
Expand Up @@ -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(
Expand All @@ -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(
Expand Down

0 comments on commit 38fb1c7

Please sign in to comment.