cc: Make async readback path use async glReadPixels.

The current async readback path for the compositor did a synchronous
glReadPixels, but since the whole thing is asynchronous, there's no
need for the glReadPixels call to be synchronous.

This mimics the code from GLHelper to make the glReadPixels into an
async call.

Tested by all the compositor pixel tests.

Depends on: https://codereview.chromium.org/14126014/

R=piman
BUG=179896
NOTRY=true

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

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

16 files changed, 447 insertions, 84 deletions

diff --git a/cc/output/direct_renderer.cc b/cc/output/direct_renderer.cc
index 7d0083f..e0cedb6 100644
--- a/cc/output/direct_renderer.cc
+++ b/cc/output/direct_renderer.cc
@@ -193,18 +193,13 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order) {
       root_render_pass->damage_rect : root_render_pass->output_rect;
   frame.root_damage_rect.Intersect(gfx::Rect(ViewportSize()));
 
-  std::vector<base::Closure> copy_callbacks;
-
   BeginDrawingFrame(&frame);
   for (size_t i = 0; i < render_passes_in_draw_order->size(); ++i) {
     DrawRenderPass(&frame, render_passes_in_draw_order->at(i));
 
     const RenderPass* pass = frame.current_render_pass;
-    for (size_t i = 0; i < pass->copy_callbacks.size(); ++i) {
-      scoped_ptr<SkBitmap> bitmap(new SkBitmap);
-      CopyCurrentRenderPassToBitmap(&frame, bitmap.get());
-      pass->copy_callbacks[i].Run(bitmap.Pass());
-    }
+    for (size_t i = 0; i < pass->copy_callbacks.size(); ++i)
+      CopyCurrentRenderPassToBitmap(&frame, pass->copy_callbacks[i]);
   }
   FinishDrawingFrame(&frame);
 
diff --git a/cc/output/direct_renderer.h b/cc/output/direct_renderer.h
index 4153750..3833231 100644
--- a/cc/output/direct_renderer.h
+++ b/cc/output/direct_renderer.h
@@ -6,6 +6,7 @@
 #define CC_OUTPUT_DIRECT_RENDERER_H_
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "cc/base/cc_export.h"
 #include "cc/output/renderer.h"
 #include "cc/resources/resource_provider.h"
@@ -109,8 +110,12 @@ class CC_EXPORT DirectRenderer : public Renderer {
   virtual bool FlippedFramebuffer() const = 0;
   virtual void EnsureScissorTestEnabled() = 0;
   virtual void EnsureScissorTestDisabled() = 0;
-  virtual void CopyCurrentRenderPassToBitmap(DrawingFrame* frame,
-                                             SkBitmap* bitmap) = 0;
+
+  typedef base::Callback<void(scoped_ptr<SkBitmap>)>
+      CopyRenderPassCallback;
+  virtual void CopyCurrentRenderPassToBitmap(
+      DrawingFrame* frame,
+      const CopyRenderPassCallback& callback) = 0;
 
   ScopedPtrHashMap<RenderPass::Id, CachedResource> render_pass_textures_;
   ResourceProvider* resource_provider_;
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index c767bd1..dfc41e7 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -30,6 +30,7 @@
 #include "cc/resources/layer_quad.h"
 #include "cc/resources/priority_calculator.h"
 #include "cc/resources/scoped_resource.h"
+#include "cc/resources/sync_point_helper.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/proxy.h"
 #include "cc/trees/single_thread_proxy.h"
@@ -87,6 +88,17 @@ const float kAntiAliasingEpsilon = 1.0f / 1024.0f;
 
 }  // anonymous namespace
 
+struct GLRenderer::PendingAsyncReadPixels {
+  PendingAsyncReadPixels() : buffer(0) {}
+
+  CopyRenderPassCallback copy_callback;
+  base::CancelableClosure finished_read_pixels_callback;
+  unsigned buffer;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PendingAsyncReadPixels);
+};
+
 scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient* client,
                                           OutputSurface* output_surface,
                                           ResourceProvider* resource_provider,
@@ -192,6 +204,13 @@ bool GLRenderer::Initialize() {
 }
 
 GLRenderer::~GLRenderer() {
+  while (!pending_async_read_pixels_.empty()) {
+    pending_async_read_pixels_.back()->finished_read_pixels_callback.Cancel();
+    pending_async_read_pixels_.back()->copy_callback.Run(
+        scoped_ptr<SkBitmap>());
+    pending_async_read_pixels_.pop_back();
+  }
+
   context_->setMemoryAllocationChangedCallbackCHROMIUM(NULL);
   CleanupSharedObjects();
 }
@@ -1777,17 +1796,10 @@ void GLRenderer::EnsureScissorTestDisabled() {
   is_scissor_enabled_ = false;
 }
 
