cc: Make DisplayItemList::Append replay into an SkPicture

cc: Make DisplayItemList::Append replay into an SkPicture

This makes DisplayItemList no longer retain individual display
items when caching into a picture is enabled, except when
tracing is also enabled (so that per-item trace output can
still be produced). Instead, only an SkPicture representation
is retained. This avoids the memory cost of storing both the
SkPicture representation (which is needed for raster performance)
as well as the individual display items.

BUG=476073
Committed: https://crrev.com/ec7c07e57bc5f91e3ba0bdeba8b6f534978615b0
Cr-Commit-Position: refs/heads/master@{#325292}

[ajuma, 2015-04-14 18:00:48]

ajuma@chromium.org changed reviewers:
+ enne@chromium.org

[ajuma, 2015-04-14 18:02:33]

https://codereview.chromium.org/1077033004/diff/40001/cc/blink/web_compositor...
File cc/blink/web_compositor_support_impl.h (left):

https://codereview.chromium.org/1077033004/diff/40001/cc/blink/web_compositor...
cc/blink/web_compositor_support_impl.h:45: virtual blink::WebDisplayItemList*
createDisplayItemList();
This has already been removed from WebCompositorSupport in Blink.

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
File cc/resources/display_item_list.cc (right):

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
cc/resources/display_item_list.cc:40: &tracing_enabled);
If tracing is enabled after the DisplayItemList is created, we can't produce
per-item trace output (since we don't have the items), but we still output the
cached SkPicture, so layer contents is still rendered in frame viewer.

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
File cc/resources/display_item_list_unittest.cc (left):

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
cc/resources/display_item_list_unittest.cc:225: TEST(DisplayItemListTest,
CompactingItems) {
This test compares rastering with and without a cached SkPicture, but rastering
without caching is no longer an option.

[danakj, 2015-04-14 20:49:34]

danakj@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2015-04-14 20:49:35]

https://codereview.chromium.org/1077033004/diff/40001/ui/compositor/layer.cc
File ui/compositor/layer.cc (right):

https://codereview.chromium.org/1077033004/diff/40001/ui/compositor/layer.cc#...
ui/compositor/layer.cc:758: scoped_refptr<cc::DisplayItemList> list =
cc::DisplayItemList::Create(clip);
This is weird. Maybe cc should make the DisplayItemList with the clip and give
it to PaintContentsToDisplayList? Is there a reason why this returns a refptr,
does it require the ability to decide to reuse or make a new DisplayItemList?

[ajuma, 2015-04-14 21:51:27]

https://codereview.chromium.org/1077033004/diff/40001/ui/compositor/layer.cc
File ui/compositor/layer.cc (right):

https://codereview.chromium.org/1077033004/diff/40001/ui/compositor/layer.cc#...
ui/compositor/layer.cc:758: scoped_refptr<cc::DisplayItemList> list =
cc::DisplayItemList::Create(clip);
On 2015/04/14 20:49:34, danakj (away until april 19) wrote:
> This is weird. Maybe cc should make the DisplayItemList with the clip and give
> it to PaintContentsToDisplayList?

Done. This approach seems a lot cleaner.

[enne (OOO), 2015-04-14 22:55:35]

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
File cc/resources/display_item_list_unittest.cc (left):

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
cc/resources/display_item_list_unittest.cc:225: TEST(DisplayItemListTest,
CompactingItems) {
On 2015/04/14 18:02:32, ajuma wrote:
> This test compares rastering with and without a cached SkPicture, but
rastering
> without caching is no longer an option.

Is rastering without caching a desirable option in the v1 short-term? This seems
like something the browser would want to do, as generating a picture for an
r-tree seems like pure overhead.

[ajuma, 2015-04-15 15:38:46]

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
File cc/resources/display_item_list_unittest.cc (left):

https://codereview.chromium.org/1077033004/diff/40001/cc/resources/display_it...
cc/resources/display_item_list_unittest.cc:225: TEST(DisplayItemListTest,
CompactingItems) {
On 2015/04/14 22:55:35, enne wrote:
> On 2015/04/14 18:02:32, ajuma wrote:
> > This test compares rastering with and without a cached SkPicture, but
> rastering
> > without caching is no longer an option.
> 
> Is rastering without caching a desirable option in the v1 short-term? This
seems
> like something the browser would want to do, as generating a picture for an
> r-tree seems like pure overhead.

Good point, I've updated DisplayItemList so that caching is optional.

[enne (OOO), 2015-04-15 16:43:19]

lgtm, if you add a PaintContentsToCachedDisplayList test

https://codereview.chromium.org/1077033004/diff/100001/cc/layers/picture_imag...
File cc/layers/picture_image_layer_unittest.cc (right):

https://codereview.chromium.org/1077033004/diff/100001/cc/layers/picture_imag...
cc/layers/picture_image_layer_unittest.cc:36: bool use_cached_picture = false;
Maybe this test should test both cached and not cached?

[ajuma, 2015-04-15 17:24:36]

https://codereview.chromium.org/1077033004/diff/100001/cc/layers/picture_imag...
File cc/layers/picture_image_layer_unittest.cc (right):

https://codereview.chromium.org/1077033004/diff/100001/cc/layers/picture_imag...
cc/layers/picture_image_layer_unittest.cc:36: bool use_cached_picture = false;
On 2015/04/15 16:43:19, enne wrote:
> Maybe this test should test both cached and not cached?

Done.

[ajuma, 2015-04-15 17:27:58]

The CQ bit was checked by ajuma@chromium.org

[ajuma, 2015-04-15 17:27:59]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/1077033004/#ps120001
(title: "Add a PaintContentsToCachedDisplayList test")

[ajuma, 2015-04-15 17:28:06]

The CQ bit was unchecked by ajuma@chromium.org

[ajuma, 2015-04-15 17:29:50]

ajuma@chromium.org changed reviewers:
+ vollick@chromium.org

[ajuma, 2015-04-15 17:30:39]

+vollick for ui/compositor

[Ian Vollick, 2015-04-15 19:11:38]

On 2015/04/15 17:30:39, ajuma wrote:
> +vollick for ui/compositor

lgtm

[Ian Vollick, 2015-04-15 19:11:42]

On 2015/04/15 17:30:39, ajuma wrote:
> +vollick for ui/compositor

lgtm

[ajuma, 2015-04-15 19:12:21]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2015-04-15 19:13:31]

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

[commit-bot: I haz the power, 2015-04-15 20:21:36]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-15 20:22:27]

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