cc: Make ResourceProvider use bindless Produce/ConsumeTextureCHROMIUM

cc/resources/resource_provider_unittest.cc

Index: cc/resources/resource_provider_unittest.cc
diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc
index 53741ab30a83a0191ef6d0e99fbe41d5f25ed29c..ed3a3e1d558021c7f6b05658afd615300c6ddc02 100644
--- a/cc/resources/resource_provider_unittest.cc
+++ b/cc/resources/resource_provider_unittest.cc
@@ -97,10 +97,8 @@ class TextureStateTrackingContext : public TestWebGraphicsContext3D {
   MOCK_METHOD3(texParameteri, void(GLenum target, GLenum pname, GLint param));
   MOCK_METHOD1(waitSyncPoint, void(GLuint sync_point));
   MOCK_METHOD0(insertSyncPoint, GLuint(void));
-  MOCK_METHOD2(produceTextureCHROMIUM,
-               void(GLenum target, const GLbyte* mailbox));
-  MOCK_METHOD2(consumeTextureCHROMIUM,

[danakj, 2015/02/13 18:12:22]

dont we want consume direct here so we can test that the function is being
called still?

[sohanjg, 2015/02/16 13:39:56]

Hmm..the thing is we have 2 implementation of createAndConsumeTextureCHROMIUM,
in TestWebGraphicsContext3D and ResourceProviderContext. 
this is required because, it is expected to return the tex id, and hence cannot
be kept as empty virtual like consumeTextureCHROMIUM in TestWebGraphicsContext3D
.

I had tried removing the impl in TestWebGraphicsContext3D, but it seems some
gl_renderer_unittests (OutputSurfaceMockContext) needs it to be there, otherwise
it fails linking.


For most of the tests here the TestWebGraphicsContext3D version is invoked, and
some uses the ResourceProviderContext implementation.

If we add it as a MOCK here, then from TestGLES2Interface, it is unable to
decide which implementation to use, and it fails.

[danakj, 2015/02/18 18:37:28]

This would override the one from TestWebGraphicsContext3D, the same way
insertSyncPoint does.

ResourceProvderContext does not inherit from this class, so that seems unrelated
here?
Why does it need to be an empty virtual? Should it not be declared as
GLuint(GLenum target, const GLbyte* mailbox) in the mock here? Are you trying to
use a void return value for it still so it doesn't match what it's overriding?

[sohanjg, 2015/02/19 14:54:47]

Sorry for not being clear.

What i meant was, If we use MOCK method here we will have some failures.
Eg. In class ResourceProviderTestTextureFilters we are creating context of type
TextureStateTrackingContext (line:1648)
As a result, from line:1743, when we invoke ScopedReadLockGL, it invokes
LockForRead, which calls CreateAndConsumeTextureCHROMIUM, it ends up in the MOCK
method (from TestGLES2Interface), and we fail the DCHECK in ScopedReadLockGL
where it expects non-zero texture id.
https://code.google.com/p/chromium/codesearch#chromium/src/cc/resources/resou...

If we dont have the MOCK method here, then
TestWebGraphicsContext3D::CreateAndConsumeTextureCHROMIUM is invoked.

Same for, ResourceProviderTestTextureMailboxGLFilters class and
TextureMailbox_GLTextureExternalOES test which uses TextureStateTrackingContext.

-               void(GLenum target, const GLbyte* mailbox));
+  MOCK_METHOD3(produceTextureDirectCHROMIUM,
+               void(GLuint texture, GLenum target, const GLbyte* mailbox));
 
   // Force all textures to be consecutive numbers starting at "1",
   // so we easily can test for them.
@@ -258,19 +256,29 @@ class ResourceProviderContext : public TestWebGraphicsContext3D {
     return shared_data_->GenMailbox(mailbox);
   }
 
-  void produceTextureCHROMIUM(GLenum target, const GLbyte* mailbox) override {
-    CheckTextureIsBound(target);
-
+  void produceTextureDirectCHROMIUM(GLuint texture,
+                                    GLenum target,
+                                    const GLbyte* mailbox) override {
     // Delay moving the texture into the mailbox until the next
     // InsertSyncPoint, so that it is not visible to other contexts that
     // haven't waited on that sync point.
     scoped_ptr<PendingProduceTexture> pending(new PendingProduceTexture);
     memcpy(pending->mailbox, mailbox, sizeof(pending->mailbox));
     base::AutoLock lock_for_texture_access(namespace_->lock);
-    pending->texture = BoundTexture(target);
+    pending->texture = UnboundTexture(texture);
     pending_produce_textures_.push_back(pending.Pass());
   }
 
+  GLuint createAndConsumeTextureCHROMIUM(GLenum target,
+                                         const GLbyte* mailbox) override {
+    GLuint texture_id = createTexture();
+    base::AutoLock lock_for_texture_access(namespace_->lock);
+    scoped_refptr<TestTexture> texture =
+        shared_data_->ConsumeTexture(mailbox, last_waited_sync_point_);
+    namespace_->textures.Replace(texture_id, texture);
+    return texture_id;
+  }
+
   void consumeTextureCHROMIUM(GLenum target, const GLbyte* mailbox) override {

[danakj, 2015/02/13 18:12:22]

no longer used?

[sohanjg, 2015/02/16 13:39:56]

it is still used internally by TestWebGraphicsContext3D and
ResourceProviderContext implementation of createAndConsumeTextureCHROMIUM.

     CheckTextureIsBound(target);
     base::AutoLock lock_for_texture_access(namespace_->lock);
@@ -461,9 +469,9 @@ class ResourceProviderTest
     if (GetParam() == ResourceProvider::GLTexture) {
       unsigned texture = child_context_->createTexture();
       gpu::Mailbox gpu_mailbox;
-      child_context_->bindTexture(GL_TEXTURE_2D, texture);
       child_context_->genMailboxCHROMIUM(gpu_mailbox.name);
-      child_context_->produceTextureCHROMIUM(GL_TEXTURE_2D, gpu_mailbox.name);
+      child_context_->produceTextureDirectCHROMIUM(texture, GL_TEXTURE_2D,
+                                                   gpu_mailbox.name);
       *sync_point = child_context_->insertSyncPoint();
       EXPECT_LT(0u, *sync_point);
 
@@ -636,12 +644,11 @@ TEST_P(ResourceProviderTest, TransferGLResources) {
   }
 
   GLuint external_texture_id = child_context_->createExternalTexture();
-  child_context_->bindTexture(GL_TEXTURE_EXTERNAL_OES, external_texture_id);
 
   gpu::Mailbox external_mailbox;
   child_context_->genMailboxCHROMIUM(external_mailbox.name);
-  child_context_->produceTextureCHROMIUM(GL_TEXTURE_EXTERNAL_OES,
-                                         external_mailbox.name);
+  child_context_->produceTextureDirectCHROMIUM(
+      external_texture_id, GL_TEXTURE_EXTERNAL_OES, external_mailbox.name);
   const GLuint external_sync_point = child_context_->insertSyncPoint();
   ResourceProvider::ResourceId id4 =
       child_resource_provider_->CreateResourceFromTextureMailbox(
@@ -1723,9 +1730,8 @@ class ResourceProviderTestTextureFilters : public ResourceProviderTest {
       resource_ids_to_transfer.push_back(id);
       TransferableResourceArray list;
 
-      EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, child_texture_id));
       EXPECT_CALL(*child_context,
-                  produceTextureCHROMIUM(GL_TEXTURE_2D, _));
+                  produceTextureDirectCHROMIUM(_, GL_TEXTURE_2D, _));
       EXPECT_CALL(*child_context, insertSyncPoint());
       child_resource_provider->PrepareSendToParent(resource_ids_to_transfer,
                                                    &list);
@@ -1734,9 +1740,6 @@ class ResourceProviderTestTextureFilters : public ResourceProviderTest {
       ASSERT_EQ(1u, list.size());
       EXPECT_EQ(static_cast<unsigned>(child_filter), list[0].filter);
 
-      EXPECT_CALL(*parent_context,
-                  bindTexture(GL_TEXTURE_2D, parent_texture_id));
-      EXPECT_CALL(*parent_context, consumeTextureCHROMIUM(GL_TEXTURE_2D, _));

[danakj, 2015/02/13 18:12:22]

shouldnt we be getting a consumetexturedirectchromium here?

[sohanjg, 2015/02/16 13:39:56]

We have to make it MOCK method then, and there seems to be some issues with that
as mentioned above.

       parent_resource_provider->ReceiveFromChild(child_id, list);
       {
         parent_resource_provider->WaitSyncPointIfNeeded(list[0].id);
@@ -1820,7 +1823,7 @@ TEST_P(ResourceProviderTest, TransferMailboxResources) {
       GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, &data);
   gpu::Mailbox mailbox;
   context()->genMailboxCHROMIUM(mailbox.name);
-  context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+  context()->produceTextureDirectCHROMIUM(texture, GL_TEXTURE_2D, mailbox.name);
   uint32 sync_point = context()->insertSyncPoint();
 
   // All the logic below assumes that the sync points are all positive.
@@ -1854,14 +1857,15 @@ TEST_P(ResourceProviderTest, TransferMailboxResources) {
     EXPECT_EQ(0u, release_sync_point);
 
     context()->waitSyncPoint(list[0].mailbox_holder.sync_point);
-    unsigned other_texture = context()->createTexture();
-    context()->bindTexture(GL_TEXTURE_2D, other_texture);
-    context()->consumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+    unsigned other_texture =
+        context()->createAndConsumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
     uint8_t test_data[4] = { 0 };
     context()->GetPixels(
         gfx::Size(1, 1), RGBA_8888, test_data);
     EXPECT_EQ(0, memcmp(data, test_data, sizeof(data)));
-    context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+
+    context()->produceTextureDirectCHROMIUM(other_texture, GL_TEXTURE_2D,
+                                            mailbox.name);
     context()->deleteTexture(other_texture);
     list[0].mailbox_holder.sync_point = context()->insertSyncPoint();
     EXPECT_LT(0u, list[0].mailbox_holder.sync_point);
@@ -1905,14 +1909,15 @@ TEST_P(ResourceProviderTest, TransferMailboxResources) {
     EXPECT_EQ(0u, release_sync_point);
 
     context()->waitSyncPoint(list[0].mailbox_holder.sync_point);
-    unsigned other_texture = context()->createTexture();
-    context()->bindTexture(GL_TEXTURE_2D, other_texture);
-    context()->consumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+    unsigned other_texture =
+        context()->createAndConsumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
     uint8_t test_data[4] = { 0 };
     context()->GetPixels(
         gfx::Size(1, 1), RGBA_8888, test_data);
     EXPECT_EQ(0, memcmp(data, test_data, sizeof(data)));
-    context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+
+    context()->produceTextureDirectCHROMIUM(other_texture, GL_TEXTURE_2D,
+                                            mailbox.name);
     context()->deleteTexture(other_texture);
     list[0].mailbox_holder.sync_point = context()->insertSyncPoint();
     EXPECT_LT(0u, list[0].mailbox_holder.sync_point);
@@ -1933,8 +1938,8 @@ TEST_P(ResourceProviderTest, TransferMailboxResources) {
   }
 
   context()->waitSyncPoint(release_sync_point);
-  context()->bindTexture(GL_TEXTURE_2D, texture);
-  context()->consumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+  texture =
+      context()->createAndConsumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
   context()->deleteTexture(texture);
 }
 
@@ -2244,7 +2249,7 @@ TEST_P(ResourceProviderTest, LostContext) {
   context()->bindTexture(GL_TEXTURE_2D, texture);
   gpu::Mailbox mailbox;
   context()->genMailboxCHROMIUM(mailbox.name);
-  context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+  context()->produceTextureDirectCHROMIUM(texture, GL_TEXTURE_2D, mailbox.name);
   uint32 sync_point = context()->insertSyncPoint();
 
   EXPECT_LT(0u, sync_point);
@@ -2624,15 +2629,13 @@ class ResourceProviderTestTextureMailboxGLFilters
                                  false,
                                  1));
 
-    unsigned texture_id = 1;
     uint32 sync_point = 30;
     unsigned target = GL_TEXTURE_2D;
 
     EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
     EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
     EXPECT_CALL(*context, insertSyncPoint()).Times(0);
-    EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
-    EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
+    EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
     gpu::Mailbox gpu_mailbox;
     memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);
@@ -2661,12 +2664,8 @@ class ResourceProviderTestTextureMailboxGLFilters
       resource_provider->WaitSyncPointIfNeeded(id);
       Mock::VerifyAndClearExpectations(context);
 
-      // Using the texture does a consume of the mailbox.
-      EXPECT_CALL(*context, bindTexture(target, texture_id)).Times(2);
-      EXPECT_CALL(*context, consumeTextureCHROMIUM(target, _));
-
       EXPECT_CALL(*context, insertSyncPoint()).Times(0);
-      EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
+      EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
       // The sampler will reset these if |mailbox_nearest_neighbor| does not
       // match |sampler_filter|.
@@ -2685,10 +2684,9 @@ class ResourceProviderTestTextureMailboxGLFilters
       // necessary.
       EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
       EXPECT_CALL(*context, insertSyncPoint());
-      EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
+      EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
       EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
-      EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
     }
 
     resource_provider->DeleteResource(id);
@@ -2773,15 +2771,13 @@ TEST_P(ResourceProviderTest, TextureMailbox_GLTextureExternalOES) {
                                false,
                                1));
 
-  unsigned texture_id = 1;
   uint32 sync_point = 30;
   unsigned target = GL_TEXTURE_EXTERNAL_OES;
 
   EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
   EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
   EXPECT_CALL(*context, insertSyncPoint()).Times(0);
-  EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
-  EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
+  EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
   gpu::Mailbox gpu_mailbox;
   memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);
@@ -2803,12 +2799,8 @@ TEST_P(ResourceProviderTest, TextureMailbox_GLTextureExternalOES) {
     resource_provider->WaitSyncPointIfNeeded(id);
     Mock::VerifyAndClearExpectations(context);
 
-    // Using the texture does a consume of the mailbox.
-    EXPECT_CALL(*context, bindTexture(target, texture_id));
-    EXPECT_CALL(*context, consumeTextureCHROMIUM(target, _));
-
     EXPECT_CALL(*context, insertSyncPoint()).Times(0);
-    EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
+    EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
     ResourceProvider::ScopedReadLockGL lock(resource_provider.get(), id);
     Mock::VerifyAndClearExpectations(context);
@@ -2817,10 +2809,9 @@ TEST_P(ResourceProviderTest, TextureMailbox_GLTextureExternalOES) {
     // necessary.
     EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
     EXPECT_CALL(*context, insertSyncPoint());
-    EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
+    EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
     EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
-    EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
   }
 }
 
@@ -2854,8 +2845,7 @@ TEST_P(ResourceProviderTest,
   EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
   EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
   EXPECT_CALL(*context, insertSyncPoint()).Times(0);
-  EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
-  EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
+  EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
   gpu::Mailbox gpu_mailbox;
   memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);
@@ -2913,8 +2903,7 @@ TEST_P(ResourceProviderTest, TextureMailbox_WaitSyncPointIfNeeded_NoSyncPoint) {
   EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
   EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
   EXPECT_CALL(*context, insertSyncPoint()).Times(0);
-  EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
-  EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
+  EXPECT_CALL(*context, produceTextureDirectCHROMIUM(_, _, _)).Times(0);
 
   gpu::Mailbox gpu_mailbox;
   memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);