Add support for base::Closure in the MessageLoop.

base/message_loop.cc

Index: base/message_loop.cc
diff --git a/base/message_loop.cc b/base/message_loop.cc
index d83adccec08b56f8cdeb991c1b06af60d7a877e0..b1a62865c9f2335219de30f0706e59c4a7c147b4 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -211,22 +211,60 @@ void MessageLoop::RemoveDestructionObserver(
 
 void MessageLoop::PostTask(
     const tracked_objects::Location& from_here, Task* task) {
-  PostTask_Helper(from_here, task, 0, true);
+  task->SetBirthPlace(from_here);
+  PendingTask pending_task(task, CalculateDelayedRuntime(0), true);
+  AddToIncomingQueue(pending_task);
 }
 
 void MessageLoop::PostDelayedTask(
     const tracked_objects::Location& from_here, Task* task, int64 delay_ms) {
-  PostTask_Helper(from_here, task, delay_ms, true);
+  task->SetBirthPlace(from_here);
+  PendingTask pending_task(task, CalculateDelayedRuntime(delay_ms), true);
+  AddToIncomingQueue(pending_task);
 }
 
 void MessageLoop::PostNonNestableTask(
     const tracked_objects::Location& from_here, Task* task) {
-  PostTask_Helper(from_here, task, 0, false);
+  task->SetBirthPlace(from_here);
+  PendingTask pending_task(task, CalculateDelayedRuntime(0), false);
+  AddToIncomingQueue(pending_task);
 }
 
 void MessageLoop::PostNonNestableDelayedTask(
     const tracked_objects::Location& from_here, Task* task, int64 delay_ms) {
-  PostTask_Helper(from_here, task, delay_ms, false);
+  task->SetBirthPlace(from_here);
+  PendingTask pending_task(task, CalculateDelayedRuntime(delay_ms), false);
+  AddToIncomingQueue(pending_task);
+}
+
+void MessageLoop::PostClosure(

[willchan no longer on Chromium, 2011/02/15 01:41:11]

For all these Post*Closure() functions, you need to assert that the Closure
isn't empty.

+    const tracked_objects::Location& from_here, const base::Closure& closure) {
+  // TODO(ajwong): Record |from_here| correctly for Closures.
+  PendingTask pending_task(closure, CalculateDelayedRuntime(0), true);
+  AddToIncomingQueue(pending_task);
+}
+
+void MessageLoop::PostDelayedClosure(
+    const tracked_objects::Location& from_here, const base::Closure& closure,
+    int64 delay_ms) {
+  // TODO(ajwong): Record |from_here| correctly for Closures.
+  PendingTask pending_task(closure, CalculateDelayedRuntime(delay_ms), true);
+  AddToIncomingQueue(pending_task);
+}
+
+void MessageLoop::PostNonNestableClosure(
+    const tracked_objects::Location& from_here, const base::Closure& closure) {
+  // TODO(ajwong): Record |from_here| correctly for Closures.
+  PendingTask pending_task(closure, CalculateDelayedRuntime(0), false);
+  AddToIncomingQueue(pending_task);
+}
+
+void MessageLoop::PostNonNestableDelayedClosure(
+    const tracked_objects::Location& from_here, const base::Closure& closure,
+    int64 delay_ms) {
+  // TODO(ajwong): Record |from_here| correctly for Closures.
+  PendingTask pending_task(closure, CalculateDelayedRuntime(delay_ms), false);
+  AddToIncomingQueue(pending_task);
 }
 
 void MessageLoop::Run() {
@@ -345,9 +383,15 @@ bool MessageLoop::ProcessNextDelayedNonNestableTask() {
     return false;
 
   Task* task = deferred_non_nestable_work_queue_.front().task;
-  deferred_non_nestable_work_queue_.pop();
-
-  RunTask(task);
+  if (task) {
+    deferred_non_nestable_work_queue_.pop();
+    RunTask(task);
+  } else {
+    // Take a copy since we will pop the queue.
+    base::Closure closure = deferred_non_nestable_work_queue_.front().closure;
+    deferred_non_nestable_work_queue_.pop();
+    RunClosure(closure);
+  }
   return true;
 }
 
@@ -366,11 +410,27 @@ void MessageLoop::RunTask(Task* task) {
   nestable_tasks_allowed_ = true;
 }
 
+void MessageLoop::RunClosure(const base::Closure& closure) {
+  DCHECK(nestable_tasks_allowed_);
+  // Execute the task and assume the worst: It is probably not reentrant.
+  nestable_tasks_allowed_ = false;
+
+  HistogramEvent(kTaskRunEvent);
+  // TODO(ajwong): Figure out how to handle FOR_EACH_OBSERVER.

[willchan no longer on Chromium, 2011/02/15 01:41:11]

Probably should add a ClosureObserver.

[awong, 2011/02/15 08:24:42]

Sounds like maybe we should modify the observer to only get the Tracked*, not
Task*.

+  closure.Run();
+
+  nestable_tasks_allowed_ = true;
+}
+
 bool MessageLoop::DeferOrRunPendingTask(const PendingTask& pending_task) {
   if (pending_task.nestable || state_->run_depth == 1) {
-    RunTask(pending_task.task);
-    // Show that we ran a task (Note: a new one might arrive as a
-    // consequence!).
+    if (pending_task.task) {
+      RunTask(pending_task.task);
+      // Show that we ran a task (Note: a new one might arrive as a
+      // consequence!).
+    } else {
+      RunClosure(pending_task.closure);
+    }
     return true;
   }
 
@@ -454,16 +514,10 @@ bool MessageLoop::DeletePendingTasks() {
   return did_work;
 }
 
-// Possibly called on a background thread!
-void MessageLoop::PostTask_Helper(
-    const tracked_objects::Location& from_here, Task* task, int64 delay_ms,
-    bool nestable) {
-  task->SetBirthPlace(from_here);
-
-  PendingTask pending_task(task, nestable);
-
+base::TimeTicks MessageLoop::CalculateDelayedRuntime(int64 delay_ms) {
+  base::TimeTicks delayed_run_time;
   if (delay_ms > 0) {
-    pending_task.delayed_run_time =
+    delayed_run_time =
         TimeTicks::Now() + TimeDelta::FromMilliseconds(delay_ms);
 
 #if defined(OS_WIN)
@@ -495,6 +549,11 @@ void MessageLoop::PostTask_Helper(
   }
 #endif
 
+  return delayed_run_time;
+}
+
+// Possibly called on a background thread!
+void MessageLoop::AddToIncomingQueue(const PendingTask& pending_task) {

[willchan no longer on Chromium, 2011/02/15 01:41:11]

You should sanity check PendingTask here. task XOR closure.

[awong, 2011/02/15 08:24:42]

Sounds like Darin is in for cowboying it up and just going wholesale Closure
now.  That will obviate this issue. 

Good times.

   // Warning: Don't try to short-circuit, and handle this thread's tasks more
   // directly, as it could starve handling of foreign threads.  Put every task
   // into this queue.
@@ -558,8 +617,13 @@ bool MessageLoop::DoWork() {
       if (!pending_task.delayed_run_time.is_null()) {
         AddToDelayedWorkQueue(pending_task);
         // If we changed the topmost task, then it is time to re-schedule.
-        if (delayed_work_queue_.top().task == pending_task.task)
+        //
+        // TODO(ajwong): I don't understand this conditional. It will call
+        // ScheduleDelayedWork() too often for Closure since task == NULL.
+        // It seems like we should be checking sequence numbers instead.
+        if (delayed_work_queue_.top().task == pending_task.task) {
           pump_->ScheduleDelayedWork(pending_task.delayed_run_time);
+        }
       } else {
         if (DeferOrRunPendingTask(pending_task))
           return true;