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
@@ skipping 57 matching lines @@
 
 bool WebGraphicsContext3DCommandBufferImpl::initialize(
     WebGraphicsContext3D::Attributes attributes,
     WebKit::WebView* web_view,
     bool render_directly_to_web_view) {
   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;
+#endif
+  }
+  context_ = NULL;
+  gl_ = NULL;
+  return true;
+}
+
+bool WebGraphicsContext3DCommandBufferImpl::InitializeGLES2() {
+  DCHECK(!context_);
+  TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::InitializeGLES2");
 
   // 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 ?
+  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.
   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 (render_directly_to_web_view_) {
+    RenderView* renderview = RenderView::FromWebView(web_view_);

[jamesr, 2011/08/23 01:23:23]

this is highly thread unsafe - it depends on a global view map that has no
mutexes or other synchronization primitives.  Is the idea that either:
1.) you'll make this call threadsafe?
2.) you won't use this in the threaded path?
3.) ????

[lain Merrick, 2011/08/23 01:31:03]

Ack, seem to have lost my earlier CL while updating this client. We need those
#ifndef lines for a start. So, I think the answer is 2, but I need to fix this
merge first.

     if (!renderview)
       return false;
-    web_view_ = web_view;

[nduca, 2011/08/23 05:08:19]

I guess this whole block is gonna go away?

-#endif
     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/23 01:23:23]

It does not appear that you've made this threadsafe either, although if I
understand correctly this function is supposed to be called from a background
thread.

[lain Merrick, 2011/08/23 01:31:03]

Will fix.

[nduca, 2011/08/23 05:08:19]

Hmm, lets talk about this tomorrow... I'd like to understand what this
g_all_contexts thing is and whether we can do away with it entirely.. I'm
concerned about mutex-guarding this and possible side-effects of that.

 
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::makeContextCurrent() {
+  if (!context_) {
+    if (!InitializeGLES2()) {

[nduca, 2011/08/23 05:08:19]

or maybe InitializeContext

[lain Merrick, 2011/08/23 15:14:01]

That would work, I really don't mind. I think I went with this name because it's
initializing the GLES2Implementation object. (Although the member variable is
just gl_ so it could equally be InitializeGL.)

+      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)