git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 52724fd)
Cyclic resources were not detected if the reference had deep containers
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Sep 2015 01:09:23 +0000 (01:09 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Sep 2015 01:09:23 +0000 (01:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149182

Reviewed by John Honeycutt.

Source/WebCore:

During our examination of the SVG rendering tree looking for cycles,
if a resource pointed to something that had a nested structure, and
one of the parent nodes in that structure was a container object
without resources itself, we were not looking into the children.

Test: svg/custom/pattern-content-cycle-w-resourceless-container.html

* rendering/svg/SVGResourcesCycleSolver.cpp:
(WebCore::SVGResourcesCycleSolver::resourceContainsCycles): We should still
check all children resources, but not exit early if there are none. Instead
we should recurse into any children.
(WebCore::SVGResourcesCycleSolver::resolveCycles): Changes to some debug
code that no longer compiled (it's still off by default, but at least
it will work now).

LayoutTests:

This test was ported from Blink. I believe it originally
came from:
https://code.google.com/p/chromium/issues/detail?id=351713

* svg/custom/pattern-content-cycle-w-resourceless-container-expected.txt: Added.
* svg/custom/pattern-content-cycle-w-resourceless-container.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 705ad26..7322565 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+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
+
+        Reviewed by John Honeycutt.
+
+        This test was ported from Blink. I believe it originally
+        came from:
+        https://code.google.com/p/chromium/issues/detail?id=351713
+
+        * svg/custom/pattern-content-cycle-w-resourceless-container-expected.txt: Added.
+        * svg/custom/pattern-content-cycle-w-resourceless-container.html: Added.
+
 2015-09-17  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add HTMLSlotElement and NonDocumentTypeChildNode.assignedSlot
diff --git a/LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container-expected.txt b/LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container-expected.txt
new file mode 100644 (file)
index 0000000..31b6673
--- /dev/null
+++ b/LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container-expected.txt
@@ -0,0 +1,3 @@
+PASS if no crash (stack overflow).
+
+
diff --git a/LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container.html b/LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container.html
new file mode 100644 (file)
index 0000000..41c5df2
--- /dev/null
+++ b/LayoutTests/svg/custom/pattern-content-cycle-w-resourceless-container.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+    window.onload = function() {
+        testRunner.notifyDone();
+    };
+}
+</script>
+<p>PASS if no crash (stack overflow).</p>
+<svg width="100" height="100">
+  <rect fill="url(#p1)" width="100" height="100"/>
+  <pattern id="p2" width="1" height="1">
+    <g>
+      <rect fill="url(#p1)" width="100" height="100"/>
+    </g>
+  </pattern>
+  <pattern id="p1" width="1" height="1">
+    <g>
+      <rect fill="url(#p2)" width="100" height="100"/>
+    </g>
+  </pattern>
+</svg>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 7e49b8e..edb5941 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+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
+
+        Reviewed by John Honeycutt.
+
+        During our examination of the SVG rendering tree looking for cycles,
+        if a resource pointed to something that had a nested structure, and
+        one of the parent nodes in that structure was a container object
+        without resources itself, we were not looking into the children.
+
+        Test: svg/custom/pattern-content-cycle-w-resourceless-container.html
+
+        * rendering/svg/SVGResourcesCycleSolver.cpp:
+        (WebCore::SVGResourcesCycleSolver::resourceContainsCycles): We should still
+        check all children resources, but not exit early if there are none. Instead
+        we should recurse into any children.
+        (WebCore::SVGResourcesCycleSolver::resolveCycles): Changes to some debug
+        code that no longer compiled (it's still off by default, but at least
+        it will work now).
+
 2015-09-17  Myles C. Maxfield  <mmaxfield@apple.com>
 
         REGRESSION(r188871): 50% regression in page load time of Wikipedia home page
diff --git a/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp b/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp
index 76e1fbd..13b88d2 100644 (file)
--- a/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp
+++ b/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp
@@ -54,7 +54,7 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) co
         HashSet<RenderSVGResourceContainer*> resourceSet;
         resources->buildSetOfResources(resourceSet);
 
-        // Walk all resources and check wheter they reference any resource contained in the resources set.
+        // Walk all resources and check whether they reference any resource contained in the resources set.
         for (auto* resource : resourceSet) {
             if (m_allResources.contains(resource))
                 return true;
@@ -65,18 +65,16 @@ 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)) {
-        auto* childResources = SVGResourcesCache::cachedResourcesForRenderer(child);
-        if (!childResources)
-            continue;
-        
-        // A child of the given 'resource' contains resources. 
-        HashSet<RenderSVGResourceContainer*> childResourceSet;
-        childResources->buildSetOfResources(childResourceSet);
-
-        // Walk all child resources and check wheter they reference any resource contained in the resources set.
-        for (auto* resource : childResourceSet) {
-            if (m_allResources.contains(resource))
-                return true;
+        if (auto* childResources = SVGResourcesCache::cachedResourcesForRenderer(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) {
+                if (m_allResources.contains(resource))
+                    return true;
+            }
         }
 
         // Walk children recursively, stop immediately if we found a cycle
@@ -107,15 +105,15 @@ void SVGResourcesCycleSolver::resolveCycles()
         ancestorResources.add(&resource);
 
 #if DEBUG_CYCLE_DETECTION > 0
-    fprintf(stderr, "\nDetecting wheter any resources references any of following objects:\n");
+    fprintf(stderr, "\nDetecting whether any resources references any of following objects:\n");
     {
         fprintf(stderr, "Local resources:\n");
-        for (auto* resource : localResources)
-            fprintf(stderr, "|> %s: object=%p (node=%p)\n", resource->renderName(), resource, resource->node());
+        for (RenderObject* resource : localResources)
+            fprintf(stderr, "|> %s : %p (node %p)\n", resource->renderName(), resource, resource->node());
 
         fprintf(stderr, "Parent resources:\n");
-        for (auto* resource : ancestorResources)
-            fprintf(stderr, "|> %s: object=%p (node=%p)\n", resource->renderName(), resource, resource->node());
+        for (RenderObject* resource : ancestorResources)
+            fprintf(stderr, "|> %s : %p (node %p)\n", resource->renderName(), resource, resource->node());
     }
 #endif
 
@@ -131,7 +129,7 @@ void SVGResourcesCycleSolver::resolveCycles()
     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.
+    // 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))
             breakCycle(*resource);
@@ -139,7 +137,7 @@ void SVGResourcesCycleSolver::resolveCycles()
 
 #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();
Mirror of the WebKit open source project from svn.webkit.org.