Repository: electron/electron
Pull request: fix: run toast creation on background thread

#### Description of Change
User reporting Electron apps freezing when notifications come in.  This is likely to hang on the COM interface. This PR puts as much possible of the toast creation on a background thread. Unfortunately the show()  method of the notification has to be called from the UI thread. Without a fix in Windows this as best as we can do.

<!--
Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Contributors guide: https://github.com/electron/electron/blob/main/CONTRIBUTING.md
-->

#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [ ] PR description included and stakeholders cc'd
- [ ] `npm test` passes
- [ ] tests are [changed or added](https://github.com/electron/electron/blob/main/docs/development/testing.md)
- [ ] relevant API documentation, tutorials, and examples are updated and follow the [documentation style guide](https://github.com/electron/electron/blob/main/docs/development/style-guide.md)
- [ ] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: Fixed a Windows notification issue where clicking a native notification would result in an application hang on certain Windows environments.


Changed files:
filename: shell/browser/notifications/notification.cc
patch: @@ -14,6 +14,12 @@ const bool debug_notifications =
     base::Environment::Create()->HasVar("ELECTRON_DEBUG_NOTIFICATIONS");
 
 NotificationOptions::NotificationOptions() = default;
+NotificationOptions::NotificationOptions(const NotificationOptions&) = default;
+NotificationOptions& NotificationOptions::operator=(
+    const NotificationOptions&) = default;
+NotificationOptions::NotificationOptions(NotificationOptions&&) = default;
+NotificationOptions& NotificationOptions::operator=(NotificationOptions&&) =
+    default;
 NotificationOptions::~NotificationOptions() = default;
 
 Notification::Notification(NotificationDelegate* delegate,

filename: shell/browser/notifications/notification.h
patch: @@ -43,6 +43,10 @@ struct NotificationOptions {
   std::u16string toast_xml;
 
   NotificationOptions();
+  NotificationOptions(const NotificationOptions&);
+  NotificationOptions& operator=(const NotificationOptions&);
+  NotificationOptions(NotificationOptions&&);
+  NotificationOptions& operator=(NotificationOptions&&);
   ~NotificationOptions();
 };
 

filename: shell/browser/notifications/win/windows_toast_notification.cc
patch: @@ -16,11 +16,13 @@
 #include "base/containers/fixed_flat_map.h"
 #include "base/hash/hash.h"
 #include "base/logging.h"
+#include "base/no_destructor.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/string_util_win.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/thread_pool.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "shell/browser/notifications/notification_delegate.h"
@@ -97,14 +99,16 @@ const std::string FailureResultToString(HRESULT failure_reason) {
         "The notification platform does not have the proper privileges to "
         "complete the request."}});
 
+  std::string hresult_str = base::StrCat(
+      {" (HRESULT: ", base::NumberToString(static_cast<long>(failure_reason)),
+       ")"});
+
   if (const auto it = kFailureMessages.find(failure_reason);
       it != kFailureMessages.end()) {
-    return base::StrCat({"Notification failed - ", it->second});
+    return base::StrCat({"Notification failed - ", it->second, hresult_str});
   }
 
-  return base::StrCat({"Notification failed - Unknown failure reason (HRESULT ",
-                       base::NumberToString(static_cast<long>(failure_reason)),
-                       ")"});
+  return hresult_str;
 }
 
 constexpr char kToast[] = "toast";
@@ -155,6 +159,17 @@ ComPtr<winui::Notifications::IToastNotifier>*
     WindowsToastNotification::toast_notifier_ = nullptr;
 
 // static
+scoped_refptr<base::SequencedTaskRunner>
+WindowsToastNotification::GetToastTaskRunner() {
+  // Use function-local static to avoid exit-time destructor warning
+  static base::NoDestructor<scoped_refptr<base::SequencedTaskRunner>>
+      task_runner(base::ThreadPool::CreateSequencedTaskRunner(
+          {base::TaskPriority::USER_BLOCKING,
+           base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN,
+           base::MayBlock()}));
+  return *task_runner;
+}
+
 bool WindowsToastNotification::Initialize() {
   // Just initialize, don't care if it fails or already initialized.
   Windows::Foundation::Initialize(RO_INIT_MULTITHREADED);
@@ -207,98 +222,304 @@ WindowsToastNotification::~WindowsToastNotification() {
   }
 }
 
