CTTE: SVGResourcesCache::cachedResourcesForRenderObject() should take a reference
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 15 Dec 2013 06:20:05 +0000 (06:20 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 15 Dec 2013 06:20:05 +0000 (06:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=125743

Patch by Sam Weinig <sam@webkit.org> on 2013-12-14
Reviewed by Dan Bernstein.

* rendering/svg/RenderSVGContainer.cpp:
(WebCore::RenderSVGContainer::selfWillPaint):
* rendering/svg/RenderSVGImage.cpp:
(WebCore::RenderSVGImage::imageChanged):
* rendering/svg/RenderSVGResource.cpp:
(WebCore::requestPaintingResource):
(WebCore::removeFromCacheAndInvalidateDependencies):
(WebCore::RenderSVGResource::markForLayoutAndParentResourceInvalidation):
* rendering/svg/RenderSVGResourceClipper.cpp:
(WebCore::RenderSVGResourceClipper::applyClippingToContext):
* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::paintReplaced):
* rendering/svg/RenderSVGShape.cpp:
(WebCore::RenderSVGShape::shouldGenerateMarkerPositions):
(WebCore::RenderSVGShape::markerRect):
(WebCore::RenderSVGShape::drawMarkers):
* rendering/svg/SVGRenderSupport.cpp:
(WebCore::invalidateResourcesOfChildren):
(WebCore::SVGRenderSupport::layoutChildren):
(WebCore::SVGRenderSupport::intersectRepaintRectWithResources):
(WebCore::SVGRenderSupport::filtersForceContainerLayout):
(WebCore::SVGRenderSupport::pointInClippingArea):
* rendering/svg/SVGRenderingContext.cpp:
(WebCore::SVGRenderingContext::prepareToRenderSVGContent):
* rendering/svg/SVGResourcesCache.cpp:
(WebCore::SVGResourcesCache::cachedResourcesForRenderObject):
(WebCore::SVGResourcesCache::clientLayoutChanged):
(WebCore::SVGResourcesCache::clientDestroyed):
* rendering/svg/SVGResourcesCache.h:
* rendering/svg/SVGResourcesCycleSolver.cpp:
(WebCore::SVGResourcesCycleSolver::resourceContainsCycles):

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

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGContainer.cpp
Source/WebCore/rendering/svg/RenderSVGImage.cpp
Source/WebCore/rendering/svg/RenderSVGResource.cpp
Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/RenderSVGShape.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.cpp
Source/WebCore/rendering/svg/SVGRenderingContext.cpp
Source/WebCore/rendering/svg/SVGResourcesCache.cpp
Source/WebCore/rendering/svg/SVGResourcesCache.h
Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp

index fea3213..c5a87b7 100644 (file)
@@ -1,3 +1,42 @@
+2013-12-14  Sam Weinig  <sam@webkit.org>
+
+        CTTE: SVGResourcesCache::cachedResourcesForRenderObject() should take a reference
+        https://bugs.webkit.org/show_bug.cgi?id=125743
+
+        Reviewed by Dan Bernstein.
+
+        * rendering/svg/RenderSVGContainer.cpp:
+        (WebCore::RenderSVGContainer::selfWillPaint):
+        * rendering/svg/RenderSVGImage.cpp:
+        (WebCore::RenderSVGImage::imageChanged):
+        * rendering/svg/RenderSVGResource.cpp:
+        (WebCore::requestPaintingResource):
+        (WebCore::removeFromCacheAndInvalidateDependencies):
+        (WebCore::RenderSVGResource::markForLayoutAndParentResourceInvalidation):
+        * rendering/svg/RenderSVGResourceClipper.cpp:
+        (WebCore::RenderSVGResourceClipper::applyClippingToContext):
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::paintReplaced):
+        * rendering/svg/RenderSVGShape.cpp:
+        (WebCore::RenderSVGShape::shouldGenerateMarkerPositions):
+        (WebCore::RenderSVGShape::markerRect):
+        (WebCore::RenderSVGShape::drawMarkers):
+        * rendering/svg/SVGRenderSupport.cpp:
+        (WebCore::invalidateResourcesOfChildren):
+        (WebCore::SVGRenderSupport::layoutChildren):
+        (WebCore::SVGRenderSupport::intersectRepaintRectWithResources):
+        (WebCore::SVGRenderSupport::filtersForceContainerLayout):
+        (WebCore::SVGRenderSupport::pointInClippingArea):
+        * rendering/svg/SVGRenderingContext.cpp:
+        (WebCore::SVGRenderingContext::prepareToRenderSVGContent):
+        * rendering/svg/SVGResourcesCache.cpp:
+        (WebCore::SVGResourcesCache::cachedResourcesForRenderObject):
+        (WebCore::SVGResourcesCache::clientLayoutChanged):
+        (WebCore::SVGResourcesCache::clientDestroyed):
+        * rendering/svg/SVGResourcesCache.h:
+        * rendering/svg/SVGResourcesCycleSolver.cpp:
+        (WebCore::SVGResourcesCycleSolver::resourceContainsCycles):
+
 2013-12-14  Dan Bernstein  <mitz@apple.com>
 
         Clean up the project after r160487
