Clear PostTaskAndReply task on the destination thread (3)

base/message_loop/message_loop_task_runner_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/message_loop/message_loop_task_runner.h"
 
 #include <memory>
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
@@ skipping 90 matching lines @@
 StaticAtomicSequenceNumber MessageLoopTaskRunnerTest::g_order;
 
 TEST_F(MessageLoopTaskRunnerTest, PostTaskAndReply_Basic) {
   MessageLoop* task_run_on = NULL;
   MessageLoop* task_deleted_on = NULL;
   int task_delete_order = -1;
   MessageLoop* reply_run_on = NULL;
   MessageLoop* reply_deleted_on = NULL;
   int reply_delete_order = -1;
 
-  scoped_refptr<LoopRecorder> task_recoder =
+  scoped_refptr<LoopRecorder> task_recorder =
       new LoopRecorder(&task_run_on, &task_deleted_on, &task_delete_order);
-  scoped_refptr<LoopRecorder> reply_recoder =
+  scoped_refptr<LoopRecorder> reply_recorder =
       new LoopRecorder(&reply_run_on, &reply_deleted_on, &reply_delete_order);
 
   ASSERT_TRUE(task_thread_.task_runner()->PostTaskAndReply(
-      FROM_HERE, Bind(&RecordLoop, task_recoder),
-      Bind(&RecordLoopAndQuit, reply_recoder)));
+      FROM_HERE, Bind(&RecordLoop, task_recorder),
+      Bind(&RecordLoopAndQuit, reply_recorder)));
 
   // Die if base::Bind doesn't retain a reference to the recorders.
-  task_recoder = NULL;
-  reply_recoder = NULL;
+  task_recorder = NULL;
+  reply_recorder = NULL;
   ASSERT_FALSE(task_deleted_on);
   ASSERT_FALSE(reply_deleted_on);
 
   UnblockTaskThread();
   RunLoop().Run();
 
   EXPECT_EQ(task_thread_.message_loop(), task_run_on);
-  EXPECT_EQ(current_loop_.get(), task_deleted_on);
+  EXPECT_EQ(task_thread_.message_loop(), task_deleted_on);
   EXPECT_EQ(current_loop_.get(), reply_run_on);
   EXPECT_EQ(current_loop_.get(), reply_deleted_on);
   EXPECT_LT(task_delete_order, reply_delete_order);
 }
 
 TEST_F(MessageLoopTaskRunnerTest, PostTaskAndReplyOnDeletedThreadDoesNotLeak) {
   MessageLoop* task_run_on = NULL;
   MessageLoop* task_deleted_on = NULL;
   int task_delete_order = -1;
   MessageLoop* reply_run_on = NULL;
   MessageLoop* reply_deleted_on = NULL;
   int reply_delete_order = -1;
 
-  scoped_refptr<LoopRecorder> task_recoder =
+  scoped_refptr<LoopRecorder> task_recorder =
       new LoopRecorder(&task_run_on, &task_deleted_on, &task_delete_order);
-  scoped_refptr<LoopRecorder> reply_recoder =
+  scoped_refptr<LoopRecorder> reply_recorder =
       new LoopRecorder(&reply_run_on, &reply_deleted_on, &reply_delete_order);
 
   // Grab a task runner to a dead MessageLoop.
   scoped_refptr<SingleThreadTaskRunner> task_runner =
       task_thread_.task_runner();
   UnblockTaskThread();
   task_thread_.Stop();
 
   ASSERT_FALSE(
-      task_runner->PostTaskAndReply(FROM_HERE, Bind(&RecordLoop, task_recoder),
-                                    Bind(&RecordLoopAndQuit, reply_recoder)));
+      task_runner->PostTaskAndReply(FROM_HERE, Bind(&RecordLoop, task_recorder),
+                                    Bind(&RecordLoopAndQuit, reply_recorder)));
 
   // The relay should have properly deleted its resources leaving us as the only
   // reference.
   EXPECT_EQ(task_delete_order, reply_delete_order);
-  ASSERT_TRUE(task_recoder->HasOneRef());
-  ASSERT_TRUE(reply_recoder->HasOneRef());
+  ASSERT_TRUE(task_recorder->HasOneRef());
+  ASSERT_TRUE(reply_recorder->HasOneRef());
 
   // Nothing should have run though.
   EXPECT_FALSE(task_run_on);
   EXPECT_FALSE(reply_run_on);
 }
 
 TEST_F(MessageLoopTaskRunnerTest, PostTaskAndReply_SameLoop) {
   MessageLoop* task_run_on = NULL;
   MessageLoop* task_deleted_on = NULL;
   int task_delete_order = -1;
   MessageLoop* reply_run_on = NULL;
   MessageLoop* reply_deleted_on = NULL;
   int reply_delete_order = -1;
 
-  scoped_refptr<LoopRecorder> task_recoder =
+  scoped_refptr<LoopRecorder> task_recorder =
       new LoopRecorder(&task_run_on, &task_deleted_on, &task_delete_order);
-  scoped_refptr<LoopRecorder> reply_recoder =
+  scoped_refptr<LoopRecorder> reply_recorder =
       new LoopRecorder(&reply_run_on, &reply_deleted_on, &reply_delete_order);
 
   // Enqueue the relay.
   ASSERT_TRUE(current_loop_->task_runner()->PostTaskAndReply(
-      FROM_HERE, Bind(&RecordLoop, task_recoder),
-      Bind(&RecordLoopAndQuit, reply_recoder)));
+      FROM_HERE, Bind(&RecordLoop, task_recorder),
+      Bind(&RecordLoopAndQuit, reply_recorder)));
 
   // Die if base::Bind doesn't retain a reference to the recorders.
