TaskScheduler: Remove base::ExecutionMode.

base/task_scheduler/scheduler_worker_pool_impl.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_pool_impl.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
 
 #include "base/atomicops.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/sequence_token.h"
-#include "base/sequenced_task_runner.h"
-#include "base/single_thread_task_runner.h"

[gab, 2016/10/31 19:10:11]

(but here you'll want to keep them since you're using it beyond just the
overrides -- tricky details of IWYU ;-))

[robliao, 2016/10/31 19:19:59]

Not sure I follow here. The style guide permits use of headers from the main
header.

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
However, any includes present in the related header do not need to be included
again in the related cc (i.e., foo.cc can rely on foo.h's includes).

[gab, 2016/10/31 19:28:45]

Right but my other comment asks to remove from header per only being used in the
overrides (which can be assumed to be included by parent class).

[robliao, 2016/10/31 19:31:48]

Gotcha. That other comment makes a lot more sense now. +1 to that.

[fdoray, 2016/10/31 19:44:51]

Done.

 #include "base/strings/stringprintf.h"
 #include "base/task_scheduler/delayed_task_manager.h"
 #include "base/task_scheduler/task_tracker.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 
 namespace base {
 namespace internal {
@@ skipping 303 matching lines @@
 
   DCHECK(!join_for_testing_returned_.IsSignaled());
   join_for_testing_returned_.Signal();
 }
 
 void SchedulerWorkerPoolImpl::DisallowWorkerDetachmentForTesting() {
   worker_detachment_disallowed_.Set();
 }
 
 scoped_refptr<TaskRunner> SchedulerWorkerPoolImpl::CreateTaskRunnerWithTraits(
-    const TaskTraits& traits,
-    ExecutionMode execution_mode) {
-  switch (execution_mode) {
-    case ExecutionMode::PARALLEL:
-      return make_scoped_refptr(new SchedulerParallelTaskRunner(traits, this));
+    const TaskTraits& traits) {
+  return make_scoped_refptr(new SchedulerParallelTaskRunner(traits, this));
+}
 
-    case ExecutionMode::SEQUENCED:
-      return make_scoped_refptr(new SchedulerSequencedTaskRunner(traits, this));
+scoped_refptr<SequencedTaskRunner>
+SchedulerWorkerPoolImpl::CreateSequencedTaskRunnerWithTraits(
+    const TaskTraits& traits) {
+  return make_scoped_refptr(new SchedulerSequencedTaskRunner(traits, this));
+}
 
-    case ExecutionMode::SINGLE_THREADED: {
-      // TODO(fdoray): Find a way to take load into account when assigning a
-      // SchedulerWorker to a SingleThreadTaskRunner. Also, this code
-      // assumes that all SchedulerWorkers are alive. Eventually, we might
-      // decide to tear down threads that haven't run tasks for a long time.
-      size_t worker_index;
-      {
-        AutoSchedulerLock auto_lock(next_worker_index_lock_);
-        worker_index = next_worker_index_;
-        next_worker_index_ = (next_worker_index_ + 1) % workers_.size();
-      }
-      return make_scoped_refptr(new SchedulerSingleThreadTaskRunner(
-          traits, this, workers_[worker_index].get()));
-    }
+scoped_refptr<SingleThreadTaskRunner>
+SchedulerWorkerPoolImpl::CreateSingleThreadTaskRunnerWithTraits(
+    const TaskTraits& traits) {
+  // TODO(fdoray): Find a way to take load into account when assigning a
+  // SchedulerWorker to a SingleThreadTaskRunner. Also, this code assumes that
+  // all SchedulerWorkers are alive. Eventually, we might decide to tear down
+  // threads that haven't run tasks for a long time.

[gab, 2016/10/31 19:10:11]

The second half of this comment is wrong with detach now (i.e. it's still fine
in practice but "alive" has been redefined)

[fdoray, 2016/10/31 19:44:51]

Removed second part of the comment since I don't believe that we'll take the
detach state into account. We'll decide what the best algorithm is when we
address this TODO.

+  size_t worker_index;
+  {
+    AutoSchedulerLock auto_lock(next_worker_index_lock_);
+    worker_index = next_worker_index_;
+    next_worker_index_ = (next_worker_index_ + 1) % workers_.size();
   }
-
-  NOTREACHED();
-  return nullptr;
+  return make_scoped_refptr(new SchedulerSingleThreadTaskRunner(
+      traits, this, workers_[worker_index].get()));
 }
 
 void SchedulerWorkerPoolImpl::ReEnqueueSequence(
     scoped_refptr<Sequence> sequence,
     const SequenceSortKey& sequence_sort_key) {
   shared_priority_queue_.BeginTransaction()->Push(std::move(sequence),
                                                   sequence_sort_key);
 
   // The thread calling this method just ran a Task from |sequence| and will
   // soon try to get another Sequence from which to run a Task. If the thread
@@ skipping 415 matching lines @@
   AutoSchedulerLock auto_lock(idle_workers_stack_lock_);
   idle_workers_stack_.Remove(worker);
 }
 
 bool SchedulerWorkerPoolImpl::CanWorkerDetachForTesting() {
   return !worker_detachment_disallowed_.IsSet();
 }
 
 }  // namespace internal
 }  // namespace base