Split WebGraphicsContext3DCommandBufferImpl::initialize() into two stages.

content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc

 // Copyright (c) 2011 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.
 
 #if defined(ENABLE_GPU)
 
 #include "content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h"
 
 #include "gpu/GLES2/gl2.h"
 #ifndef GL_GLEXT_PROTOTYPES
 #define GL_GLEXT_PROTOTYPES 1
 #endif
 #include "gpu/GLES2/gl2ext.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/lazy_instance.h"
 #include "base/string_tokenizer.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/synchronization/lock.h"
 #include "content/common/content_switches.h"
 #include "content/renderer/gpu/gpu_channel_host.h"
 #include "content/renderer/render_thread.h"
 #include "content/renderer/render_view.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/common/constants.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 #include "webkit/glue/gl_bindings_skia_cmd_buffer.h"
 
+static base::LazyInstance<base::Lock>
+    g_all_contexts_lock(base::LINKER_INITIALIZED);
 static base::LazyInstance<std::set<WebGraphicsContext3DCommandBufferImpl*> >
     g_all_contexts(base::LINKER_INITIALIZED);
 
 WebGraphicsContext3DCommandBufferImpl::WebGraphicsContext3DCommandBufferImpl()
     : context_(NULL),
       gl_(NULL),
 #ifndef WTF_USE_THREADED_COMPOSITING
       web_view_(NULL),
 #endif
 #if defined(OS_MACOSX)
       plugin_handle_(NULL),
 #endif  // defined(OS_MACOSX)
       context_lost_callback_(0),
       context_lost_reason_(GL_NO_ERROR),
       swapbuffers_complete_callback_(0),
       cached_width_(0),
       cached_height_(0),
       bound_fbo_(0) {
 }
 
 WebGraphicsContext3DCommandBufferImpl::
     ~WebGraphicsContext3DCommandBufferImpl() {
-  g_all_contexts.Pointer()->erase(this);
+  {
+    base::AutoLock lock(g_all_contexts_lock.Get());
+    g_all_contexts.Pointer()->erase(this);
+  }
   delete context_;
 }
 
 // This string should only be passed for WebGL contexts. Nothing ELSE!!!
 // Compositor contexts, Canvas2D contexts, Pepper Contexts, nor any other use of
 // a context should not pass this string.
 static const char* kWebGLPreferredGLExtensions =
     "GL_OES_packed_depth_stencil "
     "GL_OES_depth24 "
     "GL_CHROMIUM_webglsl";
 
 bool WebGraphicsContext3DCommandBufferImpl::initialize(
     WebGraphicsContext3D::Attributes attributes,
     WebKit::WebView* web_view,
     bool render_directly_to_web_view) {
+  DCHECK(!context_);
   TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::initialize");
   webkit_glue::BindSkiaToCommandBufferGL();
   RenderThread* render_thread = RenderThread::current();
   if (!render_thread)
     return false;
-  GpuChannelHost* host = render_thread->EstablishGpuChannelSync(
+  host_ = render_thread->EstablishGpuChannelSync(
     content::
       CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE);
-  if (!host)
+  if (!host_)
     return false;
-  DCHECK(host->state() == GpuChannelHost::kConnected);
+  DCHECK(host_->state() == GpuChannelHost::kConnected);
+
+  const GPUInfo& gpu_info = host_->gpu_info();
+  UMA_HISTOGRAM_ENUMERATION(
+      "GPU.WebGraphicsContext3D_Init_CanLoseContext",
+      attributes.canRecoverFromContextLoss * 2 + gpu_info.can_lose_context,
+      4);
+  if (attributes.canRecoverFromContextLoss == false) {
+    if (gpu_info.can_lose_context)
+      return false;
+  }
+
+  if (web_view && web_view->mainFrame())
+    active_url_ = GURL(web_view->mainFrame()->document().url());
+
+  attributes_ = attributes;
+  render_directly_to_web_view_ = render_directly_to_web_view;
+  if (render_directly_to_web_view_) {
+    RenderView* renderview = RenderView::FromWebView(web_view);
+    if (!renderview)
+      return false;
+    render_view_routing_id_ = renderview->routing_id(),
+#ifndef WTF_USE_THREADED_COMPOSITING
+    web_view_ = web_view;
+  } else {
+    web_view_ = NULL;

[jamesr, 2011/08/25 20:05:53]

web_view_ is set to NULL in the c'tor so no need to set it here

[lain Merrick, 2011/08/25 20:11:17]

Done.

+#endif
+  }
+  return true;
+}
+
+bool WebGraphicsContext3DCommandBufferImpl::MaybeInitializeGL() {
+  if (context_) {
+    return true;
+  }
+  TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::MaybeInitializeGL");
 
   // Convert WebGL context creation attributes into RendererGLContext / EGL size
   // requests.
-  const int alpha_size = attributes.alpha ? 8 : 0;
-  const int depth_size = attributes.depth ? 24 : 0;
-  const int stencil_size = attributes.stencil ? 8 : 0;
-  const int samples = attributes.antialias ? 4 : 0;
-  const int sample_buffers = attributes.antialias ? 1 : 0;
+  const int alpha_size = attributes_.alpha ? 8 : 0;
+  const int depth_size = attributes_.depth ? 24 : 0;
+  const int stencil_size = attributes_.stencil ? 8 : 0;
+  const int samples = attributes_.antialias ? 4 : 0;
+  const int sample_buffers = attributes_.antialias ? 1 : 0;
   const int32 attribs[] = {
     RendererGLContext::ALPHA_SIZE, alpha_size,
     RendererGLContext::DEPTH_SIZE, depth_size,
     RendererGLContext::STENCIL_SIZE, stencil_size,
     RendererGLContext::SAMPLES, samples,
     RendererGLContext::SAMPLE_BUFFERS, sample_buffers,
     RendererGLContext::NONE,
   };
 
-  const char* preferred_extensions = attributes.noExtensions ?
-      kWebGLPreferredGLExtensions : "*";
-
-  const GPUInfo& gpu_info = host->gpu_info();
-  UMA_HISTOGRAM_ENUMERATION(
-      "GPU.WebGraphicsContext3D_Init_CanLoseContext",
-      attributes.canRecoverFromContextLoss * 2 + gpu_info.can_lose_context,
-      4);
-  if (attributes.canRecoverFromContextLoss == false) {
-    if (gpu_info.can_lose_context)
-      return false;
-  }
-
-  GURL active_url;
-  if (web_view && web_view->mainFrame())
-    active_url = GURL(web_view->mainFrame()->document().url());
-
   // HACK: Assume this is a WebGL context by looking for the noExtensions
   // attribute. WebGL contexts must not go in the share group because they
   // rely on destruction of the context to clean up owned resources. Putting
   // them in a share group would prevent this from happening.
+  base::AutoLock lock(g_all_contexts_lock.Get());

[jamesr, 2011/08/25 20:05:53]

this lock is held way too long, please tighten it up. I think you only need it
until line 151, and you don't need to take it at all unless the if on 149
succeeds

[lain Merrick, 2011/08/25 20:11:17]

I was thinking of commenting this. I think it's needed because if the context
lives in another thread, it could be destroyed between here and line 158.

Unlock on line 180, maybe? Or if it's safe (but suboptimal) to use a dead
context as the share group, I could just unlock on 151, yeah.

   RendererGLContext* share_group = NULL;
-  if (!attributes.noExtensions) {
+  if (!attributes_.noExtensions) {
     share_group = g_all_contexts.Pointer()->empty() ?
         NULL : (*g_all_contexts.Pointer()->begin())->context_;
   }
 
-  render_directly_to_web_view_ = render_directly_to_web_view;
-  if (render_directly_to_web_view) {
-#ifndef WTF_USE_THREADED_COMPOSITING
-    RenderView* renderview = RenderView::FromWebView(web_view);
-    if (!renderview)
-      return false;
-    web_view_ = web_view;
-#endif
+  const char* preferred_extensions = attributes_.noExtensions ?
+      kWebGLPreferredGLExtensions : "*";
+
+  if (render_directly_to_web_view_) {
     context_ = RendererGLContext::CreateViewContext(
-        host,
-        renderview->routing_id(),
-        !attributes.noExtensions,
+        host_,
+        render_view_routing_id_,
+        !attributes_.noExtensions,
         share_group,
         preferred_extensions,
         attribs,
-        active_url);
+        active_url_);
     if (context_) {
       context_->SetSwapBuffersCallback(
           NewCallback(this,
               &WebGraphicsContext3DCommandBufferImpl::OnSwapBuffersComplete));
     }
   } else {
     context_ = RendererGLContext::CreateOffscreenContext(
-        host,
+        host_,
         gfx::Size(1, 1),
-        !attributes.noExtensions,
+        !attributes_.noExtensions,
         share_group,
         preferred_extensions,
         attribs,
-        active_url);
-#ifndef WTF_USE_THREADED_COMPOSITING
-    web_view_ = NULL;
-#endif
+        active_url_);
   }
   if (!context_)
     return false;
 
   gl_ = context_->GetImplementation();
   context_->SetContextLostCallback(
       NewCallback(this,
                   &WebGraphicsContext3DCommandBufferImpl::OnContextLost));
 
   // TODO(gman): Remove this.
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kDisableGLSLTranslator)) {
     context_->DisableShaderTranslation();
   }
 
   // Set attributes_ from created offscreen context.
   {
-    attributes_ = attributes;
     GLint alpha_bits = 0;
     getIntegerv(GL_ALPHA_BITS, &alpha_bits);
     attributes_.alpha = alpha_bits > 0;
     GLint depth_bits = 0;
     getIntegerv(GL_DEPTH_BITS, &depth_bits);
     attributes_.depth = depth_bits > 0;
     GLint stencil_bits = 0;
     getIntegerv(GL_STENCIL_BITS, &stencil_bits);
     attributes_.stencil = stencil_bits > 0;
     GLint samples = 0;
     getIntegerv(GL_SAMPLES, &samples);
     attributes_.antialias = samples > 0;
   }
 
