Title: [120559] trunk
- Revision
- 120559
- Author
- [email protected]
- Date
- 2012-06-17 19:12:17 -0700 (Sun, 17 Jun 2012)
Log Message
Prevent crash in SVGDocumentExtensions::removeAllElementReferencesForTarget.
https://bugs.webkit.org/show_bug.cgi?id=88144
Reviewed by Abhishek Arya.
Source/WebCore:
When iterating over referencing elements to rebuild after a reference change in
SVGDocumentExtensions::removeAllElementReferencesForTarget, we can
modify the underlying toBeNotified vector, invalidating it. This change checks
that a vector element is valid before rebuilding, preventing a crash.
Some definitions from SVGDocumentExtensions that may put this patch in context:
An example of a "referenced elements" is a <path>.
An example of a "referencing element" is a <textPath href=''>.
m_elementDependencies is a map from referenced elements (e.g., paths) to
a set of referencing elements (e.g., textPaths).
The check that the vector element is valid relies on checking if the referencing
element is in m_elementDependencies. This check is allowed because in the
destructor of SVGTextPathElement (and SVGFeImageElement),
removeAllTargetReferencesForElement() is called, removing the referencing element
from m_elementDependencies.
Simply checking if the referencing element is anywhere in m_elementDependencies
is enough to show it is valid, but that requires iterating over all referenced
elements to see if the given referencing element is present. This change
only checks if the textPath is still in the elements referencing the
path being removed, and only removes the referenced element from
m_elementDependencies after forcing the referencing elements to be rebuilt.
Test: svg/text/textpath-reference-crash.html
* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::removeAllElementReferencesForTarget):
LayoutTests:
* svg/text/textpath-reference-crash-expected.txt: Added.
* svg/text/textpath-reference-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (120558 => 120559)
--- trunk/LayoutTests/ChangeLog 2012-06-18 02:09:48 UTC (rev 120558)
+++ trunk/LayoutTests/ChangeLog 2012-06-18 02:12:17 UTC (rev 120559)
@@ -1,3 +1,13 @@
+2012-06-17 Philip Rogers <[email protected]>
+
+ Prevent crash in SVGDocumentExtensions::removeAllElementReferencesForTarget.
+ https://bugs.webkit.org/show_bug.cgi?id=88144
+
+ Reviewed by Abhishek Arya.
+
+ * svg/text/textpath-reference-crash-expected.txt: Added.
+ * svg/text/textpath-reference-crash.html: Added.
+
2012-06-17 Zan Dobersek <[email protected]>
Unreviewed GTK gardening, updating baselines after r120541.
Added: trunk/LayoutTests/svg/text/textpath-reference-crash-expected.txt (0 => 120559)
--- trunk/LayoutTests/svg/text/textpath-reference-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/text/textpath-reference-crash-expected.txt 2012-06-18 02:12:17 UTC (rev 120559)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/svg/text/textpath-reference-crash.html (0 => 120559)
--- trunk/LayoutTests/svg/text/textpath-reference-crash.html (rev 0)
+++ trunk/LayoutTests/svg/text/textpath-reference-crash.html 2012-06-18 02:12:17 UTC (rev 120559)
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+ <!-- Test for wkbug.com/88144 - Crash in SVGDocumentExtensions::removeAllElementReferencesForTarget. -->
+ <head>
+ <script>
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ window._onload_ = function() {
+ svg0 = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+ svg0.setAttribute('id', 'svg0');
+ document.body.appendChild(svg0);
+
+ document.body.appendChild(document.createTextNode('A'));
+
+ svg1 = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+ svg1.setAttribute('id', 'svg1');
+ svg1.appendChild(document.createTextNode('A'));
+ document.body.appendChild(svg1);
+
+ document.body.appendChild(document.createTextNode('A'));
+
+ image = document.createElementNS('http://www.w3.org/2000/svg', 'image');
+ image.setAttribute('id', 'image');
+ svg0.appendChild(image);
+
+ textPath = document.createElementNS('http://www.w3.org/2000/svg', 'textPath');
+ textPath.setAttributeNS('http://www.w3.org/1999/xlink', 'xlink:href', '#svg1');
+ textPath.setAttribute('id', 'textPath');
+ image.appendChild(textPath);
+
+ use = document.createElementNS('http://www.w3.org/2000/svg', 'use');
+ use.setAttributeNS('http://www.w3.org/1999/xlink', 'xlink:href', '#image');
+ use.setAttribute('id', 'use');
+ svg0.appendChild(use);
+
+ image.setAttributeNS('http://www.w3.org/1999/xlink', 'xlink:href', '#svg0');
+ document.designMode = 'on';
+ window.getSelection().setBaseAndExtent(svg1, 0, svg1, 0);
+ document.execCommand('ForwardDelete');
+
+ document.body.offsetTop;
+ document.body.innerHTML = "PASS";
+ }
+ </script>
+ </head>
+ <body>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (120558 => 120559)
--- trunk/Source/WebCore/ChangeLog 2012-06-18 02:09:48 UTC (rev 120558)
+++ trunk/Source/WebCore/ChangeLog 2012-06-18 02:12:17 UTC (rev 120559)
@@ -1,3 +1,39 @@
+2012-06-17 Philip Rogers <[email protected]>
+
+ Prevent crash in SVGDocumentExtensions::removeAllElementReferencesForTarget.
+ https://bugs.webkit.org/show_bug.cgi?id=88144
+
+ Reviewed by Abhishek Arya.
+
+ When iterating over referencing elements to rebuild after a reference change in
+ SVGDocumentExtensions::removeAllElementReferencesForTarget, we can
+ modify the underlying toBeNotified vector, invalidating it. This change checks
+ that a vector element is valid before rebuilding, preventing a crash.
+
+ Some definitions from SVGDocumentExtensions that may put this patch in context:
+ An example of a "referenced elements" is a <path>.
+ An example of a "referencing element" is a <textPath href=''>.
+ m_elementDependencies is a map from referenced elements (e.g., paths) to
+ a set of referencing elements (e.g., textPaths).
+
+ The check that the vector element is valid relies on checking if the referencing
+ element is in m_elementDependencies. This check is allowed because in the
+ destructor of SVGTextPathElement (and SVGFeImageElement),
+ removeAllTargetReferencesForElement() is called, removing the referencing element
+ from m_elementDependencies.
+
+ Simply checking if the referencing element is anywhere in m_elementDependencies
+ is enough to show it is valid, but that requires iterating over all referenced
+ elements to see if the given referencing element is present. This change
+ only checks if the textPath is still in the elements referencing the
+ path being removed, and only removes the referenced element from
+ m_elementDependencies after forcing the referencing elements to be rebuilt.
+
+ Test: svg/text/textpath-reference-crash.html
+
+ * svg/SVGDocumentExtensions.cpp:
+ (WebCore::SVGDocumentExtensions::removeAllElementReferencesForTarget):
+
2012-06-17 Yoshifumi Inoue <[email protected]>
Unreviewed, rolling out r120390.
Modified: trunk/Source/WebCore/svg/SVGDocumentExtensions.cpp (120558 => 120559)
--- trunk/Source/WebCore/svg/SVGDocumentExtensions.cpp 2012-06-18 02:09:48 UTC (rev 120558)
+++ trunk/Source/WebCore/svg/SVGDocumentExtensions.cpp 2012-06-18 02:12:17 UTC (rev 120559)
@@ -410,12 +410,17 @@
for (HashSet<SVGElement*>::iterator setIt = referencingElements->begin(); setIt != setEnd; ++setIt)
toBeNotified.append(*setIt);
- m_elementDependencies.remove(it);
-
// Force rebuilding the referencingElement so it knows about this change.
Vector<SVGElement*>::iterator vectorEnd = toBeNotified.end();
- for (Vector<SVGElement*>::iterator vectorIt = toBeNotified.begin(); vectorIt != vectorEnd; ++vectorIt)
- (*vectorIt)->svgAttributeChanged(XLinkNames::hrefAttr);
+ for (Vector<SVGElement*>::iterator vectorIt = toBeNotified.begin(); vectorIt != vectorEnd; ++vectorIt) {
+ // Before rebuilding referencingElement ensure it was not removed from under us.
+ if (HashSet<SVGElement*>* referencingElements = setOfElementsReferencingTarget(referencedElement)) {
+ if (referencingElements->contains(*vectorIt))
+ (*vectorIt)->svgAttributeChanged(XLinkNames::hrefAttr);
+ }
+ }
+
+ m_elementDependencies.remove(referencedElement);
}
#if ENABLE(SVG_FONTS)
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
