Let SVGElements have pending resources.

https://bugs.webkit.org/show_bug.cgi?id=99694

Reviewed by Eric Seidel.

Our SVG pending resource tracking is used for handling dynamic id changes. For example,
if an SVG element references an id that is not yet in the document (or has been removed),
the SVG element will be 'pending' an id. When styled elements are inserted into
the document, buildPendingResourcesIfNeeded() is called to force any pending elements
to resolve their dependencies. Only SVGStyledElement targets can be referenced using
this infrastructure, and that is not changed with this patch.

Previously, only SVGStyledElements could have pending resources. Some examples of where
this is violated are SVGAnimateElement and SVGMPathElement which are not a styled elements
but which can have pending references (they can reference styled elements and
paths, respectively). This patch changes the pending resource handling to allow
any SVGElement to have pending resources.

This patch is only a refactoring of code in preparation for WK99694 and does not
affect existing functionality or tests.

* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::addPendingResource):
(WebCore::SVGDocumentExtensions::isElementPendingResources):
(WebCore::SVGDocumentExtensions::isElementPendingResource):
(WebCore::SVGDocumentExtensions::removeElementFromPendingResources):
(WebCore::SVGDocumentExtensions::removeElementFromPendingResourcesForRemoval):
* svg/SVGDocumentExtensions.h:
(WebCore):
(SVGDocumentExtensions):
* svg/SVGElement.cpp:
(WebCore::SVGElement::~SVGElement):
(WebCore::SVGElement::removedFrom):
(WebCore::SVGElement::hasPendingResources):
(WebCore):
(WebCore::SVGElement::setHasPendingResources):
(WebCore::SVGElement::clearHasPendingResourcesIfPossible):
* svg/SVGElement.h:
(SVGElement):
(WebCore::SVGElement::buildPendingResource):
* svg/SVGStyledElement.cpp:
(WebCore):
(WebCore::SVGStyledElement::buildPendingResourcesIfNeeded):
(WebCore::SVGStyledElement::removedFrom):
* svg/SVGStyledElement.h:
(SVGStyledElement):
(WebCore::SVGStyledElement::selfHasRelativeLengths):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index c97dcbb..a0fcba1 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,53 @@
+2012-10-29  Philip Rogers  <pdr@google.com>
+
+        Let SVGElements have pending resources.
+        https://bugs.webkit.org/show_bug.cgi?id=99694
+
+        Reviewed by Eric Seidel.
+
+        Our SVG pending resource tracking is used for handling dynamic id changes. For example,
+        if an SVG element references an id that is not yet in the document (or has been removed),
+        the SVG element will be 'pending' an id. When styled elements are inserted into
+        the document, buildPendingResourcesIfNeeded() is called to force any pending elements
+        to resolve their dependencies. Only SVGStyledElement targets can be referenced using
+        this infrastructure, and that is not changed with this patch.
+
+        Previously, only SVGStyledElements could have pending resources. Some examples of where
+        this is violated are SVGAnimateElement and SVGMPathElement which are not a styled elements
+        but which can have pending references (they can reference styled elements and
+        paths, respectively). This patch changes the pending resource handling to allow
+        any SVGElement to have pending resources.
+
+        This patch is only a refactoring of code in preparation for WK99694 and does not
+        affect existing functionality or tests.
+
+        * svg/SVGDocumentExtensions.cpp:
+        (WebCore::SVGDocumentExtensions::addPendingResource):
+        (WebCore::SVGDocumentExtensions::isElementPendingResources):
+        (WebCore::SVGDocumentExtensions::isElementPendingResource):
+        (WebCore::SVGDocumentExtensions::removeElementFromPendingResources):
+        (WebCore::SVGDocumentExtensions::removeElementFromPendingResourcesForRemoval):
+        * svg/SVGDocumentExtensions.h:
+        (WebCore):
+        (SVGDocumentExtensions):
+        * svg/SVGElement.cpp:
+        (WebCore::SVGElement::~SVGElement):
+        (WebCore::SVGElement::removedFrom):
+        (WebCore::SVGElement::hasPendingResources):
+        (WebCore):
+        (WebCore::SVGElement::setHasPendingResources):
+        (WebCore::SVGElement::clearHasPendingResourcesIfPossible):
+        * svg/SVGElement.h:
+        (SVGElement):
+        (WebCore::SVGElement::buildPendingResource):
+        * svg/SVGStyledElement.cpp:
+        (WebCore):
+        (WebCore::SVGStyledElement::buildPendingResourcesIfNeeded):
+        (WebCore::SVGStyledElement::removedFrom):
+        * svg/SVGStyledElement.h:
+        (SVGStyledElement):
+        (WebCore::SVGStyledElement::selfHasRelativeLengths):
+
 2012-10-29  Dan Carney  <dcarney@google.com>
 
         Remove ensureAuxiliaryContext

