Implementation of PostTaskAndReply() in MessageLoopProxy and BrowserThread.

base/task.h

Index: base/task.h
diff --git a/base/task.h b/base/task.h
index 080c150dd3e51fe61d243892b3be91a5ee773631..e01c955d33da00874d35f4e234e65f339a0420af 100644
--- a/base/task.h
+++ b/base/task.h
@@ -14,8 +14,11 @@
 #include "base/tracked.h"
 #include "base/tuple.h"
 
+class MessageLoop;
+
 namespace base {
 const size_t kDeadTask = 0xDEAD7A53;
+class MessageLoopProxy;
 }
 
 // Task ------------------------------------------------------------------------
@@ -567,36 +570,49 @@ class BASE_API ScopedTaskRunner {
 
 namespace internal {
 
-class PostTaskAndReplyRelay {
+// This relay class remembers the MessageLoop that it was created on, and
+// ensures that both the |task| and |reply| Closures are deleted on this same
+// thread.
+//
+// If this is not possible because the originating MessageLoop is no longer
+// available, the the |task| and |reply| Closures are leaked.  Leaking is
+// considered preferable to having a thread-safetey violations caused by
+// invoking the Closure destructor on the wrong thread.
+class PostTaskAndReplyRelay :
+  public RefCountedThreadSafe<PostTaskAndReplyRelay, ManualDestruction> {
  public:
-  PostTaskAndReplyRelay(const Closure& task, const Closure& reply);
+  PostTaskAndReplyRelay(const tracked_objects::Location& from_here,
+                        const Closure& task, const Closure& reply);
 
   ~PostTaskAndReplyRelay();
 
   void Run();
 
  private:
+  // An alternate implementation would just call "delete this;" inside
+  // RunReplyAndSelfDestruct() instead of using refcounts with
+  // ManualDestruction.  While this works mostly, it misses the case where the
+  // target MessageLoop is in shutdown, but the origin MessageLoop is not.

[willchan no longer on Chromium, 2011/07/06 22:10:26]

I think this is unnecessarily complicated and this does not buy us anything. If
|task| does not run, then I think the behavior is undefined anyway. Allowing
|task| to be destroyed (the destructor of a Closure may have behavior) even when
not Run() may cause its own problems (it's possible destroying the Closure may
release the last reference which invokes a destructor which expects that the
task has run, and if not, crashes). If you need defined behavior in the presence
of shutdown, you need to code something up on your own.

[awong, 2011/08/15 22:07:55]

Okay, went back to simple delete.

+  //
+  // Using a refcount allows us to still properly delete the relay object even
+  // if PostTaskAndReplyRelay::Run() is never executed.
+  friend class base::ManualDestruction;
+  static void DoManualDestruction(const PostTaskAndReplyRelay* obj);
+
   void RunReplyAndSelfDestruct();
 
-  base::Closure task_;
-  base::Closure reply_;
+  tracked_objects::Location from_here_;
+  scoped_refptr<MessageLoopProxy> origin_loop_;
+  Closure task_;
+  Closure reply_;
 };
-}  // namespace internal
 
-template <typename MessageLoopType, typename ResponseType>
-void PostTaskAndReply(MessageLoopType loop, const Closure& task,
-                      const Closure& reply) {
-  PostTaskAndReplyRelay* relay = new PostTaskAndReplyRelay(task, reply);
-  loop.PostTask(&PostTaskAndReplyRelay::Run, base::Unretained(relay));
-}
-
-template <typename MessageLoopType, typename ResponseType>
-void PostTaskAndReply<MessageLoopType, ResponseType>(
-    MessageLoopType* loop, const Closure& task, const Closure& reply) {
-  PostTaskAndReplyRelay* relay = new PostTaskAndReplyRelay(task, reply);
-  loop->PostTask(&PostTaskAndReplyRelay::Run, base::Unretained(relay));
-}
+}  // namespace internal
 
+void PostTaskAndReply(MessageLoop* loop,
+                      const tracked_objects::Location& from_here,
+                      const Closure& task,
+                      const Closure& reply);
 }  // namespace base
 
 #endif  // BASE_TASK_H_