-void GLRenderer::CopyCurrentRenderPassToBitmap(DrawingFrame* frame,
-                                               SkBitmap* bitmap) {
-  gfx::Size render_pass_size = frame->current_render_pass->output_rect.size();
-  bitmap->setConfig(SkBitmap::kARGB_8888_Config,
-                    render_pass_size.width(),
-                    render_pass_size.height());
-  if (bitmap->allocPixels()) {
-    bitmap->lockPixels();
-    GetFramebufferPixels(bitmap->getPixels(), gfx::Rect(render_pass_size));
-    bitmap->unlockPixels();
-  }
+void GLRenderer::CopyCurrentRenderPassToBitmap(
+    DrawingFrame* frame,
+    const CopyRenderPassCallback& callback) {
+  GetFramebufferPixelsAsync(frame->current_render_pass->output_rect, callback);
 }
 
 void GLRenderer::ToGLMatrix(float* gl_matrix, const gfx::Transform& transform) {
@@ -1987,18 +1999,68 @@ void GLRenderer::EnsureBackbuffer() {
 }
 
 void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
+  if (!pixels || rect.IsEmpty())
+    return;
+
+  scoped_ptr<PendingAsyncReadPixels> pending_read(new PendingAsyncReadPixels);
+  pending_async_read_pixels_.insert(pending_async_read_pixels_.begin(),
+                                    pending_read.Pass());
+
+  // This is a syncronous call since the callback is null.
+  DoGetFramebufferPixels(static_cast<uint8*>(pixels),
+                         rect,
+                         AsyncGetFramebufferPixelsCleanupCallback());
+}
+
+void GLRenderer::GetFramebufferPixelsAsync(gfx::Rect rect,
+                                           CopyRenderPassCallback callback) {
+  if (callback.is_null())
+    return;
+  if (rect.IsEmpty()) {
+    callback.Run(scoped_ptr<SkBitmap>());
+    return;
+  }
+
+  scoped_ptr<SkBitmap> bitmap(new SkBitmap);
+  bitmap->setConfig(SkBitmap::kARGB_8888_Config, rect.width(), rect.height());
+  bitmap->allocPixels();
+
+  scoped_ptr<SkAutoLockPixels> lock(new SkAutoLockPixels(*bitmap));
+
+  // Save a pointer to the pixels, the bitmap is owned by the cleanup_callback.
+  uint8* pixels = static_cast<uint8*>(bitmap->getPixels());
+
+  AsyncGetFramebufferPixelsCleanupCallback cleanup_callback = base::Bind(
+      &GLRenderer::PassOnSkBitmap,
+      base::Unretained(this),
+      base::Passed(&bitmap),
+      base::Passed(&lock),
+      callback);
+
+  scoped_ptr<PendingAsyncReadPixels> pending_read(new PendingAsyncReadPixels);
+  pending_read->copy_callback = callback;
+  pending_async_read_pixels_.insert(pending_async_read_pixels_.begin(),
+                                    pending_read.Pass());
+
+  // This is an asyncronous call since the callback is not null.
+  DoGetFramebufferPixels(pixels, rect, cleanup_callback);
+}
+
+void GLRenderer::DoGetFramebufferPixels(
+    uint8* dest_pixels,
+    gfx::Rect rect,
+    const AsyncGetFramebufferPixelsCleanupCallback& cleanup_callback) {
   DCHECK(rect.right() <= ViewportWidth());
   DCHECK(rect.bottom() <= ViewportHeight());
 
-  if (!pixels)
-    return;
+  bool is_async = !cleanup_callback.is_null();
 
   MakeContextCurrent();
 
   bool do_workaround = NeedsIOSurfaceReadbackWorkaround();
 
-  GLuint temporary_texture = 0;
-  GLuint temporary_fbo = 0;
+  unsigned temporary_texture = 0;
+  unsigned temporary_fbo = 0;
 
   if (do_workaround) {
     // On Mac OS X, calling glReadPixels() against an FBO whose color attachment
@@ -2046,8 +2108,14 @@ void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
            GL_FRAMEBUFFER_COMPLETE);
   }
 
-  scoped_ptr<uint8_t[]> src_pixels(
-      new uint8_t[rect.width() * rect.height() * 4]);
+  unsigned buffer = context_->createBuffer();
+  GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                     buffer));
+  GLC(context_, context_->bufferData(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                     4 * rect.size().GetArea(),
+                                     NULL,
+                                     GL_STREAM_READ));
+
   GLC(context_,
       context_->readPixels(rect.x(),
                            ViewportSize().height() - rect.bottom(),
@@ -2055,23 +2123,10 @@ void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
                            rect.height(),
                            GL_RGBA,
                            GL_UNSIGNED_BYTE,
-                           src_pixels.get()));
-
-  uint8_t* dest_pixels = static_cast<uint8_t*>(pixels);
-  size_t row_bytes = rect.width() * 4;
-  int num_rows = rect.height();
-  size_t total_bytes = num_rows * row_bytes;
-  for (size_t dest_y = 0; dest_y < total_bytes; dest_y += row_bytes) {
-    // Flip Y axis.
-    size_t src_y = total_bytes - dest_y - row_bytes;
-    // Swizzle BGRA -> RGBA.
-    for (size_t x = 0; x < row_bytes; x += 4) {
-      dest_pixels[dest_y + (x + 0)] = src_pixels.get()[src_y + (x + 2)];
-      dest_pixels[dest_y + (x + 1)] = src_pixels.get()[src_y + (x + 1)];
-      dest_pixels[dest_y + (x + 2)] = src_pixels.get()[src_y + (x + 0)];
-      dest_pixels[dest_y + (x + 3)] = src_pixels.get()[src_y + (x + 3)];
-    }
-  }
+                           NULL));
+
+  GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                     0));
 
   if (do_workaround) {
     // Clean up.
@@ -2081,9 +2136,97 @@ void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
     GLC(context_, context_->deleteTexture(temporary_texture));
   }
 
