Revert 99648 - Reland 98392 - 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.

Original review URL: http://codereview.chromium.org/7713015
Reverted due to buildbot errors: http://codereview.chromium.org/7745042
Review URL: http://codereview.chromium.org/7828046

TBR=husky@google.com
Review URL: http://codereview.chromium.org/7838008

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

3 files changed, 59 insertions, 104 deletions

diff --git a/content/renderer/gpu/renderer_gl_context.cc b/content/renderer/gpu/renderer_gl_context.cc
index 255bd13..6320c80b 100644
--- a/content/renderer/gpu/renderer_gl_context.cc
+++ b/content/renderer/gpu/renderer_gl_context.cc
@@ -199,7 +199,6 @@ void RendererGLContext::SetContextLostCallback(
 
 bool RendererGLContext::MakeCurrent(RendererGLContext* context) {
   if (context) {
-    DCHECK(context->CalledOnValidThread());
     gles2::SetGLContext(context->gles2_implementation_);
 
     // Don't request latest error status from service. Just use the locally
@@ -291,7 +290,6 @@ bool RendererGLContext::Initialize(bool onscreen,
                                    const char* allowed_extensions,
                                    const int32* attrib_list,
                                    const GURL& active_url) {
-  DCHECK(CalledOnValidThread());
   DCHECK(size.width() >= 0 && size.height() >= 0);
   TRACE_EVENT2("gpu", "RendererGLContext::Initialize",
                    "on_screen", onscreen, "num_pixels", size.GetArea());
@@ -415,7 +413,6 @@ bool RendererGLContext::Initialize(bool onscreen,
 
 void RendererGLContext::Destroy() {
   TRACE_EVENT0("gpu", "RendererGLContext::Destroy");
-  DCHECK(CalledOnValidThread());
   SetParent(NULL);
 
   if (gles2_implementation_) {
diff --git a/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc b/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc
index 72d1a57..fb4cdb3 100644
--- a/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc
+++ b/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc
@@ -21,7 +21,6 @@
 #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"
@@ -33,8 +32,6 @@
 #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_shared_contexts_lock(base::LINKER_INITIALIZED);
 static base::LazyInstance<std::set<WebGraphicsContext3DCommandBufferImpl*> >
     g_all_shared_contexts(base::LINKER_INITIALIZED);
 
@@ -58,10 +55,7 @@ WebGraphicsContext3DCommandBufferImpl::WebGraphicsContext3DCommandBufferImpl()
 
 WebGraphicsContext3DCommandBufferImpl::
     ~WebGraphicsContext3DCommandBufferImpl() {
-  {
-    base::AutoLock lock(g_all_shared_contexts_lock.Get());
-    g_all_shared_contexts.Pointer()->erase(this);
-  }
+  g_all_shared_contexts.Pointer()->erase(this);
   delete context_;
 }
 
@@ -77,102 +71,85 @@ bool WebGraphicsContext3DCommandBufferImpl::initialize(
     WebGraphicsContext3D::Attributes attributes,
     WebKit::WebView* web_view,
     bool render_directly_to_web_view) {
-  DCHECK(!context_);
   TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::initialize");
   RenderThread* render_thread = RenderThread::current();
   if (!render_thread)
     return false;
-  host_ = render_thread->EstablishGpuChannelSync(
+  GpuChannelHost* host = render_thread->EstablishGpuChannelSync(
     content::
       CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE);
-  if (!host_)
+  if (!host)
     return false;
-  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* render_view = RenderView::FromWebView(web_view);
-    if (!render_view)
-      return false;
-    render_view_routing_id_ = render_view->routing_id();
-#ifndef WTF_USE_THREADED_COMPOSITING
-    web_view_ = web_view;
-#endif
-  }
-  return true;
-}
-
-bool WebGraphicsContext3DCommandBufferImpl::MaybeInitializeGL() {
-  if (context_)
-    return true;
-
-  TRACE_EVENT0("gpu", "WebGfxCtx3DCmdBfrImpl::MaybeInitializeGL");
+  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 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::SHARE_RESOURCES, attributes_.shareResources ? 1 : 0,
+    RendererGLContext::SHARE_RESOURCES, attributes.shareResources ? 1 : 0,
     RendererGLContext::BIND_GENERATES_RESOURCES, 0,
     RendererGLContext::NONE,
   };
 
-  const char* preferred_extensions = attributes_.noExtensions ?
+  const char* preferred_extensions = attributes.noExtensions ?
       kWebGLPreferredGLExtensions : "*";
 
-  // We need to lock g_all_shared_contexts until after RendererGLContext::Create
-  // to ensure that the context we picked for our share group isn't deleted.
-  // (There's also a lock in our destructor.)
-  {
-    base::AutoLock lock(g_all_shared_contexts_lock.Get());
-    RendererGLContext* share_group = NULL;
-    if (attributes_.shareResources) {
-      share_group = g_all_shared_contexts.Pointer()->empty() ?
-          NULL : (*g_all_shared_contexts.Pointer()->begin())->context_;
-    }
-
-    if (render_directly_to_web_view_) {
-      context_ = RendererGLContext::CreateViewContext(
-          host_,
-          render_view_routing_id_,
-          share_group,
-          preferred_extensions,
-          attribs,
-          active_url_);
-    } else {
-      context_ = RendererGLContext::CreateOffscreenContext(
-          host_,
-          gfx::Size(1, 1),
-          share_group,
-          preferred_extensions,
-          attribs,
-          active_url_);
-    }
+  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());
+
+  RendererGLContext* share_group = NULL;
+  if (attributes.shareResources) {
+    share_group = g_all_shared_contexts.Pointer()->empty() ?
+        NULL : (*g_all_shared_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
+    context_ = RendererGLContext::CreateViewContext(
+        host,
+        renderview->routing_id(),
+        share_group,
+        preferred_extensions,
+        attribs,
+        active_url);
+  } else {
+    context_ = RendererGLContext::CreateOffscreenContext(
+        host,
+        gfx::Size(1, 1),
+        share_group,
+        preferred_extensions,
+        attribs,
+        active_url);
+#ifndef WTF_USE_THREADED_COMPOSITING
+    web_view_ = NULL;
+#endif
+  }
   if (!context_)
     return false;
 
@@ -189,6 +166,7 @@ bool WebGraphicsContext3DCommandBufferImpl::MaybeInitializeGL() {
 
   // Set attributes_ from created offscreen context.
   {
+    attributes_ = attributes;
     GLint alpha_bits = 0;
     getIntegerv(GL_ALPHA_BITS, &alpha_bits);
     attributes_.alpha = alpha_bits > 0;
@@ -203,17 +181,13 @@ bool WebGraphicsContext3DCommandBufferImpl::MaybeInitializeGL() {
     attributes_.antialias = samples > 0;
   }
 
-  if (attributes_.shareResources) {
-    base::AutoLock lock(g_all_shared_contexts_lock.Get());
+  if (attributes.shareResources)
     g_all_shared_contexts.Pointer()->insert(this);
-  }
 
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::makeContextCurrent() {
-  if (!MaybeInitializeGL())
-    return false;
   return RendererGLContext::MakeCurrent(context_);
 }
 
@@ -238,6 +212,7 @@ bool WebGraphicsContext3DCommandBufferImpl::setParentContext(
 }
 
 WebGLId WebGraphicsContext3DCommandBufferImpl::getPlatformTextureId() {
+  DCHECK(context_);
   return context_->GetParentTextureId();
 }
 
diff --git a/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h b/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h
index e2afb4f..0749fa7 100644
--- a/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h
+++ b/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h
@@ -13,7 +13,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/task.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"
@@ -59,11 +58,6 @@ class WebGraphicsContext3DCommandBufferImpl
                           WebKit::WebView*,
                           bool renderDirectlyToWebView);
 
-  // Must be called after initialize() and before any of the following methods.
-  // Permanently binds to the first calling thread. Returns false if the
-  // graphics context fails to create. Do not call from more than one thread.
-  // TODO(husky): Document threading restrictions in WebGraphicsContext3D.h,
-  // enforce strictly in all implementations.
   virtual bool makeContextCurrent();
 
   virtual int width();
@@ -443,12 +437,6 @@ class WebGraphicsContext3DCommandBufferImpl
 #endif
 
  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);
@@ -458,11 +446,6 @@ class WebGraphicsContext3DCommandBufferImpl
   // The GLES2Implementation we use for OpenGL rendering.
   gpu::gles2::GLES2Implementation* gl_;
 
-  // State needed by MaybeInitializeGL.
-  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.