+// This method posts a request onto the toast background thread, which
+// creates the toast xml then posts notification creation to the UI thread. This
+// avoids blocking the UI for expensive XML parsing and COM initialization or
+// the COM server becoming unavailable. All needed fields are captured before
+// posting the task.
+// The method will eventually result in a display or failure signal being posted
+// back to the UI thread, where further callbacks (clicked, dismissed, failed)
+// are handled by ToastEventHandler.
 void WindowsToastNotification::Show(const NotificationOptions& options) {
-  if (SUCCEEDED(ShowInternal(options))) {
-    DebugLog("Notification created");
-
-    if (delegate())
-      delegate()->NotificationDisplayed();
+  DebugLog("WindowsToastNotification::Show called");
+  DebugLog(base::StrCat(
+      {"toast_xml empty: ", options.toast_xml.empty() ? "yes" : "no"}));
+  if (!options.toast_xml.empty()) {
+    DebugLog(base::StrCat({"toast_xml length: ",
+                           base::NumberToString(options.toast_xml.length())}));
   }
-}
 
-void WindowsToastNotification::Remove() {
-  DebugLog("Removing notification from action center");
+  // Capture all needed data on UI thread before posting to background thread
+  std::string notif_id = notification_id();
+  NotificationPresenter* presenter_ptr = presenter();
+  base::WeakPtr<Notification> weak_this = GetWeakPtr();
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner =
+      content::GetUIThreadTaskRunner({});
 
-  ComPtr<winui::Notifications::IToastNotificationManagerStatics2>
-      toast_manager2;
-  if (FAILED(toast_manager_->As(&toast_manager2)))
-    return;
+  DebugLog("Posting task to background thread");
+  auto task_runner = GetToastTaskRunner();
+  DebugLog(base::StrCat({"Task runner valid: ", task_runner ? "yes" : "no"}));
 
-  ComPtr<winui::Notifications::IToastNotificationHistory> notification_history;
-  if (FAILED(toast_manager2->get_History(&notification_history)))
-    return;
+  // Post Show operation to background thread to prevent blocking
+  // This is the main entry point for the notification creation process
+  bool posted = task_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &WindowsToastNotification::CreateToastNotificationOnBackgroundThread,
+          options, presenter_ptr, notif_id, weak_this, ui_task_runner));
+  DebugLog(base::StrCat(
+      {"Task posted to background thread: ", posted ? "yes" : "no"}));
+}
 
-  ScopedHString app_id;
-  if (!GetAppUserModelID(&app_id))
-    return;
+// Creates the toast XML on the background thread. If the XML is invalid, posts
+// a failure event back to the UI thread. Otherwise, continues to create the
+// toast notification on the background thread.
+void WindowsToastNotification::CreateToastNotificationOnBackgroundThread(
+    const NotificationOptions& options,
+    NotificationPresenter* presenter,
+    const std::string& notification_id,
+    base::WeakPtr<Notification> weak_notification,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
+  DebugLog("CreateToastXmlOnBackgroundThread called");
+  ComPtr<IXmlDocument> toast_xml;
 
-  ScopedHString group(kGroup);
-  ScopedHString tag(GetTag(notification_id()));
-  notification_history->RemoveGroupedTagWithId(tag, group, app_id);
-}
+  if (!CreateToastXmlDocument(options, presenter, weak_notification,
+                              ui_task_runner, &toast_xml)) {
+    return;  // Error already posted to UI thread
+  }
 
-void WindowsToastNotification::Dismiss() {
-  DebugLog("Hiding notification");
+  // Continue to create the toast notification
+  ComPtr<ABI::Windows::UI::Notifications::IToastNotification>
+      toast_notification;
+  if (!CreateToastNotification(toast_xml, notification_id, weak_notification,
+                               ui_task_runner, &toast_notification)) {
+    return;  // Error already posted to UI thread
+  }
 
-  (*toast_notifier_)->Hide(toast_notification_.Get());
+  // Setup callbacks and show on UI thread (Show must be called on UI thread)
+  scoped_refptr<base::SingleThreadTaskRunner> ui_runner =
+      content::GetUIThreadTaskRunner({});
+  ui_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&WindowsToastNotification::SetupAndShowOnUIThread,
+                     weak_notification, toast_notification));
 }
 
