cc: Make ResourceProvider correctly handle transferring mailbox-created resources

cc/resource_provider_unittest.cc

Index: cc/resource_provider_unittest.cc
diff --git a/cc/resource_provider_unittest.cc b/cc/resource_provider_unittest.cc
index 0058d111f4cb238757f003d95d74003b28b986dc..29ff4108259b717d9218d29fd7f2a83b07212fa1 100644
--- a/cc/resource_provider_unittest.cc
+++ b/cc/resource_provider_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "cc/resource_provider.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "cc/output_surface.h"
 #include "cc/scoped_ptr_deque.h"
@@ -83,8 +84,10 @@ public:
         // If the latest sync point the context has waited on is before the sync
         // point for when the mailbox was set, pretend we never saw that
         // produceTexture.
-        if (m_syncPointForMailbox[mailbox] < syncPoint)
+        if (m_syncPointForMailbox[mailbox] > syncPoint) {
+            NOTREACHED();
             return scoped_ptr<Texture>();
+        }
         return m_textures.take(mailbox);
     }
 
@@ -564,6 +567,104 @@ TEST_P(ResourceProviderTest, DeleteTransferredResources)
     EXPECT_EQ(0u, childResourceProvider->numResources());
 }
 
+void ReleaseTextureMailbox(unsigned* releaseSyncPoint, unsigned syncPoint) {
+    *releaseSyncPoint = syncPoint;
+}
+
+TEST_P(ResourceProviderTest, TransferMailboxResources)
+{
+    // Resource transfer is only supported with GL textures for now.
+    if (GetParam() != ResourceProvider::GLTexture)
+        return;
+    unsigned texture = context()->createTexture();
+    context()->bindTexture(GL_TEXTURE_2D, texture);
+    uint8_t data[4] = {1, 2, 3, 4};
+    context()->texImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, &data);
+    Mailbox mailbox;
+    context()->genMailboxCHROMIUM(mailbox.name);
+    context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+    unsigned syncPoint = context()->insertSyncPoint();
+    unsigned releaseSyncPoint = 0;
+    TextureMailbox::ReleaseCallback callback = base::Bind(ReleaseTextureMailbox, &releaseSyncPoint);

[danakj, 2013/02/22 03:50:43]

i love Bind() so much..

+    ResourceProvider::ResourceId resource = m_resourceProvider->createResourceFromTextureMailbox(TextureMailbox(mailbox, callback, syncPoint));
+    EXPECT_EQ(1u, context()->textureCount());
+    EXPECT_EQ(0u, releaseSyncPoint);
+
+    {
+        // Transfer the resource, expect the sync points to be consistent.
+        ResourceProvider::ResourceIdArray resourceIdsToTransfer;
+        resourceIdsToTransfer.push_back(resource);
+        TransferableResourceList list;
+        m_resourceProvider->prepareSendToParent(resourceIdsToTransfer, &list);
+        EXPECT_LE(syncPoint, list.sync_point);

[danakj, 2013/02/22 03:50:43]

if all the sync points were 0 i think this would pass but that would be pretty
sketchy. can you EXPECT_NE(0u, syncPoint) here?

[piman, 2013/02/22 05:12:08]

I added a check for 0 < syncPoint higher up.

+        EXPECT_EQ(1u, list.resources.size());
+        EXPECT_EQ(0u, memcmp(mailbox.name, list.resources[0].mailbox.name, sizeof(mailbox.name)));
+        EXPECT_EQ(0u, releaseSyncPoint);
+
+        context()->waitSyncPoint(list.sync_point);
+        unsigned otherTexture = context()->createTexture();
+        context()->bindTexture(GL_TEXTURE_2D, otherTexture);
+        context()->consumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+        uint8_t testData[4] = {0};
+        context()->getPixels(gfx::Size(1, 1), GL_RGBA, testData);
+        EXPECT_EQ(0u, memcmp(data, testData, sizeof(data)));
+        context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+        context()->deleteTexture(otherTexture);
+        list.sync_point = context()->insertSyncPoint();
+
+        // Receive the resource, then delete it, expect the sync points to be consistent.
+        m_resourceProvider->receiveFromParent(list);
+        EXPECT_EQ(1u, context()->textureCount());
+        EXPECT_EQ(0u, releaseSyncPoint);
+
+        m_resourceProvider->deleteResource(resource);
+        EXPECT_LE(list.sync_point, releaseSyncPoint);

[danakj, 2013/02/22 03:50:43]

and EXPECT_NE(0u, list.sync_point) here?

[piman, 2013/02/22 05:12:08]

Added above (_LT)

+    }
+
+    syncPoint = releaseSyncPoint;

[danakj, 2013/02/22 03:50:43]

We do this whole thing a second time here to make sure that the texture was not
destroyed when we deleted the resource at the end of the first time? Can you add
a comment to explain?

[piman, 2013/02/22 05:12:08]

Done.

+    releaseSyncPoint = 0;
+    resource = m_resourceProvider->createResourceFromTextureMailbox(TextureMailbox(mailbox, callback, syncPoint));
+    EXPECT_EQ(1u, context()->textureCount());
+    EXPECT_EQ(0u, releaseSyncPoint);
+
+    {
+        // Transfer the resource, expect the sync points to be consistent.
+        ResourceProvider::ResourceIdArray resourceIdsToTransfer;
+        resourceIdsToTransfer.push_back(resource);
+        TransferableResourceList list;
+        m_resourceProvider->prepareSendToParent(resourceIdsToTransfer, &list);
+        EXPECT_LE(syncPoint, list.sync_point);

[danakj, 2013/02/22 03:50:43]

add EXPECT_NE here also

[piman, 2013/02/22 05:12:08]

Added above (_LT)

+        EXPECT_EQ(1u, list.resources.size());
+        EXPECT_EQ(0u, memcmp(mailbox.name, list.resources[0].mailbox.name, sizeof(mailbox.name)));
+        EXPECT_EQ(0u, releaseSyncPoint);
+
+        context()->waitSyncPoint(list.sync_point);
+        unsigned otherTexture = context()->createTexture();
+        context()->bindTexture(GL_TEXTURE_2D, otherTexture);
+        context()->consumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+        uint8_t testData[4] = {0};
+        context()->getPixels(gfx::Size(1, 1), GL_RGBA, testData);
+        EXPECT_EQ(0u, memcmp(data, testData, sizeof(data)));
+        context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+        context()->deleteTexture(otherTexture);
+        list.sync_point = context()->insertSyncPoint();
+
+        // Delete the resource, which shouldn't do anything.
+        m_resourceProvider->deleteResource(resource);
+        EXPECT_EQ(1u, context()->textureCount());
+        EXPECT_EQ(0u, releaseSyncPoint);
+
+        // Then receive the resource which should release the mailbox, expect the sync points to be consistent.
+        m_resourceProvider->receiveFromParent(list);
+        EXPECT_LE(list.sync_point, releaseSyncPoint);

[danakj, 2013/02/22 03:50:43]

add EXPECT_NE here also

[piman, 2013/02/22 05:12:08]

Added above (_LT)

+    }
+
+    context()->waitSyncPoint(releaseSyncPoint);
+    context()->bindTexture(GL_TEXTURE_2D, texture);
+    context()->consumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
+    context()->deleteTexture(texture);
+}
+
 class TextureStateTrackingContext : public TestWebGraphicsContext3D {
 public:
     MOCK_METHOD2(bindTexture, void(WGC3Denum target, WebGLId texture));