Title: [144825] trunk
- Revision
- 144825
- Author
- schen...@chromium.org
- Date
- 2013-03-05 14:53:57 -0800 (Tue, 05 Mar 2013)
Log Message
Crash when ImageLoader deletes Element inside SVGImageElement
https://bugs.webkit.org/show_bug.cgi?id=111085
Reviewed by Abhishek Arya.
Source/WebCore:
Elements with ImageLoader objects associated with them may have their
final reference held by the ImageLoader (to allow events to be sent
and handled). Any call on Element that causes the ImageLoader to
dispatch events might then result in the final deref of the Element
itself, thus leaving all the Element's "this" pointers up the stack
pointing to invalid memory.
This change puts the deref of the Element on a timer so that, if the
deref is called via a method on Element, the call stack will unwind
before the deref occurs.
Test: svg/custom/image-with-attr-change-after-delete-crash.html
* loader/ImageLoader.cpp:
(WebCore::ImageLoader::ImageLoader): Initialize a timer
(WebCore::ImageLoader::updatedHasPendingEvent): Put deref of the
element on a oneShotTimer, with appropriate assertions and checks to
ensure we only ref/deref once.
(WebCore::ImageLoader::timerFired): Deref the element when the timer fires.
* loader/ImageLoader.h:
(ImageLoader): Define a timer for controlling deref of the element.
LayoutTests:
* svg/custom/image-with-attr-change-after-delete-crash-expected.txt: Added.
* svg/custom/image-with-attr-change-after-delete-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (144824 => 144825)
--- trunk/LayoutTests/ChangeLog 2013-03-05 22:48:54 UTC (rev 144824)
+++ trunk/LayoutTests/ChangeLog 2013-03-05 22:53:57 UTC (rev 144825)
@@ -1,3 +1,13 @@
+2013-03-05 Stephen Chenney <schen...@chromium.org>
+
+ Crash when ImageLoader deletes Element inside SVGImageElement
+ https://bugs.webkit.org/show_bug.cgi?id=111085
+
+ Reviewed by Abhishek Arya.
+
+ * svg/custom/image-with-attr-change-after-delete-crash-expected.txt: Added.
+ * svg/custom/image-with-attr-change-after-delete-crash.html: Added.
+
2013-03-05 Antoine Quint <grao...@apple.com>
Web Inspector: identify reflection layers in LayerTreeAgent
Added: trunk/LayoutTests/svg/custom/image-with-attr-change-after-delete-crash-expected.txt (0 => 144825)
--- trunk/LayoutTests/svg/custom/image-with-attr-change-after-delete-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/image-with-attr-change-after-delete-crash-expected.txt 2013-03-05 22:53:57 UTC (rev 144825)
@@ -0,0 +1 @@
+Test Passes when there is no crash
Added: trunk/LayoutTests/svg/custom/image-with-attr-change-after-delete-crash.html (0 => 144825)
--- trunk/LayoutTests/svg/custom/image-with-attr-change-after-delete-crash.html (rev 0)
+++ trunk/LayoutTests/svg/custom/image-with-attr-change-after-delete-crash.html 2013-03-05 22:53:57 UTC (rev 144825)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+ <body id="body">
+ <svg id="svgRoot" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg">
+ <image height="410" id="svgImage" width="503" x="0" y="0" xlink:href=""
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function forceGC() {
+ if (window.GCController)
+ GCController.collect();
+ else
+ gc();
+ }
+
+ svgImage = document.getElementById("svgImage");
+ docElement = document.getElementById("svgRoot");
+ newTextNode = document.createTextNode(" Test Passes");
+ setTimeout("crash()", 0);
+
+ function crash() {
+ svgImage.attributes[5].replaceChild(newTextNode, svgImage.attributes[5].childNodes[0]);
+ svgImage.parentNode.removeChild(svgImage);
+ delete svgImage;
+ forceGC();
+ docElement.appendChild(newTextNode);
+ if (window.testRunner) {
+ document.getElementById("body").innerHTML = "Test Passes when there is no crash";
+ testRunner.notifyDone();
+ }
+ }
+ </script>
+ </svg>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (144824 => 144825)
--- trunk/Source/WebCore/ChangeLog 2013-03-05 22:48:54 UTC (rev 144824)
+++ trunk/Source/WebCore/ChangeLog 2013-03-05 22:53:57 UTC (rev 144825)
@@ -1,3 +1,32 @@
+2013-03-05 Stephen Chenney <schen...@chromium.org>
+
+ Crash when ImageLoader deletes Element inside SVGImageElement
+ https://bugs.webkit.org/show_bug.cgi?id=111085
+
+ Reviewed by Abhishek Arya.
+
+ Elements with ImageLoader objects associated with them may have their
+ final reference held by the ImageLoader (to allow events to be sent
+ and handled). Any call on Element that causes the ImageLoader to
+ dispatch events might then result in the final deref of the Element
+ itself, thus leaving all the Element's "this" pointers up the stack
+ pointing to invalid memory.
+
+ This change puts the deref of the Element on a timer so that, if the
+ deref is called via a method on Element, the call stack will unwind
+ before the deref occurs.
+
+ Test: svg/custom/image-with-attr-change-after-delete-crash.html
+
+ * loader/ImageLoader.cpp:
+ (WebCore::ImageLoader::ImageLoader): Initialize a timer
+ (WebCore::ImageLoader::updatedHasPendingEvent): Put deref of the
+ element on a oneShotTimer, with appropriate assertions and checks to
+ ensure we only ref/deref once.
+ (WebCore::ImageLoader::timerFired): Deref the element when the timer fires.
+ * loader/ImageLoader.h:
+ (ImageLoader): Define a timer for controlling deref of the element.
+
2013-03-05 Antoine Quint <grao...@apple.com>
Web Inspector: identify reflection layers in LayerTreeAgent
Modified: trunk/Source/WebCore/loader/ImageLoader.cpp (144824 => 144825)
--- trunk/Source/WebCore/loader/ImageLoader.cpp 2013-03-05 22:48:54 UTC (rev 144824)
+++ trunk/Source/WebCore/loader/ImageLoader.cpp 2013-03-05 22:53:57 UTC (rev 144825)
@@ -92,6 +92,7 @@
ImageLoader::ImageLoader(Element* element)
: m_element(element)
, m_image(0)
+ , m_derefElementTimer(this, &ImageLoader::timerFired)
, m_hasPendingBeforeLoadEvent(false)
, m_hasPendingLoadEvent(false)
, m_hasPendingErrorEvent(false)
@@ -366,12 +367,22 @@
if (wasProtected == m_elementIsProtected)
return;
- if (m_elementIsProtected)
- m_element->ref();
- else
- m_element->deref();
+ if (m_elementIsProtected) {
+ if (m_derefElementTimer.isActive())
+ m_derefElementTimer.stop();
+ else
+ m_element->ref();
+ } else {
+ ASSERT(!m_derefElementTimer.isActive());
+ m_derefElementTimer.startOneShot(0);
+ }
}
+void ImageLoader::timerFired(Timer<ImageLoader>*)
+{
+ m_element->deref();
+}
+
void ImageLoader::dispatchPendingEvent(ImageEventSender* eventSender)
{
ASSERT(eventSender == &beforeLoadEventSender() || eventSender == &loadEventSender() || eventSender == &errorEventSender());
Modified: trunk/Source/WebCore/loader/ImageLoader.h (144824 => 144825)
--- trunk/Source/WebCore/loader/ImageLoader.h 2013-03-05 22:48:54 UTC (rev 144824)
+++ trunk/Source/WebCore/loader/ImageLoader.h 2013-03-05 22:53:57 UTC (rev 144825)
@@ -88,8 +88,11 @@
void setImageWithoutConsideringPendingLoadEvent(CachedImage*);
void clearFailedLoadURL();
+ void timerFired(Timer<ImageLoader>*);
+
Element* m_element;
CachedResourceHandle<CachedImage> m_image;
+ Timer<ImageLoader> m_derefElementTimer;
AtomicString m_failedLoadURL;
bool m_hasPendingBeforeLoadEvent : 1;
bool m_hasPendingLoadEvent : 1;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
