Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.

base/task_scheduler/scheduler_worker.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/scheduler_worker.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 183 matching lines @@
   if (sleep_time.is_max()) {
     // Calling TimedWait with TimeDelta::Max is not recommended per
     // http://crbug.com/465948.
     wake_up_event->Wait();
   } else {
     wake_up_event->TimedWait(sleep_time);
   }
   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() {
   AutoSchedulerLock auto_lock(thread_lock_);
 
   DCHECK(!join_called_for_testing_.IsSet());
 
   if (!thread_)
     CreateThreadAssertSynchronized();
 
@@ skipping 26 matching lines @@
   AutoSchedulerLock auto_lock(thread_lock_);
   return !!thread_;
 }
 
 void SchedulerWorker::Cleanup() {
   // |should_exit_| is synchronized with |thread_| for writes here so that we
   // can maintain access to |thread_| for wakeup. Otherwise, the thread may take
   // 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() {
   // It is unexpected for |thread_| to be alive and for SchedulerWorker to
   // destroy since SchedulerWorker owns the delegate needed by |thread_|.
   // For testing, this generally means JoinForTesting was not called.
   DCHECK(!thread_);
 }
 
 std::unique_ptr<SchedulerWorker::Thread> SchedulerWorker::DetachThreadObject(
@@ skipping 14 matching lines @@
 
   if (detach_notify == DetachNotify::DELEGATE) {
     // Call OnDetach() within the scope of |thread_lock_| to prevent the
     // delegate from being used concurrently from an old and a new thread.
     delegate_->OnDetach();
   }
 
   return std::move(thread_);
 }
 
 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() {
   thread_lock_.AssertAcquired();
   CreateThread();
 }
 
 bool SchedulerWorker::ShouldExit() {
   // The ordering of the checks is important below. This SchedulerWorker may be
   // released and outlive |task_tracker_| in unit tests. However, when the
   // SchedulerWorker is released, |should_exit_| will be set, so check that
   // first.
   return should_exit_.IsSet() || join_called_for_testing_.IsSet() ||
          task_tracker_->IsShutdownComplete();
 }
 
 }  // namespace internal
 }  // namespace base