Add support for base::Closure in the MessageLoop.

base/message_loop.h

Index: base/message_loop.h
diff --git a/base/message_loop.h b/base/message_loop.h
index 519e4a30dab9958d9036d7e3aed1c53655ebca7b..75f57360b710d6f6bb49e8d5f8d2fe887c94fcd8 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -11,11 +11,14 @@
 
 #include "base/base_api.h"
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_pump.h"
 #include "base/observer_list.h"
 #include "base/synchronization/lock.h"
 #include "base/task.h"
+#include "base/time.h"
+#include "base/tracked.h"
 
 #if defined(OS_WIN)
 // We need this to declare base::MessagePumpWin::Dispatcher, which we should
@@ -36,6 +39,12 @@ namespace base {
 class Histogram;
 }
 
+#if defined(TRACK_ALL_TASK_OBJECTS)
+namespace tracked_objects {
+class Births;
+}
+#endif  // defined(TRACK_ALL_TASK_OBJECTS)
+
 // A MessageLoop is used to process events for a particular thread.  There is
 // at most one MessageLoop instance per thread.
 //
@@ -163,6 +172,22 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
   void PostNonNestableDelayedTask(
       const tracked_objects::Location& from_here, Task* task, int64 delay_ms);
 
+  void PostClosure(
+      const tracked_objects::Location& from_here,
+      const base::Closure& closure);
+
+  void PostDelayedClosure(
+      const tracked_objects::Location& from_here,
+      const base::Closure& closure, int64 delay_ms);
+
+  void PostNonNestableClosure(
+      const tracked_objects::Location& from_here,
+      const base::Closure& closure);
+
+  void PostNonNestableDelayedClosure(
+      const tracked_objects::Location& from_here,
+      const base::Closure& closure, int64 delay_ms);
+
   // A variant on PostTask that deletes the given object.  This is useful
   // if the object needs to live until the next run of the MessageLoop (for
   // example, deleting a RenderProcessHost from within an IPC callback is not
@@ -280,28 +305,28 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
   // Returns true if we are currently running a nested message loop.
   bool IsNested();
 
-  // A TaskObserver is an object that receives task notifications from the
+  // A ClosureObserver is an object that receives task notifications from the
   // MessageLoop.
   //
-  // NOTE: A TaskObserver implementation should be extremely fast!
-  class BASE_API TaskObserver {
+  // NOTE: A ClosureObserver implementation should be extremely fast!
+  class BASE_API ClosureObserver {

[Mike Belshe, 2011/04/13 04:16:58]

Even if "ClosureObserver" is more accurate than "TaskObserver", it is far more
complexifying - especially when you consider "WillProcessClosure()", what does
that mean?  It sounds like its creating a Closure or binding things together. 
The old name "WillProcessTask" is actually much closer to what is really going
on here, can we keep that API, even if you change the data structure used
internally?

    public:
-    TaskObserver();
+    ClosureObserver();
 
     // This method is called before processing a task.
-    virtual void WillProcessTask(const Task* task) = 0;
+    virtual void WillProcessClosure(base::TimeTicks time_posted) = 0;
 
     // This method is called after processing a task.
-    virtual void DidProcessTask(const Task* task) = 0;
+    virtual void DidProcessClosure(base::TimeTicks time_posted) = 0;
 
    protected:
-    virtual ~TaskObserver();
+    virtual ~ClosureObserver();
   };
 
   // These functions can only be called on the same thread that |this| is
   // running on.
-  void AddTaskObserver(TaskObserver* task_observer);
-  void RemoveTaskObserver(TaskObserver* task_observer);
+  void AddClosureObserver(ClosureObserver* closure_observer);
+  void RemoveClosureObserver(ClosureObserver* closure_observer);
 
   // Returns true if the message loop has high resolution timers enabled.
   // Provided for testing.
@@ -355,28 +380,44 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
   };
 
   // This structure is copied around by value.
