Reference cycles during SVG dependency invalidation
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Oct 2015 23:43:13 +0000 (23:43 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Oct 2015 23:43:13 +0000 (23:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149824
<rdar://problem/22771412>

Reviewed by Tim Horton.

Source/WebCore:

Detect any reference cycles as we are invalidating.

This is mostly a merge of the following Blink commit:
https://chromium.googlesource.com/chromium/blink/+/a4bc83453bda89823b672877dc02247652a02d51

Test: svg/custom/reference-cycle.svg

* rendering/svg/RenderSVGResource.cpp:
(WebCore::removeFromCacheAndInvalidateDependencies): Keep around a hash
table of dependencies, so that we can detect if an element is already
present before marking it.

LayoutTests:

Adding a test that has a cycle between feImage resources.

Merge Blink commit:
https://chromium.googlesource.com/chromium/blink/+/a4bc83453bda89823b672877dc02247652a02d51

* svg/custom/reference-cycle-expected.txt: Added.
* svg/custom/reference-cycle.svg: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/reference-cycle-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/reference-cycle.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGResource.cpp

index 68dff64..93c8c3e 100644 (file)
@@ -1,3 +1,19 @@
+2015-10-05  Dean Jackson  <dino@apple.com>
+
+        Reference cycles during SVG dependency invalidation
+        https://bugs.webkit.org/show_bug.cgi?id=149824
+        <rdar://problem/22771412>
+
+        Reviewed by Tim Horton.
+
+        Adding a test that has a cycle between feImage resources.
+
+        Merge Blink commit:
+        https://chromium.googlesource.com/chromium/blink/+/a4bc83453bda89823b672877dc02247652a02d51
+
+        * svg/custom/reference-cycle-expected.txt: Added.
+        * svg/custom/reference-cycle.svg: Added.
+
 2015-10-05  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking compositing/video/video-poster.html as flaky for El Capitan.
diff --git a/LayoutTests/svg/custom/reference-cycle-expected.txt b/LayoutTests/svg/custom/reference-cycle-expected.txt
new file mode 100644 (file)
index 0000000..9b9c972
--- /dev/null
@@ -0,0 +1 @@
+PASS: did not crash.
diff --git a/LayoutTests/svg/custom/reference-cycle.svg b/LayoutTests/svg/custom/reference-cycle.svg
new file mode 100644 (file)
index 0000000..4b88460
--- /dev/null
@@ -0,0 +1,19 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <defs>
+    <filter>
+      <feImage id="self" xlink:href="#self"/>
+    </filter>
+
+    <filter>
+      <feImage id="indirect_1" xlink:href="#indirect_2"/>
+      <feImage id="indirect_2" xlink:href="#indirect_1"/>
+    </filter>
+  </defs>
+
+  <text y="100">PASS: did not crash.</text>
+
+  <script>
+    if (window.testRunner)
+      testRunner.dumpAsText();
+  </script>
+</svg>
index 40ea586..9612c9e 100644 (file)
@@ -1,3 +1,23 @@
+2015-10-05  Dean Jackson  <dino@apple.com>
+
+        Reference cycles during SVG dependency invalidation
+        https://bugs.webkit.org/show_bug.cgi?id=149824
+        <rdar://problem/22771412>
+
+        Reviewed by Tim Horton.
+
+        Detect any reference cycles as we are invalidating.
+
+        This is mostly a merge of the following Blink commit:
+        https://chromium.googlesource.com/chromium/blink/+/a4bc83453bda89823b672877dc02247652a02d51
+
+        Test: svg/custom/reference-cycle.svg
+
+        * rendering/svg/RenderSVGResource.cpp:
+        (WebCore::removeFromCacheAndInvalidateDependencies): Keep around a hash
+        table of dependencies, so that we can detect if an element is already
+        present before marking it.
+
 2015-10-05  Jiewen Tan  <jiewen_tan@apple.com>
 
         Fix null pointer dereference in WebSocket::connect()        
index 111dc57..699e544 100644 (file)
@@ -172,9 +172,20 @@ static inline void removeFromCacheAndInvalidateDependencies(RenderElement& rende
     HashSet<SVGElement*>* dependencies = renderer.document().accessSVGExtensions().setOfElementsReferencingTarget(downcast<SVGElement>(renderer.element()));
     if (!dependencies)
         return;
+
+    // We allow cycles in SVGDocumentExtensions reference sets in order to avoid expensive
+    // reference graph adjustments on changes, so we need to break possible cycles here.
+    static NeverDestroyed<HashSet<SVGElement*>> invalidatingDependencies;
+
     for (auto* element : *dependencies) {
-        if (auto* renderer = element->renderer())
+        if (auto* renderer = element->renderer()) {
+            if (UNLIKELY(!invalidatingDependencies.get().add(element).isNewEntry)) {
+                // Reference cycle: we are in process of invalidating this dependant.
+                continue;
+            }
             RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer, needsLayout);
+            invalidatingDependencies.get().remove(element);
+        }
     }
 }