Separate the create and start phases in SchedulerSingleThreadTaskRunnerManager.

base/task_scheduler/scheduler_worker_unittest.cc

Index: base/task_scheduler/scheduler_worker_unittest.cc
diff --git a/base/task_scheduler/scheduler_worker_unittest.cc b/base/task_scheduler/scheduler_worker_unittest.cc
index b8dea8e7454e5a41c432e49180e893e0e074d788..ac055c5162c06451194e0c8ef66cf4f75bc90f95 100644
--- a/base/task_scheduler/scheduler_worker_unittest.cc
+++ b/base/task_scheduler/scheduler_worker_unittest.cc
@@ -77,10 +77,11 @@ class TaskSchedulerWorkerTest : public testing::TestWithParam<size_t> {
                     WaitableEvent::InitialState::NOT_SIGNALED) {}
 
   void SetUp() override {
-    worker_ = SchedulerWorker::Create(
+    worker_ = make_scoped_refptr(new SchedulerWorker(
         ThreadPriority::NORMAL, MakeUnique<TestSchedulerWorkerDelegate>(this),
-        &task_tracker_, SchedulerWorker::InitialState::ALIVE);
+        &task_tracker_));
     ASSERT_TRUE(worker_);
+    worker_->Start();
     worker_set_.Signal();
     main_entry_called_.Wait();
   }
