2015-09-17 Dean Jackson <dino@apple.com>
+ Multi-hop reference cycles not detected.
+ https://bugs.webkit.org/show_bug.cgi?id=149181
+
+ Reviewed by John Honeycutt.
+
+ SVG's cycle detection was not picking up a
+ case where an element was drawing a pattern, that
+ referenced another pattern, that referenced another
+ pattern, that referenced the original pattern.
+
+ The issue was that we were forgetting to check the
+ children of the renderer itself, rather than just
+ the children of the referenced renderers.
+
+ Found by running a test from Blink.
+
+ I also took the opportunity to clean up the debugging
+ code that logs cycle detection.
+
+ Test: svg/custom/pattern-3-step-cycle.html
+
+ * platform/Logging.h: Add a new SVG channel. I can't believe we
+ didn't already have one!
+ * rendering/svg/SVGResourcesCycleSolver.cpp:
+ (WebCore::SVGResourcesCycleSolver::resourceContainsCycles): Check the referenced
+ resources for cycles.
+ (WebCore::SVGResourcesCycleSolver::resolveCycles): Logging update.
+
+2015-09-17 Dean Jackson <dino@apple.com>
+
Cyclic resources were not detected if the reference had deep containers
https://bugs.webkit.org/show_bug.cgi?id=149182
#include "config.h"
#include "SVGResourcesCycleSolver.h"
-// Set to a value > 0, to debug the resource cache.
-#define DEBUG_CYCLE_DETECTION 0
-
+#include "Logging.h"
#include "RenderAncestorIterator.h"
#include "RenderSVGResourceClipper.h"
#include "RenderSVGResourceFilter.h"
#include "SVGResources.h"
#include "SVGResourcesCache.h"
+// Set to truthy value to debug the resource cache.
+#define DEBUG_CYCLE_DETECTION 0
+
+#if DEBUG_CYCLE_DETECTION
+#define LOG_DEBUG_CYCLE(...) LOG(SVG, __VA_ARGS__)
+#else
+#define LOG_DEBUG_CYCLE(...) ((void)0)
+#endif
+
namespace WebCore {
SVGResourcesCycleSolver::SVGResourcesCycleSolver(RenderElement& renderer, SVGResources& resources)
bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) const
{
+ LOG_DEBUG_CYCLE("\n(%p) Check for cycles\n", &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)"/>
HashSet<RenderSVGResourceContainer*> resourceSet;
resources->buildSetOfResources(resourceSet);
+ LOG_DEBUG_CYCLE("(%p) Examine our cached resources\n", &renderer);
+
// Walk all resources and check whether they reference any resource contained in the resources set.
for (auto* resource : resourceSet) {
+ LOG_DEBUG_CYCLE("(%p) Check %p\n", &renderer, resource);
if (m_allResources.contains(resource))
return true;
+
+ // Now check if the resources themselves contain cycles.
+ if (resourceContainsCycles(*resource))
+ return true;
}
}
+ LOG_DEBUG_CYCLE("(%p) Now the children renderers\n", &renderer);
+
// 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 (auto& child : childrenOfType<RenderElement>(renderer)) {
+
+ LOG_DEBUG_CYCLE("(%p) Checking child %p\n", &renderer, &child);
+
if (auto* childResources = SVGResourcesCache::cachedResourcesForRenderer(child)) {
+
+ LOG_DEBUG_CYCLE("(%p) Child %p had cached resources. Check them.\n", &renderer, &child);
+
// A child of the given 'resource' contains resources.
HashSet<RenderSVGResourceContainer*> childResourceSet;
childResources->buildSetOfResources(childResourceSet);
// Walk all child resources and check whether they reference any resource contained in the resources set.
for (auto* resource : childResourceSet) {
+ LOG_DEBUG_CYCLE("(%p) Child %p had resource %p\n", &renderer, &child, resource);
if (m_allResources.contains(resource))
return true;
}
}
+ LOG_DEBUG_CYCLE("(%p) Recurse into child %p\n", &renderer, &child);
+
// Walk children recursively, stop immediately if we found a cycle
if (resourceContainsCycles(child))
return true;
+
+ LOG_DEBUG_CYCLE("\n(%p) Child %p was ok\n", &renderer, &child);
}
+ LOG_DEBUG_CYCLE("\n(%p) No cycles found\n", &renderer);
+
return false;
}
{
ASSERT(m_allResources.isEmpty());
-#if DEBUG_CYCLE_DETECTION > 0
- fprintf(stderr, "\nBefore cycle detection:\n");
+#if DEBUG_CYCLE_DETECTION
+ LOG_DEBUG_CYCLE("\nBefore cycle detection:\n");
m_resources.dump(&m_renderer);
#endif
for (auto& resource : ancestorsOfType<RenderSVGResourceContainer>(m_renderer))
ancestorResources.add(&resource);
-#if DEBUG_CYCLE_DETECTION > 0
- fprintf(stderr, "\nDetecting whether any resources references any of following objects:\n");
+#if DEBUG_CYCLE_DETECTION
+ LOG_DEBUG_CYCLE("\nDetecting whether any resources references any of following objects:\n");
{
- fprintf(stderr, "Local resources:\n");
+ LOG_DEBUG_CYCLE("Local resources:\n");
for (RenderObject* resource : localResources)
- fprintf(stderr, "|> %s : %p (node %p)\n", resource->renderName(), resource, resource->node());
+ LOG_DEBUG_CYCLE("|> %s : %p (node %p)\n", resource->renderName(), resource, resource->node());
fprintf(stderr, "Parent resources:\n");
for (RenderObject* resource : ancestorResources)
- fprintf(stderr, "|> %s : %p (node %p)\n", resource->renderName(), resource, resource->node());
+ LOG_DEBUG_CYCLE("|> %s : %p (node %p)\n", resource->renderName(), resource, resource->node());
}
#endif
ASSERT(!m_allResources.isEmpty());
+#if DEBUG_CYCLE_DETECTION
+ LOG_DEBUG_CYCLE("\nAll resources:\n");
+ for (auto* resource : m_allResources)
+ LOG_DEBUG_CYCLE("- %p\n", resource);
+#endif
+
// The job of this function is to determine wheter any of the 'resources' associated with the given 'renderer'
// references us (or whether any of its kids references us) -> that's a cycle, we need to find and break it.
for (auto* resource : localResources) {
- if (ancestorResources.contains(resource) || resourceContainsCycles(*resource))
+ if (ancestorResources.contains(resource) || resourceContainsCycles(*resource)) {
+ LOG_DEBUG_CYCLE("\n**** Detected a cycle (see the last test in the output above) ****\n");
breakCycle(*resource);
+ }
}
-#if DEBUG_CYCLE_DETECTION > 0
- fprintf(stderr, "\nAfter cycle detection:\n");
+#if DEBUG_CYCLE_DETECTION
+ LOG_DEBUG_CYCLE("\nAfter cycle detection:\n");
m_resources.dump(&m_renderer);
#endif
git://git.webkit.org / WebKit-https.git / commitdiff