Use references and more const in SVGPathUtilities
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Oct 2015 06:16:18 +0000 (06:16 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Oct 2015 06:16:18 +0000 (06:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150007

Reviewed by Zalan Bujtas.

SVGPathUtilities had lots of pointers whose non-nullness was asserted,
little const, and was very trigger-happy with stack allocations. Clean
that all up.

* svg/SVGAnimatedPath.cpp:
(WebCore::SVGAnimatedPathAnimator::constructFromString):
(WebCore::SVGAnimatedPathAnimator::resetAnimValToBaseVal):
(WebCore::SVGAnimatedPathAnimator::addAnimatedTypes):
(WebCore::SVGAnimatedPathAnimator::calculateAnimatedValue):
* svg/SVGPathByteStream.h:
(WebCore::SVGPathByteStream::begin):
(WebCore::SVGPathByteStream::end):
(WebCore::SVGPathByteStream::append):
* svg/SVGPathByteStreamSource.cpp:
(WebCore::SVGPathByteStreamSource::SVGPathByteStreamSource):
* svg/SVGPathByteStreamSource.h:
* svg/SVGPathElement.cpp:
(WebCore::SVGPathElement::getTotalLength):
(WebCore::SVGPathElement::getPointAtLength):
(WebCore::SVGPathElement::getPathSegAtLength):
(WebCore::SVGPathElement::parseAttribute):
(WebCore::SVGPathElement::svgAttributeChanged):
(WebCore::SVGPathElement::pathByteStream):
(WebCore::SVGPathElement::lookupOrCreateDWrapper):
(WebCore::SVGPathElement::pathSegListChanged):
(WebCore::SVGPathElement::SVGPathElement): Deleted.
* svg/SVGPathElement.h:
* svg/SVGPathUtilities.cpp:
(WebCore::globalSVGPathBuilder):
(WebCore::globalSVGPathSegListBuilder):
(WebCore::globalSVGPathByteStreamBuilder):
(WebCore::globalSVGPathStringBuilder):
(WebCore::globalSVGPathTraversalStateBuilder):
(WebCore::globalSVGPathParser):
(WebCore::globalSVGPathBlender):
(WebCore::buildPathFromString):
(WebCore::buildSVGPathByteStreamFromSVGPathSegList):
(WebCore::appendSVGPathByteStreamFromSVGPathSeg):
(WebCore::buildPathFromByteStream):
(WebCore::buildSVGPathSegListFromByteStream):
(WebCore::buildStringFromByteStream):
(WebCore::buildStringFromSVGPathSegList):
(WebCore::buildSVGPathByteStreamFromString):
(WebCore::buildAnimatedSVGPathByteStream):
(WebCore::addToSVGPathByteStream):
(WebCore::getSVGPathSegAtLengthFromSVGPathByteStream):
(WebCore::getTotalLengthOfSVGPathByteStream):
(WebCore::getPointAtLengthOfSVGPathByteStream):
(WebCore::buildStringFromPath):
* svg/SVGPathUtilities.h:
* svg/properties/SVGAnimatedPathSegListPropertyTearOff.h:
(WebCore::SVGAnimatedPathSegListPropertyTearOff::animValDidChange):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@190844 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/svg/SVGAnimatedPath.cpp
Source/WebCore/svg/SVGPathByteStream.h
Source/WebCore/svg/SVGPathByteStreamSource.cpp
Source/WebCore/svg/SVGPathByteStreamSource.h
Source/WebCore/svg/SVGPathElement.cpp
Source/WebCore/svg/SVGPathElement.h
Source/WebCore/svg/SVGPathUtilities.cpp
Source/WebCore/svg/SVGPathUtilities.h
Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h

index a01d007..b1baf2a 100644 (file)
@@ -1,3 +1,63 @@
+2015-10-10  Simon Fraser  <simon.fraser@apple.com>
+
+        Use references and more const in SVGPathUtilities
+        https://bugs.webkit.org/show_bug.cgi?id=150007
+
+        Reviewed by Zalan Bujtas.
+        
+        SVGPathUtilities had lots of pointers whose non-nullness was asserted,
+        little const, and was very trigger-happy with stack allocations. Clean
+        that all up.
+
+        * svg/SVGAnimatedPath.cpp:
+        (WebCore::SVGAnimatedPathAnimator::constructFromString):
+        (WebCore::SVGAnimatedPathAnimator::resetAnimValToBaseVal):
+        (WebCore::SVGAnimatedPathAnimator::addAnimatedTypes):
+        (WebCore::SVGAnimatedPathAnimator::calculateAnimatedValue):
+        * svg/SVGPathByteStream.h:
+        (WebCore::SVGPathByteStream::begin):
+        (WebCore::SVGPathByteStream::end):
+        (WebCore::SVGPathByteStream::append):
+        * svg/SVGPathByteStreamSource.cpp:
+        (WebCore::SVGPathByteStreamSource::SVGPathByteStreamSource):
+        * svg/SVGPathByteStreamSource.h:
+        * svg/SVGPathElement.cpp:
+        (WebCore::SVGPathElement::getTotalLength):
+        (WebCore::SVGPathElement::getPointAtLength):
+        (WebCore::SVGPathElement::getPathSegAtLength):
+        (WebCore::SVGPathElement::parseAttribute):
+        (WebCore::SVGPathElement::svgAttributeChanged):
+        (WebCore::SVGPathElement::pathByteStream):
+        (WebCore::SVGPathElement::lookupOrCreateDWrapper):
+        (WebCore::SVGPathElement::pathSegListChanged):
+        (WebCore::SVGPathElement::SVGPathElement): Deleted.
+        * svg/SVGPathElement.h:
+        * svg/SVGPathUtilities.cpp:
+        (WebCore::globalSVGPathBuilder):
+        (WebCore::globalSVGPathSegListBuilder):
+        (WebCore::globalSVGPathByteStreamBuilder):
+        (WebCore::globalSVGPathStringBuilder):
+        (WebCore::globalSVGPathTraversalStateBuilder):
+        (WebCore::globalSVGPathParser):
+        (WebCore::globalSVGPathBlender):
+        (WebCore::buildPathFromString):
+        (WebCore::buildSVGPathByteStreamFromSVGPathSegList):
+        (WebCore::appendSVGPathByteStreamFromSVGPathSeg):
+        (WebCore::buildPathFromByteStream):
+        (WebCore::buildSVGPathSegListFromByteStream):
+        (WebCore::buildStringFromByteStream):
+        (WebCore::buildStringFromSVGPathSegList):
+        (WebCore::buildSVGPathByteStreamFromString):
+        (WebCore::buildAnimatedSVGPathByteStream):
+        (WebCore::addToSVGPathByteStream):
+        (WebCore::getSVGPathSegAtLengthFromSVGPathByteStream):
+        (WebCore::getTotalLengthOfSVGPathByteStream):
+        (WebCore::getPointAtLengthOfSVGPathByteStream):
+        (WebCore::buildStringFromPath):
+        * svg/SVGPathUtilities.h:
+        * svg/properties/SVGAnimatedPathSegListPropertyTearOff.h:
+        (WebCore::SVGAnimatedPathSegListPropertyTearOff::animValDidChange):
+
 2015-10-10  Andreas Kling  <akling@apple.com>
 
         Reduce pointless malloc traffic in ElementRuleCollector.
index 3831fc2..0bf207f 100644 (file)
@@ -34,7 +34,7 @@ SVGAnimatedPathAnimator::SVGAnimatedPathAnimator(SVGAnimationElement* animationE
 std::unique_ptr<SVGAnimatedType> SVGAnimatedPathAnimator::constructFromString(const String& string)
 {
     auto byteStream = std::make_unique<SVGPathByteStream>();
-    buildSVGPathByteStreamFromString(string, byteStream.get(), UnalteredParsing);
+    buildSVGPathByteStreamFromString(string, *byteStream, UnalteredParsing);
     return SVGAnimatedType::createPath(WTF::move(byteStream));
 }
 
@@ -58,7 +58,7 @@ void SVGAnimatedPathAnimator::resetAnimValToBaseVal(const SVGElementAnimatedProp
     SVGAnimatedPathSegListPropertyTearOff* property = castAnimatedPropertyToActualType<SVGAnimatedPathSegListPropertyTearOff>(animatedTypes[0].properties[0].get());
     const SVGPathSegList& baseValue = property->currentBaseValue();
 
-    buildSVGPathByteStreamFromSVGPathSegList(baseValue, byteStream, UnalteredParsing);
+    buildSVGPathByteStreamFromSVGPathSegList(baseValue, *byteStream, UnalteredParsing);
 
     Vector<RefPtr<SVGAnimatedPathSegListPropertyTearOff>> result;
 
@@ -104,7 +104,7 @@ void SVGAnimatedPathAnimator::addAnimatedTypes(SVGAnimatedType* from, SVGAnimate
     unsigned fromPathSize = fromPath->size();
     if (!fromPathSize || fromPathSize != toPath->size())
         return;
-    addToSVGPathByteStream(toPath, fromPath);
+    addToSVGPathByteStream(*toPath, *fromPath);
 }
 
 void SVGAnimatedPathAnimator::calculateAnimatedValue(float percentage, unsigned repeatCount, SVGAnimatedType* from, SVGAnimatedType* to, SVGAnimatedType* toAtEndOfDuration, SVGAnimatedType* animated)
@@ -133,15 +133,15 @@ void SVGAnimatedPathAnimator::calculateAnimatedValue(float percentage, unsigned
     if (!m_animationElement->adjustFromToListValues<SVGPathByteStream>(*fromPath, *toPath, *animatedPath, percentage, false))
         return;
 
-    buildAnimatedSVGPathByteStream(fromPath, toPath, animatedPath, percentage);
+    buildAnimatedSVGPathByteStream(*fromPath, *toPath, *animatedPath, percentage);
 
     // Handle additive='sum'.
     if (lastAnimatedPath)
-        addToSVGPathByteStream(animatedPath, lastAnimatedPath.get());
+        addToSVGPathByteStream(*animatedPath, *lastAnimatedPath);
 
     // Handle accumulate='sum'.
     if (m_animationElement->isAccumulated() && repeatCount)
-        addToSVGPathByteStream(animatedPath, toAtEndOfDurationPath, repeatCount);
+        addToSVGPathByteStream(*animatedPath, *toAtEndOfDurationPath, repeatCount);
 }
 
 float SVGAnimatedPathAnimator::calculateDistance(const String&, const String&)
index a75ab46..a83afef 100644 (file)
@@ -55,10 +55,11 @@ public:
         return std::make_unique<SVGPathByteStream>(m_data);
     }
 
-    DataIterator begin() { return m_data.begin(); }
-    DataIterator end() { return m_data.end(); }
+    DataIterator begin() const { return m_data.begin(); }
+    DataIterator end() const { return m_data.end(); }
+
     void append(unsigned char byte) { m_data.append(byte); }
-    void append(SVGPathByteStream& other)
+    void append(const SVGPathByteStream& other)
     {
         for (auto stream : other)
             append(stream);
index fc1feb6..fa43922 100644 (file)
 
 namespace WebCore {
 
-SVGPathByteStreamSource::SVGPathByteStreamSource(SVGPathByteStream* stream)
+SVGPathByteStreamSource::SVGPathByteStreamSource(const SVGPathByteStream& stream)
 {
-    ASSERT(stream);
-    m_streamCurrent = stream->begin();
-    m_streamEnd = stream->end();
+    m_streamCurrent = stream.begin();
+    m_streamEnd = stream.end();
 }
 
 bool SVGPathByteStreamSource::hasMoreData() const
index edacdc9..b7a6640 100644 (file)
@@ -28,7 +28,7 @@ namespace WebCore {
 
 class SVGPathByteStreamSource : public SVGPathSource {
 public:
-    explicit SVGPathByteStreamSource(SVGPathByteStream*);
+    explicit SVGPathByteStreamSource(const SVGPathByteStream&);
 
 private:
     virtual bool hasMoreData() const override;
index 6e8405d..991b775 100644 (file)
@@ -80,7 +80,6 @@ END_REGISTER_ANIMATED_PROPERTIES
 
 inline SVGPathElement::SVGPathElement(const QualifiedName& tagName, Document& document)
     : SVGGraphicsElement(tagName, document)
-    , m_pathByteStream(std::make_unique<SVGPathByteStream>())
     , m_pathSegList(PathSegUnalteredRole)
     , m_isAnimValObserved(false)
 {
@@ -93,21 +92,21 @@ Ref<SVGPathElement> SVGPathElement::create(const QualifiedName& tagName, Documen
     return adoptRef(*new SVGPathElement(tagName, document));
 }
 
-float SVGPathElement::getTotalLength()
+float SVGPathElement::getTotalLength() const
 {
     float totalLength = 0;
     getTotalLengthOfSVGPathByteStream(pathByteStream(), totalLength);
     return totalLength;
 }
 
-SVGPoint SVGPathElement::getPointAtLength(float length)
+SVGPoint SVGPathElement::getPointAtLength(float length) const
 {
     SVGPoint point;
     getPointAtLengthOfSVGPathByteStream(pathByteStream(), length, point);
     return point;
 }
 
-unsigned SVGPathElement::getPathSegAtLength(float length)
+unsigned SVGPathElement::getPathSegAtLength(float length) const
 {
     unsigned pathSeg = 0;
     getSVGPathSegAtLengthFromSVGPathByteStream(pathByteStream(), length, pathSeg);
@@ -224,7 +223,7 @@ bool SVGPathElement::isSupportedAttribute(const QualifiedName& attrName)
 void SVGPathElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == SVGNames::dAttr) {
-        if (!buildSVGPathByteStreamFromString(value, m_pathByteStream.get(), UnalteredParsing))
+        if (!buildSVGPathByteStreamFromString(value, m_pathByteStream, UnalteredParsing))
             document().accessSVGExtensions().reportError("Problem parsing d=\"" + value + "\"");
         return;
     }
@@ -254,7 +253,7 @@ void SVGPathElement::svgAttributeChanged(const QualifiedName& attrName)
     if (attrName == SVGNames::dAttr) {
         if (m_pathSegList.shouldSynchronize && !SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(this, dPropertyInfo())->isAnimating()) {
             SVGPathSegList newList(PathSegUnalteredRole);
-            buildSVGPathSegListFromByteStream(m_pathByteStream.get(), this, newList, UnalteredParsing);
+            buildSVGPathSegListFromByteStream(m_pathByteStream, *this, newList, UnalteredParsing);
             m_pathSegList.value = newList;
         }
 
@@ -293,12 +292,17 @@ void SVGPathElement::removedFrom(ContainerNode& rootParent)
     invalidateMPathDependencies();
 }
 
-SVGPathByteStream* SVGPathElement::pathByteStream() const
+const SVGPathByteStream& SVGPathElement::pathByteStream() const
 {
     SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(this, dPropertyInfo());
     if (!property || !property->isAnimating())
-        return m_pathByteStream.get();
-    return static_cast<SVGAnimatedPathSegListPropertyTearOff*>(property)->animatedPathByteStream();
+        return m_pathByteStream;
+    
+    SVGPathByteStream* animatedPathByteStream = static_cast<SVGAnimatedPathSegListPropertyTearOff*>(property)->animatedPathByteStream();
+    if (!animatedPathByteStream)
+        return m_pathByteStream;
+
+    return *animatedPathByteStream;
 }
 
 Ref<SVGAnimatedProperty> SVGPathElement::lookupOrCreateDWrapper(SVGElement* contextElement)
@@ -310,7 +314,7 @@ Ref<SVGAnimatedProperty> SVGPathElement::lookupOrCreateDWrapper(SVGElement* cont
         return *property;
 
     // Build initial SVGPathSegList.
-    buildSVGPathSegListFromByteStream(ownerType.m_pathByteStream.get(), &ownerType, ownerType.m_pathSegList.value, UnalteredParsing);
+    buildSVGPathSegListFromByteStream(ownerType.m_pathByteStreamownerType, ownerType.m_pathSegList.value, UnalteredParsing);
 
     return SVGAnimatedProperty::lookupOrCreateWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff, SVGPathSegList>
         (&ownerType, dPropertyInfo(), ownerType.m_pathSegList.value);
@@ -359,9 +363,9 @@ void SVGPathElement::pathSegListChanged(SVGPathSegRole role, ListModification li
     case PathSegUnalteredRole:
         if (listModification == ListModificationAppend) {
             ASSERT(!m_pathSegList.value.isEmpty());
-            appendSVGPathByteStreamFromSVGPathSeg(m_pathSegList.value.last().copyRef(), m_pathByteStream.get(), UnalteredParsing);
+            appendSVGPathByteStreamFromSVGPathSeg(m_pathSegList.value.last().copyRef(), m_pathByteStream, UnalteredParsing);
         } else
-            buildSVGPathByteStreamFromSVGPathSegList(m_pathSegList.value, m_pathByteStream.get(), UnalteredParsing);
+            buildSVGPathByteStreamFromSVGPathSegList(m_pathSegList.value, m_pathByteStream, UnalteredParsing);
         break;
     case PathSegUndefinedRole:
         return;
index a7440ac..acfd38b 100644 (file)
@@ -57,9 +57,9 @@ class SVGPathElement final : public SVGGraphicsElement,
 public:
     static Ref<SVGPathElement> create(const QualifiedName&, Document&);
     
-    float getTotalLength();
-    SVGPoint getPointAtLength(float distance);
-    unsigned getPathSegAtLength(float distance);
+    float getTotalLength() const;
+    SVGPoint getPointAtLength(float distance) const;
+    unsigned getPathSegAtLength(float distance) const;
 
     Ref<SVGPathSegClosePath> createSVGPathSegClosePath(SVGPathSegRole = PathSegUndefinedRole);
     Ref<SVGPathSegMovetoAbs> createSVGPathSegMovetoAbs(float x, float y, SVGPathSegRole = PathSegUndefinedRole);
@@ -87,7 +87,7 @@ public:
     SVGPathSegListPropertyTearOff* normalizedPathSegList();
     SVGPathSegListPropertyTearOff* animatedNormalizedPathSegList();
 
-    SVGPathByteStream* pathByteStream() const;
+    const SVGPathByteStream& pathByteStream() const;
 
     void pathSegListChanged(SVGPathSegRole, ListModification = ListModificationUnknown);
 
@@ -124,7 +124,7 @@ private:
     void invalidateMPathDependencies();
 
 private:
-    std::unique_ptr<SVGPathByteStream> m_pathByteStream;
+    SVGPathByteStream m_pathByteStream;
     mutable SVGSynchronizableAnimatedProperty<SVGPathSegList> m_pathSegList;
     bool m_isAnimValObserved;
 };
index aea5f51..b3cc049 100644 (file)
 
 namespace WebCore {
 
-static SVGPathBuilder* globalSVGPathBuilder(Path& result)
+// FIXME: are all these globals warranted? Why not just make them on the stack?
+static SVGPathBuilder& globalSVGPathBuilder(Path& result)
 {
-    static SVGPathBuilder* s_builder = nullptr;
-    if (!s_builder)
-        s_builder = new SVGPathBuilder;
-
-    s_builder->setCurrentPath(&result);
-    return s_builder;
+    static NeverDestroyed<SVGPathBuilder> s_builder;
+    s_builder.get().setCurrentPath(&result);
+    return s_builder.get();
 }
 
-static SVGPathSegListBuilder* globalSVGPathSegListBuilder(SVGPathElement* element, SVGPathSegRole role, SVGPathSegList& result)
+static SVGPathSegListBuilder& globalSVGPathSegListBuilder(SVGPathElement& element, SVGPathSegRole role, SVGPathSegList& result)
 {
-    static SVGPathSegListBuilder* s_builder = nullptr;
-    if (!s_builder)
-        s_builder = new SVGPathSegListBuilder;
-
-    s_builder->setCurrentSVGPathElement(element);
-    s_builder->setCurrentSVGPathSegList(result);
-    s_builder->setCurrentSVGPathSegRole(role);
-    return s_builder;
+    static NeverDestroyed<SVGPathSegListBuilder> s_builder;
+
+    s_builder.get().setCurrentSVGPathElement(&element);
+    s_builder.get().setCurrentSVGPathSegList(result);
+    s_builder.get().setCurrentSVGPathSegRole(role);
+    return s_builder.get();
 }
 
-static SVGPathByteStreamBuilder* globalSVGPathByteStreamBuilder(SVGPathByteStream* result)
+static SVGPathByteStreamBuilder& globalSVGPathByteStreamBuilder(SVGPathByteStream& result)
 {
-    static SVGPathByteStreamBuilder* s_builder = nullptr;
-    if (!s_builder)
-        s_builder = new SVGPathByteStreamBuilder;
-
-    s_builder->setCurrentByteStream(result);
-    return s_builder;
+    static NeverDestroyed<SVGPathByteStreamBuilder> s_builder;
+    s_builder.get().setCurrentByteStream(&result);
+    return s_builder.get();
 }
 
-static SVGPathStringBuilder* globalSVGPathStringBuilder()
+static SVGPathStringBuilder& globalSVGPathStringBuilder()
 {
-    static SVGPathStringBuilder* s_builder = nullptr;
-    if (!s_builder)
-        s_builder = new SVGPathStringBuilder;
-
-    return s_builder;
+    static NeverDestroyed<SVGPathStringBuilder> s_builder;
+    return s_builder.get();
 }
 
-static SVGPathTraversalStateBuilder* globalSVGPathTraversalStateBuilder(PathTraversalState& traversalState, float length)
+static SVGPathTraversalStateBuilder& globalSVGPathTraversalStateBuilder(PathTraversalState& traversalState, float length)
 {
-    static SVGPathTraversalStateBuilder* s_builder = nullptr;
-    if (!s_builder)
-        s_builder = new SVGPathTraversalStateBuilder;
+    static NeverDestroyed<SVGPathTraversalStateBuilder> s_parser;
 
-    s_builder->setCurrentTraversalState(&traversalState);
-    s_builder->setDesiredLength(length);
-    return s_builder;
+    s_parser.get().setCurrentTraversalState(&traversalState);
+    s_parser.get().setDesiredLength(length);
+    return s_parser.get();
 }
 
-static SVGPathParser* globalSVGPathParser(SVGPathSource* source, SVGPathConsumer* consumer)
+// FIXME: why bother keeping a singleton around? Is it slow to allocate?
+static SVGPathParser& globalSVGPathParser(SVGPathSource& source, SVGPathConsumer& consumer)
 {
-    static SVGPathParser* s_parser = nullptr;
-    if (!s_parser)
-        s_parser = new SVGPathParser;
+    static NeverDestroyed<SVGPathParser> s_parser;
 
-    s_parser->setCurrentSource(source);
-    s_parser->setCurrentConsumer(consumer);
-    return s_parser;
+    s_parser.get().setCurrentSource(&source);
+    s_parser.get().setCurrentConsumer(&consumer);
+    return s_parser.get();
 }
 
-static SVGPathBlender* globalSVGPathBlender()
+static SVGPathBlender& globalSVGPathBlender()
 {
-    static SVGPathBlender* s_blender = nullptr;
-    if (!s_blender)
-        s_blender = new SVGPathBlender;
-
-    return s_blender;
+    static NeverDestroyed<SVGPathBlender> s_blender;
+    return s_blender.get();
 }
 
 bool buildPathFromString(const String& d, Path& result)
@@ -114,96 +98,91 @@ bool buildPathFromString(const String& d, Path& result)
     if (d.isEmpty())
         return true;
 
-    SVGPathBuilder* builder = globalSVGPathBuilder(result);
+    SVGPathBuilder& builder = globalSVGPathBuilder(result);
 
-    auto source = std::make_unique<SVGPathStringSource>(d);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(NormalizedParsing);
-    parser->cleanup();
+    SVGPathStringSource source(d);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(NormalizedParsing);
+    parser.cleanup();
     return ok;
 }
 
-bool buildSVGPathByteStreamFromSVGPathSegList(const SVGPathSegList& list, SVGPathByteStream* result, PathParsingMode parsingMode)
+bool buildSVGPathByteStreamFromSVGPathSegList(const SVGPathSegList& list, SVGPathByteStream& result, PathParsingMode parsingMode)
 {
-    ASSERT(result);
-    result->clear();
+    result.clear();
     if (list.isEmpty())
         return true;
 
-    SVGPathByteStreamBuilder* builder = globalSVGPathByteStreamBuilder(result);
+    SVGPathByteStreamBuilder& builder = globalSVGPathByteStreamBuilder(result);
 
-    auto source = std::make_unique<SVGPathSegListSource>(list);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(parsingMode);
-    parser->cleanup();
+    SVGPathSegListSource source(list);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(parsingMode);
+    parser.cleanup();
     return ok;
 }
 
-bool appendSVGPathByteStreamFromSVGPathSeg(RefPtr<SVGPathSeg>&& pathSeg, SVGPathByteStream* result, PathParsingMode parsingMode)
+bool appendSVGPathByteStreamFromSVGPathSeg(RefPtr<SVGPathSeg>&& pathSeg, SVGPathByteStream& result, PathParsingMode parsingMode)
 {
-    ASSERT(result);
     // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists!
     ASSERT(parsingMode == UnalteredParsing);
 
     SVGPathSegList appendedItemList(PathSegUnalteredRole);
     appendedItemList.append(WTF::move(pathSeg));
-    auto appendedByteStream = std::make_unique<SVGPathByteStream>();
 
-    SVGPathByteStreamBuilder* builder = globalSVGPathByteStreamBuilder(appendedByteStream.get());
-    auto source = std::make_unique<SVGPathSegListSource>(appendedItemList);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(parsingMode, false);
-    parser->cleanup();
+    SVGPathByteStream appendedByteStream;
+    SVGPathByteStreamBuilder& builder = globalSVGPathByteStreamBuilder(appendedByteStream);
+    SVGPathSegListSource source(appendedItemList);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(parsingMode, false);
+    parser.cleanup();
 
     if (ok)
-        result->append(*appendedByteStream);
+        result.append(appendedByteStream);
 
     return ok;
 }
 
-bool buildPathFromByteStream(SVGPathByteStream* stream, Path& result)
+bool buildPathFromByteStream(const SVGPathByteStream& stream, Path& result)
 {
-    ASSERT(stream);
-    if (stream->isEmpty())
+    if (stream.isEmpty())
         return true;
 
-    SVGPathBuilder* builder = globalSVGPathBuilder(result);
+    SVGPathBuilder& builder = globalSVGPathBuilder(result);
 
-    auto source = std::make_unique<SVGPathByteStreamSource>(stream);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(NormalizedParsing);
-    parser->cleanup();
+    SVGPathByteStreamSource source(stream);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(NormalizedParsing);
+    parser.cleanup();
     return ok;
 }
 
-bool buildSVGPathSegListFromByteStream(SVGPathByteStream* stream, SVGPathElement* element, SVGPathSegList& result, PathParsingMode parsingMode)
+bool buildSVGPathSegListFromByteStream(const SVGPathByteStream& stream, SVGPathElement& element, SVGPathSegList& result, PathParsingMode parsingMode)
 {
-    ASSERT(stream);
-    if (stream->isEmpty())
+    if (stream.isEmpty())
         return true;
 
-    SVGPathSegListBuilder* builder = globalSVGPathSegListBuilder(element, parsingMode == NormalizedParsing ? PathSegNormalizedRole : PathSegUnalteredRole, result);
+    SVGPathSegListBuilder& builder = globalSVGPathSegListBuilder(element, parsingMode == NormalizedParsing ? PathSegNormalizedRole : PathSegUnalteredRole, result);
 
-    auto source = std::make_unique<SVGPathByteStreamSource>(stream);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(parsingMode);
-    parser->cleanup();
+    SVGPathByteStreamSource source(stream);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(parsingMode);
+    parser.cleanup();
     return ok;
 }
 
-bool buildStringFromByteStream(SVGPathByteStream* stream, String& result, PathParsingMode parsingMode)
+bool buildStringFromByteStream(const SVGPathByteStream& stream, String& result, PathParsingMode parsingMode)
 {
-    ASSERT(stream);
-    if (stream->isEmpty())
+    if (stream.isEmpty())
         return true;
 
-    SVGPathStringBuilder* builder = globalSVGPathStringBuilder();
+    SVGPathStringBuilder& builder = globalSVGPathStringBuilder();
 
-    auto source = std::make_unique<SVGPathByteStreamSource>(stream);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(parsingMode);
-    result = builder->result();
-    parser->cleanup();
+    SVGPathByteStreamSource source(stream);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(parsingMode);
+    result = builder.result();
+    parser.cleanup();
     return ok;
 }
 
@@ -213,121 +192,113 @@ bool buildStringFromSVGPathSegList(const SVGPathSegList& list, String& result, P
     if (list.isEmpty())
         return true;
 
-    SVGPathStringBuilder* builder = globalSVGPathStringBuilder();
+    SVGPathStringBuilder& builder = globalSVGPathStringBuilder();
 
-    auto source = std::make_unique<SVGPathSegListSource>(list);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(parsingMode);
-    result = builder->result();
-    parser->cleanup();
+    SVGPathSegListSource source(list);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(parsingMode);
+    result = builder.result();
+    parser.cleanup();
     return ok;
 }
 
-bool buildSVGPathByteStreamFromString(const String& d, SVGPathByteStream* result, PathParsingMode parsingMode)
+bool buildSVGPathByteStreamFromString(const String& d, SVGPathByteStream& result, PathParsingMode parsingMode)
 {
-    ASSERT(result);
-    result->clear();
+    result.clear();
     if (d.isEmpty())
         return true;
 
-    SVGPathByteStreamBuilder* builder = globalSVGPathByteStreamBuilder(result);
+    SVGPathByteStreamBuilder& builder = globalSVGPathByteStreamBuilder(result);
 
-    auto source = std::make_unique<SVGPathStringSource>(d);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(parsingMode);
-    parser->cleanup();
+    SVGPathStringSource source(d);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(parsingMode);
+    parser.cleanup();
     return ok;
 }
 
-bool buildAnimatedSVGPathByteStream(SVGPathByteStream* fromStream, SVGPathByteStream* toStream, SVGPathByteStream* result, float progress)
+bool buildAnimatedSVGPathByteStream(const SVGPathByteStream& fromStream, const SVGPathByteStream& toStream, SVGPathByteStream& result, float progress)
 {
-    ASSERT(fromStream);
-    ASSERT(toStream);
-    ASSERT(result);
-    ASSERT(toStream != result);
-
-    result->clear();
-    if (toStream->isEmpty())
+    ASSERT(&toStream != &result);
+    result.clear();
+    if (toStream.isEmpty())
         return true;
 
-    SVGPathByteStreamBuilder* builder = globalSVGPathByteStreamBuilder(result);
+    SVGPathByteStreamBuilder& builder = globalSVGPathByteStreamBuilder(result);
 
-    auto fromSource = std::make_unique<SVGPathByteStreamSource>(fromStream);
-    auto toSource = std::make_unique<SVGPathByteStreamSource>(toStream);
-    SVGPathBlender* blender = globalSVGPathBlender();
-    bool ok = blender->blendAnimatedPath(progress, fromSource.get(), toSource.get(), builder);
-    blender->cleanup();
+    SVGPathByteStreamSource fromSource(fromStream);
+    SVGPathByteStreamSource toSource(toStream);
+    SVGPathBlender& blender = globalSVGPathBlender();
+    bool ok = blender.blendAnimatedPath(progress, &fromSource, &toSource, &builder);
+    blender.cleanup();
     return ok;
 }
 
-bool addToSVGPathByteStream(SVGPathByteStream* fromStream, SVGPathByteStream* byStream, unsigned repeatCount)
+bool addToSVGPathByteStream(SVGPathByteStream& streamToAppendTo, const SVGPathByteStream& byStream, unsigned repeatCount)
 {
-    ASSERT(fromStream);
-    ASSERT(byStream);
-    if (fromStream->isEmpty() || byStream->isEmpty())
+    // Why return when streamToAppendTo is empty? Don't we still need to append?
+    if (streamToAppendTo.isEmpty() || byStream.isEmpty())
         return true;
 
-    SVGPathByteStreamBuilder* builder = globalSVGPathByteStreamBuilder(fromStream);
+    // Is it OK to make the SVGPathByteStreamBuilder from a stream, and then clear that stream?
+    SVGPathByteStreamBuilder& builder = globalSVGPathByteStreamBuilder(streamToAppendTo);
 
-    auto fromStreamCopy = fromStream->copy();
-    fromStream->clear();
+    SVGPathByteStream fromStreamCopy = streamToAppendTo;
+    streamToAppendTo.clear();
 
-    auto fromSource = std::make_unique<SVGPathByteStreamSource>(fromStreamCopy.get());
-    auto bySource = std::make_unique<SVGPathByteStreamSource>(byStream);
-    SVGPathBlender* blender = globalSVGPathBlender();
-    bool ok = blender->addAnimatedPath(fromSource.get(), bySource.get(), builder, repeatCount);
-    blender->cleanup();
+    SVGPathByteStreamSource fromSource(fromStreamCopy);
+    SVGPathByteStreamSource bySource(byStream);
+    SVGPathBlender& blender = globalSVGPathBlender();
+    bool ok = blender.addAnimatedPath(&fromSource, &bySource, &builder, repeatCount);
+    blender.cleanup();
     return ok;
 }
 
-bool getSVGPathSegAtLengthFromSVGPathByteStream(SVGPathByteStream* stream, float length, unsigned& pathSeg)
+bool getSVGPathSegAtLengthFromSVGPathByteStream(const SVGPathByteStream& stream, float length, unsigned& pathSeg)
 {
-    ASSERT(stream);
-    if (stream->isEmpty())
+    if (stream.isEmpty())
         return false;
 
     PathTraversalState traversalState(PathTraversalState::Action::SegmentAtLength);
-    SVGPathTraversalStateBuilder* builder = globalSVGPathTraversalStateBuilder(traversalState, length);
+    SVGPathTraversalStateBuilder& builder = globalSVGPathTraversalStateBuilder(traversalState, length);
 
-    auto source = std::make_unique<SVGPathByteStreamSource>(stream);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(NormalizedParsing);
-    pathSeg = builder->pathSegmentIndex();
-    parser->cleanup();
+    SVGPathByteStreamSource source(stream);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(NormalizedParsing);
+    pathSeg = builder.pathSegmentIndex();
+    parser.cleanup();
     return ok;
 }
 
-bool getTotalLengthOfSVGPathByteStream(SVGPathByteStream* stream, float& totalLength)
+bool getTotalLengthOfSVGPathByteStream(const SVGPathByteStream& stream, float& totalLength)
 {
-    ASSERT(stream);
-    if (stream->isEmpty())
+    if (stream.isEmpty())
         return false;
 
     PathTraversalState traversalState(PathTraversalState::Action::TotalLength);
-    SVGPathTraversalStateBuilder* builder = globalSVGPathTraversalStateBuilder(traversalState, 0);
+    SVGPathTraversalStateBuilder& builder = globalSVGPathTraversalStateBuilder(traversalState, 0);
 
-    auto source = std::make_unique<SVGPathByteStreamSource>(stream);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(NormalizedParsing);
-    totalLength = builder->totalLength();
-    parser->cleanup();
+    SVGPathByteStreamSource source(stream);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(NormalizedParsing);
+    totalLength = builder.totalLength();
+    parser.cleanup();
     return ok;
 }
 
-bool getPointAtLengthOfSVGPathByteStream(SVGPathByteStream* stream, float length, SVGPoint& point)
+bool getPointAtLengthOfSVGPathByteStream(const SVGPathByteStream& stream, float length, SVGPoint& point)
 {
-    ASSERT(stream);
-    if (stream->isEmpty())
+    if (stream.isEmpty())
         return false;
 
     PathTraversalState traversalState(PathTraversalState::Action::VectorAtLength);
-    SVGPathTraversalStateBuilder* builder = globalSVGPathTraversalStateBuilder(traversalState, length);
+    SVGPathTraversalStateBuilder& builder = globalSVGPathTraversalStateBuilder(traversalState, length);
 
-    auto source = std::make_unique<SVGPathByteStreamSource>(stream);
-    SVGPathParser* parser = globalSVGPathParser(source.get(), builder);
-    bool ok = parser->parsePathDataFromSource(NormalizedParsing);
-    point = builder->currentPoint();
-    parser->cleanup();
+    SVGPathByteStreamSource source(stream);
+    SVGPathParser& parser = globalSVGPathParser(source, builder);
+    bool ok = parser.parsePathDataFromSource(NormalizedParsing);
+    point = builder.currentPoint();
+    parser.cleanup();
     return ok;
 }
 
@@ -361,12 +332,12 @@ bool buildStringFromPath(const Path& path, String& string)
     // Ideally we would have a SVGPathPlatformPathSource, but it's not possible to manually iterate
     // a path, only apply a function to all path elements at once.
 
-    SVGPathStringBuilder* builder = globalSVGPathStringBuilder();
-    path.apply([builder](const PathElement& pathElement) {
-        pathIteratorForBuildingString(*builder, pathElement);
+    SVGPathStringBuilder& builder = globalSVGPathStringBuilder();
+    path.apply([&builder](const PathElement& pathElement) {
+        pathIteratorForBuildingString(builder, pathElement);
     });
-    string = builder->result();
-    static_cast<SVGPathConsumer*>(builder)->cleanup();
+    string = builder.result();
+    static_cast<SVGPathConsumer&>(builder).cleanup(); // Wat?
 
     return true;
 }
index c2d1f14..f7aabdd 100644 (file)
@@ -34,26 +34,26 @@ class SVGPathSegList;
 
 // String/SVGPathByteStream -> Path
 bool buildPathFromString(const String&, Path&);
-bool buildPathFromByteStream(SVGPathByteStream*, Path&);
+bool buildPathFromByteStream(const SVGPathByteStream&, Path&);
 
 // SVGPathSegList/String -> SVGPathByteStream
-bool buildSVGPathByteStreamFromSVGPathSegList(const SVGPathSegList&, SVGPathByteStream*, PathParsingMode);
-bool appendSVGPathByteStreamFromSVGPathSeg(RefPtr<SVGPathSeg>&&, SVGPathByteStream*, PathParsingMode);
-bool buildSVGPathByteStreamFromString(const String&, SVGPathByteStream*, PathParsingMode);
+bool buildSVGPathByteStreamFromSVGPathSegList(const SVGPathSegList&, SVGPathByteStream& result, PathParsingMode);
+bool appendSVGPathByteStreamFromSVGPathSeg(RefPtr<SVGPathSeg>&&, SVGPathByteStream&, PathParsingMode);
+bool buildSVGPathByteStreamFromString(const String&, SVGPathByteStream&, PathParsingMode);
 
 // SVGPathByteStream/SVGPathSegList -> String
-bool buildStringFromByteStream(SVGPathByteStream*, String&, PathParsingMode);
+bool buildStringFromByteStream(const SVGPathByteStream&, String&, PathParsingMode);
 bool buildStringFromSVGPathSegList(const SVGPathSegList&, String&, PathParsingMode);
 
 // SVGPathByteStream -> SVGPathSegList
-bool buildSVGPathSegListFromByteStream(SVGPathByteStream*, SVGPathElement*, SVGPathSegList&, PathParsingMode);
+bool buildSVGPathSegListFromByteStream(const SVGPathByteStream&, SVGPathElement&, SVGPathSegList&, PathParsingMode);
 
-bool buildAnimatedSVGPathByteStream(SVGPathByteStream*, SVGPathByteStream*, SVGPathByteStream*, float);
-bool addToSVGPathByteStream(SVGPathByteStream*, SVGPathByteStream*, unsigned repeatCount = 1);
+bool buildAnimatedSVGPathByteStream(const SVGPathByteStream& from, const SVGPathByteStream& to, SVGPathByteStream& result, float progress);
+bool addToSVGPathByteStream(SVGPathByteStream& streamToAppendTo, const SVGPathByteStream& from, unsigned repeatCount = 1);
 
-bool getSVGPathSegAtLengthFromSVGPathByteStream(SVGPathByteStream*, float length, unsigned& pathSeg);
-bool getTotalLengthOfSVGPathByteStream(SVGPathByteStream*, float& totalLength);
-bool getPointAtLengthOfSVGPathByteStream(SVGPathByteStream*, float length, SVGPoint&);
+bool getSVGPathSegAtLengthFromSVGPathByteStream(const SVGPathByteStream&, float length, unsigned& pathSeg);
+bool getTotalLengthOfSVGPathByteStream(const SVGPathByteStream&, float& totalLength);
+bool getPointAtLengthOfSVGPathByteStream(const SVGPathByteStream&, float length, SVGPoint&);
 
 // Path -> String
 WEBCORE_EXPORT bool buildStringFromPath(const Path&, String&);
index c785d63..ff09b27 100644 (file)
@@ -95,7 +95,7 @@ public:
         if (pathElement->isAnimValObserved()) {
             SVGPathSegList& animatedList = currentAnimatedValue();
             animatedList.clear();
-            buildSVGPathSegListFromByteStream(m_animatedPathByteStream, pathElement, animatedList, UnalteredParsing);
+            buildSVGPathSegListFromByteStream(*m_animatedPathByteStream, *pathElement, animatedList, UnalteredParsing);
         }
 
         SVGAnimatedListPropertyTearOff<SVGPathSegList>::animValDidChange();