Implementation of PostTaskAndReply() in MessageLoopProxy and BrowserThread.

base/task_unittest.cc

Index: base/task_unittest.cc
diff --git a/base/task_unittest.cc b/base/task_unittest.cc
index e0cb659cf094c01db34730189989efef07a243d2..26397d3632fd6d00c9bd03f1998a01723c110ab6 100644
--- a/base/task_unittest.cc
+++ b/base/task_unittest.cc
@@ -2,8 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/memory/ref_counted.h"
 #include "base/task.h"
+
+#include "base/bind.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop.h"
+#include "base/threading/thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -107,4 +111,168 @@ TEST(TaskTest, TestScopedTaskRunnerManualRun) {
   EXPECT_TRUE(was_deleted);
 }
 
+class LoopRecorder : public base::RefCountedThreadSafe<LoopRecorder> {

[willchan no longer on Chromium, 2011/08/17 04:36:07]

I'm not sure why you make this RefCountedThreadSafe instead of just making this
a stack allocated variable and use base::Unretained(). Maybe that's my
anti-refcounting bias :P

[awong, 2011/08/17 05:16:00]

I figured that in the case that someone introduced a bug where it was deleted on
the wrong thread, this would at least ensure defined behavior.

As for why refcounted, I'm trying to detect when the Closure itself is deleted
so I need some sort of refcount in the bound argument.

+ public:
+  LoopRecorder(MessageLoop** run_on, MessageLoop** deleted_on)
+      : run_on_(run_on),
+        deleted_on_(deleted_on) {
+  }
+
+  void RecordRun() {
+    *run_on_ = MessageLoop::current();
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<LoopRecorder>;
+  ~LoopRecorder() {
+    *deleted_on_ = MessageLoop::current();
+  }
+
+  MessageLoop** run_on_;
+  MessageLoop** deleted_on_;
+};
+
+void RecordLoop(scoped_refptr<LoopRecorder> recorder) {
+  recorder->RecordRun();
+}
+
+void RecordLoopAndQuit(scoped_refptr<LoopRecorder> recorder) {
+  recorder->RecordRun();
+  MessageLoop::current()->Quit();
+}
+
+TEST(TaskTest, TestPostTaskAndReplyRelay_Basic) {
+  using base::internal::PostTaskAndReplyRelay;
+
+  MessageLoop current_loop;
+
+  MessageLoop* task_run_on = NULL;
+  MessageLoop* task_deleted_on = NULL;
+  MessageLoop* reply_run_on = NULL;
+  MessageLoop* reply_deleted_on = NULL;
+
+  scoped_refptr<LoopRecorder> task_recoder =

[willchan no longer on Chromium, 2011/08/17 04:36:07]

s/task_recoder/task_recorder/?

[awong, 2011/08/17 05:16:00]

Will fix tomorrow.

+      new LoopRecorder(&task_run_on, &task_deleted_on);
+  scoped_refptr<LoopRecorder> reply_recoder =
+      new LoopRecorder(&reply_run_on, &reply_deleted_on);
+
+  PostTaskAndReplyRelay* relay =
+      new PostTaskAndReplyRelay(
+          FROM_HERE,
+          base::Bind(&RecordLoop, task_recoder),
+          base::Bind(&RecordLoopAndQuit, reply_recoder));
+
+  // Die if base::Bind doesn't retain a reference to the recorders.
+  task_recoder = NULL;
+  reply_recoder = NULL;
+  ASSERT_FALSE(task_deleted_on);
+  ASSERT_FALSE(reply_deleted_on);
+
+  // Run the relay.
+  base::Thread task_thread("task_thread");
+  task_thread.Start();
+  task_thread.message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&PostTaskAndReplyRelay::Run, base::Unretained(relay)));
+
+  current_loop.Run();
+
+  EXPECT_EQ(task_thread.message_loop(), task_run_on);
+  EXPECT_EQ(&current_loop, task_deleted_on);
+  EXPECT_EQ(&current_loop, reply_run_on);
+  EXPECT_EQ(&current_loop, reply_deleted_on);
+}
+
+TEST(TaskTest, TestPostTaskAndReplyRelay_SameLoop) {
+  using base::internal::PostTaskAndReplyRelay;
+
+  MessageLoop current_loop;
+
+  MessageLoop* task_run_on = NULL;
+  MessageLoop* task_deleted_on = NULL;
+  MessageLoop* reply_run_on = NULL;
+  MessageLoop* reply_deleted_on = NULL;
+
+  scoped_refptr<LoopRecorder> task_recoder =
+      new LoopRecorder(&task_run_on, &task_deleted_on);
+  scoped_refptr<LoopRecorder> reply_recoder =
+      new LoopRecorder(&reply_run_on, &reply_deleted_on);
+
+  PostTaskAndReplyRelay* relay =
+      new PostTaskAndReplyRelay(
+          FROM_HERE,
+          base::Bind(&RecordLoop, task_recoder),
+          base::Bind(&RecordLoopAndQuit, reply_recoder));
+
+  // Die if base::Bind doesn't retain a reference to the recorders.
+  task_recoder = NULL;
+  reply_recoder = NULL;
+  ASSERT_FALSE(task_deleted_on);
+  ASSERT_FALSE(reply_deleted_on);
+
+  // Run the relay.
+  current_loop.PostTask(
+      FROM_HERE,
+      base::Bind(&PostTaskAndReplyRelay::Run, base::Unretained(relay)));
+
+  current_loop.Run();
+
+  EXPECT_EQ(&current_loop, task_run_on);
+  EXPECT_EQ(&current_loop, task_deleted_on);
+  EXPECT_EQ(&current_loop, reply_run_on);
+  EXPECT_EQ(&current_loop, reply_deleted_on);
+}
+
+TEST(TaskTest, TestPostTaskAndReplyRelay_DeadReplyLoopDoesNotDelete) {
+  using base::internal::PostTaskAndReplyRelay;
+
+  scoped_ptr<MessageLoop> current_loop(new MessageLoop());
+
+  MessageLoop* task_run_on = NULL;
+  MessageLoop* task_deleted_on = NULL;
+  MessageLoop* reply_run_on = NULL;
+  MessageLoop* reply_deleted_on = NULL;
+
+  scoped_refptr<LoopRecorder> task_recoder =
+      new LoopRecorder(&task_run_on, &task_deleted_on);
+  scoped_refptr<LoopRecorder> reply_recoder =
+      new LoopRecorder(&reply_run_on, &reply_deleted_on);
+
+  PostTaskAndReplyRelay* relay =
+      new PostTaskAndReplyRelay(
+          FROM_HERE,
+          base::Bind(&RecordLoop, task_recoder),
+          base::Bind(&RecordLoopAndQuit, reply_recoder));
+
+  // Die if base::Bind doesn't retain a reference to the recorders.
+  task_recoder = NULL;
+  reply_recoder = NULL;
+  ASSERT_FALSE(task_deleted_on);
+  ASSERT_FALSE(reply_deleted_on);
+
+  // Run the relay.
+  base::Thread task_thread("task_thread");
+  task_thread.Start();
+  task_thread.message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&PostTaskAndReplyRelay::Run, base::Unretained(relay)));
+
+  // Mercilessly whack the current loop before |reply| gets to run.
+  current_loop.reset();
+
+  // This should ensure the relay has been run.  We need to record the
+  // MessageLoop pointer before stopping the thread because Thread::Stop() will
+  // NULL out its own pointer.
+  MessageLoop* task_loop = task_thread.message_loop();
+  task_thread.Stop();
+
+  EXPECT_EQ(task_loop, task_run_on);
+  ASSERT_FALSE(task_deleted_on);
+  EXPECT_FALSE(reply_run_on);
+  ASSERT_FALSE(reply_deleted_on);
+
+  // Relay is leaked here.
+  // TODO(ajwong): Is there a way to make Valgrind not hate us?

[willchan no longer on Chromium, 2011/08/17 04:36:07]

delete relay; :)

[awong, 2011/08/17 05:16:00]

Tried that. Dies cause the Relay asserts that it is being deleted on the correct
thread.  I could remove that dcheck, but leaking seemed better. :(

+}
+
 }  // namespace