1 files changed, 91 insertions, 38 deletions

diff --git a/base/message_pump_glib.cc b/base/message_pump_glib.cc
index e032de4..1b973cb 100644
--- a/base/message_pump_glib.cc
+++ b/base/message_pump_glib.cc
@@ -50,6 +50,37 @@ int GetTimeIntervalMilliseconds(base::Time from) {
 // returns FALSE, GLib will destroy the source.  Dispatch calls may be recursive
 // (i.e., you can call Run from them), but Prepare and Check cannot.
 //     Finalize is called when the source is destroyed.
+// NOTE: It is common for subsytems to want to process pending events while
+// doing intensive work, for example the flash plugin. They usually use the
+// following pattern (recommended by the GTK docs):
+// while (gtk_events_pending()) {
+//   gtk_main_iteration();
+// }
+//
+// gtk_events_pending just calls g_main_context_pending, which does the
+// following:
+// - call prepare on all the sources
+// - do the poll with a timeout of 0 (not blocking)
+// - call check on all the sources
+// - *does not* call dispatch on the sources
+// - return true iff any of prepare() or check() returned true
+//
+// gtk_main_iteration just calls g_main_context_iteration, which does the whole
+// thing, respecting the timeout for the poll (and block, although it is
+// expected not to if gtk_events_pending returned true), and call dispatch.
+//
+// Thus it is important to only return true from prepare or check if we
+// actually have events or work to do. We also need to make sure we keep
+// internal state consistent so that if prepare/check return true when called
+// from gtk_events_pending, they will still return true when called right
+// after, from gtk_main_iteration.
+//
+// For the GLib pump we try to follow the Windows UI pump model:
+// - whenever we receive a wakeup event or the timer for delayed work expires,
+// we run DoWork and/or DoDelayedWork. That part will also run in the other
+// event pumps
+// - we also run DoWork, DoDelayedWork, and possibly DoIdleWork in the main
+// loop, around event handling.
 
 struct WorkSource : public GSource {
   base::MessagePumpForUI* pump;
@@ -65,8 +96,8 @@ gboolean WorkSourcePrepare(GSource* source,
 }
 
 gboolean WorkSourceCheck(GSource* source) {
-  // Always return TRUE, and Dispatch will be called.
-  return TRUE;
+  // Only return TRUE if Dispatch should be called.
+  return static_cast<WorkSource*>(source)->pump->HandleCheck();
 }
 
 gboolean WorkSourceDispatch(GSource* source,
@@ -136,46 +167,64 @@ void MessagePumpForUI::Run(Delegate* delegate) {
   state.delegate = delegate;
   state.should_quit = false;
   state.run_depth = state_ ? state_->run_depth + 1 : 1;
+  state.has_work = false;
+
+  RunState* previous_state = state_;
+  state_ = &state;
+
   // We really only do a single task for each iteration of the loop.  If we
   // have done something, assume there is likely something more to do.  This
   // will mean that we don't block on the message pump until there was nothing
   // more to do.  We also set this to true to make sure not to block on the
   // first iteration of the loop, so RunAllPending() works correctly.
-  state.more_work_is_plausible = true;
-
-  RunState* previous_state = state_;
-  state_ = &state;
+  bool more_work_is_plausible = true;
 
   // We run our own loop instead of using g_main_loop_quit in one of the
   // callbacks.  This is so we only quit our own loops, and we don't quit
   // nested loops run by others.  TODO(deanm): Is this what we want?
-  while (!state_->should_quit)
-    g_main_context_iteration(context_, true);
+  for (;;) {
+    // Don't block if we think we have more work to do.
+    bool block = !more_work_is_plausible;
+
+    // g_main_context_iteration returns true if events have been dispatched.
+    more_work_is_plausible = g_main_context_iteration(context_, block);
+    if (state_->should_quit)
+      break;
+
+    more_work_is_plausible |= state_->delegate->DoWork();
+    if (state_->should_quit)
+      break;
+
+    more_work_is_plausible |=
+        state_->delegate->DoDelayedWork(&delayed_work_time_);
+
+    if (state_->should_quit)
+      break;
+
+    if (more_work_is_plausible)
+      continue;
+
+    more_work_is_plausible = state_->delegate->DoIdleWork();
+    if (state_->should_quit)
+      break;
+  }
 
   state_ = previous_state;
 }
 
 // Return the timeout we want passed to poll.
 int MessagePumpForUI::HandlePrepare() {
-  // If it's likely that we have more work, don't let the pump
-  // block so that we can do some processing.
-  if (state_->more_work_is_plausible)
+  // We know we have work, but we haven't called HandleDispatch yet. Don't let
+  // the pump block so that we can do some processing.
+  if (state_->has_work)
     return 0;
 
-  // Work wasn't plausible, so we'll block.  In the case where glib fires
-  // our Dispatch(), |more_work_is_plausible| will be reset to whatever it
-  // should be.  However, so we don't get starved by more important work,
-  // we set |more_work_is_plausible| to true.  This means if we come back
-  // here without having been through Dispatch(), we will get a chance to be
-  // fired and properly do our work in Dispatch().
-  state_->more_work_is_plausible = true;
-
   // We don't think we have work to do, but make sure not to block
   // longer than the next time we need to run delayed work.
   return GetTimeIntervalMilliseconds(delayed_work_time_);
 }
 
-void MessagePumpForUI::HandleDispatch() {
+bool MessagePumpForUI::HandleCheck() {
   // We should only ever have a single message on the wakeup pipe, since we
   // are only signaled when the queue went from empty to non-empty.  The glib
   // poll will tell us whether there was data, so this read shouldn't block.
@@ -184,33 +233,37 @@ void MessagePumpForUI::HandleDispatch() {
     if (HANDLE_EINTR(read(wakeup_pipe_read_, &msg, 1)) != 1 || msg != '!') {
       NOTREACHED() << "Error reading from the wakeup pipe.";
     }
+    // Since we ate the message, we need to record that we have more work,
+    // because HandleCheck() may be called without HandleDispatch being called
+    // afterwards.
+    state_->has_work = true;
   }
 
-  if (state_->should_quit)
-    return;
+  if (state_->has_work)
+    return true;
 
-  state_->more_work_is_plausible = false;
+  if (GetTimeIntervalMilliseconds(delayed_work_time_) == 0) {
+    // The timer has expired. That condition will stay true until we process
+    // that delayed work, so we don't need to record this differently.
+    return true;
+  }
 
-  if (state_->delegate->DoWork())
-    state_->more_work_is_plausible = true;
+  return false;
+}
 
-  if (state_->should_quit)
-    return;
+void MessagePumpForUI::HandleDispatch() {
+  state_->has_work = false;
+  if (state_->delegate->DoWork()) {
+    // NOTE: on Windows at this point we would call ScheduleWork. But here,
+    // instead of posting a message on the wakeup pipe, we can avoid the
+    // syscalls and just signal that we have more work.
+    state_->has_work = true;
+  }
 
-  if (state_->delegate->DoDelayedWork(&delayed_work_time_))
-    state_->more_work_is_plausible = true;
   if (state_->should_quit)
     return;
 
-  // Don't do idle work if we think there are more important things
-  // that we could be doing.
-  if (state_->more_work_is_plausible)
-    return;
-
-  if (state_->delegate->DoIdleWork())
-    state_->more_work_is_plausible = true;
-  if (state_->should_quit)
-    return;
+  state_->delegate->DoDelayedWork(&delayed_work_time_);
 }
 
 void MessagePumpForUI::AddObserver(Observer* observer) {