-  struct PendingTask {
-    PendingTask(Task* task, bool nestable)
-        : task(task), sequence_num(0), nestable(nestable) {
-    }
+  struct PendingClosure {

[Mike Belshe, 2011/04/13 04:16:58]

Similar comment here.  The name "PendingClosure" sounds like it means that we're
now binding a bunch of things together and processing something that is pending
Closure.  

+    PendingClosure(const base::Closure& closure,
+                   const tracked_objects::Location& posted_from,
+                   base::TimeTicks delayed_run_time,
+                   bool nestable);
 
     // Used to support sorting.
-    bool operator<(const PendingTask& other) const;
+    bool operator<(const PendingClosure& other) const;
+
+    // The Closure to run.
+    base::Closure closure;
 
-    Task* task;                        // The task to run.
-    base::TimeTicks delayed_run_time;  // The time when the task should be run.
-    int sequence_num;                  // Secondary sort key for run time.
-    bool nestable;                     // OK to dispatch from a nested loop.
+#if defined(TRACK_ALL_TASK_OBJECTS)
+    // Counter for location where the Closure was posted from.
+    tracked_objects::Births* post_births;
+#endif  // defined(TRACK_ALL_TASK_OBJECTS)
+
+    // Time this PendingClosure was posted.
+    base::TimeTicks time_posted;
+
+    // The time when the task should be run.
+    base::TimeTicks delayed_run_time;
+
+    // Secondary sort key for run time.
+    int sequence_num;
+
+    // OK to dispatch from a nested loop.
+    bool nestable;
   };
 
-  class TaskQueue : public std::queue<PendingTask> {
+  class TaskQueue : public std::queue<PendingClosure> {

[Mike Belshe, 2011/04/13 04:16:58]

Ah - precedent - thank you for not changing this to ClosureQueue. :-)

Similar comments throughout.  I think the MessageLoop is mechanism for
processing a set of "Tasks" not a set of "Closures".  Bury an references to
Closures as deep as you can.

    public:
     void Swap(TaskQueue* queue) {
       c.swap(queue->c);  // Calls std::deque::swap
     }
   };
 
-  typedef std::priority_queue<PendingTask> DelayedTaskQueue;
+  typedef std::priority_queue<PendingClosure> DelayedTaskQueue;
 
 #if defined(OS_WIN)
   base::MessagePumpWin* pump_win() {
@@ -406,15 +447,23 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
   // Called to process any delayed non-nestable tasks.
   bool ProcessNextDelayedNonNestableTask();
 
-  // Runs the specified task and deletes it.
-  void RunTask(Task* task);
+  // Runs the specified PendingClosure.
+  void RunClosure(const PendingClosure& closure);
 
-  // Calls RunTask or queues the pending_task on the deferred task list if it
+  // Calls RunTask or queues the pending_closure on the deferred task list if it
   // cannot be run right now.  Returns true if the task was run.
-  bool DeferOrRunPendingTask(const PendingTask& pending_task);
+  bool DeferOrRunPendingClosure(const PendingClosure& pending_closure);
 
   // Adds the pending task to delayed_work_queue_.
-  void AddToDelayedWorkQueue(const PendingTask& pending_task);
+  void AddToDelayedWorkQueue(const PendingClosure& pending_closure);
+
+  // Adds the pending task to our incoming_queue_.
+  //
+  // Caller retains ownership of |pending_closure|, but this function will
+  // reset the value of pending_closure->closure.  This is needed to ensure
+  // that the posting call stack does not retain pending_closure->closure
+  // beyond this function call.
+  void AddToIncomingQueue(PendingClosure* pending_closure);
 
   // Load tasks from the incoming_queue_ into work_queue_ if the latter is
   // empty.  The former requires a lock to access, while the latter is directly
@@ -424,11 +473,10 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
   // Delete tasks that haven't run yet without running them.  Used in the
   // destructor to make sure all the task's destructors get called.  Returns
   // true if some work was done.
-  bool DeletePendingTasks();
+  bool DeletePendingClosures();
 
-  // Post a task to our incomming queue.
-  void PostTask_Helper(const tracked_objects::Location& from_here, Task* task,
-                       int64 delay_ms, bool nestable);
+  // Calcuates the time at which a PendingClosure should run.
+  base::TimeTicks CalculateDelayedRuntime(int64 delay_ms);
 
   // Start recording histogram info about events and action IF it was enabled
   // and IF the statistics recorder can accept a registration of our histogram.
@@ -477,14 +525,16 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
 
   // A null terminated list which creates an incoming_queue of tasks that are
   // acquired under a mutex for processing on this instance's thread. These
-  // tasks have not yet been sorted out into items for our work_queue_ vs
-  // items that will be handled by the TimerManager.
+  // tasks have not yet been sorted out into items for our work_queue_ vs items
+  // that will be handled by the TimerManager.
   TaskQueue incoming_queue_;
   // Protect access to incoming_queue_.
   mutable base::Lock incoming_queue_lock_;
 
   RunState* state_;
 
+  bool should_leak_tasks_;
+
 #if defined(OS_WIN)
   base::TimeTicks high_resolution_timer_expiration_;
   // Should be set to true before calling Windows APIs like TrackPopupMenu, etc
@@ -495,7 +545,7 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
   // The next sequence number to use for delayed tasks.
   int next_sequence_num_;
 
-  ObserverList<TaskObserver> task_observers_;
+  ObserverList<ClosureObserver> closure_observers_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MessageLoop);