TaskScheduler: Move TaskFactory out of scheduler_thread_pool_unittest.cc

base/task_scheduler/test_task_factory.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/test_task_factory.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/synchronization/waitable_event.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace internal {
+
+TestTaskFactory::TestTaskFactory(scoped_refptr<TaskRunner> task_runner,
+                                 ExecutionMode execution_mode)
+    : cv_(&lock_),
+      task_runner_(std::move(task_runner)),
+      execution_mode_(execution_mode) {
+  // Detach |thread_checker_| from the current thread. It will be attached to
+  // the first thread that calls ThreadCheckerImpl::CalledOnValidThread().
+  thread_checker_.DetachFromThread();
+}
+
+TestTaskFactory::~TestTaskFactory() {
+  WaitForAllTasksToRun();
+}
+
+void TestTaskFactory::PostTask(PostNestedTask post_nested_task,
+                               WaitableEvent* event) {
+  AutoLock auto_lock(lock_);
+  EXPECT_TRUE(task_runner_->PostTask(

[robliao, 2016/04/21 01:38:17]

Given that this is a utility, EXPECT_TRUE is probably less appropriate. 

The failure modes below also deserve reconsideration. It might be useful to
allow consumers to specify options on what failure modes to report and what to
check.

[fdoray, 2016/04/22 15:54:03]

Moved the EXPECT_TRUE to callers.

Do you suggest that we provide a way for callers to enable/disable checks done
in RunTaskCallback? I don't think this is useful since all checks would be
enabled all the time.

We could set booleans when conditions are not met (e.g. when a task runs more
than once) and let users of TestTaskFactory set expectations for the values of
these booleans, but gab@ said that less state makes the class easier to read.

[robliao, 2016/04/22 23:04:19]

The overarching question here is would consumers of the util expect tests to
fail due to internal workings of this class. 

Since this is our util class, we can make that decision. I tend to like to see
the failures within the unit test file as it makes it easier to see how things
can fail within the file, but this is just personal preference. If we have a
really good reason for enforcing this upon all consumers, then so be it.

[fdoray, 2016/04/25 14:25:01]

I added comments in the header file to document the failures that can be
generated by this class. If I moved the failures to the test file, there would
be duplication between the thread pool and task scheduler tests, which is this
CL tries to avoid.

+      FROM_HERE,
+      Bind(&TestTaskFactory::RunTaskCallback, Unretained(this),
+           num_posted_tasks_++, post_nested_task, Unretained(event))));
+}
+
+void TestTaskFactory::WaitForAllTasksToRun() const {
+  AutoLock auto_lock(lock_);
+  while (ran_tasks_.size() < num_posted_tasks_)
+    cv_.Wait();
+}
+
+void TestTaskFactory::RunTaskCallback(size_t task_index,
+                                      PostNestedTask post_nested_task,
+                                      WaitableEvent* event) {
+  if (post_nested_task == PostNestedTask::YES)
+    PostTask(PostNestedTask::NO, nullptr);
+
+  EXPECT_TRUE(task_runner_->RunsTasksOnCurrentThread());
+
+  {
+    AutoLock auto_lock(lock_);
+
+    if (task_index >= num_posted_tasks_)
+      ADD_FAILURE() << "A task with an invalid index ran.";
+
+    if ((execution_mode_ == ExecutionMode::SINGLE_THREADED ||
+         execution_mode_ == ExecutionMode::SEQUENCED) &&
+        task_index != ran_tasks_.size()) {
+      ADD_FAILURE() << "A task didn't run in the expected order.";
+    }
+
+    if (execution_mode_ == ExecutionMode::SINGLE_THREADED)
+      EXPECT_TRUE(thread_checker_.CalledOnValidThread());
+
+    if (ran_tasks_.find(task_index) != ran_tasks_.end())
+      ADD_FAILURE() << "A task ran more than once.";
+    ran_tasks_.insert(task_index);
+
+    cv_.Signal();
+  }
+
+  if (event)
+    event->Wait();
+}
+
+}  // namespace internal
+}  // namespace base