cc: Fix picture layer impl not to create a tiling with invalid size.

cc: Fix perftest not to create infinite pile and tiling.

Picture layer impl has been changed to use pile tiling size as layer
bounds while creating picture layer tilings. Therefore, perftests should
not create inifinite piles as they result in picture layer impls
creating infinite tilings which exhaust memory.

BUG=428844
Committed: https://crrev.com/f0d244108190fd3352db49a27b8b57db7ad9fe13
Cr-Commit-Position: refs/heads/master@{#302627}

[USE eero AT chromium.org, 2014-10-30 17:23:51]

e.hakkinen@samsung.com changed reviewers:
+ danakj@chromium.org, enne@chromium.org

[USE eero AT chromium.org, 2014-10-30 17:25:24]

PTAL.

I'm not sure if this issue should be fixed in picture layer impl or if
performance unit tests are set up incorrectly.

[danakj, 2014-10-30 17:27:09]

https://codereview.chromium.org/684273003/diff/1/cc/layers/picture_layer_impl.cc
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/684273003/diff/1/cc/layers/picture_layer_impl...
cc/layers/picture_layer_impl.cc:905: PictureLayerTiling* tiling =
tilings_->AddTiling(contents_scale, bounds());
I don't think this is the right answer. If a layer has a pile with billion sized
bounds, it's bounds would be billion sized too. Which perf tests are having the
issue, cc perftests? I had to fix some unit tests with a similar issue.

[USE eero AT chromium.org, 2014-10-30 17:40:30]

On 2014/10/30 17:27:09, danakj wrote:
> I don't think this is the right answer.

OK.

> If a layer has a pile with billion sized bounds, it's bounds would be billion
> sized too. Which perf tests are having the issue, cc perftests?

Yes, cc tile manager perftests.

Looking closer the unittests, it seems that TileManagerPerfTest specifically
uses cc::FakePicturePileImpl::CreateInfiniteFilledPile to create a picture pile
with as large bounds as possible.

[danakj, 2014-10-30 18:05:17]

On Thu, Oct 30, 2014 at 1:40 PM, <e.hakkinen@samsung.com> wrote:

> On 2014/10/30 17:27:09, danakj wrote:
>
>> I don't think this is the right answer.
>>
>
> OK.
>
>  If a layer has a pile with billion sized bounds, it's bounds would be
>> billion
>> sized too. Which perf tests are having the issue, cc perftests?
>>
>
> Yes, cc tile manager perftests.
>
> Looking closer the unittests, it seems that TileManagerPerfTest
> specifically
> uses cc::FakePicturePileImpl::CreateInfiniteFilledPile to create a
> picture pile
> with as large bounds as possible.
>

Ya ok, figures. Make it create a pile the size of the layer it's trying to
test instead :)

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[USE eero AT chromium.org, 2014-11-04 15:25:40]

On 2014/10/30 18:05:17, danakj wrote:
> Ya ok, figures. Make it create a pile the size of the layer it's trying to
> test instead :)

Done. PTAL.

[danakj, 2014-11-04 15:39:55]

https://codereview.chromium.org/684273003/diff/40001/cc/resources/tile_manage...
File cc/resources/tile_manager_perftest.cc (right):

https://codereview.chromium.org/684273003/diff/40001/cc/resources/tile_manage...
cc/resources/tile_manager_perftest.cc:362: 
Cool, this looks good. But one thing: how about just creating that pile down
here and remove it as a member of the class? it will be more clear what's
happening that way, I think.

[USE eero AT chromium.org, 2014-11-04 16:00:35]

https://codereview.chromium.org/684273003/diff/40001/cc/resources/tile_manage...
File cc/resources/tile_manager_perftest.cc (right):

https://codereview.chromium.org/684273003/diff/40001/cc/resources/tile_manage...
cc/resources/tile_manager_perftest.cc:362: 
On 2014/11/04 15:39:55, danakj wrote:
> Cool, this looks good. But one thing: how about just creating that pile down
> here and remove it as a member of the class? it will be more clear what's
> happening that way, I think.

Done.

[danakj, 2014-11-04 16:05:14]

LGTM

[danakj, 2014-11-04 16:05:17]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-11-04 16:06:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/684273003/60001

[commit-bot: I haz the power, 2014-11-04 18:13:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-11-04 18:14:29]

Patchset 4 (id:??) landed as
https://crrev.com/f0d244108190fd3352db49a27b8b57db7ad9fe13
Cr-Commit-Position: refs/heads/master@{#302627}