Modernize unittest for cancelable tasks

1) Use own test fixture {CancelableTaskManagerTest}.
2) Avoid base::AtomicWord.
3) Use unique_ptr.
4) Fix order of expected and actual values in EXPECT_EQ.

R=mstarzinger@chromium.org

Bug: v8:8238
Change-Id: I3d29785864bbf3de58c5d9d5384b9e0065255e99
Reviewed-on: https://chromium-review.googlesource.com/c/1325967
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57378}

test/unittests/cancelable-tasks-unittest.cc

@@ -13,13 +13,14 @@ namespace internal {
 
 namespace {
 
+using ResultType = std::atomic<CancelableTaskManager::Id>;
+
 class TestTask : public Task, public Cancelable {
  public:
   enum Mode { kDoNothing, kWaitTillCanceledAgain, kCheckNotRun };
 
-  TestTask(CancelableTaskManager* parent, base::AtomicWord* result,
-           Mode mode = kDoNothing)
-      : Cancelable(parent), result_(result), mode_(mode) {}
+  TestTask(CancelableTaskManager* manager, ResultType* result, Mode mode)
+      : Cancelable(manager), result_(result), mode_(mode) {}
 
   // Task overrides.
   void Run() final {
@@ -30,7 +31,7 @@ class TestTask : public Task, public Cancelable {
 
  private:
   void RunInternal() {
-    base::Release_Store(result_, id());
+    result_->store(id());
 
     switch (mode_) {
       case kWaitTillCanceledAgain:
@@ -47,216 +48,194 @@ class TestTask : public Task, public Cancelable {
     }
   }
 
-  base::AtomicWord* result_;
-  Mode mode_;
+  ResultType* const result_;
+  const Mode mode_;
 };
 
-
 class SequentialRunner {
  public:
-  explicit SequentialRunner(TestTask* task) : task_(task) {}
+  explicit SequentialRunner(std::unique_ptr<TestTask> task)
+      : task_(std::move(task)), task_id_(task_->id()) {}
 
   void Run() {
     task_->Run();
-    delete task_;
+    task_.reset();
   }
 
+  CancelableTaskManager::Id task_id() const { return task_id_; }
+
  private:
-  TestTask* task_;
+  std::unique_ptr<TestTask> task_;
+  const CancelableTaskManager::Id task_id_;
 };
 
-
 class ThreadedRunner final : public base::Thread {
  public:
-  explicit ThreadedRunner(TestTask* task)
-      : Thread(Options("runner thread")), task_(task) {}
+  explicit ThreadedRunner(std::unique_ptr<TestTask> task)
+      : Thread(Options("runner thread")),
+        task_(std::move(task)),
+        task_id_(task_->id()) {}
 
   void Run() override {
     task_->Run();
-    delete task_;
+    task_.reset();
   }
 
+  CancelableTaskManager::Id task_id() const { return task_id_; }
+
  private:
-  TestTask* task_;
+  std::unique_ptr<TestTask> task_;
+  const CancelableTaskManager::Id task_id_;
 };
 
+class CancelableTaskManagerTest : public ::testing::Test {
+ public:
+  CancelableTaskManager* manager() { return &manager_; }
 
-typedef base::AtomicWord ResultType;
-
+  std::unique_ptr<TestTask> NewTask(
+      ResultType* result, TestTask::Mode mode = TestTask::kDoNothing) {
+    return base::make_unique<TestTask>(&manager_, result, mode);
+  }
 
-intptr_t GetValue(ResultType* result) { return base::Acquire_Load(result); }
+ private:
+  CancelableTaskManager manager_;
+};
 
 }  // namespace
 
-
-TEST(CancelableTask, EmptyCancelableTaskManager) {
-  CancelableTaskManager manager;
-  manager.CancelAndWait();
+TEST_F(CancelableTaskManagerTest, EmptyCancelableTaskManager) {
+  manager()->CancelAndWait();
 }
 
-
-TEST(CancelableTask, SequentialCancelAndWait) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  SequentialRunner runner1(
-      new TestTask(&manager, &result1, TestTask::kCheckNotRun));
-  EXPECT_EQ(GetValue(&result1), 0);
-  manager.CancelAndWait();
-  EXPECT_EQ(GetValue(&result1), 0);
-  runner1.Run();  // Run to avoid leaking the Task.
-  EXPECT_EQ(GetValue(&result1), 0);
+TEST_F(CancelableTaskManagerTest, SequentialCancelAndWait) {
+  ResultType result1{0};
+  SequentialRunner runner1(NewTask(&result1, TestTask::kCheckNotRun));
+  EXPECT_EQ(0u, result1);
+  manager()->CancelAndWait();
+  EXPECT_EQ(0u, result1);
+  runner1.Run();
+  EXPECT_EQ(0u, result1);
 }
 
-
-TEST(CancelableTask, SequentialMultipleTasks) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  ResultType result2 = 0;
-  TestTask* task1 = new TestTask(&manager, &result1);
-  TestTask* task2 = new TestTask(&manager, &result2);
-  SequentialRunner runner1(task1);
-  SequentialRunner runner2(task2);
-  EXPECT_EQ(task1->id(), 1u);
-  EXPECT_EQ(task2->id(), 2u);
-
-  EXPECT_EQ(GetValue(&result1), 0);
-  runner1.Run();  // Don't touch task1 after running it.
-  EXPECT_EQ(GetValue(&result1), 1);
-
-  EXPECT_EQ(GetValue(&result2), 0);
-  runner2.Run();  // Don't touch task2 after running it.
-  EXPECT_EQ(GetValue(&result2), 2);
-
-  manager.CancelAndWait();
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(1));
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(2));
+TEST_F(CancelableTaskManagerTest, SequentialMultipleTasks) {
+  ResultType result1{0};
+  ResultType result2{0};
+  SequentialRunner runner1(NewTask(&result1));
+  SequentialRunner runner2(NewTask(&result2));
+  EXPECT_EQ(1u, runner1.task_id());
+  EXPECT_EQ(2u, runner2.task_id());
+
+  EXPECT_EQ(0u, result1);
+  runner1.Run();
+  EXPECT_EQ(1u, result1);
+
+  EXPECT_EQ(0u, result2);
+  runner2.Run();
+  EXPECT_EQ(2u, result2);
+
+  manager()->CancelAndWait();
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(1));
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(2));
 }
 
-
-TEST(CancelableTask, ThreadedMultipleTasksStarted) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  ResultType result2 = 0;
-  TestTask* task1 =
-      new TestTask(&manager, &result1, TestTask::kWaitTillCanceledAgain);
-  TestTask* task2 =
-      new TestTask(&manager, &result2, TestTask::kWaitTillCanceledAgain);
-  ThreadedRunner runner1(task1);
-  ThreadedRunner runner2(task2);
+TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksStarted) {
+  ResultType result1{0};
+  ResultType result2{0};
+  ThreadedRunner runner1(NewTask(&result1, TestTask::kWaitTillCanceledAgain));
+  ThreadedRunner runner2(NewTask(&result2, TestTask::kWaitTillCanceledAgain));
   runner1.Start();
   runner2.Start();
   // Busy wait on result to make sure both tasks are done.
-  while ((GetValue(&result1) == 0) || (GetValue(&result2) == 0)) {
+  while (result1.load() == 0 || result2.load() == 0) {
   }
-  manager.CancelAndWait();
+  manager()->CancelAndWait();
   runner1.Join();
   runner2.Join();
-  EXPECT_EQ(GetValue(&result1), 1);
-  EXPECT_EQ(GetValue(&result2), 2);
+  EXPECT_EQ(1u, result1);
+  EXPECT_EQ(2u, result2);
 }
 
-
-TEST(CancelableTask, ThreadedMultipleTasksNotRun) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  ResultType result2 = 0;
-  TestTask* task1 = new TestTask(&manager, &result1, TestTask::kCheckNotRun);
-  TestTask* task2 = new TestTask(&manager, &result2, TestTask::kCheckNotRun);
-  ThreadedRunner runner1(task1);
-  ThreadedRunner runner2(task2);
-  manager.CancelAndWait();
+TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksNotRun) {
+  ResultType result1{0};
+  ResultType result2{0};
+  ThreadedRunner runner1(NewTask(&result1, TestTask::kCheckNotRun));
+  ThreadedRunner runner2(NewTask(&result2, TestTask::kCheckNotRun));
+  manager()->CancelAndWait();
   // Tasks are canceled, hence the runner will bail out and not update result.
   runner1.Start();
   runner2.Start();
   runner1.Join();
   runner2.Join();
-  EXPECT_EQ(GetValue(&result1), 0);
-  EXPECT_EQ(GetValue(&result2), 0);
+  EXPECT_EQ(0u, result1);
+  EXPECT_EQ(0u, result2);
 }
 
