TaskScheduler: No BACKGROUND threads when unsupported.

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 6ee52aca27e17f0b97b3fb0348d08cf3f97b6fce..2c2c3392008c470885d11944154b08c104f69a39 100644
--- a/base/task_scheduler/scheduler_worker.cc
+++ b/base/task_scheduler/scheduler_worker.cc
@@ -49,9 +49,7 @@ class SchedulerWorker::Thread : public PlatformThread::Delegate {
       mac::ScopedNSAutoreleasePool autorelease_pool;
 #endif
 
-#if !defined(OS_LINUX)
       UpdateThreadPriority(GetDesiredThreadPriority());
-#endif
 
       // Get the sequence containing the next task to execute.
       scoped_refptr<Sequence> sequence = outer_->delegate_->GetWork(outer_);
@@ -131,22 +129,39 @@ class SchedulerWorker::Thread : public PlatformThread::Delegate {
     wake_up_event_.Reset();
   }
 
-  // Returns the desired thread priority based on the worker priority and the
-  // current shutdown state.
+  // Returns the priority to which the thread should be set based on the
+  // SchedulerWorker's preferred priority, current shutdown state and platform

[gab, 2016/08/03 13:35:24]

oxford comma FTW (", and") :-)

[fdoray, 2016/08/03 18:42:48]

Done.

+  // capabilities.
   ThreadPriority GetDesiredThreadPriority() {
     DCHECK(outer_);
 
-    if (outer_->task_tracker_->HasShutdownStarted() &&
+    const bool worker_priority_below_normal =
         static_cast<int>(outer_->thread_priority_) <
-            static_cast<int>(ThreadPriority::NORMAL)) {
+        static_cast<int>(ThreadPriority::NORMAL);
+
+    if (worker_priority_below_normal &&
+        outer_->task_tracker_->HasShutdownStarted()) {
       return ThreadPriority::NORMAL;
     }
+
+#if defined(OS_POSIX)

[gab, 2016/08/03 13:35:24]

Add a comment here why it's fine on Windows? This isn't captured in lock.h or
anywhere else AFAIK. Ideally this would be mentioned at a higher level than
here... I'd prefer for the call made here to not be OS_POSIX, maybe some kind of
Lock::IsThreadPrioritySafe()? which returns true on Windows (with reasoning) and
returns LockImpl::PriorityInheritanceAvailable() on POSIX)?

[fdoray, 2016/08/03 18:42:49]

n/a now that we have HandlesMultipleThreadPriorities()

+    // To avoid priority inversion, all threads have a NORMAL priority when
+    // priority inheritance isn't available.
+    if (!Lock::PriorityInheritanceAvailable())
+      return ThreadPriority::NORMAL;
+#endif
+
+    // To avoid shutdown hangs caused by BLOCK_SHUTDOWN tasks running on
+    // BACKGROUND threads, there are no BACKGROUND threads when thread priority
+    // can't be increased.
+    if (worker_priority_below_normal &&

[gab, 2016/08/03 13:35:24]

So "worker priority" is just the priority hint right? Not the actual priority? I
guess I find this statement confusing because of that.. renaming it to
|priority_hint_| as suggested above will make that clearer IMO (and this bool to
something like |priority_hint_below_normal|).

[fdoray, 2016/08/03 18:42:48]

Done

+        !PlatformThread::CanIncreaseCurrentThreadPriority()) {
+      return ThreadPriority::NORMAL;
+    }
+
     return outer_->thread_priority_;

[gab, 2016/08/03 13:35:24]

So basically the point of |worker_priority_below_normal| is to avoid returning
NORMAL when the hint is > NORMAL< right?

Would it be cleaner to instead start with

if (static_cast<int>(outer_->priority_hint_) >=
    static_cast<int>(ThreadPriority::NORMAL)) {
  return outer_->priority_hint_;
}

and then doing the rest of the logic assuming that the hint is < NORMAL?

[fdoray, 2016/08/03 18:42:49]

|worker_priority_below_normal| is gone

   }
 
-  // Increasing the thread priority requires the CAP_SYS_NICE capability on
-  // Linux.
-#if !defined(OS_LINUX)
   void UpdateThreadPriority(ThreadPriority desired_thread_priority) {
     if (desired_thread_priority == current_thread_priority_)
       return;
@@ -154,7 +169,6 @@ class SchedulerWorker::Thread : public PlatformThread::Delegate {
     PlatformThread::SetCurrentThreadPriority(desired_thread_priority);
     current_thread_priority_ = desired_thread_priority;
   }
-#endif  // !defined(OS_LINUX)
 
   PlatformThreadHandle thread_handle_;
 
@@ -164,7 +178,7 @@ class SchedulerWorker::Thread : public PlatformThread::Delegate {
   WaitableEvent wake_up_event_;
 
   // Current priority of this thread. May be different from
-  // |outer_->thread_priority_| during shutdown.
+  // |outer_->thread_priority_|.
   ThreadPriority current_thread_priority_;
 
   DISALLOW_COPY_AND_ASSIGN(Thread);