Support Closure in ALL the loops!

base/message_loop.cc

Index: base/message_loop.cc
diff --git a/base/message_loop.cc b/base/message_loop.cc
index e45c0d7cadd1c53a70fc98f4ad1fe72295beace1..8a4d1570c90f5ff968fc387a46be0b1320e759c4 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -85,40 +85,6 @@ const base::LinearHistogram::DescriptionPair event_descriptions_[] = {
 
 bool enable_histogrammer_ = false;
 
-// TODO(ajwong): This is one use case for having a Owned() tag that behaves
-// like a "Unique" pointer.  If we had that, and Tasks were always safe to
-// delete on MessageLoop shutdown, this class could just be a function.
-class TaskClosureAdapter : public base::RefCounted<TaskClosureAdapter> {
- public:
-  // |should_leak_task| points to a flag variable that can be used to determine
-  // if this class should leak the Task on destruction.  This is important
-  // at MessageLoop shutdown since not all tasks can be safely deleted without
-  // running.  See MessageLoop::DeletePendingTasks() for the exact behavior
-  // of when a Task should be deleted. It is subtle.
-  TaskClosureAdapter(Task* task, bool* should_leak_task)
-      : task_(task),
-        should_leak_task_(should_leak_task) {
-  }
-
-  void Run() {
-    task_->Run();
-    delete task_;
-    task_ = NULL;
-  }
-
- private:
-  friend class base::RefCounted<TaskClosureAdapter>;
-
-  ~TaskClosureAdapter() {
-    if (!*should_leak_task_) {
-      delete task_;
-    }
-  }
-
-  Task* task_;
-  bool* should_leak_task_;
-};
-
 }  // namespace
 
 //------------------------------------------------------------------------------
@@ -271,8 +237,9 @@ void MessageLoop::PostTask(
     const tracked_objects::Location& from_here, Task* task) {
   CHECK(task);
   PendingTask pending_task(
-      base::Bind(&TaskClosureAdapter::Run,
-                 new TaskClosureAdapter(task, &should_leak_tasks_)),
+      base::Bind(
+          &base::subtle::TaskClosureAdapter::Run,
+          new base::subtle::TaskClosureAdapter(task, &should_leak_tasks_)),

[willchan no longer on Chromium, 2011/07/10 16:47:33]

I was looking at the refcounted nature of TaskClosureAdapter and was wondering
about whether or not we should revisit this. It looks like it's refcounted so we
can just copy PendingTask. Copying PendingTask saved a heap allocation before,
but now that we have a TaskClosureAdapter, I don't know if it does. Perhaps
rather than making TaskClosureAdapter copyable, we should just making
PendingTask heap allocated. We wouldn't have to heap allocate TaskClosureAdapter
then.

I haven't completely vetted the code to see if this is possible. But offhand, it
looks possible.

[awong, 2011/07/14 23:03:30]

I'm not quite sure how this would work.  The reason we need it refcounted is to
allow Bind() to create something that PendingTask can hold in a base::Closure. 
If we made PendingClosure heap allocated, that might help in avoiding the need
to do the weird destructive handoff, but I don't think it actually saves us from
refcounting TaskClosureAdapter.

[willchan no longer on Chromium, 2011/07/19 12:18:26]

I started writing out how this would work, but the benefits aren't a clear win,
and this is code that hopefully will die in Not Too Long (TM). So I'm fine with
not changing things.

       from_here,
       CalculateDelayedRuntime(0), true);
   AddToIncomingQueue(&pending_task);
@@ -282,8 +249,9 @@ void MessageLoop::PostDelayedTask(
     const tracked_objects::Location& from_here, Task* task, int64 delay_ms) {
   CHECK(task);
   PendingTask pending_task(
-      base::Bind(&TaskClosureAdapter::Run,
-                 new TaskClosureAdapter(task, &should_leak_tasks_)),
+      base::Bind(
+          &base::subtle::TaskClosureAdapter::Run,
+          new base::subtle::TaskClosureAdapter(task, &should_leak_tasks_)),
       from_here,
       CalculateDelayedRuntime(delay_ms), true);
   AddToIncomingQueue(&pending_task);
@@ -293,8 +261,9 @@ void MessageLoop::PostNonNestableTask(
     const tracked_objects::Location& from_here, Task* task) {
   CHECK(task);
   PendingTask pending_task(
-      base::Bind(&TaskClosureAdapter::Run,
-                 new TaskClosureAdapter(task, &should_leak_tasks_)),
+      base::Bind(
+          &base::subtle::TaskClosureAdapter::Run,
+          new base::subtle::TaskClosureAdapter(task, &should_leak_tasks_)),
       from_here,
       CalculateDelayedRuntime(0), false);
   AddToIncomingQueue(&pending_task);
@@ -304,8 +273,9 @@ void MessageLoop::PostNonNestableDelayedTask(
     const tracked_objects::Location& from_here, Task* task, int64 delay_ms) {
   CHECK(task);
   PendingTask pending_task(
-      base::Bind(&TaskClosureAdapter::Run,
-                 new TaskClosureAdapter(task, &should_leak_tasks_)),
+      base::Bind(
+          &base::subtle::TaskClosureAdapter::Run,
+          new base::subtle::TaskClosureAdapter(task, &should_leak_tasks_)),
       from_here,
       CalculateDelayedRuntime(delay_ms), false);
   AddToIncomingQueue(&pending_task);
@@ -486,11 +456,9 @@ void MessageLoop::RunTask(const PendingTask& pending_task) {
                     DidProcessTask(pending_task.time_posted));
 
 #if defined(TRACK_ALL_TASK_OBJECTS)
-  if (tracked_objects::ThreadData::IsActive() && pending_task.post_births) {
-    tracked_objects::ThreadData::current()->TallyADeath(
-        *pending_task.post_births,
-        TimeTicks::Now() - pending_task.time_posted);
-  }
+  tracked_objects::ThreadData::TallyADeathIfActive(
+      pending_task.post_births,
+      TimeTicks::Now() - pending_task.time_posted);
 #endif  // defined(TRACK_ALL_TASK_OBJECTS)
 
   nestable_tasks_allowed_ = true;
@@ -780,16 +748,7 @@ MessageLoop::PendingTask::PendingTask(
       nestable(nestable),
       birth_program_counter(posted_from.program_counter()) {
 #if defined(TRACK_ALL_TASK_OBJECTS)
-  if (tracked_objects::ThreadData::IsActive()) {
-    tracked_objects::ThreadData* current_thread_data =
-        tracked_objects::ThreadData::current();
-    if (current_thread_data) {
-      post_births = current_thread_data->TallyABirth(posted_from);
-    } else {
-      // Shutdown started, and this thread wasn't registered.
-      post_births = NULL;
-    }
-  }
+  post_births = tracked_objects::ThreadData::TallyABirthIfActive(posted_from);
 #endif  // defined(TRACK_ALL_TASK_OBJECTS)
 }