Move GLHelper ownership to ImageTransportFactory, and make it thread safe

There were several problems with previous implementation:
- there was a use-after-free in CopyTextureToImpl. ReadBackFramebufferComplete was responsible for deleting the texture, but in some cases it could be called after the GLHelper was destroyed, accessing a deleted field, which could have been overwritten (i.e. we'll delete the wrong texture).
- When creating thumbnails we would create a GLHelper per page, which creates a context (and some resources). That context doesn't go away until we close the page, it's wasteful.
- If we create the thumbnail on closing the page, we would block

Instead, this patch changes it to create a single GLHelper and fixes the cancelling semantics so that we don't have use-after-frees

BUG=123933
TEST=check for no leak

Review URL: https://chromiumcodereview.appspot.com/10290007

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

6 files changed, 300 insertions, 208 deletions

diff --git a/content/browser/renderer_host/image_transport_factory.cc b/content/browser/renderer_host/image_transport_factory.cc
index 9fb5fbc..1a9dc44 100644
--- a/content/browser/renderer_host/image_transport_factory.cc
+++ b/content/browser/renderer_host/image_transport_factory.cc
@@ -14,6 +14,7 @@
 #include "content/browser/gpu/browser_gpu_channel_host_factory.h"
 #include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/browser/renderer_host/image_transport_client.h"
+#include "content/common/gpu/client/gl_helper.h"
 #include "content/common/gpu/client/gpu_channel_host.h"
 #include "content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h"
 #include "content/common/gpu/gpu_process_launch_causes.h"
@@ -30,6 +31,7 @@
 #include "webkit/gpu/webgraphicscontext3d_in_process_impl.h"
 
 using content::BrowserGpuChannelHostFactory;
+using content::GLHelper;
 
 namespace {
 
@@ -56,17 +58,16 @@ class DefaultTransportFactory
       gfx::GLSurfaceHandle surface) OVERRIDE {
   }
 
-  virtual WebKit::WebGraphicsContext3D* GetSharedContext(
-      ui::Compositor* compositor) OVERRIDE {
-    return NULL;
-  }
-
   virtual scoped_refptr<ImageTransportClient> CreateTransportClient(
       const gfx::Size& size,
       uint64* transport_handle) OVERRIDE {
     return NULL;
   }
 
+  virtual GLHelper* GetGLHelper(ui::Compositor* compositor) OVERRIDE {
+    return NULL;
+  }
+
   virtual gfx::ScopedMakeCurrent* GetScopedMakeCurrent() OVERRIDE {
     return NULL;
   }
@@ -141,12 +142,6 @@ class InProcessTransportFactory : public DefaultTransportFactory {
     return gfx::GLSurfaceHandle(gfx::kNullPluginWindow, true);
   }
 
-  virtual WebKit::WebGraphicsContext3D* GetSharedContext(
-      ui::Compositor* compositor) OVERRIDE {
-    // TODO(mazda): Implement this (http://crbug.com/118546).
-    return NULL;
-  }
-
   virtual scoped_refptr<ImageTransportClient> CreateTransportClient(
       const gfx::Size& size,
       uint64* transport_handle) OVERRIDE {
@@ -159,6 +154,11 @@ class InProcessTransportFactory : public DefaultTransportFactory {
     return surface;
   }
 
+  virtual GLHelper* GetGLHelper(ui::Compositor* compositor) OVERRIDE {
+    // TODO(mazda): Implement this (http://crbug.com/118546).
+    return NULL;
+  }
+
   // We don't generate lost context events, so we don't need to keep track of
   // observers
   virtual void AddObserver(ImageTransportFactoryObserver* observer) OVERRIDE {
@@ -308,15 +308,6 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
     }
   }
 
-  virtual WebKit::WebGraphicsContext3D* GetSharedContext(
-      ui::Compositor* compositor) OVERRIDE {
-    PerCompositorDataMap::const_iterator itr =
-        per_compositor_data_.find(compositor);
-    if (itr == per_compositor_data_.end())
-      return NULL;
-    return itr->second->shared_context.get();
-  }
-
   virtual scoped_refptr<ImageTransportClient> CreateTransportClient(
       const gfx::Size& size,
       uint64* transport_handle) {
@@ -326,6 +317,21 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
     return image;
   }
 
+  virtual GLHelper* GetGLHelper(ui::Compositor* compositor) {
+    PerCompositorData* data = per_compositor_data_[compositor];
+    if (!data)
+      data = CreatePerCompositorData(compositor);
+    if (!data->gl_helper.get()) {
+      WebKit::WebGraphicsContext3D* context_for_thread =
+          CreateContextCommon(compositor, true);
+      if (!context_for_thread)
+        return NULL;
+      data->gl_helper.reset(new GLHelper(data->shared_context.get(),
+                                         context_for_thread));
+    }
+    return data->gl_helper.get();
+  }
+
   virtual gfx::ScopedMakeCurrent* GetScopedMakeCurrent() { return NULL; }
 
   virtual void AddObserver(ImageTransportFactoryObserver* observer) {
@@ -341,6 +347,12 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
     PerCompositorData* data = per_compositor_data_[compositor];
     DCHECK(data);
 
+    // Keep old resources around while we call the observers, but ensure that
+    // new resources are created if needed.
+    scoped_ptr<WebGraphicsContext3DCommandBufferImpl> old_shared_context(
+        data->shared_context.release());
+    scoped_ptr<GLHelper> old_helper(data->gl_helper.release());
+
     // Note: this has the effect of recreating the swap_client, which means we
     // won't get more reports of lost context from the same gpu process. It's a
     // good thing.
@@ -357,6 +369,7 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
     int surface_id;
     scoped_ptr<CompositorSwapClient> swap_client;
     scoped_ptr<WebGraphicsContext3DCommandBufferImpl> shared_context;
+    scoped_ptr<GLHelper> gl_helper;
   };
 
   PerCompositorData* CreatePerCompositorData(ui::Compositor* compositor) {
diff --git a/content/browser/renderer_host/image_transport_factory.h b/content/browser/renderer_host/image_transport_factory.h
index 9416afd..600f314 100644
--- a/content/browser/renderer_host/image_transport_factory.h
+++ b/content/browser/renderer_host/image_transport_factory.h
@@ -9,6 +9,10 @@
 #include "base/memory/ref_counted.h"
 #include "ui/gfx/native_widget_types.h"
 
+namespace content {
+class GLHelper;
+}
+
 namespace gfx {
 class Size;
 class ScopedMakeCurrent;
@@ -33,6 +37,10 @@ class ImageTransportFactoryObserver {
 
   // Notifies that the surface handles generated by ImageTransportFactory for a
   // given compositor were lost.
+  // When this is called, the old resources (e.g. shared context, GL helper)
+  // still exist, but are about to be destroyed. Getting a reference to those
+  // resources from the ImageTransportFactory (e.g. through GetGLHelper) will
+  // return newly recreated, valid resources.
   virtual void OnLostResources(ui::Compositor* compositor) = 0;
 };
 
@@ -67,19 +75,16 @@ class ImageTransportFactory {
   // Destroys a shared surface handle.
   virtual void DestroySharedSurfaceHandle(gfx::GLSurfaceHandle surface) = 0;
 
-  // Returns a shared context associated with the compositor. Returns NULL if
-  // no context is associated with the compositor.
-  // Note: the context may get lost at any time, a state that an
-  // ImageTransportFactoryObserver gets notified of.
-  virtual WebKit::WebGraphicsContext3D* GetSharedContext(
-      ui::Compositor* compositor) = 0;
-
   // Creates a transport client of a given size, and using the opaque handle
   // sent by the GPU process.
   virtual scoped_refptr<ImageTransportClient> CreateTransportClient(
       const gfx::Size& size,
       uint64* transport_handle) = 0;
 
+  // Gets a GLHelper instance, associated with the compositor context. This
+  // GLHelper will get destroyed whenever the context is lost (
+  virtual content::GLHelper* GetGLHelper(ui::Compositor* compositor) = 0;
+
   // Returns a ScopedMakeCurrent that can be used to make current a context that
   // is shared with the compositor context, e.g. to create a texture in its
   // namespace. The caller gets ownership of the object.
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 8f16ab9..8e2aa25 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -8,6 +8,7 @@
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
+#include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "content/browser/renderer_host/backing_store_skia.h"
 #include "content/browser/renderer_host/image_transport_client.h"
@@ -104,18 +105,6 @@ bool CanRendererHandleEvent(const aura::MouseEvent* event) {
   return true;
 }
 
-// Creates a content::GLHelper for the given compositor.
-content::GLHelper* CreateGLHelper(ui::Compositor* compositor) {
-  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
-  WebKit::WebGraphicsContext3D* context =
-      factory->GetSharedContext(compositor);
-  DCHECK(context);
-  WebKit::WebGraphicsContext3D* context_for_thread =
-      factory->AsContextFactory()->CreateOffscreenContext(compositor);
-  DCHECK(context_for_thread);
-  return new content::GLHelper(context, context_for_thread);
-}
-
 void GetScreenInfoForWindow(WebKit::WebScreenInfo* results,
                             aura::Window* window) {
   const gfx::Monitor monitor = window ?
@@ -449,15 +438,17 @@ bool RenderWidgetHostViewAura::CopyFromCompositingSurface(
   if (!output->initialize(size.width(), size.height(), true))
     return false;
 
-  if (!gl_helper_.get())
-    gl_helper_.reset(CreateGLHelper(compositor));
+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  content::GLHelper* gl_helper = factory->GetGLHelper(compositor);
+  if (!gl_helper)
+    return false;
 
   unsigned char* addr = static_cast<unsigned char*>(
       output->getTopDevice()->accessBitmap(true).getPixels());
-  return gl_helper_->CopyTextureTo(container->texture_id(),
-                                   container->size(),
-                                   size,
-                                   addr);
+  return gl_helper->CopyTextureTo(container->texture_id(),
+                                  container->size(),
+                                  size,
+                                  addr);
 }
 
 void RenderWidgetHostViewAura::AsyncCopyFromCompositingSurface(
@@ -476,17 +467,19 @@ void RenderWidgetHostViewAura::AsyncCopyFromCompositingSurface(
   if (!output->initialize(size.width(), size.height(), true))
     return;
 
-  if (!gl_helper_.get())
-    gl_helper_.reset(CreateGLHelper(compositor));
+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  content::GLHelper* gl_helper = factory->GetGLHelper(compositor);
+  if (!gl_helper)
+    return;
 
   scoped_callback_runner.Release();
   unsigned char* addr = static_cast<unsigned char*>(
       output->getTopDevice()->accessBitmap(true).getPixels());
-  gl_helper_->AsyncCopyTextureTo(container->texture_id(),
-                                 container->size(),
-                                 size,
-                                 addr,
-                                 callback);
+  gl_helper->AsyncCopyTextureTo(container->texture_id(),
+                                container->size(),
+                                size,
+                                addr,
+                                callback);
 }
 
 void RenderWidgetHostViewAura::OnAcceleratedCompositingStateChange() {
@@ -1178,13 +1171,6 @@ void RenderWidgetHostViewAura::OnLostResources(ui::Compositor* compositor) {
   shared_surface_handle_ = factory->CreateSharedSurfaceHandle(compositor);
   host_->CompositingSurfaceUpdated();
   host_->ScheduleComposite();
-
-  if (gl_helper_.get()) {
-    // Detach the resources to avoid deleting them using the invalid context.
-    // They've been released anyway when the GPU process died.
-    gl_helper_->Detach();
-    gl_helper_.reset(CreateGLHelper(compositor));
-  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h b/content/browser/renderer_host/render_widget_host_view_aura.h
index 15857e8..b4bcef1 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.h
+++ b/content/browser/renderer_host/render_widget_host_view_aura.h
@@ -23,7 +23,6 @@
 #include "webkit/glue/webcursor.h"
 
 namespace content {
-class GLHelper;
 class RenderWidgetHostImpl;
 class RenderWidgetHostView;
 }
@@ -225,8 +224,6 @@ class RenderWidgetHostViewAura
   // The model object.
   content::RenderWidgetHostImpl* host_;
 
-  scoped_ptr<content::GLHelper> gl_helper_;
-
   aura::Window* window_;
 
   scoped_ptr<WindowObserver> window_observer_;
diff --git a/content/common/gpu/client/gl_helper.cc b/content/common/gpu/client/gl_helper.cc
index 38106be..a850e95f 100644
--- a/content/common/gpu/client/gl_helper.cc
+++ b/content/common/gpu/client/gl_helper.cc
@@ -4,10 +4,14 @@
 
 #include "content/common/gpu/client/gl_helper.h"
 
+#include <queue>
+
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
+#include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
@@ -16,6 +20,9 @@
 #include "ui/gfx/gl/gl_bindings.h"
 #include "ui/gfx/size.h"
 
+using WebKit::WebGLId;
+using WebKit::WebGraphicsContext3D;
+
 namespace {
 
 const char kGLHelperThreadName[] = "GLHelperThread";
@@ -35,21 +42,23 @@ base::LazyInstance<GLHelperThread> g_gl_helper_thread =
 
 class ScopedWebGLId {
  public:
-  typedef void (WebKit::WebGraphicsContext3D::*DeleteFunc)(WebKit::WebGLId);
-  ScopedWebGLId(WebKit::WebGraphicsContext3D* context,
-                WebKit::WebGLId id,
+  typedef void (WebGraphicsContext3D::*DeleteFunc)(WebGLId);
+  ScopedWebGLId(WebGraphicsContext3D* context,
+                WebGLId id,
                 DeleteFunc delete_func)
       : context_(context),
         id_(id),
         delete_func_(delete_func) {
   }
 
-  operator WebKit::WebGLId() const {
+  operator WebGLId() const {
     return id_;
   }
 
-  WebKit::WebGLId Detach() {
-    WebKit::WebGLId id = id_;
+  WebGLId id() const { return id_; }
+
+  WebGLId Detach() {
+    WebGLId id = id_;
     id_ = 0;
     return id;
   }
@@ -60,8 +69,8 @@ class ScopedWebGLId {
   }
 
  private:
-  WebKit::WebGraphicsContext3D* context_;
-  WebKit::WebGLId id_;
+  WebGraphicsContext3D* context_;
+  WebGLId id_;
   DeleteFunc delete_func_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedWebGLId);
@@ -69,65 +78,65 @@ class ScopedWebGLId {
 
 class ScopedBuffer : public ScopedWebGLId {
  public:
-  ScopedBuffer(WebKit::WebGraphicsContext3D* context,
-               WebKit::WebGLId id)
+  ScopedBuffer(WebGraphicsContext3D* context,
+               WebGLId id)
       : ScopedWebGLId(context,
                       id,
-                      &WebKit::WebGraphicsContext3D::deleteBuffer) {}
+                      &WebGraphicsContext3D::deleteBuffer) {}
 };
 
 class ScopedFramebuffer : public ScopedWebGLId {
  public:
-  ScopedFramebuffer(WebKit::WebGraphicsContext3D* context,
-                    WebKit::WebGLId id)
+  ScopedFramebuffer(WebGraphicsContext3D* context,
+                    WebGLId id)
       : ScopedWebGLId(context,
                       id,
-                      &WebKit::WebGraphicsContext3D::deleteFramebuffer) {}
+                      &WebGraphicsContext3D::deleteFramebuffer) {}
 };
 
 class ScopedRenderbuffer : public ScopedWebGLId {
  public:
-  ScopedRenderbuffer(WebKit::WebGraphicsContext3D* context,
-                     WebKit::WebGLId id)
+  ScopedRenderbuffer(WebGraphicsContext3D* context,
+                     WebGLId id)
       : ScopedWebGLId(context,
                       id,
-                      &WebKit::WebGraphicsContext3D::deleteRenderbuffer) {}
+                      &WebGraphicsContext3D::deleteRenderbuffer) {}
 };
 
 class ScopedProgram : public ScopedWebGLId {
  public:
-  ScopedProgram(WebKit::WebGraphicsContext3D* context,
-                WebKit::WebGLId id)
+  ScopedProgram(WebGraphicsContext3D* context,
+                WebGLId id)
       : ScopedWebGLId(context,
                       id,
-                      &WebKit::WebGraphicsContext3D::deleteProgram) {}
+                      &WebGraphicsContext3D::deleteProgram) {}
 };
 
 class ScopedShader : public ScopedWebGLId {
  public:
-  ScopedShader(WebKit::WebGraphicsContext3D* context,
-               WebKit::WebGLId id)
+  ScopedShader(WebGraphicsContext3D* context,
+               WebGLId id)
       : ScopedWebGLId(context,
                       id,
-                      &WebKit::WebGraphicsContext3D::deleteShader) {}
+                      &WebGraphicsContext3D::deleteShader) {}
 };
 
 class ScopedTexture : public ScopedWebGLId {
  public:
-  ScopedTexture(WebKit::WebGraphicsContext3D* context,
-                WebKit::WebGLId id)
+  ScopedTexture(WebGraphicsContext3D* context,
+                WebGLId id)
       : ScopedWebGLId(context,
                       id,
-                      &WebKit::WebGraphicsContext3D::deleteTexture) {}
+                      &WebGraphicsContext3D::deleteTexture) {}
 };
 
 template <WebKit::WGC3Denum target>
 class ScopedBinder {
  public:
-  typedef void (WebKit::WebGraphicsContext3D::*BindFunc)(WebKit::WGC3Denum,
-                                                         WebKit::WebGLId);
-  ScopedBinder(WebKit::WebGraphicsContext3D* context,
-               WebKit::WebGLId id,
+  typedef void (WebGraphicsContext3D::*BindFunc)(WebKit::WGC3Denum,
+                                                         WebGLId);
+  ScopedBinder(WebGraphicsContext3D* context,
+               WebGLId id,
                BindFunc bind_func)
       : context_(context),
         id_(id),
@@ -141,8 +150,8 @@ class ScopedBinder {
   }
 
  private:
-  WebKit::WebGraphicsContext3D* context_;
-  WebKit::WebGLId id_;
+  WebGraphicsContext3D* context_;
+  WebGLId id_;
   BindFunc bind_func_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedBinder);
@@ -151,50 +160,50 @@ class ScopedBinder {
 template <WebKit::WGC3Denum target>
 class ScopedBufferBinder : ScopedBinder<target> {
  public:
-  ScopedBufferBinder(WebKit::WebGraphicsContext3D* context,
-                     WebKit::WebGLId id)
+  ScopedBufferBinder(WebGraphicsContext3D* context,
+                     WebGLId id)
       : ScopedBinder<target>(
           context,
           id,
-          &WebKit::WebGraphicsContext3D::bindBuffer) {}
+          &WebGraphicsContext3D::bindBuffer) {}
 };
 
 template <WebKit::WGC3Denum target>
 class ScopedFramebufferBinder : ScopedBinder<target> {
  public:
-  ScopedFramebufferBinder(WebKit::WebGraphicsContext3D* context,
-                          WebKit::WebGLId id)
+  ScopedFramebufferBinder(WebGraphicsContext3D* context,
+                          WebGLId id)
       : ScopedBinder<target>(
           context,
           id,
-          &WebKit::WebGraphicsContext3D::bindFramebuffer) {}
+          &WebGraphicsContext3D::bindFramebuffer) {}
 };
 
 template <WebKit::WGC3Denum target>
 class ScopedRenderbufferBinder : ScopedBinder<target> {
  public:
-  ScopedRenderbufferBinder(WebKit::WebGraphicsContext3D* context,
-                           WebKit::WebGLId id)
+  ScopedRenderbufferBinder(WebGraphicsContext3D* context,
+                           WebGLId id)
       : ScopedBinder<target>(
           context,
           id,
-          &WebKit::WebGraphicsContext3D::bindRenderbuffer) {}
+          &WebGraphicsContext3D::bindRenderbuffer) {}
 };
 
 template <WebKit::WGC3Denum target>
 class ScopedTextureBinder : ScopedBinder<target> {
  public:
-  ScopedTextureBinder(WebKit::WebGraphicsContext3D* context,
-                      WebKit::WebGLId id)
+  ScopedTextureBinder(WebGraphicsContext3D* context,
+                      WebGLId id)
       : ScopedBinder<target>(
           context,
           id,
-          &WebKit::WebGraphicsContext3D::bindTexture) {}
+          &WebGraphicsContext3D::bindTexture) {}
 };
 
 class ScopedFlush {
  public:
-  ScopedFlush(WebKit::WebGraphicsContext3D* context)
+  ScopedFlush(WebGraphicsContext3D* context)
       : context_(context) {
   }
 
@@ -203,56 +212,12 @@ class ScopedFlush {
   }
 
  private:
-  WebKit::WebGraphicsContext3D* context_;
+  WebGraphicsContext3D* context_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedFlush);
 };
 
-void ReadBackFramebuffer(
-    WebKit::WebGraphicsContext3D* context,
-    unsigned char* pixels,
-    gfx::Size size,
-    WebKit::WebGLId dst_texture,
-    bool* result) {
-  *result = false;
-  if (!context->makeContextCurrent())
-    return;
-  if (context->isContextLost())
-    return;
-  ScopedFlush flush(context);
-  ScopedFramebuffer dst_framebuffer(context, context->createFramebuffer());
-  {
-    ScopedFramebufferBinder<GL_DRAW_FRAMEBUFFER> framebuffer_binder(
-        context, dst_framebuffer);
-    ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(
-        context, dst_texture);
-    context->framebufferTexture2D(GL_DRAW_FRAMEBUFFER,
-                                  GL_COLOR_ATTACHMENT0,
-                                  GL_TEXTURE_2D,
-                                  dst_texture,
-                                  0);
-  }
-  *result = context->readBackFramebuffer(
-        pixels,
-        4 * size.GetArea(),
-        static_cast<WebKit::WebGLId>(dst_framebuffer),
-        size.width(),
-        size.height());
-}
-
-void ReadBackFramebufferComplete(WebKit::WebGraphicsContext3D* context,
-                                 WebKit::WebGLId* dst_texture,
-                                 base::Callback<void(bool)> callback,
-                                 bool* result) {
-  callback.Run(*result);
-  if (*dst_texture != 0) {
-    context->deleteTexture(*dst_texture);
-    context->flush();
-    *dst_texture = 0;
-  }
-}
-
-void DeleteContext(WebKit::WebGraphicsContext3D* context) {
+void DeleteContext(WebGraphicsContext3D* context) {
   delete context;
 }
 
@@ -267,44 +232,88 @@ namespace content {
 // Implements GLHelper::CopyTextureTo and encapsulates the data needed for it.
 class GLHelper::CopyTextureToImpl {
  public:
-  CopyTextureToImpl(WebKit::WebGraphicsContext3D* context,
-                    WebKit::WebGraphicsContext3D* context_for_thread,
+  CopyTextureToImpl(WebGraphicsContext3D* context,
+                    WebGraphicsContext3D* context_for_thread,
                     GLHelper* helper)
       : context_(context),
         context_for_thread_(context_for_thread),
         helper_(helper),
+        flush_(context),
         program_(context, context->createProgram()),
-        vertex_attributes_buffer_(context_, context_->createBuffer()),
-        dst_texture_(0) {
+        vertex_attributes_buffer_(context_, context_->createBuffer()) {
     InitBuffer();
     InitProgram();
   }
   ~CopyTextureToImpl() {
+    CancelRequests();
     DeleteContextForThread();
   }
 
   void InitBuffer();
   void InitProgram();
-  void Detach();
 
-  bool CopyTextureTo(WebKit::WebGLId src_texture,
+  bool CopyTextureTo(WebGLId src_texture,
                      const gfx::Size& src_size,
                      const gfx::Size& dst_size,
                      unsigned char* out);
 
-  void AsyncCopyTextureTo(WebKit::WebGLId src_texture,
+  void AsyncCopyTextureTo(WebGLId src_texture,
                           const gfx::Size& src_size,
                           const gfx::Size& dst_size,
                           unsigned char* out,
-                          base::Callback<void(bool)> callback);
+                          const base::Callback<void(bool)>& callback);
  private:
+  // A single request to AsyncCopyTextureTo.
+  // Thread-safety notes: the main thread creates instances of this class. The
+  // main thread can cancel the request, before it's handled by the helper
+  // thread, by resetting the texture and pixels fields. Alternatively, the
+  // thread marks that it handles the request by resetting the pixels field
+  // (meaning it guarantees that the callback with be called).
+  // In either case, the callback must be called exactly once, and the texture
+  // must be deleted by the main thread context.
+  struct Request : public base::RefCounted<Request> {
+    Request(CopyTextureToImpl* impl,
+            WebGLId texture_,
+            const gfx::Size& size_,
+            unsigned char* pixels_,
+            const base::Callback<void(bool)>& callback_)
+        : copy_texture_impl(impl),
+        size(size_),
+        callback(callback_),
+        lock(),
+        texture(texture_),
+        pixels(pixels_) {
+    }
+
+    // These members are only accessed on the main thread.
+    GLHelper::CopyTextureToImpl* copy_texture_impl;
+    gfx::Size size;
+    base::Callback<void(bool)> callback;
+
+    // Locks access to below members, which can be accessed on any thread.
+    base::Lock lock;
+    WebGLId texture;
+    unsigned char* pixels;
+
+   private:
+    friend class base::RefCounted<Request>;
+    ~Request() {}
+  };
+
   // Returns the id of a framebuffer that
-  WebKit::WebGLId ScaleTexture(WebKit::WebGLId src_texture,
+  WebGLId ScaleTexture(WebGLId src_texture,
                                const gfx::Size& src_size,
                                const gfx::Size& dst_size);
 
   // Deletes the context for GLHelperThread.
   void DeleteContextForThread();
+  static void ReadBackFramebuffer(scoped_refptr<Request> request,
+                                  WebGraphicsContext3D* context,
+                                  scoped_refptr<base::TaskRunner> reply_loop);
+  static void ReadBackFramebufferComplete(scoped_refptr<Request> request,
+                                          bool result);
+  void FinishRequest(scoped_refptr<Request> request);
+  void CancelRequests();
 
   // Interleaved array of 2-dimentional vertex positions (x, y) and
   // 2-dimentional texture coordinates (s, t).
@@ -313,10 +322,13 @@ class GLHelper::CopyTextureToImpl {
   static const WebKit::WGC3Dchar kCopyVertexShader[];
   static const WebKit::WGC3Dchar kCopyFragmentShader[];
 
-  WebKit::WebGraphicsContext3D* context_;
-  WebKit::WebGraphicsContext3D* context_for_thread_;
+  WebGraphicsContext3D* context_;
+  WebGraphicsContext3D* context_for_thread_;
   GLHelper* helper_;
 
+  // A scoped flush that will ensure all resource deletions are flushed when
+  // this object is destroyed. Must be declared before other Scoped* fields.
+  ScopedFlush flush_;
   // A program for copying a source texture into a destination texture.
   ScopedProgram program_;
   // The buffer that holds the vertices and the texture coordinates data for
@@ -328,8 +340,7 @@ class GLHelper::CopyTextureToImpl {
   WebKit::WGC3Dint texcoord_location_;
   // The location of the source texture in the program.
   WebKit::WGC3Dint texture_location_;
-  // The destination texture id. This is used only for the asynchronous version.
-  WebKit::WebGLId dst_texture_;
+  std::queue<scoped_refptr<Request> > request_queue_;
 };
 
 const WebKit::WGC3Dfloat GLHelper::CopyTextureToImpl::kVertexAttributes[] = {
@@ -369,11 +380,11 @@ void GLHelper::CopyTextureToImpl::InitProgram() {
   // Shaders to map the source texture to |dst_texture_|.
   ScopedShader vertex_shader(context_, helper_->CompileShaderFromSource(
       kCopyVertexShader, GL_VERTEX_SHADER));
-  DCHECK_NE(0U, static_cast<WebKit::WebGLId>(vertex_shader));
+  DCHECK_NE(0U, vertex_shader.id());
   context_->attachShader(program_, vertex_shader);
   ScopedShader fragment_shader(context_, helper_->CompileShaderFromSource(
       kCopyFragmentShader, GL_FRAGMENT_SHADER));
-  DCHECK_NE(0U, static_cast<WebKit::WebGLId>(fragment_shader));
+  DCHECK_NE(0U, fragment_shader.id());
   context_->attachShader(program_, fragment_shader);
   context_->linkProgram(program_);
 
@@ -389,17 +400,11 @@ void GLHelper::CopyTextureToImpl::InitProgram() {
   texture_location_ = context_->getUniformLocation(program_, "s_texture");
 }
 
-void GLHelper::CopyTextureToImpl::Detach() {
-  program_.Detach();
-  vertex_attributes_buffer_.Detach();
-  dst_texture_ = 0;
-}
-
-WebKit::WebGLId GLHelper::CopyTextureToImpl::ScaleTexture(
-    WebKit::WebGLId src_texture,
+WebGLId GLHelper::CopyTextureToImpl::ScaleTexture(
+    WebGLId src_texture,
     const gfx::Size& src_size,
     const gfx::Size& dst_size) {
-  WebKit::WebGLId dst_texture = context_->createTexture();
+  WebGLId dst_texture = context_->createTexture();
   {
     ScopedFramebuffer dst_framebuffer(context_, context_->createFramebuffer());
     ScopedFramebufferBinder<GL_DRAW_FRAMEBUFFER> framebuffer_binder(
@@ -468,7 +473,7 @@ void GLHelper::CopyTextureToImpl::DeleteContextForThread() {
 }
 
 bool GLHelper::CopyTextureToImpl::CopyTextureTo(
-    WebKit::WebGLId src_texture,
+    WebGLId src_texture,
     const gfx::Size& src_size,
     const gfx::Size& dst_size,
     unsigned char* out) {
@@ -490,44 +495,138 @@ bool GLHelper::CopyTextureToImpl::CopyTextureTo(
   return context_->readBackFramebuffer(
       out,
       4 * dst_size.GetArea(),
-      static_cast<WebKit::WebGLId>(dst_framebuffer),
+      dst_framebuffer.id(),
       dst_size.width(),
       dst_size.height());
 }
 
 void GLHelper::CopyTextureToImpl::AsyncCopyTextureTo(
-    WebKit::WebGLId src_texture,
+    WebGLId src_texture,
     const gfx::Size& src_size,
     const gfx::Size& dst_size,
     unsigned char* out,
-    base::Callback<void(bool)> callback) {
+    const base::Callback<void(bool)>& callback) {
   if (!context_for_thread_) {
     callback.Run(false);
     return;
   }
 
-  dst_texture_ = ScaleTexture(src_texture, src_size, dst_size);
+  WebGLId texture = ScaleTexture(src_texture, src_size, dst_size);
   context_->flush();
-  bool* result = new bool(false);
-  g_gl_helper_thread.Pointer()->message_loop_proxy()->PostTaskAndReply(
-      FROM_HERE,
+  scoped_refptr<Request> request =
+      new Request(this, texture, dst_size, out, callback);
+  request_queue_.push(request);
+
+  g_gl_helper_thread.Pointer()->message_loop_proxy()->PostTask(FROM_HERE,
       base::Bind(&ReadBackFramebuffer,
+                 request,
                  context_for_thread_,
-                 out,
-                 dst_size,
-                 dst_texture_,
-                 result),
-      base::Bind(&ReadBackFramebufferComplete,
-                 context_,
-                 &dst_texture_,
-                 callback,
-                 base::Owned(result)));
+                 base::MessageLoopProxy::current()));
+}
+
+void GLHelper::CopyTextureToImpl::ReadBackFramebuffer(
+    scoped_refptr<Request> request,
+    WebGraphicsContext3D* context,
+    scoped_refptr<base::TaskRunner> reply_loop) {
+  DCHECK(context);
+  if (!context->makeContextCurrent() || context->isContextLost()) {
+    base::AutoLock auto_lock(request->lock);
+    if (request->pixels) {
+      // Only report failure if the request wasn't canceled (otherwise the
+      // failure has already been reported).
+      request->pixels = NULL;
+      reply_loop->PostTask(
+          FROM_HERE, base::Bind(ReadBackFramebufferComplete, request, false));
+    }
+    return;
+  }
+  ScopedFlush flush(context);
+  ScopedFramebuffer dst_framebuffer(context, context->createFramebuffer());
+  unsigned char* pixels = NULL;
+  gfx::Size size;
+  {
+    // Note: We don't want to keep the lock while doing the readBack (since we
+    // don't want to block the UI thread). We rely on the fact that once the
+    // texture is bound to a FBO (that isn't current), deleting the texture is
+    // delayed until the FBO is deleted. We ensure ordering by flushing while
+    // the lock is held. Either the main thread cancelled before we get the
+    // lock, and we'll exit early, or we ensure that the texture is bound to the
+    // framebuffer before the main thread has a chance to delete it.
+    base::AutoLock auto_lock(request->lock);
+    if (!request->texture || !request->pixels)
+      return;
+    pixels = request->pixels;
+    request->pixels = NULL;
+    size = request->size;
+    {
+      ScopedFlush flush(context);
+      ScopedFramebufferBinder<GL_DRAW_FRAMEBUFFER> framebuffer_binder(
+          context, dst_framebuffer);
+      ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(
+          context, request->texture);
+      context->framebufferTexture2D(GL_DRAW_FRAMEBUFFER,
+                                    GL_COLOR_ATTACHMENT0,
+                                    GL_TEXTURE_2D,
+                                    request->texture,
+                                    0);
+    }
+  }
+  bool result = context->readBackFramebuffer(
+        pixels,
+        4 * size.GetArea(),
+        dst_framebuffer.id(),
+        size.width(),
+        size.height());
+  reply_loop->PostTask(
+      FROM_HERE, base::Bind(ReadBackFramebufferComplete, request, result));
+}
+
+void GLHelper::CopyTextureToImpl::ReadBackFramebufferComplete(
+    scoped_refptr<Request> request,
+    bool result) {
+  request->callback.Run(result);
+  if (request->copy_texture_impl)
+    request->copy_texture_impl->FinishRequest(request);
+}
+
+void GLHelper::CopyTextureToImpl::FinishRequest(
+    scoped_refptr<Request> request) {
+  CHECK(request_queue_.front() == request);
+  request_queue_.pop();
+  base::AutoLock auto_lock(request->lock);
+  if (request->texture != 0) {
+    context_->deleteTexture(request->texture);
+    request->texture = 0;
+    context_->flush();
+  }
+}
+
+void GLHelper::CopyTextureToImpl::CancelRequests() {
+  while (!request_queue_.empty()) {
+    scoped_refptr<Request> request = request_queue_.front();
+    request_queue_.pop();
+    request->copy_texture_impl = NULL;
+    bool cancelled = false;
+    {
+      base::AutoLock auto_lock(request->lock);
+      if (request->texture != 0) {
+        context_->deleteTexture(request->texture);
+        request->texture = 0;
+      }
+      if (request->pixels != NULL) {
+        request->pixels = NULL;
+        cancelled = true;
+      }
+    }
+    if (cancelled)
+      request->callback.Run(false);
+  }
 }
 
 base::subtle::Atomic32 GLHelper::count_ = 0;
 
-GLHelper::GLHelper(WebKit::WebGraphicsContext3D* context,
-                   WebKit::WebGraphicsContext3D* context_for_thread)
+GLHelper::GLHelper(WebGraphicsContext3D* context,
+                   WebGraphicsContext3D* context_for_thread)
     : context_(context),
       context_for_thread_(context_for_thread) {
   base::subtle::NoBarrier_AtomicIncrement(&count_, 1);
@@ -554,16 +653,11 @@ GLHelper::~GLHelper() {
   }
 }
 
-WebKit::WebGraphicsContext3D* GLHelper::context() const {
+WebGraphicsContext3D* GLHelper::context() const {
   return context_;
 }
 
-void GLHelper::Detach() {
-  if (copy_texture_to_impl_.get())
-    copy_texture_to_impl_->Detach();
-}
-
-bool GLHelper::CopyTextureTo(WebKit::WebGLId src_texture,
+bool GLHelper::CopyTextureTo(WebGLId src_texture,
                              const gfx::Size& src_size,
                              const gfx::Size& dst_size,
                              unsigned char* out) {
@@ -580,7 +674,7 @@ bool GLHelper::CopyTextureTo(WebKit::WebGLId src_texture,
       out);
 }
 
-void GLHelper::AsyncCopyTextureTo(WebKit::WebGLId src_texture,
+void GLHelper::AsyncCopyTextureTo(WebGLId src_texture,
                                   const gfx::Size& src_size,
                                   const gfx::Size& dst_size,
                                   unsigned char* out,
@@ -598,7 +692,7 @@ void GLHelper::AsyncCopyTextureTo(WebKit::WebGLId src_texture,
                                             callback);
 }
 
-WebKit::WebGLId GLHelper::CompileShaderFromSource(
+WebGLId GLHelper::CompileShaderFromSource(
     const WebKit::WGC3Dchar* source,
     WebKit::WGC3Denum type) {
   ScopedShader shader(context_, context_->createShader(type));
diff --git a/content/common/gpu/client/gl_helper.h b/content/common/gpu/client/gl_helper.h
index 771cad3..873339e 100644
--- a/content/common/gpu/client/gl_helper.h
+++ b/content/common/gpu/client/gl_helper.h
@@ -28,9 +28,6 @@ class GLHelper {
 
   WebKit::WebGraphicsContext3D* context() const;
 
-  // Detaches all the resources GLHelper manages.
-  void Detach();
-
   // Copies the contents of |src_texture| with the size of |src_size| into
   // |out|. The contents is transformed so that it fits in |dst_size|.
   bool CopyTextureTo(WebKit::WebGLId src_texture,