-  task_recoder = NULL;
-  reply_recoder = NULL;
+  task_recorder = NULL;
+  reply_recorder = NULL;
   ASSERT_FALSE(task_deleted_on);
   ASSERT_FALSE(reply_deleted_on);
 
   RunLoop().Run();
 
   EXPECT_EQ(current_loop_.get(), task_run_on);
   EXPECT_EQ(current_loop_.get(), task_deleted_on);
   EXPECT_EQ(current_loop_.get(), reply_run_on);
   EXPECT_EQ(current_loop_.get(), reply_deleted_on);
   EXPECT_LT(task_delete_order, reply_delete_order);
 }
 
 TEST_F(MessageLoopTaskRunnerTest, PostTaskAndReply_DeadReplyLoopDoesNotDelete) {

[gab, 2017/01/31 19:34:59]

Rename

[dcheng, 2017/02/07 07:28:44]

Per gab's comment, how about naming this:
PostTaskAndReply_DeadReplyTaskRunnerBehavior

[tzik, 2017/02/07 08:05:34]

Done.

[tzik, 2017/02/07 08:05:34]

Acknowledged.

[tzik, 2017/02/07 08:33:26]

Done.

   // Annotate the scope as having memory leaks to suppress heapchecker reports.
   ANNOTATE_SCOPED_MEMORY_LEAK;
   MessageLoop* task_run_on = NULL;
   MessageLoop* task_deleted_on = NULL;
   int task_delete_order = -1;
   MessageLoop* reply_run_on = NULL;
   MessageLoop* reply_deleted_on = NULL;
   int reply_delete_order = -1;
 
-  scoped_refptr<LoopRecorder> task_recoder =
+  scoped_refptr<LoopRecorder> task_recorder =
       new LoopRecorder(&task_run_on, &task_deleted_on, &task_delete_order);
-  scoped_refptr<LoopRecorder> reply_recoder =
+  scoped_refptr<LoopRecorder> reply_recorder =
       new LoopRecorder(&reply_run_on, &reply_deleted_on, &reply_delete_order);
 
   // Enqueue the relay.
   task_thread_.task_runner()->PostTaskAndReply(
-      FROM_HERE, Bind(&RecordLoop, task_recoder),
-      Bind(&RecordLoopAndQuit, reply_recoder));
+      FROM_HERE, Bind(&RecordLoop, task_recorder),
+      Bind(&RecordLoopAndQuit, reply_recorder));
 
   // Die if base::Bind doesn't retain a reference to the recorders.
-  task_recoder = NULL;
-  reply_recoder = NULL;
+  task_recorder = NULL;
+  reply_recorder = NULL;
   ASSERT_FALSE(task_deleted_on);
   ASSERT_FALSE(reply_deleted_on);
 
   UnblockTaskThread();
 
   // 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);

[dcheng, 2017/02/07 07:28:44]

And add some comments here to explain the new behavior.

"Even if the reply task runner is already gone, the original task should already
be deleted. However, the reply which hasn't executed yet should leak to avoid
thread-safety issues."

[tzik, 2017/02/07 08:05:34]

Acknowledged.

[tzik, 2017/02/07 08:33:26]

Done.

-  ASSERT_FALSE(task_deleted_on);
+  EXPECT_EQ(task_loop, task_deleted_on);
   EXPECT_FALSE(reply_run_on);
   ASSERT_FALSE(reply_deleted_on);
-  EXPECT_EQ(task_delete_order, reply_delete_order);
 
   // The PostTaskAndReplyRelay is leaked here.  Even if we had a reference to
   // it, we cannot just delete it because PostTaskAndReplyRelay's destructor
   // checks that MessageLoop::current() is the the same as when the
   // PostTaskAndReplyRelay object was constructed.  However, this loop must have
   // already been deleted in order to perform this test.  See
   // http://crbug.com/86301.
 }
 
 class MessageLoopTaskRunnerThreadingTest : public testing::Test {
@@ skipping 98 matching lines @@
         new Thread("MessageLoopTaskRunnerThreadingTest_Dummy"));
     test_thread->Start();
     task_runner = test_thread->task_runner();
   }
   bool ret = task_runner->PostTask(
       FROM_HERE, Bind(&MessageLoopTaskRunnerThreadingTest::AssertNotRun));
   EXPECT_FALSE(ret);
 }
 
 }  // namespace base