Title: [100046] trunk
- Revision
- 100046
- Author
- [email protected]
- Date
- 2011-11-11 16:34:57 -0800 (Fri, 11 Nov 2011)
Log Message
Null deref when no use element exists for SVG element instance
https://bugs.webkit.org/show_bug.cgi?id=59136
Second attempt, with a fix to handle JS garbage collection.
Patch by Stephen Chenney <[email protected]> on 2011-11-11
Reviewed by Nikolas Zimmermann.
Source/WebCore:
Test: svg/custom/element-instance-held-by-js-crash.svg
* svg/SVGElementInstance.cpp:
(WebCore::SVGElementInstance::~SVGElementInstance): Added call to detach() to clear
anything not yet cleared.
(WebCore::SVGElementInstance::detach): New method to replace old clear methods. This one
clears all the pointers it can, and removes the instance from the corresponding elements
instance list.
* svg/SVGElementInstance.h: Removed clear methods and replaced with detach.
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::detachInstance): Modified calls to clean up an SVGElementInstance.
LayoutTests:
* svg/custom/element-instance-held-by-js-crash-expected.txt: Added.
* svg/custom/element-instance-held-by-js-crash.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (100045 => 100046)
--- trunk/LayoutTests/ChangeLog 2011-11-12 00:30:05 UTC (rev 100045)
+++ trunk/LayoutTests/ChangeLog 2011-11-12 00:34:57 UTC (rev 100046)
@@ -1,3 +1,15 @@
+2011-11-11 Stephen Chenney <[email protected]>
+
+ Null deref when no use element exists for SVG element instance
+ https://bugs.webkit.org/show_bug.cgi?id=59136
+
+ Second attempt, with a fix to handle JS garbage collection.
+
+ Reviewed by Nikolas Zimmermann.
+
+ * svg/custom/element-instance-held-by-js-crash-expected.txt: Added.
+ * svg/custom/element-instance-held-by-js-crash.svg: Added.
+
2011-11-11 Florin Malita <[email protected]>
clipPath does not work on foreignObject
Added: trunk/LayoutTests/svg/custom/element-instance-held-by-js-crash-expected.txt (0 => 100046)
--- trunk/LayoutTests/svg/custom/element-instance-held-by-js-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/element-instance-held-by-js-crash-expected.txt 2011-11-12 00:34:57 UTC (rev 100046)
@@ -0,0 +1,2 @@
+PASS - Null corresponding element dereference does not crash.
+
Added: trunk/LayoutTests/svg/custom/element-instance-held-by-js-crash.svg (0 => 100046)
--- trunk/LayoutTests/svg/custom/element-instance-held-by-js-crash.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/element-instance-held-by-js-crash.svg 2011-11-12 00:34:57 UTC (rev 100046)
@@ -0,0 +1,27 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <script>
+ <![CDATA[
+ window._onload_ = function() {
+ // Grab a reference to an SVGElementInstance native object. This reference will prevent the
+ // object from deletion when the shadow DOM is removed due to a style change.
+ instance = document.getElementById("use_elem").instanceRoot;
+
+ // Setting an attribute forces re-creation of the shadow DOM
+ document.getElementById("circleID").setAttribute("cx", 30);
+
+ // The animate element tries to modify the element, which tries to update the
+ // instances in the circle, which crashes if it holds a pointer to a non-existent element.
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ }
+ //]]>
+ </script>
+ <circle transform="translate(1)" id="circleID" fill="green" cy="15" cx="15" r="10" >
+ <animate attributeName="cy" />
+ </circle>
+ <text id="resultText" y="20" x="50" >
+ PASS - Null corresponding element dereference does not crash.
+ </text>
+ <use id="use_elem" xlink:href="" />
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (100045 => 100046)
--- trunk/Source/WebCore/ChangeLog 2011-11-12 00:30:05 UTC (rev 100045)
+++ trunk/Source/WebCore/ChangeLog 2011-11-12 00:34:57 UTC (rev 100046)
@@ -1,3 +1,24 @@
+2011-11-11 Stephen Chenney <[email protected]>
+
+ Null deref when no use element exists for SVG element instance
+ https://bugs.webkit.org/show_bug.cgi?id=59136
+
+ Second attempt, with a fix to handle JS garbage collection.
+
+ Reviewed by Nikolas Zimmermann.
+
+ Test: svg/custom/element-instance-held-by-js-crash.svg
+
+ * svg/SVGElementInstance.cpp:
+ (WebCore::SVGElementInstance::~SVGElementInstance): Added call to detach() to clear
+ anything not yet cleared.
+ (WebCore::SVGElementInstance::detach): New method to replace old clear methods. This one
+ clears all the pointers it can, and removes the instance from the corresponding elements
+ instance list.
+ * svg/SVGElementInstance.h: Removed clear methods and replaced with detach.
+ * svg/SVGUseElement.cpp:
+ (WebCore::SVGUseElement::detachInstance): Modified calls to clean up an SVGElementInstance.
+
2011-11-11 Florin Malita <[email protected]>
clipPath does not work on foreignObject
Modified: trunk/Source/WebCore/svg/SVGElementInstance.cpp (100045 => 100046)
--- trunk/Source/WebCore/svg/SVGElementInstance.cpp 2011-11-12 00:30:05 UTC (rev 100045)
+++ trunk/Source/WebCore/svg/SVGElementInstance.cpp 2011-11-12 00:34:57 UTC (rev 100046)
@@ -61,18 +61,31 @@
SVGElementInstance::~SVGElementInstance()
{
+ // Call detach because we may be deleted directly if we are a child of a detached instance.
+ detach();
+
#ifndef NDEBUG
instanceCounter.decrement();
#endif
- // Deregister as instance for passed element.
- m_element->removeInstanceMapping(this);
-
- clearChildren();
+ m_element = 0;
}
-void SVGElementInstance::clearChildren()
+void SVGElementInstance::detach()
{
+ // Clear all pointers. When the node is detached from the shadow DOM it should be removed but,
+ // due to ref counting, it may not be. So clear everything to avoid dangling pointers.
+
+ // Deregister as instance for passed element, if we haven't already.
+ if (m_element->instancesForElement().contains(this))
+ m_element->removeInstanceMapping(this);
+ // DO NOT clear ref to m_element because _javascript_Core uses it for garbage collection
+
+ m_shadowTreeElement = 0;
+
+ m_directUseElement = 0;
+ m_correspondingUseElement = 0;
+
removeAllChildrenInContainer<SVGElementInstance, SVGElementInstance>(this);
}
Modified: trunk/Source/WebCore/svg/SVGElementInstance.h (100045 => 100046)
--- trunk/Source/WebCore/svg/SVGElementInstance.h 2011-11-12 00:30:05 UTC (rev 100045)
+++ trunk/Source/WebCore/svg/SVGElementInstance.h 2011-11-12 00:34:57 UTC (rev 100046)
@@ -60,13 +60,9 @@
SVGUseElement* correspondingUseElement() const { return m_correspondingUseElement; }
SVGUseElement* directUseElement() const { return m_directUseElement; }
SVGElement* shadowTreeElement() const { return m_shadowTreeElement.get(); }
- void clearChildren();
- void clearUseElements()
- {
- m_directUseElement = 0;
- m_correspondingUseElement = 0;
- }
+ void detach();
+
SVGElementInstance* parentNode() const { return parent(); }
PassRefPtr<SVGElementInstanceList> childNodes();
Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (100045 => 100046)
--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2011-11-12 00:30:05 UTC (rev 100045)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2011-11-12 00:34:57 UTC (rev 100046)
@@ -624,8 +624,7 @@
{
if (!m_targetElementInstance)
return;
- m_targetElementInstance->clearUseElements();
- m_targetElementInstance->clearChildren();
+ m_targetElementInstance->detach();
m_targetElementInstance = 0;
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