@@ -390,6 +391,10 @@ class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate {
 
     void WaitForDelegateDestroy() { destroyed_.Wait(); }
 
+    void set_expect_get_work(bool expect_get_work) {
+      expect_get_work_ = expect_get_work;
+    }
+
     void ResetState() {
       work_running_.Signal();
       work_processed_.Reset();
@@ -412,6 +417,7 @@ class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate {
     WaitableEvent can_detach_block_;
     WaitableEvent destroyed_;
 
+    bool expect_get_work_ = true;
     bool can_detach_ = false;
     bool work_requested_ = false;
 
@@ -425,6 +431,8 @@ class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate {
 
   scoped_refptr<Sequence> GetWork(SchedulerWorker* worker)
       override {
+    EXPECT_TRUE(controls_->expect_get_work_);
+
     // Sends one item of work to signal |work_processed_|. On subsequent calls,
     // sends nullptr to indicate there's no more work to be done.
     if (controls_->work_requested_)
@@ -501,9 +509,9 @@ TEST(TaskSchedulerWorkerTest, WorkerDetaches) {
       delegate->controls();
   controls->set_can_detach(true);
   EXPECT_CALL(*delegate, OnMainEntry(_));
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
   controls->WaitForWorkToRun();
   Mock::VerifyAndClear(delegate);
@@ -525,9 +533,9 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupBeforeDetach) {
   controls->set_can_detach(true);
   controls->MakeCanDetachBlock();
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
 
   controls->WaitForDetachRequest();
@@ -549,9 +557,9 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupAfterDetach) {
 
   controls->set_can_detach(true);
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
 
   controls->WaitForDetach();
@@ -572,9 +580,9 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupDuringWork) {
 
   controls->HaveWorkBlock();
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
 
   controls->WaitForWorkToRun();
@@ -594,9 +602,9 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupDuringWait) {
   scoped_refptr<ControllableDetachDelegate::Controls> controls =
       delegate->controls();
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
 
   controls->WaitForDetachRequest();
@@ -617,9 +625,9 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupDuringShutdown) {
 
   controls->HaveWorkBlock();
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
 
   controls->WaitForWorkToRun();
@@ -630,6 +638,29 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupDuringShutdown) {
   controls->WaitForDelegateDestroy();
 }
 
+// Verify that Start() is a no-op after Cleanup().
+TEST(TaskSchedulerWorkerTest, CleanupBeforeStart) {
+  TaskTracker task_tracker;
+  // Will be owned by SchedulerWorker.
+  // No mock here as that's reasonably covered by other tests and the delegate
+  // may destroy on a different thread. Mocks aren't designed with that in mind.
+  std::unique_ptr<ControllableDetachDelegate> delegate =
+      MakeUnique<ControllableDetachDelegate>(&task_tracker);
+  scoped_refptr<ControllableDetachDelegate::Controls> controls =
+      delegate->controls();
+  controls->set_expect_get_work(false);
+
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+
+  worker->Cleanup();
+
+  worker->Start();
+  worker->WakeUp();
+
+  EXPECT_FALSE(worker->ThreadAliveForTesting());
+}
+
 namespace {
 
 class CallJoinFromDifferentThread : public SimpleThread {
@@ -670,9 +701,9 @@ TEST(TaskSchedulerWorkerTest, WorkerCleanupDuringJoin) {
 
   controls->HaveWorkBlock();
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, std::move(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
 
   controls->WaitForWorkToRun();
@@ -701,9 +732,9 @@ TEST(TaskSchedulerWorkerTest, WorkerDetachesAndWakes) {
 
   controls->set_can_detach(true);
   EXPECT_CALL(*delegate, OnMainEntry(_));
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker));
+  worker->Start();
   worker->WakeUp();
   controls->WaitForWorkToRun();
   Mock::VerifyAndClear(delegate);
@@ -725,16 +756,16 @@ TEST(TaskSchedulerWorkerTest, WorkerDetachesAndWakes) {
   worker->JoinForTesting();
 }
 
-TEST(TaskSchedulerWorkerTest, CreateDetached) {
+TEST(TaskSchedulerWorkerTest, StartDetached) {
   TaskTracker task_tracker;
   // Will be owned by SchedulerWorker.
   MockedControllableDetachDelegate* delegate =
       new StrictMock<MockedControllableDetachDelegate>(&task_tracker);
   scoped_refptr<ControllableDetachDelegate::Controls> controls =
       delegate->controls();
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker,
-      SchedulerWorker::InitialState::DETACHED);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker));
+  worker->Start(SchedulerWorker::InitialState::DETACHED);
   ASSERT_FALSE(worker->ThreadAliveForTesting());
   EXPECT_CALL(*delegate, OnMainEntry(worker.get()));
   worker->WakeUp();
@@ -803,9 +834,9 @@ TEST(TaskSchedulerWorkerTest, BumpPriorityOfAliveThreadDuringShutdown) {
           ? ThreadPriority::BACKGROUND
           : ThreadPriority::NORMAL);
 
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker));
+  worker->Start();
 
   // Verify that the initial thread priority is BACKGROUND (or NORMAL if thread
   // priority can't be increased).
@@ -830,9 +861,9 @@ TEST(TaskSchedulerWorkerTest, BumpPriorityOfDetachedThreadDuringShutdown) {
   delegate_raw->SetExpectedThreadPriority(ThreadPriority::NORMAL);
 
   // Create a DETACHED thread.
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
-      ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::DETACHED);
+  auto worker = make_scoped_refptr(new SchedulerWorker(
+      ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker));
+  worker->Start(SchedulerWorker::InitialState::DETACHED);
 
   // Pretend that shutdown has started.
   task_tracker.SetHasShutdownStartedForTesting();
@@ -887,10 +918,10 @@ TEST(TaskSchedulerWorkerTest, BackwardCompatibilityEnabled) {
 
   // Create a worker with backward compatibility ENABLED. Wake it up and wait
   // until GetWork() returns.
-  auto worker = SchedulerWorker::Create(
+  auto worker = make_scoped_refptr(new SchedulerWorker(
       ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE,
-      SchedulerBackwardCompatibility::INIT_COM_STA);
+      SchedulerBackwardCompatibility::INIT_COM_STA));
+  worker->Start();
   worker->WakeUp();
   delegate_raw->WaitUntilGetWorkReturned();
 
@@ -908,10 +939,10 @@ TEST(TaskSchedulerWorkerTest, BackwardCompatibilityDisabled) {
 
   // Create a worker with backward compatibility DISABLED. Wake it up and wait
   // until GetWork() returns.
-  auto worker = SchedulerWorker::Create(
+  auto worker = make_scoped_refptr(new SchedulerWorker(
       ThreadPriority::NORMAL, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::ALIVE,
-      SchedulerBackwardCompatibility::DISABLED);
+      SchedulerBackwardCompatibility::DISABLED));
+  worker->Start();
   worker->WakeUp();
   delegate_raw->WaitUntilGetWorkReturned();