fix: Windows Toast notification dismissal from Action Center (#40197)

* fix: Windows Toast notification dismissal from Action Center

* docs: note Toast behavior in  event

* chore: address feedback from review

docs/api/notification.md

@@ -85,6 +85,8 @@ 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 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_
 
 Returns:
@@ -127,6 +129,8 @@ shown notification and create a new one with identical properties.
 
 Dismisses the notification.
 
+On Windows, calling `notification.close()` while the notification is visible on screen will dismiss the notification and remove it from the Action Center. If `notification.close()` is called after the notification is no longer visible on screen, calling `notification.close()` will try remove it from the Action Center.
+
 ### Instance Properties
 
 #### `notification.title`

shell/browser/api/electron_api_notification.cc

@@ -216,7 +216,11 @@ void Notification::NotificationClosed() {
 
 void Notification::Close() {
   if (notification_) {
-    notification_->Dismiss();
+    if (notification_->is_dismissed()) {
+      notification_->Remove();
+    } else {
+      notification_->Dismiss();
+    }
     notification_->set_delegate(nullptr);
     notification_.reset();
   }

shell/browser/notifications/notification.cc

@@ -27,10 +27,14 @@ void Notification::NotificationClicked() {
   Destroy();
 }
 
-void Notification::NotificationDismissed() {
+void Notification::NotificationDismissed(bool should_destroy) {
   if (delegate())
     delegate()->NotificationClosed();
-  Destroy();
+
+  set_is_dismissed(true);
+
+  if (should_destroy)
+    Destroy();
 }
 
 void Notification::NotificationFailed(const std::string& error) {
@@ -39,6 +43,8 @@ void Notification::NotificationFailed(const std::string& error) {
   Destroy();
 }
 
+void Notification::Remove() {}
+
 void Notification::Destroy() {
   presenter()->RemoveNotification(this);
 }

shell/browser/notifications/notification.h

@@ -50,13 +50,19 @@ 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.
   void NotificationClicked();
-  void NotificationDismissed();
+  void NotificationDismissed(bool should_destroy = true);
   void NotificationFailed(const std::string& error = "");
 
   // delete this.
@@ -68,10 +74,12 @@ class Notification {
 
   void set_delegate(NotificationDelegate* delegate) { delegate_ = delegate; }
   void set_notification_id(const std::string& id) { notification_id_ = id; }
+  void set_is_dismissed(bool dismissed) { is_dismissed_ = dismissed; }
 
   NotificationDelegate* delegate() const { return delegate_; }
   NotificationPresenter* presenter() const { return presenter_; }
   const std::string& notification_id() const { return notification_id_; }
+  bool is_dismissed() const { return is_dismissed_; }
 
   // disable copy
   Notification(const Notification&) = delete;
@@ -85,6 +93,7 @@ class Notification {
   raw_ptr<NotificationDelegate> delegate_;
   raw_ptr<NotificationPresenter> presenter_;
   std::string notification_id_;
+  bool is_dismissed_ = false;
 
   base::WeakPtrFactory<Notification> weak_factory_{this};
 };

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>
 
@@ -34,6 +36,8 @@ using ABI::Windows::Data::Xml::Dom::IXmlNodeList;
 using ABI::Windows::Data::Xml::Dom::IXmlText;
 using Microsoft::WRL::Wrappers::HStringReference;
 
+namespace winui = ABI::Windows::UI;
+
 #define RETURN_IF_FAILED(hr) \
   do {                       \
     HRESULT _hrTemp = hr;    \
@@ -47,8 +51,7 @@ using Microsoft::WRL::Wrappers::HStringReference;
     std::string _msgTemp = msg;                                          \
     if (FAILED(_hrTemp)) {                                               \
       std::string _err = _msgTemp + ",ERROR " + std::to_string(_hrTemp); \
-      if (IsDebuggingNotifications())                                    \
+      DebugLog(_err);                                                    \
-        LOG(INFO) << _err;                                               \
       Notification::NotificationFailed(_err);                            \
       return _hrTemp;                                                    \
     }                                                                    \
@@ -58,17 +61,23 @@ namespace electron {
 
 namespace {
 
-bool IsDebuggingNotifications() {
+// This string needs to be max 16 characters to work on Windows 10 prior to
-  return base::Environment::Create()->HasVar("ELECTRON_DEBUG_NOTIFICATIONS");
+// applying Creators Update (build 15063).
+constexpr wchar_t kGroup[] = L"Notifications";
+
+void DebugLog(std::string_view log_msg) {
+  if (base::Environment::Create()->HasVar("ELECTRON_DEBUG_NOTIFICATIONS"))
+    LOG(INFO) << log_msg;
 }
+
 }  // namespace
 
 // static
-ComPtr<ABI::Windows::UI::Notifications::IToastNotificationManagerStatics>
+ComPtr<winui::Notifications::IToastNotificationManagerStatics>
     WindowsToastNotification::toast_manager_;
 
 // static
-ComPtr<ABI::Windows::UI::Notifications::IToastNotifier>
+ComPtr<winui::Notifications::IToastNotifier>
     WindowsToastNotification::toast_notifier_;
 
 // static
@@ -112,17 +121,37 @@ WindowsToastNotification::~WindowsToastNotification() {
 
 void WindowsToastNotification::Show(const NotificationOptions& options) {
   if (SUCCEEDED(ShowInternal(options))) {
-    if (IsDebuggingNotifications())
+    DebugLog("Notification created");
-      LOG(INFO) << "Notification created";
 
     if (delegate())
       delegate()->NotificationDisplayed();
   }
 }
 
+void WindowsToastNotification::Remove() {
+  DebugLog("Removing notification from action center");
+
+  ComPtr<winui::Notifications::IToastNotificationManagerStatics2>
+      toast_manager2;
+  if (FAILED(toast_manager_.As(&toast_manager2)))
+    return;
+
+  ComPtr<winui::Notifications::IToastNotificationHistory> notification_history;
+  if (FAILED(toast_manager2->get_History(&notification_history)))
+    return;
+
+  ScopedHString app_id;
+  if (!GetAppUserModelID(&app_id))
+    return;
+
+  ScopedHString group(kGroup);
+  ScopedHString tag(base::as_wcstr(base::UTF8ToUTF16(notification_id())));
+  notification_history->RemoveGroupedTagWithId(tag, group, app_id);
+}
+
 void WindowsToastNotification::Dismiss() {
-  if (IsDebuggingNotifications())
+  DebugLog("Hiding notification");
-    LOG(INFO) << "Hiding notification";
+
   toast_notifier_->Hide(toast_notification_.Get());
 }
 
@@ -151,8 +180,7 @@ HRESULT WindowsToastNotification::ShowInternal(
     return E_FAIL;
   }
 
-  ComPtr<ABI::Windows::UI::Notifications::IToastNotificationFactory>
+  ComPtr<winui::Notifications::IToastNotificationFactory> toast_factory;
-      toast_factory;
   REPORT_AND_RETURN_IF_FAILED(
       Windows::Foundation::GetActivationFactory(toast_str, &toast_factory),
       "WinAPI: GetActivationFactory failed");
@@ -161,6 +189,19 @@ HRESULT WindowsToastNotification::ShowInternal(
                                   toast_xml.Get(), &toast_notification_),
                               "WinAPI: CreateToastNotification failed");
 
+  ComPtr<winui::Notifications::IToastNotification2> toast2;
+  REPORT_AND_RETURN_IF_FAILED(
+      toast_notification_->QueryInterface(IID_PPV_ARGS(&toast2)),
+      "WinAPI: Getting Notification interface failed");
+
+  ScopedHString group(kGroup);
+  REPORT_AND_RETURN_IF_FAILED(toast2->put_Group(group),
+                              "WinAPI: Setting group failed");
+
+  ScopedHString tag(base::as_wcstr(base::UTF8ToUTF16(notification_id())));
+  REPORT_AND_RETURN_IF_FAILED(toast2->put_Tag(tag),
+                              "WinAPI: Setting tag failed");
+
   REPORT_AND_RETURN_IF_FAILED(SetupCallbacks(toast_notification_.Get()),
                               "WinAPI: SetupCallbacks failed");
 
@@ -170,22 +211,20 @@ HRESULT WindowsToastNotification::ShowInternal(
 }
 
 HRESULT WindowsToastNotification::GetToastXml(
-    ABI::Windows::UI::Notifications::IToastNotificationManagerStatics*
+    winui::Notifications::IToastNotificationManagerStatics* toastManager,
-        toastManager,
     const std::u16string& title,
     const std::u16string& msg,
     const std::wstring& icon_path,
     const std::u16string& timeout_type,
     bool silent,
     IXmlDocument** toast_xml) {
-  ABI::Windows::UI::Notifications::ToastTemplateType template_type;
+  winui::Notifications::ToastTemplateType template_type;
   if (title.empty() || msg.empty()) {
     // Single line toast.
     template_type =
         icon_path.empty()
-            ? ABI::Windows::UI::Notifications::ToastTemplateType_ToastText01
+            ? winui::Notifications::ToastTemplateType_ToastText01
-            : ABI::Windows::UI::Notifications::
+            : winui::Notifications::ToastTemplateType_ToastImageAndText01;
-                  ToastTemplateType_ToastImageAndText01;
     REPORT_AND_RETURN_IF_FAILED(
         toast_manager_->GetTemplateContent(template_type, toast_xml),
         "XML: Fetching XML ToastImageAndText01 template failed");
@@ -199,9 +238,8 @@ HRESULT WindowsToastNotification::GetToastXml(
     // Title and body toast.
     template_type =
         icon_path.empty()
-            ? ABI::Windows::UI::Notifications::ToastTemplateType_ToastText02
+            ? winui::Notifications::ToastTemplateType_ToastText02
-            : ABI::Windows::UI::Notifications::
+            : winui::Notifications::ToastTemplateType_ToastImageAndText02;
-                  ToastTemplateType_ToastImageAndText02;
     REPORT_AND_RETURN_IF_FAILED(
         toastManager->GetTemplateContent(template_type, toast_xml),
         "XML: Fetching XML ToastImageAndText02 template failed");
@@ -567,7 +605,7 @@ HRESULT WindowsToastNotification::XmlDocumentFromString(
 }
 
 HRESULT WindowsToastNotification::SetupCallbacks(
-    ABI::Windows::UI::Notifications::IToastNotification* toast) {
+    winui::Notifications::IToastNotification* toast) {
   event_handler_ = Make<ToastEventHandler>(this);
   RETURN_IF_FAILED(
       toast->add_Activated(event_handler_.Get(), &activated_token_));
@@ -578,7 +616,7 @@ HRESULT WindowsToastNotification::SetupCallbacks(
 }
 
 bool WindowsToastNotification::RemoveCallbacks(
-    ABI::Windows::UI::Notifications::IToastNotification* toast) {
+    winui::Notifications::IToastNotification* toast) {
   if (FAILED(toast->remove_Activated(activated_token_)))
     return false;
 
@@ -597,32 +635,29 @@ ToastEventHandler::ToastEventHandler(Notification* notification)
 ToastEventHandler::~ToastEventHandler() = default;
 
 IFACEMETHODIMP ToastEventHandler::Invoke(
-    ABI::Windows::UI::Notifications::IToastNotification* sender,
+    winui::Notifications::IToastNotification* sender,
     IInspectable* args) {
   content::GetUIThreadTaskRunner({})->PostTask(
       FROM_HERE,
       base::BindOnce(&Notification::NotificationClicked, notification_));
-  if (IsDebuggingNotifications())
+  DebugLog("Notification clicked");
-    LOG(INFO) << "Notification clicked";
 
   return S_OK;
 }
 
 IFACEMETHODIMP ToastEventHandler::Invoke(
-    ABI::Windows::UI::Notifications::IToastNotification* sender,
+    winui::Notifications::IToastNotification* sender,
-    ABI::Windows::UI::Notifications::IToastDismissedEventArgs* e) {
+    winui::Notifications::IToastDismissedEventArgs* e) {
   content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE,
+      FROM_HERE, base::BindOnce(&Notification::NotificationDismissed,
-      base::BindOnce(&Notification::NotificationDismissed, notification_));
+                                notification_, false));
-  if (IsDebuggingNotifications())
+  DebugLog("Notification dismissed");
-    LOG(INFO) << "Notification dismissed";
-
   return S_OK;
 }
 
 IFACEMETHODIMP ToastEventHandler::Invoke(
-    ABI::Windows::UI::Notifications::IToastNotification* sender,
+    winui::Notifications::IToastNotification* sender,
-    ABI::Windows::UI::Notifications::IToastFailedEventArgs* e) {
+    winui::Notifications::IToastFailedEventArgs* e) {
   HRESULT error;
   e->get_ErrorCode(&error);
   std::string errorMessage =
@@ -630,8 +665,7 @@ IFACEMETHODIMP ToastEventHandler::Invoke(
   content::GetUIThreadTaskRunner({})->PostTask(
       FROM_HERE, base::BindOnce(&Notification::NotificationFailed,
                                 notification_, errorMessage));
-  if (IsDebuggingNotifications())
+  DebugLog(errorMessage);
-    LOG(INFO) << errorMessage;
 
   return S_OK;
 }

shell/browser/notifications/win/windows_toast_notification.h

@@ -52,6 +52,7 @@ class WindowsToastNotification : public Notification {
   // Notification:
   void Show(const NotificationOptions& options) override;
   void Dismiss() override;
+  void Remove() override;
 
  private:
   friend class ToastEventHandler;