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 747c80b27b86238031f74cff227457e1bf903e43..8d2d1b02b5fcbb0aea5043d22ffc5cf5e940f222 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();
   }
@@ -391,6 +392,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();
@@ -413,6 +418,7 @@ class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate {
     WaitableEvent can_detach_block_;
     WaitableEvent destroyed_;
 
+    bool expect_get_work_ = true;
     bool can_detach_ = false;
     bool work_requested_ = false;
 
@@ -426,6 +432,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_)
@@ -502,9 +510,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);
@@ -526,9 +534,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();
@@ -550,9 +558,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();
@@ -573,9 +581,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();
@@ -595,9 +603,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();
@@ -618,9 +626,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();
@@ -631,6 +639,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 {
@@ -671,9 +702,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();
@@ -702,9 +733,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);
@@ -726,16 +757,18 @@ 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(
+  auto worker = make_scoped_refptr(new SchedulerWorker(
       ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker,
-      SchedulerWorker::InitialState::DETACHED);
+      SchedulerBackwardCompatibility::DISABLED,
+      SchedulerWorker::InitialState::DETACHED));
+  worker->Start();
   ASSERT_FALSE(worker->ThreadAliveForTesting());
   EXPECT_CALL(*delegate, OnMainEntry(worker.get()));
   worker->WakeUp();
@@ -804,9 +837,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).
@@ -831,9 +864,11 @@ TEST(TaskSchedulerWorkerTest, BumpPriorityOfDetachedThreadDuringShutdown) {
   delegate_raw->SetExpectedThreadPriority(ThreadPriority::NORMAL);
 
   // Create a DETACHED thread.
-  scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create(
+  auto worker = make_scoped_refptr(new SchedulerWorker(
       ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker,
-      SchedulerWorker::InitialState::DETACHED);
+      SchedulerBackwardCompatibility::DISABLED,
+      SchedulerWorker::InitialState::DETACHED));
+  worker->Start();
 
   // Pretend that shutdown has started.
   task_tracker.SetHasShutdownStartedForTesting();
@@ -888,10 +923,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();
 
@@ -909,10 +944,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();