[webkit-reviews] review granted: [Bug 43682] Add missing String builder for SVGPathParser : [Attachment 63832] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 43682: Add missing String builder for SVGPathParser
https://bugs.webkit.org/show_bug.cgi?id=43682

Attachment 63832: Patch
https://bugs.webkit.org/attachment.cgi?id=63832&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
WebCore/svg/SVGPathParserFactory.cpp:80
 +	s_parser->setCurrentSource(source);
For consistency, add a newline before this statement and remove the one before
return s_parser :-)

WebCore/svg/SVGPathParserFactory.cpp:181
 +	ASSERT(stream);
Ok you misunderstood the suggestion I gave.

The passed in OwnPtr should not be initialized, this is the task of this
function.
The function should be used like this:

OwnPtr<SVGPathByteStream> stream; // stream pointer is null here!
if (!factory->buildSVGPathByteStreamFromString(d, stream, NormalizedParsing))
    return false;
// stream pointer is now set.

That means, the ASSERT(stream) should go away. The function should be used like
this:

bool SVGPathParserFactory::buildSVGPathByteStreamFromString(const String& d,
OwnPtr<SVGPathByteStream>& result, PathParsingMode parsingMode)
{
    if (d.isEmpty())
	return false;

    OwnPtr<SVGPathByteStream> stream = SVGPathByteStream::create();
    SVGPathByteStreamBuilder* builder =
globalSVGPathByteStreamBuilder(stream.get());

    OwnPtr<SVGPathStringSource> source = SVGPathStringSource::create(d);
    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
    bool ok = parser->parsePathDataFromSource(parsingMode);
    parser->cleanup();
    result = stream.release(); // this is the important step
    return ok;
}

r=me, with those fixes.