Changeset 57659 in webkit

Timestamp:

Apr 15, 2010 11:32:31 AM (14 years ago)

Author:

Nikolas Zimmermann

Message:

2010-04-15 Nikolas Zimmermann <​nzimmermann@rim.com>

Reviewed by Beth Dakin.

RenderSVGResource <-> id mapping should be cached
​https://bugs.webkit.org/show_bug.cgi?id=37575

Test: svg/custom/clip-path-id-changes.svg

• rendering/RenderSVGResource.h: (WebCore::RenderSVGResource::RenderSVGResource): (WebCore::RenderSVGResource::~RenderSVGResource): (WebCore::RenderSVGResource::idChanged): (WebCore::getRenderSVGResourceById):
• svg/SVGDocumentExtensions.cpp: (WebCore::SVGDocumentExtensions::addResource): (WebCore::SVGDocumentExtensions::removeResource): (WebCore::SVGDocumentExtensions::resourceById):
• svg/SVGDocumentExtensions.h:
• svg/SVGStyledElement.cpp: (WebCore::SVGStyledElement::svgAttributeChanged):

2010-04-15 Nikolas Zimmermann <​nzimmermann@rim.com>

Reviewed by Beth Dakin.

RenderSVGResource <-> id mapping should be cached
​https://bugs.webkit.org/show_bug.cgi?id=37575

• platform/mac/svg/custom/clip-path-id-changes-expected.checksum: Added.
• platform/mac/svg/custom/clip-path-id-changes-expected.png: Added.
• platform/mac/svg/custom/clip-path-id-changes-expected.txt: Added.
• svg/custom/clip-path-id-changes.svg: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/svg/custom/clip-path-id-changes-expected.checksum (added)
• LayoutTests/platform/mac/svg/custom/clip-path-id-changes-expected.png (added)
• LayoutTests/platform/mac/svg/custom/clip-path-id-changes-expected.txt (added)
• LayoutTests/svg/custom/clip-path-id-changes.svg (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/rendering/RenderSVGResource.h (modified) (3 diffs)
• WebCore/svg/SVGDocumentExtensions.cpp (modified) (1 diff)
• WebCore/svg/SVGDocumentExtensions.h (modified) (3 diffs)
• WebCore/svg/SVGStyledElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-04-15  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Beth Dakin.
+
+        RenderSVGResource <-> id mapping should be cached
+        https://bugs.webkit.org/show_bug.cgi?id=37575
+
+        * platform/mac/svg/custom/clip-path-id-changes-expected.checksum: Added.
+        * platform/mac/svg/custom/clip-path-id-changes-expected.png: Added.
+        * platform/mac/svg/custom/clip-path-id-changes-expected.txt: Added.
+        * svg/custom/clip-path-id-changes.svg: Added.
+
 2010-04-15  Anton Muhin   <antonm@chromium.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-04-15  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Beth Dakin.
+
+        RenderSVGResource <-> id mapping should be cached
+        https://bugs.webkit.org/show_bug.cgi?id=37575
+
+        Test: svg/custom/clip-path-id-changes.svg
+
+        * rendering/RenderSVGResource.h:
+        (WebCore::RenderSVGResource::RenderSVGResource):
+        (WebCore::RenderSVGResource::~RenderSVGResource):
+        (WebCore::RenderSVGResource::idChanged):
+        (WebCore::getRenderSVGResourceById):
+        * svg/SVGDocumentExtensions.cpp:
+        (WebCore::SVGDocumentExtensions::addResource):
+        (WebCore::SVGDocumentExtensions::removeResource):
+        (WebCore::SVGDocumentExtensions::resourceById):
+        * svg/SVGDocumentExtensions.h:
+        * svg/SVGStyledElement.cpp:
+        (WebCore::SVGStyledElement::svgAttributeChanged):
+
 2010-04-15  Justin Schuh  <jschuh@chromium.org>
 

trunk/WebCore/rendering/RenderSVGResource.h

@@ -36 +36 @@
 class RenderSVGResource : public RenderSVGHiddenContainer {
 public:
-    RenderSVGResource(SVGStyledElement* node) : RenderSVGHiddenContainer(node) { }
+    RenderSVGResource(SVGStyledElement* node)
+        : RenderSVGHiddenContainer(node)
+        , m_id(node->getIDAttribute())
+    {
+        ASSERT(node->document());
+        node->document()->accessSVGExtensions()->addResource(m_id, this);
+    }
+
+    virtual ~RenderSVGResource()
+    {
+        ASSERT(node());
+        ASSERT(node()->document());
+        node()->document()->accessSVGExtensions()->removeResource(m_id);
+    }
+
+    void idChanged()
+    {
+        ASSERT(node());
+        ASSERT(node()->document());
+        SVGDocumentExtensions* extensions = node()->document()->accessSVGExtensions();
+
+        // Remove old id, that is guaranteed to be present in cache
+        extensions->removeResource(m_id);
+
+        m_id = static_cast<Element*>(node())->getIDAttribute();
+
+        // It's possible that an element is referencing us with the new id, and has to be notified that we're existing now
+        if (extensions->isPendingResource(m_id)) {
+            OwnPtr<HashSet<SVGStyledElement*> > clients(extensions->removePendingResource(m_id));
+            if (clients->isEmpty())
+                return;
+
+            HashSet<SVGStyledElement*>::const_iterator it = clients->begin();
+            const HashSet<SVGStyledElement*>::const_iterator end = clients->end();
+
+            for (; it != end; ++it) {
+                if (RenderObject* renderer = (*it)->renderer())
+                    renderer->setNeedsLayout(true);
+            }
+        }
+
+        // Recache us with the new id
+        extensions->addResource(m_id, this);
+    }
 
     template<class Renderer>
@@ -58 +101 @@
 
     virtual RenderSVGResourceType resourceType() const = 0;
+
+private:
+    AtomicString m_id;
 };
 
@@ -66 +112 @@
         return 0;
 
-    Element* element = document->getElementById(id);
-    if (!element || !element->isSVGElement())
-        return 0;
+    if (RenderSVGResource* renderResource = document->accessSVGExtensions()->resourceById(id))
+        return renderResource->cast<Renderer>();
 
-    RenderObject* renderer = element->renderer();
-    if (!renderer)
-        return 0;
-
-    RenderSVGResource* renderResource = renderer->toRenderSVGResource();
-    if (!renderResource)
-        return 0;
-
-    return renderResource->cast<Renderer>();
+    return 0;
 }
 

