Tighten typing in SVGResourcesCycleSolver a bit.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Nov 2013 16:11:22 +0000 (16:11 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Nov 2013 16:11:22 +0000 (16:11 +0000)
<https://webkit.org/b/124104>

Make the SVGResourcesCycleSolver constructor take a RenderElement&
and a SVGResources&.

While I was in the neighborhood, also converted one loop to use a
renderer iterator instead of walking siblings manually.

Finally used "auto" to clean up some overly wordy loops.

Reviewed by Anders Carlsson.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/SVGResourcesCache.cpp
Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp
Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h

index f5485c5..40c83b4 100644 (file)
@@ -1,5 +1,20 @@
 2013-11-09  Andreas Kling  <akling@apple.com>
 
+        Tighten typing in SVGResourcesCycleSolver a bit.
+        <https://webkit.org/b/124104>
+
+        Make the SVGResourcesCycleSolver constructor take a RenderElement&
+        and a SVGResources&.
+
+        While I was in the neighborhood, also converted one loop to use a
+        renderer iterator instead of walking siblings manually.
+
+        Finally used "auto" to clean up some overly wordy loops.
+
+        Reviewed by Anders Carlsson.
+
+2013-11-09  Andreas Kling  <akling@apple.com>
+
         Beat SVGRenderSupport with the RenderElement stick.
         <https://webkit.org/b/124102>
 
index c03cff7..8b9cc83 100644 (file)
@@ -50,15 +50,15 @@ void SVGResourcesCache::addResourcesFromRenderer(RenderElement& renderer, const
         return;
 
     // Put object in cache.
-    SVGResources* resources = m_cache.add(&renderer, newResources.release()).iterator->value.get();
+    SVGResources& resources = *m_cache.add(&renderer, newResources.release()).iterator->value;
 
     // Run cycle-detection _afterwards_, so self-references can be caught as well.
-    SVGResourcesCycleSolver solver(&renderer, resources);
+    SVGResourcesCycleSolver solver(renderer, resources);
     solver.resolveCycles();
 
     // Walk resources and register the render object at each resources.
     HashSet<RenderSVGResourceContainer*> resourceSet;
-    resources->buildSetOfResources(resourceSet);
+    resources.buildSetOfResources(resourceSet);
 
     for (auto it = resourceSet.begin(), end = resourceSet.end(); it != end; ++it)
         (*it)->addClient(&renderer);
index e679c39..8462e71 100644 (file)
@@ -24,6 +24,8 @@
 #define DEBUG_CYCLE_DETECTION 0
 
 #if ENABLE(SVG)
+#include "RenderElement.h"
+#include "RenderIterator.h"
 #include "RenderSVGResourceClipper.h"
 #include "RenderSVGResourceFilter.h"
 #include "RenderSVGResourceMarker.h"
 
 namespace WebCore {
 
-SVGResourcesCycleSolver::SVGResourcesCycleSolver(RenderObject* renderer, SVGResources* resources)
+SVGResourcesCycleSolver::SVGResourcesCycleSolver(RenderElement& renderer, SVGResources& resources)
     : m_renderer(renderer)
     , m_resources(resources)
 {
-    ASSERT(m_renderer);
-    ASSERT(m_resources);
 }
 
 SVGResourcesCycleSolver::~SVGResourcesCycleSolver()
 {
 }
 
-bool SVGResourcesCycleSolver::resourceContainsCycles(RenderObject* renderer) const
+bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) const
 {
-    ASSERT(renderer);
-
     // 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.
-        HashSet<RenderSVGResourceContainer*>::iterator end = resourceSet.end();
-        for (HashSet<RenderSVGResourceContainer*>::iterator it = resourceSet.begin(); it != end; ++it) {
+        for (auto it = resourceSet.begin(), end = resourceSet.end(); it != end; ++it) {
             if (m_allResources.contains(*it))
                 return true;
         }
@@ -70,8 +67,9 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderObject* renderer) con
     // Then operate on the child resources of the given renderer.
     // <marker id="a"> <path marker-start="url(#b)"/> ...
     // <marker id="b"> <path marker-start="url(#a)"/> ...
-    for (RenderObject* child = renderer->firstChildSlow(); child; child = child->nextSibling()) {
-        SVGResources* childResources = SVGResourcesCache::cachedResourcesForRenderObject(child);
+    auto children = childrenOfType<RenderElement>(renderer);
+    for (auto child = children.begin(), end = children.end(); child != end; ++child) {
+        SVGResources* childResources = SVGResourcesCache::cachedResourcesForRenderObject(&*child);
         if (!childResources)
             continue;
         
@@ -80,14 +78,13 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderObject* renderer) con
         childResources->buildSetOfResources(childSet);
 
         // Walk all child resources and check wheter they reference any resource contained in the resources set.
-        HashSet<RenderSVGResourceContainer*>::iterator end = childSet.end();
-        for (HashSet<RenderSVGResourceContainer*>::iterator it = childSet.begin(); it != end; ++it) {
+        for (auto it = childSet.begin(), end = childSet.end(); it != end; ++it) {
             if (m_allResources.contains(*it))
                 return true;
         }
 
         // Walk children recursively, stop immediately if we found a cycle
-        if (resourceContainsCycles(child))
+        if (resourceContainsCycles(*child))
             return true;
     }
 
@@ -100,17 +97,17 @@ void SVGResourcesCycleSolver::resolveCycles()
 
 #if DEBUG_CYCLE_DETECTION > 0
     fprintf(stderr, "\nBefore cycle detection:\n");
-    m_resources->dump(m_renderer);
+    m_resources.dump(&m_renderer);
 #endif
 
     // Stash all resources into a HashSet for the ease of traversing.
     HashSet<RenderSVGResourceContainer*> localResources;
-    m_resources->buildSetOfResources(localResources);
+    m_resources.buildSetOfResources(localResources);
     ASSERT(!localResources.isEmpty());
 
     // Add all parent resource containers to the HashSet.
     HashSet<RenderSVGResourceContainer*> parentResources;
-    RenderObject* parent = m_renderer->parent();
+    auto parent = m_renderer.parent();
     while (parent) {
         if (parent->isSVGResourceContainer())
             parentResources.add(parent->toRenderSVGResourceContainer());
@@ -121,86 +118,81 @@ void SVGResourcesCycleSolver::resolveCycles()
     fprintf(stderr, "\nDetecting wheter any resources references any of following objects:\n");
     {
         fprintf(stderr, "Local resources:\n");
-        HashSet<RenderSVGResourceContainer*>::iterator end = localResources.end();
-        for (HashSet<RenderSVGResourceContainer*>::iterator it = localResources.begin(); it != end; ++it)
+        for (auto it = localResources.begin(), end = localResources.end(); it != end; ++it)
             fprintf(stderr, "|> %s: object=%p (node=%p)\n", (*it)->renderName(), *it, (*it)->node());
 
         fprintf(stderr, "Parent resources:\n");
-        end = parentResources.end();
-        for (HashSet<RenderSVGResourceContainer*>::iterator it = parentResources.begin(); it != end; ++it)
+        for (auto it = parentResources.begin(), end = parentResources.end(); it != end; ++it)
             fprintf(stderr, "|> %s: object=%p (node=%p)\n", (*it)->renderName(), *it, (*it)->node());
     }
 #endif
 
     // Build combined set of local and parent resources.
     m_allResources = localResources;
-    HashSet<RenderSVGResourceContainer*>::iterator end = parentResources.end();
-    for (HashSet<RenderSVGResourceContainer*>::iterator it = parentResources.begin(); it != end; ++it)
+    for (auto it = parentResources.begin(), end = parentResources.end(); it != end; ++it)
         m_allResources.add(*it);
 
     // If we're a resource, add ourselves to the HashSet.
-    if (m_renderer->isSVGResourceContainer())
-        m_allResources.add(m_renderer->toRenderSVGResourceContainer());
+    if (m_renderer.isSVGResourceContainer())
+        m_allResources.add(m_renderer.toRenderSVGResourceContainer());
 
     ASSERT(!m_allResources.isEmpty());
 
     // The job of this function is to determine wheter any of the 'resources' associated with the given 'renderer'
     // references us (or wheter any of its kids references us) -> that's a cycle, we need to find and break it.
-    end = localResources.end();
-    for (HashSet<RenderSVGResourceContainer*>::iterator it = localResources.begin(); it != end; ++it) {
-        RenderSVGResourceContainer* resource = *it;
-        if (parentResources.contains(resource) || resourceContainsCycles(resource))
+    for (auto it = localResources.begin(), end = localResources.end(); it != end; ++it) {
+        RenderSVGResourceContainer& resource = **it;
+        if (parentResources.contains(&resource) || resourceContainsCycles(resource))
             breakCycle(resource);
     }
 
 #if DEBUG_CYCLE_DETECTION > 0
     fprintf(stderr, "\nAfter cycle detection:\n");
-    m_resources->dump(m_renderer);
+    m_resources.dump(m_renderer);
 #endif
 
     m_allResources.clear();
 }
 
-void SVGResourcesCycleSolver::breakCycle(RenderSVGResourceContainer* resourceLeadingToCycle)
+void SVGResourcesCycleSolver::breakCycle(RenderSVGResourceContainer& resourceLeadingToCycle)
 {
-    ASSERT(resourceLeadingToCycle);
-    if (resourceLeadingToCycle == m_resources->linkedResource()) {
-        m_resources->resetLinkedResource();
+    if (&resourceLeadingToCycle == m_resources.linkedResource()) {
+        m_resources.resetLinkedResource();
         return;
     }
 
-    switch (resourceLeadingToCycle->resourceType()) {
+    switch (resourceLeadingToCycle.resourceType()) {
     case MaskerResourceType:
-        ASSERT(resourceLeadingToCycle == m_resources->masker());
-        m_resources->resetMasker();
+        ASSERT(&resourceLeadingToCycle == m_resources.masker());
+        m_resources.resetMasker();
         break;
     case MarkerResourceType:
-        ASSERT(resourceLeadingToCycle == m_resources->markerStart() || resourceLeadingToCycle == m_resources->markerMid() || resourceLeadingToCycle == m_resources->markerEnd());
-        if (m_resources->markerStart() == resourceLeadingToCycle)
-            m_resources->resetMarkerStart();
-        if (m_resources->markerMid() == resourceLeadingToCycle)
-            m_resources->resetMarkerMid();
-        if (m_resources->markerEnd() == resourceLeadingToCycle)
-            m_resources->resetMarkerEnd();
+        ASSERT(&resourceLeadingToCycle == m_resources.markerStart() || &resourceLeadingToCycle == m_resources.markerMid() || &resourceLeadingToCycle == m_resources.markerEnd());
+        if (m_resources.markerStart() == &resourceLeadingToCycle)
+            m_resources.resetMarkerStart();
+        if (m_resources.markerMid() == &resourceLeadingToCycle)
+            m_resources.resetMarkerMid();
+        if (m_resources.markerEnd() == &resourceLeadingToCycle)
+            m_resources.resetMarkerEnd();
         break;
     case PatternResourceType:
     case LinearGradientResourceType:
     case RadialGradientResourceType:
-        ASSERT(resourceLeadingToCycle == m_resources->fill() || resourceLeadingToCycle == m_resources->stroke());
-        if (m_resources->fill() == resourceLeadingToCycle)
-            m_resources->resetFill();
-        if (m_resources->stroke() == resourceLeadingToCycle)
-            m_resources->resetStroke();
+        ASSERT(&resourceLeadingToCycle == m_resources.fill() || &resourceLeadingToCycle == m_resources.stroke());
+        if (m_resources.fill() == &resourceLeadingToCycle)
+            m_resources.resetFill();
+        if (m_resources.stroke() == &resourceLeadingToCycle)
+            m_resources.resetStroke();
         break;
     case FilterResourceType:
 #if ENABLE(FILTERS)
-        ASSERT(resourceLeadingToCycle == m_resources->filter());
-        m_resources->resetFilter();
+        ASSERT(&resourceLeadingToCycle == m_resources.filter());
+        m_resources.resetFilter();
 #endif
         break;
     case ClipperResourceType:
-        ASSERT(resourceLeadingToCycle == m_resources->clipper());
-        m_resources->resetClipper();
+        ASSERT(&resourceLeadingToCycle == m_resources.clipper());
+        m_resources.resetClipper();
         break;
     case SolidColorResourceType:
         ASSERT_NOT_REACHED();
index 099f91c..5bac21e 100644 (file)
 
 namespace WebCore {
 
-class RenderObject;
+class RenderElement;
 class RenderSVGResourceContainer;
 class SVGResources;
 
 class SVGResourcesCycleSolver {
     WTF_MAKE_NONCOPYABLE(SVGResourcesCycleSolver);
 public:
-    SVGResourcesCycleSolver(RenderObject*, SVGResources*);
+    SVGResourcesCycleSolver(RenderElement&, SVGResources&);
     ~SVGResourcesCycleSolver();
 
     void resolveCycles();
 
 private:
-    bool resourceContainsCycles(RenderObject*) const;
-    void breakCycle(RenderSVGResourceContainer*);
+    bool resourceContainsCycles(RenderElement&) const;
+    void breakCycle(RenderSVGResourceContainer&);
 
-    RenderObject* m_renderer;
-    SVGResources* m_resources;
+    RenderElement& m_renderer;
+    SVGResources& m_resources;
     HashSet<RenderSVGResourceContainer*> m_allResources; 
 };