Browse Source

refactor: pass MessageBox params as a struct (#18732)

Historically, we've been passing in all MessageBox parameters individually, which makes augmenting or improving MessageBox functionality challenging because to change or add even one argument requires a huge cascade of argument changes that leaves room for errors.

For other file dialog related APIs, we use a struct (DialogSettings), and so this PR takes a similar approach and refactors MessageBox parameters into a struct (MessageBoxSettings) which we then use to simplify argument passing and which will enable us to more quickly iterate and improve upon functionality in the future.
Shelley Vohr 5 years ago
parent
commit
bfcce8aa27

+ 5 - 31
atom/browser/api/atom_api_dialog.cc

@@ -15,6 +15,7 @@
 #include "atom/common/native_mate_converters/file_dialog_converter.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/native_mate_converters/image_converter.h"
+#include "atom/common/native_mate_converters/message_box_converter.h"
 #include "atom/common/native_mate_converters/net_converter.h"
 #include "atom/common/node_includes.h"
 #include "atom/common/promise_util.h"
@@ -22,21 +23,8 @@
 
 namespace {
 
-int ShowMessageBoxSync(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) {
-  return atom::ShowMessageBoxSync(
-      window, static_cast<atom::MessageBoxType>(type), buttons, default_id,
-      cancel_id, options, title, message, detail, icon);
+int ShowMessageBoxSync(const atom::MessageBoxSettings& settings) {
+  return atom::ShowMessageBoxSync(settings);
 }
 
 void ResolvePromiseObject(atom::util::Promise promise,
@@ -50,28 +38,14 @@ void ResolvePromiseObject(atom::util::Promise promise,
   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,
+v8::Local<v8::Promise> ShowMessageBox(const atom::MessageBoxSettings& settings,
                                       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)));
+      settings, base::BindOnce(&ResolvePromiseObject, std::move(promise)));
 
   return handle;
 }

+ 8 - 3
atom/browser/atom_javascript_dialog_manager.cc

@@ -91,10 +91,15 @@ void AtomJavaScriptDialogManager::RunJavaScriptDialog(
       window = relay->GetNativeWindow();
   }
 
+  atom::MessageBoxSettings settings;
+  settings.parent_window = window;
+  settings.buttons = buttons;
+  settings.default_id = default_id;
+  settings.cancel_id = cancel_id;
+  settings.message = base::UTF16ToUTF8(message_text);
+
   atom::ShowMessageBox(
-      window, atom::MessageBoxType::kNone, buttons, default_id, cancel_id,
-      atom::MessageBoxOptions::MESSAGE_BOX_NONE, "",
-      base::UTF16ToUTF8(message_text), "", checkbox, false, gfx::ImageSkia(),
+      settings,
       base::BindOnce(&AtomJavaScriptDialogManager::OnMessageBoxCallback,
                      base::Unretained(this), base::Passed(std::move(callback)),
                      origin));

+ 0 - 1
atom/browser/ui/file_dialog.h

@@ -11,7 +11,6 @@
 
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/promise_util.h"
-#include "base/callback_forward.h"
 #include "base/files/file_path.h"
 #include "native_mate/dictionary.h"
 

+ 22 - 26
atom/browser/ui/message_box.h

@@ -10,10 +10,7 @@
 
 #include "base/callback_forward.h"
 #include "base/strings/string16.h"
-
-namespace gfx {
-class ImageSkia;
-}
+#include "ui/gfx/image/image_skia.h"
 
 namespace atom {
 
@@ -32,32 +29,31 @@ enum MessageBoxOptions {
   MESSAGE_BOX_NO_LINK = 1 << 0,
 };
 
-int ShowMessageBoxSync(NativeWindow* parent_window,
-                       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);
+struct MessageBoxSettings {
+  atom::NativeWindow* parent_window = nullptr;
+  MessageBoxType type = atom::MessageBoxType::kNone;
+  std::vector<std::string> buttons;
+  int default_id;
+  int cancel_id;
+  int options = atom::MessageBoxOptions::MESSAGE_BOX_NONE;
+  std::string title;
+  std::string message;
+  std::string detail;
+  std::string checkbox_label;
+  bool checkbox_checked = false;
+  gfx::ImageSkia icon;
+
+  MessageBoxSettings();
+  MessageBoxSettings(const MessageBoxSettings&);
+  ~MessageBoxSettings();
+};
+
+int ShowMessageBoxSync(const MessageBoxSettings& settings);
 
 typedef base::OnceCallback<void(int code, bool checkbox_checked)>
     MessageBoxCallback;
 
-void ShowMessageBox(NativeWindow* parent_window,
-                    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 std::string& checkbox_label,
-                    bool checkbox_checked,
-                    const gfx::ImageSkia& icon,
+void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback);
 
 // Like ShowMessageBox with simplest settings, but safe to call at very early

+ 35 - 60
atom/browser/ui/message_box_gtk.cc

@@ -27,42 +27,37 @@
 
 namespace atom {
 
+MessageBoxSettings::MessageBoxSettings() = default;
+MessageBoxSettings::MessageBoxSettings(const MessageBoxSettings&) = default;
+MessageBoxSettings::~MessageBoxSettings() = default;
+
 namespace {
 
 class GtkMessageBox : public NativeWindowObserver {
  public:
-  GtkMessageBox(NativeWindow* parent_window,
-                MessageBoxType type,
-                const std::vector<std::string>& buttons,
-                int default_id,
-                int cancel_id,
-                const std::string& title,
-                const std::string& message,
-                const std::string& detail,
-                const std::string& checkbox_label,
-                bool checkbox_checked,
-                const gfx::ImageSkia& icon)
-      : cancel_id_(cancel_id),
-        parent_(static_cast<NativeWindow*>(parent_window)) {
+  explicit GtkMessageBox(const MessageBoxSettings& settings)
+      : cancel_id_(settings.cancel_id),
+        parent_(static_cast<NativeWindow*>(settings.parent_window)) {
     // Create dialog.
     dialog_ =
         gtk_message_dialog_new(nullptr,                         // parent
                                static_cast<GtkDialogFlags>(0),  // no flags
-                               GetMessageType(type),            // type
+                               GetMessageType(settings.type),   // type
                                GTK_BUTTONS_NONE,                // no buttons
-                               "%s", message.c_str());
-    if (!detail.empty())
+                               "%s", settings.message.c_str());
+    if (!settings.detail.empty())
       gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog_),
-                                               "%s", detail.c_str());
-    if (!title.empty())
-      gtk_window_set_title(GTK_WINDOW(dialog_), title.c_str());
+                                               "%s", settings.detail.c_str());
+    if (!settings.title.empty())
+      gtk_window_set_title(GTK_WINDOW(dialog_), settings.title.c_str());
 
-    if (!icon.isNull()) {
+    if (!settings.icon.isNull()) {
       // No easy way to obtain this programmatically, but GTK+'s docs
       // define GTK_ICON_SIZE_DIALOG to be 48 pixels
       static constexpr int pixel_width = 48;
       static constexpr int pixel_height = 48;
-      GdkPixbuf* pixbuf = libgtkui::GdkPixbufFromSkBitmap(*icon.bitmap());
+      GdkPixbuf* pixbuf =
+          libgtkui::GdkPixbufFromSkBitmap(*settings.icon.bitmap());
       GdkPixbuf* scaled_pixbuf = gdk_pixbuf_scale_simple(
           pixbuf, pixel_width, pixel_height, GDK_INTERP_BILINEAR);
       GtkWidget* w = gtk_image_new_from_pixbuf(scaled_pixbuf);
@@ -72,25 +67,26 @@ class GtkMessageBox : public NativeWindowObserver {
       g_clear_pointer(&pixbuf, g_object_unref);
     }
 
-    if (!checkbox_label.empty()) {
+    if (!settings.checkbox_label.empty()) {
       GtkWidget* message_area =
           gtk_message_dialog_get_message_area(GTK_MESSAGE_DIALOG(dialog_));
       GtkWidget* check_button =
-          gtk_check_button_new_with_label(checkbox_label.c_str());
+          gtk_check_button_new_with_label(settings.checkbox_label.c_str());
       g_signal_connect(check_button, "toggled",
                        G_CALLBACK(OnCheckboxToggledThunk), this);
       gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(check_button),
-                                   checkbox_checked);
+                                   settings.checkbox_checked);
       gtk_container_add(GTK_CONTAINER(message_area), check_button);
       gtk_widget_show(check_button);
     }
 
     // Add buttons.
     GtkDialog* dialog = GTK_DIALOG(dialog_);
-    for (size_t i = 0; i < buttons.size(); ++i) {
-      gtk_dialog_add_button(dialog, TranslateToStock(i, buttons[i]), i);
+    for (size_t i = 0; i < settings.buttons.size(); ++i) {
+      gtk_dialog_add_button(dialog, TranslateToStock(i, settings.buttons[i]),
+                            i);
     }
-    gtk_dialog_set_default_response(dialog, default_id);
+    gtk_dialog_set_default_response(dialog, settings.default_id);
 
     // Parent window.
     if (parent_) {
@@ -200,46 +196,25 @@ void GtkMessageBox::OnCheckboxToggled(GtkWidget* widget) {
 
 }  // namespace
 
-int ShowMessageBoxSync(NativeWindow* parent,
-                       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) {
-  return GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
-                       message, detail, "", false, icon)
-      .RunSynchronous();
+int ShowMessageBoxSync(const MessageBoxSettings& settings) {
+  return GtkMessageBox(settings).RunSynchronous();
 }
 
-void ShowMessageBox(NativeWindow* parent,
-                    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 std::string& checkbox_label,
-                    bool checkbox_checked,
-                    const gfx::ImageSkia& icon,
+void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback) {
-  (new GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
-                     message, detail, checkbox_label, checkbox_checked, icon))
-      ->RunAsynchronous(std::move(callback));
+  (new GtkMessageBox(settings))->RunAsynchronous(std::move(callback));
 }
 
 void ShowErrorBox(const base::string16& title, const base::string16& content) {
   if (Browser::Get()->is_ready()) {
-    GtkMessageBox(nullptr, MessageBoxType::kError, {"OK"}, -1, 0, "Error",
-                  base::UTF16ToUTF8(title).c_str(),
-                  base::UTF16ToUTF8(content).c_str(), "", false,
-                  gfx::ImageSkia())
-        .RunSynchronous();
+    atom::MessageBoxSettings settings;
+    settings.type = atom::MessageBoxType::kError;
+    settings.buttons = {"OK"};
+    settings.title = "Error";
+    settings.message = base::UTF16ToUTF8(title);
+    settings.detail = base::UTF16ToUTF8(content);
+
+    GtkMessageBox(settings).RunSynchronous();
   } else {
     fprintf(stderr,
             ANSI_TEXT_BOLD ANSI_BACKGROUND_GRAY ANSI_FOREGROUND_RED

+ 34 - 59
atom/browser/ui/message_box_mac.mm

@@ -19,25 +19,19 @@
 
 namespace atom {
 
+MessageBoxSettings::MessageBoxSettings() = default;
+MessageBoxSettings::MessageBoxSettings(const MessageBoxSettings&) = default;
+MessageBoxSettings::~MessageBoxSettings() = default;
+
 namespace {
 
-NSAlert* CreateNSAlert(NativeWindow* parent_window,
-                       MessageBoxType type,
-                       const std::vector<std::string>& buttons,
-                       int default_id,
-                       int cancel_id,
-                       const std::string& title,
-                       const std::string& message,
-                       const std::string& detail,
-                       const std::string& checkbox_label,
-                       bool checkbox_checked,
-                       const gfx::ImageSkia& icon) {
+NSAlert* CreateNSAlert(const MessageBoxSettings& settings) {
   // Ignore the title; it's the window title on other platforms and ignorable.
   NSAlert* alert = [[NSAlert alloc] init];
-  [alert setMessageText:base::SysUTF8ToNSString(message)];
-  [alert setInformativeText:base::SysUTF8ToNSString(detail)];
+  [alert setMessageText:base::SysUTF8ToNSString(settings.message)];
+  [alert setInformativeText:base::SysUTF8ToNSString(settings.detail)];
 
-  switch (type) {
+  switch (settings.type) {
     case MessageBoxType::kInformation:
       alert.alertStyle = NSInformationalAlertStyle;
       break;
@@ -52,10 +46,10 @@ NSAlert* CreateNSAlert(NativeWindow* parent_window,
       break;
   }
 
-  for (size_t i = 0; i < buttons.size(); ++i) {
-    NSString* title = base::SysUTF8ToNSString(buttons[i]);
+  for (size_t i = 0; i < settings.buttons.size(); ++i) {
+    NSString* title = base::SysUTF8ToNSString(settings.buttons[i]);
     // An empty title causes crash on macOS.
-    if (buttons[i].empty())
+    if (settings.buttons[i].empty())
       title = @"(empty)";
     NSButton* button = [alert addButtonWithTitle:title];
     [button setTag:i];
@@ -64,28 +58,31 @@ NSAlert* CreateNSAlert(NativeWindow* parent_window,
   NSArray* ns_buttons = [alert buttons];
   int button_count = static_cast<int>([ns_buttons count]);
 
-  if (default_id >= 0 && default_id < button_count) {
+  if (settings.default_id >= 0 && settings.default_id < button_count) {
     // Focus the button at default_id if the user opted to do so.
     // The first button added gets set as the default selected.
     // So remove that default, and make the requested button the default.
     [[ns_buttons objectAtIndex:0] setKeyEquivalent:@""];
-    [[ns_buttons objectAtIndex:default_id] setKeyEquivalent:@"\r"];
+    [[ns_buttons objectAtIndex:settings.default_id] setKeyEquivalent:@"\r"];
   }
 
   // Bind cancel id button to escape key if there is more than one button
-  if (button_count > 1 && cancel_id >= 0 && cancel_id < button_count) {
-    [[ns_buttons objectAtIndex:cancel_id] setKeyEquivalent:@"\e"];
+  if (button_count > 1 && settings.cancel_id >= 0 &&
+      settings.cancel_id < button_count) {
+    [[ns_buttons objectAtIndex:settings.cancel_id] setKeyEquivalent:@"\e"];
   }
 
-  if (!checkbox_label.empty()) {
+  if (!settings.checkbox_label.empty()) {
     alert.showsSuppressionButton = YES;
-    alert.suppressionButton.title = base::SysUTF8ToNSString(checkbox_label);
-    alert.suppressionButton.state = checkbox_checked ? NSOnState : NSOffState;
+    alert.suppressionButton.title =
+        base::SysUTF8ToNSString(settings.checkbox_label);
+    alert.suppressionButton.state =
+        settings.checkbox_checked ? NSOnState : NSOffState;
   }
 
-  if (!icon.isNull()) {
+  if (!settings.icon.isNull()) {
     NSImage* image = skia::SkBitmapToNSImageWithColorSpace(
-        *icon.bitmap(), base::mac::GetGenericRGBColorSpace());
+        *settings.icon.bitmap(), base::mac::GetGenericRGBColorSpace());
     [alert setIcon:image];
   }
 
@@ -94,28 +91,18 @@ NSAlert* CreateNSAlert(NativeWindow* parent_window,
 
 }  // namespace
 
-int ShowMessageBoxSync(NativeWindow* parent_window,
-                       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) {
-  NSAlert* alert =
-      CreateNSAlert(parent_window, type, buttons, default_id, cancel_id, title,
-                    message, detail, "", false, icon);
+int ShowMessageBoxSync(const MessageBoxSettings& settings) {
+  NSAlert* alert = CreateNSAlert(settings);
 
   // Use runModal for synchronous alert without parent, since we don't have a
   // window to wait for.
-  if (!parent_window)
+  if (!settings.parent_window)
     return [[alert autorelease] runModal];
 
   __block int ret_code = -1;
 
-  NSWindow* window = parent_window->GetNativeWindow().GetNativeNSWindow();
+  NSWindow* window =
+      settings.parent_window->GetNativeWindow().GetNativeNSWindow();
   [alert beginSheetModalForWindow:window
                 completionHandler:^(NSModalResponse response) {
                   ret_code = response;
@@ -126,32 +113,20 @@ int ShowMessageBoxSync(NativeWindow* parent_window,
   return ret_code;
 }
 
-void ShowMessageBox(NativeWindow* parent_window,
-                    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 std::string& checkbox_label,
-                    bool checkbox_checked,
-                    const gfx::ImageSkia& icon,
+void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback) {
-  NSAlert* alert =
-      CreateNSAlert(parent_window, type, buttons, default_id, cancel_id, title,
-                    message, detail, checkbox_label, checkbox_checked, icon);
+  NSAlert* alert = CreateNSAlert(settings);
 
   // Use runModal for synchronous alert without parent, since we don't have a
   // window to wait for.
-  if (!parent_window) {
+  if (!settings.parent_window) {
     int ret = [[alert autorelease] runModal];
     std::move(callback).Run(ret, alert.suppressionButton.state == NSOnState);
   } else {
     NSWindow* window =
-        parent_window ? parent_window->GetNativeWindow().GetNativeNSWindow()
-                      : nil;
+        settings.parent_window
+            ? settings.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.

+ 24 - 61
atom/browser/ui/message_box_win.cc

@@ -26,6 +26,10 @@
 
 namespace atom {
 
+MessageBoxSettings::MessageBoxSettings() = default;
+MessageBoxSettings::MessageBoxSettings(const MessageBoxSettings&) = default;
+MessageBoxSettings::~MessageBoxSettings() = default;
+
 namespace {
 
 // Small command ID values are already taken by Windows, we have to start from
@@ -183,88 +187,49 @@ int ShowTaskDialogUTF16(NativeWindow* parent,
     return cancel_id;
 }
 
-int ShowTaskDialogUTF8(NativeWindow* parent,
-                       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 std::string& checkbox_label,
-                       bool* checkbox_checked,
-                       const gfx::ImageSkia& icon) {
+int ShowTaskDialogUTF8(const MessageBoxSettings& settings) {
   std::vector<base::string16> utf16_buttons;
-  for (const auto& button : buttons)
+  for (const auto& button : settings.buttons)
     utf16_buttons.push_back(base::UTF8ToUTF16(button));
 
+  const base::string16 title_16 = base::UTF8ToUTF16(settings.title);
+  const base::string16 message_16 = base::UTF8ToUTF16(settings.message);
+  const base::string16 detail_16 = base::UTF8ToUTF16(settings.detail);
+  const base::string16 checkbox_label_16 =
+      base::UTF8ToUTF16(settings.checkbox_label);
+  bool cb_checked = settings.checkbox_checked;
+
   return ShowTaskDialogUTF16(
-      parent, type, utf16_buttons, default_id, cancel_id, options,
-      base::UTF8ToUTF16(title), base::UTF8ToUTF16(message),
-      base::UTF8ToUTF16(detail), base::UTF8ToUTF16(checkbox_label),
-      checkbox_checked, icon);
+      settings.parent_window, settings.type, utf16_buttons, settings.default_id,
+      settings.cancel_id, settings.options, title_16, message_16, detail_16,
+      checkbox_label_16, &cb_checked, settings.icon);
 }
 
 void RunMessageBoxInNewThread(base::Thread* thread,
-                              NativeWindow* parent,
-                              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 std::string& checkbox_label,
-                              bool checkbox_checked,
-                              const gfx::ImageSkia& icon,
+                              const MessageBoxSettings& settings,
                               MessageBoxCallback callback) {
-  int result = ShowTaskDialogUTF8(parent, type, buttons, default_id, cancel_id,
-                                  options, title, message, detail,
-                                  checkbox_label, &checkbox_checked, icon);
+  int result = ShowTaskDialogUTF8(settings);
   base::PostTaskWithTraits(
       FROM_HERE, {content::BrowserThread::UI},
-      base::BindOnce(std::move(callback), result, checkbox_checked));
+      base::BindOnce(std::move(callback), result, settings.checkbox_checked));
   content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE,
                                      thread);
 }
 
 }  // namespace
 
-int ShowMessageBoxSync(NativeWindow* parent,
-                       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 ShowMessageBoxSync(const MessageBoxSettings& settings) {
   atom::UnresponsiveSuppressor suppressor;
-  return ShowTaskDialogUTF8(parent, type, buttons, default_id, cancel_id,
-                            options, title, message, detail, "", nullptr, icon);
+  return ShowTaskDialogUTF8(settings);
 }
 
-void ShowMessageBox(NativeWindow* parent,
-                    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 std::string& checkbox_label,
-                    bool checkbox_checked,
-                    const gfx::ImageSkia& icon,
+void ShowMessageBox(const MessageBoxSettings& settings,
                     MessageBoxCallback callback) {
   auto thread =
       std::make_unique<base::Thread>(ATOM_PRODUCT_NAME "MessageBoxThread");
   thread->init_com_with_mta(false);
   if (!thread->Start()) {
-    std::move(callback).Run(cancel_id, checkbox_checked);
+    std::move(callback).Run(settings.cancel_id, settings.checkbox_checked);
     return;
   }
 
@@ -272,9 +237,7 @@ void ShowMessageBox(NativeWindow* parent,
   unretained->task_runner()->PostTask(
       FROM_HERE,
       base::BindOnce(&RunMessageBoxInNewThread, base::Unretained(unretained),
-                     parent, type, buttons, default_id, cancel_id, options,
-                     title, message, detail, checkbox_label, checkbox_checked,
-                     icon, std::move(callback)));
+                     settings, std::move(callback)));
 }
 
 void ShowErrorBox(const base::string16& title, const base::string16& content) {

+ 38 - 0
atom/common/native_mate_converters/message_box_converter.cc

@@ -0,0 +1,38 @@
+// Copyright (c) 2015 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/common/native_mate_converters/message_box_converter.h"
+
+#include "atom/browser/api/atom_api_browser_window.h"
+#include "atom/common/native_mate_converters/file_path_converter.h"
+#include "atom/common/native_mate_converters/image_converter.h"
+#include "native_mate/dictionary.h"
+
+namespace mate {
+
+bool Converter<atom::MessageBoxSettings>::FromV8(
+    v8::Isolate* isolate,
+    v8::Local<v8::Value> val,
+    atom::MessageBoxSettings* out) {
+  mate::Dictionary dict;
+  int type = 0;
+  if (!ConvertFromV8(isolate, val, &dict))
+    return false;
+  dict.Get("window", &out->parent_window);
+  dict.Get("type", &type);
+  out->type = static_cast<atom::MessageBoxType>(type);
+  dict.Get("buttons", &out->buttons);
+  dict.Get("defaultId", &out->default_id);
+  dict.Get("cancelId", &out->cancel_id);
+  dict.Get("options", &out->options);
+  dict.Get("title", &out->title);
+  dict.Get("message", &out->message);
+  dict.Get("detail", &out->detail);
+  dict.Get("checkboxLabel", &out->checkbox_label);
+  dict.Get("checkboxChecked", &out->checkbox_checked);
+  dict.Get("icon", &out->icon);
+  return true;
+}
+
+}  // namespace mate

+ 22 - 0
atom/common/native_mate_converters/message_box_converter.h

@@ -0,0 +1,22 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_
+#define ATOM_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_
+
+#include "atom/browser/ui/message_box.h"
+#include "native_mate/converter.h"
+
+namespace mate {
+
+template <>
+struct Converter<atom::MessageBoxSettings> {
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     atom::MessageBoxSettings* out);
+};
+
+}  // namespace mate
+
+#endif  // ATOM_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_

+ 2 - 0
filenames.gni

@@ -523,6 +523,8 @@ filenames = {
     "atom/common/native_mate_converters/content_converter.h",
     "atom/common/native_mate_converters/file_dialog_converter.cc",
     "atom/common/native_mate_converters/file_dialog_converter.h",
+    "atom/common/native_mate_converters/message_box_converter.cc",
+    "atom/common/native_mate_converters/message_box_converter.h",
     "atom/common/native_mate_converters/file_path_converter.h",
     "atom/common/native_mate_converters/gfx_converter.cc",
     "atom/common/native_mate_converters/gfx_converter.h",

+ 17 - 6
lib/browser/api/dialog.js

@@ -172,14 +172,25 @@ const messageBox = (sync, window, options) => {
 
   const flags = options.noLink ? messageBoxOptions.noLink : 0
 
+  const settings = {
+    window,
+    messageBoxType,
+    buttons,
+    defaultId,
+    cancelId,
+    flags,
+    title,
+    message,
+    detail,
+    checkboxLabel,
+    checkboxChecked,
+    icon
+  }
+
   if (sync) {
-    return binding.showMessageBoxSync(messageBoxType, buttons,
-      defaultId, cancelId, flags, title, message, detail,
-      checkboxLabel, checkboxChecked, icon, window)
+    return binding.showMessageBoxSync(settings)
   } else {
-    return binding.showMessageBox(messageBoxType, buttons,
-      defaultId, cancelId, flags, title, message, detail,
-      checkboxLabel, checkboxChecked, icon, window)
+    return binding.showMessageBox(settings)
   }
 }