mus::GpuService: Support establish GpuChannel asynchronously.

components/mus/common/gpu_service.cc

Index: components/mus/common/gpu_service.cc
diff --git a/components/mus/common/gpu_service.cc b/components/mus/common/gpu_service.cc
index 833e62b339ad2cd8911f0d1d7c01166f84f9add8..dae675f8f207191817e5ce6cb4ba74f997709057 100644
--- a/components/mus/common/gpu_service.cc
+++ b/components/mus/common/gpu_service.cc
@@ -6,8 +6,8 @@
 
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
+#include "base/run_loop.h"
 #include "components/mus/common/gpu_type_converters.h"
-#include "components/mus/common/mojo_gpu_memory_buffer_manager.h"
 #include "components/mus/common/switches.h"
 #include "components/mus/public/interfaces/gpu_service.mojom.h"
 #include "mojo/public/cpp/bindings/sync_call_restrictions.h"
@@ -16,12 +16,23 @@
 
 namespace mus {
 
+namespace {
+
+void PostTask(scoped_refptr<base::SingleThreadTaskRunner> runner,
+              const base::Closure& callback) {
+  runner->PostTask(FROM_HERE, callback);

[piman, 2016/06/22 17:58:10]

nit: can we move the FROM_HERE to the caller? I.e. add a
tracked_objects::Location parameter (it's copyable) to this PostTask, and have
the caller code do base::Bind(&PostTast, FROM_HERE, runner, closure);
That way we can identify the source better in traces and other things.

[Peng, 2016/06/22 19:28:30]

Done.

+}
+}
+
 GpuService::GpuService()
-    : main_message_loop_(base::MessageLoop::current()),
+    : main_message_loop_(nullptr),
+      connector_(nullptr),
       shutdown_event_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
                       base::WaitableEvent::InitialState::NOT_SIGNALED),
       io_thread_("GPUIOThread"),
-      gpu_memory_buffer_manager_(new MojoGpuMemoryBufferManager) {
+      gpu_memory_buffer_manager_(new MojoGpuMemoryBufferManager),
+      is_establishing_(false),
+      establishing_condition_(&lock_) {
   base::Thread::Options thread_options(base::MessageLoop::TYPE_IO, 0);
   thread_options.priority = base::ThreadPriority::NORMAL;
   CHECK(io_thread_.StartWithOptions(thread_options));
@@ -36,47 +47,170 @@ bool GpuService::UseChromeGpuCommandBuffer() {
 }
 
 // static
+void GpuService::Initialize(shell::Connector* connector) {
+  GetInstance()->Init(connector);
+}
+
+// static
 GpuService* GpuService::GetInstance() {
   return base::Singleton<GpuService,

[piman, 2016/06/22 17:58:10]

Because initialization needs to be explicit, could we use a global pointer, that
would be initialized in ::Initialize instead of a Singleton, and DCHECK here
that the pointer is valid?

IOW:

static GpuService* g_gpu_service = nullptr;

void GpuService::Initialize(shell::Connector* connector) {
  DCHECK(!g_gpu_service);
  g_gpu_service = new GpuService();
  g_gpu_service->Init(connector);
}

GpuService* GpuService::GetInstance() {
  DCHECK(g_gpu_service);
  return g_gpu_service;
}


You may even be able to fold Init() into the constructor.

[Peng, 2016/06/22 19:28:30]

Done.

                          base::LeakySingletonTraits<GpuService>>::get();
 }
 
-scoped_refptr<gpu::GpuChannelHost> GpuService::EstablishGpuChannel(
-    shell::Connector* connector) {
+void GpuService::EstablishGpuChannel(const base::Closure& callback) {
+  DCHECK(main_message_loop_);
+  DCHECK(connector_);
+
   base::AutoLock auto_lock(lock_);
-  if (gpu_channel_ && gpu_channel_->IsLost()) {
-    gpu_channel_->DestroyChannel();
-    gpu_channel_ = nullptr;
+  auto runner = base::ThreadTaskRunnerHandle::Get();
+  if (GetGpuChannelLocked()) {
+    runner->PostTask(FROM_HERE, callback);
+    return;
+  }
+  base::Closure wrapper_callback =
+      IsMainThread() ? callback : base::Bind(PostTask, runner, callback);
+  establish_callbacks_.push_back(wrapper_callback);
+
+  if (!is_establishing_) {
+    is_establishing_ = true;
+    main_message_loop_->task_runner()->PostTask(
+        FROM_HERE, base::Bind(&GpuService::EstablishGpuChannelOnMainThread,
+                              base::Unretained(this)));
   }
+}
 
-  if (gpu_channel_)
+scoped_refptr<gpu::GpuChannelHost> GpuService::EstablishGpuChannelSync() {
+  DCHECK(main_message_loop_);
+  DCHECK(connector_);
+
+  base::AutoLock auto_lock(lock_);
+  if (GetGpuChannelLocked())
     return gpu_channel_;
 
-  mus::mojom::GpuServicePtr gpu_service;
-  connector->ConnectToInterface("mojo:mus", &gpu_service);
-
-  int client_id = 0;
-  mojom::ChannelHandlePtr channel_handle;
-  mojom::GpuInfoPtr gpu_info;
-  {
-    // TODO(penghuang): Remove the ScopedAllowSyncCall when HW rendering is
-    // enabled in mus chrome.
-    mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_call;
-    if (!gpu_service->EstablishGpuChannel(&client_id, &channel_handle,
-                                          &gpu_info)) {
-      DLOG(WARNING)
-          << "Channel encountered error while establishing gpu channel.";
-      return nullptr;
+  if (!IsMainThread()) {
+    if (!is_establishing_) {
+      // Create an establishing gpu channel task, if there isn't one.
+      is_establishing_ = true;
+      main_message_loop_->task_runner()->PostTask(
+          FROM_HERE, base::Bind(&GpuService::EstablishGpuChannelOnMainThread,
+                                base::Unretained(this)));
+    }
+    // Wait until the pending establishing task is finished.
+    do {
+      establishing_condition_.Wait();
+    } while (is_establishing_);
+  } else {
+    if (is_establishing_) {
+      // A establishing task has been posted and we are on the main thread,
+      // so we have to run a nest run loop.

[piman, 2016/06/22 17:58:10]

This seems bad.
Could we instead run the code below (other side of the if), and just make sure
the posted task is cancelled (e.g. return early if is_establishing_ is false
under the lock) ?

[Peng, 2016/06/22 19:28:30]

Done.

+      base::RunLoop run_loop;
+      DCHECK(quite_run_loop_closure_.is_null());
+      quite_run_loop_closure_ = run_loop.QuitClosure();
+      {
+        base::AutoUnlock auto_unlock(lock_);
+        run_loop.Run();
+      }
+      DCHECK(quite_run_loop_closure_.is_null());
+      DCHECK(!is_establishing_);
+    } else {
+      is_establishing_ = true;
+      int client_id = 0;
+      mojom::ChannelHandlePtr channel_handle;
+      mojom::GpuInfoPtr gpu_info;
+      mus::mojom::GpuServicePtr gpu_service;
+      connector_->ConnectToInterface("mojo:mus", &gpu_service);
+      {
+        // Unlock the |lock_| during the sync mojo method call.
+        base::AutoUnlock auto_unlock(lock_);
+        // TODO(penghuang): Remove the ScopedAllowSyncCall when HW rendering is
+        // enabled in mus chrome.
+        mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_call;
+        if (!gpu_service->EstablishGpuChannel(&client_id, &channel_handle,
+                                              &gpu_info)) {
+          DLOG(WARNING)
+              << "Channel encountered error while establishing gpu channel.";

[piman, 2016/06/22 17:58:10]

should you return nullptr here?

[Peng, 2016/06/22 19:28:30]

Done.

+        }
+      }
+      const bool locked = true;
+      EstablishGpuChannelOnMainThreadDone(
+          locked, client_id, std::move(channel_handle), std::move(gpu_info));
     }
   }
+  return gpu_channel_;
+}
 
-  // TODO(penghuang): Get the real gpu info from mus.
-  gpu_channel_ = gpu::GpuChannelHost::Create(
-      this, client_id, gpu::GPUInfo(), channel_handle.To<IPC::ChannelHandle>(),
-      &shutdown_event_, gpu_memory_buffer_manager_.get());
+scoped_refptr<gpu::GpuChannelHost> GpuService::GetGpuChannel() {
+  base::AutoLock auto_lock(lock_);
+  return GetGpuChannelLocked();
+}
+
+void GpuService::Init(shell::Connector* connector) {
+  DCHECK(!main_message_loop_);
+  DCHECK(!connector_);
+  main_message_loop_ = base::MessageLoop::current();
+  connector_ = connector;
+  DCHECK(main_message_loop_);
+  DCHECK(connector_);
+}
+
+scoped_refptr<gpu::GpuChannelHost> GpuService::GetGpuChannelLocked() {
+  if (gpu_channel_ && gpu_channel_->IsLost()) {
+    main_message_loop_->task_runner()->PostTask(
+        FROM_HERE,
+        base::Bind(&gpu::GpuChannelHost::DestroyChannel, gpu_channel_));
+    gpu_channel_ = nullptr;
+  }
   return gpu_channel_;
 }
 
+void GpuService::EstablishGpuChannelOnMainThread() {
+  base::AutoLock auto_lock(lock_);
+  DCHECK(IsMainThread());
+  DCHECK(!gpu_channel_);
+  DCHECK(connector_);
+
+  mus::mojom::GpuServicePtr gpu_service;
+  connector_->ConnectToInterface("mojo:mus", &gpu_service);
+  const bool locked = false;
+  gpu_service->EstablishGpuChannel(
+      base::Bind(&GpuService::EstablishGpuChannelOnMainThreadDone,
+                 base::Unretained(this), locked));
+}
+
+void GpuService::EstablishGpuChannelOnMainThreadDone(
+    bool locked,
+    int client_id,
+    mojom::ChannelHandlePtr channel_handle,
+    mojom::GpuInfoPtr gpu_info) {
+  DCHECK(IsMainThread());
+  scoped_refptr<gpu::GpuChannelHost> gpu_channel;
+  if (client_id) {
+    // TODO(penghuang): Get the real gpu info from mus.
+    gpu_channel = gpu::GpuChannelHost::Create(
+        this, client_id, gpu::GPUInfo(),
+        channel_handle.To<IPC::ChannelHandle>(), &shutdown_event_,
+        gpu_memory_buffer_manager_.get());
+  }
+
+  std::unique_ptr<base::AutoLock> auto_lock;
+  if (!locked)
+    auto_lock = base::MakeUnique<base::AutoLock>(lock_);
+  DCHECK(is_establishing_);
+  DCHECK(!gpu_channel_);
+
+  is_establishing_ = false;
+  gpu_channel_ = gpu_channel;
+  establishing_condition_.Broadcast();
+
+  for (const auto& i : establish_callbacks_)
+    i.Run();
+  establish_callbacks_.clear();
+  if (quite_run_loop_closure_) {
+    quite_run_loop_closure_.Run();
+    quite_run_loop_closure_.Reset;
+  }
+}
+
 bool GpuService::IsMainThread() {
   return base::MessageLoop::current() == main_message_loop_;
 }