diff options
author | jbates@chromium.org <jbates@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-28 22:57:30 +0000 |
---|---|---|
committer | jbates@chromium.org <jbates@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-28 22:57:30 +0000 |
commit | 8e937c1e6a1cf0bdce081324965e105a6b17a3fc (patch) | |
tree | ffec2c670d3ceb188c0c1244b846b15adbe7838e | |
parent | ed50d3ee0ed2e26da0ff805dc52ee0c03f80df2e (diff) | |
download | chromium_src-8e937c1e6a1cf0bdce081324965e105a6b17a3fc.zip chromium_src-8e937c1e6a1cf0bdce081324965e105a6b17a3fc.tar.gz chromium_src-8e937c1e6a1cf0bdce081324965e105a6b17a3fc.tar.bz2 |
Add base::RunLoop and update ui_test_utils to use it to reduce flakiness
Timeout flakiness has been observed in multiple tests that use Quit. This changes various test utility APIs to use QuitNow via base::RunLoop instead. Some instances of Quit are left as-is where it appears they may have a use case.
The ui_test_utils QuitThisRunLoop function does a safer form of MessageLoop::QuitWhenIdle that allows a few generations of tasks to run before actually quitting the MessageLoop. This addresses the design assumptions of many existing tests while hopefully reducing flaky timeouts by moving away from QuitWhenIdle.
This fixes throughput_tests.cc which is currently timing out on Mac.
BUG=124906,130141,131220,128305,132932
Review URL: https://chromiumcodereview.appspot.com/10479018
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@144824 0039d316-1c4b-4281-b951-d872f2087c98
48 files changed, 994 insertions, 389 deletions
diff --git a/ash/accelerators/nested_dispatcher_controller.cc b/ash/accelerators/nested_dispatcher_controller.cc index 5daeca2..45ad0e4 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/run_loop.h" namespace ash { @@ -25,7 +26,10 @@ void NestedDispatcherController::RunWithDispatcher( AcceleratorDispatcher dispatcher(nested_dispatcher, associated_window); - loop->RunWithDispatcher(&dispatcher); + // TODO(jbates) crbug.com/134753 Find quitters of this RunLoop and have them + // use run_loop.QuitClosure(). + base::RunLoop run_loop(&dispatcher); + run_loop.Run(); loop->SetNestableTasksAllowed(did_allow_task_nesting); } diff --git a/ash/drag_drop/drag_drop_controller.cc b/ash/drag_drop/drag_drop_controller.cc index 51ca88d..dcc1029 100644 --- a/ash/drag_drop/drag_drop_controller.cc +++ b/ash/drag_drop/drag_drop_controller.cc @@ -7,6 +7,7 @@ #include "ash/drag_drop/drag_image_view.h" #include "ash/shell.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "ui/aura/client/capture_client.h" #include "ui/aura/client/drag_drop_delegate.h" #include "ui/aura/cursor_manager.h" @@ -82,9 +83,11 @@ int DragDropController::StartDragAndDrop(const ui::OSExchangeData& data, #if !defined(OS_MACOSX) if (should_block_during_drag_drop_) { + base::RunLoop run_loop(aura::Env::GetInstance()->GetDispatcher()); + quit_closure_ = run_loop.QuitClosure(); MessageLoopForUI* loop = MessageLoopForUI::current(); MessageLoop::ScopedNestableTaskAllower allow_nested(loop); - loop->RunWithDispatcher(aura::Env::GetInstance()->GetDispatcher()); + run_loop.Run(); } #endif // !defined(OS_MACOSX) @@ -163,7 +166,7 @@ void DragDropController::Drop(aura::Window* target, Cleanup(); if (should_block_during_drag_drop_) - MessageLoop::current()->QuitNow(); + quit_closure_.Run(); } void DragDropController::DragCancel() { @@ -181,7 +184,7 @@ void DragDropController::DragCancel() { drag_operation_ = 0; StartCanceledAnimation(); if (should_block_during_drag_drop_) - MessageLoop::current()->QuitNow(); + quit_closure_.Run(); } bool DragDropController::IsDragDropInProgress() { diff --git a/ash/drag_drop/drag_drop_controller.h b/ash/drag_drop/drag_drop_controller.h index d44ba20..df13d15 100644 --- a/ash/drag_drop/drag_drop_controller.h +++ b/ash/drag_drop/drag_drop_controller.h @@ -7,6 +7,7 @@ #pragma once #include "ash/ash_export.h" +#include "base/callback.h" #include "ui/aura/client/drag_drop_client.h" #include "ui/aura/event.h" #include "ui/aura/event_filter.h" @@ -99,6 +100,9 @@ class ASH_EXPORT DragDropController // Only be used for tests. bool should_block_during_drag_drop_; + // Closure for quitting nested message loop. + base::Closure quit_closure_; + DISALLOW_COPY_AND_ASSIGN(DragDropController); }; diff --git a/ash/test/ash_test_base.cc b/ash/test/ash_test_base.cc index 45996b2..07f93d0 100644 --- a/ash/test/ash_test_base.cc +++ b/ash/test/ash_test_base.cc @@ -9,6 +9,7 @@ #include "ash/shell.h" #include "ash/test/test_shell_delegate.h" +#include "base/run_loop.h" #include "base/string_split.h" #include "content/public/test/web_contents_tester.h" #include "ui/aura/env.h" @@ -90,8 +91,9 @@ void AshTestBase::UpdateDisplay(const std::string& display_specs) { void AshTestBase::RunAllPendingInMessageLoop() { #if !defined(OS_MACOSX) - message_loop_.RunAllPendingWithDispatcher( - aura::Env::GetInstance()->GetDispatcher()); + DCHECK(MessageLoopForUI::current() == &message_loop_); + base::RunLoop run_loop(aura::Env::GetInstance()->GetDispatcher()); + run_loop.RunUntilIdle(); #endif } diff --git a/ash/wm/toplevel_window_event_filter.cc b/ash/wm/toplevel_window_event_filter.cc index b53f786..43abc80 100644 --- a/ash/wm/toplevel_window_event_filter.cc +++ b/ash/wm/toplevel_window_event_filter.cc @@ -12,6 +12,7 @@ #include "ash/wm/window_util.h" #include "ash/wm/workspace/snap_sizer.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "ui/aura/client/aura_constants.h" #include "ui/aura/env.h" #include "ui/aura/event.h" @@ -98,7 +99,7 @@ bool ToplevelWindowEventFilter::PreHandleMouseEvent(aura::Window* target, DRAG_COMPLETE : DRAG_REVERT, event->flags()); if (in_move_loop_) { - MessageLoop::current()->Quit(); + quit_closure_.Run(); in_move_loop_ = false; } // Completing the drag may result in hiding the window. If this happens @@ -154,7 +155,7 @@ ui::GestureStatus ToplevelWindowEventFilter::PreHandleGestureEvent( return ui::GESTURE_STATUS_UNKNOWN; CompleteDrag(DRAG_COMPLETE, event->flags()); if (in_move_loop_) { - MessageLoop::current()->Quit(); + quit_closure_.Run(); in_move_loop_ = false; } in_gesture_resize_ = false; @@ -220,7 +221,9 @@ void ToplevelWindowEventFilter::RunMoveLoop(aura::Window* source) { #if !defined(OS_MACOSX) MessageLoopForUI* loop = MessageLoopForUI::current(); MessageLoop::ScopedNestableTaskAllower allow_nested(loop); - loop->RunWithDispatcher(aura::Env::GetInstance()->GetDispatcher()); + base::RunLoop run_loop(aura::Env::GetInstance()->GetDispatcher()); + quit_closure_ = run_loop.QuitClosure(); + run_loop.Run(); #endif // !defined(OS_MACOSX) in_gesture_resize_ = in_move_loop_ = false; } @@ -234,7 +237,7 @@ void ToplevelWindowEventFilter::EndMoveLoop() { window_resizer_->RevertDrag(); window_resizer_.reset(); } - MessageLoopForUI::current()->Quit(); + quit_closure_.Run(); } // static diff --git a/ash/wm/toplevel_window_event_filter.h b/ash/wm/toplevel_window_event_filter.h index 7f0915e..377ae38 100644 --- a/ash/wm/toplevel_window_event_filter.h +++ b/ash/wm/toplevel_window_event_filter.h @@ -8,6 +8,7 @@ #include <set> +#include "base/callback.h" #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "ui/aura/client/window_move_client.h" @@ -92,6 +93,8 @@ class ASH_EXPORT ToplevelWindowEventFilter : scoped_ptr<WindowResizer> window_resizer_; + base::Closure quit_closure_; + DISALLOW_COPY_AND_ASSIGN(ToplevelWindowEventFilter); }; diff --git a/base/base.gypi b/base/base.gypi index a45cea6..653f9b3 100644 --- a/base/base.gypi +++ b/base/base.gypi @@ -290,6 +290,8 @@ 'rand_util.h', 'rand_util_posix.cc', 'rand_util_win.cc', + 'run_loop.cc', + 'run_loop.h', 'safe_strerror_posix.cc', 'safe_strerror_posix.h', 'scoped_native_library.cc', diff --git a/base/message_loop.cc b/base/message_loop.cc index b040ecc..8eb4e3b 100644 --- a/base/message_loop.cc +++ b/base/message_loop.cc @@ -16,6 +16,7 @@ #include "base/message_loop_proxy_impl.h" #include "base/message_pump_default.h" #include "base/metrics/histogram.h" +#include "base/run_loop.h" #include "base/third_party/dynamic_annotations/dynamic_annotations.h" #include "base/thread_task_runner_handle.h" #include "base/threading/thread_local.h" @@ -131,7 +132,7 @@ MessageLoop::MessageLoop(Type type) nestable_tasks_allowed_(true), exception_restoration_(false), message_histogram_(NULL), - state_(NULL), + run_loop_(NULL), #ifdef OS_WIN os_modal_loop_(false), #endif // OS_WIN @@ -180,7 +181,7 @@ MessageLoop::MessageLoop(Type type) MessageLoop::~MessageLoop() { DCHECK_EQ(this, current()); - DCHECK(!state_); + DCHECK(!run_loop_); // Clean up any unprocessed tasks, but take care: deleting a task could // result in the addition of more tasks (e.g., via DeleteSoon). We set a @@ -293,20 +294,19 @@ void MessageLoop::PostNonNestableDelayedTask( } void MessageLoop::Run() { - AutoRunState save_state(this); - RunHandler(); + base::RunLoop run_loop; + run_loop.Run(); } -void MessageLoop::RunAllPending() { - AutoRunState save_state(this); - state_->quit_received = true; // Means run until we would otherwise block. - RunHandler(); +void MessageLoop::RunUntilIdle() { + base::RunLoop run_loop; + run_loop.RunUntilIdle(); } -void MessageLoop::Quit() { +void MessageLoop::QuitWhenIdle() { DCHECK_EQ(this, current()); - if (state_) { - state_->quit_received = true; + if (run_loop_) { + run_loop_->quit_when_idle_received_ = true; } else { NOTREACHED() << "Must be inside Run to call Quit"; } @@ -314,20 +314,20 @@ void MessageLoop::Quit() { void MessageLoop::QuitNow() { DCHECK_EQ(this, current()); - if (state_) { + if (run_loop_) { pump_->Quit(); } else { NOTREACHED() << "Must be inside Run to call Quit"; } } -static void QuitCurrent() { - MessageLoop::current()->Quit(); +static void QuitCurrentWhenIdle() { + MessageLoop::current()->QuitWhenIdle(); } // static -base::Closure MessageLoop::QuitClosure() { - return base::Bind(&QuitCurrent); +base::Closure MessageLoop::QuitWhenIdleClosure() { + return base::Bind(&QuitCurrentWhenIdle); } void MessageLoop::SetNestableTasksAllowed(bool allowed) { @@ -345,7 +345,7 @@ bool MessageLoop::NestableTasksAllowed() const { } bool MessageLoop::IsNested() { - return state_->run_depth > 1; + return run_loop_->run_depth_ > 1; } void MessageLoop::AddTaskObserver(TaskObserver* task_observer) { @@ -366,7 +366,7 @@ void MessageLoop::AssertIdle() const { bool MessageLoop::is_running() const { DCHECK_EQ(this, current()); - return state_ != NULL; + return run_loop_ != NULL; } //------------------------------------------------------------------------------ @@ -404,9 +404,9 @@ void MessageLoop::RunInternal() { StartHistogrammer(); #if !defined(OS_MACOSX) && !defined(OS_ANDROID) - if (state_->dispatcher && type() == TYPE_UI) { + if (run_loop_->dispatcher_ && type() == TYPE_UI) { static_cast<base::MessagePumpForUI*>(pump_.get())-> - RunWithDispatcher(this, state_->dispatcher); + RunWithDispatcher(this, run_loop_->dispatcher_); return; } #endif @@ -415,7 +415,7 @@ void MessageLoop::RunInternal() { } bool MessageLoop::ProcessNextDelayedNonNestableTask() { - if (state_->run_depth != 1) + if (run_loop_->run_depth_ != 1) return false; if (deferred_non_nestable_work_queue_.empty()) @@ -463,7 +463,7 @@ void MessageLoop::RunTask(const PendingTask& pending_task) { } bool MessageLoop::DeferOrRunPendingTask(const PendingTask& pending_task) { - if (pending_task.nestable || state_->run_depth == 1) { + if (pending_task.nestable || run_loop_->run_depth_ == 1) { RunTask(pending_task); // Show that we ran a task (Note: a new one might arrive as a // consequence!). @@ -685,7 +685,7 @@ bool MessageLoop::DoIdleWork() { if (ProcessNextDelayedNonNestableTask()) return true; - if (state_->quit_received) + if (run_loop_->quit_when_idle_received_) pump_->Quit(); return false; @@ -705,30 +705,6 @@ void MessageLoop::ReleaseSoonInternal( } //------------------------------------------------------------------------------ -// MessageLoop::AutoRunState - -MessageLoop::AutoRunState::AutoRunState(MessageLoop* loop) : loop_(loop) { - // Make the loop reference us. - previous_state_ = loop_->state_; - if (previous_state_) { - run_depth = previous_state_->run_depth + 1; - } else { - run_depth = 1; - } - loop_->state_ = this; - - // Initialize the other fields: - quit_received = false; -#if !defined(OS_MACOSX) && !defined(OS_ANDROID) - dispatcher = NULL; -#endif -} - -MessageLoop::AutoRunState::~AutoRunState() { - loop_->state_ = previous_state_; -} - -//------------------------------------------------------------------------------ // MessageLoopForUI #if defined(OS_WIN) @@ -753,19 +729,6 @@ void MessageLoopForUI::RemoveObserver(Observer* observer) { pump_ui()->RemoveObserver(observer); } -void MessageLoopForUI::RunWithDispatcher(Dispatcher* dispatcher) { - AutoRunState save_state(this); - state_->dispatcher = dispatcher; - RunHandler(); -} - -void MessageLoopForUI::RunAllPendingWithDispatcher(Dispatcher* dispatcher) { - AutoRunState save_state(this); - state_->dispatcher = dispatcher; - state_->quit_received = true; // Means run until we would otherwise block. - RunHandler(); -} - #endif // !defined(OS_MACOSX) && !defined(OS_NACL) && !defined(OS_ANDROID) //------------------------------------------------------------------------------ diff --git a/base/message_loop.h b/base/message_loop.h index a92ff7e..227bfe3 100644 --- a/base/message_loop.h +++ b/base/message_loop.h @@ -42,7 +42,11 @@ namespace base { class Histogram; +class RunLoop; class ThreadTaskRunnerHandle; +#if defined(OS_ANDROID) +class MessagePumpForUI; +#endif } // namespace base // A MessageLoop is used to process events for a particular thread. There is @@ -209,30 +213,51 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate { this, from_here, object); } + // Deprecated: use RunLoop instead. // Run the message loop. void Run(); + // Deprecated: use RunLoop instead. // Process all pending tasks, windows messages, etc., but don't wait/sleep. // Return as soon as all items that can be run are taken care of. - void RunAllPending(); + void RunUntilIdle(); + + // TODO(jbates) remove this. crbug.com/131220. See RunUntilIdle(). + void RunAllPending() { RunUntilIdle(); } + + // TODO(jbates) remove this. crbug.com/131220. See QuitWhenIdle(). + void Quit() { QuitWhenIdle(); } - // Signals the Run method to return after it is done processing all pending - // messages. This method may only be called on the same thread that called - // Run, and Run must still be on the call stack. + // Deprecated: use RunLoop instead. // - // Use QuitClosure if you need to Quit another thread's MessageLoop, but note - // that doing so is fairly dangerous if the target thread makes nested calls - // to MessageLoop::Run. The problem being that you won't know which nested - // run loop you are quitting, so be careful! - void Quit(); + // Signals the Run method to return when it becomes idle. It will continue to + // process pending messages and future messages as long as they are enqueued. + // Warning: if the MessageLoop remains busy, it may never quit. Only use this + // Quit method when looping procedures (such as web pages) have been shut + // down. + // + // This method may only be called on the same thread that called Run, and Run + // must still be on the call stack. + // + // Use QuitClosure variants if you need to Quit another thread's MessageLoop, + // but note that doing so is fairly dangerous if the target thread makes + // nested calls to MessageLoop::Run. The problem being that you won't know + // which nested run loop you are quitting, so be careful! + void QuitWhenIdle(); + // Deprecated: use RunLoop instead. + // // This method is a variant of Quit, that does not wait for pending messages // to be processed before returning from Run. void QuitNow(); - // Invokes Quit on the current MessageLoop when run. Useful to schedule an - // arbitrary MessageLoop to Quit. - static base::Closure QuitClosure(); + // TODO(jbates) remove this. crbug.com/131220. See QuitWhenIdleClosure(). + static base::Closure QuitClosure() { return QuitWhenIdleClosure(); } + + // Deprecated: use RunLoop instead. + // Construct a Closure that will call QuitWhenIdle(). Useful to schedule an + // arbitrary MessageLoop to QuitWhenIdle. + static base::Closure QuitWhenIdleClosure(); // Returns the type passed to the constructor. Type type() const { return type_; } @@ -354,35 +379,7 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate { //---------------------------------------------------------------------------- protected: - struct RunState { - // Used to count how many Run() invocations are on the stack. - int run_depth; - - // Used to record that Quit() was called, or that we should quit the pump - // once it becomes idle. - bool quit_received; - -#if !defined(OS_MACOSX) && !defined(OS_ANDROID) - Dispatcher* dispatcher; -#endif - }; - -#if defined(OS_ANDROID) - // Android Java process manages the UI thread message loop. So its - // MessagePumpForUI needs to keep the RunState. - public: -#endif - class BASE_EXPORT AutoRunState : RunState { - public: - explicit AutoRunState(MessageLoop* loop); - ~AutoRunState(); - private: - MessageLoop* loop_; - RunState* previous_state_; - }; -#if defined(OS_ANDROID) - protected: -#endif + friend class base::RunLoop; #if defined(OS_WIN) base::MessagePumpWin* pump_win() { @@ -496,7 +493,7 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate { // Protect access to incoming_queue_. mutable base::Lock incoming_queue_lock_; - RunState* state_; + base::RunLoop* run_loop_; #if defined(OS_WIN) base::TimeTicks high_resolution_timer_expiration_; @@ -525,7 +522,6 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate { void(*releaser)(const void*), const void* object); - DISALLOW_COPY_AND_ASSIGN(MessageLoop); }; @@ -563,8 +559,6 @@ class BASE_EXPORT MessageLoopForUI : public MessageLoop { // methods. void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); - void RunWithDispatcher(Dispatcher* dispatcher); - void RunAllPendingWithDispatcher(Dispatcher* dispatcher); protected: // TODO(rvargas): Make this platform independent. diff --git a/base/message_loop_unittest.cc b/base/message_loop_unittest.cc index 7167c5e..8ac9c64 100644 --- a/base/message_loop_unittest.cc +++ b/base/message_loop_unittest.cc @@ -11,6 +11,7 @@ #include "base/logging.h" #include "base/memory/ref_counted.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "base/thread_task_runner_handle.h" #include "base/threading/platform_thread.h" #include "base/threading/thread.h" @@ -599,9 +600,10 @@ enum TaskType { RECURSIVE, TIMEDMESSAGELOOP, QUITMESSAGELOOP, - ORDERERD, + ORDERED, PUMPS, SLEEP, + RUNS, }; // Saves the order in which the tasks executed. @@ -628,7 +630,7 @@ std::ostream& operator <<(std::ostream& os, TaskType type) { case RECURSIVE: os << "RECURSIVE"; break; case TIMEDMESSAGELOOP: os << "TIMEDMESSAGELOOP"; break; case QUITMESSAGELOOP: os << "QUITMESSAGELOOP"; break; - case ORDERERD: os << "ORDERERD"; break; + case ORDERED: os << "ORDERED"; break; case PUMPS: os << "PUMPS"; break; case SLEEP: os << "SLEEP"; break; default: @@ -674,8 +676,8 @@ class TaskList { // Saves the order the tasks ran. void OrderedFunc(TaskList* order, int cookie) { - order->RecordStart(ORDERERD, cookie); - order->RecordEnd(ORDERERD, cookie); + order->RecordStart(ORDERED, cookie); + order->RecordEnd(ORDERED, cookie); } #if defined(OS_WIN) @@ -847,8 +849,8 @@ void RunTest_RecursiveDenial3(MessageLoop::Type message_loop_type) { EXPECT_EQ(order.Get(3), TaskItem(RECURSIVE, 2, false)); EXPECT_EQ(order.Get(4), TaskItem(RECURSIVE, 1, true)); EXPECT_EQ(order.Get(5), TaskItem(RECURSIVE, 1, false)); - EXPECT_EQ(order.Get(6), TaskItem(ORDERERD, 3, true)); - EXPECT_EQ(order.Get(7), TaskItem(ORDERERD, 3, false)); + EXPECT_EQ(order.Get(6), TaskItem(ORDERED, 3, true)); + EXPECT_EQ(order.Get(7), TaskItem(ORDERED, 3, false)); EXPECT_EQ(order.Get(8), TaskItem(RECURSIVE, 2, true)); EXPECT_EQ(order.Get(9), TaskItem(RECURSIVE, 2, false)); EXPECT_EQ(order.Get(10), TaskItem(QUITMESSAGELOOP, 4, true)); @@ -997,6 +999,19 @@ void FuncThatPumps(TaskList* order, int cookie) { order->RecordEnd(PUMPS, cookie); } +void FuncThatRuns(TaskList* order, int cookie, base::RunLoop* run_loop) { + order->RecordStart(RUNS, cookie); + { + MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current()); + run_loop->Run(); + } + order->RecordEnd(RUNS, cookie); +} + +void FuncThatQuitsNow() { + MessageLoop::current()->QuitNow(); +} + // Tests that non nestable tasks run in FIFO if there are no nested loops. void RunTest_NonNestableWithNoNesting( MessageLoop::Type message_loop_type) { @@ -1015,10 +1030,10 @@ void RunTest_NonNestableWithNoNesting( // FIFO order. ASSERT_EQ(6U, order.Size()); - EXPECT_EQ(order.Get(0), TaskItem(ORDERERD, 1, true)); - EXPECT_EQ(order.Get(1), TaskItem(ORDERERD, 1, false)); - EXPECT_EQ(order.Get(2), TaskItem(ORDERERD, 2, true)); - EXPECT_EQ(order.Get(3), TaskItem(ORDERERD, 2, false)); + EXPECT_EQ(order.Get(0), TaskItem(ORDERED, 1, true)); + EXPECT_EQ(order.Get(1), TaskItem(ORDERED, 1, false)); + EXPECT_EQ(order.Get(2), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(3), TaskItem(ORDERED, 2, false)); EXPECT_EQ(order.Get(4), TaskItem(QUITMESSAGELOOP, 3, true)); EXPECT_EQ(order.Get(5), TaskItem(QUITMESSAGELOOP, 3, false)); } @@ -1066,19 +1081,296 @@ void RunTest_NonNestableInNestedLoop(MessageLoop::Type message_loop_type, // FIFO order. ASSERT_EQ(12U, order.Size()); EXPECT_EQ(order.Get(0), TaskItem(PUMPS, 1, true)); - EXPECT_EQ(order.Get(1), TaskItem(ORDERERD, 3, true)); - EXPECT_EQ(order.Get(2), TaskItem(ORDERERD, 3, false)); + EXPECT_EQ(order.Get(1), TaskItem(ORDERED, 3, true)); + EXPECT_EQ(order.Get(2), TaskItem(ORDERED, 3, false)); EXPECT_EQ(order.Get(3), TaskItem(SLEEP, 4, true)); EXPECT_EQ(order.Get(4), TaskItem(SLEEP, 4, false)); - EXPECT_EQ(order.Get(5), TaskItem(ORDERERD, 5, true)); - EXPECT_EQ(order.Get(6), TaskItem(ORDERERD, 5, false)); + EXPECT_EQ(order.Get(5), TaskItem(ORDERED, 5, true)); + EXPECT_EQ(order.Get(6), TaskItem(ORDERED, 5, false)); EXPECT_EQ(order.Get(7), TaskItem(PUMPS, 1, false)); - EXPECT_EQ(order.Get(8), TaskItem(ORDERERD, 2, true)); - EXPECT_EQ(order.Get(9), TaskItem(ORDERERD, 2, false)); + EXPECT_EQ(order.Get(8), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(9), TaskItem(ORDERED, 2, false)); EXPECT_EQ(order.Get(10), TaskItem(QUITMESSAGELOOP, 6, true)); EXPECT_EQ(order.Get(11), TaskItem(QUITMESSAGELOOP, 6, false)); } +// Tests RunLoopQuit only quits the corresponding MessageLoop::Run. +void RunTest_QuitNow(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop run_loop; + + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 1, base::Unretained(&run_loop))); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 2)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&FuncThatQuitsNow)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 3)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&FuncThatQuitsNow)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 4)); // never runs + + MessageLoop::current()->Run(); + + ASSERT_EQ(6U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 3, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 3, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + +// Tests RunLoopQuit works before RunWithID. +void RunTest_RunLoopQuitOrderBefore(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop run_loop; + + run_loop.Quit(); + + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 1)); // never runs + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&FuncThatQuitsNow)); // never runs + + run_loop.Run(); + + ASSERT_EQ(0U, order.Size()); +} + +// Tests RunLoopQuit works during RunWithID. +void RunTest_RunLoopQuitOrderDuring(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop run_loop; + + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 1)); + MessageLoop::current()->PostTask( + FROM_HERE, run_loop.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 2)); // never runs + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&FuncThatQuitsNow)); // never runs + + run_loop.Run(); + + ASSERT_EQ(2U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 1, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + +// Tests RunLoopQuit works after RunWithID. +void RunTest_RunLoopQuitOrderAfter(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop run_loop; + + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 1, base::Unretained(&run_loop))); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 2)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&FuncThatQuitsNow)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 3)); + MessageLoop::current()->PostTask( + FROM_HERE, run_loop.QuitClosure()); // has no affect + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 4)); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&FuncThatQuitsNow)); + + base::RunLoop outer_run_loop; + outer_run_loop.Run(); + + ASSERT_EQ(8U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 3, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 3, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 4, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 4, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + +// Tests RunLoopQuit only quits the corresponding MessageLoop::Run. +void RunTest_RunLoopQuitTop(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop outer_run_loop; + base::RunLoop nested_run_loop; + + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 1, base::Unretained(&nested_run_loop))); + MessageLoop::current()->PostTask( + FROM_HERE, outer_run_loop.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 2)); + MessageLoop::current()->PostTask( + FROM_HERE, nested_run_loop.QuitClosure()); + + outer_run_loop.Run(); + + ASSERT_EQ(4U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + +// Tests RunLoopQuit only quits the corresponding MessageLoop::Run. +void RunTest_RunLoopQuitNested(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop outer_run_loop; + base::RunLoop nested_run_loop; + + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 1, base::Unretained(&nested_run_loop))); + MessageLoop::current()->PostTask( + FROM_HERE, nested_run_loop.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 2)); + MessageLoop::current()->PostTask( + FROM_HERE, outer_run_loop.QuitClosure()); + + outer_run_loop.Run(); + + ASSERT_EQ(4U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + +// Tests RunLoopQuit only quits the corresponding MessageLoop::Run. +void RunTest_RunLoopQuitBogus(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop outer_run_loop; + base::RunLoop nested_run_loop; + base::RunLoop bogus_run_loop; + + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 1, base::Unretained(&nested_run_loop))); + MessageLoop::current()->PostTask( + FROM_HERE, bogus_run_loop.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 2)); + MessageLoop::current()->PostTask( + FROM_HERE, outer_run_loop.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, nested_run_loop.QuitClosure()); + + outer_run_loop.Run(); + + ASSERT_EQ(4U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 2, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + +// Tests RunLoopQuit only quits the corresponding MessageLoop::Run. +void RunTest_RunLoopQuitDeep(MessageLoop::Type message_loop_type) { + MessageLoop loop(message_loop_type); + + TaskList order; + + base::RunLoop outer_run_loop; + base::RunLoop nested_loop1; + base::RunLoop nested_loop2; + base::RunLoop nested_loop3; + base::RunLoop nested_loop4; + + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 1, base::Unretained(&nested_loop1))); + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 2, base::Unretained(&nested_loop2))); + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 3, base::Unretained(&nested_loop3))); + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&FuncThatRuns, &order, 4, base::Unretained(&nested_loop4))); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 5)); + MessageLoop::current()->PostTask( + FROM_HERE, outer_run_loop.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 6)); + MessageLoop::current()->PostTask( + FROM_HERE, nested_loop1.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 7)); + MessageLoop::current()->PostTask( + FROM_HERE, nested_loop2.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 8)); + MessageLoop::current()->PostTask( + FROM_HERE, nested_loop3.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 9)); + MessageLoop::current()->PostTask( + FROM_HERE, nested_loop4.QuitClosure()); + MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&OrderedFunc, &order, 10)); + + outer_run_loop.Run(); + + ASSERT_EQ(18U, order.Size()); + int task_index = 0; + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 2, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 3, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 4, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 5, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 5, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 6, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 6, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 7, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 7, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 8, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 8, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 9, true)); + EXPECT_EQ(order.Get(task_index++), TaskItem(ORDERED, 9, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 4, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 3, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 2, false)); + EXPECT_EQ(order.Get(task_index++), TaskItem(RUNS, 1, false)); + EXPECT_EQ(static_cast<size_t>(task_index), order.Size()); +} + #if defined(OS_WIN) class DispatcherImpl : public MessageLoopForUI::Dispatcher { @@ -1112,7 +1404,8 @@ void RunTest_Dispatcher(MessageLoop::Type message_loop_type) { base::Bind(&MouseDownUp), TimeDelta::FromMilliseconds(100)); DispatcherImpl dispatcher; - MessageLoopForUI::current()->RunWithDispatcher(&dispatcher); + base::RunLoop run_loop(&dispatcher); + run_loop.Run(); ASSERT_EQ(2, dispatcher.dispatch_count_); } @@ -1137,7 +1430,8 @@ void RunTest_DispatcherWithMessageHook(MessageLoop::Type message_loop_type) { NULL, GetCurrentThreadId()); DispatcherImpl dispatcher; - MessageLoopForUI::current()->RunWithDispatcher(&dispatcher); + base::RunLoop run_loop(&dispatcher); + run_loop.Run(); ASSERT_EQ(1, dispatcher.dispatch_count_); UnhookWindowsHookEx(msg_hook); } @@ -1434,6 +1728,54 @@ TEST(MessageLoopTest, NonNestableDelayedInNestedLoop) { RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_IO, true); } +TEST(MessageLoopTest, QuitNow) { + RunTest_QuitNow(MessageLoop::TYPE_DEFAULT); + RunTest_QuitNow(MessageLoop::TYPE_UI); + RunTest_QuitNow(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitTop) { + RunTest_RunLoopQuitTop(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitTop(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitTop(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitNested) { + RunTest_RunLoopQuitNested(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitNested(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitNested(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitBogus) { + RunTest_RunLoopQuitBogus(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitBogus(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitBogus(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitDeep) { + RunTest_RunLoopQuitDeep(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitDeep(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitDeep(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitOrderBefore) { + RunTest_RunLoopQuitOrderBefore(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitOrderBefore(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitOrderBefore(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitOrderDuring) { + RunTest_RunLoopQuitOrderDuring(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitOrderDuring(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitOrderDuring(MessageLoop::TYPE_IO); +} + +TEST(MessageLoopTest, RunLoopQuitOrderAfter) { + RunTest_RunLoopQuitOrderAfter(MessageLoop::TYPE_DEFAULT); + RunTest_RunLoopQuitOrderAfter(MessageLoop::TYPE_UI); + RunTest_RunLoopQuitOrderAfter(MessageLoop::TYPE_IO); +} + void PostNTasksThenQuit(int posts_remaining) { if (posts_remaining > 1) { MessageLoop::current()->PostTask( diff --git a/base/message_pump_android.cc b/base/message_pump_android.cc index 2daf98d..25f7fb7 100644 --- a/base/message_pump_android.cc +++ b/base/message_pump_android.cc @@ -10,6 +10,7 @@ #include "base/android/scoped_java_ref.h" #include "base/lazy_instance.h" #include "base/logging.h" +#include "base/run_loop.h" #include "jni/system_message_handler_jni.h" using base::android::ScopedJavaLocalRef; @@ -61,7 +62,7 @@ static jboolean DoRunLoopOnce(JNIEnv* env, jobject obj, jint native_delegate) { namespace base { MessagePumpForUI::MessagePumpForUI() - : state_(NULL) { + : run_loop_(NULL) { } MessagePumpForUI::~MessagePumpForUI() { @@ -73,7 +74,11 @@ void MessagePumpForUI::Run(Delegate* delegate) { } void MessagePumpForUI::Start(Delegate* delegate) { - state_ = new MessageLoop::AutoRunState(MessageLoop::current()); + run_loop_ = new base::RunLoop(); + // Since the RunLoop was just created above, BeforeRun should be guaranteed to + // return true (it only returns false if the RunLoop has been Quit already). + if (!run_loop_->BeforeRun()) + NOTREACHED(); DCHECK(g_system_message_handler_obj.Get().is_null()); @@ -94,9 +99,10 @@ void MessagePumpForUI::Quit() { g_system_message_handler_obj.Get().Reset(); } - if (state_) { - delete state_; - state_ = NULL; + if (run_loop_) { + run_loop_->AfterRun(); + delete run_loop_; + run_loop_ = NULL; } } diff --git a/base/message_pump_android.h b/base/message_pump_android.h index 7310db6..e10a770 100644 --- a/base/message_pump_android.h +++ b/base/message_pump_android.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -7,11 +7,11 @@ #pragma once #include "base/compiler_specific.h" -#include "base/message_loop.h" #include "base/message_pump.h" #include "base/time.h" namespace base { +class RunLoop; // This class implements a MessagePump needed for TYPE_UI MessageLoops on // OS_ANDROID platform. @@ -28,7 +28,7 @@ class MessagePumpForUI : public MessagePump { virtual void Start(Delegate* delegate); private: - MessageLoop::AutoRunState* state_; + base::RunLoop* run_loop_; DISALLOW_COPY_AND_ASSIGN(MessagePumpForUI); }; diff --git a/base/message_pump_dispatcher.h b/base/message_pump_dispatcher.h index 685e92b..b8efd32 100644 --- a/base/message_pump_dispatcher.h +++ b/base/message_pump_dispatcher.h @@ -11,12 +11,11 @@ namespace base { -// Dispatcher is used during a nested invocation of Run to dispatch -// events when |MessageLoop::RunWithDispatcher| is invoked. If -// |MessageLoop::Run| is invoked, MessageLoop does not dispatch events -// (or invoke TranslateMessage), rather every message is passed to -// Dispatcher's Dispatch method for dispatch. It is up to the -// Dispatcher whether or not to dispatch the event. +// Dispatcher is used during a nested invocation of Run to dispatch events when +// |RunLoop(dispatcher).Run()| is used. If |RunLoop().Run()| is invoked, +// MessageLoop does not dispatch events (or invoke TranslateMessage), rather +// every message is passed to Dispatcher's Dispatch method for dispatch. It is +// up to the Dispatcher whether or not to dispatch the event. // // The nested loop is exited by either posting a quit, or returning false // from Dispatch. diff --git a/base/run_loop.cc b/base/run_loop.cc new file mode 100644 index 0000000..991571e --- /dev/null +++ b/base/run_loop.cc @@ -0,0 +1,94 @@ +// Copyright (c) 2012 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 "base/run_loop.h" + +#include "base/bind.h" + +namespace base { + +RunLoop::RunLoop() + : loop_(MessageLoop::current()), + weak_factory_(this), + previous_run_loop_(NULL), + run_depth_(0), + run_called_(false), + quit_called_(false), + running_(false), + quit_when_idle_received_(false) { +#if !defined(OS_MACOSX) && !defined(OS_ANDROID) + dispatcher_ = NULL; +#endif +} + +#if !defined(OS_MACOSX) && !defined(OS_ANDROID) +RunLoop::RunLoop(MessageLoop::Dispatcher* dispatcher) + : loop_(MessageLoop::current()), + weak_factory_(this), + previous_run_loop_(NULL), + dispatcher_(dispatcher), + run_depth_(0), + run_called_(false), + quit_called_(false), + running_(false), + quit_when_idle_received_(false) { +} +#endif + +RunLoop::~RunLoop() { +} + +void RunLoop::Run() { + if (!BeforeRun()) + return; + loop_->RunHandler(); + AfterRun(); +} + +void RunLoop::RunUntilIdle() { + quit_when_idle_received_ = true; + Run(); +} + +void RunLoop::Quit() { + quit_called_ = true; + if (running_ && loop_->run_loop_ == this) { + // This is the inner-most RunLoop, so quit now. + loop_->QuitNow(); + } +} + +base::Closure RunLoop::QuitClosure() { + return base::Bind(&RunLoop::Quit, weak_factory_.GetWeakPtr()); +} + +bool RunLoop::BeforeRun() { + DCHECK(!run_called_); + run_called_ = true; + + // Allow Quit to be called before Run. + if (quit_called_) + return false; + + // Push RunLoop stack: + previous_run_loop_ = loop_->run_loop_; + run_depth_ = previous_run_loop_? previous_run_loop_->run_depth_ + 1 : 1; + loop_->run_loop_ = this; + + running_ = true; + return true; +} + +void RunLoop::AfterRun() { + running_ = false; + + // Pop RunLoop stack: + loop_->run_loop_ = previous_run_loop_; + + // Execute deferred QuitNow, if any: + if (previous_run_loop_ && previous_run_loop_->quit_called_) + loop_->QuitNow(); +} + +} // namespace base diff --git a/base/run_loop.h b/base/run_loop.h new file mode 100644 index 0000000..6fc0ed2 --- /dev/null +++ b/base/run_loop.h @@ -0,0 +1,112 @@ +// Copyright (c) 2012 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. + +#ifndef BASE_RUN_LOOP_H_ +#define BASE_RUN_LOOP_H_ +#pragma once + +#include "base/base_export.h" +#include "base/callback.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop.h" + +namespace base { +#if defined(OS_ANDROID) +class MessagePumpForUI; +#endif + +// Helper class to Run a nested MessageLoop. Please do not use nested +// MessageLoops in production code! If you must, use this class instead of +// calling MessageLoop::Run/Quit directly. RunLoop::Run can only be called once +// per RunLoop lifetime. Create a RunLoop on the stack and call Run/Quit to run +// a nested MessageLoop. +class BASE_EXPORT RunLoop { + public: + RunLoop(); +#if !defined(OS_MACOSX) && !defined(OS_ANDROID) + explicit RunLoop(MessageLoop::Dispatcher* dispatcher); +#endif + ~RunLoop(); + +#if !defined(OS_MACOSX) && !defined(OS_ANDROID) + void set_dispatcher(MessageLoop::Dispatcher* dispatcher) { + dispatcher_ = dispatcher; + } +#endif + + // Run the current MessageLoop. This blocks until Quit is called. Before + // calling Run, be sure to grab an AsWeakPtr or the QuitClosure in order to + // stop the MessageLoop asynchronously. MessageLoop::Quit and QuitNow will + // also trigger a return from Run, but those are deprecated. + void Run(); + + // Run the current MessageLoop until it doesn't find any tasks or messages in + // the queue (it goes idle). WARNING: This may never return! Only use this + // when repeating tasks such as animated web pages have been shut down. + void RunUntilIdle(); + + bool running() const { return running_; } + + // Quit an earlier call to Run(). There can be other nested RunLoops servicing + // the same task queue (MessageLoop); Quitting one RunLoop has no bearing on + // the others. Quit can be called before, during or after Run. If called + // before Run, Run will return immediately when called. Calling Quit after the + // RunLoop has already finished running has no effect. + // + // WARNING: You must NEVER assume that a call to Quit will terminate the + // targetted message loop. If a nested message loop continues running, the + // target may NEVER terminate. It is very easy to livelock (run forever) in + // such a case. + void Quit(); + + // Convenience method to get a closure that safely calls Quit (has no effect + // if the RunLoop instance is gone). + // + // Example: + // RunLoop run_loop; + // PostTask(run_loop.QuitClosure()); + // run_loop.Run(); + base::Closure QuitClosure(); + + private: + friend class ::MessageLoop; +#if defined(OS_ANDROID) + // Android doesn't support the blocking MessageLoop::Run, so it calls + // BeforeRun and AfterRun directly. + friend class base::MessagePumpForUI; +#endif + + // Return false to abort the Run. + bool BeforeRun(); + void AfterRun(); + + MessageLoop* loop_; + + // WeakPtrFactory for QuitClosure safety. + base::WeakPtrFactory<RunLoop> weak_factory_; + + // Parent RunLoop or NULL if this is the top-most RunLoop. + RunLoop* previous_run_loop_; + +#if !defined(OS_MACOSX) && !defined(OS_ANDROID) + MessageLoop::Dispatcher* dispatcher_; +#endif + + // Used to count how many nested Run() invocations are on the stack. + int run_depth_; + + bool run_called_; + bool quit_called_; + bool running_; + + // Used to record that QuitWhenIdle() was called on the MessageLoop, meaning + // that we should quit Run once it becomes idle. + bool quit_when_idle_received_; + + DISALLOW_COPY_AND_ASSIGN(RunLoop); +}; + +} // namespace base + +#endif // BASE_RUN_LOOP_H_ diff --git a/base/test/test_support_android.cc b/base/test/test_support_android.cc index 259c59f..82271d6 100644 --- a/base/test/test_support_android.cc +++ b/base/test/test_support_android.cc @@ -8,6 +8,7 @@ #include "base/file_path.h" #include "base/logging.h" #include "base/memory/singleton.h" +#include "base/message_loop.h" #include "base/message_pump_android.h" #include "base/path_service.h" #include "base/synchronization/waitable_event.h" diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index abe66f2..06e1fde 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -4,6 +4,7 @@ #include "chrome/browser/autocomplete/search_provider.h" +#include "base/run_loop.h" #include "base/string_util.h" #include "base/time.h" #include "base/utf_string_conversions.h" @@ -178,13 +179,12 @@ void SearchProviderTest::RunTillProviderDone() { return; quit_when_done_ = true; -#if defined(OS_MACOSX) - message_loop_.Run(); -#elif defined(OS_ANDROID) +#if defined(OS_ANDROID) // Android doesn't have Run(), only Start(). message_loop_.Start(); #else - message_loop_.RunWithDispatcher(NULL); + base::RunLoop run_loop; + run_loop.Run(); #endif } diff --git a/chrome/browser/automation/automation_provider_win.cc b/chrome/browser/automation/automation_provider_win.cc index 13885c6..d156b5e 100644 --- a/chrome/browser/automation/automation_provider_win.cc +++ b/chrome/browser/automation/automation_provider_win.cc @@ -8,6 +8,7 @@ #include "base/callback.h" #include "base/debug/trace_event.h" #include "base/json/json_reader.h" +#include "base/run_loop.h" #include "base/utf_string_conversions.h" #include "chrome/browser/automation/automation_browser_tracker.h" #include "chrome/browser/automation/automation_tab_tracker.h" @@ -36,6 +37,17 @@ using content::WebContents; namespace { +// Allow some pending tasks up to |num_deferrals| generations to complete. +void DeferredQuitRunLoop(const base::Closure& quit_task, + int num_quit_deferrals) { + if (num_quit_deferrals <= 0) { + quit_task.Run(); + } else { + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&DeferredQuitRunLoop, quit_task, num_quit_deferrals - 1)); + } +} + // This callback just adds another callback to the event queue. This is useful // if you want to ensure that any callbacks added to the event queue after this // one have already been processed by the time |callback| is run. @@ -139,6 +151,11 @@ void AutomationProvider::WindowSimulateDrag( if (press_escape_en_route) { // Press Escape, making sure we wait until chrome processes the escape. // TODO(phajdan.jr): make this use ui_test_utils::SendKeyPressSync. + views::AcceleratorHandler handler; + base::RunLoop run_loop(&handler); + // Number of times to repost Quit task to allow pending tasks to complete. + // See kNumQuitDeferrals in ui_test_utils.cc for explanation. + int num_quit_deferrals = 10; ui_controls::SendKeyPressNotifyWhenDone( window, ui::VKEY_ESCAPE, ((flags & ui::EF_CONTROL_DOWN) == @@ -147,11 +164,11 @@ void AutomationProvider::WindowSimulateDrag( ui::EF_SHIFT_DOWN), ((flags & ui::EF_ALT_DOWN) == ui::EF_ALT_DOWN), false, - MessageLoop::QuitClosure()); + base::Bind(&DeferredQuitRunLoop, run_loop.QuitClosure(), + num_quit_deferrals)); MessageLoopForUI* loop = MessageLoopForUI::current(); - views::AcceleratorHandler handler; MessageLoop::ScopedNestableTaskAllower allow(loop); - loop->RunWithDispatcher(&handler); + run_loop.Run(); } SendMessage(top_level_hwnd, up_message, wparam_flags, MAKELPARAM(end.x, end.y)); diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index bf106a4..54f308b 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -19,6 +19,7 @@ #include "base/path_service.h" #include "base/process_info.h" #include "base/process_util.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "base/string_piece.h" #include "base/string_split.h" @@ -1900,16 +1901,14 @@ bool ChromeBrowserMainParts::MainMessageLoopRun(int* result_code) { // UI thread message loop as possible to get a stable measurement // across versions. RecordBrowserStartupTime(); -#if defined(USE_AURA) - MessageLoopForUI::current()->Run(); -#elif defined(TOOLKIT_VIEWS) + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); +#if !defined(USE_AURA) && defined(TOOLKIT_VIEWS) views::AcceleratorHandler accelerator_handler; - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); -#elif defined(USE_X11) - MessageLoopForUI::current()->RunWithDispatcher(NULL); -#elif defined(OS_POSIX) - MessageLoopForUI::current()->Run(); + base::RunLoop run_loop(&accelerator_handler); +#else + base::RunLoop run_loop; #endif + run_loop.Run(); return true; } diff --git a/chrome/browser/referrer_policy_browsertest.cc b/chrome/browser/referrer_policy_browsertest.cc index 3e6fc20..7d59dbd 100644 --- a/chrome/browser/referrer_policy_browsertest.cc +++ b/chrome/browser/referrer_policy_browsertest.cc @@ -255,8 +255,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, HttpsMiddleClickTargetBlankOrigin) { // Context menu, from HTTP to HTTP. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuOrigin) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", false, false, false, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); @@ -265,8 +264,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuOrigin) { // Context menu, from HTTPS to HTTP. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_HttpsContextMenuOrigin) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", true, false, false, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); @@ -352,8 +350,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, // Context menu, from HTTP to HTTP via server redirect. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuRedirect) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", false, false, true, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); @@ -362,8 +359,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuRedirect) { // Context menu, from HTTPS to HTTP via server redirect. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_HttpsContextMenuRedirect) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", true, false, true, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc b/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc index e95b26c..362d57f 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc +++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc @@ -101,8 +101,7 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, MAYBE_RealMenu) { ContextMenuNotificationObserver menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); ui_test_utils::WindowedTabAddedNotificationObserver tab_observer( content::NotificationService::AllSources()); diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc index 900581b..b86fbe4 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc +++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc @@ -11,20 +11,11 @@ #include "content/public/browser/notification_service.h" ContextMenuNotificationObserver::ContextMenuNotificationObserver( - int command_to_execute, - int expected_notification) - : command_to_execute_(command_to_execute), - expected_notification_(expected_notification), - seen_expected_notification_(false) { + int command_to_execute) + : command_to_execute_(command_to_execute) { registrar_.Add(this, chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN, content::NotificationService::AllSources()); - registrar_.Add(this, - chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_CLOSED, - content::NotificationService::AllSources()); - registrar_.Add(this, - expected_notification_, - content::NotificationService::AllSources()); } ContextMenuNotificationObserver::~ContextMenuNotificationObserver() { @@ -34,11 +25,6 @@ void ContextMenuNotificationObserver::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { - if (type == expected_notification_) { - seen_expected_notification_ = true; - return; - } - switch (type) { case chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN: { RenderViewContextMenu* context_menu = @@ -49,17 +35,6 @@ void ContextMenuNotificationObserver::Observe( base::Unretained(this), context_menu)); break; } - case chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_CLOSED: { - // Aura is running a nested message loop for menus. That means that - // whatever the test running us is waiting for (e.g. a new tab to be - // added) happened while the menu message loop was running, so the - // message loop run by this test never quits. - if (seen_expected_notification_ && - MessageLoop::current()->is_running()) { - MessageLoop::current()->QuitNow(); - } - break; - } default: NOTREACHED(); diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h index 815454f..dac0c03 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h +++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h @@ -15,14 +15,7 @@ class RenderViewContextMenu; class ContextMenuNotificationObserver : public content::NotificationObserver { public: // Wait for a context menu to be shown, and then execute |command_to_execute|. - // As the context menu might spin a nested message loop, the usual way to wait - // for the result of the |command_to_execute|, i.e. running - // ui_test_utils::WindowedNotificationObserver::Wait, won't work. The - // ContextMenuNotificationObserver can work around this problem. In order to - // do so, you need to also specify what notification the - // WindowedNotificationObserver is waiting for. - ContextMenuNotificationObserver(int command_to_execute, - int expected_notification); + explicit ContextMenuNotificationObserver(int command_to_execute); virtual ~ContextMenuNotificationObserver(); private: @@ -34,8 +27,6 @@ class ContextMenuNotificationObserver : public content::NotificationObserver { content::NotificationRegistrar registrar_; int command_to_execute_; - int expected_notification_; - bool seen_expected_notification_; DISALLOW_COPY_AND_ASSIGN(ContextMenuNotificationObserver); }; diff --git a/chrome/browser/ui/views/simple_message_box_views.cc b/chrome/browser/ui/views/simple_message_box_views.cc index 805c6b5..a399885 100644 --- a/chrome/browser/ui/views/simple_message_box_views.cc +++ b/chrome/browser/ui/views/simple_message_box_views.cc @@ -8,6 +8,7 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "chrome/browser/browser_process.h" #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -179,7 +180,8 @@ MessageBoxResult ShowMessageBox(gfx::NativeWindow parent, #else { MessageLoop::ScopedNestableTaskAllower allow(MessageLoopForUI::current()); - MessageLoopForUI::current()->RunWithDispatcher(dialog); + base::RunLoop run_loop(dialog); + run_loop.Run(); } #endif return dialog->result(); diff --git a/chrome/browser/ui/views/uninstall_view.cc b/chrome/browser/ui/views/uninstall_view.cc index 808d245..41fee09 100644 --- a/chrome/browser/ui/views/uninstall_view.cc +++ b/chrome/browser/ui/views/uninstall_view.cc @@ -6,6 +6,7 @@ #include "base/message_loop.h" #include "base/process_util.h" +#include "base/run_loop.h" #include "base/string16.h" #include "base/utf_string_conversions.h" #include "chrome/browser/shell_integration.h" @@ -23,19 +24,21 @@ #include "ui/views/layout/layout_constants.h" #include "ui/views/widget/widget.h" -UninstallView::UninstallView(int* user_selection) +UninstallView::UninstallView(int* user_selection, + const base::Closure& quit_closure) : confirm_label_(NULL), delete_profile_(NULL), change_default_browser_(NULL), browsers_combo_(NULL), browsers_(NULL), - user_selection_(*user_selection) { + user_selection_(*user_selection), + quit_closure_(quit_closure) { SetupControls(); } UninstallView::~UninstallView() { // Exit the message loop we were started with so that uninstall can continue. - MessageLoop::current()->Quit(); + quit_closure_.Run(); } void UninstallView::SetupControls() { @@ -164,10 +167,13 @@ string16 UninstallView::GetItemAt(int index) { namespace browser { int ShowUninstallBrowserPrompt() { + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); int result = content::RESULT_CODE_NORMAL_EXIT; - views::Widget::CreateWindow(new UninstallView(&result))->Show(); views::AcceleratorHandler accelerator_handler; - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + base::RunLoop run_loop(&accelerator_handler); + UninstallView* view = new UninstallView(&result, run_loop.QuitClosure()); + views::Widget::CreateWindow(view)->Show(); + run_loop.Run(); return result; } diff --git a/chrome/browser/ui/views/uninstall_view.h b/chrome/browser/ui/views/uninstall_view.h index 181c07a..47c02f2 100644 --- a/chrome/browser/ui/views/uninstall_view.h +++ b/chrome/browser/ui/views/uninstall_view.h @@ -8,6 +8,7 @@ #include <map> +#include "base/callback.h" #include "base/string16.h" #include "ui/base/models/combobox_model.h" #include "ui/views/window/dialog_delegate.h" @@ -25,7 +26,8 @@ class UninstallView : public views::ButtonListener, public views::DialogDelegateView, public ui::ComboboxModel { public: - explicit UninstallView(int* user_selection); + explicit UninstallView(int* user_selection, + const base::Closure& quit_closure); virtual ~UninstallView(); // Overridden form views::ButtonListener. @@ -56,6 +58,7 @@ class UninstallView : public views::ButtonListener, typedef std::map<std::wstring, std::wstring> BrowsersMap; scoped_ptr<BrowsersMap> browsers_; int& user_selection_; + base::Closure quit_closure_; DISALLOW_COPY_AND_ASSIGN(UninstallView); }; diff --git a/chrome/browser/ui/views/user_data_dir_dialog_view.cc b/chrome/browser/ui/views/user_data_dir_dialog_view.cc index a0e7128..2a1f29c 100644 --- a/chrome/browser/ui/views/user_data_dir_dialog_view.cc +++ b/chrome/browser/ui/views/user_data_dir_dialog_view.cc @@ -5,6 +5,7 @@ #include "chrome/browser/ui/views/user_data_dir_dialog_view.h" #include "base/logging.h" +#include "base/run_loop.h" #include "base/utf_string_conversions.h" #include "chrome/browser/ui/user_data_dir_dialog.h" #include "grit/chromium_strings.h" @@ -100,10 +101,12 @@ void UserDataDirDialogView::FileSelectionCanceled(void* params) { namespace browser { FilePath ShowUserDataDirDialog(const FilePath& user_data_dir) { + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); // When the window closes, it will delete itself. UserDataDirDialogView* dialog = new UserDataDirDialogView(user_data_dir); views::Widget::CreateWindow(dialog)->Show(); - MessageLoopForUI::current()->RunWithDispatcher(dialog); + base::RunLoop run_loop(dialog); + run_loop.Run(); return dialog->user_data_dir(); } diff --git a/chrome/test/base/bookmark_load_observer.cc b/chrome/test/base/bookmark_load_observer.cc index 5b80070..3bc1b7f 100644 --- a/chrome/test/base/bookmark_load_observer.cc +++ b/chrome/test/base/bookmark_load_observer.cc @@ -1,15 +1,16 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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 "chrome/test/base/bookmark_load_observer.h" -#include "base/message_loop.h" +#include "chrome/test/base/ui_test_utils.h" -BookmarkLoadObserver::BookmarkLoadObserver() {} +BookmarkLoadObserver::BookmarkLoadObserver(const base::Closure& quit_task) + : quit_task_(quit_task) {} BookmarkLoadObserver::~BookmarkLoadObserver() {} void BookmarkLoadObserver::Loaded(BookmarkModel* model, bool ids_reassigned) { - MessageLoop::current()->Quit(); + quit_task_.Run(); } diff --git a/chrome/test/base/bookmark_load_observer.h b/chrome/test/base/bookmark_load_observer.h index 0619838e..bb21615 100644 --- a/chrome/test/base/bookmark_load_observer.h +++ b/chrome/test/base/bookmark_load_observer.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -7,7 +7,9 @@ #pragma once #include "base/basictypes.h" +#include "base/callback.h" #include "base/compiler_specific.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" // BookmarkLoadObserver is used when blocking until the BookmarkModel @@ -15,7 +17,7 @@ // loop is quit. class BookmarkLoadObserver : public BookmarkModelObserver { public: - BookmarkLoadObserver(); + explicit BookmarkLoadObserver(const base::Closure& quit_task); virtual ~BookmarkLoadObserver(); virtual void Loaded(BookmarkModel* model, bool ids_reassigned) OVERRIDE; @@ -40,6 +42,7 @@ class BookmarkLoadObserver : public BookmarkModelObserver { const BookmarkNode* node) OVERRIDE {} private: + base::Closure quit_task_; DISALLOW_COPY_AND_ASSIGN(BookmarkLoadObserver); }; diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc index 34a9c70..30a2eca 100644 --- a/chrome/test/base/in_process_browser_test.cc +++ b/chrome/test/base/in_process_browser_test.cc @@ -374,6 +374,12 @@ void InProcessBrowserTest::RunTestOnMainThreadLoop() { autorelease_pool_->Recycle(); #endif + // Sometimes tests leave Quit tasks in the MessageLoop (for shame), so let's + // run all pending messages here to avoid preempting the QuitBrowsers tasks. + // TODO(jbates) Once crbug.com/134753 is fixed, this can be removed because it + // will not be possible to post Quit tasks. + ui_test_utils::RunAllPendingInMessageLoop(); + QuitBrowsers(); CHECK(BrowserList::empty()); } diff --git a/chrome/test/base/test_web_dialog_observer.cc b/chrome/test/base/test_web_dialog_observer.cc index 2056592..75b7a34 100644 --- a/chrome/test/base/test_web_dialog_observer.cc +++ b/chrome/test/base/test_web_dialog_observer.cc @@ -42,7 +42,7 @@ void TestWebDialogObserver::Observe( // If the message loop is running stop it. if (running_) { running_ = false; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } break; default: @@ -73,7 +73,8 @@ content::WebUI* TestWebDialogObserver::GetWebUI() { if (!done_) { EXPECT_FALSE(running_); running_ = true; - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new ui_test_utils::MessageLoopRunner; + message_loop_runner_->Run(); } return web_ui_; } diff --git a/chrome/test/base/test_web_dialog_observer.h b/chrome/test/base/test_web_dialog_observer.h index b77d667..67f1f74 100644 --- a/chrome/test/base/test_web_dialog_observer.h +++ b/chrome/test/base/test_web_dialog_observer.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "chrome/test/base/ui_test_utils.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "ui/web_dialogs/web_dialog_observer.h" @@ -55,6 +56,7 @@ class TestWebDialogObserver : public content::NotificationObserver, content::WebUI* web_ui_; bool done_; bool running_; + scoped_refptr<ui_test_utils::MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(TestWebDialogObserver); }; diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc index a652086..c69447f 100644 --- a/chrome/test/base/testing_profile.cc +++ b/chrome/test/base/testing_profile.cc @@ -11,6 +11,7 @@ #include "base/file_util.h" #include "base/message_loop_proxy.h" #include "base/path_service.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "chrome/browser/autocomplete/autocomplete_classifier.h" #include "chrome/browser/bookmarks/bookmark_model.h" @@ -378,9 +379,11 @@ void TestingProfile::BlockUntilBookmarkModelLoaded() { DCHECK(GetBookmarkModel()); if (GetBookmarkModel()->IsLoaded()) return; - BookmarkLoadObserver observer; + base::RunLoop run_loop; + BookmarkLoadObserver observer( + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); GetBookmarkModel()->AddObserver(&observer); - MessageLoop::current()->Run(); + run_loop.Run(); GetBookmarkModel()->RemoveObserver(&observer); DCHECK(GetBookmarkModel()->IsLoaded()); } diff --git a/chrome/test/base/tracing.cc b/chrome/test/base/tracing.cc index 790ab3c..3c29bba 100644 --- a/chrome/test/base/tracing.cc +++ b/chrome/test/base/tracing.cc @@ -38,7 +38,8 @@ class InProcessTraceController : public content::TraceSubscriber { return false; // Wait for OnEndTracingComplete() to quit the message loop. // OnTraceDataCollected may be called multiple times while blocking here. - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new ui_test_utils::MessageLoopRunner; + message_loop_runner_->Run(); trace_buffer_.Finish(); trace_buffer_.SetOutputCallback(TraceResultBuffer::OutputCallback()); @@ -51,7 +52,7 @@ class InProcessTraceController : public content::TraceSubscriber { // TraceSubscriber virtual void OnEndTracingComplete() OVERRIDE { - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } // TraceSubscriber @@ -63,6 +64,8 @@ class InProcessTraceController : public content::TraceSubscriber { // For collecting trace data asynchronously. base::debug::TraceResultBuffer trace_buffer_; + scoped_refptr<ui_test_utils::MessageLoopRunner> message_loop_runner_; + DISALLOW_COPY_AND_ASSIGN(InProcessTraceController); }; diff --git a/chrome/test/base/ui_test_utils.cc b/chrome/test/base/ui_test_utils.cc index f3f51b7..7b4af42 100644 --- a/chrome/test/base/ui_test_utils.cc +++ b/chrome/test/base/ui_test_utils.cc @@ -93,6 +93,17 @@ using content::WebContents; static const int kDefaultWsPort = 8880; +// Number of times to repost a Quit task so that the MessageLoop finishes up +// pending tasks and tasks posted by those pending tasks without risking the +// potential hang behavior of MessageLoop::QuitWhenIdle. +// The criteria for choosing this number: it should be high enough to make the +// quit act like QuitWhenIdle, while taking into account that any page which is +// animating may be rendering another frame for each quit deferral. For an +// animating page, the potential delay to quitting the RunLoop would be +// kNumQuitDeferrals * frame_render_time. Some perf tests run slow, such as +// 200ms/frame. +static const int kNumQuitDeferrals = 10; + namespace ui_test_utils { namespace { @@ -106,7 +117,8 @@ class DOMOperationObserver : public content::NotificationObserver, did_respond_(false) { registrar_.Add(this, content::NOTIFICATION_DOM_OPERATION_RESPONSE, content::Source<RenderViewHost>(render_view_host)); - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); } virtual void Observe(int type, @@ -116,12 +128,12 @@ class DOMOperationObserver : public content::NotificationObserver, content::Details<DomOperationNotificationDetails> dom_op_details(details); response_ = dom_op_details->json; did_respond_ = true; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } // Overridden from content::WebContentsObserver: virtual void RenderViewGone(base::TerminationStatus status) OVERRIDE { - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } bool GetResponse(std::string* response) WARN_UNUSED_RESULT { @@ -133,6 +145,7 @@ class DOMOperationObserver : public content::NotificationObserver, content::NotificationRegistrar registrar_; std::string response_; bool did_respond_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(DOMOperationObserver); }; @@ -147,7 +160,8 @@ class FindInPageNotificationObserver : public content::NotificationObserver { parent_tab->find_tab_helper()->current_find_request_id(); registrar_.Add(this, chrome::NOTIFICATION_FIND_RESULT_AVAILABLE, content::Source<WebContents>(parent_tab_->web_contents())); - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); } int active_match_ordinal() const { return active_match_ordinal_; } @@ -165,7 +179,7 @@ class FindInPageNotificationObserver : public content::NotificationObserver { active_match_ordinal_ = find_details->active_match_ordinal(); if (find_details->final_update()) { number_of_matches_ = find_details->number_of_matches(); - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } else { DVLOG(1) << "Ignoring, since we only care about the final message"; } @@ -185,6 +199,7 @@ class FindInPageNotificationObserver : public content::NotificationObserver { // The id of the current find request, obtained from WebContents. Allows us // to monitor when the search completes. int current_find_request_id_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(FindInPageNotificationObserver); }; @@ -263,39 +278,52 @@ bool ExecuteJavaScriptHelper(RenderViewHost* render_view_host, return true; } -void RunAllPendingMessageAndSendQuit(content::BrowserThread::ID thread_id) { - MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); +void RunAllPendingMessageAndSendQuit(content::BrowserThread::ID thread_id, + const base::Closure& quit_task) { + MessageLoop::current()->PostTask(FROM_HERE, + MessageLoop::QuitWhenIdleClosure()); RunMessageLoop(); - content::BrowserThread::PostTask(thread_id, FROM_HERE, - MessageLoop::QuitClosure()); + content::BrowserThread::PostTask(thread_id, FROM_HERE, quit_task); } } // namespace void RunMessageLoop() { - MessageLoop* loop = MessageLoop::current(); - MessageLoopForUI* ui_loop = - content::BrowserThread::CurrentlyOn(content::BrowserThread::UI) ? - MessageLoopForUI::current() : NULL; - MessageLoop::ScopedNestableTaskAllower allow(loop); - if (ui_loop) { -#if defined(USE_AURA) - ui_loop->Run(); -#elif defined(TOOLKIT_VIEWS) - views::AcceleratorHandler handler; - ui_loop->RunWithDispatcher(&handler); -#elif defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID) - ui_loop->RunWithDispatcher(NULL); -#else - ui_loop->Run(); + base::RunLoop run_loop; + RunThisRunLoop(&run_loop); +} + +void RunThisRunLoop(base::RunLoop* run_loop) { + MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current()); +#if !defined(USE_AURA) && defined(TOOLKIT_VIEWS) + scoped_ptr<views::AcceleratorHandler> handler; + if (content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) { + handler.reset(new views::AcceleratorHandler); + run_loop->set_dispatcher(handler.get()); + } #endif + run_loop->Run(); +} + +// TODO(jbates) move this to a new test_utils.cc in content/test/ +static void DeferredQuitRunLoop(const base::Closure& quit_task, + int num_quit_deferrals) { + if (num_quit_deferrals <= 0) { + quit_task.Run(); } else { - loop->Run(); + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&DeferredQuitRunLoop, quit_task, num_quit_deferrals - 1)); } } +base::Closure GetQuitTaskForRunLoop(base::RunLoop* run_loop) { + return base::Bind(&DeferredQuitRunLoop, run_loop->QuitClosure(), + kNumQuitDeferrals); +} + void RunAllPendingInMessageLoop() { - MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); + MessageLoop::current()->PostTask(FROM_HERE, + MessageLoop::QuitWhenIdleClosure()); ui_test_utils::RunMessageLoop(); } @@ -309,10 +337,12 @@ void RunAllPendingInMessageLoop(content::BrowserThread::ID thread_id) { NOTREACHED(); return; } - content::BrowserThread::PostTask(thread_id, FROM_HERE, - base::Bind(&RunAllPendingMessageAndSendQuit, current_thread_id)); - ui_test_utils::RunMessageLoop(); + base::RunLoop run_loop; + content::BrowserThread::PostTask(thread_id, FROM_HERE, + base::Bind(&RunAllPendingMessageAndSendQuit, current_thread_id, + run_loop.QuitClosure())); + ui_test_utils::RunThisRunLoop(&run_loop); } bool GetCurrentTabTitle(const Browser* browser, string16* title) { @@ -331,10 +361,10 @@ void WaitForNavigations(NavigationController* controller, content::TestNavigationObserver observer( content::Source<NavigationController>(controller), NULL, number_of_navigations); + base::RunLoop run_loop; observer.WaitForObservation( - base::Bind(&ui_test_utils::RunMessageLoop), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoopForUI::current()))); + base::Bind(&ui_test_utils::RunThisRunLoop, base::Unretained(&run_loop)), + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); } void WaitForNewTab(Browser* browser) { @@ -377,11 +407,10 @@ void NavigateToURL(browser::NavigateParams* params) { content::TestNavigationObserver observer( content::NotificationService::AllSources(), NULL, 1); browser::Navigate(params); + base::RunLoop run_loop; observer.WaitForObservation( - base::Bind(&ui_test_utils::RunMessageLoop), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoopForUI::current()))); - + base::Bind(&ui_test_utils::RunThisRunLoop, base::Unretained(&run_loop)), + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); } void NavigateToURL(Browser* browser, const GURL& url) { @@ -451,10 +480,10 @@ static void NavigateToURLWithDispositionBlockUntilNavigationsComplete( web_contents = browser->GetActiveWebContents(); } if (disposition == CURRENT_TAB) { + base::RunLoop run_loop; same_tab_observer.WaitForObservation( - base::Bind(&ui_test_utils::RunMessageLoop), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoopForUI::current()))); + base::Bind(&ui_test_utils::RunThisRunLoop, base::Unretained(&run_loop)), + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); return; } else if (web_contents) { NavigationController* controller = &web_contents->GetController(); @@ -704,9 +733,10 @@ void RegisterAndWait(content::NotificationObserver* observer, void WaitForBookmarkModelToLoad(BookmarkModel* model) { if (model->IsLoaded()) return; - BookmarkLoadObserver observer; + base::RunLoop run_loop; + BookmarkLoadObserver observer(GetQuitTaskForRunLoop(&run_loop)); model->AddObserver(&observer); - RunMessageLoop(); + RunThisRunLoop(&run_loop); model->RemoveObserver(&observer); ASSERT_TRUE(model->IsLoaded()); } @@ -764,14 +794,14 @@ bool SendKeyPressSync(const Browser* browser, gfx::NativeWindow window = NULL; if (!GetNativeWindow(browser, &window)) return false; - + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; bool result; result = ui_controls::SendKeyPressNotifyWhenDone( - window, key, control, shift, alt, command, MessageLoop::QuitClosure()); + window, key, control, shift, alt, command, runner->QuitClosure()); #if defined(OS_WIN) if (!result && BringBrowserWindowToFront(browser)) { result = ui_controls::SendKeyPressNotifyWhenDone( - window, key, control, shift, alt, command, MessageLoop::QuitClosure()); + window, key, control, shift, alt, command, runner->QuitClosure()); } #endif if (!result) { @@ -782,7 +812,7 @@ bool SendKeyPressSync(const Browser* browser, // Run the message loop. It'll stop running when either the key was received // or the test timed out (in which case testing::Test::HasFatalFailure should // be set). - RunMessageLoop(); + runner->Run(); return !testing::Test::HasFatalFailure(); } @@ -804,51 +834,41 @@ bool SendKeyPressAndWait(const Browser* browser, } bool SendMouseMoveSync(const gfx::Point& location) { - if (!ui_controls::SendMouseMoveNotifyWhenDone(location.x(), location.y(), - MessageLoop::QuitClosure())) { + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; + if (!ui_controls::SendMouseMoveNotifyWhenDone( + location.x(), location.y(), runner->QuitClosure())) { return false; } - RunMessageLoop(); + runner->Run(); return !testing::Test::HasFatalFailure(); } bool SendMouseEventsSync(ui_controls::MouseButton type, int state) { + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; if (!ui_controls::SendMouseEventsNotifyWhenDone( - type, state, MessageLoop::QuitClosure())) { + type, state, runner->QuitClosure())) { return false; } - RunMessageLoop(); + runner->Run(); return !testing::Test::HasFatalFailure(); } -TimedMessageLoopRunner::TimedMessageLoopRunner() - : loop_(new MessageLoopForUI()), - owned_(true), - quit_loop_invoked_(false) { +MessageLoopRunner::MessageLoopRunner() { } -TimedMessageLoopRunner::~TimedMessageLoopRunner() { - if (owned_) - delete loop_; +MessageLoopRunner::~MessageLoopRunner() { } -void TimedMessageLoopRunner::RunFor(int ms) { - QuitAfter(ms); - quit_loop_invoked_ = false; - loop_->Run(); +void MessageLoopRunner::Run() { + ui_test_utils::RunThisRunLoop(&run_loop_); } -void TimedMessageLoopRunner::Quit() { - quit_loop_invoked_ = true; - loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure()); +base::Closure MessageLoopRunner::QuitClosure() { + return base::Bind(&MessageLoopRunner::Quit, this); } -void TimedMessageLoopRunner::QuitAfter(int ms) { - quit_loop_invoked_ = true; - loop_->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitClosure(), - base::TimeDelta::FromMilliseconds(ms)); +void MessageLoopRunner::Quit() { + ui_test_utils::GetQuitTaskForRunLoop(&run_loop_).Run(); } TestWebSocketServer::TestWebSocketServer() @@ -1013,7 +1033,8 @@ void WindowedNotificationObserver::Wait() { return; running_ = true; - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); EXPECT_TRUE(seen_); } @@ -1027,7 +1048,7 @@ void WindowedNotificationObserver::Observe( if (!running_) return; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); running_ = false; } @@ -1078,7 +1099,8 @@ const string16& TitleWatcher::WaitAndGetTitle() { if (expected_title_observed_) return observed_title_; quit_loop_on_observation_ = true; - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); return observed_title_; } @@ -1104,8 +1126,11 @@ void TitleWatcher::Observe(int type, return; observed_title_ = *it; expected_title_observed_ = true; - if (quit_loop_on_observation_) - MessageLoopForUI::current()->Quit(); + if (quit_loop_on_observation_) { + // Only call Quit once, on first Observe: + quit_loop_on_observation_ = false; + message_loop_runner_->Quit(); + } } BrowserAddedObserver::BrowserAddedObserver() @@ -1140,7 +1165,7 @@ void DOMMessageQueue::Observe(int type, message_queue_.push(dom_op_details->json); if (waiting_for_message_) { waiting_for_message_ = false; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } } @@ -1152,7 +1177,8 @@ bool DOMMessageQueue::WaitForMessage(std::string* message) { if (message_queue_.empty()) { waiting_for_message_ = true; // This will be quit when a new message comes in. - RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); } // The queue should not be empty, unless we were quit because of a timeout. if (message_queue_.empty()) @@ -1180,7 +1206,8 @@ class SnapshotTaker { base::Bind(&SnapshotTaker::OnSnapshotTaken, base::Unretained(this)), page_size, desired_size); - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); return snapshot_taken_; } @@ -1215,13 +1242,14 @@ class SnapshotTaker { void OnSnapshotTaken(const SkBitmap& bitmap) { *bitmap_ = bitmap; snapshot_taken_ = true; - MessageLoop::current()->Quit(); + message_loop_runner_->Quit(); } SkBitmap* bitmap_; // Whether the snapshot was actually taken and received by this SnapshotTaker. // This will be false if the test times out. bool snapshot_taken_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(SnapshotTaker); }; @@ -1247,9 +1275,9 @@ void OverrideGeolocation(double latitude, double longitude) { position.altitude = 0.; position.accuracy = 0.; position.timestamp = base::Time::Now(); - content::OverrideLocationForTesting(position, - base::Bind(MessageLoop::QuitClosure())); - RunMessageLoop(); + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; + content::OverrideLocationForTesting(position, runner->QuitClosure()); + runner->Run(); } namespace internal { diff --git a/chrome/test/base/ui_test_utils.h b/chrome/test/base/ui_test_utils.h index 9af9372..a2ec811 100644 --- a/chrome/test/base/ui_test_utils.h +++ b/chrome/test/base/ui_test_utils.h @@ -13,8 +13,8 @@ #include <vector> #include "base/basictypes.h" -#include "base/message_loop.h" #include "base/process.h" +#include "base/run_loop.h" #include "base/scoped_temp_dir.h" #include "base/string16.h" #include "chrome/browser/ui/view_ids.h" @@ -96,6 +96,13 @@ enum BrowserTestWaitFlags { // process browser tests that need to block until a condition is met. void RunMessageLoop(); +// Variant of RunMessageLoop that takes RunLoop. +void RunThisRunLoop(base::RunLoop* run_loop); + +// Get task to quit the given RunLoop. It allows a few generations of pending +// tasks to run as opposed to run_loop->QuitClosure(). +base::Closure GetQuitTaskForRunLoop(base::RunLoop* run_loop); + // Turns on nestable tasks, runs all pending tasks in the message loop, // then resets nestable tasks to what they were originally. Prefer this // over MessageLoop::RunAllPending for in process browser tests to run @@ -300,37 +307,34 @@ bool SendMouseMoveSync(const gfx::Point& location) WARN_UNUSED_RESULT; bool SendMouseEventsSync(ui_controls::MouseButton type, int state) WARN_UNUSED_RESULT; -// Run a message loop only for the specified amount of time. -class TimedMessageLoopRunner { +// Helper class to Run and Quit the message loop. Run and Quit can only happen +// once per instance. Make a new instance for each use. Calling Quit after Run +// has returned is safe and has no effect. +class MessageLoopRunner + : public base::RefCounted<MessageLoopRunner> { public: - // Create new MessageLoopForUI and attach to it. - TimedMessageLoopRunner(); - - // Attach to an existing message loop. - explicit TimedMessageLoopRunner(MessageLoop* loop) - : loop_(loop), owned_(false), quit_loop_invoked_(false) {} - - ~TimedMessageLoopRunner(); + MessageLoopRunner(); - // Run the message loop for ms milliseconds. - void RunFor(int ms); + // Run the current MessageLoop. + void Run(); - // Post Quit task to the message loop. + // Quit the matching call to Run (nested MessageLoops are unaffected). void Quit(); - // Post delayed Quit task to the message loop. - void QuitAfter(int ms); - - bool WasTimedOut() const { - return !quit_loop_invoked_; - } + // Hand this closure off to code that uses callbacks to notify completion. + // Example: + // scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; + // kick_off_some_api(runner.QuitNowClosure()); + // runner.Run(); + base::Closure QuitClosure(); private: - MessageLoop* loop_; - bool owned_; - bool quit_loop_invoked_; + friend class base::RefCounted<MessageLoopRunner>; + ~MessageLoopRunner(); + + base::RunLoop run_loop_; - DISALLOW_COPY_AND_ASSIGN(TimedMessageLoopRunner); + DISALLOW_COPY_AND_ASSIGN(MessageLoopRunner); }; // This is a utility class for running a python websocket server @@ -445,6 +449,7 @@ class WindowedNotificationObserver : public content::NotificationObserver { content::NotificationSource source_; content::NotificationDetails details_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(WindowedNotificationObserver); }; @@ -538,6 +543,7 @@ class TitleWatcher : public content::NotificationObserver { content::WebContents* web_contents_; std::vector<string16> expected_titles_; content::NotificationRegistrar notification_registrar_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; // The most recently observed expected title, if any. string16 observed_title_; @@ -637,6 +643,7 @@ class DOMMessageQueue : public content::NotificationObserver { content::NotificationRegistrar registrar_; std::queue<std::string> message_queue_; bool waiting_for_message_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(DOMMessageQueue); }; diff --git a/chrome/test/perf/rendering/throughput_tests.cc b/chrome/test/perf/rendering/throughput_tests.cc index 606fb94..18965ca 100644 --- a/chrome/test/perf/rendering/throughput_tests.cc +++ b/chrome/test/perf/rendering/throughput_tests.cc @@ -8,6 +8,7 @@ #include "base/json/json_reader.h" #include "base/memory/scoped_ptr.h" #include "base/path_service.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "base/stringprintf.h" #include "base/test/trace_event_analyzer.h" @@ -195,11 +196,11 @@ class ThroughputTest : public BrowserPerfTest { } void Wait(int ms) { + base::RunLoop run_loop; MessageLoop::current()->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitClosure(), + FROM_HERE, run_loop.QuitClosure(), base::TimeDelta::FromMilliseconds(ms)); - ui_test_utils::RunMessageLoop(); + ui_test_utils::RunThisRunLoop(&run_loop); } // Take snapshot of the current tab, encode it as PNG, and save to a SkBitmap. @@ -468,13 +469,7 @@ IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, DISABLED_TestURL) { RunTestWithURL(kAllowExternalDNS); } -// crbug.com/124049 -#if defined(OS_MACOSX) -#define MAYBE_Particles DISABLED_Particles -#else -#define MAYBE_Particles Particles -#endif -IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, MAYBE_Particles) { +IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, Particles) { RunTest("particles", kInternal); } @@ -527,13 +522,7 @@ IN_PROC_BROWSER_TEST_F(ThroughputTestSW, CanvasTextSW) { RunTest("canvas2d_balls_text", kNone); } -// crbug.com/124049 -#if defined(OS_MACOSX) -#define MAYBE_CanvasTextGPU DISABLED_CanvasTextGPU -#else -#define MAYBE_CanvasTextGPU CanvasTextGPU -#endif -IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, MAYBE_CanvasTextGPU) { +IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, CanvasTextGPU) { RunTest("canvas2d_balls_text", kNone | kIsGpuCanvasTest); } diff --git a/chrome_frame/test/chrome_frame_automation_mock.cc b/chrome_frame/test/chrome_frame_automation_mock.cc index 8b553a5..d307712 100644 --- a/chrome_frame/test/chrome_frame_automation_mock.cc +++ b/chrome_frame/test/chrome_frame_automation_mock.cc @@ -16,7 +16,8 @@ TEST(ChromeFrame, Launch) { loop.PostDelayedTask(FROM_HERE, MessageLoop::QuitClosure(), kLongWaitTimeout); mock_launch.Navigate("about:blank"); - loop.RunWithDispatcher(NULL); + base::RunLoop run_loop(NULL); + run_loop.Run(); EXPECT_TRUE(mock_launch.launch_result()); } @@ -28,7 +29,8 @@ TEST(ChromeFrame, Navigate) { loop.PostDelayedTask(FROM_HERE, MessageLoop::QuitClosure(), kLongWaitTimeout); mock_navigate.NavigateRelativeFile(L"postmessage_basic_frame.html"); - loop.RunWithDispatcher(NULL); + base::RunLoop run_loop(NULL); + run_loop.Run(); EXPECT_FALSE(mock_navigate.navigation_result()); } @@ -40,7 +42,8 @@ TEST(ChromeFrame, PostMessage) { loop.PostDelayedTask(FROM_HERE, MessageLoop::QuitClosure(), kLongWaitTimeout); mock_postmessage.NavigateRelativeFile(L"postmessage_basic_frame.html"); - loop.RunWithDispatcher(NULL); + base::RunLoop run_loop(NULL); + run_loop.Run(); EXPECT_FALSE(mock_postmessage.postmessage_result()); } @@ -52,7 +55,8 @@ TEST(ChromeFrame, RequestStart) { loop.PostDelayedTask(FROM_HERE, MessageLoop::QuitClosure(), kLongWaitTimeout); mock_request_start.NavigateRelative(L"postmessage_basic_frame.html"); - loop.RunWithDispatcher(NULL); + base::RunLoop run_loop(NULL); + run_loop.Run(); EXPECT_TRUE(mock_request_start.request_start_result()); } diff --git a/chrome_frame/test/chrome_frame_test_utils.h b/chrome_frame/test/chrome_frame_test_utils.h index b4ef4fd..bca9ae1 100644 --- a/chrome_frame/test/chrome_frame_test_utils.h +++ b/chrome_frame/test/chrome_frame_test_utils.h @@ -18,6 +18,7 @@ #include "base/file_path.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "base/process_util.h" #include "base/time.h" #include "base/test/test_reg_util_win.h" diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc index 5ce5c3e..2932aeb 100644 --- a/content/browser/browser_main_loop.cc +++ b/content/browser/browser_main_loop.cc @@ -12,6 +12,7 @@ #include "base/message_loop.h" #include "base/metrics/field_trial.h" #include "base/metrics/histogram.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "base/threading/thread_restrictions.h" #include "content/browser/browser_thread_impl.h" @@ -634,16 +635,16 @@ void BrowserMainLoop::InitializeToolkit() { } void BrowserMainLoop::MainMessageLoopRun() { - if (parameters_.ui_task) - MessageLoopForUI::current()->PostTask(FROM_HERE, *parameters_.ui_task); - -#if defined(OS_MACOSX) - MessageLoopForUI::current()->Run(); -#elif defined(OS_ANDROID) +#if defined(OS_ANDROID) // Android's main message loop is the Java message loop. NOTREACHED(); #else - MessageLoopForUI::current()->RunWithDispatcher(NULL); + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); + if (parameters_.ui_task) + MessageLoopForUI::current()->PostTask(FROM_HERE, *parameters_.ui_task); + + base::RunLoop run_loop; + run_loop.Run(); #endif } diff --git a/content/test/test_navigation_observer.cc b/content/test/test_navigation_observer.cc index 89ff36f..2342c63 100644 --- a/content/test/test_navigation_observer.cc +++ b/content/test/test_navigation_observer.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" #include "content/public/browser/render_view_host_observer.h" @@ -14,6 +15,21 @@ namespace content { +namespace { + +// Allow some pending tasks up to |num_deferrals| generations to complete. +void DeferredQuitRunLoop(const base::Closure& quit_task, + int num_quit_deferrals) { + if (num_quit_deferrals <= 0) { + quit_task.Run(); + } else { + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&DeferredQuitRunLoop, quit_task, num_quit_deferrals - 1)); + } +} + +} // namespace + // This class observes |render_view_host| and calls OnJsInjectionReady() of // |js_injection_ready_observer| when the time is right to inject JavaScript // into the page. @@ -84,11 +100,14 @@ void TestNavigationObserver::WaitForObservation( } void TestNavigationObserver::Wait() { + base::RunLoop run_loop; + // Number of times to repost Quit task to allow pending tasks to complete. See + // kNumQuitDeferrals in ui_test_utils.cc for explanation. + const int num_quit_deferrals = 10; WaitForObservation( - base::Bind(&MessageLoop::Run, - base::Unretained(MessageLoop::current())), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoop::current()))); + base::Bind(&base::RunLoop::Run, base::Unretained(&run_loop)), + base::Bind(&DeferredQuitRunLoop, run_loop.QuitClosure(), + num_quit_deferrals)); } TestNavigationObserver::TestNavigationObserver( @@ -131,8 +150,10 @@ void TestNavigationObserver::Observe( ++navigations_completed_ == number_of_navigations_) { navigation_started_ = false; done_ = true; - if (running_) + if (running_) { + running_ = false; done_callback_.Run(); + } } break; case NOTIFICATION_RENDER_VIEW_HOST_CREATED: diff --git a/ui/aura/desktop/desktop_dispatcher_client.cc b/ui/aura/desktop/desktop_dispatcher_client.cc index d9a33cf..0228746 100644 --- a/ui/aura/desktop/desktop_dispatcher_client.cc +++ b/ui/aura/desktop/desktop_dispatcher_client.cc @@ -4,6 +4,8 @@ #include "ui/aura/desktop/desktop_dispatcher_client.h" +#include "base/run_loop.h" + namespace aura { DesktopDispatcherClient::DesktopDispatcherClient() {} @@ -24,7 +26,8 @@ void DesktopDispatcherClient::RunWithDispatcher( // DefaultAcceleratorDispatcher dispatcher(nested_dispatcher, // associated_window); - loop->RunWithDispatcher(nested_dispatcher); + base::RunLoop run_loop(nested_dispatcher); + run_loop.Run(); loop->SetNestableTasksAllowed(did_allow_task_nesting); } diff --git a/ui/aura/env.h b/ui/aura/env.h index bef9483..c2b56c7 100644 --- a/ui/aura/env.h +++ b/ui/aura/env.h @@ -68,9 +68,8 @@ class AURA_EXPORT Env { CursorManager* cursor_manager() { return &cursor_manager_; } // Returns the native event dispatcher. The result should only be passed to - // MessageLoopForUI::RunWithDispatcher() or - // MessageLoopForUI::RunAllPendingWithDispatcher(), or used to dispatch - // an event by |Dispatch(const NativeEvent&)| on it. It must never be stored. + // base::RunLoop(dispatcher), or used to dispatch an event by + // |Dispatch(const NativeEvent&)| on it. It must never be stored. #if !defined(OS_MACOSX) MessageLoop::Dispatcher* GetDispatcher(); #endif diff --git a/ui/aura/test/aura_test_helper.cc b/ui/aura/test/aura_test_helper.cc index 1dabfd4..1453c86 100644 --- a/ui/aura/test/aura_test_helper.cc +++ b/ui/aura/test/aura_test_helper.cc @@ -5,6 +5,7 @@ #include "ui/aura/test/aura_test_helper.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "ui/aura/client/aura_constants.h" #include "ui/aura/env.h" #include "ui/aura/focus_manager.h" @@ -78,8 +79,10 @@ void AuraTestHelper::TearDown() { void AuraTestHelper::RunAllPendingInMessageLoop() { #if !defined(OS_MACOSX) - message_loop_->RunAllPendingWithDispatcher( - Env::GetInstance()->GetDispatcher()); + // TODO(jbates) crbug.com/134753 Find quitters of this RunLoop and have them + // use run_loop.QuitClosure(). + base::RunLoop run_loop(Env::GetInstance()->GetDispatcher()); + run_loop.RunUntilIdle(); #endif } diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc index feda07e..6cf1f43 100644 --- a/ui/views/controls/menu/menu_controller.cc +++ b/ui/views/controls/menu/menu_controller.cc @@ -6,6 +6,7 @@ #include "base/i18n/case_conversion.h" #include "base/i18n/rtl.h" +#include "base/run_loop.h" #include "base/time.h" #include "base/utf_string_conversions.h" #include "ui/base/dragdrop/drag_utils.h" @@ -343,7 +344,8 @@ MenuItemView* MenuController::Run(Widget* parent, { MessageLoopForUI* loop = MessageLoopForUI::current(); MessageLoop::ScopedNestableTaskAllower allow(loop); - loop->RunWithDispatcher(this); + base::RunLoop run_loop(this); + run_loop.Run(); } #endif message_loop_depth_--; diff --git a/ui/views/examples/content_client/examples_browser_main_parts.cc b/ui/views/examples/content_client/examples_browser_main_parts.cc index c8bdba7..174bc80 100644 --- a/ui/views/examples/content_client/examples_browser_main_parts.cc +++ b/ui/views/examples/content_client/examples_browser_main_parts.cc @@ -7,6 +7,7 @@ #include "base/bind.h" #include "base/command_line.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "base/threading/thread.h" #include "base/threading/thread_restrictions.h" @@ -77,10 +78,11 @@ bool ExamplesBrowserMainParts::MainMessageLoopRun(int* result_code) { // xxx: Hax here because this kills event handling. #if !defined(USE_AURA) views::AcceleratorHandler accelerator_handler; - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + base::RunLoop run_loop(&accelerator_handler); #else - MessageLoopForUI::current()->Run(); + base::RunLoop run_loop; #endif + run_loop.Run(); return true; } diff --git a/ui/views/focus/focus_manager_unittest_win.cc b/ui/views/focus/focus_manager_unittest_win.cc index 2a18e54..0380293 100644 --- a/ui/views/focus/focus_manager_unittest_win.cc +++ b/ui/views/focus/focus_manager_unittest_win.cc @@ -4,6 +4,8 @@ #include "ui/views/focus/focus_manager.h" +#include "base/memory/scoped_ptr.h" +#include "base/run_loop.h" #include "base/utf_string_conversions.h" #include "ui/views/controls/button/text_button.h" #include "ui/views/focus/accelerator_handler.h" @@ -215,8 +217,8 @@ TEST_F(FocusManagerTest, IgnoreKeyupForAccelerators) { PostKeyDown(ui::VKEY_9); PostKeyUp(ui::VKEY_9); AcceleratorHandler accelerator_handler; - MessageLoopForUI::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + scoped_ptr<base::RunLoop> run_loop(new base::RunLoop(&accelerator_handler)); + run_loop->RunUntilIdle(); // Make sure we get a key-up and key-down. ASSERT_EQ(1U, mtv->keys_pressed().size()); EXPECT_EQ(ui::VKEY_9, mtv->keys_pressed()[0]); @@ -234,8 +236,8 @@ TEST_F(FocusManagerTest, IgnoreKeyupForAccelerators) { PostKeyUp(ui::VKEY_9); PostKeyUp(ui::VKEY_7); PostKeyUp(ui::VKEY_8); - MessageLoopForUI::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + run_loop.reset(new base::RunLoop(&accelerator_handler)); + run_loop->RunUntilIdle(); // Make sure we get a key-up and key-down. ASSERT_EQ(5U, mtv->keys_pressed().size()); EXPECT_EQ(ui::VKEY_9, mtv->keys_pressed()[0]); @@ -253,8 +255,8 @@ TEST_F(FocusManagerTest, IgnoreKeyupForAccelerators) { // Now send an accelerator key sequence. PostKeyDown(ui::VKEY_0); PostKeyUp(ui::VKEY_0); - MessageLoopForUI::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + run_loop.reset(new base::RunLoop(&accelerator_handler)); + run_loop->RunUntilIdle(); EXPECT_TRUE(mtv->keys_pressed().empty()); EXPECT_TRUE(mtv->keys_released().empty()); EXPECT_TRUE(mtv->accelerator_pressed()); @@ -268,8 +270,8 @@ TEST_F(FocusManagerTest, IgnoreKeyupForAccelerators) { PostKeyDown(ui::VKEY_0); PostKeyUp(ui::VKEY_1); PostKeyUp(ui::VKEY_0); - MessageLoopForUI::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + run_loop.reset(new base::RunLoop(&accelerator_handler)); + run_loop->RunUntilIdle(); EXPECT_TRUE(mtv->keys_pressed().empty()); EXPECT_TRUE(mtv->keys_released().empty()); EXPECT_TRUE(mtv->accelerator_pressed()); diff --git a/ui/views/test/views_test_base.cc b/ui/views/test/views_test_base.cc index 329c9f3..67c49c4 100644 --- a/ui/views/test/views_test_base.cc +++ b/ui/views/test/views_test_base.cc @@ -4,6 +4,8 @@ #include "ui/views/test/views_test_base.h" +#include "base/run_loop.h" + #if defined(USE_AURA) #include "ui/aura/env.h" #include "ui/aura/test/aura_test_helper.h" @@ -47,12 +49,11 @@ void ViewsTestBase::TearDown() { } void ViewsTestBase::RunPendingMessages() { + base::RunLoop run_loop; #if defined(USE_AURA) - message_loop_.RunAllPendingWithDispatcher( - aura::Env::GetInstance()->GetDispatcher()); -#else - message_loop_.RunAllPending(); + run_loop.set_dispatcher(aura::Env::GetInstance()->GetDispatcher()); #endif + run_loop.RunUntilIdle(); } } // namespace views |