From 0c9b33551fe6235b54ba6be511492c8b14901097 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Tue, 17 Oct 2023 23:33:45 +0000 Subject: [PATCH] chore: address feedback from review Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> --- docs/api/notification.md | 2 +- shell/browser/notifications/notification.h | 9 +++++++-- .../notifications/win/windows_toast_notification.cc | 4 +++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/api/notification.md b/docs/api/notification.md index d7405d902f..fe12a9d050 100644 --- a/docs/api/notification.md +++ b/docs/api/notification.md @@ -85,7 +85,7 @@ Emitted when the notification is closed by manual intervention from the user. This event is not guaranteed to be emitted in all cases where the notification is closed. -On Windows, the `close` event is emitted when the notification leaves the screen either by programmatic dismissal via `notification.close()`, the user closing the notification, or via system timeout. If the notification remains in the Action Center afterwards, a further call to `notification.close()` will remove the notification from the action center, but the `close` event will not be emitted again. +On Windows, the `close` event can be emitted in one of three ways: programmatic dismissal with `notification.close()`, by the user closing the notification, or via system timeout. If a notification is in the Action Center after the initial `close` event is emitted, a call to `notification.close()` will remove the notification from the action center but the `close` event will not be emitted again. #### Event: 'reply' _macOS_ diff --git a/shell/browser/notifications/notification.h b/shell/browser/notifications/notification.h index dae517f93d..391d0e26fd 100644 --- a/shell/browser/notifications/notification.h +++ b/shell/browser/notifications/notification.h @@ -50,9 +50,14 @@ class Notification { // Shows the notification. virtual void Show(const NotificationOptions& options) = 0; - // Closes the notification, this instance will be destroyed after the - // notification gets closed. + + // Dismisses the notification. On some platforms this will result in full + // removal and destruction of the notification, but if the initial dismissal + // does not fully get rid of the notification it will be destroyed in Remove. virtual void Dismiss() = 0; + + // Removes the notification if it was not fully removed during dismissal, + // as can happen on some platforms including Windows. virtual void Remove(); // Should be called by derived classes. diff --git a/shell/browser/notifications/win/windows_toast_notification.cc b/shell/browser/notifications/win/windows_toast_notification.cc index 9d892365c5..ba64376c74 100644 --- a/shell/browser/notifications/win/windows_toast_notification.cc +++ b/shell/browser/notifications/win/windows_toast_notification.cc @@ -8,6 +8,8 @@ #include "shell/browser/notifications/win/windows_toast_notification.h" +#include <string_view> + #include <shlobj.h> #include <wrl\wrappers\corewrappers.h> @@ -63,7 +65,7 @@ namespace { // applying Creators Update (build 15063). constexpr wchar_t kGroup[] = L"Notifications"; -void DebugLog(const std::string& log_msg) { +void DebugLog(std::string_view log_msg) { if (base::Environment::Create()->HasVar("ELECTRON_DEBUG_NOTIFICATIONS")) LOG(INFO) << log_msg; } -- GitLab