Modified: trunk/Source/WebCore/ChangeLog (158999 => 159000)
--- trunk/Source/WebCore/ChangeLog 2013-11-09 11:32:57 UTC (rev 158999)
+++ trunk/Source/WebCore/ChangeLog 2013-11-09 16:11:22 UTC (rev 159000)
@@ -1,5 +1,20 @@
2013-11-09 Andreas Kling <[email protected]>
+ 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 <[email protected]>
+
Beat SVGRenderSupport with the RenderElement stick.
<https://webkit.org/b/124102>
Modified: trunk/Source/WebCore/rendering/svg/SVGResourcesCache.cpp (158999 => 159000)
--- trunk/Source/WebCore/rendering/svg/SVGResourcesCache.cpp 2013-11-09 11:32:57 UTC (rev 158999)
+++ trunk/Source/WebCore/rendering/svg/SVGResourcesCache.cpp 2013-11-09 16:11:22 UTC (rev 159000)
@@ -50,15 +50,15 @@
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);
Modified: trunk/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp (158999 => 159000)
--- trunk/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp 2013-11-09 11:32:57 UTC (rev 158999)
+++ trunk/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp 2013-11-09 16:11:22 UTC (rev 159000)
@@ -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"
@@ -36,32 +38,27 @@
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 @@
// 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 @@
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 @@
#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 @@
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();
Modified: trunk/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h (158999 => 159000)
--- trunk/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h 2013-11-09 11:32:57 UTC (rev 158999)
+++ trunk/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h 2013-11-09 16:11:22 UTC (rev 159000)
@@ -25,24 +25,24 @@
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;
};