-HRESULT WindowsToastNotification::ShowInternal(
-    const NotificationOptions& options) {
-  ComPtr<IXmlDocument> toast_xml;
+// Creates the toast XML document on the background thread. Returns true on
+// success, false on failure. On failure, posts error to UI thread. static
+bool WindowsToastNotification::CreateToastXmlDocument(
+    const NotificationOptions& options,
+    NotificationPresenter* presenter,
+    base::WeakPtr<Notification> weak_notification,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+    ComPtr<IXmlDocument>* toast_xml) {
   // The custom xml takes priority over the preset template.
   if (!options.toast_xml.empty()) {
-    REPORT_AND_RETURN_IF_FAILED(
-        XmlDocumentFromString(base::as_wcstr(options.toast_xml), &toast_xml),
-        "XML: Invalid XML");
+    DebugLog(base::StrCat({"Processing custom toast_xml, length: ",
+                           base::NumberToString(options.toast_xml.length())}));
+    HRESULT hr = XmlDocumentFromString(base::as_wcstr(options.toast_xml),
+                                       toast_xml->GetAddressOf());
+    DebugLog(base::StrCat({"XmlDocumentFromString returned HRESULT: ",
+                           base::NumberToString(hr)}));
+    if (FAILED(hr)) {
+      std::string err =
+          base::StrCat({"XML: Invalid XML, ERROR ", FailureResultToString(hr)});
+      DebugLog(base::StrCat({"XML parsing failed, posting error: ", err}));
+      PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+      DebugLog("PostNotificationFailedToUIThread called");
+      return false;
+    }
+    DebugLog("XML parsing succeeded");
   } else {
-    auto* presenter_win = static_cast<NotificationPresenterWin*>(presenter());
+    auto* presenter_win = static_cast<NotificationPresenterWin*>(presenter);
     std::wstring icon_path =
         presenter_win->SaveIconToFilesystem(options.icon, options.icon_url);
     std::u16string toast_xml_str =
         GetToastXml(options.title, options.msg, icon_path, options.timeout_type,
                     options.silent);
-    REPORT_AND_RETURN_IF_FAILED(
-        XmlDocumentFromString(base::as_wcstr(toast_xml_str), &toast_xml),
-        "XML: Invalid XML");
+    HRESULT hr = XmlDocumentFromString(base::as_wcstr(toast_xml_str),
+                                       toast_xml->GetAddressOf());
+    if (FAILED(hr)) {
+      std::string err =
+          base::StrCat({"XML: Invalid XML, ERROR ", FailureResultToString(hr)});
+      DebugLog(err);
+      PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+      return false;
+    }
   }
+  return true;
+}
 
+// Creates the toast notification on the background thread. Returns true on
+// success, false on failure. On failure, posts error to UI thread. On success,
+// returns the created notification via out parameter.
+bool WindowsToastNotification::CreateToastNotification(
+    ComPtr<ABI::Windows::Data::Xml::Dom::IXmlDocument> toast_xml,
+    const std::string& notification_id,
+    base::WeakPtr<Notification> weak_notification,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+    ComPtr<ABI::Windows::UI::Notifications::IToastNotification>*
+        toast_notification) {
   ScopedHString toast_str(
       RuntimeClass_Windows_UI_Notifications_ToastNotification);
   if (!toast_str.success()) {
-    NotificationFailed("Creating ScopedHString failed");
-    return E_FAIL;
+    PostNotificationFailedToUIThread(
+        weak_notification, "Creating ScopedHString failed", ui_task_runner);
+    return false;
   }
 
   ComPtr<winui::Notifications::IToastNotificationFactory> toast_factory;
-  REPORT_AND_RETURN_IF_FAILED(
-      Windows::Foundation::GetActivationFactory(toast_str, &toast_factory),
-      "WinAPI: GetActivationFactory failed");
+  HRESULT hr =
+      Windows::Foundation::GetActivationFactory(toast_str, &toast_factory);
+  if (FAILED(hr)) {
+    std::string err =
+        base::StrCat({"WinAPI: GetActivationFactory failed, ERROR ",
+                      FailureResultToString(hr)});
+    DebugLog(err);
+    PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+    return false;
+  }
 
-  REPORT_AND_RETURN_IF_FAILED(toast_factory->CreateToastNotification(
-                                  toast_xml.Get(), &toast_notification_),
-                              "WinAPI: CreateToastNotification failed");
+  hr = toast_factory->CreateToastNotification(
+      toast_xml.Get(), toast_notification->GetAddressOf());
+  if (FAILED(hr)) {
+    std::string err =
+        base::StrCat({"WinAPI: CreateToastNotification failed, ERROR ",
+                      FailureResultToString(hr)});
+    DebugLog(err);
+    PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+    return false;
+  }
 
   ComPtr<winui::Notifications::IToastNotification2> toast2;
-  REPORT_AND_RETURN_IF_FAILED(
-      toast_notification_->QueryInterface(IID_PPV_ARGS(&toast2)),
-      "WinAPI: Getting Notification interface failed");
+  hr = (*toast_notification)->QueryInterface(IID_PPV_ARGS(&toast2));
+  if (FAILED(hr)) {
+    std::string err =
+        base::StrCat({"WinAPI: Getting Notification interface failed, ERROR ",
+                      FailureResultToString(hr)});
+    DebugLog(err);
+    PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+    return false;
+  }
 
   ScopedHString group(kGroup);
-  REPORT_AND_RETURN_IF_FAILED(toast2->put_Group(group),
-                              "WinAPI: Setting group failed");
+  hr = toast2->put_Group(group);
+  if (FAILED(hr)) {
+    std::string err = base::StrCat(
+        {"WinAPI: Setting group failed, ERROR ", FailureResultToString(hr)});
+    DebugLog(err);
+    PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+    return false;
+  }
 
+  ScopedHString tag(GetTag(notification_id));
+  hr = toast2->put_Tag(tag);
+  if (FAILED(hr)) {
+    std::string err = base::StrCat(
+        {"WinAPI: Setting tag failed, ERROR ", FailureResultToString(hr)});
+    DebugLog(err);
+    PostNotificationFailedToUIThread(weak_notification, err, ui_task_runner);
+    return false;
+  }
+
+  return true;
+}
+
+// Sets up callbacks and shows the notification on the UI thread.
+// This part has to be called on the UI thread. This WinRT API
+// does not allow calls from background threads.
+void WindowsToastNotification::SetupAndShowOnUIThread(
+    base::WeakPtr<Notification> weak_notification,
+    ComPtr<ABI::Windows::UI::Notifications::IToastNotification> notification) {
+  auto* notif = static_cast<WindowsToastNotification*>(weak_notification.get());
+  if (!notif)
+    return;
+
+  // Setup callbacks and store notification on UI thread
+  HRESULT hr = notif->SetupCallbacks(notification.Get());
+  if (FAILED(hr)) {
+    std::string err = base::StrCat(
+        {"WinAPI: SetupCallbacks failed, ERROR ", FailureResultToString(hr)});
+    DebugLog(err);
+    notif->NotificationFailed(err);
+    return;
+  }
+
+  notif->toast_notification_ = notification;
+
+  // Show notification on UI thread (must be called on UI thread)
+  hr = (*toast_notifier_)->Show(notification.Get());
+  if (FAILED(hr)) {
+    std::string err = base::StrCat(
+        {"WinAPI: Show failed, ERROR ", FailureResultToString(hr)});
+    DebugLog(err);
+    notif->NotificationFailed(err);
+    return;
+  }
+
+  DebugLog("Notification created");
+  if (notif->delegate())
+    notif->delegate()->NotificationDisplayed();
+}
+
+// Posts a notification failure event back to the UI thread. If the UI thread's
+// task runner is not provided, it fetches it. On the UI thread, calls
+// NotificationFailed on the Notification instance (if it is still valid), which
+// will invoke the delegate (if set) and clean up.
+void WindowsToastNotification::PostNotificationFailedToUIThread(
+    base::WeakPtr<Notification> weak_notification,
+    const std::string& error,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
+  DebugLog(base::StrCat(
+      {"PostNotificationFailedToUIThread called with error: ", error}));
+  if (!ui_task_runner) {
+    ui_task_runner = content::GetUIThreadTaskRunner({});
+  }
+  ui_task_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](base::WeakPtr<Notification> weak_notification,
+             const std::string& error) {
+            DebugLog(
+                "PostNotificationFailedToUIThread lambda executing on UI "
+                "thread");
+            if (!weak_notification) {
+              DebugLog(base::StrCat(
+                  {"Notification failed but object destroyed: ", error}));
+              return;
+            }
+
+            // Call NotificationFailed - it will check for delegate internally
+            // and emit the event if delegate is set
+            DebugLog(base::StrCat(
+                {"Calling NotificationFailed with error: ", error}));
+            auto* delegate = weak_notification->delegate();
+            DebugLog(
+                base::StrCat({"Delegate is set: ", delegate ? "yes" : "no"}));
+
+            // Call NotificationFailed which will call delegate if set, then
+            // cleanup
+            weak_notification->NotificationFailed(error);
+            DebugLog("NotificationFailed call completed");
+          },
+          weak_notification, error));
+}
+
+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(GetTag(notification_id()));
-  REPORT_AND_RETURN_IF_FAILED(toast2->put_Tag(tag),
-                              "WinAPI: Setting tag failed");
+  notification_history->RemoveGroupedTagWithId(tag, group, app_id);
+}
 
