Use the default dispatcher where possible for nested message loops.

Notable changes:
 * Add QuitNestedMessageLoop() to client::DispatcherClient, which can be used to
   terminate a nested loop started by RunWithDispatcher().
 * FirstRunDialog is no longer a MessagePumpDispatcher. The default dispatcher
   is used instead, and QuitNestedMessageLoop() is called to terminate the loop
   instead of returning POST_DISPATCH_QUIT_LOOP from the Dispatch() override.

This change was previously committed in r253723 and r254089, but reverted in
r253744 and r254095 because some interactive_ui_tests on windows bots failed on
SimpleMessageBoxViews related change. So this CL aims to reland that CL without
the change in SimpleMessageBoxViews for now.

BUG=342338
R=sky@chromium.org

Review URL: https://codereview.chromium.org/192743007

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

10 files changed, 64 insertions, 34 deletions

diff --git a/ash/accelerators/accelerator_dispatcher.cc b/ash/accelerators/accelerator_dispatcher.cc
index 0395ee7..91ea13b 100644
--- a/ash/accelerators/accelerator_dispatcher.cc
+++ b/ash/accelerators/accelerator_dispatcher.cc
@@ -70,7 +70,6 @@ AcceleratorDispatcher::AcceleratorDispatcher(
     aura::Window* associated_window)
     : nested_dispatcher_(nested_dispatcher),
       associated_window_(associated_window) {
-  DCHECK(nested_dispatcher_);
   associated_window_->AddObserver(this);
 }
 
@@ -121,10 +120,13 @@ uint32_t AcceleratorDispatcher::Dispatch(const base::NativeEvent& event) {
         return POST_DISPATCH_NONE;
     }
 
-    return nested_dispatcher_->Dispatch(key_event.native_event());
+    return nested_dispatcher_
+               ? nested_dispatcher_->Dispatch(key_event.native_event())
+               : POST_DISPATCH_PERFORM_DEFAULT;
   }
 
-  return nested_dispatcher_->Dispatch(event);
+  return nested_dispatcher_ ? nested_dispatcher_->Dispatch(event)
+                            : POST_DISPATCH_PERFORM_DEFAULT;
 }
 
 }  // namespace ash
diff --git a/ash/accelerators/accelerator_dispatcher.h b/ash/accelerators/accelerator_dispatcher.h
index a7ae40a..a46368c 100644
--- a/ash/accelerators/accelerator_dispatcher.h
+++ b/ash/accelerators/accelerator_dispatcher.h
@@ -15,8 +15,8 @@ namespace ash {
 // Dispatcher for handling accelerators from menu.
 //
 // Wraps a nested dispatcher to which control is passed if no accelerator key
-// has been pressed.
-// TODO(pkotwicz): Port AcceleratorDispatcher to mac.
+// has been pressed. If the nested dispatcher is NULL, then the control is
+// passed back to the default dispatcher.
 // TODO(pkotwicz): Add support for a |nested_dispatcher| which sends
 //  events to a system IME.
 class ASH_EXPORT AcceleratorDispatcher : public base::MessagePumpDispatcher,
diff --git a/ash/accelerators/nested_dispatcher_controller.cc b/ash/accelerators/nested_dispatcher_controller.cc
index c551e94..6977f07 100644
--- a/ash/accelerators/nested_dispatcher_controller.cc
+++ b/ash/accelerators/nested_dispatcher_controller.cc
@@ -6,6 +6,7 @@
 
 #include "ash/accelerators/accelerator_dispatcher.h"
 #include "ash/shell.h"
+#include "base/auto_reset.h"
 #include "base/run_loop.h"
 
 namespace ash {
@@ -27,7 +28,14 @@ void NestedDispatcherController::RunWithDispatcher(
   // TODO(jbates) crbug.com/134753 Find quitters of this RunLoop and have them
   //              use run_loop.QuitClosure().
   base::RunLoop run_loop(&dispatcher);
+  base::AutoReset<base::Closure> reset_closure(&quit_closure_,
+                                               run_loop.QuitClosure());
   run_loop.Run();
 }
 
+void NestedDispatcherController::QuitNestedMessageLoop() {
+  CHECK(!quit_closure_.is_null());
+  quit_closure_.Run();
+}
+
 }  // namespace ash
diff --git a/ash/accelerators/nested_dispatcher_controller.h b/ash/accelerators/nested_dispatcher_controller.h
index 958cbae..1f8b52e 100644
--- a/ash/accelerators/nested_dispatcher_controller.h
+++ b/ash/accelerators/nested_dispatcher_controller.h
@@ -6,6 +6,7 @@
 #define ASH_ACCELERATORS_NESTED_DISPATCHER_CONTROLLER_H_
 
 #include "ash/ash_export.h"
+#include "base/callback.h"
 #include "base/message_loop/message_loop.h"
 #include "ui/aura/client/dispatcher_client.h"
 #include "ui/aura/window.h"
@@ -22,10 +23,14 @@ class ASH_EXPORT NestedDispatcherController
   NestedDispatcherController();
   virtual ~NestedDispatcherController();
 
+  // aura::client::DispatcherClient:
   virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher,
                                  aura::Window* associated_window) OVERRIDE;
