TaskScheduler: Don't use a callback to post a task from TaskTracker.

base/task_scheduler/task_tracker_unittest.cc

 // Copyright 2016 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/task_scheduler/task_tracker.h"
 
 #include <memory>
-#include <queue>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/task.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/task_scheduler/test_utils.h"
 #include "base/threading/platform_thread.h"
@@ skipping 49 matching lines @@
   TaskSchedulerTaskTrackerTest() = default;
 
   // Creates a task with |shutdown_behavior|.
   std::unique_ptr<Task> CreateTask(TaskShutdownBehavior shutdown_behavior) {
     return WrapUnique(new Task(
         FROM_HERE,
         Bind(&TaskSchedulerTaskTrackerTest::RunTaskCallback, Unretained(this)),
         TaskTraits().WithShutdownBehavior(shutdown_behavior)));
   }
 
-  // Tries to post |task| via |tracker_|. If |tracker_| approves the operation,
-  // |task| is added to |posted_tasks_|.
-  void PostTaskViaTracker(std::unique_ptr<Task> task) {
-    tracker_.PostTask(
-        Bind(&TaskSchedulerTaskTrackerTest::PostTaskCallback, Unretained(this)),
-        std::move(task));
-  }
-
-  // Tries to run the next task in |posted_tasks_| via |tracker_|.
-  void RunNextPostedTaskViaTracker() {
-    ASSERT_FALSE(posted_tasks_.empty());
-    tracker_.RunTask(posted_tasks_.front().get());
-    posted_tasks_.pop();
-  }
-
   // Calls tracker_->Shutdown() on a new thread. When this returns, Shutdown()
   // method has been entered on the new thread, but it hasn't necessarily
   // returned.
   void CallShutdownAsync() {
     ASSERT_FALSE(thread_calling_shutdown_);
     thread_calling_shutdown_.reset(new ThreadCallingShutdown(&tracker_));
     thread_calling_shutdown_->Start();
     while (!tracker_.IsShuttingDownForTesting() &&
            !tracker_.shutdown_completed()) {
       PlatformThread::YieldCurrentThread();
     }
   }
 
   void WaitForAsyncShutdownCompleted() {
     ASSERT_TRUE(thread_calling_shutdown_);
     thread_calling_shutdown_->Join();
     EXPECT_TRUE(thread_calling_shutdown_->has_returned());
     EXPECT_TRUE(tracker_.shutdown_completed());
   }
 
   void VerifyAsyncShutdownInProgress() {
     ASSERT_TRUE(thread_calling_shutdown_);
     EXPECT_FALSE(thread_calling_shutdown_->has_returned());
     EXPECT_FALSE(tracker_.shutdown_completed());
     EXPECT_TRUE(tracker_.IsShuttingDownForTesting());
   }
 
   TaskTracker tracker_;
   size_t num_tasks_executed_ = 0;
-  std::queue<std::unique_ptr<Task>> posted_tasks_;
 
  private:
-  void PostTaskCallback(std::unique_ptr<Task> task) {
-    posted_tasks_.push(std::move(task));
-  }
-
   void RunTaskCallback() { ++num_tasks_executed_; }
 
   std::unique_ptr<ThreadCallingShutdown> thread_calling_shutdown_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskSchedulerTaskTrackerTest);
 };
 
 #define WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED() \
   do {                                      \
     SCOPED_TRACE("");                       \
     WaitForAsyncShutdownCompleted();        \
   } while (false)
 
 #define VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS() \
   do {                                      \
     SCOPED_TRACE("");                       \
     VerifyAsyncShutdownInProgress();        \
   } while (false)
 
 }  // namespace
 
 TEST_P(TaskSchedulerTaskTrackerTest, PostAndRunBeforeShutdown) {
-  std::unique_ptr<Task> task_to_post(CreateTask(GetParam()));
-  const Task* task_to_post_raw = task_to_post.get();
+  std::unique_ptr<Task> task(CreateTask(GetParam()));
 
   // Post the task.

[danakj, 2016/04/06 19:38:30]

update comment

[robliao, 2016/04/06 19:42:33]

Maybe update the test names too.

[fdoray, 2016/04/06 20:59:27]

Done.

-  EXPECT_TRUE(posted_tasks_.empty());
-  PostTaskViaTracker(std::move(task_to_post));
-  EXPECT_EQ(1U, posted_tasks_.size());
-  EXPECT_EQ(task_to_post_raw, posted_tasks_.front().get());
+  EXPECT_TRUE(tracker_.WillPostTask(task.get()));
 
   // Run the posted task.
   EXPECT_EQ(0U, num_tasks_executed_);
-  RunNextPostedTaskViaTracker();
+  tracker_.RunTask(task.get());
   EXPECT_EQ(1U, num_tasks_executed_);
 
   // Shutdown() shouldn't block.
   tracker_.Shutdown();
 }
 
 TEST_P(TaskSchedulerTaskTrackerTest, PostAndRunLongTaskBeforeShutdown) {
   // Post a task that will block until |event| is signaled.
-  EXPECT_TRUE(posted_tasks_.empty());
   WaitableEvent event(false, false);
-  PostTaskViaTracker(WrapUnique(
+  std::unique_ptr<Task> blocked_task(
       new Task(FROM_HERE, Bind(&WaitableEvent::Wait, Unretained(&event)),
-               TaskTraits().WithShutdownBehavior(GetParam()))));
-  EXPECT_EQ(1U, posted_tasks_.size());
+               TaskTraits().WithShutdownBehavior(GetParam())));
+  EXPECT_TRUE(tracker_.WillPostTask(blocked_task.get()));
 
   // Run the task asynchronouly.