-
-TEST(CancelableTask, RemoveBeforeCancelAndWait) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  TestTask* task1 = new TestTask(&manager, &result1, TestTask::kCheckNotRun);
-  ThreadedRunner runner1(task1);
-  CancelableTaskManager::Id id = task1->id();
-  EXPECT_EQ(id, 1u);
-  EXPECT_EQ(TryAbortResult::kTaskAborted, manager.TryAbort(id));
+TEST_F(CancelableTaskManagerTest, RemoveBeforeCancelAndWait) {
+  ResultType result1{0};
+  ThreadedRunner runner1(NewTask(&result1, TestTask::kCheckNotRun));
+  CancelableTaskManager::Id id = runner1.task_id();
+  EXPECT_EQ(1u, id);
+  EXPECT_EQ(TryAbortResult::kTaskAborted, manager()->TryAbort(id));
   runner1.Start();
   runner1.Join();
-  manager.CancelAndWait();
-  EXPECT_EQ(GetValue(&result1), 0);
+  manager()->CancelAndWait();
+  EXPECT_EQ(0u, result1);
 }
 
-
-TEST(CancelableTask, RemoveAfterCancelAndWait) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  TestTask* task1 = new TestTask(&manager, &result1);
-  ThreadedRunner runner1(task1);
-  CancelableTaskManager::Id id = task1->id();
-  EXPECT_EQ(id, 1u);
+TEST_F(CancelableTaskManagerTest, RemoveAfterCancelAndWait) {
+  ResultType result1{0};
+  ThreadedRunner runner1(NewTask(&result1));
+  CancelableTaskManager::Id id = runner1.task_id();
+  EXPECT_EQ(1u, id);
   runner1.Start();
   runner1.Join();
-  manager.CancelAndWait();
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(id));
-  EXPECT_EQ(GetValue(&result1), 1);
+  manager()->CancelAndWait();
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(id));
+  EXPECT_EQ(1u, result1);
 }
 
