Revert "Revert 210138 "Perform glReadPixels with PBOs in the gpu, if PBO...""

Third try for getting asynchrous readpixels to work everywhere.
This is identical to: https://codereview.chromium.org/16831004/, but with the following fixes:

OutOfProcessPPAPITests.Graphics3D (removed a check in query_tracker.cc)
GLReadbackTest.ReadPixelsWithPBO (removed, no longer a valid test)
GLReadbackTest.ReadPixelsWithPBOAndQuery (now uses the correct query)

And, new in this CL:

Disable asynchronous readpixels on macosx prior to 10.8, as it would seem
that the GL_ARB_fence extension doesn't seem to work on any driver, and
there are crashes in readpixel itself on intel drivers. The original
CL only disabled the async readpixels path for intel drivers.


BUG=258169

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

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

19 files changed, 325 insertions, 121 deletions

diff --git a/gpu/GLES2/gl2extchromium.h b/gpu/GLES2/gl2extchromium.h
index 2568074..433242c 100644
--- a/gpu/GLES2/gl2extchromium.h
+++ b/gpu/GLES2/gl2extchromium.h
@@ -339,6 +339,9 @@ typedef void (GL_APIENTRYP PFNGLBINDUNIFORMLOCATIONCHROMIUMPROC) (
 #ifndef GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM
 #define GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM 0x84F5
 #endif
+#ifndef GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM
+#define GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM 0x84F6
+#endif
 #endif  /* GL_CHROMIUM_async_pixel_transfers */
 
 /* GL_CHROMIUM_copy_texture */
diff --git a/gpu/command_buffer/build_gles2_cmd_buffer.py b/gpu/command_buffer/build_gles2_cmd_buffer.py
index 460ae72..6555da2 100755
--- a/gpu/command_buffer/build_gles2_cmd_buffer.py
+++ b/gpu/command_buffer/build_gles2_cmd_buffer.py
@@ -826,6 +826,7 @@ _ENUM_LISTS = {
       'GL_COMMANDS_ISSUED_CHROMIUM',
       'GL_LATENCY_QUERY_CHROMIUM',
       'GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM',
+      'GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM',
     ],
   },
   'RenderBufferParameter': {
@@ -1908,7 +1909,8 @@ _FUNCTION_INFO = {
         'GLint x, GLint y, GLsizei width, GLsizei height, '
         'GLenumReadPixelFormat format, GLenumReadPixelType type, '
         'uint32 pixels_shm_id, uint32 pixels_shm_offset, '
-        'uint32 result_shm_id, uint32 result_shm_offset',
+        'uint32 result_shm_id, uint32 result_shm_offset, '
+        'GLboolean async',
     'result': ['uint32'],
     'defer_reads': True,
   },
diff --git a/gpu/command_buffer/client/gles2_cmd_helper_autogen.h b/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
index 742c97e..8d2fd7d 100644
--- a/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
+++ b/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
@@ -923,12 +923,12 @@
   void ReadPixels(
       GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
       GLenum type, uint32 pixels_shm_id, uint32 pixels_shm_offset,
-      uint32 result_shm_id, uint32 result_shm_offset) {
+      uint32 result_shm_id, uint32 result_shm_offset, GLboolean async) {
     gles2::cmds::ReadPixels* c = GetCmdSpace<gles2::cmds::ReadPixels>();
     if (c) {
       c->Init(
           x, y, width, height, format, type, pixels_shm_id, pixels_shm_offset,
-          result_shm_id, result_shm_offset);
+          result_shm_id, result_shm_offset, async);
     }
   }
 
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 1bc3f0b3..e13a1ec 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -2236,8 +2236,7 @@ void GLES2Implementation::ReadPixels(
     if (buffer && buffer->shm_id() != -1) {
       helper_->ReadPixels(xoffset, yoffset, width, height, format, type,
                           buffer->shm_id(), buffer->shm_offset(),
-                          0, 0);
-      buffer->set_transfer_ready_token(helper_->InsertToken());
+                          0, 0, true);
       CheckGLError();
     }
     return;
@@ -2269,7 +2268,8 @@ void GLES2Implementation::ReadPixels(
     helper_->ReadPixels(
         xoffset, yoffset, width, num_rows, format, type,
         buffer.shm_id(), buffer.offset(),
-        GetResultShmId(), GetResultShmOffset());
+        GetResultShmId(), GetResultShmOffset(),
+        false);
     WaitForCmd();
     if (*result != 0) {
       // when doing a y-flip we have to iterate through top-to-bottom chunks
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index 7208425..bdbde8f 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -1438,11 +1438,12 @@ TEST_F(GLES2ImplementationTest, ReadPixels2Reads) {
   Cmds expected;
   expected.read1.Init(
       0, 0, kWidth, kHeight / 2, kFormat, kType,
-      mem1.id, mem1.offset, result1.id, result1.offset);
+      mem1.id, mem1.offset, result1.id, result1.offset,
+      false);
   expected.set_token1.Init(GetNextToken());
   expected.read2.Init(
       0, kHeight / 2, kWidth, kHeight / 2, kFormat, kType,
-      mem2.id, mem2.offset, result2.id, result2.offset);
+      mem2.id, mem2.offset, result2.id, result2.offset, false);
   expected.set_token2.Init(GetNextToken());
   scoped_ptr<int8[]> buffer(new int8[kWidth * kHeight * kBytesPerPixel]);
 
