Bug 123467 – Manage SVGPathByteStream through std::unique_ptr

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

Manage SVGPathByteStream through std::unique_ptr

[Zan Dobersek]

Created attachment 215426 [details]
Patch

[Anders Carlsson]

Comment on attachment 215426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215426&action=review

Patch looks good but I'd like to see a new iteration that doesn't change the createPath overload to take a raw pointer.

> Source/WebCore/svg/SVGAnimatedPath.cpp:38
> +    std::unique_ptr<SVGPathByteStream> byteStream = std::make_unique<SVGPathByteStream>();

Use auto here.

> Source/WebCore/svg/SVGAnimatedPath.cpp:50
> +    std::unique_ptr<SVGPathByteStream> byteStream = std::make_unique<SVGPathByteStream>();

Use auto here.

> Source/WebCore/svg/SVGAnimatedType.cpp:183
> -PassOwnPtr<SVGAnimatedType> SVGAnimatedType::createPath(PassOwnPtr<SVGPathByteStream> path)
> +PassOwnPtr<SVGAnimatedType> SVGAnimatedType::createPath(SVGPathByteStream* path)

I don't understand this change. This should take a unique_ptr and call .release() on it.

> Source/WebCore/svg/SVGPathByteStream.h:54
> +    std::unique_ptr<SVGPathByteStream> copy()

This can be const.

> Source/WebCore/svg/SVGPathUtilities.cpp:151
> +    std::unique_ptr<SVGPathByteStream> appendedByteStream = std::make_unique<SVGPathByteStream>();

Auto.

> Source/WebCore/svg/SVGPathUtilities.cpp:273
> +    std::unique_ptr<SVGPathByteStream> fromStreamCopy = fromStream->copy();

Auto.

[Zan Dobersek]

Comment on attachment 215426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215426&action=review

>> Source/WebCore/svg/SVGPathByteStream.h:54
>> +    std::unique_ptr<SVGPathByteStream> copy()
> 
> This can be const.

That enforces constness on the m_data member and calls the non-existent SVGPathByteStream(const Data&) constructor, causing compilation failures.

[Zan Dobersek]

Created attachment 215532 [details]
Patch

[Anders Carlsson]

Comment on attachment 215426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215426&action=review

>>> Source/WebCore/svg/SVGPathByteStream.h:54
>>> +    std::unique_ptr<SVGPathByteStream> copy()
>> 
>> This can be const.
> 
> That enforces constness on the m_data member and calls the non-existent SVGPathByteStream(const Data&) constructor, causing compilation failures.

Why does the SVGPathByteStream need to take a non-const reference?

[Zan Dobersek]

Comment on attachment 215426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215426&action=review

>>>> Source/WebCore/svg/SVGPathByteStream.h:54
>>>> +    std::unique_ptr<SVGPathByteStream> copy()
>>> 
>>> This can be const.
>> 
>> That enforces constness on the m_data member and calls the non-existent SVGPathByteStream(const Data&) constructor, causing compilation failures.
> 
> Why does the SVGPathByteStream need to take a non-const reference?

No need actually, the constructor can work with a const reference. copy() can be const as well then.

[Zan Dobersek]

Created attachment 215638 [details]
Patch

[Zan Dobersek]

Comment on attachment 215638 [details]
Patch

Clearing flags on attachment: 215638

Committed r158359: <http://trac.webkit.org/changeset/158359>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.