-  ThreadRunningTask thread_running_task(&tracker_, posted_tasks_.front().get());
+  ThreadRunningTask thread_running_task(&tracker_, blocked_task.get());
   thread_running_task.Start();
 
   // Initiate shutdown while the task is running.
   CallShutdownAsync();
 
   if (GetParam() == TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN) {
     // Shutdown should complete even with a CONTINUE_ON_SHUTDOWN in progress.
     WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED();
   } else {
     // Shutdown should block with any non CONTINUE_ON_SHUTDOWN task in progress.
     VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS();
   }
 
   // Unblock the task.
   event.Signal();
   thread_running_task.Join();
 
   // Shutdown should now complete for a non CONTINUE_ON_SHUTDOWN task.
   if (GetParam() != TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
     WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED();
 }
 
 TEST_P(TaskSchedulerTaskTrackerTest, PostBeforeShutdownRunDuringShutdown) {
-  std::unique_ptr<Task> task_to_post(CreateTask(GetParam()));
-  const Task* task_to_post_raw = task_to_post.get();
-
-  // Post the task.
-  EXPECT_TRUE(posted_tasks_.empty());
-  PostTaskViaTracker(std::move(task_to_post));
-  EXPECT_EQ(1U, posted_tasks_.size());
-  EXPECT_EQ(task_to_post_raw, posted_tasks_.front().get());
+  // Post a task.

[danakj, 2016/04/06 19:38:30]

update comment?

[fdoray, 2016/04/06 20:59:27]

Done.

+  std::unique_ptr<Task> task(CreateTask(GetParam()));
+  EXPECT_TRUE(tracker_.WillPostTask(task.get()));
 
   // Post a BLOCK_SHUTDOWN task just to block shutdown.

[danakj, 2016/04/06 19:38:30]

and here, "Post" isn't strictly right.

[fdoray, 2016/04/06 20:59:27]

Done.

-  PostTaskViaTracker(CreateTask(TaskShutdownBehavior::BLOCK_SHUTDOWN));
+  std::unique_ptr<Task> block_shutdown_task(
+      CreateTask(TaskShutdownBehavior::BLOCK_SHUTDOWN));
+  EXPECT_TRUE(tracker_.WillPostTask(block_shutdown_task.get()));
 
   // Call Shutdown() asynchronously.
   CallShutdownAsync();
   VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS();
 
   // Try to run the first posted task. Only BLOCK_SHUTDOWN tasks should run,
   // others should be discarded.
   EXPECT_EQ(0U, num_tasks_executed_);
-  RunNextPostedTaskViaTracker();
+  tracker_.RunTask(task.get());
   EXPECT_EQ(GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN ? 1U : 0U,
             num_tasks_executed_);
   VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS();
 
   // Unblock shutdown by running the remaining BLOCK_SHUTDOWN task.
-  RunNextPostedTaskViaTracker();
+  tracker_.RunTask(block_shutdown_task.get());
   EXPECT_EQ(GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN ? 2U : 1U,
             num_tasks_executed_);
   WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED();
 }
 
 TEST_P(TaskSchedulerTaskTrackerTest, PostBeforeShutdownRunAfterShutdown) {
-  std::unique_ptr<Task> task_to_post(CreateTask(GetParam()));
-  const Task* task_to_post_raw = task_to_post.get();
-
-  // Post the task.
-  EXPECT_TRUE(posted_tasks_.empty());
-  PostTaskViaTracker(std::move(task_to_post));
-  EXPECT_EQ(1U, posted_tasks_.size());
-  EXPECT_EQ(task_to_post_raw, posted_tasks_.front().get());
+  // Post a task.

[robliao, 2016/04/06 19:42:33]

Same here.

[fdoray, 2016/04/06 20:59:27]

Done.

+  std::unique_ptr<Task> task(CreateTask(GetParam()));
+  EXPECT_TRUE(tracker_.WillPostTask(task.get()));
 
   // Call Shutdown() asynchronously.
   CallShutdownAsync();
   EXPECT_EQ(0U, num_tasks_executed_);
 
   if (GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN) {
     VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS();
 
     // Run the task to unblock shutdown.
-    RunNextPostedTaskViaTracker();
+    tracker_.RunTask(task.get());
     EXPECT_EQ(1U, num_tasks_executed_);
     WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED();
 
     // It is not possible to test running a BLOCK_SHUTDOWN task posted before
     // shutdown after shutdown because Shutdown() won't return if there are
     // pending BLOCK_SHUTDOWN tasks.
   } else {
     WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED();
 
     // The task shouldn't be allowed to run after shutdown.
-    RunNextPostedTaskViaTracker();
+    tracker_.RunTask(task.get());
     EXPECT_EQ(0U, num_tasks_executed_);
   }
 }
 
 TEST_P(TaskSchedulerTaskTrackerTest, PostAndRunDuringShutdown) {
   // Post a BLOCK_SHUTDOWN task just to block shutdown.
-  PostTaskViaTracker(CreateTask(TaskShutdownBehavior::BLOCK_SHUTDOWN));
-  std::unique_ptr<Task> block_shutdown_task = std::move(posted_tasks_.front());
-  posted_tasks_.pop();
+  std::unique_ptr<Task> block_shutdown_task(
+      CreateTask(TaskShutdownBehavior::BLOCK_SHUTDOWN));
+  EXPECT_TRUE(tracker_.WillPostTask(block_shutdown_task.get()));
 
   // Call Shutdown() asynchronously.
   CallShutdownAsync();
   VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS();
 
   if (GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN) {
     // Post a BLOCK_SHUTDOWN task. This should succeed.
-    EXPECT_TRUE(posted_tasks_.empty());
-    PostTaskViaTracker(CreateTask(GetParam()));
-    EXPECT_EQ(1U, posted_tasks_.size());
+    std::unique_ptr<Task> task(CreateTask(GetParam()));
+    EXPECT_TRUE(tracker_.WillPostTask(task.get()));
 
     // Run the BLOCK_SHUTDOWN task. This should succeed.
     EXPECT_EQ(0U, num_tasks_executed_);
-    RunNextPostedTaskViaTracker();
+    tracker_.RunTask(task.get());
     EXPECT_EQ(1U, num_tasks_executed_);
   } else {
     // It shouldn't be possible to post a non BLOCK_SHUTDOWN task.
-    PostTaskViaTracker(CreateTask(GetParam()));
-    EXPECT_TRUE(posted_tasks_.empty());
+    std::unique_ptr<Task> task(CreateTask(GetParam()));
+    EXPECT_FALSE(tracker_.WillPostTask(task.get()));
 
     // Don't try to run the task, because it hasn't been posted successfully.
   }
 
   // Unblock shutdown by running the BLOCK_SHUTDOWN task posted at the beginning
   // of the test.
   VERIFY_ASYNC_SHUTDOWN_IN_PROGRESS();
   tracker_.RunTask(block_shutdown_task.get());
   EXPECT_EQ(GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN ? 2U : 1U,
             num_tasks_executed_);
   WAIT_FOR_ASYNC_SHUTDOWN_COMPLETED();
 }
 
 TEST_P(TaskSchedulerTaskTrackerTest, PostAfterShutdown) {
   // It is not possible to post a task after shutdown.
   tracker_.Shutdown();
-  EXPECT_TRUE(posted_tasks_.empty());
+
+  std::unique_ptr<Task> task(CreateTask(GetParam()));
 
   if (GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN) {
-    EXPECT_DCHECK_DEATH({ PostTaskViaTracker(CreateTask(GetParam())); }, "");
+    EXPECT_DCHECK_DEATH({ tracker_.WillPostTask(task.get()); }, "");
   } else {
-    PostTaskViaTracker(CreateTask(GetParam()));
+    EXPECT_FALSE(tracker_.WillPostTask(task.get()));
   }
-
-  EXPECT_TRUE(posted_tasks_.empty());
 }
 
 INSTANTIATE_TEST_CASE_P(
     ContinueOnShutdown,
     TaskSchedulerTaskTrackerTest,
     ::testing::Values(TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN));
 INSTANTIATE_TEST_CASE_P(
     SkipOnShutdown,
     TaskSchedulerTaskTrackerTest,
     ::testing::Values(TaskShutdownBehavior::SKIP_ON_SHUTDOWN));
 INSTANTIATE_TEST_CASE_P(
     BlockShutdown,
     TaskSchedulerTaskTrackerTest,
     ::testing::Values(TaskShutdownBehavior::BLOCK_SHUTDOWN));
 
 }  // namespace internal
 }  // namespace base