Split WebGraphicsContext3DCommandBufferImpl::initialize() into two stages.

After the context is created, the remaining calls should be done on
the compositor thread. This CL adds a MaybeInitializeGL() method, which
we run on each call to makeContextCurrent().

Added asserts to RendererGLContext to check that it's always used on
the same thread.

Review URL: http://codereview.chromium.org/7713015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98392 0039d316-1c4b-4281-b951-d872f2087c98

content/renderer/gpu/renderer_gl_context.cc

@@ -10,6 +10,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
+#include "base/message_loop.h"
 #include "base/shared_memory.h"
 #include "content/common/view_messages.h"
 #include "content/renderer/gpu/command_buffer_proxy.h"
@@ -209,6 +210,7 @@ void RendererGLContext::SetContextLostCallback(
 
 bool RendererGLContext::MakeCurrent(RendererGLContext* context) {
   if (context) {
+    DCHECK(MessageLoop::current() == context->message_loop_);
     gles2::SetGLContext(context->gles2_implementation_);
 
     // Don't request latest error status from service. Just use the locally
@@ -284,7 +286,11 @@ RendererGLContext::RendererGLContext(GpuChannelHost* channel)
       transfer_buffer_id_(-1),
       gles2_implementation_(NULL),
       last_error_(SUCCESS),
-      frame_number_(0) {
+      frame_number_(0)
+#ifndef NDEBUG
+      , message_loop_(MessageLoop::current())
+#endif
+{
   DCHECK(channel);
 }
 
@@ -297,6 +303,7 @@ bool RendererGLContext::Initialize(bool onscreen,
                                    const char* allowed_extensions,
                                    const int32* attrib_list,
                                    const GURL& active_url) {
+  DCHECK(MessageLoop::current() == message_loop_);
   DCHECK(size.width() >= 0 && size.height() >= 0);
   TRACE_EVENT2("gpu", "RendererGLContext::Initialize",
                    "on_screen", onscreen, "num_pixels", size.GetArea());
@@ -415,6 +422,7 @@ bool RendererGLContext::Initialize(bool onscreen,
 
 void RendererGLContext::Destroy() {
   TRACE_EVENT0("gpu", "RendererGLContext::Destroy");
+  DCHECK(MessageLoop::current() == message_loop_);
   SetParent(NULL);
 
   if (gles2_implementation_) {

content/renderer/gpu/renderer_gl_context.h

@@ -201,6 +201,10 @@ class RendererGLContext : public base::SupportsWeakPtr<RendererGLContext> {
   gpu::gles2::GLES2Implementation* gles2_implementation_;
   Error last_error_;
   int frame_number_;
+#ifndef NDEBUG
+  // Used to assert that this object is used on a single thread.
+  MessageLoop* message_loop_;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(RendererGLContext);
 };

content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc

@@ -21,6 +21,7 @@
 #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"
@@ -32,6 +33,8 @@
 #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);
 
@@ -54,7 +57,10 @@ WebGraphicsContext3DCommandBufferImpl::WebGraphicsContext3DCommandBufferImpl()
 
 WebGraphicsContext3DCommandBufferImpl::
     ~WebGraphicsContext3DCommandBufferImpl() {
-  g_all_contexts.Pointer()->erase(this);
+  {
+    base::AutoLock lock(g_all_contexts_lock.Get());
+    g_all_contexts.Pointer()->erase(this);
+  }
   delete context_;
 }
 
@@ -70,38 +76,20 @@ 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);
-
-  // 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 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 : "*";
+  DCHECK(host_->state() == GpuChannelHost::kConnected);
 
-  const GPUInfo& gpu_info = host->gpu_info();
+  const GPUInfo& gpu_info = host_->gpu_info();
   UMA_HISTOGRAM_ENUMERATION(
       "GPU.WebGraphicsContext3D_Init_CanLoseContext",
       attributes.canRecoverFromContextLoss * 2 + gpu_info.can_lose_context,
@@ -111,53 +99,86 @@ bool WebGraphicsContext3DCommandBufferImpl::initialize(
       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) {
-    share_group = g_all_contexts.Pointer()->empty() ?
-        NULL : (*g_all_contexts.Pointer()->begin())->context_;
-  }
+    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) {
-#ifndef WTF_USE_THREADED_COMPOSITING
+  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;
 #endif
-    context_ = RendererGLContext::CreateViewContext(
-        host,
-        renderview->routing_id(),
-        !attributes.noExtensions,
-        share_group,
-        preferred_extensions,
-        attribs,
-        active_url);
-    if (context_) {
-      context_->SetSwapBuffersCallback(
-          NewCallback(this,
+  }
+  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 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,
+  };
+
+  // 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.
+  {
+      // Need to hold this lock until after RendererGLContext::Create, to ensure
+      // the context we picked for our share group isn't deleted (there's also
+      // a lock in the destructor).
+      base::AutoLock lock(g_all_contexts_lock.Get());
+      RendererGLContext* share_group = NULL;
+      if (!attributes_.noExtensions) {
+        share_group = g_all_contexts.Pointer()->empty() ?
+            NULL : (*g_all_contexts.Pointer()->begin())->context_;
+      }
+
+      const char* preferred_extensions = attributes_.noExtensions ?
+          kWebGLPreferredGLExtensions : "*";
+
+      if (render_directly_to_web_view_) {
+        context_ = RendererGLContext::CreateViewContext(
+            host_,
+            render_view_routing_id_,
+            !attributes_.noExtensions,
+            share_group,
+            preferred_extensions,
+            attribs,
+            active_url_);
+        if (context_) {
+          context_->SetSwapBuffersCallback(NewCallback(this,
               &WebGraphicsContext3DCommandBufferImpl::OnSwapBuffersComplete));
-    }
-  } else {
-    context_ = RendererGLContext::CreateOffscreenContext(
-        host,
-        gfx::Size(1, 1),
-        !attributes.noExtensions,
-        share_group,
-        preferred_extensions,
-        attribs,
-        active_url);
-#ifndef WTF_USE_THREADED_COMPOSITING
-    web_view_ = NULL;
-#endif
+        }
+      } else {
+        context_ = RendererGLContext::CreateOffscreenContext(
+            host_,
+            gfx::Size(1, 1),
+            !attributes_.noExtensions,
+            share_group,
+            preferred_extensions,
+            attribs,
+            active_url_);
+      }
   }
   if (!context_)
     return false;
@@ -175,7 +196,6 @@ bool WebGraphicsContext3DCommandBufferImpl::initialize(
 
   // Set attributes_ from created offscreen context.
   {
-    attributes_ = attributes;
     GLint alpha_bits = 0;
     getIntegerv(GL_ALPHA_BITS, &alpha_bits);
     attributes_.alpha = alpha_bits > 0;
@@ -190,13 +210,17 @@ bool WebGraphicsContext3DCommandBufferImpl::initialize(
     attributes_.antialias = samples > 0;
   }
 
-  if (!attributes.noExtensions)
+  if (!attributes_.noExtensions) {
+    base::AutoLock lock(g_all_contexts_lock.Get());
     g_all_contexts.Pointer()->insert(this);
+  }
 
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::makeContextCurrent() {
+  if (!MaybeInitializeGL())
+    return false;
   return RendererGLContext::MakeCurrent(context_);
 }
 
@@ -221,7 +245,6 @@ bool WebGraphicsContext3DCommandBufferImpl::setParentContext(
 }
 
 WebGLId WebGraphicsContext3DCommandBufferImpl::getPlatformTextureId() {
-  DCHECK(context_);
   return context_->GetParentTextureId();
 }
 

content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h

@@ -12,6 +12,7 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "content/renderer/gpu/renderer_gl_context.h"
+#include "googleurl/src/gurl.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebGraphicsContext3D.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
@@ -57,6 +58,10 @@ class WebGraphicsContext3DCommandBufferImpl
                           WebKit::WebView*,
                           bool renderDirectlyToWebView);
 
+  // Must be called after initialize() and before any of the following methods.
+  // Binds this object to the current thread. We can't bind to multiple threads.
+  // TODO(husky): Document threading restrictions in WebGraphicsContext3D.h,
+  // enforce strictly in all implementations.
   virtual bool makeContextCurrent();
 
   virtual int width();
@@ -431,6 +436,12 @@ class WebGraphicsContext3DCommandBufferImpl
           WebGraphicsSwapBuffersCompleteCallbackCHROMIUM* callback);
 
  private:
+  // Initialize the underlying GL context. May be called multiple times; second
+  // and subsequent calls are ignored. Must be called from the thread that is
+  // going to use this object to issue GL commands (which might not be the main
+  // thread).
+  bool MaybeInitializeGL();
+
   // SwapBuffers callback.
   void OnSwapBuffersComplete();
   virtual void OnContextLost(RendererGLContext::ContextLostReason reason);
@@ -440,6 +451,11 @@ class WebGraphicsContext3DCommandBufferImpl
   // The GLES2Implementation we use for OpenGL rendering.
   gpu::gles2::GLES2Implementation* gl_;
 
+  // State needed by InitializeGLES2.
+  GpuChannelHost* host_;
+  GURL active_url_;
+  int32 render_view_routing_id_;
+
   bool render_directly_to_web_view_;
 #ifndef WTF_USE_THREADED_COMPOSITING
   // If rendering directly to WebView, weak pointer to it.