Allow filters to specify their initial visual rect.

Allow filters to specify their initial visual rect.

BUG=637056
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/8062fe31e77b286dfcf0f32127bb5e591449d11f
Cr-Commit-Position: refs/heads/master@{#411516}

[wkorman, 2016-08-11 22:04:13]

Description was changed from

==========
Allow filters to specify their initial visual rect.

BUG=637056
==========

to

==========
Allow filters to specify their initial visual rect.

BUG=637056
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[wkorman, 2016-08-11 22:04:43]

wkorman@chromium.org changed reviewers:
+ chrishtr@chromium.org, vmpstr@chromium.org

[chrishtr, 2016-08-11 22:18:44]

lgtm

[vmpstr, 2016-08-11 23:39:37]

https://codereview.chromium.org/2239913002/diff/1/cc/playback/display_item_li...
File cc/playback/display_item_list.h (right):

https://codereview.chromium.org/2239913002/diff/1/cc/playback/display_item_li...
cc/playback/display_item_list.h:89: const DisplayItemType&
CreateAndAppendPairedBeginItemWithVisualRect(
It seems like we could just have CreateAndAppendPairedBeginItem always take a
visual rect and require clients to specify it even if it's empty. Why do you
prefer this approach?

[wkorman, 2016-08-12 00:17:08]

https://codereview.chromium.org/2239913002/diff/1/cc/playback/display_item_li...
File cc/playback/display_item_list.h (right):

https://codereview.chromium.org/2239913002/diff/1/cc/playback/display_item_li...
cc/playback/display_item_list.h:89: const DisplayItemType&
CreateAndAppendPairedBeginItemWithVisualRect(
On 2016/08/11 at 23:39:37, vmpstr wrote:
> It seems like we could just have CreateAndAppendPairedBeginItem always take a
visual rect and require clients to specify it even if it's empty. Why do you
prefer this approach?

Yeah, I thought about that. There are ~41 callsites and in all but one case
they'd be passing an empty rect. And I also liked the idea of making it more
explicit via a separate method name.

If we could use a default arg I would have done that, but I think we can't due
to the following varargs.

In any case I'm fine with switching to one method and passing empty rect in the
41 callsites (only 14 are outside of tests) if you'd prefer.

[vmpstr, 2016-08-12 00:23:06]

I think it's fine as it is. It kind of also documents what the rect is by the
virtue of function naming. lgtm

[wkorman, 2016-08-12 00:25:20]

The CQ bit was checked by wkorman@chromium.org

[commit-bot: I haz the power, 2016-08-12 00:26:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 02:04:28]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-12 02:05:40]

Description was changed from

==========
Allow filters to specify their initial visual rect.

BUG=637056
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Allow filters to specify their initial visual rect.

BUG=637056
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/8062fe31e77b286dfcf0f32127bb5e591449d11f
Cr-Commit-Position: refs/heads/master@{#411516}
==========

[commit-bot: I haz the power, 2016-08-12 02:05:41]

Patchset 1 (id:??) landed as
https://crrev.com/8062fe31e77b286dfcf0f32127bb5e591449d11f
Cr-Commit-Position: refs/heads/master@{#411516}