@@ -1474,7 +1475,7 @@ TEST_F(GLES2ImplementationTest, ReadPixelsBadFormatType) {
   Cmds expected;
   expected.read.Init(
       0, 0, kWidth, kHeight, kFormat, kType,
-      mem1.id, mem1.offset, result1.id, result1.offset);
+      mem1.id, mem1.offset, result1.id, result1.offset, false);
   expected.set_token.Init(GetNextToken());
   scoped_ptr<int8[]> buffer(new int8[kWidth * kHeight * kBytesPerPixel]);
 
diff --git a/gpu/command_buffer/client/query_tracker.cc b/gpu/command_buffer/client/query_tracker.cc
index 7cdc7fd..35732bf 100644
--- a/gpu/command_buffer/client/query_tracker.cc
+++ b/gpu/command_buffer/client/query_tracker.cc
@@ -113,9 +113,7 @@ void QueryTracker::Query::Begin(GLES2Implementation* gl) {
       gl->helper()->BeginQueryEXT(target(), id(), shm_id(), shm_offset());
       break;
     case GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM:
-      // tell service about id, shared memory and count
-      gl->helper()->BeginQueryEXT(target(), id(), shm_id(), shm_offset());
-      break;
+    case GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM:
     default:
       // tell service about id, shared memory and count
       gl->helper()->BeginQueryEXT(target(), id(), shm_id(), shm_offset());
@@ -165,8 +163,7 @@ bool QueryTracker::Query::CheckResultsAvailable(
                              static_cast<uint64>(0xFFFFFFFFL));
           break;
         case GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM:
-          result_ = info_.sync->result;
-          break;
+        case GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM:
         default:
           result_ = info_.sync->result;
           break;
diff --git a/gpu/command_buffer/common/gles2_cmd_format_autogen.h b/gpu/command_buffer/common/gles2_cmd_format_autogen.h
index c84a363..11b9901 100644
--- a/gpu/command_buffer/common/gles2_cmd_format_autogen.h
+++ b/gpu/command_buffer/common/gles2_cmd_format_autogen.h
@@ -4887,7 +4887,7 @@ struct ReadPixels {
   void Init(
       GLint _x, GLint _y, GLsizei _width, GLsizei _height, GLenum _format,
       GLenum _type, uint32 _pixels_shm_id, uint32 _pixels_shm_offset,
-      uint32 _result_shm_id, uint32 _result_shm_offset) {
+      uint32 _result_shm_id, uint32 _result_shm_offset, GLboolean _async) {
     SetHeader();
     x = _x;
     y = _y;
@@ -4899,17 +4899,18 @@ struct ReadPixels {
     pixels_shm_offset = _pixels_shm_offset;
     result_shm_id = _result_shm_id;
     result_shm_offset = _result_shm_offset;
+    async = _async;
   }
 
   void* Set(
       void* cmd, GLint _x, GLint _y, GLsizei _width, GLsizei _height,
       GLenum _format, GLenum _type, uint32 _pixels_shm_id,
       uint32 _pixels_shm_offset, uint32 _result_shm_id,
-      uint32 _result_shm_offset) {
+      uint32 _result_shm_offset, GLboolean _async) {
     static_cast<ValueType*>(
         cmd)->Init(
             _x, _y, _width, _height, _format, _type, _pixels_shm_id,
-            _pixels_shm_offset, _result_shm_id, _result_shm_offset);
+            _pixels_shm_offset, _result_shm_id, _result_shm_offset, _async);
     return NextCmdAddress<ValueType>(cmd);
   }
 
@@ -4924,10 +4925,11 @@ struct ReadPixels {
   uint32 pixels_shm_offset;
   uint32 result_shm_id;
   uint32 result_shm_offset;
+  uint32 async;
 };
 
-COMPILE_ASSERT(sizeof(ReadPixels) == 44,
-               Sizeof_ReadPixels_is_not_44);
+COMPILE_ASSERT(sizeof(ReadPixels) == 48,
+               Sizeof_ReadPixels_is_not_48);
 COMPILE_ASSERT(offsetof(ReadPixels, header) == 0,
                OffsetOf_ReadPixels_header_not_0);
 COMPILE_ASSERT(offsetof(ReadPixels, x) == 4,
@@ -4950,6 +4952,8 @@ COMPILE_ASSERT(offsetof(ReadPixels, result_shm_id) == 36,
                OffsetOf_ReadPixels_result_shm_id_not_36);
 COMPILE_ASSERT(offsetof(ReadPixels, result_shm_offset) == 40,
                OffsetOf_ReadPixels_result_shm_offset_not_40);
+COMPILE_ASSERT(offsetof(ReadPixels, async) == 44,
+               OffsetOf_ReadPixels_async_not_44);
 
 struct ReleaseShaderCompiler {
   typedef ReleaseShaderCompiler ValueType;
diff --git a/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h b/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
index 87e426f..740e6cb 100644
--- a/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
+++ b/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
@@ -1803,7 +1803,8 @@ TEST_F(GLES2FormatTest, ReadPixels) {
       static_cast<uint32>(17),
       static_cast<uint32>(18),
       static_cast<uint32>(19),
-      static_cast<uint32>(20));
+      static_cast<uint32>(20),
+      static_cast<GLboolean>(21));
   EXPECT_EQ(static_cast<uint32>(cmds::ReadPixels::kCmdId),
             cmd.header.command);
   EXPECT_EQ(sizeof(cmd), cmd.header.size * 4u);
@@ -1817,6 +1818,7 @@ TEST_F(GLES2FormatTest, ReadPixels) {
   EXPECT_EQ(static_cast<uint32>(18), cmd.pixels_shm_offset);
   EXPECT_EQ(static_cast<uint32>(19), cmd.result_shm_id);
   EXPECT_EQ(static_cast<uint32>(20), cmd.result_shm_offset);
+  EXPECT_EQ(static_cast<GLboolean>(21), cmd.async);
   CheckBytesWrittenMatchesExpectedSize(
       next_cmd, sizeof(cmd));
 }
diff --git a/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h b/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
index c2fab6a..573aee9 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
+++ b/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
@@ -228,7 +228,7 @@ static const GLES2Util::EnumToString enum_to_string_table[] = {
   { 0x84F5, "GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM", },
   { 0x882A, "GL_DRAW_BUFFER5_NV", },
   { 0x80AA, "GL_SAMPLE_COVERAGE_VALUE", },
-  { 0x84F6, "GL_TEXTURE_BINDING_RECTANGLE_ARB", },
+  { 0x84F6, "GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM", },
   { 0x80AB, "GL_SAMPLE_COVERAGE_INVERT", },
   { 0x8FC4, "GL_SHADER_BINARY_VIV", },
   { 0x882B, "GL_DRAW_BUFFER6_NV", },
@@ -1080,6 +1080,8 @@ std::string GLES2Util::GetStringQueryTarget(uint32 value) {
     { GL_LATENCY_QUERY_CHROMIUM, "GL_LATENCY_QUERY_CHROMIUM" },
     { GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM,
     "GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM" },
+    { GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM,
+    "GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM" },
   };
   return GLES2Util::GetQualifiedEnumString(
       string_table, arraysize(string_table), value);
diff --git a/gpu/command_buffer/service/feature_info.cc b/gpu/command_buffer/service/feature_info.cc
index f759bef..b0fb120 100644
--- a/gpu/command_buffer/service/feature_info.cc
+++ b/gpu/command_buffer/service/feature_info.cc
@@ -119,7 +119,8 @@ FeatureInfo::FeatureFlags::FeatureFlags()
       enable_shader_name_hashing(false),
       enable_samplers(false),
       ext_draw_buffers(false),
-      ext_frag_depth(false) {
+      ext_frag_depth(false),
+      use_async_readpixels(false) {
 }
 
 FeatureInfo::Workarounds::Workarounds() :
@@ -642,6 +643,16 @@ void FeatureInfo::AddFeatures(const CommandLine& command_line) {
     feature_flags_.ext_frag_depth = true;
   }
 
+  bool ui_gl_fence_works =
+      extensions.Contains("GL_NV_fence") ||
+      extensions.Contains("GL_ARB_sync");
+
+  if (ui_gl_fence_works &&
+      extensions.Contains("GL_ARB_pixel_buffer_object") &&
+      !workarounds_.disable_async_readpixels) {
+    feature_flags_.use_async_readpixels = true;
+  }
+
   if (!disallowed_features_.swap_buffer_complete_callback)
     AddExtensionString("GL_CHROMIUM_swapbuffers_complete_callback");
 
diff --git a/gpu/command_buffer/service/feature_info.h b/gpu/command_buffer/service/feature_info.h
index de52159..a9ccf64 100644
--- a/gpu/command_buffer/service/feature_info.h
+++ b/gpu/command_buffer/service/feature_info.h
@@ -48,6 +48,7 @@ class GPU_EXPORT FeatureInfo : public base::RefCounted<FeatureInfo> {
     bool enable_samplers;
     bool ext_draw_buffers;
     bool ext_frag_depth;
+    bool use_async_readpixels;
   };
 
   struct Workarounds {
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 3c49a73..1075b34 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -59,6 +59,7 @@
 #include "gpu/command_buffer/service/vertex_array_manager.h"
 #include "gpu/command_buffer/service/vertex_attrib_manager.h"
 #include "ui/gl/gl_bindings.h"
+#include "ui/gl/gl_fence.h"
 #include "ui/gl/gl_image.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
@@ -477,6 +478,19 @@ class BackFramebuffer {
   DISALLOW_COPY_AND_ASSIGN(BackFramebuffer);
 };
 
+struct FenceCallback {
+  explicit FenceCallback()
+      : fence(gfx::GLFence::Create()) {
+    DCHECK(fence);
+  }
+  void AddCallback(base::Closure cb) {
+    callbacks.push_back(cb);
+  }
+  std::vector<base::Closure> callbacks;
+  scoped_ptr<gfx::GLFence> fence;
+};
+
+
 // }  // anonymous namespace.
 
 bool GLES2Decoder::GetServiceTextureId(uint32 client_texture_id,
@@ -587,6 +601,8 @@ class GLES2DecoderImpl : public GLES2Decoder {
   virtual bool HasMoreIdleWork() OVERRIDE;
   virtual void PerformIdleWork() OVERRIDE;
 
+  virtual void WaitForReadPixels(base::Closure callback) OVERRIDE;
+
   virtual void SetResizeCallback(
       const base::Callback<void(gfx::Size, float)>& callback) OVERRIDE;
 
@@ -1561,6 +1577,9 @@ class GLES2DecoderImpl : public GLES2Decoder {
            surface_->DeferDraws();
   }
 
+  void ProcessPendingReadPixels();
+  void FinishReadPixels(const cmds::ReadPixels& c, GLuint buffer);
+
   void ForceCompileShaderIfPending(Shader* shader);
 
   // Generate a member function prototype for each command in an automated and
@@ -1729,6 +1748,8 @@ class GLES2DecoderImpl : public GLES2Decoder {
 
   scoped_ptr<GPUTracer> gpu_tracer_;
 
+  std::queue<linked_ptr<FenceCallback> > pending_readpixel_fences_;
+
   DISALLOW_COPY_AND_ASSIGN(GLES2DecoderImpl);
 };
 
@@ -2820,6 +2841,7 @@ bool GLES2DecoderImpl::MakeCurrent() {
 }
 
 void GLES2DecoderImpl::ProcessFinishedAsyncTransfers() {
+  ProcessPendingReadPixels();
   if (engine() && query_manager_.get())
     query_manager_->ProcessPendingTransferQueries();
 
@@ -3548,6 +3570,7 @@ bool GLES2DecoderImpl::CreateShaderHelper(GLenum type, GLuint client_id) {
 
 void GLES2DecoderImpl::DoFinish() {
   glFinish();
+  ProcessPendingReadPixels();
   ProcessPendingQueries();
 }
 
@@ -6762,6 +6785,96 @@ error::Error GLES2DecoderImpl::HandleVertexAttribDivisorANGLE(
   return error::kNoError;
 }
 
+void GLES2DecoderImpl::FinishReadPixels(
+    const cmds::ReadPixels& c,
+    GLuint buffer) {
+  TRACE_EVENT0("gpu", "GLES2DecoderImpl::FinishReadPixels");
+  GLsizei width = c.width;
+  GLsizei height = c.height;
+  GLenum format = c.format;
+  GLenum type = c.type;
+  typedef cmds::ReadPixels::Result Result;
+  uint32 pixels_size;
+  Result* result = NULL;
+  if (c.result_shm_id != 0) {
+    result = GetSharedMemoryAs<Result*>(
+        c.result_shm_id, c.result_shm_offset, sizeof(*result));
+    if (!result) {
+      if (buffer != 0) {
+        glDeleteBuffersARB(1, &buffer);
+      }
+      return;
+    }
+  }
+  GLES2Util::ComputeImageDataSizes(
+      width, height, format, type, state_.pack_alignment, &pixels_size,
+      NULL, NULL);
+  void* pixels = GetSharedMemoryAs<void*>(
+      c.pixels_shm_id, c.pixels_shm_offset, pixels_size);
+  if (!pixels) {
+    if (buffer != 0) {
+      glDeleteBuffersARB(1, &buffer);
+    }
+    return;
+  }
+
+  if (buffer != 0) {
+    glBindBuffer(GL_PIXEL_PACK_BUFFER_ARB, buffer);
+    void* data = glMapBuffer(GL_PIXEL_PACK_BUFFER_ARB, GL_READ_ONLY);
+    memcpy(pixels, data, pixels_size);
+    // GL_PIXEL_PACK_BUFFER_ARB is currently unused, so we don't
+    // have to restore the state.
+    glUnmapBuffer(GL_PIXEL_PACK_BUFFER_ARB);
+    glBindBuffer(GL_PIXEL_PACK_BUFFER_ARB, 0);
+    glDeleteBuffersARB(1, &buffer);
+  }
+
+  if (result != NULL) {
+    *result = true;
+  }
+
+  GLenum read_format = GetBoundReadFrameBufferInternalFormat();
+  uint32 channels_exist = GLES2Util::GetChannelsForFormat(read_format);
+  if ((channels_exist & 0x0008) == 0 &&
+      workarounds().clear_alpha_in_readpixels) {
+    // Set the alpha to 255 because some drivers are buggy in this regard.
+    uint32 temp_size;
+
+    uint32 unpadded_row_size;
+    uint32 padded_row_size;
+    if (!GLES2Util::ComputeImageDataSizes(
+            width, 2, format, type, state_.pack_alignment, &temp_size,
+            &unpadded_row_size, &padded_row_size)) {
+      return;
+    }
+    // NOTE: Assumes the type is GL_UNSIGNED_BYTE which was true at the time
+    // of this implementation.
+    if (type != GL_UNSIGNED_BYTE) {
+      return;
+    }
+    switch (format) {
+      case GL_RGBA:
+      case GL_BGRA_EXT:
+      case GL_ALPHA: {
+        int offset = (format == GL_ALPHA) ? 0 : 3;
+        int step = (format == GL_ALPHA) ? 1 : 4;
+        uint8* dst = static_cast<uint8*>(pixels) + offset;
+        for (GLint yy = 0; yy < height; ++yy) {
+          uint8* end = dst + unpadded_row_size;
+          for (uint8* d = dst; d < end; d += step) {
+            *d = 255;
+          }
+          dst += padded_row_size;
+        }
+        break;
+      }
+      default:
+        break;
+    }
+  }
+}
+
+
 error::Error GLES2DecoderImpl::HandleReadPixels(
     uint32 immediate_data_size, const cmds::ReadPixels& c) {
   if (ShouldDeferReads())
@@ -6772,6 +6885,7 @@ error::Error GLES2DecoderImpl::HandleReadPixels(
   GLsizei height = c.height;
   GLenum format = c.format;
   GLenum type = c.type;
+  GLboolean async = c.async;
   if (width < 0 || height < 0) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glReadPixels", "dimensions < 0");
     return error::kNoError;
@@ -6871,6 +6985,25 @@ error::Error GLES2DecoderImpl::HandleReadPixels(
       dst += padded_row_size;
     }
   } else {
+    if (async && features().use_async_readpixels) {
+      GLuint buffer;
+      glGenBuffersARB(1, &buffer);
+      glBindBuffer(GL_PIXEL_PACK_BUFFER_ARB, buffer);
+      glBufferData(GL_PIXEL_PACK_BUFFER_ARB, pixels_size, NULL, GL_STREAM_READ);
+      GLenum error = glGetError();
+      if (error == GL_NO_ERROR) {
+        glReadPixels(x, y, width, height, format, type, 0);
+        pending_readpixel_fences_.push(linked_ptr<FenceCallback>(
+            new FenceCallback()));
+        WaitForReadPixels(base::Bind(
+            &GLES2DecoderImpl::FinishReadPixels,
+            base::internal::SupportsWeakPtrBase::StaticAsWeakPtr
+            <GLES2DecoderImpl>(this),
+            c, buffer));
+        glBindBuffer(GL_PIXEL_PACK_BUFFER_ARB, 0);
+        return error::kNoError;
+      }
+    }
     glReadPixels(x, y, width, height, format, type, pixels);
   }
   GLenum error = LOCAL_PEEK_GL_ERROR("glReadPixels");
@@ -6878,51 +7011,7 @@ error::Error GLES2DecoderImpl::HandleReadPixels(
     if (result != NULL) {
       *result = true;
     }
-
-    GLenum read_format = GetBoundReadFrameBufferInternalFormat();
-    uint32 channels_exist = GLES2Util::GetChannelsForFormat(read_format);
-    if ((channels_exist & 0x0008) == 0 &&
-        workarounds().clear_alpha_in_readpixels) {
-      // Set the alpha to 255 because some drivers are buggy in this regard.
-      uint32 temp_size;
-
-      uint32 unpadded_row_size;
-      uint32 padded_row_size;
-      if (!GLES2Util::ComputeImageDataSizes(
-          width, 2, format, type, state_.pack_alignment, &temp_size,
-          &unpadded_row_size, &padded_row_size)) {
-        LOCAL_SET_GL_ERROR(
-            GL_INVALID_VALUE, "glReadPixels", "dimensions out of range");
-        return error::kNoError;
-      }
-      // NOTE: Assumes the type is GL_UNSIGNED_BYTE which was true at the time
-      // of this implementation.
-      if (type != GL_UNSIGNED_BYTE) {
-        LOCAL_SET_GL_ERROR(
-            GL_INVALID_OPERATION, "glReadPixels",
-            "unsupported readPixel format");
-        return error::kNoError;
-      }
-      switch (format) {
-        case GL_RGBA:
-        case GL_BGRA_EXT:
-        case GL_ALPHA: {
-          int offset = (format == GL_ALPHA) ? 0 : 3;
-          int step = (format == GL_ALPHA) ? 1 : 4;
-          uint8* dst = static_cast<uint8*>(pixels) + offset;
-          for (GLint yy = 0; yy < height; ++yy) {
-            uint8* end = dst + unpadded_row_size;
-            for (uint8* d = dst; d < end; d += step) {
-              *d = 255;
-            }
-            dst += padded_row_size;
-          }
-          break;
-        }
-        default:
-          break;
-      }
-    }
+    FinishReadPixels(c, 0);
   }
 
   return error::kNoError;
@@ -9124,11 +9213,35 @@ bool GLES2DecoderImpl::ProcessPendingQueries() {
   return query_manager_->HavePendingQueries();
 }
 
+// Note that if there are no pending readpixels right now,
+// this function will call the callback immediately.
+void GLES2DecoderImpl::WaitForReadPixels(base::Closure callback) {
+  if (features().use_async_readpixels && !pending_readpixel_fences_.empty()) {
+    pending_readpixel_fences_.back()->callbacks.push_back(callback);
+  } else {
+    callback.Run();
+  }
+}
+
+void GLES2DecoderImpl::ProcessPendingReadPixels() {
+  while (!pending_readpixel_fences_.empty() &&
+         pending_readpixel_fences_.front()->fence->HasCompleted()) {
+    std::vector<base::Closure> callbacks =
+        pending_readpixel_fences_.front()->callbacks;
+    pending_readpixel_fences_.pop();
+    for (size_t i = 0; i < callbacks.size(); i++) {
+      callbacks[i].Run();
+    }
+  }
+}
+
 bool GLES2DecoderImpl::HasMoreIdleWork() {
-  return async_pixel_transfer_manager_->NeedsProcessMorePendingTransfers();
+  return !pending_readpixel_fences_.empty() ||
+      async_pixel_transfer_manager_->NeedsProcessMorePendingTransfers();
 }
 
 void GLES2DecoderImpl::PerformIdleWork() {
+  ProcessPendingReadPixels();
   if (!async_pixel_transfer_manager_->NeedsProcessMorePendingTransfers())
     return;
   async_pixel_transfer_manager_->ProcessMorePendingTransfers();
@@ -9146,6 +9259,7 @@ error::Error GLES2DecoderImpl::HandleBeginQueryEXT(
     case GL_COMMANDS_ISSUED_CHROMIUM:
     case GL_LATENCY_QUERY_CHROMIUM:
     case GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM:
+    case GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM:
     case GL_GET_ERROR_QUERY_CHROMIUM:
       break;
     default:
@@ -9158,6 +9272,8 @@ error::Error GLES2DecoderImpl::HandleBeginQueryEXT(
       break;
   }
 
+  // TODO(hubbe): Make it possible to have one query per type running at the
+  // same time.
   if (state_.current_query.get()) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION, "glBeginQueryEXT", "query already in progress");
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.h b/gpu/command_buffer/service/gles2_cmd_decoder.h
index 4f66a62..57445c7 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.h
@@ -213,6 +213,7 @@ class GPU_EXPORT GLES2Decoder : public base::SupportsWeakPtr<GLES2Decoder>,
   virtual void SetWaitSyncPointCallback(
       const WaitSyncPointCallback& callback) = 0;
 
+  virtual void WaitForReadPixels(base::Closure callback) = 0;
   virtual uint32 GetTextureUploadCount() = 0;
   virtual base::TimeDelta GetTotalTextureUploadTime() = 0;
   virtual base::TimeDelta GetTotalProcessingCommandsTime() = 0;
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_mock.h b/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
index 5fe0600..e6c497f 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
@@ -102,6 +102,8 @@ class MockGLES2Decoder : public GLES2Decoder {
                void(const ShaderCacheCallback& callback));
   MOCK_METHOD1(SetWaitSyncPointCallback,
                void(const WaitSyncPointCallback& callback));
+  MOCK_METHOD1(WaitForReadPixels,
+               void(base::Closure callback));
   MOCK_METHOD0(GetTextureUploadCount, uint32());
   MOCK_METHOD0(GetTotalTextureUploadTime, base::TimeDelta());
   MOCK_METHOD0(GetTotalProcessingCommandsTime, base::TimeDelta());
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index 09ed184..b426286 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -2316,7 +2316,8 @@ void GLES2DecoderTest::CheckReadPixelsOutOfRange(
   cmd.Init(in_read_x, in_read_y, in_read_width, in_read_height,
            kFormat, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
 
   GLint unpadded_row_size = emu.ComputeImageDataSize(in_read_width, 1);
@@ -2393,7 +2394,8 @@ TEST_F(GLES2DecoderTest, ReadPixels) {
   ReadPixels cmd;
   cmd.Init(0, 0, kWidth, kHeight, GL_RGB, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   for (GLint yy = 0; yy < kHeight; ++yy) {
     EXPECT_TRUE(emu.CompareRowSegment(
@@ -2440,7 +2442,8 @@ TEST_F(GLES2DecoderRGBBackbufferTest, ReadPixelsNoAlphaBackbuffer) {
   ReadPixels cmd;
   cmd.Init(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   for (GLint yy = 0; yy < kHeight; ++yy) {
     EXPECT_TRUE(emu.CompareRowSegment(
@@ -2477,34 +2480,41 @@ TEST_F(GLES2DecoderTest, ReadPixelsInvalidArgs) {
   ReadPixels cmd;
   cmd.Init(0, 0, -1, 1, GL_RGB, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_VALUE, GetGLError());
   cmd.Init(0, 0, 1, -1, GL_RGB, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_VALUE, GetGLError());
   cmd.Init(0, 0, 1, 1, GL_RGB, GL_INT,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_ENUM, GetGLError());
   cmd.Init(0, 0, 1, 1, GL_RGB, GL_UNSIGNED_BYTE,
            kInvalidSharedMemoryId, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
   cmd.Init(0, 0, 1, 1, GL_RGB, GL_UNSIGNED_BYTE,
            pixels_shm_id, kInvalidSharedMemoryOffset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
   cmd.Init(0, 0, 1, 1, GL_RGB, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           kInvalidSharedMemoryId, result_shm_offset);
+           kInvalidSharedMemoryId, result_shm_offset,
+           false);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
   cmd.Init(0, 0, 1, 1, GL_RGB, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, kInvalidSharedMemoryOffset);
+           result_shm_id, kInvalidSharedMemoryOffset,
+           false);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
 }
 
@@ -4932,7 +4942,8 @@ TEST_F(GLES2DecoderTest, ReadPixelsGLError) {
   ReadPixels cmd;
   cmd.Init(x, y, width, height, kFormat, GL_UNSIGNED_BYTE,
            pixels_shm_id, pixels_shm_offset,
-           result_shm_id, result_shm_offset);
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_OUT_OF_MEMORY, GetGLError());
 }
@@ -6664,8 +6675,9 @@ TEST_F(GLES2DecoderWithShaderTest, UnClearedAttachmentsGetClearedOnReadPixels) {
   uint32 pixels_shm_offset = kSharedMemoryOffset + sizeof(*result);
   ReadPixels cmd;
   cmd.Init(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE,
-         pixels_shm_id, pixels_shm_offset,
-         result_shm_id, result_shm_offset);
+           pixels_shm_id, pixels_shm_offset,
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
@@ -6724,8 +6736,9 @@ TEST_F(GLES2DecoderManualInitTest,
   uint32 pixels_shm_offset = kSharedMemoryOffset + sizeof(Result);
   ReadPixels cmd;
   cmd.Init(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE,
-         pixels_shm_id, pixels_shm_offset,
-         result_shm_id, result_shm_offset);
+           pixels_shm_id, pixels_shm_offset,
+           result_shm_id, result_shm_offset,
+           false);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
diff --git a/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h b/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
index 444cec3..b8194d9 100644
--- a/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
@@ -300,6 +300,7 @@ static const GLenum valid_query_target_table[] = {
   GL_COMMANDS_ISSUED_CHROMIUM,
   GL_LATENCY_QUERY_CHROMIUM,
   GL_ASYNC_PIXEL_TRANSFERS_COMPLETED_CHROMIUM,
+  GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM,
 };
 
 static const GLenum valid_read_pixel_format_table[] = {
diff --git a/gpu/command_buffer/service/query_manager.cc b/gpu/command_buffer/service/query_manager.cc
index 4af0349..7566099 100644
--- a/gpu/command_buffer/service/query_manager.cc
+++ b/gpu/command_buffer/service/query_manager.cc
@@ -288,6 +288,62 @@ void CommandLatencyQuery::Destroy(bool /* have_context */) {
 CommandLatencyQuery::~CommandLatencyQuery() {
 }
 
+
+class AsyncReadPixelsCompletedQuery
+    : public QueryManager::Query,
+      public base::SupportsWeakPtr<AsyncReadPixelsCompletedQuery> {
+ public:
+  AsyncReadPixelsCompletedQuery(
+      QueryManager* manager, GLenum target, int32 shm_id, uint32 shm_offset);
+
+  virtual bool Begin() OVERRIDE;
+  virtual bool End(uint32 submit_count) OVERRIDE;
+  virtual bool Process() OVERRIDE;
+  virtual void Destroy(bool have_context) OVERRIDE;
+
+ protected:
+  void Complete();
+  virtual ~AsyncReadPixelsCompletedQuery();
+};
+
+AsyncReadPixelsCompletedQuery::AsyncReadPixelsCompletedQuery(
+    QueryManager* manager, GLenum target, int32 shm_id, uint32 shm_offset)
+    : Query(manager, target, shm_id, shm_offset) {
+}
+
+bool AsyncReadPixelsCompletedQuery::Begin() {
+  return true;
+}
+
+bool AsyncReadPixelsCompletedQuery::End(uint32 submit_count) {
+  if (!AddToPendingQueue(submit_count)) {
+    return false;
+  }
+  manager()->decoder()->WaitForReadPixels(
+      base::Bind(&AsyncReadPixelsCompletedQuery::Complete,
+                 AsWeakPtr()));
+
+  return true;
+}
+
+void AsyncReadPixelsCompletedQuery::Complete() {
+  MarkAsCompleted(1);
+}
+
+bool AsyncReadPixelsCompletedQuery::Process() {
+  return true;
+}
+
+void AsyncReadPixelsCompletedQuery::Destroy(bool /* have_context */) {
+  if (!IsDeleted()) {
+    MarkAsDeleted();
+  }
+}
+
+AsyncReadPixelsCompletedQuery::~AsyncReadPixelsCompletedQuery() {
+}
+
+
 class GetErrorQuery : public QueryManager::Query {
  public:
   GetErrorQuery(
@@ -379,6 +435,10 @@ QueryManager::Query* QueryManager::CreateQuery(
       query = new AsyncPixelTransfersCompletedQuery(
           this, target, shm_id, shm_offset);
       break;
+    case GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM:
+      query = new AsyncReadPixelsCompletedQuery(
+          this, target, shm_id, shm_offset);
+      break;
     case GL_GET_ERROR_QUERY_CHROMIUM:
       query = new GetErrorQuery(this, target, shm_id, shm_offset);
       break;
diff --git a/gpu/command_buffer/tests/gl_readback_unittests.cc b/gpu/command_buffer/tests/gl_readback_unittests.cc
index e725213..4ebb09a 100644
--- a/gpu/command_buffer/tests/gl_readback_unittests.cc
+++ b/gpu/command_buffer/tests/gl_readback_unittests.cc
@@ -6,6 +6,9 @@
 #include <GLES2/gl2ext.h>
 #include <GLES2/gl2extchromium.h>
 
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "gpu/command_buffer/tests/gl_manager.h"
 #include "gpu/command_buffer/tests/gl_test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -23,38 +26,28 @@ class GLReadbackTest : public testing::Test {
     gl_.Destroy();
   }
 
-  GLManager gl_;
-};
+  static void WaitForQueryCallback(int q, base::Closure cb) {
+    unsigned int done = 0;
+    glGetQueryObjectuivEXT(q, GL_QUERY_RESULT_AVAILABLE_EXT, &done);
+    if (done) {
+      cb.Run();
+    } else {
+      base::MessageLoop::current()->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&WaitForQueryCallback, q, cb),
+          base::TimeDelta::FromMilliseconds(3));
+    }
+  }
 
+  void WaitForQuery(int q) {
+    base::RunLoop run_loop;
+    WaitForQueryCallback(q, run_loop.QuitClosure());
+    run_loop.Run();
+  }
 
-TEST_F(GLReadbackTest, ReadPixelsWithPBO) {
-  const GLint kBytesPerPixel = 4;
-  const GLint kWidth = 2;
-  const GLint kHeight = 2;
+  GLManager gl_;
+};
 
-  GLuint b;
-  glClearColor(0.0, 0.0, 1.0, 1.0);
-  glClear(GL_COLOR_BUFFER_BIT);
-  glGenBuffers(1, &b);
-  glBindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, b);
-  glBufferData(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
-               kWidth * kHeight * kBytesPerPixel,
-               NULL,
-               GL_STREAM_READ);
-  glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, 0);
-  unsigned char *data = static_cast<unsigned char *>(
-      glMapBufferCHROMIUM(
-          GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
-          GL_READ_ONLY));
-  EXPECT_TRUE(data);
-  EXPECT_EQ(data[0], 0);   // red
-  EXPECT_EQ(data[1], 0);   // green
-  EXPECT_EQ(data[2], 255); // blue
-  glUnmapBufferCHROMIUM(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM);
-  glBindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, 0);
-  glDeleteBuffers(1, &b);
-  GLTestHelper::CheckGLError("no errors", __LINE__);
-}
 
 TEST_F(GLReadbackTest, ReadPixelsWithPBOAndQuery) {
   const GLint kBytesPerPixel = 4;
@@ -71,14 +64,11 @@ TEST_F(GLReadbackTest, ReadPixelsWithPBOAndQuery) {
                kWidth * kHeight * kBytesPerPixel,
                NULL,
                GL_STREAM_READ);
-  glBeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM, q);
+  glBeginQueryEXT(GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM, q);
   glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, 0);
-  glEndQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM);
+  glEndQueryEXT(GL_ASYNC_READ_PIXELS_COMPLETED_CHROMIUM);
   glFlush();
-  unsigned int done = 0;
-  while (!done) {
-    glGetQueryObjectuivEXT(q, GL_QUERY_RESULT_AVAILABLE_EXT, &done);
-  }
+  WaitForQuery(q);
 
   // TODO(hubbe): Check that glMapBufferCHROMIUM does not block here.
   unsigned char *data = static_cast<unsigned char *>(
diff --git a/gpu/config/gpu_driver_bug_list_json.cc b/gpu/config/gpu_driver_bug_list_json.cc
index 597fc32..76ea7d2 100644
--- a/gpu/config/gpu_driver_bug_list_json.cc
+++ b/gpu/config/gpu_driver_bug_list_json.cc
@@ -389,8 +389,7 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST(
     {
       "id": 25,
       "cr_bugs": [152225],
-      "description":
-          "Intel OSX drivers prior to mountain lion crashes when using PBOs",
+      "description": "GL_ARB_sync doesn't work on OSX 10.7",
       "os": {
         "type": "macosx",
         "version": {
@@ -398,7 +397,6 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST(
           "number": "10.8"
         }
       },
-      "vendor_id": "0x8086",
       "features": [
         "disable_async_readpixels"
       ]