-  REPORT_AND_RETURN_IF_FAILED(SetupCallbacks(toast_notification_.Get()),
-                              "WinAPI: SetupCallbacks failed");
+void WindowsToastNotification::Dismiss() {
+  DebugLog("Hiding notification");
 
-  REPORT_AND_RETURN_IF_FAILED(
-      (*toast_notifier_)->Show(toast_notification_.Get()),
-      "WinAPI: Show failed");
-  return S_OK;
+  (*toast_notifier_)->Hide(toast_notification_.Get());
 }
 
 std::u16string WindowsToastNotification::GetToastXml(

filename: shell/browser/notifications/win/windows_toast_notification.h
patch: @@ -14,6 +14,8 @@
 #include <wrl/implements.h>
 #include <string>
 
+#include "base/memory/scoped_refptr.h"
+#include "base/task/single_thread_task_runner.h"
 #include "shell/browser/notifications/notification.h"
 
 using Microsoft::WRL::ClassicCom;
@@ -58,25 +60,56 @@ class WindowsToastNotification : public Notification {
   friend class ToastEventHandler;
 
   HRESULT ShowInternal(const NotificationOptions& options);
-  std::u16string GetToastXml(const std::u16string& title,
-                             const std::u16string& msg,
-                             const std::wstring& icon_path,
-                             const std::u16string& timeout_type,
-                             const bool silent);
-  HRESULT XmlDocumentFromString(
+  static std::u16string GetToastXml(const std::u16string& title,
+                                    const std::u16string& msg,
+                                    const std::wstring& icon_path,
+                                    const std::u16string& timeout_type,
+                                    const bool silent);
+  static HRESULT XmlDocumentFromString(
       const wchar_t* xmlString,
       ABI::Windows::Data::Xml::Dom::IXmlDocument** doc);
   HRESULT SetupCallbacks(
       ABI::Windows::UI::Notifications::IToastNotification* toast);
   bool RemoveCallbacks(
       ABI::Windows::UI::Notifications::IToastNotification* toast);
 
+  // Helper methods for async Show() implementation
+  static bool CreateToastXmlDocument(
+      const NotificationOptions& options,
+      NotificationPresenter* presenter,
+      base::WeakPtr<Notification> weak_notification,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+      ComPtr<ABI::Windows::Data::Xml::Dom::IXmlDocument>* toast_xml);
+  static void CreateToastNotificationOnBackgroundThread(
+      const NotificationOptions& options,
+      NotificationPresenter* presenter,
+      const std::string& notification_id,
+      base::WeakPtr<Notification> weak_notification,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
+  static bool CreateToastNotification(
+      ComPtr<ABI::Windows::Data::Xml::Dom::IXmlDocument> toast_xml,
+      const std::string& notification_id,
+      base::WeakPtr<Notification> weak_notification,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+      ComPtr<ABI::Windows::UI::Notifications::IToastNotification>*
+          toast_notification);
+  static void SetupAndShowOnUIThread(
+      base::WeakPtr<Notification> weak_notification,
+      ComPtr<ABI::Windows::UI::Notifications::IToastNotification> notification);
+  static void PostNotificationFailedToUIThread(
+      base::WeakPtr<Notification> weak_notification,
+      const std::string& error,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
+
   static ComPtr<
       ABI::Windows::UI::Notifications::IToastNotificationManagerStatics>*
       toast_manager_;
   static ComPtr<ABI::Windows::UI::Notifications::IToastNotifier>*
       toast_notifier_;
 
+  // Returns the task runner for toast operations, creating it if necessary.
+  static scoped_refptr<base::SequencedTaskRunner> GetToastTaskRunner();
+
   EventRegistrationToken activated_token_;
   EventRegistrationToken dismissed_token_;
   EventRegistrationToken failed_token_;