summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorjar@google.com <jar@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2008-07-30 19:03:59 +0000
committerjar@google.com <jar@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2008-07-30 19:03:59 +0000
commit3882c43300022509c46993d61893414ac8a8b7fb (patch)
tree538231d18e9cab60b0e1f626d32c8669486f539f
parent4aed32afe2a17d07c8879de8d8d7857225f88537 (diff)
downloadchromium_src-3882c43300022509c46993d61893414ac8a8b7fb.zip
chromium_src-3882c43300022509c46993d61893414ac8a8b7fb.tar.gz
chromium_src-3882c43300022509c46993d61893414ac8a8b7fb.tar.bz2
(Re-landing of) Support RunOnce() in message loop.
Deeply nested calls to MessageLoop::Run() were made, and a multitude of Quit() messages were handled at the most nested level. This left outer invocations hung, waiting for "their" kQuitMsg to arrive (but the the nested loop had already discarded them in its processing of pending messages before exit :-/ ). We now use a more controlled run of the message loop, which does not rely on kQuitMsg. This re-landing doesn't have the anti-hang assertion, which was breaking a lot of tests. bug=1291034 r=darin,mpcomplete M base/message_loop.h M base/message_loop.cc M chrome/common/ipc_sync_channel.cc git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--base/message_loop.cc75
-rw-r--r--base/message_loop.h29
-rw-r--r--chrome/common/ipc_sync_channel.cc7
3 files changed, 63 insertions, 48 deletions
diff --git a/base/message_loop.cc b/base/message_loop.cc
index 623ec99..d348908 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -169,7 +169,15 @@ void MessageLoop::RemoveObserver(Observer *obs) {
}
void MessageLoop::Run() {
- Run(NULL);
+ RunHandler(NULL, false);
+}
+
+void MessageLoop::Run(Dispatcher* dispatcher) {
+ RunHandler(dispatcher, false);
+}
+
+void MessageLoop::RunOnce() {
+ RunHandler(NULL, true);
}
// Runs the loop in two different SEH modes:
@@ -177,20 +185,19 @@ void MessageLoop::Run() {
// one that calls SetUnhandledExceptionFilter().
// enable_SEH_restoration_ = true : any unhandled exception goes to the filter
// that was existed before the loop was run.
-void MessageLoop::Run(Dispatcher* dispatcher) {
+void MessageLoop::RunHandler(Dispatcher* dispatcher, bool run_loop_once) {
if (exception_restoration_) {
LPTOP_LEVEL_EXCEPTION_FILTER current_filter = GetTopSEHFilter();
__try {
- RunInternal(dispatcher);
+ RunInternal(dispatcher, run_loop_once);
} __except(SEHFilter(current_filter)) {
}
} else {
- RunInternal(dispatcher);
+ RunInternal(dispatcher, run_loop_once);
}
}
//------------------------------------------------------------------------------
-// Methods supporting various strategies for servicing the numerous queues.
// IF this was just a simple PeekMessage() loop (servicing all passible work
// queues), then Windows would try to achieve the following order according to
// MSDN documentation about PeekMessage with no filter):
@@ -203,7 +210,7 @@ void MessageLoop::Run(Dispatcher* dispatcher) {
// Summary: none of the above classes is starved, and sent messages has twice
// the chance of being processed (i.e., reduced service time).
-void MessageLoop::RunInternal(Dispatcher* dispatcher) {
+void MessageLoop::RunInternal(Dispatcher* dispatcher, bool run_loop_once) {
// Preserve ability to be called recursively.
ScopedStateSave save(this); // State is restored on exit.
dispatcher_ = dispatcher;
@@ -211,38 +218,30 @@ void MessageLoop::RunInternal(Dispatcher* dispatcher) {
DCHECK(this == current());
//
- // Process all pending messages and signaled objects.
+ // Process pending messages and signaled objects.
//
// Flush these queues before exiting due to a kMsgQuit or else we risk not
// shutting down properly as some operations may depend on further event
// processing. (Note: some tests may use quit_now_ to exit more swiftly,
// and leave messages pending, so don't assert the above fact).
- //
-
- RunTraditional();
- DCHECK(quit_received_ || quit_now_);
+ RunTraditional(run_loop_once);
+ DCHECK(run_loop_once || quit_received_ || quit_now_);
}
-typedef bool (MessageLoop::*ProcessingMethod)();
-typedef ProcessingMethod ProcessingMethods[];
-
-void MessageLoop::RunTraditional() {
- run_depth_++;
- for (;;) {
+void MessageLoop::RunTraditional(bool run_loop_once) {
+ do {
// If we do any work, we may create more messages etc., and more work
- // may possibly be waiting in another task group. In addition, each method
- // call here typically limits work to 1 (worst case 2) items. As a result,
- // when we (for example) ProcessNextWindowsMessage() there is a good chance
- // there are still more waiting (same thing for ProcessNextDeferredTask(),
- // which responds to only one signaled object.). On the other hand, when
- // any of these methods return having done no work, then it is pretty
- // unlikely that calling them again quickly will find any work to do.
+ // may possibly be waiting in another task group. When we (for example)
+ // ProcessNextWindowsMessage(), there is a good chance there are still more
+ // messages waiting (same thing for ProcessNextObject(), which responds to
+ // only one signaled object; etc.). On the other hand, when any of these
+ // methods return having done no work, then it is pretty unlikely that
+ // calling them again quickly will find any work to do.
// Finally, if they all say they had no work, then it is a good time to
// consider sleeping (waiting) for more work.
- bool more_work_is_plausible = false;
- more_work_is_plausible |= ProcessNextWindowsMessage();
+ bool more_work_is_plausible = ProcessNextWindowsMessage();
if (quit_now_)
- break;
+ return;
more_work_is_plausible |= ProcessNextDeferredTask();
more_work_is_plausible |= ProcessNextObject();
@@ -250,7 +249,7 @@ void MessageLoop::RunTraditional() {
continue;
if (quit_received_)
- break;
+ return;
// Run any timer that is ready to run. It may create messages etc.
if (ProcessSomeTimers())
@@ -258,17 +257,20 @@ void MessageLoop::RunTraditional() {
// We run delayed non nestable tasks only after all nestable tasks have
// run, to preserve FIFO ordering.
- more_work_is_plausible = ProcessNextDelayedNonNestableTask();
- if (more_work_is_plausible)
+ if (ProcessNextDelayedNonNestableTask())
continue;
+ if (run_loop_once)
+ return;
+
// We service APCs in WaitForWork, without returning.
WaitForWork(); // Wait (sleep) until we have work to do again.
- }
-
- run_depth_--;
+ } while (!run_loop_once);
}
+//------------------------------------------------------------------------------
+// Wrapper functions for use in above message loop framework.
+
bool MessageLoop::ProcessNextDelayedNonNestableTask() {
if (run_depth_ != 1)
return false;
@@ -280,9 +282,6 @@ bool MessageLoop::ProcessNextDelayedNonNestableTask() {
return true;
}
-//------------------------------------------------------------------------------
-// Wrapper functions for use in above message loop frameworks.
-
bool MessageLoop::ProcessNextDeferredTask() {
ReloadWorkQueue();
return QueueOrRunTask(NULL);
@@ -399,6 +398,10 @@ LRESULT MessageLoop::MessageWndProc(HWND hwnd, UINT message,
}
case kMsgQuit: {
+ // TODO(jar): bug 1300541 The following assert should be used, but
+ // currently too much code actually triggers the assert, especially in
+ // tests :-(.
+ //CHECK(!quit_received_); // Discarding a second quit will cause a hang.
quit_received_ = true;
return 0;
}
diff --git a/base/message_loop.h b/base/message_loop.h
index 1d1e11b..ecb7e27 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -249,9 +249,12 @@ class MessageLoop {
PostTask(from_here, new ReleaseTask<T>(object));
}
- // Run the message loop
+ // Run the message loop.
void Run();
+ // Run just one pass through the message loop.
+ void RunOnce();
+
// See description of Dispatcher for how Run uses Dispatcher.
void Run(Dispatcher* dispatcher);
@@ -347,11 +350,14 @@ class MessageLoop {
: loop_(loop),
dispatcher_(loop->dispatcher_),
quit_now_(loop->quit_now_),
- quit_received_(loop->quit_received_) {
+ quit_received_(loop->quit_received_),
+ run_depth_(loop->run_depth_) {
loop->quit_now_ = loop->quit_received_ = false;
+ ++loop->run_depth_;
}
~ScopedStateSave() {
+ loop_->run_depth_ = run_depth_;
loop_->quit_received_ = quit_received_;
loop_->quit_now_ = quit_now_;
loop_->dispatcher_ = dispatcher_;
@@ -362,6 +368,7 @@ class MessageLoop {
Dispatcher* dispatcher_;
bool quit_now_;
bool quit_received_;
+ int run_depth_;
}; // struct ScopedStateSave
// A prioritized queue with interface that mostly matches std::queue<>.
@@ -431,15 +438,21 @@ class MessageLoop {
void InitMessageWnd();
- // The actual message loop implementation. Called by all flavors of Run().
+
+ // A function to encapsulate all the exception handling capability in the
+ // stacks around the running of a main message loop.
// It will run the message loop in a SEH try block or not depending on the
// set_SEH_restoration() flag.
- void RunInternal(Dispatcher* dispatcher);
+ void RunHandler(Dispatcher* dispatcher, bool run_loop_once);
- //----------------------------------------------------------------------------
- // A list of alternate message loop priority systems. The strategy_selector_
- // determines which one to actually use.
- void RunTraditional();
+ // A surrounding stack frame around the running of the message loop that
+ // supports all saving and restoring of state, as is needed for any/all (ugly)
+ // recursive calls.
+ void RunInternal(Dispatcher* dispatcher, bool run_loop_once);
+
+ // An extended message loop (message pump) that loops mostly forever, and
+ // processes task, signals, timers, etc.
+ void RunTraditional(bool run_loop_once);
//----------------------------------------------------------------------------
// A list of method wrappers with identical calling signatures (no arguments)
diff --git a/chrome/common/ipc_sync_channel.cc b/chrome/common/ipc_sync_channel.cc
index bbd7a23..1dc106c 100644
--- a/chrome/common/ipc_sync_channel.cc
+++ b/chrome/common/ipc_sync_channel.cc
@@ -445,11 +445,10 @@ bool SyncChannel::Send(IPC::Message* message) {
// shutdown the nested loop when there are no more messages.
pump_messages_events_.push(pump_messages_event);
bool old_state = MessageLoop::current()->NestableTasksAllowed();
- // Insert a Quit message so that we just flush the MessageLoop without
- // letting the MessageLoop wait for more messages.
- MessageLoop::current()->Quit();
MessageLoop::current()->SetNestableTasksAllowed(true);
- MessageLoop::current()->Run();
+ // Process a message, but come right back out of the MessageLoop (don't
+ // loop, sleep, or wait for a kMsgQuit).
+ MessageLoop::current()->RunOnce();
MessageLoop::current()->SetNestableTasksAllowed(old_state);
pump_messages_events_.pop();
} else {