Collapse the initialize methods in WebGraphicsContext3DCommandBufferImpl

content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc

 // Copyright (c) 2012 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 "content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h"
 
 #include "third_party/khronos/GLES2/gl2.h"
 #ifndef GL_GLEXT_PROTOTYPES
 #define GL_GLEXT_PROTOTYPES 1
 #endif
@@ skipping 28 matching lines @@
 
 namespace content {
 
 WebGraphicsContext3DCommandBufferImpl::WebGraphicsContext3DCommandBufferImpl(
     gpu::SurfaceHandle surface_handle,
     const GURL& active_url,
     gpu::GpuChannelHost* host,
     gfx::GpuPreference gpu_preference,
     bool automatic_flushes)
     : automatic_flushes_(automatic_flushes),
-      host_(host),
       surface_handle_(surface_handle),
       active_url_(active_url),
       gpu_preference_(gpu_preference),
-      weak_ptr_factory_(this) {
-  DCHECK(host);
+      host_(host) {
+  DCHECK(host_);
 }
 
 WebGraphicsContext3DCommandBufferImpl::
     ~WebGraphicsContext3DCommandBufferImpl() {
   if (real_gl_)
     real_gl_->SetLostContextCallback(base::Closure());
-
-  Destroy();
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::MaybeInitializeGL(
     const gpu::SharedMemoryLimits& memory_limits,
     gpu::CommandBufferProxyImpl* shared_command_buffer,
     scoped_refptr<gpu::gles2::ShareGroup> share_group,
     const gpu::gles2::ContextCreationAttribHelper& attributes,
     command_buffer_metrics::ContextType context_type) {
-  if (initialized_)
-    return true;
-
-  if (initialize_failed_)
-    return false;
-
+  DCHECK_EQ(!!shared_command_buffer, !!share_group);
   TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::MaybeInitializeGL");
 
-  // TODO(vadimt): Remove ScopedTracker below once crbug.com/125248 is fixed.

[danakj, 2016/04/30 01:56:39]

Moved this out to GpuProcessTransportFactory, since this only applies to the
browser compositor and we already know it's tracking the time of
BindToCurrentThread, most of which is in the sync IPCs creating the
CommandBufferProxyImpl.

-  tracked_objects::ScopedTracker tracking_profile(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "125248 WebGraphicsContext3DCommandBufferImpl::MaybeInitializeGL"));
-
-  if (!CreateContext(memory_limits, shared_command_buffer,
-                     std::move(share_group), attributes, context_type)) {
-    Destroy();
-
-    initialize_failed_ = true;
-    return false;
-  }
-
-  real_gl_->SetLostContextCallback(
-      base::Bind(&WebGraphicsContext3DCommandBufferImpl::OnContextLost,
-                 // The callback is unset in the destructor.
-                 base::Unretained(this)));
-
-  real_gl_->TraceBeginCHROMIUM("WebGraphicsContext3D",
-                               "CommandBufferContext");
-
-  initialized_ = true;
-  return true;
-}
-
-bool WebGraphicsContext3DCommandBufferImpl::InitializeCommandBuffer(
-    gpu::CommandBufferProxyImpl* shared_command_buffer,
-    const gpu::gles2::ContextCreationAttribHelper& attributes,
-    command_buffer_metrics::ContextType context_type) {
-  if (!host_.get())
-    return false;
-
   DCHECK(attributes.buffer_preserved);
   std::vector<int32_t> serialized_attributes;
   attributes.Serialize(&serialized_attributes);
 
   // Create a proxy to a command buffer in the GPU process.
   command_buffer_ = host_->CreateCommandBuffer(
       surface_handle_, gfx::Size(), shared_command_buffer,
       gpu::GpuChannelHost::kDefaultStreamId,
       gpu::GpuChannelHost::kDefaultStreamPriority, serialized_attributes,
       active_url_, gpu_preference_);
 
   if (!command_buffer_) {
     DLOG(ERROR) << "GpuChannelHost failed to create command buffer.";
     command_buffer_metrics::UmaRecordContextInitFailed(context_type);
     return false;
   }
+  if (!command_buffer_->Initialize()) {
+    DLOG(ERROR) << "CommandBufferProxy::Initialize failed.";
+    command_buffer_metrics::UmaRecordContextInitFailed(context_type);
+    return false;
+  }
 
-  DVLOG_IF(1, gpu::error::IsError(command_buffer_->GetLastError()))

[danakj, 2016/04/30 01:56:39]

I can't see a way for last_state_ to be set to anything interesting after
creating the CommandBuffer. We do a sync call but don't do pass it a last_state_
pointer like we do in WaitForToken, so I dropped this print.

-      << "Context dead on arrival. Last error: "
-      << command_buffer_->GetLastError();
-  // Initialize the command buffer.
-  bool result = command_buffer_->Initialize();
-  LOG_IF(ERROR, !result) << "CommandBufferProxy::Initialize failed.";
-  if (!result)
-    command_buffer_metrics::UmaRecordContextInitFailed(context_type);
-  return result;
-}
-
-bool WebGraphicsContext3DCommandBufferImpl::CreateContext(
-    const gpu::SharedMemoryLimits& memory_limits,
-    gpu::CommandBufferProxyImpl* shared_command_buffer,
-    scoped_refptr<gpu::gles2::ShareGroup> share_group,
-    const gpu::gles2::ContextCreationAttribHelper& attributes,
-    command_buffer_metrics::ContextType context_type) {
-  TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::CreateContext");
-  DCHECK_EQ(!!shared_command_buffer, !!share_group);
-
-  if (!InitializeCommandBuffer(shared_command_buffer, attributes,
-                               context_type)) {
-    LOG(ERROR) << "Failed to initialize command buffer.";
+  // The GLES2 helper writes the command buffer protocol.
+  gles2_helper_.reset(new gpu::gles2::GLES2CmdHelper(command_buffer_.get()));
+  gles2_helper_->SetAutomaticFlushes(automatic_flushes_);

[danakj, 2016/04/30 01:56:39]

I thot it was a bit weird that we set this after calling Initialize which
potentially makes it do more work, so I put this above Initialize so it's just
always true. It seems like this should be ok since Init calls AllocateRingBuffer
which does CalcImmediateEntries, and that's the only place this value is used.
And that's what SetAutomaticFlushes would do also after Initialize (tho here it
would just early out now so it's cheaper?) I also considered making it part of
the constructor. WDYT, is this ok?

+  if (!gles2_helper_->Initialize(memory_limits.command_buffer_size)) {
+    DLOG(ERROR) << "Failed to initialize GLES2CmdHelper.";

[danakj, 2016/04/30 01:56:39]

I turned all the LOGs into DLOGs which print when DCHECK_IS_ON so it's there on
the gpu bots.

     return false;
   }
 
-  // Create the GLES2 helper, which writes the command buffer protocol.
-  gles2_helper_.reset(new gpu::gles2::GLES2CmdHelper(command_buffer_.get()));
-  if (!gles2_helper_->Initialize(memory_limits.command_buffer_size)) {
-    LOG(ERROR) << "Failed to initialize GLES2CmdHelper.";
-    return false;
-  }
-
-  if (!automatic_flushes_)
-    gles2_helper_->SetAutomaticFlushes(false);
-  // Create a transfer buffer used to copy resources between the renderer
+  // The transfer buffer is used to copy resources between the client
   // process and the GPU process.
   transfer_buffer_ .reset(new gpu::TransferBuffer(gles2_helper_.get()));
 
-  DCHECK(host_.get());
-
   const bool bind_generates_resource = attributes.bind_generates_resource;
   const bool lose_context_when_out_of_memory =
       attributes.lose_context_when_out_of_memory;
   const bool support_client_side_arrays = false;
 
-  // Create the object exposing the OpenGL API.
+  // The GLES2Implementation exposes the OpenGLES2 API, as well as the
+  // gpu::ContextSupport interface.
   real_gl_.reset(new gpu::gles2::GLES2Implementation(
       gles2_helper_.get(), std::move(share_group), transfer_buffer_.get(),
       bind_generates_resource, lose_context_when_out_of_memory,
       support_client_side_arrays, command_buffer_.get()));
   if (!real_gl_->Initialize(memory_limits.start_transfer_buffer_size,
                             memory_limits.min_transfer_buffer_size,
                             memory_limits.max_transfer_buffer_size,
                             memory_limits.mapped_memory_reclaim_limit)) {
-    LOG(ERROR) << "Failed to initialize GLES2Implementation.";
+    DLOG(ERROR) << "Failed to initialize GLES2Implementation.";
     return false;
   }
 
+  real_gl_->SetLostContextCallback(
+      base::Bind(&WebGraphicsContext3DCommandBufferImpl::OnContextLost,
+                 // The callback is unset in the destructor.
+                 base::Unretained(this)));
+  real_gl_->TraceBeginCHROMIUM("WebGraphicsContext3D", "CommandBufferContext");
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::InitializeOnCurrentThread(
     const gpu::SharedMemoryLimits& memory_limits,
     gpu::CommandBufferProxyImpl* shared_command_buffer,
     scoped_refptr<gpu::gles2::ShareGroup> share_group,
     const gpu::gles2::ContextCreationAttribHelper& attributes,
     command_buffer_metrics::ContextType context_type) {
   if (!MaybeInitializeGL(memory_limits, shared_command_buffer,
-                         std::move(share_group), attributes, context_type)) {
-    DLOG(ERROR) << "Failed to initialize context.";
+                         std::move(share_group), attributes, context_type))
     return false;
-  }
   if (gpu::error::IsError(command_buffer_->GetLastError())) {
-    LOG(ERROR) << "Context dead on arrival. Last error: "
-               << command_buffer_->GetLastError();
+    DLOG(ERROR) << "Context dead on arrival. Last error: "
+                << command_buffer_->GetLastError();
     return false;
   }
 
   return true;
 }
 
-void WebGraphicsContext3DCommandBufferImpl::Destroy() {
-  trace_gl_.reset();
-  real_gl_.reset();
-  transfer_buffer_.reset();
-  gles2_helper_.reset();
-  real_gl_.reset();
-  command_buffer_.reset();
-
-  host_ = nullptr;
-}
-
-gpu::ContextSupport*
-WebGraphicsContext3DCommandBufferImpl::GetContextSupport() {

[danakj, 2016/04/30 01:56:39]

This is just collatoral damage, as it's pointless since we return the
GLES2Implementation already. So just use that in the provider.

-  return real_gl_.get();
-}
-
 void WebGraphicsContext3DCommandBufferImpl::OnContextLost() {
   if (context_lost_callback_)
     context_lost_callback_->onContextLost();
 }
 
 }  // namespace content