This CL is a clean-up of the app_modal_dialog_queue.cc in an attempt to fix a bug 10699.

Not sure what is causing the crasher.
Hopefully after this clean-up we'll get a different stack-trace that might help.

BUG=10699
TEST=Make sure alert/confirm boxes work properly. make sure a background tab that shows a (delayed) alert box  works. Same with a background browser.
     
Review URL: http://codereview.chromium.org/113932

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

6 files changed, 44 insertions, 48 deletions

diff --git a/chrome/browser/app_modal_dialog.cc b/chrome/browser/app_modal_dialog.cc
index 3cfe47f..afabff8 100644
--- a/chrome/browser/app_modal_dialog.cc
+++ b/chrome/browser/app_modal_dialog.cc
@@ -18,7 +18,8 @@ AppModalDialog::AppModalDialog(TabContents* tab_contents,
                                bool display_suppress_checkbox,
                                bool is_before_unload_dialog,
                                IPC::Message* reply_msg)
-    : tab_contents_(tab_contents),
+    : dialog_(NULL),
+      tab_contents_(tab_contents),
       title_(title),
       dialog_flags_(dialog_flags),
       message_text_(message_text),
@@ -62,7 +63,7 @@ void AppModalDialog::ShowModalDialog() {
   // If the TabContents that created this dialog navigated away before this
   // dialog became visible, simply show the next dialog if any.
   if (!tab_contents_) {
-    AppModalDialogQueue::ShowNextDialog();
+    Singleton<AppModalDialogQueue>()->ShowNextDialog();
     delete this;
     return;
   }
@@ -78,7 +79,7 @@ void AppModalDialog::OnCancel() {
   // that were still open in the ModalDialogQueue, which would send activation
   // back to this one. The framework should be improved to handle this, so this
   // is a temporary workaround.
-  AppModalDialogQueue::ShowNextDialog();
+  Singleton<AppModalDialogQueue>()->ShowNextDialog();
 
   if (tab_contents_) {
     tab_contents_->OnJavaScriptMessageBoxClosed(reply_msg_, false,
@@ -88,7 +89,7 @@ void AppModalDialog::OnCancel() {
 
 void AppModalDialog::OnAccept(const std::wstring& prompt_text,
                               bool suppress_js_messages) {
-  AppModalDialogQueue::ShowNextDialog();
+  Singleton<AppModalDialogQueue>()->ShowNextDialog();
 
   if (tab_contents_) {
     tab_contents_->OnJavaScriptMessageBoxClosed(reply_msg_, true,
diff --git a/chrome/browser/app_modal_dialog_queue.cc b/chrome/browser/app_modal_dialog_queue.cc
index e24a7b0..2ab1398 100644
--- a/chrome/browser/app_modal_dialog_queue.cc
+++ b/chrome/browser/app_modal_dialog_queue.cc
@@ -6,47 +6,30 @@
 
 #include "chrome/browser/browser_list.h"
 
-// static
-std::queue<AppModalDialog*>*
-    AppModalDialogQueue::app_modal_dialog_queue_ = NULL;
-AppModalDialog* AppModalDialogQueue::active_dialog_ = NULL;
-
-// static
 void AppModalDialogQueue::AddDialog(AppModalDialog* dialog) {
-  if (!app_modal_dialog_queue_) {
-    app_modal_dialog_queue_ = new std::queue<AppModalDialog*>;
+  if (!active_dialog_) {
     ShowModalDialog(dialog);
+    return;
   }
-
-  // ShowModalDialog can wind up calling ShowNextDialog in some cases, which
-  // can then make app_modal_dialog_queue_ NULL.
-  if (app_modal_dialog_queue_)
-    app_modal_dialog_queue_->push(dialog);
+  app_modal_dialog_queue_.push(dialog);
 }
 
-// static
 void AppModalDialogQueue::ShowNextDialog() {
-  app_modal_dialog_queue_->pop();
-  active_dialog_ = NULL;
-  if (!app_modal_dialog_queue_->empty()) {
-    ShowModalDialog(app_modal_dialog_queue_->front());
+  if (!app_modal_dialog_queue_.empty()) {
+    AppModalDialog* dialog = app_modal_dialog_queue_.front();
+    app_modal_dialog_queue_.pop();
+    ShowModalDialog(dialog);
   } else {
-    delete app_modal_dialog_queue_;
-    app_modal_dialog_queue_ = NULL;
+    active_dialog_ = NULL;
   }
 }
 
-// static
 void AppModalDialogQueue::ActivateModalDialog() {
-  if (!app_modal_dialog_queue_->empty())
-    app_modal_dialog_queue_->front()->ActivateModalDialog();
+  if (active_dialog_)
+    active_dialog_->ActivateModalDialog();
 }
 
-// static
 void AppModalDialogQueue::ShowModalDialog(AppModalDialog* dialog) {
-  // ShowModalDialog can wind up calling ShowNextDialog in some cases,
-  // which will wind up calling this method recursively, so active_dialog_
-  // must be set first.
-  active_dialog_ = dialog;
   dialog->ShowModalDialog();
+  active_dialog_ = dialog;
 }
diff --git a/chrome/browser/app_modal_dialog_queue.h b/chrome/browser/app_modal_dialog_queue.h
index bfe755c..8053342 100644
--- a/chrome/browser/app_modal_dialog_queue.h
+++ b/chrome/browser/app_modal_dialog_queue.h
@@ -7,10 +7,12 @@
 
 #include <queue>
 
+#include "base/singleton.h"
 #include "chrome/browser/app_modal_dialog.h"
 
 // Keeps a queue of AppModalDialogs, making sure only one app modal
 // dialog is shown at a time.
+// This class is a singleton.
 class AppModalDialogQueue {
  public:
   // Adds a modal dialog to the queue, if there are no other dialogs in the
@@ -23,13 +25,13 @@ class AppModalDialogQueue {
   // sloppy app modality.
   // Note: The AppModalDialog |dialog| must be window modal before it
   // can be added as app modal.
-  static void AddDialog(AppModalDialog* dialog);
+  void AddDialog(AppModalDialog* dialog);
 
   // Removes the current dialog in the queue (the one that is being shown).
   // Shows the next dialog in the queue, if any is present. This does not
   // ensure that the currently showing dialog is closed, it just makes it no
   // longer app modal.
-  static void ShowNextDialog();
+  void ShowNextDialog();
 
   // Activates and shows the current dialog, if the user clicks on one of the
   // windows disabled by the presence of an app modal dialog. This forces
@@ -37,29 +39,35 @@ class AppModalDialogQueue {
   // opened the dialog on another virtual desktop. Assumes there is currently a
   // dialog being shown. (Call BrowserList::IsShowingAppModalDialog to test
   // this condition).
-  static void ActivateModalDialog();
+  void ActivateModalDialog();
 
   // Returns true if there is currently an active app modal dialog box.
-  static bool HasActiveDialog() {
+  bool HasActiveDialog() {
     return active_dialog_ != NULL;
   }
 
   // Accessor for |active_dialog_|.
-  static AppModalDialog* active_dialog() {
+  AppModalDialog* active_dialog() {
     return active_dialog_;
   }
 
  private:
+  friend struct DefaultSingletonTraits<AppModalDialogQueue>;
+
+  AppModalDialogQueue() : active_dialog_(NULL) { }
+
   // Shows |dialog| and notifies the BrowserList that a modal dialog is showing.
-  static void ShowModalDialog(AppModalDialog* dialog);
+  void ShowModalDialog(AppModalDialog* dialog);
 
   // Contains all app modal dialogs which are waiting to be shown, with the
   // currently modal dialog at the front of the queue.
-  static std::queue<AppModalDialog*>* app_modal_dialog_queue_;
+  std::queue<AppModalDialog*> app_modal_dialog_queue_;
 
   // The currently active app-modal dialog box's delegate. NULL if there is no
   // active app-modal dialog box.
-  static AppModalDialog* active_dialog_;
+  AppModalDialog* active_dialog_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppModalDialogQueue);
 };
 
 #endif // CHROME_BROWSER_APP_MODAL_DIALOG_QUEUE_H__
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index c2b2db8..c2e0877 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -1342,7 +1342,8 @@ void AutomationProvider::GetNormalBrowserWindowCount(int* window_count) {
 
 void AutomationProvider::GetShowingAppModalDialog(bool* showing_dialog,
                                                   int* dialog_button) {
-  AppModalDialog* dialog_delegate = AppModalDialogQueue::active_dialog();
+  AppModalDialog* dialog_delegate =
+      Singleton<AppModalDialogQueue>()->active_dialog();
   *showing_dialog = (dialog_delegate != NULL);
   if (*showing_dialog)
     *dialog_button = dialog_delegate->GetDialogButtons();
@@ -1353,7 +1354,8 @@ void AutomationProvider::GetShowingAppModalDialog(bool* showing_dialog,
 void AutomationProvider::ClickAppModalDialogButton(int button, bool* success) {
   *success = false;
 
-  AppModalDialog* dialog_delegate = AppModalDialogQueue::active_dialog();
+  AppModalDialog* dialog_delegate =
+      Singleton<AppModalDialogQueue>()->active_dialog();
   if (dialog_delegate &&
       (dialog_delegate->GetDialogButtons() & button) == button) {
     if ((button & MessageBoxFlags::DIALOGBUTTON_OK) ==
diff --git a/chrome/browser/jsmessage_box_handler.cc b/chrome/browser/jsmessage_box_handler.cc
index 151a33a..3923646 100644
--- a/chrome/browser/jsmessage_box_handler.cc
+++ b/chrome/browser/jsmessage_box_handler.cc
@@ -68,9 +68,11 @@ void RunJavascriptMessageBox(TabContents* tab_contents,
                              bool display_suppress_checkbox,
                              IPC::Message* reply_msg) {
   std::wstring title = GetWindowTitle(tab_contents, frame_url, dialog_flags);
-  AppModalDialogQueue::AddDialog(new AppModalDialog(tab_contents, title,
-      dialog_flags, MakeTextSafe(message_text), default_prompt_text,
-      display_suppress_checkbox, false, reply_msg));
+  Singleton<AppModalDialogQueue>()->AddDialog(
+      new AppModalDialog(tab_contents, title,
+                         dialog_flags, MakeTextSafe(message_text),
+                         default_prompt_text, display_suppress_checkbox,
+                         false, reply_msg));
 }
 
 void RunBeforeUnloadDialog(TabContents* tab_contents,
@@ -79,7 +81,7 @@ void RunBeforeUnloadDialog(TabContents* tab_contents,
   std::wstring full_message =
       message_text + L"\n\n" +
       l10n_util::GetString(IDS_BEFOREUNLOAD_MESSAGEBOX_FOOTER);
-  AppModalDialogQueue::AddDialog(new AppModalDialog(
+  Singleton<AppModalDialogQueue>()->AddDialog(new AppModalDialog(
       tab_contents, l10n_util::GetString(IDS_BEFOREUNLOAD_MESSAGEBOX_TITLE),
       MessageBoxFlags::kIsJavascriptConfirm, MakeTextSafe(message_text),
       std::wstring(), false, true, reply_msg));
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 43ac628..339c99c 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -471,13 +471,13 @@ bool BrowserView::GetAccelerator(int cmd_id, views::Accelerator* accelerator) {
 
 bool BrowserView::ActivateAppModalDialog() const {
   // If another browser is app modal, flash and activate the modal browser.
-  if (AppModalDialogQueue::HasActiveDialog()) {
+  if (Singleton<AppModalDialogQueue>()->HasActiveDialog()) {
     Browser* active_browser = BrowserList::GetLastActive();
     if (active_browser && (browser_ != active_browser)) {
       active_browser->window()->FlashFrame();
       active_browser->window()->Activate();
     }
-    AppModalDialogQueue::ActivateModalDialog();
+    Singleton<AppModalDialogQueue>()->ActivateModalDialog();
     return true;
   }
   return false;