cc: Use CHROMIUM_sync_query for CopyResource.

Use COMMANDS_COMPLETED queries instead of making the assumption
that it's safe to access the source resource once the command has
been processed on the service side.

BUG=269808

Review URL: https://codereview.chromium.org/245753003

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

7 files changed, 15 insertions, 7 deletions

diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index f208341..7ad1492 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -1267,7 +1267,8 @@ ResourceProvider::ResourceProvider(OutputSurface* output_surface,
       max_texture_size_(0),
       best_texture_format_(RGBA_8888),
       use_rgba_4444_texture_format_(use_rgba_4444_texture_format),
-      id_allocation_chunk_size_(id_allocation_chunk_size) {
+      id_allocation_chunk_size_(id_allocation_chunk_size),
+      use_sync_query_(false) {
   DCHECK(output_surface_->HasClient());
   DCHECK(id_allocation_chunk_size_);
 }
@@ -1300,6 +1301,7 @@ bool ResourceProvider::InitializeGL() {
   use_texture_storage_ext_ = caps.gpu.texture_storage;
   use_texture_usage_hint_ = caps.gpu.texture_usage;
   use_compressed_texture_etc1_ = caps.gpu.texture_format_etc1;
+  use_sync_query_ = caps.gpu.sync_query;
 
   GLES2Interface* gl = ContextGL();
   DCHECK(gl);
@@ -2243,13 +2245,10 @@ void ResourceProvider::CopyResource(ResourceId source_id, ResourceId dest_id) {
       gl->BindTexture(source_resource->target, source_resource->gl_id);
       BindImageForSampling(source_resource);
     }
+    DCHECK(use_sync_query_) << "CHROMIUM_sync_query extension missing";
     if (!source_resource->gl_read_lock_query_id)
       gl->GenQueriesEXT(1, &source_resource->gl_read_lock_query_id);
-    // Note: Use of COMMANDS_ISSUED target assumes that it's safe to access the
-    // source resource once the command has been processed on the service side.
-    // TODO(reveman): Implement COMMANDS_COMPLETED query that can be used to
-    // accurately determine when it's safe to access the source resource again.
-    gl->BeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM,
+    gl->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM,
                       source_resource->gl_read_lock_query_id);
     DCHECK(!dest_resource->image_id);
     dest_resource->allocated = true;
@@ -2261,7 +2260,7 @@ void ResourceProvider::CopyResource(ResourceId source_id, ResourceId dest_id) {
                             GLDataType(dest_resource->format));
     // End query and create a read lock fence that will prevent access to
     // source resource until CopyTextureCHROMIUM command has completed.
-    gl->EndQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM);
+    gl->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM);
     source_resource->read_lock_fence = make_scoped_refptr(
         new QueryFence(gl, source_resource->gl_read_lock_query_id));
   } else {
diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h
index 9853cb4..6136015 100644
--- a/cc/resources/resource_provider.h
+++ b/cc/resources/resource_provider.h
@@ -652,6 +652,8 @@ class CC_EXPORT ResourceProvider {
   scoped_ptr<IdAllocator> texture_id_allocator_;
   scoped_ptr<IdAllocator> buffer_id_allocator_;
 
+  bool use_sync_query_;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceProvider);
 };
 
diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc
index f3e2e30..f72a68a 100644
--- a/cc/resources/resource_provider_unittest.cc
+++ b/cc/resources/resource_provider_unittest.cc
@@ -3033,6 +3033,7 @@ TEST_P(ResourceProviderTest, CopyResource_GLTexture) {
   scoped_ptr<AllocationTrackingContext3D> context_owned(
       new StrictMock<AllocationTrackingContext3D>);
   AllocationTrackingContext3D* context = context_owned.get();
+  context_owned->set_support_sync_query(true);
 
   FakeOutputSurfaceClient output_surface_client;
   scoped_ptr<OutputSurface> output_surface(FakeOutputSurface::Create3d(
diff --git a/cc/test/test_web_graphics_context_3d.h b/cc/test/test_web_graphics_context_3d.h
index 39f8767..56ffc85 100644
--- a/cc/test/test_web_graphics_context_3d.h
+++ b/cc/test/test_web_graphics_context_3d.h
@@ -301,6 +301,9 @@ class TestWebGraphicsContext3D {
   void set_support_texture_storage(bool support) {
     test_capabilities_.gpu.texture_storage = support;
   }
+  void set_support_sync_query(bool support) {
+    test_capabilities_.gpu.sync_query = support;
+  }
 
   // When this context is lost, all contexts in its share group are also lost.
   void add_share_group_context(TestWebGraphicsContext3D* context3d) {
diff --git a/gpu/command_buffer/common/capabilities.cc b/gpu/command_buffer/common/capabilities.cc
index 9f35c82..efc63eb 100644
--- a/gpu/command_buffer/common/capabilities.cc
+++ b/gpu/command_buffer/common/capabilities.cc
@@ -17,6 +17,7 @@ Capabilities::Capabilities()
       texture_usage(false),
       texture_storage(false),
       discard_framebuffer(false),
+      sync_query(false),
       map_image(false) {}
 
 }  // namespace gpu
diff --git a/gpu/command_buffer/common/capabilities.h b/gpu/command_buffer/common/capabilities.h
index b6c34da..81e3d55 100644
--- a/gpu/command_buffer/common/capabilities.h
+++ b/gpu/command_buffer/common/capabilities.h
@@ -20,6 +20,7 @@ struct GPU_EXPORT Capabilities {
   bool texture_usage;
   bool texture_storage;
   bool discard_framebuffer;
+  bool sync_query;
 
   // Capabilities below are not populated by GLES2Decoder.
   bool map_image;
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 7575cb5..07d34d7 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2682,6 +2682,7 @@ Capabilities GLES2DecoderImpl::GetCapabilities() {
   caps.texture_storage = feature_info_->feature_flags().ext_texture_storage;
   caps.discard_framebuffer =
       feature_info_->feature_flags().ext_discard_framebuffer;
+  caps.sync_query = feature_info_->feature_flags().chromium_sync_query;
 
 #if defined(OS_MACOSX)
   // This is unconditionally true on mac, no need to test for it at runtime.