+  virtual void QuitNestedMessageLoop() OVERRIDE;
 
  private:
+  base::Closure quit_closure_;
+
   DISALLOW_COPY_AND_ASSIGN(NestedDispatcherController);
 };
 
diff --git a/chrome/browser/ui/views/first_run_dialog.cc b/chrome/browser/ui/views/first_run_dialog.cc
index 2a236b5..dc84312 100644
--- a/chrome/browser/ui/views/first_run_dialog.cc
+++ b/chrome/browser/ui/views/first_run_dialog.cc
@@ -65,7 +65,7 @@ bool FirstRunDialog::Show(Profile* profile) {
     aura::Window* anchor = dialog->GetWidget()->GetNativeWindow();
     aura::client::DispatcherClient* client =
         aura::client::GetDispatcherClient(anchor->GetRootWindow());
-    client->RunWithDispatcher(dialog, anchor);
+    client->RunWithDispatcher(NULL, anchor);
     dialog_shown = true;
   }
 #endif  // defined(GOOGLE_CHROME_BUILD)
@@ -76,8 +76,7 @@ bool FirstRunDialog::Show(Profile* profile) {
 FirstRunDialog::FirstRunDialog(Profile* profile)
     : profile_(profile),
       make_default_(NULL),
-      report_crashes_(NULL),
-      should_show_dialog_(true) {
+      report_crashes_(NULL) {
   GridLayout* layout = GridLayout::CreatePanel(this);
   SetLayoutManager(layout);
 
@@ -102,6 +101,13 @@ FirstRunDialog::FirstRunDialog(Profile* profile)
 FirstRunDialog::~FirstRunDialog() {
 }
 
+void FirstRunDialog::Done() {
+  aura::Window* window = GetWidget()->GetNativeView();
+  aura::client::DispatcherClient* client =
+      aura::client::GetDispatcherClient(window->GetRootWindow());
+  client->QuitNestedMessageLoop();
+}
+
 views::View* FirstRunDialog::CreateExtraView() {
   views::Link* link = new views::Link(l10n_util::GetStringUTF16(
       IDS_LEARN_MORE));
@@ -110,12 +116,11 @@ views::View* FirstRunDialog::CreateExtraView() {
 }
 
 void FirstRunDialog::OnClosed() {
-  should_show_dialog_ = false;
   first_run::SetShouldShowWelcomePage();
+  Done();
 }
 
 bool FirstRunDialog::Accept() {
-  should_show_dialog_ = false;
   GetWidget()->Hide();
 
   if (report_crashes_ && report_crashes_->checked()) {
@@ -128,6 +133,7 @@ bool FirstRunDialog::Accept() {
   if (make_default_ && make_default_->checked())
     ShellIntegration::SetAsDefaultBrowser();
 
+  Done();
   return true;
 }
 
@@ -138,10 +144,3 @@ int FirstRunDialog::GetDialogButtons() const {
 void FirstRunDialog::LinkClicked(views::Link* source, int event_flags) {
   platform_util::OpenExternal(profile_, GURL(chrome::kLearnMoreReportingURL));
 }
-
-uint32_t FirstRunDialog::Dispatch(const base::NativeEvent& event) {
-  uint32_t action = POST_DISPATCH_PERFORM_DEFAULT;
-  if (!should_show_dialog_)
-    action |= POST_DISPATCH_QUIT_LOOP;
-  return action;
-}
diff --git a/chrome/browser/ui/views/first_run_dialog.h b/chrome/browser/ui/views/first_run_dialog.h
index 9e374a6..2eb9d5d 100644
--- a/chrome/browser/ui/views/first_run_dialog.h
+++ b/chrome/browser/ui/views/first_run_dialog.h
@@ -5,7 +5,6 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_FIRST_RUN_DIALOG_H_
 #define CHROME_BROWSER_UI_VIEWS_FIRST_RUN_DIALOG_H_
 
-#include "base/message_loop/message_pump_dispatcher.h"
 #include "ui/views/controls/link_listener.h"
 #include "ui/views/window/dialog_delegate.h"
 
@@ -17,8 +16,7 @@ class Link;
 }
 
 class FirstRunDialog : public views::DialogDelegateView,
-                       public views::LinkListener,
-                       public base::MessagePumpDispatcher {
+                       public views::LinkListener {
  public:
   // Displays the first run UI for reporting opt-in, import data etc.
   // Returns true if the dialog was shown.
@@ -28,6 +26,9 @@ class FirstRunDialog : public views::DialogDelegateView,
   explicit FirstRunDialog(Profile* profile);
   virtual ~FirstRunDialog();
 
+  // This terminates the nested message-loop.
+  void Done();
+
   // views::DialogDelegate:
   virtual views::View* CreateExtraView() OVERRIDE;
   virtual void OnClosed() OVERRIDE;
@@ -37,16 +38,11 @@ class FirstRunDialog : public views::DialogDelegateView,
   // views::LinkListener:
   virtual void LinkClicked(views::Link* source, int event_flags) OVERRIDE;
 
-  // Overridden from MessagePumpDispatcher:
-  virtual uint32_t Dispatch(const base::NativeEvent& event) OVERRIDE;
-
   Profile* profile_;
   views::Checkbox* make_default_;
   views::Checkbox* report_crashes_;
 
-  // Set to false as soon as the user clicks a dialog button; this tells the
-  // dispatcher we're done.
-  bool should_show_dialog_;
+  DISALLOW_COPY_AND_ASSIGN(FirstRunDialog);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_FIRST_RUN_DIALOG_H_
diff --git a/ui/aura/client/dispatcher_client.h b/ui/aura/client/dispatcher_client.h
index ae8c84c..769c266 100644
--- a/ui/aura/client/dispatcher_client.h
+++ b/ui/aura/client/dispatcher_client.h
@@ -18,6 +18,8 @@ class AURA_EXPORT DispatcherClient {
  public:
   virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher,
                                  aura::Window* associated_window) = 0;
+
+  virtual void QuitNestedMessageLoop() = 0;
 };
 
 AURA_EXPORT void SetDispatcherClient(Window* root_window,
diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
index 6b5fc4e..0cd2c72 100644
--- a/ui/views/controls/menu/menu_controller.cc
+++ b/ui/views/controls/menu/menu_controller.cc
@@ -13,6 +13,7 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "ui/aura/client/dispatcher_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/base/dragdrop/drag_utils.h"
@@ -2299,18 +2300,23 @@ void MenuController::SetExitType(ExitType type) {
   // Exit nested message loops as soon as possible. We do this as
   // MessagePumpDispatcher is only invoked before native events, which means
   // its entirely possible for a Widget::CloseNow() task to be processed before
-  // the next native message. By using QuitNow() we ensures the nested message
-  // loop returns as soon as possible and avoids having deleted views classes
-  // (such as widgets and rootviews) on the stack when the nested message loop
-  // stops.
+  // the next native message. We quite the nested message loop as soon as
+  // possible to avoid having deleted views classes (such as widgets and
+  // rootviews) on the stack when the nested message loop stops.
   //
-  // It's safe to invoke QuitNow multiple times, it only effects the current
-  // loop.
+  // It's safe to invoke QuitNestedMessageLoop() multiple times, it only effects
+  // the current loop.
   bool quit_now = ShouldQuitNow() && exit_type_ != EXIT_NONE &&
       message_loop_depth_;
 
-  if (quit_now)
-    base::MessageLoop::current()->QuitNow();
+  if (quit_now) {
+    if (owner_) {
+      aura::Window* root = owner_->GetNativeWindow()->GetRootWindow();
+      aura::client::GetDispatcherClient(root)->QuitNestedMessageLoop();
+    } else {
+      base::MessageLoop::current()->QuitNow();
+    }
+  }
 }
 
 void MenuController::HandleMouseLocation(SubmenuView* source,
diff --git a/ui/views/widget/desktop_aura/desktop_dispatcher_client.cc b/ui/views/widget/desktop_aura/desktop_dispatcher_client.cc
index be251da..977d0de 100644
--- a/ui/views/widget/desktop_aura/desktop_dispatcher_client.cc
+++ b/ui/views/widget/desktop_aura/desktop_dispatcher_client.cc
@@ -4,6 +4,7 @@
 
 #include "ui/views/widget/desktop_aura/desktop_dispatcher_client.h"
 
+#include "base/auto_reset.h"
 #include "base/run_loop.h"
 
 namespace views {
@@ -23,7 +24,14 @@ void DesktopDispatcherClient::RunWithDispatcher(
   base::MessageLoopForUI::ScopedNestableTaskAllower allow_nested(loop);
 
   base::RunLoop run_loop(nested_dispatcher);
+  base::AutoReset<base::Closure> reset_closure(&quit_closure_,
+                                               run_loop.QuitClosure());
   run_loop.Run();
 }
 
+void DesktopDispatcherClient::QuitNestedMessageLoop() {
+  CHECK(!quit_closure_.is_null());
+  quit_closure_.Run();
+}
+
 }  // namespace views
diff --git a/ui/views/widget/desktop_aura/desktop_dispatcher_client.h b/ui/views/widget/desktop_aura/desktop_dispatcher_client.h
index b59f4da..1af8f94 100644
--- a/ui/views/widget/desktop_aura/desktop_dispatcher_client.h
+++ b/ui/views/widget/desktop_aura/desktop_dispatcher_client.h
@@ -6,6 +6,7 @@
 #define UI_VIEWS_WIDGET_DESKTOP_AURA_DESKTOP_DISPATCHER_CLIENT_H_
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "ui/aura/client/dispatcher_client.h"
 #include "ui/views/views_export.h"
 
@@ -20,8 +21,11 @@ class VIEWS_EXPORT DesktopDispatcherClient
 
   virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher,
                                  aura::Window* associated_window) OVERRIDE;
+  virtual void QuitNestedMessageLoop() OVERRIDE;
 
  private:
+  base::Closure quit_closure_;
+
   DISALLOW_COPY_AND_ASSIGN(DesktopDispatcherClient);
 };