Report viewport memory and timing correctly for Slimming Paint.

Report viewport memory and timing correctly for Slimming Paint.

The rasterize_and_record benchmark was not using data from the
DisplayItemList. Now it does.

R=ajuma@chromium.org,vmpstr@chromium.org
BUG=451448
Committed: https://crrev.com/c76f528131949d7fe60b7facf1e7b40362aacfd5
Cr-Commit-Position: refs/heads/master@{#313420}

[Stephen Chennney, 2015-01-23 19:50:22]

Seems this is the best way to get correct viewport stats for Slimming Paint. I'm
open to suggestions.

[vmpstr, 2015-01-23 20:03:06]

There does seem to be a lot of code duplication, but I'm not sure how to solve
it short of a template function that takes Picture or DisplayList and also some
sort of a lambda that creates the object. That might be too convoluted though.

I'm OK with this change, so LGTM assuming ajuma is ok with it as well. What do
the numbers look like in comparison to picture recording?

https://codereview.chromium.org/867803003/diff/1/cc/debug/rasterize_and_recor...
File cc/debug/rasterize_and_record_benchmark.cc (right):

https://codereview.chromium.org/867803003/diff/1/cc/debug/rasterize_and_recor...
cc/debug/rasterize_and_record_benchmark.cc:176: gfx::Rect visible_content_rect =
gfx::ScaleToEnclosingRect(
nit: Can you make this calculation a separate function, so that we can be sure
we're always measuring the same visible_content_rect (with and without display
lists). I'm worried that it's too easy to change one right now and forget about
the other. Either that or take visible_content_rect as a parameter and move the
call to this function lower.

[ajuma, 2015-01-23 20:10:19]

https://codereview.chromium.org/867803003/diff/1/cc/debug/rasterize_and_recor...
File cc/debug/rasterize_and_record_benchmark.cc (right):

https://codereview.chromium.org/867803003/diff/1/cc/debug/rasterize_and_recor...
cc/debug/rasterize_and_record_benchmark.cc:113: RunOnDisplayListLayer(layer);
Perhaps also define a RunOnPicturePileLayer for the logic that's currently below
in RunOnLayer, and call that when use_display_lists is false?

https://codereview.chromium.org/867803003/diff/1/cc/debug/rasterize_and_recor...
cc/debug/rasterize_and_record_benchmark.cc:206: display_list =
painter->PaintContentsToDisplayList(
Will this actually cause the display list to be re-created every time in Blink?
(Is this where we need the "flush display item caches" method you mentioned in
the bug?)

[Stephen Chennney, 2015-01-27 22:46:07]

Patchset #2 (id:20001) has been deleted

[Stephen Chennney, 2015-01-27 22:51:57]

I think this is somewhat better than before.

[ajuma, 2015-01-27 22:55:56]

On 2015/01/27 22:51:57, Stephen Chenney wrote:
> I think this is somewhat better than before.

Thanks, lgtm.

[vmpstr, 2015-01-27 22:57:33]

lgtm with nits

https://codereview.chromium.org/867803003/diff/40001/cc/debug/rasterize_and_r...
File cc/debug/rasterize_and_record_benchmark.cc (right):

https://codereview.chromium.org/867803003/diff/40001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark.cc:187: const int kTimeLimitMillis = 1;
nit: You can move this to anonymous namespace

https://codereview.chromium.org/867803003/diff/40001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark.cc:237: total_best_time[mode_index] =
base::TimeDelta();
nit: I don't think this is required?

[Stephen Chennney, 2015-01-28 00:24:08]

https://codereview.chromium.org/867803003/diff/40001/cc/debug/rasterize_and_r...
File cc/debug/rasterize_and_record_benchmark.cc (right):

https://codereview.chromium.org/867803003/diff/40001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark.cc:187: const int kTimeLimitMillis = 1;
On 2015/01/27 22:57:32, vmpstr wrote:
> nit: You can move this to anonymous namespace

Done.

https://codereview.chromium.org/867803003/diff/40001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark.cc:237: total_best_time[mode_index] =
base::TimeDelta();
On 2015/01/27 22:57:32, vmpstr wrote:
> nit: I don't think this is required?

Done, and verified that you still see 0 in output.

[Stephen Chennney, 2015-01-28 00:24:13]

The CQ bit was checked by schenney@chromium.org

[commit-bot: I haz the power, 2015-01-28 00:24:52]

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

[commit-bot: I haz the power, 2015-01-28 04:02:45]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-01-28 04:04:37]

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

[oystein (OOO til 10th of July), 2015-02-03 00:36:13]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/885753004/ by oysteine@chromium.org.

The reason for reverting is: Reverting due to https://crbug.com/453131.