diff --git a/Source/WebCore/svg/SVGDocumentExtensions.cpp b/Source/WebCore/svg/SVGDocumentExtensions.cpp
index ccb6323..db0eb72 100644 (file)
--- a/Source/WebCore/svg/SVGDocumentExtensions.cpp
+++ b/Source/WebCore/svg/SVGDocumentExtensions.cpp
@@ -210,7 +210,7 @@ void SVGDocumentExtensions::reportError(const String& message)
     reportMessage(m_document, ErrorMessageLevel, "Error: " + message);
 }
 
-void SVGDocumentExtensions::addPendingResource(const AtomicString& id, SVGStyledElement* element)
+void SVGDocumentExtensions::addPendingResource(const AtomicString& id, SVGElement* element)
 {
     ASSERT(element);
 
@@ -237,7 +237,7 @@ bool SVGDocumentExtensions::hasPendingResource(const AtomicString& id) const
     return m_pendingResources.contains(id);
 }
 
-bool SVGDocumentExtensions::isElementPendingResources(SVGStyledElement* element) const
+bool SVGDocumentExtensions::isElementPendingResources(SVGElement* element) const
 {
     // This algorithm takes time proportional to the number of pending resources and need not.
     // If performance becomes an issue we can keep a counted set of elements and answer the question efficiently.
@@ -255,7 +255,7 @@ bool SVGDocumentExtensions::isElementPendingResources(SVGStyledElement* element)
     return false;
 }
 
