blimp: Set up the CompositorDependencies for blimp in Chrome.

chrome/browser/android/blimp/chrome_compositor_dependencies.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 "chrome/browser/android/blimp/chrome_compositor_dependencies.h"
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "cc/output/context_provider.h"
+#include "content/public/common/content_switches.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
+#include "gpu/command_buffer/client/shared_memory_limits.h"
+#include "gpu/command_buffer/common/gles2_cmd_utils.h"
+
+namespace {
+
+bool IsValidWorkerContext(cc::ContextProvider* context_provider) {
+  cc::ContextProvider::ScopedContextLock lock(context_provider);
+  return context_provider->ContextGL()->GetGraphicsResetStatusKHR() ==
+         GL_NO_ERROR;
+}
+
+gpu::gles2::ContextCreationAttribHelper
+GetOffscreenContextCreationAttributes() {
+  gpu::gles2::ContextCreationAttribHelper attributes;
+  attributes.alpha_size = -1;
+  attributes.depth_size = 0;
+  attributes.stencil_size = 0;
+  attributes.samples = 0;
+  attributes.sample_buffers = 0;
+  attributes.bind_generates_resource = false;
+  attributes.lose_context_when_out_of_memory = true;
+  return attributes;
+}
+
+bool IsAsyncWorkerContextEnabled() {
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+
+  if (command_line.HasSwitch(switches::kDisableGpuAsyncWorkerContext))
+    return false;
+  else if (command_line.HasSwitch(switches::kEnableGpuAsyncWorkerContext))
+    return true;
+
+  return false;
+}
+
+}  // namespace
+
+ChromeCompositorDependencies::ChromeCompositorDependencies(
+    ui::ContextProviderFactory* context_provider_factory)
+    : context_provider_factory_(context_provider_factory), weak_factory_(this) {
+  DCHECK(context_provider_factory_);
+}
+
+ChromeCompositorDependencies::~ChromeCompositorDependencies() = default;
+
+gpu::GpuMemoryBufferManager*
+ChromeCompositorDependencies::GetGpuMemoryBufferManager() {
+  return context_provider_factory_->GetGpuMemoryBufferManager();
+}
+
+cc::SurfaceManager* ChromeCompositorDependencies::GetSurfaceManager() {
+  return context_provider_factory_->GetSurfaceManager();
+}
+
+uint32_t ChromeCompositorDependencies::AllocateSurfaceClientId() {
+  return context_provider_factory_->AllocateSurfaceClientId();
+}
+
+void ChromeCompositorDependencies::GetContextProviders(
+    const ContextProviderCallback& callback) {
+  pending_requests_.push_back(callback);

[danakj, 2016/09/01 23:35:18]

It feels like you could just bind the callback into the OnGpuChannelEstablished
and remove a few LOC dealing with pending_requests_ and trying to answer more
than one at a time.

[Khushal, 2016/09/02 00:24:33]

Oh yes, didn't realize without the 2 async calls, this makes this so much
simpler too. Thanks! Done.

+  context_provider_factory_->RequestGpuChannelHost(
+      base::Bind(&ChromeCompositorDependencies::OnGpuChannelEstablished,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void ChromeCompositorDependencies::OnGpuChannelEstablished(
+    const scoped_refptr<gpu::GpuChannelHost>& gpu_channel_host,

[danakj, 2016/09/01 23:35:18]

receive refptrs by value

[Khushal, 2016/09/02 00:24:32]

Is there a reason why that is better? I thought it does an unnecessary ref and
unref if the caller doesn't need a copy?

[danakj, 2016/09/02 00:30:56]

The caller should std::move if it doesnt need a copy, so no this doesn't add an
additional ref. It's better because if you already have an rvalue the compiler
doesnt have to generate a temporary lvalue to make a const& to, which is an
extra ref.

[Khushal, 2016/09/02 01:13:35]

Sorry if this is getting repetitive, I just want to make sure I understand this
correctly. Thanks for taking a look at this.

So I know we do std::move if the caller doesn't want to keep a copy. What I
actually meant to ask was about the case when the caller wants to keep it, but
the function I'm passing the refptr too doesn't need a copy. In this case, the
ContextProviderFactoryImpl needs to keep the GpuChannelHost for all the
callbacks it is responding to, but this function only uses it to pass it back to
the ContextProviderFactoryImpl. In this case does using const& avoid creating an
additional copy of the refptr? And how would passing by value be better?

+    ui::ContextProviderFactory::GpuChannelHostResult result) {
+  // We queue multiple requests, so early out in case the previous requests were
+  // already handled.
+  if (pending_requests_.empty())
+    return;
+
+  switch (result) {
+    case ui::ContextProviderFactory::GpuChannelHostResult::

[danakj, 2016/09/01 23:35:18]

if you "using ui::ContextProviderFactory::GpuChannelHostResult" in this function
then these cases can be shorter.

[Khushal, 2016/09/02 00:24:33]

Done.

+        FAILURE_GPU_PROCESS_INITIALIZATION_FAILED:
+      // Pass the null result to the caller and let them try again.
+      break;
+    case ui::ContextProviderFactory::GpuChannelHostResult::
+        FAILURE_FACTORY_SHUTDOWN:
+      // If the factory is shutting down, early out and drop the requests.
+      return;
+    case ui::ContextProviderFactory::GpuChannelHostResult::SUCCESS:
+      // Create the worker context first if necessary.
+      if (!shared_main_thread_worker_context_ ||
+          !IsValidWorkerContext(shared_main_thread_worker_context_.get())) {
+        shared_main_thread_worker_context_ =
+            context_provider_factory_->CreateOffscreenContextProvider(
+                ui::ContextProviderFactory::ContextType::
+                    BLIMP_RENDER_WORKER_CONTEXT,
+                gpu::SharedMemoryLimits(),
+                GetOffscreenContextCreationAttributes(),
+                true /* support_locking */, false /* automatic_flushes */,
+                nullptr, gpu_channel_host.get());
+        if (!shared_main_thread_worker_context_->BindToCurrentThread()) {
+          shared_main_thread_worker_context_ = nullptr;
+        }
+      }
+      break;
+  }
+
+  // Copy the requests first since we can get more requests as we run these
+  // callbacks.
+  std::list<ContextProviderCallback> context_requests = pending_requests_;
+  pending_requests_.clear();
+
+  for (ContextProviderCallback& context_request : context_requests) {
+    scoped_refptr<cc::ContextProvider> compositor_context_provider;
+
+    if (gpu_channel_host && shared_main_thread_worker_context_) {
+      // The compositor context shares resources with the worker context unless
+      // the worker is async.
+      cc::ContextProvider* shared_context =
+          IsAsyncWorkerContextEnabled()

[danakj, 2016/09/01 23:35:18]

I'm not sure what the answer is, maybe talk with reveman about it, but it
stresses me out a bit that we are checking command line flags here and in
RenderThreadImpl and they are eventually going to not agree with each other on
decisions being made.

[Khushal, 2016/09/02 00:24:32]

I think all of this is in compositor_util in content/. May be the gpu specific
switches could be in gpu/ and we could use the same code here?

[danakj, 2016/09/02 00:30:56]

That's content/browser though.
We need something shared for the renderer or something. Consolidating logic into
compositor_util would help tho, that's not a bad idea, break it down there so
you only get a single flag to the renderer. That's harder tho not impossible to
bitrot.

+              ? nullptr
+              : shared_main_thread_worker_context_.get();
+
+      compositor_context_provider =
+          context_provider_factory_->CreateOffscreenContextProvider(
+              ui::ContextProviderFactory::ContextType::
+                  BLIMP_RENDER_COMPOSITOR_CONTEXT,
+              gpu::SharedMemoryLimits::ForMailboxContext(),
+              GetOffscreenContextCreationAttributes(),
+              false /* support_locking */, false /* automatic_flushes */,
+              shared_context, gpu_channel_host.get());

[danakj, 2016/09/01 23:35:18]

Why is this passing raw pointer to gpu_channel_host, when we're going to be
reffing it over there?

[Khushal, 2016/09/02 00:24:32]

Done.

+    }
+
+    context_request.Run(compositor_context_provider,
+                        shared_main_thread_worker_context_);
+  }
+}