Move sync point from TransferResourceList to individual TransferResources

Move sync point from TransferResourceList to individual TransferResources

This allows better granularity for sync points, and saves waits for the
mailbox-created resources. It also allows the ÜC patch to drop the problematic
TransferResourceList::Merge

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

[piman, 2013-02-26 06:44:23]

danakj: please review
jschuh: OWNERS for cc_messages.h

Note: this CL assumes waitSyncPoint is cheap, which it is after
https://codereview.chromium.org/12330129/ that has yet to land

[danakj, 2013-02-26 16:01:28]

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.cc
File cc/resource_provider.cc (right):

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.cc#ne...
cc/resource_provider.cc:745: GLC(context3d,
context3d->waitSyncPoint(it->sync_point));
To make sure we're on the same page: Is next step to make this store the mailbox
on the resource with the sync point instead of waiting and consuming it here?
Probably by replacing this function with createResourceFromTextureMailbox?

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.h
File cc/resource_provider.h (right):

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.h#new...
cc/resource_provider.h:126: void prepareSendToParent(const ResourceIdArray&,
TransferableResourceArray*);
nit: can you stick variable names in here re chromium style while you're
changing the function signatures?

https://codereview.chromium.org/12335088/diff/4001/content/common/cc_messages.h
File content/common/cc_messages.h (right):

https://codereview.chromium.org/12335088/diff/4001/content/common/cc_messages...
content/common/cc_messages.h:221: IPC_STRUCT_TRAITS_MEMBER(sync_point)
Probably not but I'll ask, would it be better to have these in the same order
they appear in the struct?

[danakj, 2013-02-26 16:04:20]

LGTM

[piman, 2013-02-26 23:09:27]

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.cc
File cc/resource_provider.cc (right):

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.cc#ne...
cc/resource_provider.cc:745: GLC(context3d,
context3d->waitSyncPoint(it->sync_point));
On 2013/02/26 16:01:28, danakj wrote:
> To make sure we're on the same page: Is next step to make this store the
mailbox
> on the resource with the sync point instead of waiting and consuming it here?
> Probably by replacing this function with createResourceFromTextureMailbox?

Something along those lines, merging the 2 paths has some benefits. I don't
think the callback mechanism scales very well will 100s of resources (both in
terms of run-time perf, memory and in terms of complexity), but we can figure
something out. Mostly for this CL I wanted to unblock the hack in the ÜC CL, the
rest can be optimized/simplified under the hood as a follow-up.

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.h
File cc/resource_provider.h (right):

https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.h#new...
cc/resource_provider.h:126: void prepareSendToParent(const ResourceIdArray&,
TransferableResourceArray*);
On 2013/02/26 16:01:28, danakj wrote:
> nit: can you stick variable names in here re chromium style while you're
> changing the function signatures?

Done.

https://codereview.chromium.org/12335088/diff/4001/content/common/cc_messages.h
File content/common/cc_messages.h (right):

https://codereview.chromium.org/12335088/diff/4001/content/common/cc_messages...
content/common/cc_messages.h:221: IPC_STRUCT_TRAITS_MEMBER(sync_point)
On 2013/02/26 16:01:28, danakj wrote:
> Probably not but I'll ask, would it be better to have these in the same order
> they appear in the struct?

No, you're right, done.

[jschuh, 2013-02-27 01:30:43]

On 2013/02/26 23:09:27, piman wrote:
> https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.cc
> File cc/resource_provider.cc (right):
> 
>
https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.cc#ne...
> cc/resource_provider.cc:745: GLC(context3d,
> context3d->waitSyncPoint(it->sync_point));
> On 2013/02/26 16:01:28, danakj wrote:
> > To make sure we're on the same page: Is next step to make this store the
> mailbox
> > on the resource with the sync point instead of waiting and consuming it
here?
> > Probably by replacing this function with createResourceFromTextureMailbox?
> 
> Something along those lines, merging the 2 paths has some benefits. I don't
> think the callback mechanism scales very well will 100s of resources (both in
> terms of run-time perf, memory and in terms of complexity), but we can figure
> something out. Mostly for this CL I wanted to unblock the hack in the ÜC CL,
the
> rest can be optimized/simplified under the hood as a follow-up.
> 
> https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.h
> File cc/resource_provider.h (right):
> 
>
https://codereview.chromium.org/12335088/diff/4001/cc/resource_provider.h#new...
> cc/resource_provider.h:126: void prepareSendToParent(const ResourceIdArray&,
> TransferableResourceArray*);
> On 2013/02/26 16:01:28, danakj wrote:
> > nit: can you stick variable names in here re chromium style while you're
> > changing the function signatures?
> 
> Done.
> 
>
https://codereview.chromium.org/12335088/diff/4001/content/common/cc_messages.h
> File content/common/cc_messages.h (right):
> 
>
https://codereview.chromium.org/12335088/diff/4001/content/common/cc_messages...
> content/common/cc_messages.h:221: IPC_STRUCT_TRAITS_MEMBER(sync_point)
> On 2013/02/26 16:01:28, danakj wrote:
> > Probably not but I'll ask, would it be better to have these in the same
order
> > they appear in the struct?
> 
> No, you're right, done.

Then there's really no security change, so rubberstamp lgtm.

[commit-bot: I haz the power, 2013-02-27 01:34:49]

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

[commit-bot: I haz the power, 2013-02-27 03:32:17]

Change committed as 184861