-  if (!attributes.noExtensions)
+  if (!attributes_.noExtensions)
     g_all_contexts.Pointer()->insert(this);

[jamesr, 2011/08/25 20:05:53]

you forgot a lock here

[lain Merrick, 2011/08/25 20:11:17]

Still inside the lock on line 147 (but maybe that's way too long)

 
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::makeContextCurrent() {
+  if (!MaybeInitializeGL())
+    return false;
   return RendererGLContext::MakeCurrent(context_);
 }
 
 int WebGraphicsContext3DCommandBufferImpl::width() {
   return cached_width_;
 }
 
 int WebGraphicsContext3DCommandBufferImpl::height() {
   return cached_height_;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::isGLES2Compliant() {
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::setParentContext(
     WebGraphicsContext3D* parent_context) {
   WebGraphicsContext3DCommandBufferImpl* parent_context_impl =
       static_cast<WebGraphicsContext3DCommandBufferImpl*>(parent_context);
   return context_->SetParent(
       parent_context_impl ? parent_context_impl->context() : NULL);
 }
 
 WebGLId WebGraphicsContext3DCommandBufferImpl::getPlatformTextureId() {
-  DCHECK(context_);
   return context_->GetParentTextureId();
 }
 
 void WebGraphicsContext3DCommandBufferImpl::prepareTexture() {
   // Copies the contents of the off-screen render target into the texture
   // used by the compositor.
 #ifndef WTF_USE_THREADED_COMPOSITING
   RenderView* renderview =
       web_view_ ? RenderView::FromWebView(web_view_) : NULL;
   if (renderview)
@@ skipping 824 matching lines @@
   }
 #ifndef WTF_USE_THREADED_COMPOSITING
   RenderView* renderview =
       web_view_ ? RenderView::FromWebView(web_view_) : NULL;
   if (renderview)
     renderview->OnViewContextSwapBuffersAborted();
 #endif
 }
 
 #endif  // defined(ENABLE_GPU)