Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.

base/task_scheduler/scheduler_worker.cc

Index: base/task_scheduler/scheduler_worker.cc
diff --git a/base/task_scheduler/scheduler_worker.cc b/base/task_scheduler/scheduler_worker.cc
index 4084c527a73c17e6ba4fa80398e792b1c268566b..997aed486976ecc9f6e91a5aae9be5a460f1f5d8 100644
--- a/base/task_scheduler/scheduler_worker.cc
+++ b/base/task_scheduler/scheduler_worker.cc
@@ -201,25 +201,39 @@ void SchedulerWorker::Delegate::WaitForWork(WaitableEvent* wake_up_event) {
   wake_up_event->Reset();
 }
 
-scoped_refptr<SchedulerWorker> SchedulerWorker::Create(
+SchedulerWorker::SchedulerWorker(
     ThreadPriority priority_hint,
     std::unique_ptr<Delegate> delegate,
     TaskTracker* task_tracker,
-    InitialState initial_state,
-    SchedulerBackwardCompatibility backward_compatibility) {
-  scoped_refptr<SchedulerWorker> worker(
-      new SchedulerWorker(priority_hint, std::move(delegate), task_tracker,
-                          backward_compatibility));
-  // Creation happens before any other thread can reference this one, so no
-  // synchronization is necessary.
-  if (initial_state == SchedulerWorker::InitialState::ALIVE) {
-    worker->CreateThread();
-    if (!worker->thread_) {
-      return nullptr;
-    }
+    SchedulerBackwardCompatibility backward_compatibility)
+    : priority_hint_(priority_hint),
+      delegate_(std::move(delegate)),
+      task_tracker_(task_tracker)
+#if defined(OS_WIN)
+      ,
+      backward_compatibility_(backward_compatibility)
+#endif
+{
+  DCHECK(delegate_);
+  DCHECK(task_tracker_);
+}
+
+bool SchedulerWorker::Start(InitialState initial_state) {
+  AutoSchedulerLock auto_lock(thread_lock_);
+  DCHECK(!started_);
+  DCHECK(!thread_);
+
+  if (should_exit_.IsSet() || join_called_for_testing_.IsSet())
+    return true;
+
+  started_ = true;
+
+  if (initial_state == InitialState::ALIVE) {
+    CreateThreadAssertSynchronized();
+    return !!thread_;
   }
 
-  return worker;
+  return true;
 }
 
 void SchedulerWorker::WakeUp() {
@@ -266,27 +280,9 @@ void SchedulerWorker::Cleanup() {
   // away |thread_| for destruction.
   AutoSchedulerLock auto_lock(thread_lock_);
   DCHECK(!should_exit_.IsSet());
-  if (thread_) {
-    should_exit_.Set();
+  should_exit_.Set();
+  if (thread_)
     thread_->WakeUp();
-  }
-}
-
-SchedulerWorker::SchedulerWorker(
-    ThreadPriority priority_hint,
-    std::unique_ptr<Delegate> delegate,
-    TaskTracker* task_tracker,
-    SchedulerBackwardCompatibility backward_compatibility)
-    : priority_hint_(priority_hint),
-      delegate_(std::move(delegate)),
-      task_tracker_(task_tracker)
-#if defined(OS_WIN)
-      ,
-      backward_compatibility_(backward_compatibility)
-#endif
-{
-  DCHECK(delegate_);
-  DCHECK(task_tracker_);
 }
 
 SchedulerWorker::~SchedulerWorker() {
@@ -322,7 +318,8 @@ std::unique_ptr<SchedulerWorker::Thread> SchedulerWorker::DetachThreadObject(
 }
 
 void SchedulerWorker::CreateThread() {

[gab, 2017/04/10 18:04:56]

Last caller of this is now CreateThreadAssertSynchronized(), merge into this one
and always assert lock acquired?

(even more so since |started_| isn't thread-safe)

[fdoray, 2017/04/10 19:05:00]

Done.

-  thread_ = Thread::Create(make_scoped_refptr(this));
+  if (started_)
+    thread_ = Thread::Create(make_scoped_refptr(this));
 }
 
 void SchedulerWorker::CreateThreadAssertSynchronized() {