Add "Testing Components Which Post Tasks" docs

This is the complimentary testing docs to the Threading and Tasks usage
docs.

Updated in-code comments to reflect the current reality better too.

R=fdoray@chromium.org

Bug: 1002654
Change-Id: Ic9f4227e462ddc7180a1a6f3005935f1203ff064
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1798985
Auto-Submit: Gabriel Charette <gab@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697592}

base/test/null_task_runner.h

@@ -12,12 +12,11 @@
 
 namespace base {
 
-// ATTENTION: Prefer TaskEnvironment::ThreadPoolExecutionMode::QUEUED and
-// a task runner obtained from base/task/post_task.h over this class. A
-// NullTaskRunner might seem appealing, but not running tasks is under-testing
-// the side-effects of the code under tests. ThreadPoolExecutionMode::QUEUED
-// will delay execution until the end of the test (if not requested earlier) but
-// will at least exercise the tasks posted as a side-effect of the test.
+// ATTENTION: Prefer SingleThreadTaskEnvironment or TaskEnvironment w/
+// ThreadPoolExecutionMode::QUEUED over this class. A NullTaskRunner might seem
+// appealing, but not running tasks is under-testing the potential side-effects
+// of the code under tests. All tests should be okay if tasks born from their
+// actions are run or deleted at a later point.
 //
 // Helper class for tests that need to provide an implementation of a
 // *TaskRunner class but don't actually care about tasks being run.

base/test/scoped_mock_time_message_loop_task_runner.h

@@ -22,9 +22,9 @@ class SingleThreadTaskRunner;
 // ScopedMockTimeMessageLoopTaskRunner, the underlying TestMockTimeTaskRunner's
 // methods must be used instead to pump tasks.
 //
-// DEPRECATED: Use a TestMockTimeTaskRunner::Type::kBoundToThread instead of a
-// MessageLoop + ScopedMockTimeMessageLoopTaskRunner.
-// TODO(gab): Remove usage of this API and delete it.
+// Note: Use TaskEnvironment + TimeSource::MOCK_TIME instead of this in unit
+// tests. In browser tests you unfortunately still need this at the moment to
+// mock delayed tasks on the main thread...
 class ScopedMockTimeMessageLoopTaskRunner {
  public:
   ScopedMockTimeMessageLoopTaskRunner();

base/test/test_simple_task_runner.h

@@ -20,10 +20,10 @@ class TimeDelta;
 
 // ATTENTION: Prefer using base::test::TaskEnvironment and a task runner
 // obtained from base/task/post_task.h over this class. This class isn't as
-// "simple" as it seems specifically because its attempt at being simple makes
-// it run tasks in a surprising order (delays aren't respected and nesting
-// doesn't behave as usual). Should you prefer to flush all tasks regardless of
-// delays, TaskEnvironment::TimeSource::MOCK_TIME and
+// "simple" as it seems specifically because it runs tasks in a surprising order
+// (delays aren't respected and nesting doesn't behave as usual). Should you
+// prefer to flush all tasks regardless of delays,
+// TaskEnvironment::TimeSource::MOCK_TIME and
 // TaskEnvironment::FastForwardUntilNoTasksRemain() have you covered.
 //
 // TestSimpleTaskRunner is a simple TaskRunner implementation that can

base/threading/sequenced_task_runner_handle.cc

@@ -23,8 +23,10 @@ LazyInstance<ThreadLocalPointer<SequencedTaskRunnerHandle>>::Leaky
 const scoped_refptr<SequencedTaskRunner>& SequencedTaskRunnerHandle::Get() {
   const SequencedTaskRunnerHandle* current =
       sequenced_task_runner_tls.Pointer()->Get();
-  CHECK(current) << "Error: This caller requires a sequenced context (i.e. the "
-                    "current task needs to run from a SequencedTaskRunner).";
+  CHECK(current)
+      << "Error: This caller requires a sequenced context (i.e. the current "
+         "task needs to run from a SequencedTaskRunner). If you're in a test "
+         "refer to //docs/threading_and_tasks_testing.md.";
   return current->task_runner_;
 }
 

base/threading/thread_task_runner_handle.cc

@@ -26,9 +26,10 @@ base::LazyInstance<base::ThreadLocalPointer<ThreadTaskRunnerHandle>>::Leaky
 const scoped_refptr<SingleThreadTaskRunner>& ThreadTaskRunnerHandle::Get() {
   const ThreadTaskRunnerHandle* current =
       thread_task_runner_tls.Pointer()->Get();
-  CHECK(current) << "Error: This caller requires a single-threaded context "
-                    "(i.e. the current task needs to run from a "
-                    "SingleThreadTaskRunner).";
+  CHECK(current)
+      << "Error: This caller requires a single-threaded context (i.e. the "
+         "current task needs to run from a SingleThreadTaskRunner). If you're "
+         "in a test refer to //docs/threading_and_tasks_testing.md.";
   return current->task_runner_;
 }
 

docs/threading_and_tasks.md

@@ -38,8 +38,8 @@ This documentation assumes familiarity with computer science
    dedicated task queue until Quit(). You should pretty much never be creating
    your own `base::Thread`'s.
  * **Thread pool**: A pool of physical threads with a shared task queue. In
-   Chrome, this is `base::ThreadPoolInstance`. There's exactly one instance per Chrome
-   process, it serves tasks posted through
+   Chrome, this is `base::ThreadPoolInstance`. There's exactly one instance per
+   Chrome process, it serves tasks posted through
    [`base/task/post_task.h`](https://cs.chromium.org/chromium/src/base/task/post_task.h)
    and as such you should rarely need to use the `base::ThreadPoolInstance` API
    directly (more on posting tasks later).
@@ -625,11 +625,14 @@ cancelable_task_tracker.TryCancelAll();
 
 ## Testing
 
+For more details see [Testing Components Which Post
+Tasks](threading_and_tasks_testing.md).
+
 To test code that uses `base::ThreadTaskRunnerHandle`,
 `base::SequencedTaskRunnerHandle` or a function in
 [`base/task/post_task.h`](https://cs.chromium.org/chromium/src/base/task/post_task.h),
 instantiate a
-[`base::test::TaskEnvironment`](https://cs.chromium.org/chromium/src/base/test/scoped_task_environment.h)
+[`base::test::TaskEnvironment`](https://cs.chromium.org/chromium/src/base/test/task_environment.h)
 for the scope of the test. If you need BrowserThreads, use
 `content::BrowserTaskEnvironment` instead of
 `base::test::TaskEnvironment`.