Improve usage documentation of scoped task environments.

base/task_scheduler/scheduler_single_thread_task_runner_manager.cc

Index: base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
diff --git a/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc b/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
index 721e1daf76dc56d86959378db34ed561194fe84b..e6fba6fba7c237ed0f6e84bca3e00ac484fec208 100644
--- a/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
+++ b/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
@@ -11,6 +11,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/debug/stack_trace.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
@@ -284,6 +285,11 @@ class SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunner
 
  private:
   ~SchedulerSingleThreadTaskRunner() override {
+    // Note: this will crash if SchedulerSingleThreadTaskRunnerManager() is

[robliao, 2017/05/04 21:54:27]

Nit: Capitalize 'this'
Nit: No () as we're talking about the object itself.

[gab, 2017/05/04 22:27:41]

Done.

+    // incorrectly destroyed first in tests (in production the TaskScheduler and
+    // all of its state are intentionally leaked after
+    // TaskScheduler::Shutdown(). See ~SchedulerSingleThreadTaskRunnerManager()
+    // for more details.
     outer_->UnregisterSchedulerWorker(worker_);
   }
 
@@ -325,9 +331,25 @@ SchedulerSingleThreadTaskRunnerManager::
 #if DCHECK_IS_ON()
   size_t workers_unregistered_during_join =
       subtle::NoBarrier_Load(&workers_unregistered_during_join_);
-  DCHECK_EQ(workers_unregistered_during_join, workers_.size())
-      << "There cannot be outstanding SingleThreadTaskRunners upon destruction "
-         "of SchedulerSingleThreadTaskRunnerManager or the Task Scheduler";
+  // Log an ERROR instead of DCHECK'ing as it's often useful to have both the
+  // stack trace of this call and the crash stack trace of the upcoming
+  // out-of-order ~SchedulerSingleThreadTaskRunner() call to know what to flip.
+  DLOG_IF(ERROR, workers_unregistered_during_join != workers_.size())
+      << "Expect incoming crash in ~SchedulerSingleThreadTaskRunner()!!! There "
+         "cannot be outstanding SingleThreadTaskRunners upon destruction "
+         "of SchedulerSingleThreadTaskRunnerManager in tests "
+      << workers_.size() - workers_unregistered_during_join << " outstanding). "
+      << "Hint: If you're hitting this it's most likely because your test "

[robliao, 2017/05/04 21:54:28]

s/Hint/Hint 1/

[gab, 2017/05/04 22:27:41]

Done.

+         "fixture is destroying its TaskScheduler too early (e.g. via "
+         "~base::test::ScopedTaskEnvironment() or "

[robliao, 2017/05/04 21:54:27]

Nit: base::test::~ScopedTaskEnvironment() and
content::~TestBrowserThreadBundle()

[gab, 2017/05/04 22:27:41]

Done.

+         "~content::TestBrowserThreadBundle()). Refer to the following stack "
+         "trace to know what caused this destruction as well as to the "
+         "upcoming crash in ~SchedulerSingleThreadTaskRunner() to know what "
+         "should have happened before. "
+         "Hint hint: base::test::ScopedTaskEnvironment et al. should typically "

[robliao, 2017/05/04 21:54:27]

s/Hint hint/Hint 2/

[gab, 2017/05/04 22:27:41]

Done.

+         "be the first member in a test fixture to ensure it's initialized "
+         "first and destroyed last.\n"
+      << base::debug::StackTrace().ToString();
 #endif
 }