Add PictureRecorder for sp/non-sp recording without an existing context

Add PictureRecorder for sp/non-sp recording without an existing context

This patch adds a PictureRecorder for recording when there is no
existing graphics context. For slimming paint, this patch does not
change behavior. For non-slimming paint this patch now creates a
graphics context using GC::deprecatedCreateWithCanvas which prevents
crashes seen in https://codereview.chromium.org/1087633002.

BUG=470553
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193923

[pdr., 2015-04-15 20:55:06]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org, wangxianzhu@chromium.org

[Xianzhu, 2015-04-15 22:51:21]

I uploaded a similar change https://codereview.chromium.org/1048223002/ 2 weeks
ago but finally closed it because it seemed not useful at that time.

I'd like to hear chrishtr@'s opinion.

[pdr., 2015-04-15 23:23:36]

On 2015/04/15 at 22:51:21, wangxianzhu wrote:
> I uploaded a similar change https://codereview.chromium.org/1048223002/ 2
weeks ago but finally closed it because it seemed not useful at that time.
> 
> I'd like to hear chrishtr@'s opinion.

Oh wow, I hadn't seen your patch but we independently wrote nearly the exact
same code. I even had the assert boolean but took it out so the patch would be
easier to review (we can catch two calls to endRecording with an existing
assert).

An alternative to this approach is to build this logic into GraphicsContext
itself so that GC::endRecording replays any display items. This is ugly today
because it doesn't map onto the nested recording concept GraphicsContext
currently supports (m_recordingStateStack, etc). We could add a TODO to this
class to merge it back into GraphicsContext.

[chrishtr, 2015-04-16 18:23:11]

pdr is going to change this to be cleaner, per offline discussion.

[pdr., 2015-04-16 23:15:53]

On 2015/04/16 at 18:23:11, chrishtr wrote:
> pdr is going to change this to be cleaner, per offline discussion.

After much thinking we've come to the conclusion that SkPicture is the best
abstraction we have in a mixed sp/non-sp world. I've added a comment to this
class to reflect that, and renamed it SkPictureBuilder. PTAL

[chrishtr, 2015-04-17 00:02:24]

lgtm

https://codereview.chromium.org/1089293002/diff/20001/Source/core/layout/svg/...
File Source/core/layout/svg/LayoutSVGResourcePattern.cpp (right):

https://codereview.chromium.org/1089293002/diff/20001/Source/core/layout/svg/...
Source/core/layout/svg/LayoutSVGResourcePattern.cpp:170: SkPictureBuilder
pictureBuilder(bounds);
Just pass the bounds directly instead of via a local variable?

[pdr., 2015-04-17 00:06:13]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-04-17 00:06:14]

https://codereview.chromium.org/1089293002/diff/20001/Source/core/layout/svg/...
File Source/core/layout/svg/LayoutSVGResourcePattern.cpp (right):

https://codereview.chromium.org/1089293002/diff/20001/Source/core/layout/svg/...
Source/core/layout/svg/LayoutSVGResourcePattern.cpp:170: SkPictureBuilder
pictureBuilder(bounds);
On 2015/04/17 at 00:02:23, chrishtr wrote:
> Just pass the bounds directly instead of via a local variable?

Sadly not an lvalue :'(

[commit-bot: I haz the power, 2015-04-17 00:06:32]

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

[pdr., 2015-04-17 04:30:34]

https://codereview.chromium.org/1089293002/diff/20001/Source/core/layout/svg/...
File Source/core/layout/svg/LayoutSVGResourceMasker.cpp (right):

https://codereview.chromium.org/1089293002/diff/20001/Source/core/layout/svg/...
Source/core/layout/svg/LayoutSVGResourceMasker.cpp:91: m_maskContentPicture =
pictureBuilder.context().endRecording();
This should be pictureBuilder.endRecording().

[pdr., 2015-04-17 04:35:40]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-04-17 04:35:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1089293002/#ps40001
(title: "Fix endRecording mistake")

[commit-bot: I haz the power, 2015-04-17 04:35:46]

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

[commit-bot: I haz the power, 2015-04-17 05:40:00]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193923