cc: Make ResourceProvider correctly handle transferring mailbox-created resources

cc/resource_provider_unittest.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resource_provider.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "cc/output_surface.h"
 #include "cc/scoped_ptr_deque.h"
 #include "cc/scoped_ptr_hash_map.h"
 #include "cc/test/fake_output_surface.h"
 #include "cc/test/test_web_graphics_context_3d.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebGraphicsContext3D.h"
@@ skipping 59 matching lines @@
 
     scoped_ptr<Texture> consumeTexture(const WGC3Dbyte* mailboxName, unsigned syncPoint)
     {
         unsigned mailbox = 0;
         memcpy(&mailbox, mailboxName, sizeof(mailbox));
         DCHECK(mailbox && mailbox < m_nextMailBox);
 
         // 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);
     }
 
 private:
     ContextSharedData()
         : m_nextSyncPoint(1)
         , m_nextMailBox(1)
     { }
 
     unsigned m_nextSyncPoint;
@@ skipping 459 matching lines @@
         resourceIdsToTransfer.push_back(mappedId);
         TransferableResourceList list;
         m_resourceProvider->prepareSendToChild(childId, resourceIdsToTransfer, &list);
         EXPECT_NE(0u, list.sync_point);
         EXPECT_EQ(1u, list.resources.size());
         childResourceProvider->receiveFromParent(list);
     }
     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));
     MOCK_METHOD3(texParameteri, void(WGC3Denum target, WGC3Denum pname, WGC3Dint param));
 
     // Force all textures to be "1" so we can test for them.
     virtual WebKit::WebGLId NextTextureId() { return 1; }
 };
 
 TEST_P(ResourceProviderTest, ScopedSampler)
@@ skipping 159 matching lines @@
     Mock::VerifyAndClearExpectations(context);
 }
 
 INSTANTIATE_TEST_CASE_P(ResourceProviderTests,
                         ResourceProviderTest,
                         ::testing::Values(ResourceProvider::GLTexture,
                                           ResourceProvider::Bitmap));
 
 }  // namespace
 }  // namespace cc