Introduce ui::PaintRecorder and use it for CursorWindowDelegate.

Introduce ui::PaintRecorder and use it for CursorWindowDelegate.

This makes the mouse cursor appear in slimming-ui impl-side painting.

In the CursorWindowDelegate, we don't have invalidation, so we don't
expect to be asked to repaint the layer after the first time. For that
reason we don't cache the SkPicture in the delegate. This makes use of
the TakeDisplayItem() helper method.

Conversely in View we intend to cache the SkPicture, so it pulls the
SkPicture itself out of the recorder, with a TODO to cache it in
upcoming changes, and inserts the SkPicture into the DisplayItemList
itself.

R=enne, piman@chromium.org, sadrul, sky
BUG=466426

[danakj, 2015-03-19 20:51:59]

This should be a smaller CL to give some context on API decisions around what
the PaintToDisplayList() method looks like.

The reason it takes a DisplayItemList rather than an SkCanvas, is to allow each
paint implementor to paint into their own SkPicture and store that result.

But for some, there is no need, as in the cursor in this CL. For them the
PaintRecorder can help reduce the amount of code in the paint method.

Critiques/Suggestions welcome :)

[piman, 2015-03-19 20:59:19]

lgtm

https://codereview.chromium.org/1021823002/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1021823002/diff/1/ui/views/view.cc#newcode1484
ui/views/view.cc:1484: // TODO(danakj): Cache this picture in the View and reuse
it.
nit: Is it not appropriate to cache the DrawingDisplayItem? Maybe adding a
Clone() method there... That way, no need to expose the SkPicture.

[danakj, 2015-03-19 21:06:32]

https://codereview.chromium.org/1021823002/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1021823002/diff/1/ui/views/view.cc#newcode1484
ui/views/view.cc:1484: // TODO(danakj): Cache this picture in the View and reuse
it.
On 2015/03/19 20:59:19, piman (Very slow to review) wrote:
> nit: Is it not appropriate to cache the DrawingDisplayItem? Maybe adding a
> Clone() method there... That way, no need to expose the SkPicture.

Thanks for asking, I shall do that. Slimming paint folks agree that storing a
DisplayItem is preferable.

[danakj, 2015-03-19 21:13:45]

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

[danakj, 2015-03-19 21:13:46]

+ajuma for cc/ change.

https://codereview.chromium.org/1021823002/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1021823002/diff/1/ui/views/view.cc#newcode1484
ui/views/view.cc:1484: // TODO(danakj): Cache this picture in the View and reuse
it.
On 2015/03/19 21:06:32, danakj wrote:
> On 2015/03/19 20:59:19, piman (Very slow to review) wrote:
> > nit: Is it not appropriate to cache the DrawingDisplayItem? Maybe adding a
> > Clone() method there... That way, no need to expose the SkPicture.
> 
> Thanks for asking, I shall do that. Slimming paint folks agree that storing a
> DisplayItem is preferable.

Done.

[ajuma, 2015-03-19 21:23:40]

On 2015/03/19 21:13:46, danakj wrote:
> +ajuma for cc/ change.

lgtm

[danakj, 2015-03-19 22:05:16]

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

[oshima, 2015-03-19 22:19:28]

ash lgtm

[sadrul, 2015-03-19 22:51:47]

Is this on top of the other CL?

[danakj, 2015-03-19 22:52:15]

On 2015/03/19 22:51:47, sadrul wrote:
> Is this on top of the other CL?

Yep

[sky, 2015-03-20 00:05:16]

This one is harder to see through though as it doesn't look like you're adding
OnPaintLayerToDisplayList to this patch, but OnPaintLayerToDisplayList hasn't
been added yet.

[danakj, 2015-03-20 01:10:40]

Sorry I can't add the function in just one place and still compile it (pure
virtual) or use it (plumbing thru all the levels of abstraction) :/. This
just shows how I'll be filling the functions in. That's why my other patch
is just changing the api but mostly leaving the functions empty cuz the
point of it is the api.

I'm not sure how to give smaller pieces than what I've done here, would you
prefer nonfunctional nonpure virtual inflterfaces? This is a followup step
after adding the api in the larger cl. There would be many smaller ones to
implement each of the delegates.
This one is harder to see through though as it doesn't look like you're
adding
OnPaintLayerToDisplayList to this patch, but OnPaintLayerToDisplayList
hasn't
been added yet.

https://codereview.chromium.org/1021823002/

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

[sky, 2015-03-20 14:52:45]

https://codereview.chromium.org/1021823002/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1021823002/diff/20001/ui/views/view.cc#newcod...
ui/views/view.cc:1467: void View::OnPaintLayerToDisplayList(cc::DisplayItemList*
list,
I was commenting on this function. This hasn't landed in master yet, which is
why I find this patch a bit confusing.

[danakj, 2015-03-20 15:26:36]

https://codereview.chromium.org/1021823002/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1021823002/diff/20001/ui/views/view.cc#newcod...
ui/views/view.cc:1467: void View::OnPaintLayerToDisplayList(cc::DisplayItemList*
list,
On 2015/03/20 14:52:45, sky wrote:
> I was commenting on this function. This hasn't landed in master yet, which is
> why I find this patch a bit confusing.

Ya this patch depends on the first one to add the functions. I changed this
function here just to demonstrate that both views (caching) and other (non
caching) can both share code by using the paint recorder. 

This shouldn't make the views implementation here any more complete yet.