Move more of SVG resources cache to using RenderElement.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Oct 2013 19:41:03 +0000 (19:41 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Oct 2013 19:41:03 +0000 (19:41 +0000)
<https://webkit.org/b/123452>

Make some more RenderSVGResourcesCache methods take RenderElement&
instead of RenderObject*.

Also removed a double hash lookup in removeResourcesFromRenderer().

Reviewed by Antti Koivisto.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGContainer.cpp
Source/WebCore/rendering/svg/RenderSVGInline.cpp
Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/RenderSVGText.cpp
Source/WebCore/rendering/svg/SVGResources.cpp
Source/WebCore/rendering/svg/SVGResources.h
Source/WebCore/rendering/svg/SVGResourcesCache.cpp
Source/WebCore/rendering/svg/SVGResourcesCache.h

index ed4596a..7d1d170 100644 (file)
@@ -1,3 +1,15 @@
+2013-10-29  Andreas Kling  <akling@apple.com>
+
+        Move more of SVG resources cache to using RenderElement.
+        <https://webkit.org/b/123452>
+
+        Make some more RenderSVGResourcesCache methods take RenderElement&
+        instead of RenderObject*.
+
+        Also removed a double hash lookup in removeResourcesFromRenderer().
+
+        Reviewed by Antti Koivisto.
+
 2013-10-29  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Remove old Inspector.json version files and generators
index 23aa59b..0ab3db1 100644 (file)
@@ -92,12 +92,12 @@ void RenderSVGContainer::layout()
 void RenderSVGContainer::addChild(RenderObject* child, RenderObject* beforeChild)
 {
     RenderSVGModelObject::addChild(child, beforeChild);
-    SVGResourcesCache::clientWasAddedToTree(child, &child->style());
+    SVGResourcesCache::clientWasAddedToTree(*child);
 }
 
 void RenderSVGContainer::removeChild(RenderObject& child)
 {
-    SVGResourcesCache::clientWillBeRemovedFromTree(&child);
+    SVGResourcesCache::clientWillBeRemovedFromTree(child);
     RenderSVGModelObject::removeChild(child);
 }
 
index 714b6f5..806b9ef 100644 (file)
@@ -117,7 +117,7 @@ void RenderSVGInline::styleDidChange(StyleDifference diff, const RenderStyle* ol
 void RenderSVGInline::addChild(RenderObject* child, RenderObject* beforeChild)
 {
     RenderInline::addChild(child, beforeChild);
-    SVGResourcesCache::clientWasAddedToTree(child, &child->style());
+    SVGResourcesCache::clientWasAddedToTree(*child);
 
     if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this))
         textRenderer->subtreeChildWasAdded(child);
@@ -125,7 +125,7 @@ void RenderSVGInline::addChild(RenderObject* child, RenderObject* beforeChild)
 
 void RenderSVGInline::removeChild(RenderObject& child)
 {
-    SVGResourcesCache::clientWillBeRemovedFromTree(&child);
+    SVGResourcesCache::clientWillBeRemovedFromTree(child);
 
     RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this);
     if (!textRenderer) {
index a58eb34..d6c3c2f 100644 (file)
@@ -64,7 +64,7 @@ void RenderSVGResourceContainer::layout()
 
 void RenderSVGResourceContainer::willBeDestroyed()
 {
-    SVGResourcesCache::resourceDestroyed(this);
+    SVGResourcesCache::resourceDestroyed(*this);
     RenderSVGHiddenContainer::willBeDestroyed();
 }
 
index 496dbfa..d347c85 100644 (file)
@@ -324,12 +324,12 @@ void RenderSVGRoot::styleDidChange(StyleDifference diff, const RenderStyle* oldS
 void RenderSVGRoot::addChild(RenderObject* child, RenderObject* beforeChild)
 {
     RenderReplaced::addChild(child, beforeChild);
-    SVGResourcesCache::clientWasAddedToTree(child, &child->style());
+    SVGResourcesCache::clientWasAddedToTree(*child);
 }
 
 void RenderSVGRoot::removeChild(RenderObject& child)
 {
-    SVGResourcesCache::clientWillBeRemovedFromTree(&child);
+    SVGResourcesCache::clientWillBeRemovedFromTree(child);
     RenderReplaced::removeChild(child);
 }
 
index b2f98d6..59384c3 100644 (file)
@@ -539,13 +539,13 @@ void RenderSVGText::addChild(RenderObject* child, RenderObject* beforeChild)
 {
     RenderSVGBlock::addChild(child, beforeChild);
 
-    SVGResourcesCache::clientWasAddedToTree(child, &child->style());
+    SVGResourcesCache::clientWasAddedToTree(*child);
     subtreeChildWasAdded(child);
 }
 
 void RenderSVGText::removeChild(RenderObject& child)
 {
-    SVGResourcesCache::clientWillBeRemovedFromTree(&child);
+    SVGResourcesCache::clientWillBeRemovedFromTree(child);
 
     Vector<SVGTextLayoutAttributes*, 2> affectedAttributes;
     FontCachePurgePreventer fontCachePurgePreventer;
index fbe00ad..a4bd3e8 100644 (file)
@@ -325,13 +325,12 @@ void SVGResources::removeClientFromCache(RenderObject* object, bool markForInval
     }
 }
 
-void SVGResources::resourceDestroyed(RenderSVGResourceContainer* resource)
+void SVGResources::resourceDestroyed(RenderSVGResourceContainer& resource)
 {
-    ASSERT(resource);
     if (!m_clipperFilterMaskerData && !m_markerData && !m_fillStrokeData && !m_linkedResource)
         return;
 
-    if (m_linkedResource == resource) {
+    if (m_linkedResource == &resource) {
         ASSERT(!m_clipperFilterMaskerData);
         ASSERT(!m_markerData);
         ASSERT(!m_fillStrokeData);
@@ -340,11 +339,11 @@ void SVGResources::resourceDestroyed(RenderSVGResourceContainer* resource)
         return;
     }
 
-    switch (resource->resourceType()) {
+    switch (resource.resourceType()) {
     case MaskerResourceType:
         if (!m_clipperFilterMaskerData)
             break;
-        if (m_clipperFilterMaskerData->masker == resource) {
+        if (m_clipperFilterMaskerData->masker == &resource) {
             m_clipperFilterMaskerData->masker->removeAllClientsFromCache();
             m_clipperFilterMaskerData->masker = 0;
         }
@@ -352,15 +351,15 @@ void SVGResources::resourceDestroyed(RenderSVGResourceContainer* resource)
     case MarkerResourceType:
         if (!m_markerData)
             break;
-        if (m_markerData->markerStart == resource) {
+        if (m_markerData->markerStart == &resource) {
             m_markerData->markerStart->removeAllClientsFromCache();
             m_markerData->markerStart = 0;
         }
-        if (m_markerData->markerMid == resource) {
+        if (m_markerData->markerMid == &resource) {
             m_markerData->markerMid->removeAllClientsFromCache();
             m_markerData->markerMid = 0;
         }
-        if (m_markerData->markerEnd == resource) {
+        if (m_markerData->markerEnd == &resource) {
             m_markerData->markerEnd->removeAllClientsFromCache();
             m_markerData->markerEnd = 0;
         }
@@ -370,11 +369,11 @@ void SVGResources::resourceDestroyed(RenderSVGResourceContainer* resource)
     case RadialGradientResourceType:
         if (!m_fillStrokeData)
             break;
-        if (m_fillStrokeData->fill == resource) {
+        if (m_fillStrokeData->fill == &resource) {
             m_fillStrokeData->fill->removeAllClientsFromCache();
             m_fillStrokeData->fill = 0;
         }
-        if (m_fillStrokeData->stroke == resource) {
+        if (m_fillStrokeData->stroke == &resource) {
             m_fillStrokeData->stroke->removeAllClientsFromCache();
             m_fillStrokeData->stroke = 0;
         }
@@ -383,7 +382,7 @@ void SVGResources::resourceDestroyed(RenderSVGResourceContainer* resource)
 #if ENABLE(FILTERS)
         if (!m_clipperFilterMaskerData)
             break;
-        if (m_clipperFilterMaskerData->filter == resource) {
+        if (m_clipperFilterMaskerData->filter == &resource) {
             m_clipperFilterMaskerData->filter->removeAllClientsFromCache();
             m_clipperFilterMaskerData->filter = 0;
         }
@@ -394,7 +393,7 @@ void SVGResources::resourceDestroyed(RenderSVGResourceContainer* resource)
     case ClipperResourceType:
         if (!m_clipperFilterMaskerData)
             break; 
-        if (m_clipperFilterMaskerData->clipper == resource) {
+        if (m_clipperFilterMaskerData->clipper == &resource) {
             m_clipperFilterMaskerData->clipper->removeAllClientsFromCache();
             m_clipperFilterMaskerData->clipper = 0;
         }
index 0079b73..ae370b4 100644 (file)
@@ -72,7 +72,7 @@ public:
 
     // Methods operating on all cached resources
     void removeClientFromCache(RenderObject*, bool markForInvalidation = true) const;
-    void resourceDestroyed(RenderSVGResourceContainer*);
+    void resourceDestroyed(RenderSVGResourceContainer&);
 
 #ifndef NDEBUG
     void dump(const RenderObject*);
index 8c584de..6289524 100644 (file)
@@ -38,55 +38,50 @@ SVGResourcesCache::~SVGResourcesCache()
 {
 }
 
-void SVGResourcesCache::addResourcesFromRenderObject(RenderObject* object, const RenderStyle* style)
+void SVGResourcesCache::addResourcesFromRenderer(RenderElement& renderer, const RenderStyle& style)
 {
-    ASSERT(object);
-    ASSERT(style);
-    ASSERT(!m_cache.contains(object));
+    ASSERT(!m_cache.contains(&renderer));
 
-    const SVGRenderStyle* svgStyle = style->svgStyle();
+    const SVGRenderStyle* svgStyle = style.svgStyle();
     ASSERT(svgStyle);
 
     // Build a list of all resources associated with the passed RenderObject
     OwnPtr<SVGResources> newResources = adoptPtr(new SVGResources);
-    if (!newResources->buildCachedResources(object, svgStyle))
+    if (!newResources->buildCachedResources(&renderer, svgStyle))
         return;
 
     // Put object in cache.
-    SVGResources* resources = m_cache.set(object, newResources.release()).iterator->value.get();
+    SVGResources* resources = m_cache.add(&renderer, newResources.release()).iterator->value.get();
 
     // Run cycle-detection _afterwards_, so self-references can be caught as well.
-    SVGResourcesCycleSolver solver(object, resources);
+    SVGResourcesCycleSolver solver(&renderer, resources);
     solver.resolveCycles();
 
     // Walk resources and register the render object at each resources.
     HashSet<RenderSVGResourceContainer*> resourceSet;
     resources->buildSetOfResources(resourceSet);
 
-    HashSet<RenderSVGResourceContainer*>::iterator end = resourceSet.end();
-    for (HashSet<RenderSVGResourceContainer*>::iterator it = resourceSet.begin(); it != end; ++it)
-        (*it)->addClient(object);
+    for (auto it = resourceSet.begin(), end = resourceSet.end(); it != end; ++it)
+        (*it)->addClient(&renderer);
 }
 
-void SVGResourcesCache::removeResourcesFromRenderObject(RenderObject* object)
+void SVGResourcesCache::removeResourcesFromRenderer(RenderElement& renderer)
 {
-    if (!m_cache.contains(object))
+    OwnPtr<SVGResources> resources = m_cache.take(&renderer);
+    if (!resources)
         return;
 
-    OwnPtr<SVGResources> resources = m_cache.take(object);
-
     // Walk resources and register the render object at each resources.
     HashSet<RenderSVGResourceContainer*> resourceSet;
     resources->buildSetOfResources(resourceSet);
 
-    HashSet<RenderSVGResourceContainer*>::iterator end = resourceSet.end();
-    for (HashSet<RenderSVGResourceContainer*>::iterator it = resourceSet.begin(); it != end; ++it)
-        (*it)->removeClient(object);
+    for (auto it = resourceSet.begin(), end = resourceSet.end(); it != end; ++it)
+        (*it)->removeClient(&renderer);
 }
 
-static inline SVGResourcesCache* resourcesCacheFromRenderObject(const RenderObject* renderer)
+static inline SVGResourcesCache* resourcesCacheFromRenderObject(const RenderObject& renderer)
 {
-    SVGDocumentExtensions* extensions = renderer->document().accessSVGExtensions();
+    SVGDocumentExtensions* extensions = renderer.document().accessSVGExtensions();
     ASSERT(extensions);
 
     SVGResourcesCache* cache = extensions->resourcesCache();
@@ -98,7 +93,7 @@ static inline SVGResourcesCache* resourcesCacheFromRenderObject(const RenderObje
 SVGResources* SVGResourcesCache::cachedResourcesForRenderObject(const RenderObject* renderer)
 {
     ASSERT(renderer);
-    return resourcesCacheFromRenderObject(renderer)->m_cache.get(renderer);
+    return resourcesCacheFromRenderObject(*renderer)->m_cache.get(renderer);
 }
 
 void SVGResourcesCache::clientLayoutChanged(RenderElement& renderer)
@@ -113,10 +108,9 @@ void SVGResourcesCache::clientLayoutChanged(RenderElement& renderer)
         resources->removeClientFromCache(&renderer);
 }
 
-static inline bool rendererCanHaveResources(RenderObject* renderer)
+static inline bool rendererCanHaveResources(RenderObject& renderer)
 {
-    ASSERT(renderer);
-    return renderer->node() && renderer->node()->isSVGElement() && !renderer->isSVGInlineText();
+    return renderer.node() && renderer.node()->isSVGElement() && !renderer.isSVGInlineText();
 }
 
 void SVGResourcesCache::clientStyleChanged(RenderElement& renderer, StyleDifference diff, const RenderStyle& newStyle)
@@ -131,10 +125,10 @@ void SVGResourcesCache::clientStyleChanged(RenderElement& renderer, StyleDiffere
     // Dynamic changes of CSS properties like 'clip-path' may require us to recompute the associated resources for a renderer.
     // FIXME: Avoid passing in a useless StyleDifference, but instead compare oldStyle/newStyle to see which resources changed
     // to be able to selectively rebuild individual resources, instead of all of them.
-    if (rendererCanHaveResources(&renderer)) {
-        SVGResourcesCache* cache = resourcesCacheFromRenderObject(&renderer);
-        cache->removeResourcesFromRenderObject(&renderer);
-        cache->addResourcesFromRenderObject(&renderer, &newStyle);
+    if (rendererCanHaveResources(renderer)) {
+        SVGResourcesCache* cache = resourcesCacheFromRenderObject(renderer);
+        cache->removeResourcesFromRenderer(renderer);
+        cache->addResourcesFromRenderer(renderer, newStyle);
     }
 
     RenderSVGResource::markForLayoutAndParentResourceInvalidation(&renderer, false);
@@ -143,28 +137,32 @@ void SVGResourcesCache::clientStyleChanged(RenderElement& renderer, StyleDiffere
         renderer.element()->setNeedsStyleRecalc(SyntheticStyleChange);
 }
 
-void SVGResourcesCache::clientWasAddedToTree(RenderObject* renderer, const RenderStyle* newStyle)
+void SVGResourcesCache::clientWasAddedToTree(RenderObject& renderer)
 {
-    if (!renderer->node())
+    if (renderer.isAnonymous())
         return;
-    RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer, false);
+
+    RenderSVGResource::markForLayoutAndParentResourceInvalidation(&renderer, false);
 
     if (!rendererCanHaveResources(renderer))
         return;
-    SVGResourcesCache* cache = resourcesCacheFromRenderObject(renderer);
-    cache->addResourcesFromRenderObject(renderer, newStyle);
+    RenderElement& elementRenderer = toRenderElement(renderer);
+    SVGResourcesCache* cache = resourcesCacheFromRenderObject(elementRenderer);
+    cache->addResourcesFromRenderer(elementRenderer, elementRenderer.style());
 }
 
-void SVGResourcesCache::clientWillBeRemovedFromTree(RenderObject* renderer)
+void SVGResourcesCache::clientWillBeRemovedFromTree(RenderObject& renderer)
 {
-    if (!renderer->node())
+    if (renderer.isAnonymous())
         return;
-    RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer, false);
+
+    RenderSVGResource::markForLayoutAndParentResourceInvalidation(&renderer, false);
 
     if (!rendererCanHaveResources(renderer))
         return;
-    SVGResourcesCache* cache = resourcesCacheFromRenderObject(renderer);
-    cache->removeResourcesFromRenderObject(renderer);
+    RenderElement& elementRenderer = toRenderElement(renderer);
+    SVGResourcesCache* cache = resourcesCacheFromRenderObject(elementRenderer);
+    cache->removeResourcesFromRenderer(elementRenderer);
 }
 
 void SVGResourcesCache::clientDestroyed(RenderElement& renderer)
@@ -173,28 +171,26 @@ void SVGResourcesCache::clientDestroyed(RenderElement& renderer)
     if (resources)
         resources->removeClientFromCache(&renderer);
 
-    SVGResourcesCache* cache = resourcesCacheFromRenderObject(&renderer);
-    cache->removeResourcesFromRenderObject(&renderer);
+    SVGResourcesCache* cache = resourcesCacheFromRenderObject(renderer);
+    cache->removeResourcesFromRenderer(renderer);
 }
 
-void SVGResourcesCache::resourceDestroyed(RenderSVGResourceContainer* resource)
+void SVGResourcesCache::resourceDestroyed(RenderSVGResourceContainer& resource)
 {
-    ASSERT(resource);
     SVGResourcesCache* cache = resourcesCacheFromRenderObject(resource);
 
     // The resource itself may have clients, that need to be notified.
-    cache->removeResourcesFromRenderObject(resource);
+    cache->removeResourcesFromRenderer(resource);
 
-    CacheMap::iterator end = cache->m_cache.end();
-    for (CacheMap::iterator it = cache->m_cache.begin(); it != end; ++it) {
+    for (auto it = cache->m_cache.begin(), end = cache->m_cache.end(); it != end; ++it) {
         it->value->resourceDestroyed(resource);
 
         // Mark users of destroyed resources as pending resolution based on the id of the old resource.
-        Element& resourceElement = resource->element();
+        Element& resourceElement = resource.element();
         Element* clientElement = toElement(it->key->node());
         SVGDocumentExtensions* extensions = clientElement->document().accessSVGExtensions();
 
-        extensions->addPendingResource(resourceElement.fastGetAttribute(HTMLNames::idAttr), clientElement);
+        extensions->addPendingResource(resourceElement.getIdAttribute(), clientElement);
     }
 }
 
index 7180d41..8dbe735 100644 (file)
@@ -42,10 +42,10 @@ public:
     static SVGResources* cachedResourcesForRenderObject(const RenderObject*);
 
     // Called from all SVG renderers addChild() methods.
-    static void clientWasAddedToTree(RenderObject*, const RenderStyle* newStyle);
+    static void clientWasAddedToTree(RenderObject&);
 
     // Called from all SVG renderers removeChild() methods.
-    static void clientWillBeRemovedFromTree(RenderObject*);
+    static void clientWillBeRemovedFromTree(RenderObject&);
 
     // Called from all SVG renderers destroy() methods - except for RenderSVGResourceContainer.
     static void clientDestroyed(RenderElement&);
@@ -57,11 +57,11 @@ public:
     static void clientStyleChanged(RenderElement&, StyleDifference, const RenderStyle& newStyle);
 
     // Called from RenderSVGResourceContainer::willBeDestroyed().
-    static void resourceDestroyed(RenderSVGResourceContainer*);
+    static void resourceDestroyed(RenderSVGResourceContainer&);
 
 private:
-    void addResourcesFromRenderObject(RenderObject*, const RenderStyle*);
-    void removeResourcesFromRenderObject(RenderObject*);
+    void addResourcesFromRenderer(RenderElement&, const RenderStyle&);
+    void removeResourcesFromRenderer(RenderElement&);
 
     typedef HashMap<const RenderObject*, OwnPtr<SVGResources>> CacheMap;
     CacheMap m_cache;