services/ui: Allow injecting the IO thread task runner for gpu channel.

services/ui/public/cpp/gpu_service.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 "services/ui/public/cpp/gpu_service.h"
 
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
@@ skipping 17 matching lines @@
                    base::Bind(callback, std::move(established_channel_host)));
 }
 
 }  // namespace
 
 GpuService::GpuService(shell::Connector* connector)
     : main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       connector_(connector),
       shutdown_event_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
                       base::WaitableEvent::InitialState::NOT_SIGNALED),
-      io_thread_("GPUIOThread"),
       gpu_memory_buffer_manager_(new MojoGpuMemoryBufferManager),
       is_establishing_(false),
       establishing_condition_(&lock_) {
   DCHECK(main_task_runner_);
   DCHECK(connector_);
-  base::Thread::Options thread_options(base::MessageLoop::TYPE_IO, 0);
-  thread_options.priority = base::ThreadPriority::NORMAL;
-  CHECK(io_thread_.StartWithOptions(thread_options));
 }
 
 GpuService::~GpuService() {
   DCHECK(IsMainThread());
   if (gpu_channel_)
     gpu_channel_->DestroyChannel();
 }
 
 // static
-std::unique_ptr<GpuService> GpuService::Initialize(
+std::unique_ptr<GpuService> GpuService::Create(
     shell::Connector* connector) {
   return base::WrapUnique(new GpuService(connector));
 }
 
+void GpuService::SetIOThreadTaskRunner(
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
+  DCHECK(!io_task_runner_)

[piman, 2016/08/22 19:42:03]

Would it make sense instead to pass the task runner explicitly to Create, and
creating the thread in the constructor if the task runner is null?

In particular, I'm concerned that GetIOThreadTaskRunner can be called on any
thread, making the logic there unsafe.

[sadrul, 2016/08/22 20:05:54]

Ah, interesting. Yes, Fady brought up the issue earlier. It's not entirely
straight forward because of how GpuService is created for the chrome browser
process (see
https://codereview.chromium.org/2249413004/diff/1/services/ui/public/cpp/gpu_...).
Would it make more sense if I create this thread in EstablishGpuChannel[Sync]
instead?

[piman, 2016/08/22 20:31:50]

I feel like plumbing the task runner through WindowManagerConnection is easy
enough, and feels (to me) cleaner than the Law-of-Demeter-cringy logic in
ChromeBrowserMainExtraPartsViews::MojoShellConnectionStarted - but I guess
opinions can differ on that.


I think deferring creating the thread to EstablishGpuChannel* would work, but
then I would prefer making GetIOThreadTaskRunner private (and even making the
GpuChannelHostFactory inheritance private, but that's iffy by the style guide) -
making it clear that it's only called by the GpuChannelHost, which by
construction would only be after the thread is created.

But in general, I prefer "obviously correct" code rather than defending with
documentation.

[sadrul, 2016/08/22 21:20:38]

Agreed. And contrary to popular meme, obviously correct is the best kind of
correct. Done.

+      << "The task runner cannot be changed once set.";
+  DCHECK(io_task_runner);
+  io_task_runner_ = std::move(io_task_runner);
+}
+
 void GpuService::EstablishGpuChannel(
     const gpu::GpuChannelEstablishedCallback& callback) {
   base::AutoLock auto_lock(lock_);
   auto runner = base::ThreadTaskRunnerHandle::Get();
   scoped_refptr<gpu::GpuChannelHost> channel = GetGpuChannelLocked();
   if (channel) {
     PostTask(runner, FROM_HERE, callback, std::move(channel));
     return;
   }
   establish_callbacks_.push_back(
@@ skipping 127 matching lines @@
     i.Run(gpu_channel_);
   establish_callbacks_.clear();
 }
 
 bool GpuService::IsMainThread() {
   return main_task_runner_->BelongsToCurrentThread();
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
 GpuService::GetIOThreadTaskRunner() {
-  return io_thread_.task_runner();
+  if (!io_task_runner_) {
+    io_thread_.reset(new base::Thread("GPUIOThread"));
+    base::Thread::Options thread_options(base::MessageLoop::TYPE_IO, 0);
+    thread_options.priority = base::ThreadPriority::NORMAL;
+    CHECK(io_thread_->StartWithOptions(thread_options));
+    io_task_runner_ = io_thread_->task_runner();
+  }
+  return io_task_runner_;
 }
 
 std::unique_ptr<base::SharedMemory> GpuService::AllocateSharedMemory(
     size_t size) {
   mojo::ScopedSharedBufferHandle handle =
       mojo::SharedBufferHandle::Create(size);
   if (!handle.is_valid())
     return nullptr;
 
   base::SharedMemoryHandle platform_handle;
   size_t shared_memory_size;
   bool readonly;
   MojoResult result = mojo::UnwrapSharedMemoryHandle(
       std::move(handle), &platform_handle, &shared_memory_size, &readonly);
   if (result != MOJO_RESULT_OK)
     return nullptr;
   DCHECK_EQ(shared_memory_size, size);
 
   return base::MakeUnique<base::SharedMemory>(platform_handle, readonly);
 }
 
 }  // namespace ui