wm: Change the DispatcherClient interface.

ui/wm/core/nested_accelerator_controller.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 
 namespace wm {
 
 NestedAcceleratorController::NestedAcceleratorController(
     NestedAcceleratorDelegate* delegate)
     : dispatcher_delegate_(delegate) {
   DCHECK(delegate);
 }
 
 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();
 }
 
 }  // namespace wm