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

services/ui/public/cpp/gpu_service.h

 // 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.
 
 #ifndef SERVICES_UI_PUBLIC_CPP_GPU_SERVICE_H_
 #define SERVICES_UI_PUBLIC_CPP_GPU_SERVICE_H_
 
 #include <stdint.h>
 #include <vector>
 
@@ skipping 17 matching lines @@
 class GpuService : public gpu::GpuChannelHostFactory,
                    public gpu::GpuChannelEstablishFactory {
  public:
   ~GpuService() override;
 
   gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager() const {
     return gpu_memory_buffer_manager_.get();
   }
 
   // The GpuService has to be initialized in the main thread before establishing
-  // the gpu channel.
+  // the gpu channel. The IO thread task-runner needed for the gpu channel can
+  // be set using SetIOTaskRunner(). If no task-runner is set, then a new thread
+  // is created for IO instead.
   static std::unique_ptr<GpuService> Initialize(shell::Connector* connector);
 
+  void SetIOThreadTaskRunner(
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);

[Fady Samuel, 2016/08/18 11:00:56]

Can this not be passed in Initialize to avoid giving the impression that the
IOThreadTaskRunner can change at any time?

Also optional nit: GpuService::Initialize sounds weird because it actually
creates the object rather than initialize an already constructed object.

It would be nice to call this GpuService::Create, but optional I guess

[sadrul, 2016/08/18 12:56:53]

It's not straight forward, because for the chrome browser, the GpuService is
created by the views::WindowManagerConnection object. So we would have to plumb
the task-runner through that to GpuService. This is cleaner.
SetIOThreadTaskRunner() already has a DCHECK() to make sure the task runner
cannot be changed once set. I have added some messaging to the DCHECK() to
clarify that as well.
I have disliked Initialize() myself. Renamed to Create().

[Fady Samuel, 2016/08/18 13:05:39]

LGTM but I still feel like this is brittle because we could accidentally create
an IOThread if someone calls GetIOThreadTaskRunner() too early before
SetIOThreadTaskRunner is called

+
   // gpu::GpuChannelEstablishFactory:
   void EstablishGpuChannel(
       const gpu::GpuChannelEstablishedCallback& callback) override;
   scoped_refptr<gpu::GpuChannelHost> EstablishGpuChannelSync() override;
   gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() override;
 
  private:
   friend struct base::DefaultSingletonTraits<GpuService>;
 
   explicit GpuService(shell::Connector* connector);
 
   scoped_refptr<gpu::GpuChannelHost> GetGpuChannelLocked();
   void EstablishGpuChannelOnMainThread();
   void EstablishGpuChannelOnMainThreadSyncLocked();
   void EstablishGpuChannelOnMainThreadDone(
       bool locked,
       int client_id,
       mojom::ChannelHandlePtr channel_handle,
       const gpu::GPUInfo& gpu_info);
 
   // gpu::GpuChannelHostFactory overrides:
   bool IsMainThread() override;
   scoped_refptr<base::SingleThreadTaskRunner> GetIOThreadTaskRunner() override;
   std::unique_ptr<base::SharedMemory> AllocateSharedMemory(
       size_t size) override;
 
   scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
   shell::Connector* connector_;
   base::WaitableEvent shutdown_event_;
-  base::Thread io_thread_;
+  std::unique_ptr<base::Thread> io_thread_;
   std::unique_ptr<MojoGpuMemoryBufferManager> gpu_memory_buffer_manager_;
 
   // Lock for |gpu_channel_|, |establish_callbacks_| & |is_establishing_|.
   base::Lock lock_;
   bool is_establishing_;
   ui::mojom::GpuServicePtr gpu_service_;
   scoped_refptr<gpu::GpuChannelHost> gpu_channel_;
   std::vector<gpu::GpuChannelEstablishedCallback> establish_callbacks_;
   base::ConditionVariable establishing_condition_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuService);
 };
 
 }  // namespace ui
 
 #endif  // SERVICES_UI_PUBLIC_CPP_GPU_SERVICE_H_