-
-TEST(CancelableTask, RemoveUnmanagedId) {
-  CancelableTaskManager manager;
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(1));
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(2));
-  manager.CancelAndWait();
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(1));
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbort(3));
+TEST_F(CancelableTaskManagerTest, RemoveUnmanagedId) {
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(1));
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(2));
+  manager()->CancelAndWait();
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(1));
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbort(3));
 }
 
-TEST(CancelableTask, EmptyTryAbortAll) {
-  CancelableTaskManager manager;
-  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager.TryAbortAll());
+TEST_F(CancelableTaskManagerTest, EmptyTryAbortAll) {
+  EXPECT_EQ(TryAbortResult::kTaskRemoved, manager()->TryAbortAll());
 }
 
-TEST(CancelableTask, ThreadedMultipleTasksNotRunTryAbortAll) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  ResultType result2 = 0;
-  TestTask* task1 = new TestTask(&manager, &result1, TestTask::kCheckNotRun);
-  TestTask* task2 = new TestTask(&manager, &result2, TestTask::kCheckNotRun);
-  ThreadedRunner runner1(task1);
-  ThreadedRunner runner2(task2);
-  EXPECT_EQ(TryAbortResult::kTaskAborted, manager.TryAbortAll());
+TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksNotRunTryAbortAll) {
+  ResultType result1{0};
+  ResultType result2{0};
+  ThreadedRunner runner1(NewTask(&result1, TestTask::kCheckNotRun));
+  ThreadedRunner runner2(NewTask(&result2, TestTask::kCheckNotRun));
+  EXPECT_EQ(TryAbortResult::kTaskAborted, manager()->TryAbortAll());
   // Tasks are canceled, hence the runner will bail out and not update result.
   runner1.Start();
   runner2.Start();
   runner1.Join();
   runner2.Join();
-  EXPECT_EQ(GetValue(&result1), 0);
-  EXPECT_EQ(GetValue(&result2), 0);
+  EXPECT_EQ(0u, result1);
+  EXPECT_EQ(0u, result2);
 }
 
-TEST(CancelableTask, ThreadedMultipleTasksStartedTryAbortAll) {
-  CancelableTaskManager manager;
-  ResultType result1 = 0;
-  ResultType result2 = 0;
-  TestTask* task1 =
-      new TestTask(&manager, &result1, TestTask::kWaitTillCanceledAgain);
-  TestTask* task2 =
-      new TestTask(&manager, &result2, TestTask::kWaitTillCanceledAgain);
-  ThreadedRunner runner1(task1);
-  ThreadedRunner runner2(task2);
+TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksStartedTryAbortAll) {
+  ResultType result1{0};
+  ResultType result2{0};
+  ThreadedRunner runner1(NewTask(&result1, TestTask::kWaitTillCanceledAgain));
+  ThreadedRunner runner2(NewTask(&result2, TestTask::kWaitTillCanceledAgain));
   runner1.Start();
   // Busy wait on result to make sure task1 is done.
-  while (GetValue(&result1) == 0) {
+  while (result1.load() == 0) {
   }
-  EXPECT_EQ(TryAbortResult::kTaskRunning, manager.TryAbortAll());
+  EXPECT_EQ(TryAbortResult::kTaskRunning, manager()->TryAbortAll());
   runner2.Start();
   runner1.Join();
   runner2.Join();
-  EXPECT_EQ(GetValue(&result1), 1);
-  EXPECT_EQ(GetValue(&result2), 0);
+  EXPECT_EQ(1u, result1);
+  EXPECT_EQ(0u, result2);
 }
 
 }  // namespace internal