Apply BlinkCompositorUseDisplayThreadPriority feature to IO thread

The original change to use ThreadPriority::DISPLAY for the renderer
compositor thread (7b97c324 "base: Allow renderer thread priorities
to be changed.") also applied the priority to the IO thread to avoid
delays in IPC message delivery to the compositor thread.

We recently added a feature to control compositor thread priority, but it
does not affect setting the IO thread priority. Instead, we always try to
raise the priority on Linux, Android, & Chrome OS, and never try on
Windows or Mac. This causes strange situations such as having render
IO threads run with higher priority than any browser or GPU process
thread even though the priority Features were disabled.

Fix the BlinkCompositorUseDisplayThreadPriority feature to also apply to
the IO thread and use it to control the initial thread priority, as well
as whether we ask the browser to adjust it (which is used on Chrome OS
for some cgroup manipulation that needs to be done from the browser).

After this change, each of the display priority flags also control the IO
thread priority of the corresponding process.

Bug: 937462

Change-Id: I1950182fccd8bc6ea35359792878283c2fd9401c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1714204Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681121}

content/child/child_process.cc

@@ -16,6 +16,7 @@
 #include "base/threading/thread_local.h"
 #include "build/build_config.h"
 #include "content/child/child_thread_impl.h"
+#include "third_party/blink/public/common/features.h"
 
 namespace content {
 
@@ -57,7 +58,10 @@ ChildProcess::ChildProcess(base::ThreadPriority io_thread_priority,
 #if defined(OS_ANDROID)
   // TODO(reveman): Remove this in favor of setting it explicitly for each type
   // of process.
-  thread_options.priority = base::ThreadPriority::DISPLAY;
+  if (base::FeatureList::IsEnabled(
+          blink::features::kBlinkCompositorUseDisplayThreadPriority)) {
+    thread_options.priority = base::ThreadPriority::DISPLAY;
+  }
 #endif
   CHECK(io_thread_.StartWithOptions(thread_options));
 }

content/renderer/render_process.cc

@@ -4,15 +4,28 @@
 
 #include <utility>
 
+#include "base/feature_list.h"
 #include "content/renderer/render_process.h"
+#include "third_party/blink/public/common/features.h"
 
 namespace content {
 
+namespace {
+
+base::ThreadPriority GetRenderIOThreadPriority() {
+  if (base::FeatureList::IsEnabled(
+          blink::features::kBlinkCompositorUseDisplayThreadPriority))
+    return base::ThreadPriority::DISPLAY;
+  return base::ThreadPriority::NORMAL;
+}
+
+}  // namespace
+
 RenderProcess::RenderProcess(
     const std::string& thread_pool_name,
     std::unique_ptr<base::ThreadPoolInstance::InitParams>
         thread_pool_init_params)
-    : ChildProcess(base::ThreadPriority::NORMAL,
+    : ChildProcess(GetRenderIOThreadPriority(),
                    thread_pool_name,
                    std::move(thread_pool_init_params)) {}
 

content/renderer/render_thread_impl.cc

@@ -137,6 +137,7 @@
 #include "services/viz/public/cpp/gpu/context_provider_command_buffer.h"
 #include "services/viz/public/cpp/gpu/gpu.h"
 #include "skia/ext/skia_memory_dump_provider.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"
 #include "third_party/blink/public/platform/web_cache.h"
 #include "third_party/blink/public/platform/web_image_generator.h"
@@ -927,8 +928,11 @@ void RenderThreadImpl::Init() {
       discardable_shared_memory_manager_.get());
 
 #if defined(OS_LINUX)
-  render_message_filter()->SetThreadPriority(
-      ChildProcess::current()->io_thread_id(), base::ThreadPriority::DISPLAY);
+  if (base::FeatureList::IsEnabled(
+          blink::features::kBlinkCompositorUseDisplayThreadPriority)) {
+    render_message_filter()->SetThreadPriority(
+        ChildProcess::current()->io_thread_id(), base::ThreadPriority::DISPLAY);
+  }
 #endif
 
   process_foregrounded_count_ = 0;

third_party/blink/common/features.cc

@@ -352,5 +352,16 @@ const base::Feature kVerifyHTMLFetchedFromAppCacheBeforeDelay{
     "VerifyHTMLFetchedFromAppCacheBeforeDelay",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Controls whether we use ThreadPriority::DISPLAY for renderer
+// compositor & IO threads.
+const base::Feature kBlinkCompositorUseDisplayThreadPriority {
+  "BlinkCompositorUseDisplayThreadPriority",
+#if defined(OS_ANDROID) || defined(OS_CHROMEOS)
+      base::FEATURE_ENABLED_BY_DEFAULT
+#else
+      base::FEATURE_DISABLED_BY_DEFAULT
+#endif
+};
+
 }  // namespace features
 }  // namespace blink

third_party/blink/public/common/features.h

@@ -112,6 +112,9 @@ BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
 BLINK_COMMON_EXPORT extern const base::Feature
     kVerifyHTMLFetchedFromAppCacheBeforeDelay;
 
+BLINK_COMMON_EXPORT extern const base::Feature
+    kBlinkCompositorUseDisplayThreadPriority;
+
 }  // namespace features
 }  // namespace blink
 

third_party/blink/renderer/platform/scheduler/common/thread.cc

@@ -29,16 +29,6 @@ namespace blink {
 
 namespace {
 
-// Controls whether we use ThreadPriority::DISPLAY for compositor thread.
-const base::Feature kBlinkCompositorUseDisplayThreadPriority {
-  "BlinkCompositorUseDisplayThreadPriority",
-#if defined(OS_ANDROID) || defined(OS_CHROMEOS)
-      base::FEATURE_ENABLED_BY_DEFAULT
-#else
-      base::FEATURE_DISABLED_BY_DEFAULT
-#endif
-};
-
 // Thread-local storage for "blink::Thread"s.
 Thread*& ThreadTLSSlot() {
   DEFINE_THREAD_SAFE_STATIC_LOCAL(WTF::ThreadSpecific<Thread*>, thread_tls_slot,
@@ -112,7 +102,8 @@ void Thread::CreateAndSetCompositorThread() {
   DCHECK(!GetCompositorThread());
 
   ThreadCreationParams params(WebThreadType::kCompositorThread);
-  if (base::FeatureList::IsEnabled(kBlinkCompositorUseDisplayThreadPriority))
+  if (base::FeatureList::IsEnabled(
+          features::kBlinkCompositorUseDisplayThreadPriority))
     params.thread_priority = base::ThreadPriority::DISPLAY;
 
   auto compositor_thread =
@@ -120,7 +111,8 @@ void Thread::CreateAndSetCompositorThread() {
   compositor_thread->Init();
   GetCompositorThread() = std::move(compositor_thread);
 
-  if (base::FeatureList::IsEnabled(kBlinkCompositorUseDisplayThreadPriority)) {
+  if (base::FeatureList::IsEnabled(
+          features::kBlinkCompositorUseDisplayThreadPriority)) {
     // Chrome OS moves tasks between control groups on thread priority changes.
     // This is not possible inside the sandbox, so ask the browser to do it.
     // TODO(spang): Check if we can remove this on non-Chrome OS builds.