index fd43753..beecb82 100644 (file)
@@ -105,7 +105,7 @@ void RenderSVGContainer::removeChild(RenderObject& child)
 
 bool RenderSVGContainer::selfWillPaint()
 {
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this);
     return resources && resources->filter();
 }
 
index 57fc9be..c4ea14a 100644 (file)
@@ -194,7 +194,7 @@ void RenderSVGImage::imageChanged(WrappedImagePtr, const IntRect*)
 {
     // The image resource defaults to nullImage until the resource arrives.
     // This empty image may be cached by SVG resources which must be invalidated.
-    if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this))
+    if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this))
         resources->removeClientFromCache(this);
 
     // Eventually notify parent resources, that we've changed.
index 1c53485..408f481 100644 (file)
@@ -113,7 +113,7 @@ static inline RenderSVGResource* requestPaintingResource(RenderSVGResourceMode m
     }
 
     // If no resources are associated with the given renderer, return the color resource.
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer);
     if (!resources) {
         if (paintType == SVGPaint::SVG_PAINTTYPE_URI_NONE || !inheritColorFromParentStyleIfNeeded(renderer, applyToFill, color))
             return nullptr;
@@ -156,29 +156,27 @@ RenderSVGResourceSolidColor* RenderSVGResource::sharedSolidPaintingResource()
     return s_sharedSolidPaintingResource;
 }
 
-static inline void removeFromCacheAndInvalidateDependencies(RenderObject* object, bool needsLayout)
+static inline void removeFromCacheAndInvalidateDependencies(RenderObject& object, bool needsLayout)
 {
-    ASSERT(object);
     if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(object)) {
 #if ENABLE(FILTERS)
         if (RenderSVGResourceFilter* filter = resources->filter())
-            filter->removeClientFromCache(object);
+            filter->removeClientFromCache(&object);
 #endif
         if (RenderSVGResourceMasker* masker = resources->masker())
-            masker->removeClientFromCache(object);
+            masker->removeClientFromCache(&object);
 
         if (RenderSVGResourceClipper* clipper = resources->clipper())
-            clipper->removeClientFromCache(object);
+            clipper->removeClientFromCache(&object);
     }
 
-    if (!object->node() || !object->node()->isSVGElement())
+    if (!object.node() || !object.node()->isSVGElement())
         return;
-    HashSet<SVGElement*>* dependencies = object->document().accessSVGExtensions()->setOfElementsReferencingTarget(toSVGElement(object->node()));
+    HashSet<SVGElement*>* dependencies = object.document().accessSVGExtensions()->setOfElementsReferencingTarget(toSVGElement(object.node()));
     if (!dependencies)
         return;
-    HashSet<SVGElement*>::iterator end = dependencies->end();
-    for (HashSet<SVGElement*>::iterator it = dependencies->begin(); it != end; ++it) {
-        if (RenderObject* renderer = (*it)->renderer())
+    for (auto element : *dependencies) {
+        if (RenderObject* renderer = element->renderer())
             RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer, needsLayout);
     }
 }