+  base::Closure finished_callback =
+      base::Bind(&GLRenderer::FinishedReadback,
+                 base::Unretained(this),
+                 cleanup_callback,
+                 buffer,
+                 dest_pixels,
+                 rect.size());
+  // Save the finished_callback so it can be cancelled.
+  pending_async_read_pixels_.front()->finished_read_pixels_callback.Reset(
+      finished_callback);
+
+  // Save the buffer to verify the callbacks happen in the expected order.
+  pending_async_read_pixels_.front()->buffer = buffer;
+
+  if (is_async) {
+    unsigned sync_point = context_->insertSyncPoint();
+    SyncPointHelper::SignalSyncPoint(
+        context_,
+        sync_point,
+        finished_callback);
+  } else {
+    resource_provider_->Finish();
+    finished_callback.Run();
+  }
+
   EnforceMemoryPolicy();
 }
 
+void GLRenderer::FinishedReadback(
+    const AsyncGetFramebufferPixelsCleanupCallback& cleanup_callback,
+    unsigned source_buffer,
+    uint8* dest_pixels,
+    gfx::Size size) {
+  DCHECK(!pending_async_read_pixels_.empty());
+  DCHECK_EQ(source_buffer, pending_async_read_pixels_.back()->buffer);
+
+  uint8* src_pixels = NULL;
+
+  if (source_buffer != 0) {
+    GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       source_buffer));
+    src_pixels = static_cast<uint8*>(
+        context_->mapBufferCHROMIUM(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                    GL_READ_ONLY));
+
+    if (src_pixels) {
+      size_t row_bytes = size.width() * 4;
+      int num_rows = size.height();
+      size_t total_bytes = num_rows * row_bytes;
+      for (size_t dest_y = 0; dest_y < total_bytes; dest_y += row_bytes) {
+        // Flip Y axis.
+        size_t src_y = total_bytes - dest_y - row_bytes;
+        // Swizzle BGRA -> RGBA.
+        for (size_t x = 0; x < row_bytes; x += 4) {
+          dest_pixels[dest_y + (x + 0)] = src_pixels[src_y + (x + 2)];
+          dest_pixels[dest_y + (x + 1)] = src_pixels[src_y + (x + 1)];
+          dest_pixels[dest_y + (x + 2)] = src_pixels[src_y + (x + 0)];
+          dest_pixels[dest_y + (x + 3)] = src_pixels[src_y + (x + 3)];
+        }
+      }
+
+      GLC(context_, context_->unmapBufferCHROMIUM(
+          GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM));
+    }
+    GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       0));
+    GLC(context_, context_->deleteBuffer(source_buffer));
+  }
+
+  // TODO(danakj): This can go away when synchronous readback is no more and its
+  // contents can just move here.
+  if (!cleanup_callback.is_null())
+    cleanup_callback.Run(src_pixels != NULL);
+
+  pending_async_read_pixels_.pop_back();
+}
+
+void GLRenderer::PassOnSkBitmap(
+    scoped_ptr<SkBitmap> bitmap,
+    scoped_ptr<SkAutoLockPixels> lock,
+    const CopyRenderPassCallback& callback,
+    bool success) {
+  DCHECK(callback.Equals(pending_async_read_pixels_.back()->copy_callback));
+
+  lock.reset();
+  if (success)
+    callback.Run(bitmap.Pass());
+  else
+    callback.Run(scoped_ptr<SkBitmap>());
+}
+
 bool GLRenderer::GetFramebufferTexture(ScopedResource* texture,
                                        gfx::Rect device_rect) {
   DCHECK(!texture->id() || (texture->size() == device_rect.size() &&
diff --git a/cc/output/gl_renderer.h b/cc/output/gl_renderer.h
index 7047aac..19bf8a9 100644
--- a/cc/output/gl_renderer.h
+++ b/cc/output/gl_renderer.h
@@ -5,7 +5,9 @@
 #ifndef CC_OUTPUT_GL_RENDERER_H_
 #define CC_OUTPUT_GL_RENDERER_H_
 
+#include "base/cancelable_callback.h"
 #include "cc/base/cc_export.h"
+#include "cc/base/scoped_ptr_vector.h"
 #include "cc/output/direct_renderer.h"
 #include "cc/output/gl_renderer_draw_cache.h"
 #include "cc/output/renderer.h"
@@ -20,6 +22,8 @@
 #include "third_party/WebKit/Source/Platform/chromium/public/WebGraphicsMemoryAllocation.h"
 #include "ui/gfx/quad_f.h"
 
+class SkBitmap;
+
 namespace cc {
 
 class GLRendererShaderTest;
@@ -87,6 +91,8 @@ class CC_EXPORT GLRenderer
     return shared_geometry_.get();
   }
 
+  void GetFramebufferPixelsAsync(gfx::Rect rect,
+                                 CopyRenderPassCallback callback);
   bool GetFramebufferTexture(ScopedResource* resource, gfx::Rect device_rect);
   void ReleaseRenderPassTextures();
 
@@ -103,8 +109,9 @@ class CC_EXPORT GLRenderer
   virtual bool FlippedFramebuffer() const OVERRIDE;
   virtual void EnsureScissorTestEnabled() OVERRIDE;
   virtual void EnsureScissorTestDisabled() OVERRIDE;
-  virtual void CopyCurrentRenderPassToBitmap(DrawingFrame* frame,
-                                             SkBitmap* bitmap) OVERRIDE;
+  virtual void CopyCurrentRenderPassToBitmap(
+      DrawingFrame* frame,
+      const CopyRenderPassCallback& callback) OVERRIDE;
   virtual void FinishDrawingQuadList() OVERRIDE;
 
  private:
@@ -179,6 +186,23 @@ class CC_EXPORT GLRenderer
   bool InitializeSharedObjects();
   void CleanupSharedObjects();
 
+  typedef base::Callback<void(bool success)>
+      AsyncGetFramebufferPixelsCleanupCallback;
+  void DoGetFramebufferPixels(
+      uint8* pixels,
+      gfx::Rect rect,
+      const AsyncGetFramebufferPixelsCleanupCallback& cleanup_callback);
+  void FinishedReadback(
+      const AsyncGetFramebufferPixelsCleanupCallback& cleanup_callback,
+      unsigned source_buffer,
+      uint8_t* dest_pixels,
+      gfx::Size size);
+  void PassOnSkBitmap(
+      scoped_ptr<SkBitmap> bitmap,
+      scoped_ptr<SkAutoLockPixels> lock,
+      const CopyRenderPassCallback& callback,
+      bool success);
+
   // WebKit::
   // WebGraphicsContext3D::WebGraphicsMemoryAllocationChangedCallbackCHROMIUM
   // implementation.
@@ -378,6 +402,9 @@ class CC_EXPORT GLRenderer
   TexturedQuadDrawCache draw_cache_;
   int highp_threshold_min_;
 
+  struct PendingAsyncReadPixels;
+  ScopedPtrVector<PendingAsyncReadPixels> pending_async_read_pixels_;
+
   scoped_ptr<ResourceProvider::ScopedWriteLockGL> current_framebuffer_lock_;
 
   scoped_refptr<ResourceProvider::Fence> last_swap_fence_;
diff --git a/cc/output/software_renderer.cc b/cc/output/software_renderer.cc
index 0b61ffe..abb9bba 100644
--- a/cc/output/software_renderer.cc
+++ b/cc/output/software_renderer.cc
@@ -404,13 +404,18 @@ void SoftwareRenderer::DrawUnsupportedQuad(const DrawingFrame* frame,
                             current_paint_);
 }
 
-void SoftwareRenderer::CopyCurrentRenderPassToBitmap(DrawingFrame* frame,
-                                                     SkBitmap* bitmap) {
+void SoftwareRenderer::CopyCurrentRenderPassToBitmap(
+    DrawingFrame* frame,
+    const CopyRenderPassCallback& callback) {
   gfx::Size render_pass_size = frame->current_render_pass->output_rect.size();
+
+  scoped_ptr<SkBitmap> bitmap(new SkBitmap);
   bitmap->setConfig(SkBitmap::kARGB_8888_Config,
                     render_pass_size.width(),
                     render_pass_size.height());
-  current_canvas_->readPixels(bitmap, 0, 0);
+  current_canvas_->readPixels(bitmap.get(), 0, 0);
+
+  callback.Run(bitmap.Pass());
 }
 
 void SoftwareRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
diff --git a/cc/output/software_renderer.h b/cc/output/software_renderer.h
index 9ee2777..09664c1 100644
--- a/cc/output/software_renderer.h
+++ b/cc/output/software_renderer.h
@@ -58,8 +58,9 @@ class CC_EXPORT SoftwareRenderer : public DirectRenderer {
   virtual bool FlippedFramebuffer() const OVERRIDE;
   virtual void EnsureScissorTestEnabled() OVERRIDE;
   virtual void EnsureScissorTestDisabled() OVERRIDE;
-  virtual void CopyCurrentRenderPassToBitmap(DrawingFrame* frame,
-                                             SkBitmap* bitmap) OVERRIDE;
+  virtual void CopyCurrentRenderPassToBitmap(
+      DrawingFrame* frame,
+      const CopyRenderPassCallback& callback) OVERRIDE;
 
  private:
   SoftwareRenderer(
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index eb92fbf..fad2cc7 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -445,6 +445,13 @@ void ResourceProvider::Flush() {
     context3d->flush();
 }
 
+void ResourceProvider::Finish() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  WebGraphicsContext3D* context3d = output_surface_->context3d();
+  if (context3d)
+    context3d->finish();
+}
+
 bool ResourceProvider::ShallowFlushIfSupported() {
   DCHECK(thread_checker_.CalledOnValidThread());
   WebGraphicsContext3D* context3d = output_surface_->context3d();
diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h
index e964437..6f146d9 100644
--- a/cc/resources/resource_provider.h
+++ b/cc/resources/resource_provider.h
@@ -127,6 +127,10 @@ class CC_EXPORT ResourceProvider {
   // respect to other contexts.
   void Flush();
 
+  // Finish all context operations, causing any pending callbacks to be
+  // scheduled.
+  void Finish();
+
   // Only flush the command buffer if supported.
   // Returns true if the shallow flush occurred, false otherwise.
   bool ShallowFlushIfSupported();
diff --git a/cc/test/pixel_test.cc b/cc/test/pixel_test.cc
index 297f542..5d28d59 100644
--- a/cc/test/pixel_test.cc
+++ b/cc/test/pixel_test.cc
@@ -5,6 +5,7 @@
 #include "cc/test/pixel_test.h"
 
 #include "base/path_service.h"
+#include "base/run_loop.h"
 #include "cc/output/compositor_frame_metadata.h"
 #include "cc/output/gl_renderer.h"
 #include "cc/output/output_surface.h"
@@ -60,19 +61,27 @@ PixelTest::~PixelTest() {}
 bool PixelTest::RunPixelTest(RenderPassList* pass_list,
                              const base::FilePath& ref_file,
                              const PixelComparator& comparator) {
+  base::RunLoop run_loop;
+
   pass_list->back()->copy_callbacks.push_back(
-      base::Bind(&PixelTest::ReadbackResult, base::Unretained(this)));
+      base::Bind(&PixelTest::ReadbackResult,
+                 base::Unretained(this),
+                 run_loop.QuitClosure()));
 
   renderer_->DecideRenderPassAllocationsForFrame(*pass_list);
   renderer_->DrawFrame(pass_list);
 
-  // TODO(danakj): When the glReadPixels is async, wait for it to finish.
+  // Wait for the readback to complete.
+  resource_provider_->Finish();
+  run_loop.Run();
 
   return PixelsMatchReference(ref_file, comparator);
 }
 
-void PixelTest::ReadbackResult(scoped_ptr<SkBitmap> bitmap) {
+void PixelTest::ReadbackResult(base::Closure quit_run_loop,
+                               scoped_ptr<SkBitmap> bitmap) {
   result_bitmap_ = bitmap.Pass();
+  quit_run_loop.Run();
 }
 
 bool PixelTest::PixelsMatchReference(const base::FilePath& ref_file,
diff --git a/cc/test/pixel_test.h b/cc/test/pixel_test.h
index c390c3b..6fb8135 100644
--- a/cc/test/pixel_test.h
+++ b/cc/test/pixel_test.h
@@ -39,7 +39,7 @@ class PixelTest : public testing::Test {
   void SetUpSoftwareRenderer();
 
  private:
-  void ReadbackResult(scoped_ptr<SkBitmap> bitmap);
+  void ReadbackResult(base::Closure quit_run_loop, scoped_ptr<SkBitmap> bitmap);
 
   bool PixelsMatchReference(const base::FilePath& ref_file,
                             const PixelComparator& comparator);
diff --git a/cc/test/test_web_graphics_context_3d.cc b/cc/test/test_web_graphics_context_3d.cc
index e315f65..3772a8e 100644
--- a/cc/test/test_web_graphics_context_3d.cc
+++ b/cc/test/test_web_graphics_context_3d.cc
@@ -7,7 +7,9 @@
 #include <algorithm>
 #include <string>
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
@@ -24,11 +26,10 @@ using WebKit::WebGraphicsContext3D;
 
 namespace cc {
 
-static const WebGLId kBufferId = 1;
-static const WebGLId kFramebufferId = 2;
-static const WebGLId kProgramId = 3;
-static const WebGLId kRenderbufferId = 4;
-static const WebGLId kShaderId = 5;
+static const WebGLId kFramebufferId = 1;
+static const WebGLId kProgramId = 2;
+static const WebGLId kRenderbufferId = 3;
+static const WebGLId kShaderId = 4;
 
 static unsigned s_context_id = 1;
 
@@ -37,6 +38,7 @@ const WebGLId TestWebGraphicsContext3D::kExternalTextureId = 1337;
 TestWebGraphicsContext3D::TestWebGraphicsContext3D()
     : FakeWebGraphicsContext3D(),
       context_id_(s_context_id++),
+      next_buffer_id_(1),
       next_texture_id_(1),
       have_extension_io_surface_(false),
       have_extension_egl_image_(false),
@@ -47,13 +49,15 @@ TestWebGraphicsContext3D::TestWebGraphicsContext3D()
       context_lost_callback_(NULL),
       max_texture_size_(1024),
       width_(0),
-      height_(0) {
+      height_(0),
+      bound_buffer_(0) {
 }
 
 TestWebGraphicsContext3D::TestWebGraphicsContext3D(
     const WebGraphicsContext3D::Attributes& attributes)
     : FakeWebGraphicsContext3D(),
       context_id_(s_context_id++),
+      next_buffer_id_(1),
       next_texture_id_(1),
       attributes_(attributes),
       have_extension_io_surface_(false),
@@ -65,10 +69,15 @@ TestWebGraphicsContext3D::TestWebGraphicsContext3D(
       context_lost_callback_(NULL),
       max_texture_size_(1024),
       width_(0),
-      height_(0) {
+      height_(0),
+      bound_buffer_(0) {
 }
 
 TestWebGraphicsContext3D::~TestWebGraphicsContext3D() {
+  for (size_t i = 0; i < sync_point_callbacks_.size(); ++i) {
+    if (sync_point_callbacks_[i] != NULL)
+      delete sync_point_callbacks_[i];
+  }
 }
 
 bool TestWebGraphicsContext3D::makeContextCurrent() {
@@ -176,11 +185,14 @@ WGC3Dboolean TestWebGraphicsContext3D::isTexture(
 }
 
 WebGLId TestWebGraphicsContext3D::createBuffer() {
-  return kBufferId | context_id_ << 16;
+  return NextBufferId();
 }
 
 void TestWebGraphicsContext3D::deleteBuffer(WebGLId id) {
-  EXPECT_EQ(kBufferId | context_id_ << 16, id);
+  unsigned context_id = id >> 17;
+  unsigned buffer_id = id & 0x1ffff;
+  DCHECK(buffer_id && buffer_id < next_buffer_id_);
+  DCHECK_EQ(context_id, context_id_);
 }
 
 WebGLId TestWebGraphicsContext3D::createFramebuffer() {
@@ -239,12 +251,6 @@ void TestWebGraphicsContext3D::useProgram(WebGLId program) {
   EXPECT_EQ(kProgramId | context_id_ << 16, program);
 }
 
-void TestWebGraphicsContext3D::bindBuffer(WGC3Denum target, WebGLId buffer) {
-  if (!buffer)
-    return;
-  EXPECT_EQ(kBufferId | context_id_ << 16, buffer);
-}
-
 void TestWebGraphicsContext3D::bindFramebuffer(
     WGC3Denum target, WebGLId framebuffer) {
   if (!framebuffer)
@@ -334,6 +340,84 @@ void TestWebGraphicsContext3D::loseContextCHROMIUM(WGC3Denum current,
   shared_contexts_.clear();
 }
 
+void TestWebGraphicsContext3D::signalSyncPoint(
+    unsigned sync_point,
+    WebGraphicsSyncPointCallback* callback) {
+  sync_point_callbacks_.push_back(callback);
+}
+
+void TestWebGraphicsContext3D::prepareTexture() {
+  CallAllSyncPointCallbacks();
+}
+
+void TestWebGraphicsContext3D::finish() {
+  CallAllSyncPointCallbacks();
+}
+
+void TestWebGraphicsContext3D::flush() {
+  CallAllSyncPointCallbacks();
+}
+
+static void CallAndDestroy(
+    WebKit::WebGraphicsContext3D::WebGraphicsSyncPointCallback* callback) {
+  if (!callback)
+    return;
+  callback->onSyncPointReached();
+  delete callback;
+}
+
+void TestWebGraphicsContext3D::CallAllSyncPointCallbacks() {
+  for (size_t i = 0; i < sync_point_callbacks_.size(); ++i) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&CallAndDestroy,
+                   sync_point_callbacks_[i]));
+  }
+  sync_point_callbacks_.clear();
+}
+
+void TestWebGraphicsContext3D::bindBuffer(WebKit::WGC3Denum target,
+                                          WebKit::WebGLId buffer) {
+  bound_buffer_ = buffer;
+  if (!bound_buffer_)
+    return;
+  unsigned context_id = buffer >> 17;
+  unsigned buffer_id = buffer & 0x1ffff;
+  DCHECK(buffer_id && buffer_id < next_buffer_id_);
+  DCHECK_EQ(context_id, context_id_);
+
+  if (buffers_.count(bound_buffer_) == 0)
+    buffers_.set(bound_buffer_, make_scoped_ptr(new Buffer).Pass());
+
+  buffers_.get(bound_buffer_)->target = target;
+}
+
+void TestWebGraphicsContext3D::bufferData(WebKit::WGC3Denum target,
+                                          WebKit::WGC3Dsizeiptr size,
+                                          const void* data,
+                                          WebKit::WGC3Denum usage) {
+  DCHECK_GT(buffers_.count(bound_buffer_), 0u);
+  DCHECK_EQ(target, buffers_.get(bound_buffer_)->target);
+  buffers_.get(bound_buffer_)->pixels.reset(new uint8[size]);
+  if (data != NULL)
+    memcpy(buffers_.get(bound_buffer_)->pixels.get(), data, size);
+}
+
+void* TestWebGraphicsContext3D::mapBufferCHROMIUM(WebKit::WGC3Denum target,
+                                                  WebKit::WGC3Denum access) {
+  DCHECK_GT(buffers_.count(bound_buffer_), 0u);
+  DCHECK_EQ(target, buffers_.get(bound_buffer_)->target);
+  return buffers_.get(bound_buffer_)->pixels.get();
+}
+
+WebKit::WGC3Dboolean TestWebGraphicsContext3D::unmapBufferCHROMIUM(
+    WebKit::WGC3Denum target) {
+  DCHECK_GT(buffers_.count(bound_buffer_), 0u);
+  DCHECK_EQ(target, buffers_.get(bound_buffer_)->target);
+  buffers_.get(bound_buffer_)->pixels.reset();
+  return true;
+}
+
 WebGLId TestWebGraphicsContext3D::NextTextureId() {
   WebGLId texture_id = next_texture_id_++;
   DCHECK(texture_id < (1 << 16));
@@ -341,4 +425,15 @@ WebGLId TestWebGraphicsContext3D::NextTextureId() {
   return texture_id;
 }
 
+WebGLId TestWebGraphicsContext3D::NextBufferId() {
+  WebGLId buffer_id = next_buffer_id_++;
+  DCHECK(buffer_id < (1 << 17));
+  buffer_id |= context_id_ << 17;
+  return buffer_id;
+}
+
+TestWebGraphicsContext3D::Buffer::Buffer() : target(0) {}
+
+TestWebGraphicsContext3D::Buffer::~Buffer() {}
+
 }  // namespace cc
diff --git a/cc/test/test_web_graphics_context_3d.h b/cc/test/test_web_graphics_context_3d.h
index c7a92a5..26ccc54 100644
--- a/cc/test/test_web_graphics_context_3d.h
+++ b/cc/test/test_web_graphics_context_3d.h
@@ -11,6 +11,7 @@
 #include "base/hash_tables.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
+#include "cc/base/scoped_ptr_hash_map.h"
 #include "cc/debug/fake_web_graphics_context_3d.h"
 #include "third_party/khronos/GLES2/gl2.h"
 
@@ -38,7 +39,6 @@ class TestWebGraphicsContext3D : public FakeWebGraphicsContext3D {
   virtual WebKit::WGC3Denum getGraphicsResetStatusARB();
 
   virtual void attachShader(WebKit::WebGLId program, WebKit::WebGLId shader);
-  virtual void bindBuffer(WebKit::WGC3Denum target, WebKit::WebGLId buffer);
   virtual void bindFramebuffer(
       WebKit::WGC3Denum target, WebKit::WebGLId framebuffer);
   virtual void bindRenderbuffer(
@@ -105,6 +105,23 @@ class TestWebGraphicsContext3D : public FakeWebGraphicsContext3D {
   virtual void loseContextCHROMIUM(WebKit::WGC3Denum current,
                                    WebKit::WGC3Denum other);
 
+  // Takes ownership of the |callback|.
+  virtual void signalSyncPoint(unsigned sync_point,
+                               WebGraphicsSyncPointCallback* callback);
+
+  virtual void prepareTexture();
+  virtual void finish();
+  virtual void flush();
+
+  virtual void bindBuffer(WebKit::WGC3Denum target, WebKit::WebGLId buffer);
+  virtual void bufferData(WebKit::WGC3Denum target,
+                          WebKit::WGC3Dsizeiptr size,
+                          const void* data,
+                          WebKit::WGC3Denum usage);
+  virtual void* mapBufferCHROMIUM(WebKit::WGC3Denum target,
+                                  WebKit::WGC3Denum access);
+  virtual WebKit::WGC3Dboolean unmapBufferCHROMIUM(WebKit::WGC3Denum target);
+
   // When set, MakeCurrent() will fail after this many times.
   void set_times_make_current_succeeds(int times) {
     times_make_current_succeeds_ = times;
@@ -142,12 +159,17 @@ class TestWebGraphicsContext3D : public FakeWebGraphicsContext3D {
   static const WebKit::WebGLId kExternalTextureId;
   virtual WebKit::WebGLId NextTextureId();
 
+  virtual WebKit::WebGLId NextBufferId();
+
  protected:
   TestWebGraphicsContext3D();
   TestWebGraphicsContext3D(
       const WebKit::WebGraphicsContext3D::Attributes& attributes);
 
+  void CallAllSyncPointCallbacks();
+
   unsigned context_id_;
+  unsigned next_buffer_id_;
   unsigned next_texture_id_;
   Attributes attributes_;
   bool have_extension_io_surface_;
@@ -157,12 +179,26 @@ class TestWebGraphicsContext3D : public FakeWebGraphicsContext3D {
   int times_end_query_succeeds_;
   bool context_lost_;
   WebGraphicsContextLostCallback* context_lost_callback_;
+  std::vector<WebGraphicsSyncPointCallback*> sync_point_callbacks_;
   std::vector<WebKit::WebGLId> textures_;
   base::hash_set<WebKit::WebGLId> used_textures_;
   std::vector<WebKit::WebGraphicsContext3D*> shared_contexts_;
   int max_texture_size_;
   int width_;
   int height_;
+
+  struct Buffer {
+    Buffer();
+    ~Buffer();
+
+    WebKit::WGC3Denum target;
+    scoped_ptr<uint8[]> pixels;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Buffer);
+  };
+  ScopedPtrHashMap<unsigned, Buffer> buffers_;
+  unsigned bound_buffer_;
 };
 
 }  // namespace cc
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index 08c2228..172f003 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -2583,6 +2583,18 @@ class LayerTreeHostTestAsyncReadback : public LayerTreeHostTest {
   }
 
   virtual void DidCommitAndDrawFrame() {
+    WaitForCallback();
+  }
+
+  void WaitForCallback() {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &LayerTreeHostTestAsyncReadback::NextStep,
+            base::Unretained(this)));
+  }
+
+  void NextStep() {
     int frame = layer_tree_host()->commit_number();
     switch (frame) {
       case 1:
@@ -2592,11 +2604,11 @@ class LayerTreeHostTestAsyncReadback : public LayerTreeHostTest {
         EXPECT_EQ(0u, callbacks_.size());
         break;
       case 2:
-        // Flush the message loops and make sure the callbacks run.
-        layer_tree_host()->SetNeedsCommit();
-        break;
-      case 3:
-        ASSERT_EQ(1u, callbacks_.size());
+        if (callbacks_.size() < 1u) {
+          WaitForCallback();
+          return;
+        }
+        EXPECT_EQ(1u, callbacks_.size());
         EXPECT_EQ(gfx::Size(10, 10).ToString(), callbacks_[0].ToString());
 
         child->RequestCopyAsBitmap(base::Bind(
@@ -2610,12 +2622,12 @@ class LayerTreeHostTestAsyncReadback : public LayerTreeHostTest {
             base::Unretained(this)));
         EXPECT_EQ(1u, callbacks_.size());
         break;
-      case 4:
-        // Flush the message loops and make sure the callbacks run.
-        layer_tree_host()->SetNeedsCommit();
-        break;
-      case 5:
-        ASSERT_EQ(4u, callbacks_.size());
+      case 3:
+        if (callbacks_.size() < 4u) {
+          WaitForCallback();
+          return;
+        }
+        EXPECT_EQ(4u, callbacks_.size());
         // The child was copied to a bitmap and passed back twice.
         EXPECT_EQ(gfx::Size(10, 10).ToString(), callbacks_[1].ToString());
         EXPECT_EQ(gfx::Size(10, 10).ToString(), callbacks_[2].ToString());
@@ -2632,7 +2644,9 @@ class LayerTreeHostTestAsyncReadback : public LayerTreeHostTest {
     callbacks_.push_back(gfx::Size(bitmap->width(), bitmap->height()));
   }
 
-  virtual void AfterTest() {}
+  virtual void AfterTest() {
+    EXPECT_EQ(4u, callbacks_.size());
+  }
 
   virtual scoped_ptr<OutputSurface> CreateOutputSurface() OVERRIDE {
     if (use_gl_renderer_)
diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
index ef8cce7..60c0452 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -547,7 +547,20 @@ void ThreadProxy::Stop() {
   DCHECK(IsMainThread());
   DCHECK(started_);
 
-  // Synchronously deletes the impl.
+  // Synchronously finishes pending GL operations and deletes the impl.
+  // The two steps are done as separate post tasks, so that tasks posted
+  // by the GL implementation due to the Finish can be executed by the
+  // renderer before shutting it down.
+  {
+    DebugScopedSetMainThreadBlocked main_thread_blocked(this);
+
+    CompletionEvent completion;
+    Proxy::ImplThread()->PostTask(
+        base::Bind(&ThreadProxy::FinishGLOnImplThread,
+                   impl_thread_weak_ptr_,
+                   &completion));
+    completion.Wait();
+  }
   {
     DebugScopedSetMainThreadBlocked main_thread_blocked(this);
 
@@ -1157,6 +1170,14 @@ void ThreadProxy::InitializeRendererOnImplThread(
   completion->Signal();
 }
 
+void ThreadProxy::FinishGLOnImplThread(CompletionEvent* completion) {
+  TRACE_EVENT0("cc", "ThreadProxy::FinishGLOnImplThread");
+  DCHECK(IsImplThread());
+  if (layer_tree_host_impl_->resource_provider())
+    layer_tree_host_impl_->resource_provider()->Finish();
+  completion->Signal();
+}
+
 void ThreadProxy::LayerTreeHostClosedOnImplThread(CompletionEvent* completion) {
   TRACE_EVENT0("cc", "ThreadProxy::LayerTreeHostClosedOnImplThread");
   DCHECK(IsImplThread());
diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h
index 0bc1cf1..01fe74e 100644
--- a/cc/trees/thread_proxy.h
+++ b/cc/trees/thread_proxy.h
@@ -162,6 +162,7 @@ class ThreadProxy : public Proxy,
   void InitializeRendererOnImplThread(CompletionEvent* completion,
                                       bool* initialize_succeeded,
                                       RendererCapabilities* capabilities);
+  void FinishGLOnImplThread(CompletionEvent* completion);
   void LayerTreeHostClosedOnImplThread(CompletionEvent* completion);
   void ManageTilesOnImplThread();
   void AcquireLayerTexturesForMainThreadOnImplThread(
diff --git a/content/common/gpu/client/gl_helper.cc b/content/common/gpu/client/gl_helper.cc
index cad3838..09dcd1c 100644
--- a/content/common/gpu/client/gl_helper.cc
+++ b/content/common/gpu/client/gl_helper.cc
@@ -555,7 +555,7 @@ void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(
                        GL_STREAM_READ);
 
   context_->readPixels(0, 0, size.width(), size.height(),
-                       GL_RGBA, GL_UNSIGNED_BYTE, 0);
+                       GL_RGBA, GL_UNSIGNED_BYTE, NULL);
   context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, 0);
   context_->GetCommandBufferProxy()->SignalSyncPoint(
       context_->insertSyncPoint(),