cc: Make ResourceProvider correctly handle transferring mailbox-created resources

cc: Make ResourceProvider correctly handle transferring mailbox-created resources

For mailbox-created resources, the texture ID is created lazily, in lockForRead. If we use a delegating renderer, we won't lock it, so at the time we transfer it to the parent and back, we don't have a texture ID, and we shouldn't be producing/consuming. We still need to correctly handle the synchronization logic.

This change implements all this, and adds tests.

BUG=164095
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184089

[danakj, 2013-02-22 03:21:26]

Can you describe the problem/solution in the description area please?

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

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest.cc
File cc/resource_provider_unittest.cc (right):

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:588: TextureMailbox::ReleaseCallback callback =
base::Bind(ReleaseTextureMailbox, &releaseSyncPoint);
i love Bind() so much..

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:599: EXPECT_LE(syncPoint, list.sync_point);
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?

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:621: EXPECT_LE(list.sync_point,
releaseSyncPoint);
and EXPECT_NE(0u, list.sync_point) here?

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:624: syncPoint = releaseSyncPoint;
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?

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:636: EXPECT_LE(syncPoint, list.sync_point);
add EXPECT_NE here also

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:659: EXPECT_LE(list.sync_point,
releaseSyncPoint);
add EXPECT_NE here also

[danakj, 2013-02-22 04:09:49]

LGTM with the nittyness resolved!

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

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest.cc
File cc/resource_provider_unittest.cc (right):

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:599: EXPECT_LE(syncPoint, list.sync_point);
On 2013/02/22 03:50:43, danakj wrote:
> 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?

I added a check for 0 < syncPoint higher up.

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:621: EXPECT_LE(list.sync_point,
releaseSyncPoint);
On 2013/02/22 03:50:43, danakj wrote:
> and EXPECT_NE(0u, list.sync_point) here?

Added above (_LT)

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:624: syncPoint = releaseSyncPoint;
On 2013/02/22 03:50:43, danakj wrote:
> 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?

Done.

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:636: EXPECT_LE(syncPoint, list.sync_point);
On 2013/02/22 03:50:43, danakj wrote:
> add EXPECT_NE here also

Added above (_LT)

https://codereview.chromium.org/12315046/diff/1/cc/resource_provider_unittest...
cc/resource_provider_unittest.cc:659: EXPECT_LE(list.sync_point,
releaseSyncPoint);
On 2013/02/22 03:50:43, danakj wrote:
> add EXPECT_NE here also

Added above (_LT)

[piman, 2013-02-22 05:12:47]

On Thu, Feb 21, 2013 at 7:21 PM, <danakj@chromium.org> wrote:

> Can you describe the problem/solution in the description area please?
>

Done.

>
>
https://codereview.chromium.**org/12315046/<https://codereview.chromium.org/1...
>

[commit-bot: I haz the power, 2013-02-22 05:12:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/12315046/1016

[commit-bot: I haz the power, 2013-02-22 11:17:41]

Change committed as 184089