@@ -191,12 +189,12 @@ void RenderSVGResource::markForLayoutAndParentResourceInvalidation(RenderObject*
     if (needsLayout && !object->documentBeingDestroyed())
         object->setNeedsLayout();
 
-    removeFromCacheAndInvalidateDependencies(object, needsLayout);
+    removeFromCacheAndInvalidateDependencies(*object, needsLayout);
 
     // Invalidate resources in ancestor chain, if needed.
     RenderObject* current = object->parent();
     while (current) {
-        removeFromCacheAndInvalidateDependencies(current, needsLayout);
+        removeFromCacheAndInvalidateDependencies(*current, needsLayout);
 
         if (current->isSVGResourceContainer()) {
             // This will process the rest of the ancestors.
index 53ce239..936ad79 100644 (file)
@@ -171,7 +171,7 @@ bool RenderSVGResourceClipper::applyClippingToContext(RenderElement& renderer, c
         maskContext->concatCTM(animatedLocalTransform);
 
         // clipPath can also be clipped by another clipPath.
-        SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
+        SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this);
         RenderSVGResourceClipper* clipper;
         bool succeeded;
         if (resources && (clipper = resources->clipper())) {
index 2dfd79d..aeaec82 100644 (file)
@@ -257,7 +257,7 @@ void RenderSVGRoot::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paint
 
     // Don't paint if we don't have kids, except if we have filters we should paint those.
     if (!firstChild()) {
-        SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
+        SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this);
         if (!resources || !resources->filter()) {
             if (page && paintInfo.phase == PaintPhaseForeground)
                 page->addRelevantUnpaintedObject(this, visualOverflowRect());
index 60de2b8..4ad4e86 100644 (file)
@@ -224,7 +224,7 @@ bool RenderSVGShape::shouldGenerateMarkerPositions() const
     if (!graphicsElement().supportsMarkers())
         return false;
 
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this);
     if (!resources)
         return false;
 
@@ -367,7 +367,7 @@ FloatRect RenderSVGShape::markerRect(float strokeWidth) const
 {
     ASSERT(!m_markerPositions.isEmpty());
 
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this);
     ASSERT(resources);
 
     RenderSVGResourceMarker* markerStart = resources->markerStart();
@@ -443,7 +443,7 @@ void RenderSVGShape::drawMarkers(PaintInfo& paintInfo)
 {
     ASSERT(!m_markerPositions.isEmpty());
 
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*this);
     if (!resources)
         return;
 
index 1cbd5d7..a6e8868 100644 (file)
@@ -190,14 +190,14 @@ const RenderSVGRoot& SVGRenderSupport::findTreeRootObject(const RenderElement& s
     return toRenderSVGRoot(*renderer);
 }
 
-static inline void invalidateResourcesOfChildren(RenderObject* start)
+static inline void invalidateResourcesOfChildren(RenderObject& start)
 {
-    ASSERT(!start->needsLayout());
+    ASSERT(!start.needsLayout());
     if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(start))
-        resources->removeClientFromCache(start, false);
+        resources->removeClientFromCache(&start, false);
 
-    for (RenderObject* child = start->firstChildSlow(); child; child = child->nextSibling())
-        invalidateResourcesOfChildren(child);
+    for (RenderObject* child = start.firstChildSlow(); child; child = child->nextSibling())
+        invalidateResourcesOfChildren(*child);
 }
 
 static inline bool layoutSizeOfNearestViewportChanged(const RenderElement& renderer)
@@ -293,9 +293,8 @@ void SVGRenderSupport::layoutChildren(RenderElement& start, bool selfNeedsLayout
     }
 
     // If the layout size changed, invalidate all resources of all children that didn't go through the layout() code path.
-    HashSet<RenderObject*>::iterator end = notlayoutedObjects.end();
-    for (HashSet<RenderObject*>::iterator it = notlayoutedObjects.begin(); it != end; ++it)
-        invalidateResourcesOfChildren(*it);
+    for (auto child : notlayoutedObjects)
+        invalidateResourcesOfChildren(*child);
 }
 
 bool SVGRenderSupport::isOverflowHidden(const RenderElement& renderer)
@@ -369,7 +368,7 @@ void SVGRenderSupport::intersectRepaintRectWithShadows(const RenderElement& rend
 
 void SVGRenderSupport::intersectRepaintRectWithResources(const RenderElement& renderer, FloatRect& repaintRect)
 {
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer);
     if (!resources)
         return;
 
@@ -392,7 +391,7 @@ bool SVGRenderSupport::filtersForceContainerLayout(const RenderElement& renderer
     if (!renderer.normalChildNeedsLayout())
         return false;
 
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer);
     if (!resources || !resources->filter())
         return false;
 
@@ -403,7 +402,7 @@ bool SVGRenderSupport::pointInClippingArea(const RenderElement& renderer, const
 {
     // We just take clippers into account to determine if a point is on the node. The Specification may
     // change later and we also need to check maskers.
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer);
     if (!resources)
         return true;
 
index 69bcdfa..72e363d 100644 (file)
@@ -128,7 +128,7 @@ void SVGRenderingContext::prepareToRenderSVGContent(RenderElement& renderer, Pai
         m_paintInfo->context->clipPath(clipPath->path(renderer.objectBoundingBox()), clipPath->windRule());
     }
 
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(m_renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(*m_renderer);
     if (!resources) {
 #if ENABLE(FILTERS)
         if (svgStyle.hasFilter())
index 8b9cc83..8f2f864 100644 (file)
@@ -89,15 +89,14 @@ static inline SVGResourcesCache* resourcesCacheFromRenderObject(const RenderObje
     return cache;
 }
 
-SVGResources* SVGResourcesCache::cachedResourcesForRenderObject(const RenderObject* renderer)
+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)
 {
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer);
     if (!resources)
         return;
 
@@ -166,7 +165,7 @@ void SVGResourcesCache::clientWillBeRemovedFromTree(RenderObject& renderer)
 
 void SVGResourcesCache::clientDestroyed(RenderElement& renderer)
 {
-    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer);
+    SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer);
     if (resources)
         resources->removeClientFromCache(&renderer);
 
index 8dbe735..fe6d5e6 100644 (file)
@@ -39,7 +39,7 @@ public:
     SVGResourcesCache();
     ~SVGResourcesCache();
 
-    static SVGResources* cachedResourcesForRenderObject(const RenderObject*);
+    static SVGResources* cachedResourcesForRenderObject(const RenderObject&);
 
     // Called from all SVG renderers addChild() methods.
     static void clientWasAddedToTree(RenderObject&);
index 3779784..eeead7d 100644 (file)
@@ -53,13 +53,13 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) co
     // First operate on the resources of the given renderer.
     // <marker id="a"> <path marker-start="url(#b)"/> ...
     // <marker id="b" marker-start="url(#a)"/>
-    if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(&renderer)) {
+    if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(renderer)) {
         HashSet<RenderSVGResourceContainer*> resourceSet;
         resources->buildSetOfResources(resourceSet);
 
         // Walk all resources and check wheter they reference any resource contained in the resources set.
-        for (auto it = resourceSet.begin(), end = resourceSet.end(); it != end; ++it) {
-            if (m_allResources.contains(*it))
+        for (auto resource : resourceSet) {
+            if (m_allResources.contains(resource))
                 return true;
         }
     }
@@ -68,7 +68,7 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) co
     // <marker id="a"> <path marker-start="url(#b)"/> ...
     // <marker id="b"> <path marker-start="url(#a)"/> ...
     for (auto& child : childrenOfType<RenderElement>(renderer)) {
-        SVGResources* childResources = SVGResourcesCache::cachedResourcesForRenderObject(&child);
+        SVGResources* childResources = SVGResourcesCache::cachedResourcesForRenderObject(child);
         if (!childResources)
             continue;