wm: Change the DispatcherClient interface.

ui/wm/core/nested_accelerator_controller.cc

Index: ui/wm/core/nested_accelerator_controller.cc
diff --git a/ui/wm/core/nested_accelerator_controller.cc b/ui/wm/core/nested_accelerator_controller.cc
index 782ac6d891cedc35fa0b7d6ff9c1ebb825f9d17d..e4ace4f777ad42177cdd89605133622ec5e91f98 100644
--- a/ui/wm/core/nested_accelerator_controller.cc
+++ b/ui/wm/core/nested_accelerator_controller.cc
@@ -5,6 +5,7 @@
 #include "ui/wm/core/nested_accelerator_controller.h"
 
 #include "base/auto_reset.h"
+#include "base/bind.h"
 #include "base/run_loop.h"
 #include "ui/wm/core/nested_accelerator_delegate.h"
 #include "ui/wm/core/nested_accelerator_dispatcher.h"
@@ -20,28 +21,36 @@ NestedAcceleratorController::NestedAcceleratorController(
 NestedAcceleratorController::~NestedAcceleratorController() {
 }
 
-void NestedAcceleratorController::RunWithDispatcher(
-    base::MessagePumpDispatcher* nested_dispatcher) {
-  base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
-  base::MessageLoopForUI::ScopedNestableTaskAllower allow_nested(loop);
-
+void NestedAcceleratorController::PrepareNestedLoopClosures(
+    base::MessagePumpDispatcher* nested_dispatcher,
+    base::Closure* run_closure,
+    base::Closure* quit_closure) {
   scoped_ptr<NestedAcceleratorDispatcher> old_accelerator_dispatcher =
       accelerator_dispatcher_.Pass();
   accelerator_dispatcher_ = NestedAcceleratorDispatcher::Create(
       dispatcher_delegate_.get(), nested_dispatcher);
 
-  // TODO(jbates) crbug.com/134753 Find quitters of this RunLoop and have them
-  //              use run_loop.QuitClosure().
   scoped_ptr<base::RunLoop> run_loop = accelerator_dispatcher_->CreateRunLoop();
-  base::AutoReset<base::Closure> reset_closure(&quit_closure_,
-                                               run_loop->QuitClosure());
+  *quit_closure =
+      base::Bind(&NestedAcceleratorController::QuitNestedMessageLoop,
+                 base::Unretained(this),
+                 run_loop->QuitClosure());

[sky, 2014/05/28 23:11:12]

This is more of what I was thinking, but having to express an API in terms of
closures feels awkward. Is there a reason you don't want real Run/Quit methods
DispatcherClient that DispatcherRunLoop calls?

[sadrul, 2014/05/29 00:25:39]

I think that makes it possible for the wrong dispatcher RunLoop to be terminated
if Quit() is called for the same RunLoop multiple times. It also makes it
difficult to get to the QuitClosure before already calling Run(). The
alternative is to not have QuitClosure, and pass around a pointer to the
dispatcher RunLoop instead. But the closure thing feels better I think.

+  *run_closure = base::Bind(&NestedAcceleratorController::RunNestedMessageLoop,
+                            base::Unretained(this),
+                            base::Passed(&run_loop),
+                            base::Passed(&old_accelerator_dispatcher));
+}
+
+void NestedAcceleratorController::RunNestedMessageLoop(
+    scoped_ptr<base::RunLoop> run_loop,
+    scoped_ptr<NestedAcceleratorDispatcher> old_accelerator_dispatcher) {
   run_loop->Run();
   accelerator_dispatcher_ = old_accelerator_dispatcher.Pass();
 }
 
-void NestedAcceleratorController::QuitNestedMessageLoop() {
-  CHECK(!quit_closure_.is_null());
-  quit_closure_.Run();
+void NestedAcceleratorController::QuitNestedMessageLoop(
+    const base::Closure& quit_runloop) {
+  quit_runloop.Run();
   accelerator_dispatcher_.reset();
 }