trunk/WebCore/svg/SVGDocumentExtensions.cpp

@@ -61 +61 @@
 {
     m_timeContainers.remove(element);
+}
+
+void SVGDocumentExtensions::addResource(const String& id, RenderSVGResource* resource)
+{
+    ASSERT(resource);
+
+    if (id.isEmpty())
+        return;
+
+    // Replaces resource if already present, to handle potential id changes
+    m_resources.set(id, resource);
+}
+
+void SVGDocumentExtensions::removeResource(const String& id)
+{
+    if (id.isEmpty())
+        return;
+
+    ASSERT(m_resources.contains(id));
+    m_resources.remove(id);
+}
+
+RenderSVGResource* SVGDocumentExtensions::resourceById(const String& id) const
+{
+    if (id.isEmpty())
+        return 0;
+
+    return m_resources.get(id);
 }
 

trunk/WebCore/svg/SVGDocumentExtensions.h

@@ -36 +36 @@
 
 class Document;
+class RenderSVGResource;
 class String;
 class SVGStyledElement;
@@ -48 +49 @@
     void addTimeContainer(SVGSVGElement*);
     void removeTimeContainer(SVGSVGElement*);
-    
+
+    void addResource(const String& id, RenderSVGResource*);
+    void removeResource(const String& id);
+    RenderSVGResource* resourceById(const String& id) const;
+
     void startAnimations();
     void pauseAnimations();
@@ -60 +65 @@
     Document* m_doc; // weak reference
     HashSet<SVGSVGElement*> m_timeContainers; // For SVG 1.2 support this will need to be made more general.
+    HashMap<String, RenderSVGResource*> m_resources;
     HashMap<String, HashSet<SVGStyledElement*>*> m_pendingResources;
 

trunk/WebCore/svg/SVGStyledElement.cpp

@@ -200 +200 @@
         classAttributeChanged(className());
 
+    if (attrName == idAttributeName()) {
+        // Notify resources about id changes, this is important as we cache resources by id in SVGDocumentExtensions
+        if (renderer() && renderer()->isSVGResource()) {
+            RenderSVGResource* resource = renderer()->toRenderSVGResource();
+            resource->idChanged();
+        }
+    }
+
     // If we're the child of a resource element, be sure to invalidate it.
     invalidateResourcesInAncestorChain();