feat: add signal option to dialog.showMessageBox (#26102)

* mac: add dialog.closeMessageBox API

* win: Implement dialog.closeMessageBox

* mac: Return cancelId with closeMessageBox

* gtk: Implement dialog.closeMessageBox

* win: Fix 32bit build

* win: Reduce the scope of lock

* fix: Build error after rebase

* feat: Use AbortSignal to close message box

* chore: silently handle duplicate ID

* win: Add more notes about the threads

* chore: apply reviews

* fix: base::NoDestructor should be warpped in function

* chore: fix style on windows

docs/api/dialog.md

@@ -273,6 +273,11 @@ If `browserWindow` is not shown dialog will not be attached to it. In such case
     will result in one button labeled "OK".
   * `defaultId` Integer (optional) - Index of the button in the buttons array which will
     be selected by default when the message box opens.
+  * `signal` AbortSignal (optional) - Pass an instance of [AbortSignal][] to
+    optionally close the message box, the message box will behave as if it was
+    cancelled by the user. On macOS, `signal` does not work with message boxes
+    that do not have a parent window, since those message boxes run
+    synchronously due to platform limitations.
   * `title` String (optional) - Title of the message box, some platforms will not show it.
   * `detail` String (optional) - Extra information of the message.
   * `checkboxLabel` String (optional) - If provided, the message box will
@@ -360,3 +365,5 @@ window is provided.
 
 You can call `BrowserWindow.getCurrentWindow().setSheetOffset(offset)` to change
 the offset from the window frame where sheets are attached.
+
+[AbortSignal]: https://nodejs.org/api/globals.html#globals_class_abortsignal

lib/browser/api/dialog.ts

@@ -22,6 +22,11 @@ enum OpenFileDialogProperties {
   dontAddToRecent = 1 << 8 // Windows
 }
 
+let nextId = 0;
+const getNextId = function () {
+  return ++nextId;
+};
+
 const normalizeAccessKey = (text: string) => {
   if (typeof text !== 'string') return text;
 
@@ -157,6 +162,7 @@ const messageBox = (sync: boolean, window: BrowserWindow | null, options?: Messa
   let {
     buttons = [],
     cancelId,
+    signal,
     checkboxLabel = '',
     checkboxChecked,
     defaultId = -1,
@@ -196,10 +202,21 @@ const messageBox = (sync: boolean, window: BrowserWindow | null, options?: Messa
     }
   }
 
+  // AbortSignal processing.
+  let id: number | undefined;
+  if (signal) {
+    // Generate an ID used for closing the message box.
+    id = getNextId();
+    // Close the message box when signal is aborted.
+    if (signal.aborted) { return Promise.resolve({ cancelId, checkboxChecked }); }
+    signal.addEventListener('abort', () => dialogBinding._closeMessageBox(id));
+  }
+
   const settings = {
     window,
     messageBoxType,
     buttons,
+    id,
     defaultId,
     cancelId,
     noLink,

shell/browser/api/electron_api_dialog.cc

@@ -91,6 +91,7 @@ void Initialize(v8::Local<v8::Object> exports,
   gin_helper::Dictionary dict(isolate, exports);
   dict.SetMethod("showMessageBoxSync", &ShowMessageBoxSync);
   dict.SetMethod("showMessageBox", &ShowMessageBox);
+  dict.SetMethod("_closeMessageBox", &electron::CloseMessageBox);
   dict.SetMethod("showErrorBox", &electron::ShowErrorBox);
   dict.SetMethod("showOpenDialogSync", &ShowOpenDialogSync);
   dict.SetMethod("showOpenDialog", &ShowOpenDialog);

shell/browser/ui/message_box.h

@@ -6,10 +6,10 @@
 #define SHELL_BROWSER_UI_MESSAGE_BOX_H_
 
 #include <string>
-#include <utility>
 #include <vector>
 
 #include "base/callback_forward.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/gfx/image/image_skia.h"
 
 namespace electron {
@@ -24,12 +24,11 @@ enum class MessageBoxType {
   kQuestion,
 };
 
-using DialogResult = std::pair<int, bool>;
-
 struct MessageBoxSettings {
   electron::NativeWindow* parent_window = nullptr;
   MessageBoxType type = electron::MessageBoxType::kNone;
   std::vector<std::string> buttons;
+  absl::optional<int> id;
   int default_id;
   int cancel_id;
   bool no_link = false;
@@ -53,6 +52,8 @@ typedef base::OnceCallback<void(int code, bool checkbox_checked)>
 void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback);
 
+void CloseMessageBox(int id);
+
 // Like ShowMessageBox with simplest settings, but safe to call at very early
 // stage of application.
 void ShowErrorBox(const std::u16string& title, const std::u16string& content);

shell/browser/ui/message_box_gtk.cc

@@ -2,15 +2,19 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include "shell/browser/ui/gtk_util.h"
 #include "shell/browser/ui/message_box.h"
 
+#include <map>
+
 #include "base/callback.h"
+#include "base/containers/contains.h"
+#include "base/no_destructor.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "shell/browser/browser.h"
 #include "shell/browser/native_window_observer.h"
 #include "shell/browser/native_window_views.h"
+#include "shell/browser/ui/gtk_util.h"
 #include "shell/browser/unresponsive_suppressor.h"
 #include "ui/base/glib/glib_signal.h"
 #include "ui/gfx/image/image_skia.h"
@@ -38,10 +42,17 @@ MessageBoxSettings::~MessageBoxSettings() = default;
 
 namespace {
 
+// <ID, messageBox> map
+std::map<int, GtkWidget*>& GetDialogsMap() {
+  static base::NoDestructor<std::map<int, GtkWidget*>> dialogs;
+  return *dialogs;
+}
+
 class GtkMessageBox : public NativeWindowObserver {
  public:
   explicit GtkMessageBox(const MessageBoxSettings& settings)
-      : cancel_id_(settings.cancel_id),
+      : id_(settings.id),
+        cancel_id_(settings.cancel_id),
         parent_(static_cast<NativeWindow*>(settings.parent_window)) {
     // Create dialog.
     dialog_ =
@@ -50,6 +61,8 @@ class GtkMessageBox : public NativeWindowObserver {
                                GetMessageType(settings.type),   // type
                                GTK_BUTTONS_NONE,                // no buttons
                                "%s", settings.message.c_str());
+    if (id_)
+      GetDialogsMap()[*id_] = dialog_;
     if (!settings.detail.empty())
       gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog_),
                                                "%s", settings.detail.c_str());
@@ -183,6 +196,9 @@ class GtkMessageBox : public NativeWindowObserver {
  private:
   electron::UnresponsiveSuppressor unresponsive_suppressor_;
 
+  // The id of the dialog.
+  absl::optional<int> id_;
+
   // The id to return when the dialog is closed without pressing buttons.
   int cancel_id_ = 0;
 
@@ -196,6 +212,8 @@ class GtkMessageBox : public NativeWindowObserver {
 };
 
 void GtkMessageBox::OnResponseDialog(GtkWidget* widget, int response) {
+  if (id_)
+    GetDialogsMap().erase(*id_);
   gtk_widget_hide(dialog_);
 
   if (response < 0)
@@ -217,9 +235,20 @@ int ShowMessageBoxSync(const MessageBoxSettings& settings) {
 
 void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback) {
+  if (settings.id && base::Contains(GetDialogsMap(), *settings.id))
+    CloseMessageBox(*settings.id);
   (new GtkMessageBox(settings))->RunAsynchronous(std::move(callback));
 }
 
+void CloseMessageBox(int id) {
+  auto it = GetDialogsMap().find(id);
+  if (it == GetDialogsMap().end()) {
+    LOG(ERROR) << "CloseMessageBox called with nonexistent ID";
+    return;
+  }
+  gtk_window_close(GTK_WINDOW(it->second));
+}
+
 void ShowErrorBox(const std::u16string& title, const std::u16string& content) {
   if (Browser::Get()->is_ready()) {
     electron::MessageBoxSettings settings;

shell/browser/ui/message_box_mac.mm

@@ -4,6 +4,7 @@
 
 #include "shell/browser/ui/message_box.h"
 
+#include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ -11,8 +12,10 @@
 #import <Cocoa/Cocoa.h>
 
 #include "base/callback.h"
+#include "base/containers/contains.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_nsobject.h"
+#include "base/no_destructor.h"
 #include "base/strings/sys_string_conversions.h"
 #include "shell/browser/native_window.h"
 #include "skia/ext/skia_utils_mac.h"
@@ -26,6 +29,12 @@ MessageBoxSettings::~MessageBoxSettings() = default;
 
 namespace {
 
+// <ID, messageBox> map
+std::map<int, NSAlert*>& GetDialogsMap() {
+  static base::NoDestructor<std::map<int, NSAlert*>> dialogs;
+  return *dialogs;
+}
+
 NSAlert* CreateNSAlert(const MessageBoxSettings& settings) {
   // Ignore the title; it's the window title on other platforms and ignorable.
   NSAlert* alert = [[NSAlert alloc] init];
@@ -128,6 +137,12 @@ void ShowMessageBox(const MessageBoxSettings& settings,
     int ret = [[alert autorelease] runModal];
     std::move(callback).Run(ret, alert.suppressionButton.state == NSOnState);
   } else {
+    if (settings.id) {
+      if (base::Contains(GetDialogsMap(), *settings.id))
+        CloseMessageBox(*settings.id);
+      GetDialogsMap()[*settings.id] = alert;
+    }
+
     NSWindow* window =
         settings.parent_window
             ? settings.parent_window->GetNativeWindow().GetNativeNSWindow()
@@ -136,9 +151,19 @@ void ShowMessageBox(const MessageBoxSettings& settings,
     // Duplicate the callback object here since c is a reference and gcd would
     // only store the pointer, by duplication we can force gcd to store a copy.
     __block MessageBoxCallback callback_ = std::move(callback);
+    __block absl::optional<int> id = std::move(settings.id);
+    __block int cancel_id = settings.cancel_id;
 
     [alert beginSheetModalForWindow:window
                   completionHandler:^(NSModalResponse response) {
+                    if (id)
+                      GetDialogsMap().erase(*id);
+                    // When the alert is cancelled programmatically, the
+                    // response would be something like -1000. This currently
+                    // only happens when users call CloseMessageBox API, and we
+                    // should return cancelId as result.
+                    if (response < 0)
+                      response = cancel_id;
                     std::move(callback_).Run(
                         response, alert.suppressionButton.state == NSOnState);
                     [alert release];
@@ -146,6 +171,15 @@ void ShowMessageBox(const MessageBoxSettings& settings,
   }
 }
 
+void CloseMessageBox(int id) {
+  auto it = GetDialogsMap().find(id);
+  if (it == GetDialogsMap().end()) {
+    LOG(ERROR) << "CloseMessageBox called with nonexistent ID";
+    return;
+  }
+  [NSApp endSheet:it->second.window];
+}
+
 void ShowErrorBox(const std::u16string& title, const std::u16string& content) {
   NSAlert* alert = [[NSAlert alloc] init];
   [alert setMessageText:base::SysUTF16ToNSString(title)];

shell/browser/ui/message_box_win.cc

@@ -11,8 +11,11 @@
 #include <map>
 #include <vector>
 
+#include "base/containers/contains.h"
+#include "base/no_destructor.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/lock.h"
 #include "base/task/post_task.h"
 #include "base/win/scoped_gdi_object.h"
 #include "shell/browser/browser.h"
@@ -30,6 +33,41 @@ MessageBoxSettings::~MessageBoxSettings() = default;
 
 namespace {
 
+struct DialogResult {
+  int button_id;
+  bool verification_flag_checked;
+};
+
+// <ID, messageBox> map.
+//
+// Note that the HWND is stored in a unique_ptr, because the pointer of HWND
+// will be passed between threads and we need to ensure the memory of HWND is
+// not changed while dialogs map is modified.
+std::map<int, std::unique_ptr<HWND>>& GetDialogsMap() {
+  static base::NoDestructor<std::map<int, std::unique_ptr<HWND>>> dialogs;
+  return *dialogs;
+}
+
+// Speical HWND used by the dialogs map.
+//
+// - ID is used but window has not been created yet.
+const HWND kHwndReserve = reinterpret_cast<HWND>(-1);
+// - Notification to cancel message box.
+const HWND kHwndCancel = reinterpret_cast<HWND>(-2);
+
+// Lock used for modifying HWND between threads.
+//
+// Note that there might be multiple dialogs being opened at the same time, but
+// we only use one lock for them all, because each dialog is independent from
+// each other and there is no need to use different lock for each one.
+// Also note that the |GetDialogsMap| is only used in the main thread, what is
+// shared between threads is the memory of HWND, so there is no need to use lock
+// when accessing dialogs map.
+base::Lock& GetHWNDLock() {
+  static base::NoDestructor<base::Lock> lock;
+  return *lock;
+}
+
 // Small command ID values are already taken by Windows, we have to start from
 // a large number to avoid conflicts with Windows.
 const int kIDStart = 100;
@@ -76,6 +114,31 @@ void MapToCommonID(const std::vector<std::wstring>& buttons,
   }
 }
 
+// Callback of the task dialog. The TaskDialogIndirect API does not provide the
+// HWND of the dialog, and we have to listen to the TDN_CREATED message to get
+// it.
+// Note that this callback runs in dialog thread instead of main thread, so it
+// is possible for CloseMessageBox to be called before or all after the dialog
+// window is created.
+HRESULT CALLBACK
+TaskDialogCallback(HWND hwnd, UINT msg, WPARAM, LPARAM, LONG_PTR data) {
+  if (msg == TDN_CREATED) {
+    HWND* target = reinterpret_cast<HWND*>(data);
+    // Lock since CloseMessageBox might be called.
+    base::AutoLock lock(GetHWNDLock());
+    if (*target == kHwndCancel) {
+      // The dialog is cancelled before it is created, close it directly.
+      ::PostMessage(hwnd, WM_CLOSE, 0, 0);
+    } else if (*target == kHwndReserve) {
+      // Otherwise save the hwnd.
+      *target = hwnd;
+    } else {
+      NOTREACHED();
+    }
+  }
+  return S_OK;
+}
+
 DialogResult ShowTaskDialogWstr(NativeWindow* parent,
                                 MessageBoxType type,
                                 const std::vector<std::wstring>& buttons,
@@ -87,7 +150,8 @@ DialogResult ShowTaskDialogWstr(NativeWindow* parent,
                                 const std::u16string& detail,
                                 const std::u16string& checkbox_label,
                                 bool checkbox_checked,
-                                const gfx::ImageSkia& icon) {
+                                const gfx::ImageSkia& icon,
+                                HWND* hwnd) {
   TASKDIALOG_FLAGS flags =
       TDF_SIZE_TO_CONTENT |           // Show all content.
       TDF_ALLOW_DIALOG_CANCELLATION;  // Allow canceling the dialog.
@@ -169,12 +233,17 @@ DialogResult ShowTaskDialogWstr(NativeWindow* parent,
       config.dwFlags |= TDF_USE_COMMAND_LINKS;  // custom buttons as links.
   }
 
-  int button_id;
+  // Pass a callback to receive the HWND of the message box.
+  if (hwnd) {
+    config.pfCallback = &TaskDialogCallback;
+    config.lpCallbackData = reinterpret_cast<LONG_PTR>(hwnd);
+  }
 
   int id = 0;
-  BOOL verificationFlagChecked = FALSE;
-  TaskDialogIndirect(&config, &id, nullptr, &verificationFlagChecked);
+  BOOL verification_flag_checked = FALSE;
+  TaskDialogIndirect(&config, &id, nullptr, &verification_flag_checked);
 
+  int button_id;
   if (id_map.find(id) != id_map.end())  // common button.
     button_id = id_map[id];
   else if (id >= kIDStart)  // custom button.
@@ -182,10 +251,11 @@ DialogResult ShowTaskDialogWstr(NativeWindow* parent,
   else
     button_id = cancel_id;
 
-  return std::make_pair(button_id, verificationFlagChecked);
+  return {button_id, static_cast<bool>(verification_flag_checked)};
 }
 
-DialogResult ShowTaskDialogUTF8(const MessageBoxSettings& settings) {
+DialogResult ShowTaskDialogUTF8(const MessageBoxSettings& settings,
+                                HWND* hwnd) {
   std::vector<std::wstring> buttons;
   for (const auto& button : settings.buttons)
     buttons.push_back(base::UTF8ToWide(button));
@@ -199,31 +269,67 @@ DialogResult ShowTaskDialogUTF8(const MessageBoxSettings& settings) {
   return ShowTaskDialogWstr(
       settings.parent_window, settings.type, buttons, settings.default_id,
       settings.cancel_id, settings.no_link, title, message, detail,
-      checkbox_label, settings.checkbox_checked, settings.icon);
+      checkbox_label, settings.checkbox_checked, settings.icon, hwnd);
 }
 
 }  // namespace
 
 int ShowMessageBoxSync(const MessageBoxSettings& settings) {
   electron::UnresponsiveSuppressor suppressor;
-  DialogResult result = ShowTaskDialogUTF8(settings);
-  return result.first;
+  DialogResult result = ShowTaskDialogUTF8(settings, nullptr);
+  return result.button_id;
 }
 
 void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback) {
-  dialog_thread::Run(base::BindOnce(&ShowTaskDialogUTF8, settings),
+  // The dialog is created in a new thread so we don't know its HWND yet, put
+  // kHwndReserve in the dialogs map for now.
+  HWND* hwnd = nullptr;
+  if (settings.id) {
+    if (base::Contains(GetDialogsMap(), *settings.id))
+      CloseMessageBox(*settings.id);
+    auto it = GetDialogsMap().emplace(*settings.id,
+                                      std::make_unique<HWND>(kHwndReserve));
+    hwnd = it.first->second.get();
+  }
+
+  dialog_thread::Run(
+      base::BindOnce(&ShowTaskDialogUTF8, settings, base::Unretained(hwnd)),
       base::BindOnce(
-                         [](MessageBoxCallback callback, DialogResult result) {
-                           std::move(callback).Run(result.first, result.second);
+          [](MessageBoxCallback callback, absl::optional<int> id,
+             DialogResult result) {
+            if (id)
+              GetDialogsMap().erase(*id);
+            std::move(callback).Run(result.button_id,
+                                    result.verification_flag_checked);
           },
-                         std::move(callback)));
+          std::move(callback), settings.id));
+}
+
+void CloseMessageBox(int id) {
+  auto it = GetDialogsMap().find(id);
+  if (it == GetDialogsMap().end()) {
+    LOG(ERROR) << "CloseMessageBox called with nonexistent ID";
+    return;
+  }
+  HWND* hwnd = it->second.get();
+  // Lock since the TaskDialogCallback might be saving the dialog's HWND.
+  base::AutoLock lock(GetHWNDLock());
+  DCHECK(*hwnd != kHwndCancel);
+  if (*hwnd == kHwndReserve) {
+    // If the dialog window has not been created yet, tell it to cancel.
+    *hwnd = kHwndCancel;
+  } else {
+    // Otherwise send a message to close it.
+    ::PostMessage(*hwnd, WM_CLOSE, 0, 0);
+  }
 }
 
 void ShowErrorBox(const std::u16string& title, const std::u16string& content) {
   electron::UnresponsiveSuppressor suppressor;
   ShowTaskDialogWstr(nullptr, MessageBoxType::kError, {}, -1, 0, false,
-                     u"Error", title, content, u"", false, gfx::ImageSkia());
+                     u"Error", title, content, u"", false, gfx::ImageSkia(),
+                     nullptr);
 }
 
 }  // namespace electron

shell/common/gin_converters/message_box_converter.cc

@@ -4,9 +4,9 @@
 
 #include "shell/common/gin_converters/message_box_converter.h"
 
-#include "gin/dictionary.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_converters/native_window_converter.h"
+#include "shell/common/gin_helper/dictionary.h"
 
 namespace gin {
 
@@ -14,7 +14,7 @@ bool Converter<electron::MessageBoxSettings>::FromV8(
     v8::Isolate* isolate,
     v8::Local<v8::Value> val,
     electron::MessageBoxSettings* out) {
-  gin::Dictionary dict(nullptr);
+  gin_helper::Dictionary dict;
   int type = 0;
   if (!ConvertFromV8(isolate, val, &dict))
     return false;
@@ -22,6 +22,7 @@ bool Converter<electron::MessageBoxSettings>::FromV8(
   dict.Get("messageBoxType", &type);
   out->type = static_cast<electron::MessageBoxType>(type);
   dict.Get("buttons", &out->buttons);
+  dict.GetOptional("id", &out->id);
   dict.Get("defaultId", &out->default_id);
   dict.Get("cancelId", &out->cancel_id);
   dict.Get("title", &out->title);

spec-main/api-dialog-spec.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 import { dialog, BrowserWindow } from 'electron/main';
 import { closeAllWindows } from './window-helpers';
-import { ifit } from './spec-helpers';
+import { ifit, delay } from './spec-helpers';
 
 describe('dialog module', () => {
   describe('showOpenDialog', () => {
@@ -121,6 +121,62 @@ describe('dialog module', () => {
     });
   });
 
+  describe('showMessageBox with signal', () => {
+    afterEach(closeAllWindows);
+
+    it('closes message box immediately', async () => {
+      const controller = new AbortController();
+      const signal = controller.signal;
+      const w = new BrowserWindow();
+      const p = dialog.showMessageBox(w, { signal, message: 'i am message' });
+      controller.abort();
+      const result = await p;
+      expect(result.response).to.equal(0);
+    });
+
+    it('closes message box after a while', async () => {
+      const controller = new AbortController();
+      const signal = controller.signal;
+      const w = new BrowserWindow();
+      const p = dialog.showMessageBox(w, { signal, message: 'i am message' });
+      await delay(500);
+      controller.abort();
+      const result = await p;
+      expect(result.response).to.equal(0);
+    });
+
+    it('cancels message box', async () => {
+      const controller = new AbortController();
+      const signal = controller.signal;
+      const w = new BrowserWindow();
+      const p = dialog.showMessageBox(w, {
+        signal,
+        message: 'i am message',
+        buttons: ['OK', 'Cancel'],
+        cancelId: 1
+      });
+      controller.abort();
+      const result = await p;
+      expect(result.response).to.equal(1);
+    });
+
+    it('cancels message box after a while', async () => {
+      const controller = new AbortController();
+      const signal = controller.signal;
+      const w = new BrowserWindow();
+      const p = dialog.showMessageBox(w, {
+        signal,
+        message: 'i am message',
+        buttons: ['OK', 'Cancel'],
+        cancelId: 1
+      });
+      await delay(500);
+      controller.abort();
+      const result = await p;
+      expect(result.response).to.equal(1);
+    });
+  });
+
   describe('showErrorBox', () => {
     it('throws errors when the options are invalid', () => {
       expect(() => {