-bool SVGDocumentExtensions::isElementPendingResource(SVGStyledElement* element, const AtomicString& id) const
+bool SVGDocumentExtensions::isElementPendingResource(SVGElement* element, const AtomicString& id) const
 {
     ASSERT(element);
 
@@ -265,7 +265,7 @@ bool SVGDocumentExtensions::isElementPendingResource(SVGStyledElement* element,
     return m_pendingResources.get(id)->contains(element);
 }
 
-void SVGDocumentExtensions::removeElementFromPendingResources(SVGStyledElement* element)
+void SVGDocumentExtensions::removeElementFromPendingResources(SVGElement* element)
 {
     ASSERT(element);
 
@@ -336,7 +336,7 @@ void SVGDocumentExtensions::markPendingResourcesForRemoval(const AtomicString& i
         m_pendingResourcesForRemoval.add(id, existing);
 }
 
-SVGStyledElement* SVGDocumentExtensions::removeElementFromPendingResourcesForRemoval(const AtomicString& id)
+SVGElement* SVGDocumentExtensions::removeElementFromPendingResourcesForRemoval(const AtomicString& id)
 {
     if (id.isEmpty())
         return 0;
@@ -346,7 +346,7 @@ SVGStyledElement* SVGDocumentExtensions::removeElementFromPendingResourcesForRem
         return 0;
 
     SVGPendingElements::iterator firstElement = resourceSet->begin();
-    SVGStyledElement* element = *firstElement;
+    SVGElement* element = *firstElement;
 
     resourceSet->remove(firstElement);
 

diff --git a/Source/WebCore/svg/SVGDocumentExtensions.h b/Source/WebCore/svg/SVGDocumentExtensions.h
index 3fe83e3..b4455c5 100644 (file)
--- a/Source/WebCore/svg/SVGDocumentExtensions.h
+++ b/Source/WebCore/svg/SVGDocumentExtensions.h
@@ -40,12 +40,11 @@ class SVGFontFaceElement;
 class SVGResourcesCache;
 class SVGSMILElement;
 class SVGSVGElement;
-class SVGStyledElement;
 
 class SVGDocumentExtensions {
     WTF_MAKE_NONCOPYABLE(SVGDocumentExtensions); WTF_MAKE_FAST_ALLOCATED;
 public:
-    typedef HashSet<SVGStyledElement*> SVGPendingElements;
+    typedef HashSet<SVGElement*> SVGPendingElements;
     SVGDocumentExtensions(Document*);
     ~SVGDocumentExtensions();
     
@@ -97,17 +96,17 @@ private:
 public:
     // This HashMap contains a list of pending resources. Pending resources, are such
     // which are referenced by any object in the SVG document, but do NOT exist yet.
-    // For instance, dynamically build gradients / patterns / clippers...
-    void addPendingResource(const AtomicString& id, SVGStyledElement*);
+    // For instance, dynamically built gradients / patterns / clippers...
+    void addPendingResource(const AtomicString& id, SVGElement*);
     bool hasPendingResource(const AtomicString& id) const;
-    bool isElementPendingResources(SVGStyledElement*) const;
-    bool isElementPendingResource(SVGStyledElement*, const AtomicString& id) const;
-    void removeElementFromPendingResources(SVGStyledElement*);
+    bool isElementPendingResources(SVGElement*) const;
+    bool isElementPendingResource(SVGElement*, const AtomicString& id) const;
+    void removeElementFromPendingResources(SVGElement*);
     PassOwnPtr<SVGPendingElements> removePendingResource(const AtomicString& id);
 
     // The following two functions are used for scheduling a pending resource to be removed.
     void markPendingResourcesForRemoval(const AtomicString&);
-    SVGStyledElement* removeElementFromPendingResourcesForRemoval(const AtomicString&);
+    SVGElement* removeElementFromPendingResourcesForRemoval(const AtomicString&);
 
 private:
     PassOwnPtr<SVGPendingElements> removePendingResourceForRemoval(const AtomicString&);

diff --git a/Source/WebCore/svg/SVGElement.cpp b/Source/WebCore/svg/SVGElement.cpp
index a110f5e..6382f71 100644 (file)
--- a/Source/WebCore/svg/SVGElement.cpp
+++ b/Source/WebCore/svg/SVGElement.cpp
@@ -64,6 +64,10 @@ SVGElement::~SVGElement()
     if (!hasSVGRareData())
         ASSERT(!SVGElementRareData::rareDataMap().contains(this));
     else {
+        ASSERT(document());
+        if (hasPendingResources())
+            document()->accessSVGExtensions()->removeElementFromPendingResources(this);
+        ASSERT(!hasPendingResources());
         SVGElementRareData::SVGElementRareDataMap& rareDataMap = SVGElementRareData::rareDataMap();
         SVGElementRareData::SVGElementRareDataMap::iterator it = rareDataMap.find(this);
         ASSERT(it != rareDataMap.end());
@@ -78,6 +82,7 @@ SVGElement::~SVGElement()
         delete rareData;
         rareDataMap.remove(it);
     }
+    ASSERT(document());
     document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(this);
     document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
 }
@@ -180,6 +185,7 @@ void SVGElement::removedFrom(ContainerNode* rootParent)
     if (wasInDocument) {
         document()->accessSVGExtensions()->removeAllAnimationElementsFromTarget(this);
         document()->accessSVGExtensions()->removeAllElementReferencesForTarget(this);
+        document()->accessSVGExtensions()->removeElementFromPendingResources(this);
     }
 }
 
@@ -550,6 +556,22 @@ void SVGElement::attributeChanged(const QualifiedName& name, const AtomicString&
         svgAttributeChanged(name);
 }
 
+bool SVGElement::hasPendingResources() const
+{
+    return hasSVGRareData() && svgRareData()->hasPendingResources();
+}
+
+void SVGElement::setHasPendingResources()
+{
+    ensureSVGRareData()->setHasPendingResources(true);
+}
+
+void SVGElement::clearHasPendingResourcesIfPossible()
+{
+    if (!document()->accessSVGExtensions()->isElementPendingResources(this))
+        ensureSVGRareData()->setHasPendingResources(false);
+}
+
 void SVGElement::updateAnimatedSVGAttribute(const QualifiedName& name) const
 {
     if (isSynchronizingSVGAttributes() || areSVGAttributesValid())

diff --git a/Source/WebCore/svg/SVGElement.h b/Source/WebCore/svg/SVGElement.h
index 5720c35..c52e52a 100644 (file)
--- a/Source/WebCore/svg/SVGElement.h
+++ b/Source/WebCore/svg/SVGElement.h
@@ -70,6 +70,11 @@ public:
 
     virtual void svgAttributeChanged(const QualifiedName&) { }
 
+    bool hasPendingResources() const;
+    void setHasPendingResources();
+    void clearHasPendingResourcesIfPossible();
+    virtual void buildPendingResource() { }
+
     virtual void animatedPropertyTypeForAttribute(const QualifiedName&, Vector<AnimatedPropertyType>&);
 
     void sendSVGLoadEventIfPossible(bool sendParentLoadEvents = false);

diff --git a/Source/WebCore/svg/SVGStyledElement.cpp b/Source/WebCore/svg/SVGStyledElement.cpp
index c765dc6..5c1dbb8 100644 (file)
--- a/Source/WebCore/svg/SVGStyledElement.cpp
+++ b/Source/WebCore/svg/SVGStyledElement.cpp
@@ -74,14 +74,6 @@ SVGStyledElement::SVGStyledElement(const QualifiedName& tagName, Document* docum
     registerAnimatedPropertiesForSVGStyledElement();
 }
 
-SVGStyledElement::~SVGStyledElement()
-{
-    if (hasPendingResources() && document())
-        document()->accessSVGExtensions()->removeElementFromPendingResources(this);
-
-    ASSERT(!hasPendingResources());
-}
-
 String SVGStyledElement::title() const
 {
     // According to spec, we should not return titles when hovering over root <svg> elements (those
@@ -375,7 +367,7 @@ void SVGStyledElement::buildPendingResourcesIfNeeded()
     extensions->markPendingResourcesForRemoval(resourceId);
 
     // Rebuild pending resources for each client of a pending resource that is being removed.
-    while (SVGStyledElement* clientElement = extensions->removeElementFromPendingResourcesForRemoval(resourceId)) {
+    while (SVGElement* clientElement = extensions->removeElementFromPendingResourcesForRemoval(resourceId)) {
         ASSERT(clientElement->hasPendingResources());
         if (clientElement->hasPendingResources()) {
             clientElement->buildPendingResource();
@@ -390,11 +382,6 @@ void SVGStyledElement::removedFrom(ContainerNode* rootParent)
         updateRelativeLengthsInformation(false, this);
     SVGElement::removedFrom(rootParent);
     SVGElementInstance::invalidateAllInstancesOfElement(this);
-    Document* document = this->document();
-    if (!rootParent->inDocument() || !document)
-        return;
-
-    document->accessSVGExtensions()->removeElementFromPendingResources(this);
 }
 
 void SVGStyledElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
@@ -434,22 +421,6 @@ void SVGStyledElement::setInstanceUpdatesBlocked(bool value)
         svgRareData()->setInstanceUpdatesBlocked(value);
 }
 
-bool SVGStyledElement::hasPendingResources() const
-{
-    return hasSVGRareData() && svgRareData()->hasPendingResources();
-}
-
-void SVGStyledElement::setHasPendingResources()
-{
-    ensureSVGRareData()->setHasPendingResources(true);
-}
-
-void SVGStyledElement::clearHasPendingResourcesIfPossible()
-{
-    if (!document()->accessSVGExtensions()->isElementPendingResources(this))
-        ensureSVGRareData()->setHasPendingResources(false);
-}
-
 AffineTransform SVGStyledElement::localCoordinateSpaceTransform(SVGLocatable::CTMScope) const
 {
     // To be overriden by SVGStyledLocatableElement/SVGStyledTransformableElement (or as special case SVGTextElement and SVGPatternElement)

diff --git a/Source/WebCore/svg/SVGStyledElement.h b/Source/WebCore/svg/SVGStyledElement.h
index 037d22d..7f5c95a 100644 (file)
--- a/Source/WebCore/svg/SVGStyledElement.h
+++ b/Source/WebCore/svg/SVGStyledElement.h
@@ -35,8 +35,6 @@ void mapAttributeToCSSProperty(HashMap<AtomicStringImpl*, CSSPropertyID>* proper
 class SVGStyledElement : public SVGElement,
                          public SVGStylable {
 public:
-    virtual ~SVGStyledElement();
-
     virtual String title() const;
 
     bool hasRelativeLengths() const { return !m_elementsWithRelativeLengths.isEmpty(); }
@@ -50,10 +48,6 @@ public:
     bool instanceUpdatesBlocked() const;
     void setInstanceUpdatesBlocked(bool);
 
-    bool hasPendingResources() const;
-    void setHasPendingResources();
-    void clearHasPendingResourcesIfPossible();
-
     virtual void animatedPropertyTypeForAttribute(const QualifiedName&, Vector<AnimatedPropertyType>&);
     static bool isAnimatableCSSProperty(const QualifiedName&);
 
@@ -69,7 +63,7 @@ protected:
     virtual void parseAttribute(const Attribute&) OVERRIDE;
     virtual bool isPresentationAttribute(const QualifiedName&) const OVERRIDE;
     virtual void collectStyleForAttribute(const Attribute&, StylePropertySet*) OVERRIDE;
-    virtual void svgAttributeChanged(const QualifiedName&);
+    virtual void svgAttributeChanged(const QualifiedName&) OVERRIDE;
 
     virtual InsertionNotificationRequest insertedInto(ContainerNode*) OVERRIDE;
     virtual void removedFrom(ContainerNode*) OVERRIDE;
@@ -80,7 +74,6 @@ protected:
     void updateRelativeLengthsInformation(bool hasRelativeLengths, SVGStyledElement*);
 
     virtual bool selfHasRelativeLengths() const { return false; }
-    virtual void buildPendingResource() { }
 
 private:
     virtual bool isStyled() const { return true; }