Remove globalSVGPath* from SVGPathUtilities.cpp

Remove globalSVGPath* from SVGPathUtilities.cpp

All the globalSVGPath* globals, don't really help much since all the state
they carry are transient and not particularly expensive to recreate.
Doing this also allows us to get rid of the cleanup() method on the various
classes involved, since this is now handled by the regular scoping rules.
(In most/many cases this means that cleanup isn't necessary anymore,
because the fields being "cleaned up" previously were types without
destructors). Inline a few of the constructors to benefit more from this.
Take this opportunity to make these classes more RAII-like, and replace
pointers with references.

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

[fs, 2014-11-24 16:23:58]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, kouhei@chromium.org, pdr@chromium.org

[fs, 2014-11-24 16:23:59]

This ballooned a bit, but I think it should be reasonably easy to review anyway
- it's all trivial stuff (even SVGPathUtilities.cpp which ended up with a
strange-looking diff...)

[f(malita), 2014-11-24 16:56:57]

Nice cleanup, LGTM!

(+some follow-up ideas)

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathBlend...
File Source/core/svg/SVGPathBlender.h (right):

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathBlend...
Source/core/svg/SVGPathBlender.h:38: SVGPathBlender(SVGPathSource* fromSource,
SVGPathSource* toSource, SVGPathConsumer*);
I think the sources should be const&, but AFAICT the current interface doesn't
allow it. We can look at cleaning this up later.

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
File Source/core/svg/SVGPathUtilities.cpp (right):

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
Source/core/svg/SVGPathUtilities.cpp:45: OwnPtrWillBeRawPtr<SVGPathStringSource>
source = SVGPathStringSource::create(d);
We don't have to address it in this CL, but is there any reason for this to be
heap-allocated? Can we expose the SVGPathStringSource ctor and just allocate it
on stack?

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
Source/core/svg/SVGPathUtilities.cpp:84: OwnPtrWillBeRawPtr<SVGPathStringSource>
source = SVGPathStringSource::create(d);
Same follow-up idea: local ownptr -> stack object?

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
Source/core/svg/SVGPathUtilities.cpp:106: bool
getSVGPathSegAtLengthFromSVGPathByteStream(const SVGPathByteStream& stream,
float length, unsigned& pathSeg)
Moar followup ideas: we don't even use the returned bool -> just return the
index instead of using an out-param?

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
Source/core/svg/SVGPathUtilities.cpp:120: bool
getTotalLengthOfSVGPathByteStream(const SVGPathByteStream& stream, float&
totalLength)
Ditto.

https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
Source/core/svg/SVGPathUtilities.cpp:134: bool
getPointAtLengthOfSVGPathByteStream(const SVGPathByteStream& stream, float
length, FloatPoint& point)
Ditto.

[fs, 2014-11-24 18:46:04]

On 2014/11/24 16:56:57, Florin Malita wrote:
> Nice cleanup, LGTM!
> 
> (+some follow-up ideas)
> 
>
https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathBlend...
> File Source/core/svg/SVGPathBlender.h (right):
> 
>
https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathBlend...
> Source/core/svg/SVGPathBlender.h:38: SVGPathBlender(SVGPathSource* fromSource,
> SVGPathSource* toSource, SVGPathConsumer*);
> I think the sources should be const&, but AFAICT the current interface doesn't
> allow it. We can look at cleaning this up later.

Yes, had this initially, but did feel like dealing with the fallout in this CL.
Will see what it amounts to.
 
>
https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
> File Source/core/svg/SVGPathUtilities.cpp (right):
> 
>
https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
> Source/core/svg/SVGPathUtilities.cpp:45:
OwnPtrWillBeRawPtr<SVGPathStringSource>
> source = SVGPathStringSource::create(d);
> We don't have to address it in this CL, but is there any reason for this to be
> heap-allocated? Can we expose the SVGPathStringSource ctor and just allocate
it
> on stack?

Haven't looked into this, but I don't see any obvious reason for the
heap-allocation either.

>
https://codereview.chromium.org/734053005/diff/1/Source/core/svg/SVGPathUtili...
> Source/core/svg/SVGPathUtilities.cpp:106: bool
> getSVGPathSegAtLengthFromSVGPathByteStream(const SVGPathByteStream& stream,
> float length, unsigned& pathSeg)
> Moar followup ideas: we don't even use the returned bool -> just return the
> index instead of using an out-param?

(And other value returns.) Yeah, makes sense.

[fs, 2014-11-24 18:46:16]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-11-24 18:47:15]

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

[commit-bot: I haz the power, 2014-11-24 19:25:24]

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