Make a new yes/no messagebox wrapper function, use it in the bookmark alert.

Recommitting of r46101.

BUG=http://crbug.com/34481; http://crbug.com/40011
TEST=on Mac/ChromeOS, have at least 15 bookmarks on bookmarks bar or other bookmarks,
right click on one of those and click on Open all bookmarks. A confirmation
dialog box should be shown. No other visible change.

Review URL: http://codereview.chromium.org/1745024

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50894 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 105 insertions, 57 deletions

diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 63cf429..211c4e6 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/history/query_parser.h"
+#include "chrome/browser/platform_util.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/page_navigator.h"
@@ -164,37 +165,11 @@ bool ShouldOpenAll(gfx::NativeWindow parent,
   if (descendant_count < bookmark_utils::num_urls_before_prompting)
     return true;
 
-  // Bug 40011: we should refactor this into a cross-platform "prompt before
-  // continuing" function.
-#if defined(OS_WIN)
-  std::wstring message =
-      l10n_util::GetStringF(IDS_BOOKMARK_BAR_SHOULD_OPEN_ALL,
-                            IntToWString(descendant_count));
-  return MessageBox(parent, message.c_str(),
-                    l10n_util::GetString(IDS_PRODUCT_NAME).c_str(),
-                    MB_YESNO | MB_ICONWARNING | MB_TOPMOST) == IDYES;
-#elif defined(TOOLKIT_GTK)
-  std::string message = l10n_util::GetStringFUTF8(
+  string16 message = l10n_util::GetStringFUTF16(
       IDS_BOOKMARK_BAR_SHOULD_OPEN_ALL,
       IntToString16(descendant_count));
-  GtkWidget* dialog = gtk_message_dialog_new(parent,
-      static_cast<GtkDialogFlags>(
-          GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT),
-      GTK_MESSAGE_QUESTION,
-      GTK_BUTTONS_YES_NO,
-      "%s", message.c_str());
-  gtk_util::ApplyMessageDialogQuirks(dialog);
-  gtk_window_set_title(GTK_WINDOW(dialog),
-                       l10n_util::GetStringUTF8(IDS_PRODUCT_NAME).c_str());
-  gint result = gtk_dialog_run(GTK_DIALOG(dialog));
-  gtk_widget_destroy(dialog);
-  return (result == GTK_RESPONSE_YES);
-#else
-  // TODO(port): Display a dialog prompt.
-  // http://crbug.com/34481
-  NOTIMPLEMENTED();
-  return true;
-#endif
+  string16 title = l10n_util::GetStringUTF16(IDS_PRODUCT_NAME);
+  return platform_util::SimpleYesNoBox(parent, title, message);
 }
 
 // Comparison function that compares based on date modified of the two nodes.
@@ -358,7 +333,8 @@ void DragBookmarks(Profile* profile,
   BookmarkDragData drag_data(nodes);
   drag_data.Write(profile, &data);
 
-  views::RootView* root_view = views::Widget::GetWidgetFromNativeView(view)->GetRootView();
+  views::RootView* root_view =
+      views::Widget::GetWidgetFromNativeView(view)->GetRootView();
 
   // Allow nested message loop so we get DnD events as we drag this around.
   bool was_nested = MessageLoop::current()->IsNested();
diff --git a/chrome/browser/hang_monitor/hung_plugin_action.cc b/chrome/browser/hang_monitor/hung_plugin_action.cc
index 5bed393..5fdcfd1 100644
--- a/chrome/browser/hang_monitor/hung_plugin_action.cc
+++ b/chrome/browser/hang_monitor/hung_plugin_action.cc
@@ -9,6 +9,7 @@
 #include "app/l10n_util.h"
 #include "app/win_util.h"
 #include "base/win_util.h"
+#include "chrome/browser/platform_util.h"
 #include "chrome/common/logging_chrome.h"
 #include "grit/generated_resources.h"
 #include "webkit/glue/plugins/webplugin_delegate_impl.h"
@@ -62,8 +63,7 @@ bool HungPluginAction::OnHungWindowDetected(HWND hung_window,
                           HungWindowResponseCallback,
                           reinterpret_cast<ULONG_PTR>(this));
       current_hung_plugin_window_ = hung_window;
-      const UINT mb_flags = MB_YESNO | MB_ICONQUESTION | MB_SETFOREGROUND;
-      if (IDYES == win_util::MessageBox(NULL, msg, title, mb_flags)) {
+      if (platform_util::SimpleYesNoBox(NULL, title, msg)) {
         *action = HungWindowNotification::HUNG_WINDOW_TERMINATE_PROCESS;
       } else {
         // If the user choses to ignore the hung window warning, the
diff --git a/chrome/browser/platform_util.h b/chrome/browser/platform_util.h
index 4ab45b2..1140561 100644
--- a/chrome/browser/platform_util.h
+++ b/chrome/browser/platform_util.h
@@ -39,11 +39,18 @@ bool IsVisible(gfx::NativeView view);
 
 // Pops up an error box with an OK button. If |parent| is non-null, the box
 // will be modal on it. (On Mac, it is always app-modal.) Generally speaking,
-// this class should not be used for much. Infobars are preferred.
+// this function should not be used for much. Infobars are preferred.
 void SimpleErrorBox(gfx::NativeWindow parent,
                     const string16& title,
                     const string16& message);
 
+// Pops up a dialog box with two buttons (Yes/No), with the default button of
+// Yes. If |parent| is non-null, the box will be modal on it. (On Mac, it is
+// always app-modal.) Returns true if the Yes button was chosen.
+bool SimpleYesNoBox(gfx::NativeWindow parent,
+                    const string16& title,
+                    const string16& message);
+
 // Return a human readable modifier for the version string.  For a
 // branded Chrome (not Chromium), this modifier is the channel (dev,
 // beta, but "" for stable).
diff --git a/chrome/browser/platform_util_common_linux.cc b/chrome/browser/platform_util_common_linux.cc
index 88e9048..40edb70 100644
--- a/chrome/browser/platform_util_common_linux.cc
+++ b/chrome/browser/platform_util_common_linux.cc
@@ -8,6 +8,7 @@
 
 #include "app/gtk_util.h"
 #include "base/file_util.h"
+#include "base/message_loop.h"
 #include "base/process_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/process_watcher.h"
@@ -15,6 +16,45 @@
 #include "gfx/native_widget_types.h"
 #include "googleurl/src/gurl.h"
 
+namespace {
+
+void SetDialogTitle(GtkWidget* dialog, const string16& title) {
+  gtk_window_set_title(GTK_WINDOW(dialog), UTF16ToUTF8(title).c_str());
+
+#if !defined(OS_CHROMEOS)
+  // The following code requires the dialog to be realized. However, we host
+  // dialog's content in a Chrome window without really realize the dialog
+  // on ChromeOS. Thus, skip the following code for ChromeOS.
+  gtk_widget_realize(dialog);
+
+  // Make sure it's big enough to show the title.
+  GtkRequisition req;
+  gtk_widget_size_request(dialog, &req);
+  int width;
+  gtk_util::GetWidgetSizeFromCharacters(dialog, title.length(), 0,
+                                        &width, NULL);
+  // The fudge factor accounts for extra space needed by the frame
+  // decorations as well as width differences between average text and the
+  // actual title text.
+  width = width * 1.2 + 50;
+
+  if (width > req.width)
+    gtk_widget_set_size_request(dialog, width, -1);
+#endif  // !defined(OS_CHROMEOS)
+}
+
+int g_dialog_response;
+
+void HandleOnResponseDialog(GtkWidget* widget,
+                            int response,
+                            void* user_data) {
+  g_dialog_response = response;
+  gtk_widget_destroy(widget);
+  MessageLoop::current()->QuitNow();
+}
+
+}  // namespace
+
 namespace platform_util {
 
 gfx::NativeWindow GetTopLevel(gfx::NativeView view) {
@@ -42,30 +82,29 @@ void SimpleErrorBox(gfx::NativeWindow parent,
   GtkWidget* dialog = gtk_message_dialog_new(parent, GTK_DIALOG_MODAL,
       GTK_MESSAGE_ERROR, GTK_BUTTONS_OK, "%s", UTF16ToUTF8(message).c_str());
   gtk_util::ApplyMessageDialogQuirks(dialog);
-  gtk_window_set_title(GTK_WINDOW(dialog), UTF16ToUTF8(title).c_str());
+  SetDialogTitle(dialog, title);
 
   g_signal_connect(dialog, "response", G_CALLBACK(gtk_widget_destroy), NULL);
   gtk_util::ShowDialog(dialog);
+}
 
-#if !defined(OS_CHROMEOS)
-  // The following code requires the dialog to be realized. However, we host
-  // dialog's content in a Chrome window without really realize the dialog
-  // on ChromeOS. Thus, skip the following code for ChromeOS.
-
-  // Make sure it's big enough to show the title.
-  GtkRequisition req;
-  gtk_widget_size_request(dialog, &req);
-  int width;
-  gtk_util::GetWidgetSizeFromCharacters(dialog, title.length(), 0,
-                                        &width, NULL);
-  // The fudge factor accounts for extra space needed by the frame
-  // decorations as well as width differences between average text and the
-  // actual title text.
-  width = width * 1.2 + 50;
+bool SimpleYesNoBox(gfx::NativeWindow parent,
+                    const string16& title,
+                    const string16& message) {
+  GtkWidget* dialog = gtk_message_dialog_new(parent, GTK_DIALOG_MODAL,
+      GTK_MESSAGE_QUESTION, GTK_BUTTONS_YES_NO, "%s",
+      UTF16ToUTF8(message).c_str());
+  gtk_util::ApplyMessageDialogQuirks(dialog);
+  SetDialogTitle(dialog, title);
 
-  if (width > req.width)
-    gtk_widget_set_size_request(dialog, width, -1);
-#endif  // !defined(OS_CHROMEOS)
+  g_signal_connect(dialog,
+                   "response",
+                   G_CALLBACK(HandleOnResponseDialog),
+                   NULL);
+  gtk_util::ShowDialog(dialog);
+  // Not gtk_dialog_run as it prevents timers from running in the unit tests.
+  MessageLoop::current()->Run();
+  return g_dialog_response == GTK_RESPONSE_YES;
 }
 
 /* Warning: this may be either Linux or ChromeOS */
diff --git a/chrome/browser/platform_util_mac.mm b/chrome/browser/platform_util_mac.mm
index d7a932b..3192fcc 100644
--- a/chrome/browser/platform_util_mac.mm
+++ b/chrome/browser/platform_util_mac.mm
@@ -63,14 +63,31 @@ bool IsVisible(gfx::NativeView view) {
 void SimpleErrorBox(gfx::NativeWindow parent,
                     const string16& title,
                     const string16& message) {
+  // Ignore the title; it's the window title on other platforms and ignorable.
   NSAlert* alert = [[[NSAlert alloc] init] autorelease];
   [alert addButtonWithTitle:l10n_util::GetNSString(IDS_OK)];
-  [alert setMessageText:base::SysUTF16ToNSString(title)];
-  [alert setInformativeText:base::SysUTF16ToNSString(message)];
+  [alert setMessageText:base::SysUTF16ToNSString(message)];
   [alert setAlertStyle:NSWarningAlertStyle];
   [alert runModal];
 }
 
+bool SimpleYesNoBox(gfx::NativeWindow parent,
+                    const string16& title,
+                    const string16& message) {
+  // Ignore the title; it's the window title on other platforms and ignorable.
+  NSAlert* alert = [[[NSAlert alloc] init] autorelease];
+  [alert setMessageText:base::SysUTF16ToNSString(message)];
+  [alert setAlertStyle:NSWarningAlertStyle];
+
+  [alert addButtonWithTitle:
+      l10n_util::GetNSString(IDS_CONFIRM_MESSAGEBOX_YES_BUTTON_LABEL)];
+  [alert addButtonWithTitle:
+      l10n_util::GetNSString(IDS_CONFIRM_MESSAGEBOX_NO_BUTTON_LABEL)];
+
+  NSInteger result = [alert runModal];
+  return result == NSAlertFirstButtonReturn;
+}
+
 string16 GetVersionStringModifier() {
 #if defined(GOOGLE_CHROME_BUILD)
   // Use the main application bundle and not the framework bundle. Keystone
diff --git a/chrome/browser/platform_util_win.cc b/chrome/browser/platform_util_win.cc
index cef8155e..9b0a12c 100644
--- a/chrome/browser/platform_util_win.cc
+++ b/chrome/browser/platform_util_win.cc
@@ -158,6 +158,13 @@ void SimpleErrorBox(gfx::NativeWindow parent,
   win_util::MessageBox(parent, message, title, MB_OK | MB_SETFOREGROUND);
 }
 
+bool SimpleYesNoBox(gfx::NativeWindow parent,
+                    const string16& title,
+                    const string16& message) {
+  return win_util::MessageBox(parent, message.c_str(), title.c_str(),
+      MB_YESNO | MB_ICONWARNING | MB_SETFOREGROUND) == IDYES;
+}
+
 string16 GetVersionStringModifier() {
 #if defined(GOOGLE_CHROME_BUILD)
   FilePath module;
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 3f7dd84..90fcf20 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -14,6 +14,7 @@
 #include "base/win_util.h"
 #include "chrome/browser/browser_init.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/platform_util.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/profile_manager.h"
 #include "chrome/common/chrome_constants.h"
@@ -143,8 +144,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
   if (visible_window) {
     std::wstring text = l10n_util::GetString(IDS_BROWSER_HUNGBROWSER_MESSAGE);
     std::wstring caption = l10n_util::GetString(IDS_PRODUCT_NAME);
-    if (IDYES != win_util::MessageBox(NULL, text, caption,
-                                      MB_YESNO | MB_ICONSTOP | MB_TOPMOST)) {
+    if (!platform_util::SimpleYesNoBox(NULL, caption, text)) {
       // The user denied. Quit silently.
       return PROCESS_NOTIFIED;
     }
diff --git a/chrome/browser/views/bookmark_bar_view_test.cc b/chrome/browser/views/bookmark_bar_view_test.cc
index 77d24c1..8c53df0 100644
--- a/chrome/browser/views/bookmark_bar_view_test.cc
+++ b/chrome/browser/views/bookmark_bar_view_test.cc
@@ -34,8 +34,10 @@
 // See http://crbug.com/40039 for details.
 #define MAYBE_KeyEvents DISABLED_KeyEvents
 
-// See http://crbug.com/47089 for details.
+// Two bugs here. http://crbug.com/47089 for general Linux Views, and
+// http://crbug.com/47452 for ChromiumOS.
 #define MAYBE_CloseWithModalDialog DISABLED_CloseWithModalDialog
+// See http://crbug.com/47089 for details.
 #define MAYBE_CloseMenuAfterClosingContextMenu \
         DISABLED_CloseMenuAfterClosingContextMenu
 #else