feat: promisify dialog.showMessageBox() (#17298)

* feat: promisify dialog.showMessageBox()

* address feedback from review

atom/browser/api/atom_api_dialog.cc

@@ -22,33 +22,58 @@
 
 namespace {
 
-void ShowMessageBox(int type,
+int ShowMessageBoxSync(int type,
-                    const std::vector<std::string>& buttons,
+                       const std::vector<std::string>& buttons,
-                    int default_id,
+                       int default_id,
-                    int cancel_id,
+                       int cancel_id,
-                    int options,
+                       int options,
-                    const std::string& title,
+                       const std::string& title,
-                    const std::string& message,
+                       const std::string& message,
-                    const std::string& detail,
+                       const std::string& detail,
-                    const std::string& checkbox_label,
+                       const std::string& checkbox_label,
-                    bool checkbox_checked,
+                       bool checkbox_checked,
-                    const gfx::ImageSkia& icon,
+                       const gfx::ImageSkia& icon,
-                    atom::NativeWindow* window,
+                       atom::NativeWindow* window) {
-                    mate::Arguments* args) {
+  return atom::ShowMessageBoxSync(
-  v8::Local<v8::Value> peek = args->PeekNext();
+      window, static_cast<atom::MessageBoxType>(type), buttons, default_id,
-  atom::MessageBoxCallback callback;
+      cancel_id, options, title, message, detail, icon);
-  if (mate::Converter<atom::MessageBoxCallback>::FromV8(args->isolate(), peek,
+}
-                                                        &callback)) {
+
-    atom::ShowMessageBox(window, static_cast<atom::MessageBoxType>(type),
+void ResolvePromiseObject(atom::util::Promise promise,
-                         buttons, default_id, cancel_id, options, title,
+                          int result,
-                         message, detail, checkbox_label, checkbox_checked,
+                          bool checkbox_checked) {
-                         icon, callback);
+  mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate());
-  } else {
+
-    int chosen = atom::ShowMessageBox(
+  dict.Set("response", result);
-        window, static_cast<atom::MessageBoxType>(type), buttons, default_id,
+  dict.Set("checkboxChecked", checkbox_checked);
-        cancel_id, options, title, message, detail, icon);
+
-    args->Return(chosen);
+  promise.Resolve(dict.GetHandle());
-  }
+}
+
+v8::Local<v8::Promise> ShowMessageBox(int type,
+                                      const std::vector<std::string>& buttons,
+                                      int default_id,
+                                      int cancel_id,
+                                      int options,
+                                      const std::string& title,
+                                      const std::string& message,
+                                      const std::string& detail,
+                                      const std::string& checkbox_label,
+                                      bool checkbox_checked,
+                                      const gfx::ImageSkia& icon,
+                                      atom::NativeWindow* window,
+                                      mate::Arguments* args) {
+  v8::Isolate* isolate = args->isolate();
+  atom::util::Promise promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  atom::ShowMessageBox(
+      window, static_cast<atom::MessageBoxType>(type), buttons, default_id,
+      cancel_id, options, title, message, detail, checkbox_label,
+      checkbox_checked, icon,
+      base::BindOnce(&ResolvePromiseObject, std::move(promise)));
+
+  return handle;
 }
 
 void ShowOpenDialogSync(const file_dialog::DialogSettings& settings,
@@ -89,6 +114,7 @@ void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Context> context,
                 void* priv) {
   mate::Dictionary dict(context->GetIsolate(), exports);
+  dict.SetMethod("showMessageBoxSync", &ShowMessageBoxSync);
   dict.SetMethod("showMessageBox", &ShowMessageBox);
   dict.SetMethod("showErrorBox", &atom::ShowErrorBox);
   dict.SetMethod("showOpenDialogSync", &ShowOpenDialogSync);

atom/browser/ui/message_box.h

@@ -32,19 +32,19 @@ enum MessageBoxOptions {
   MESSAGE_BOX_NO_LINK = 1 << 0,
 };
 
-typedef base::Callback<void(int code, bool checkbox_checked)>
+int ShowMessageBoxSync(NativeWindow* parent_window,
-    MessageBoxCallback;
+                       MessageBoxType type,
+                       const std::vector<std::string>& buttons,
+                       int default_id,
+                       int cancel_id,
+                       int options,
+                       const std::string& title,
+                       const std::string& message,
+                       const std::string& detail,
+                       const gfx::ImageSkia& icon);
 
-int ShowMessageBox(NativeWindow* parent_window,
+typedef base::OnceCallback<void(int code, bool checkbox_checked)>
-                   MessageBoxType type,
+    MessageBoxCallback;
-                   const std::vector<std::string>& buttons,
-                   int default_id,
-                   int cancel_id,
-                   int options,
-                   const std::string& title,
-                   const std::string& message,
-                   const std::string& detail,
-                   const gfx::ImageSkia& icon);
 
 void ShowMessageBox(NativeWindow* parent_window,
                     MessageBoxType type,
@@ -58,7 +58,7 @@ void ShowMessageBox(NativeWindow* parent_window,
                     const std::string& checkbox_label,
                     bool checkbox_checked,
                     const gfx::ImageSkia& icon,
-                    const MessageBoxCallback& callback);
+                    MessageBoxCallback callback);
 
 // Like ShowMessageBox with simplest settings, but safe to call at very early
 // stage of application.

atom/browser/ui/message_box_gtk.cc

@@ -148,14 +148,12 @@ class GtkMessageBox : public NativeWindowObserver {
   int RunSynchronous() {
     Show();
     int response = gtk_dialog_run(GTK_DIALOG(dialog_));
-    if (response < 0)
+    return (response < 0) ? cancel_id_ : response;
-      return cancel_id_;
-    else
-      return response;
   }
 
-  void RunAsynchronous(const MessageBoxCallback& callback) {
+  void RunAsynchronous(MessageBoxCallback callback) {
-    callback_ = callback;
+    callback_ = std::move(callback);
+
     g_signal_connect(dialog_, "delete-event",
                      G_CALLBACK(gtk_widget_hide_on_delete), nullptr);
     g_signal_connect(dialog_, "response", G_CALLBACK(OnResponseDialogThunk),
@@ -190,9 +188,9 @@ void GtkMessageBox::OnResponseDialog(GtkWidget* widget, int response) {
   gtk_widget_hide(dialog_);
 
   if (response < 0)
-    callback_.Run(cancel_id_, checkbox_checked_);
+    std::move(callback_).Run(cancel_id_, checkbox_checked_);
   else
-    callback_.Run(response, checkbox_checked_);
+    std::move(callback_).Run(response, checkbox_checked_);
   delete this;
 }
 
@@ -202,16 +200,16 @@ void GtkMessageBox::OnCheckboxToggled(GtkWidget* widget) {
 
 }  // namespace
 
-int ShowMessageBox(NativeWindow* parent,
+int ShowMessageBoxSync(NativeWindow* parent,
-                   MessageBoxType type,
+                       MessageBoxType type,
-                   const std::vector<std::string>& buttons,
+                       const std::vector<std::string>& buttons,
-                   int default_id,
+                       int default_id,
-                   int cancel_id,
+                       int cancel_id,
-                   int options,
+                       int options,
-                   const std::string& title,
+                       const std::string& title,
-                   const std::string& message,
+                       const std::string& message,
-                   const std::string& detail,
+                       const std::string& detail,
-                   const gfx::ImageSkia& icon) {
+                       const gfx::ImageSkia& icon) {
   return GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
                        message, detail, "", false, icon)
       .RunSynchronous();
@@ -229,10 +227,10 @@ void ShowMessageBox(NativeWindow* parent,
                     const std::string& checkbox_label,
                     bool checkbox_checked,
                     const gfx::ImageSkia& icon,
-                    const MessageBoxCallback& callback) {
+                    MessageBoxCallback callback) {
   (new GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
                      message, detail, checkbox_label, checkbox_checked, icon))
-      ->RunAsynchronous(callback);
+      ->RunAsynchronous(std::move(callback));
 }
 
 void ShowErrorBox(const base::string16& title, const base::string16& content) {

atom/browser/ui/message_box_mac.mm

@@ -90,16 +90,16 @@ NSAlert* CreateNSAlert(NativeWindow* parent_window,
 
 }  // namespace
 
-int ShowMessageBox(NativeWindow* parent_window,
+int ShowMessageBoxSync(NativeWindow* parent_window,
-                   MessageBoxType type,
+                       MessageBoxType type,
-                   const std::vector<std::string>& buttons,
+                       const std::vector<std::string>& buttons,
-                   int default_id,
+                       int default_id,
-                   int cancel_id,
+                       int cancel_id,
-                   int options,
+                       int options,
-                   const std::string& title,
+                       const std::string& title,
-                   const std::string& message,
+                       const std::string& message,
-                   const std::string& detail,
+                       const std::string& detail,
-                   const gfx::ImageSkia& icon) {
+                       const gfx::ImageSkia& icon) {
   NSAlert* alert =
       CreateNSAlert(parent_window, type, buttons, default_id, cancel_id, title,
                     message, detail, "", false, icon);
@@ -134,7 +134,7 @@ void ShowMessageBox(NativeWindow* parent_window,
                     const std::string& checkbox_label,
                     bool checkbox_checked,
                     const gfx::ImageSkia& icon,
-                    const MessageBoxCallback& callback) {
+                    MessageBoxCallback callback) {
   NSAlert* alert =
       CreateNSAlert(parent_window, type, buttons, default_id, cancel_id, title,
                     message, detail, checkbox_label, checkbox_checked, icon);
@@ -143,18 +143,20 @@ void ShowMessageBox(NativeWindow* parent_window,
   // window to wait for.
   if (!parent_window) {
     int ret = [[alert autorelease] runModal];
-    callback.Run(ret, alert.suppressionButton.state == NSOnState);
+    std::move(callback).Run(ret, alert.suppressionButton.state == NSOnState);
   } else {
     NSWindow* window =
         parent_window ? parent_window->GetNativeWindow().GetNativeNSWindow()
                       : nil;
+
     // 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_ = callback;
+    __block MessageBoxCallback callback_ = std::move(callback);
+
     [alert beginSheetModalForWindow:window
                   completionHandler:^(NSModalResponse response) {
-                    callback_.Run(response,
+                    std::move(callback_).Run(
-                                  alert.suppressionButton.state == NSOnState);
+                        response, alert.suppressionButton.state == NSOnState);
                   }];
   }
 }

atom/browser/ui/message_box_win.cc

@@ -14,7 +14,6 @@
 #include "atom/browser/browser.h"
 #include "atom/browser/native_window_views.h"
 #include "atom/browser/unresponsive_suppressor.h"
-#include "base/callback.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
@@ -220,28 +219,29 @@ void RunMessageBoxInNewThread(base::Thread* thread,
                               const std::string& checkbox_label,
                               bool checkbox_checked,
                               const gfx::ImageSkia& icon,
-                              const MessageBoxCallback& callback) {
+                              MessageBoxCallback callback) {
   int result = ShowTaskDialogUTF8(parent, type, buttons, default_id, cancel_id,
                                   options, title, message, detail,
                                   checkbox_label, &checkbox_checked, icon);
-  base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI},
+  base::PostTaskWithTraits(
-                           base::Bind(callback, result, checkbox_checked));
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(std::move(callback), result, checkbox_checked));
   content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE,
                                      thread);
 }
 
 }  // namespace
 
-int ShowMessageBox(NativeWindow* parent,
+int ShowMessageBoxSync(NativeWindow* parent,
-                   MessageBoxType type,
+                       MessageBoxType type,
-                   const std::vector<std::string>& buttons,
+                       const std::vector<std::string>& buttons,
-                   int default_id,
+                       int default_id,
-                   int cancel_id,
+                       int cancel_id,
-                   int options,
+                       int options,
-                   const std::string& title,
+                       const std::string& title,
-                   const std::string& message,
+                       const std::string& message,
-                   const std::string& detail,
+                       const std::string& detail,
-                   const gfx::ImageSkia& icon) {
+                       const gfx::ImageSkia& icon) {
   atom::UnresponsiveSuppressor suppressor;
   return ShowTaskDialogUTF8(parent, type, buttons, default_id, cancel_id,
                             options, title, message, detail, "", nullptr, icon);
@@ -259,22 +259,22 @@ void ShowMessageBox(NativeWindow* parent,
                     const std::string& checkbox_label,
                     bool checkbox_checked,
                     const gfx::ImageSkia& icon,
-                    const MessageBoxCallback& callback) {
+                    MessageBoxCallback callback) {
   auto thread =
       std::make_unique<base::Thread>(ATOM_PRODUCT_NAME "MessageBoxThread");
   thread->init_com_with_mta(false);
   if (!thread->Start()) {
-    callback.Run(cancel_id, checkbox_checked);
+    std::move(callback).Run(cancel_id, checkbox_checked);
     return;
   }
 
   base::Thread* unretained = thread.release();
   unretained->task_runner()->PostTask(
       FROM_HERE,
-      base::Bind(&RunMessageBoxInNewThread, base::Unretained(unretained),
+      base::BindOnce(&RunMessageBoxInNewThread, base::Unretained(unretained),
-                 parent, type, buttons, default_id, cancel_id, options, title,
+                     parent, type, buttons, default_id, cancel_id, options,
-                 message, detail, checkbox_label, checkbox_checked, icon,
+                     title, message, detail, checkbox_label, checkbox_checked,
-                 callback));
+                     icon, std::move(callback)));
 }
 
 void ShowErrorBox(const base::string16& title, const base::string16& content) {

docs/api/dialog.md

@@ -209,7 +209,7 @@ The `filters` specifies an array of file types that can be displayed, see
 **Note:** On macOS, using the asynchronous version is recommended to avoid issues when
 expanding and collapsing the dialog.
 
-### `dialog.showMessageBox([browserWindow, ]options[, callback])`
+### `dialog.showMessageBoxSync([browserWindow, ]options)`
 
 * `browserWindow` [BrowserWindow](browser-window.md) (optional)
 * `options` Object
@@ -247,21 +247,61 @@ expanding and collapsing the dialog.
     untouched on Windows. For example, a button label of `Vie&w` will be
     converted to `Vie_w` on Linux and `View` on macOS and can be selected
     via `Alt-W` on Windows and Linux.
-* `callback` Function (optional)
-  * `response` Number - The index of the button that was clicked.
-  * `checkboxChecked` Boolean - The checked state of the checkbox if
-    `checkboxLabel` was set. Otherwise `false`.
 
-Returns `Integer`, the index of the clicked button, if a callback is provided
+Returns `Integer` - the index of the clicked button.
-it returns undefined.
 
 Shows a message box, it will block the process until the message box is closed.
 It returns the index of the clicked button.
 
 The `browserWindow` argument allows the dialog to attach itself to a parent window, making it modal.
 
-If the `callback` and `browserWindow` arguments are passed, the dialog will not block the process. The API call
+### `dialog.showMessageBox([browserWindow, ]options)`
-will be asynchronous and the result will be passed via `callback(response)`.
+
+* `browserWindow` [BrowserWindow](browser-window.md) (optional)
+* `options` Object
+  * `type` String (optional) - Can be `"none"`, `"info"`, `"error"`, `"question"` or
+  `"warning"`. On Windows, `"question"` displays the same icon as `"info"`, unless
+  you set an icon using the `"icon"` option. On macOS, both `"warning"` and
+  `"error"` display the same warning icon.
+  * `buttons` String[] (optional) - Array of texts for buttons. On Windows, an empty array
+    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.
+  * `title` String (optional) - Title of the message box, some platforms will not show it.
+  * `message` String - Content of the message box.
+  * `detail` String (optional) - Extra information of the message.
+  * `checkboxLabel` String (optional) - If provided, the message box will
+    include a checkbox with the given label. The checkbox state can be
+    inspected only when using `callback`.
+  * `checkboxChecked` Boolean (optional) - Initial checked state of the
+    checkbox. `false` by default.
+  * `icon` [NativeImage](native-image.md) (optional)
+  * `cancelId` Integer (optional) - The index of the button to be used to cancel the dialog, via
+    the `Esc` key. By default this is assigned to the first button with "cancel" or "no" as the
+    label. If no such labeled buttons exist and this option is not set, `0` will be used as the
+    return value or callback response.
+  * `noLink` Boolean (optional) - On Windows Electron will try to figure out which one of
+    the `buttons` are common buttons (like "Cancel" or "Yes"), and show the
+    others as command links in the dialog. This can make the dialog appear in
+    the style of modern Windows apps. If you don't like this behavior, you can
+    set `noLink` to `true`.
+  * `normalizeAccessKeys` Boolean (optional) - Normalize the keyboard access keys
+    across platforms. Default is `false`. Enabling this assumes `&` is used in
+    the button labels for the placement of the keyboard shortcut access key
+    and labels will be converted so they work correctly on each platform, `&`
+    characters are removed on macOS, converted to `_` on Linux, and left
+    untouched on Windows. For example, a button label of `Vie&w` will be
+    converted to `Vie_w` on Linux and `View` on macOS and can be selected
+    via `Alt-W` on Windows and Linux.
+
+Returns `Promise<Object>` - resolves with a promise containing the following properties:
+  * `response` Number - The index of the clicked button.
+  * `checkboxChecked` Boolean - The checked state of the checkbox if
+  `checkboxLabel` was set. Otherwise `false`.
+
+Shows a message box, it will block the process until the message box is closed.
+
+The `browserWindow` argument allows the dialog to attach itself to a parent window, making it modal.
 
 ### `dialog.showErrorBox(title, content)`
 

lib/browser/api/dialog.js

@@ -73,7 +73,7 @@ const checkAppInitialized = function () {
 const saveDialog = (sync, window, options) => {
   checkAppInitialized()
 
-  if (window.constructor !== BrowserWindow) options = window
+  if (window && window.constructor !== BrowserWindow) options = window
   if (options == null) options = { title: 'Save' }
 
   const {
@@ -100,7 +100,7 @@ const saveDialog = (sync, window, options) => {
 const openDialog = (sync, window, options) => {
   checkAppInitialized()
 
-  if (window.constructor !== BrowserWindow) options = window
+  if (window && window.constructor !== BrowserWindow) options = window
   if (options == null) {
     options = {
       title: 'Open',
@@ -138,6 +138,62 @@ const openDialog = (sync, window, options) => {
   return (sync) ? binding.showOpenDialogSync(settings) : binding.showOpenDialog(settings)
 }
 
+const messageBox = (sync, window, options) => {
+  checkAppInitialized()
+
+  if (window && window.constructor !== BrowserWindow) options = window
+  if (options == null) options = { type: 'none' }
+
+  let {
+    buttons = [],
+    cancelId,
+    checkboxLabel = '',
+    checkboxChecked,
+    defaultId = -1,
+    detail = '',
+    icon = null,
+    message = '',
+    title = '',
+    type = 'none'
+  } = options
+
+  const messageBoxType = messageBoxTypes.indexOf(type)
+  if (messageBoxType === -1) throw new TypeError('Invalid message box type')
+  if (!Array.isArray(buttons)) throw new TypeError('Buttons must be an array')
+  if (options.normalizeAccessKeys) buttons = buttons.map(normalizeAccessKey)
+  if (typeof title !== 'string') throw new TypeError('Title must be a string')
+  if (typeof message !== 'string') throw new TypeError('Message must be a string')
+  if (typeof detail !== 'string') throw new TypeError('Detail must be a string')
+  if (typeof checkboxLabel !== 'string') throw new TypeError('checkboxLabel must be a string')
+
+  checkboxChecked = !!checkboxChecked
+
+  // Choose a default button to get selected when dialog is cancelled.
+  if (cancelId == null) {
+    // If the defaultId is set to 0, ensure the cancel button is a different index (1)
+    cancelId = (defaultId === 0 && buttons.length > 1) ? 1 : 0
+    for (let i = 0; i < buttons.length; i++) {
+      const text = buttons[i].toLowerCase()
+      if (text === 'cancel' || text === 'no') {
+        cancelId = i
+        break
+      }
+    }
+  }
+
+  const flags = options.noLink ? messageBoxOptions.noLink : 0
+
+  if (sync) {
+    return binding.showMessageBoxSync(messageBoxType, buttons,
+      defaultId, cancelId, flags, title, message, detail,
+      checkboxLabel, checkboxChecked, icon, window)
+  } else {
+    return binding.showMessageBox(messageBoxType, buttons,
+      defaultId, cancelId, flags, title, message, detail,
+      checkboxLabel, checkboxChecked, icon, window)
+  }
+}
+
 module.exports = {
   showOpenDialog: function (window, options) {
     return openDialog(false, window, options)
@@ -155,92 +211,12 @@ module.exports = {
     return saveDialog(true, window, options)
   },
 
-  showMessageBox: function (...args) {
+  showMessageBox: function (window, options) {
-    checkAppInitialized()
+    return messageBox(false, window, options)
+  },
 
-    let [window, options, callback] = parseArgs(...args)
+  showMessageBoxSync: function (window, options) {
-
+    return messageBox(true, window, options)
-    if (options == null) {
-      options = {
-        type: 'none'
-      }
-    }
-
-    let {
-      buttons, cancelId, checkboxLabel, checkboxChecked, defaultId, detail,
-      icon, message, title, type
-    } = options
-
-    if (type == null) {
-      type = 'none'
-    }
-
-    const messageBoxType = messageBoxTypes.indexOf(type)
-    if (messageBoxType === -1) {
-      throw new TypeError('Invalid message box type')
-    }
-
-    if (buttons == null) {
-      buttons = []
-    } else if (!Array.isArray(buttons)) {
-      throw new TypeError('Buttons must be an array')
-    }
-
-    if (options.normalizeAccessKeys) {
-      buttons = buttons.map(normalizeAccessKey)
-    }
-
-    if (title == null) {
-      title = ''
-    } else if (typeof title !== 'string') {
-      throw new TypeError('Title must be a string')
-    }
-
-    if (message == null) {
-      message = ''
-    } else if (typeof message !== 'string') {
-      throw new TypeError('Message must be a string')
-    }
-
-    if (detail == null) {
-      detail = ''
-    } else if (typeof detail !== 'string') {
-      throw new TypeError('Detail must be a string')
-    }
-
-    checkboxChecked = !!checkboxChecked
-
-    if (checkboxLabel == null) {
-      checkboxLabel = ''
-    } else if (typeof checkboxLabel !== 'string') {
-      throw new TypeError('checkboxLabel must be a string')
-    }
-
-    if (icon == null) {
-      icon = null
-    }
-
-    if (defaultId == null) {
-      defaultId = -1
-    }
-
-    // Choose a default button to get selected when dialog is cancelled.
-    if (cancelId == null) {
-      // If the defaultId is set to 0, ensure the cancel button is a different index (1)
-      cancelId = (defaultId === 0 && buttons.length > 1) ? 1 : 0
-      for (let i = 0; i < buttons.length; i++) {
-        const text = buttons[i].toLowerCase()
-        if (text === 'cancel' || text === 'no') {
-          cancelId = i
-          break
-        }
-      }
-    }
-
-    const flags = options.noLink ? messageBoxOptions.noLink : 0
-    return binding.showMessageBox(messageBoxType, buttons, defaultId, cancelId,
-      flags, title, message, detail, checkboxLabel,
-      checkboxChecked, icon, window, callback)
   },
 
   showErrorBox: function (...args) {
@@ -269,6 +245,7 @@ module.exports = {
   }
 }
 
+module.exports.showMessageBox = deprecate.promisify(module.exports.showMessageBox)
 module.exports.showOpenDialog = deprecate.promisify(module.exports.showOpenDialog)
 module.exports.showSaveDialog = deprecate.promisify(module.exports.showSaveDialog)