Title: [120096] trunk
Revision
120096
Author
beid...@apple.com
Date
2012-06-12 11:24:27 -0700 (Tue, 12 Jun 2012)

Log Message

<rdar://problem/11593686> and https://bugs.webkit.org/show_bug.cgi?id=88683
Garbage collection of an <img> element can cause reentrant event dispatch.

Reviewed by Darin Adler.

Source/WebCore:

The most straightforward solution is for ImageLoader to keep its Element alive
with ref/deref any time the Image is actually loading.

ImageLoader should always do this for all Elements, and if those Elements want/need
different behavior for when they are detached then they need to manually stop their
loads.

Tests: http/tests/loading/embed-image-load-outlives-gc-without-crashing.html
       http/tests/loading/image-input-type-outlives-gc-without-crashing.html
       http/tests/loading/image-load-outlives-gc-without-crashing.html
       http/tests/loading/object-image-load-outlives-gc-without-crashing.html
       http/tests/loading/svg-image-load-outlives-gc-without-crashing.html
       http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html

* loader/ImageLoader.cpp:
(WebCore::ImageLoader::ImageLoader):
(WebCore::ImageLoader::~ImageLoader):
(WebCore::ImageLoader::setImage):
(WebCore::ImageLoader::updateFromElement):
(WebCore::ImageLoader::notifyFinished):
(WebCore::ImageLoader::updatedHasPendingLoadEvent):
(WebCore::ImageLoader::dispatchPendingBeforeLoadEvent):
(WebCore::ImageLoader::dispatchPendingLoadEvent):
* loader/ImageLoader.h:
(ImageLoader):

LayoutTests:

New tests for all users of ImageLoader:
* http/tests/loading/embed-image-load-outlives-gc-without-crashing-expected.txt: Added.
* http/tests/loading/embed-image-load-outlives-gc-without-crashing.html: Added.
* http/tests/loading/image-input-type-outlives-gc-without-crashing-expected.txt: Added.
* http/tests/loading/image-input-type-outlives-gc-without-crashing.html: Added.
* http/tests/loading/image-load-outlives-gc-without-crashing-expected.txt: Added.
* http/tests/loading/image-load-outlives-gc-without-crashing.html: Added.
* http/tests/loading/object-image-load-outlives-gc-without-crashing-expected.txt: Added.
* http/tests/loading/object-image-load-outlives-gc-without-crashing.html: Added.
* http/tests/loading/resources/slowimage.php: Added.
* http/tests/loading/svg-image-load-outlives-gc-without-crashing-expected.txt: Added.
* http/tests/loading/svg-image-load-outlives-gc-without-crashing.html: Added.
* http/tests/loading/video-poster-image-load-outlives-gc-without-crashing-expected.txt: Added.
* http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html: Added.

Shared sub resource for all of those tests:
* http/tests/loading/resources/slowimage.php: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120095 => 120096)


--- trunk/LayoutTests/ChangeLog	2012-06-12 18:21:32 UTC (rev 120095)
+++ trunk/LayoutTests/ChangeLog	2012-06-12 18:24:27 UTC (rev 120096)
@@ -1,3 +1,28 @@
+2012-06-12  Brady Eidson  <beid...@apple.com>
+
+        <rdar://problem/11593686> and https://bugs.webkit.org/show_bug.cgi?id=88683
+        Garbage collection of an <img> element can cause reentrant event dispatch.
+
+        Reviewed by Darin Adler.
+
+        New tests for all users of ImageLoader:
+        * http/tests/loading/embed-image-load-outlives-gc-without-crashing-expected.txt: Added.
+        * http/tests/loading/embed-image-load-outlives-gc-without-crashing.html: Added.
+        * http/tests/loading/image-input-type-outlives-gc-without-crashing-expected.txt: Added.
+        * http/tests/loading/image-input-type-outlives-gc-without-crashing.html: Added.
+        * http/tests/loading/image-load-outlives-gc-without-crashing-expected.txt: Added.
+        * http/tests/loading/image-load-outlives-gc-without-crashing.html: Added.
+        * http/tests/loading/object-image-load-outlives-gc-without-crashing-expected.txt: Added.
+        * http/tests/loading/object-image-load-outlives-gc-without-crashing.html: Added.
+        * http/tests/loading/resources/slowimage.php: Added.
+        * http/tests/loading/svg-image-load-outlives-gc-without-crashing-expected.txt: Added.
+        * http/tests/loading/svg-image-load-outlives-gc-without-crashing.html: Added.
+        * http/tests/loading/video-poster-image-load-outlives-gc-without-crashing-expected.txt: Added.
+        * http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html: Added.
+
+        Shared sub resource for all of those tests:
+        * http/tests/loading/resources/slowimage.php: Added.
+
 2012-06-12  Ojan Vafai  <o...@chromium.org>
 
         Update some tests to better match actual results on the bots.

Added: trunk/LayoutTests/http/tests/loading/embed-image-load-outlives-gc-without-crashing-expected.txt (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/embed-image-load-outlives-gc-without-crashing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/embed-image-load-outlives-gc-without-crashing-expected.txt	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didFailLoadWithError
+This has an embed element representing an image. That embed element is wrapped in a div. It removes the div, forces garbage collection, and makes sure that the window load event does not fire. It also makes sure there is no crash.
+

Added: trunk/LayoutTests/http/tests/loading/embed-image-load-outlives-gc-without-crashing.html (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/embed-image-load-outlives-gc-without-crashing.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/embed-image-load-outlives-gc-without-crashing.html	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,53 @@
+<script>
+
+if (window.layoutTestController) {
+	layoutTestController.waitUntilDone();
+	layoutTestController.dumpAsText();
+}
+
+function loaded() {
+	// If the garbage collection causes the image load to stop and therefore causes the load event to fire on the main frame, we failed.
+	alert("FAIL: The load event fired");
+	
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+</script>
+<body _onload_="loaded();">
+
+This has an embed element representing an image.  That embed element is wrapped in a div.  It removes the div, forces garbage collection, and makes sure that the window load event does not fire.  It also makes sure there is no crash.<br>
+<div id="thediv">
+<embed type="image/gif" src=""
+</embed>
+</div>
+</body>
+<script>
+
+function finished() {
+	window.stop()
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+function forceGC() {
+    if (this.GCController)
+        GCController.collect();
+    else {
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+	}
+
+	setTimeout("finished();", 0);
+}
+
+function removeTheDiv() {
+	var element = window.document.getElementById("thediv");
+	element.parentNode.removeChild(element);
+	element = null;
+	setTimeout("forceGC();", 0);
+}
+
+setTimeout("removeTheDiv();", 0);
+
+</script>

Added: trunk/LayoutTests/http/tests/loading/image-input-type-outlives-gc-without-crashing-expected.txt (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/image-input-type-outlives-gc-without-crashing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/image-input-type-outlives-gc-without-crashing-expected.txt	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didFailLoadWithError
+This test has a form with an input type=image element in it. The form is wrapped in a div. It removes the div, forces garbage collection, and makes sure that the window load event does not fire. It also makes sure there is no crash.
+

Added: trunk/LayoutTests/http/tests/loading/image-input-type-outlives-gc-without-crashing.html (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/image-input-type-outlives-gc-without-crashing.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/image-input-type-outlives-gc-without-crashing.html	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,54 @@
+<script>
+
+if (window.layoutTestController) {
+	layoutTestController.waitUntilDone();
+	layoutTestController.dumpAsText();
+}
+
+function loaded() {
+	// If the garbage collection causes the image load to stop and therefore causes the load event to fire on the main frame, we failed.
+	alert("FAIL: The load event fired");
+	
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+</script>
+<body _onload_="loaded();">
+
+This test has a form with an input type=image element in it.  The form is wrapped in a div.  It removes the div, forces garbage collection, and makes sure that the window load event does not fire.  It also makes sure there is no crash.<br>
+<div id="thediv">
+<form>
+<input type="image" src=""
+</form>
+</div>
+</body>
+<script>
+
+function finished() {
+	window.stop()
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+function forceGC() {
+    if (this.GCController)
+        GCController.collect();
+    else {
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+	}
+
+	setTimeout("finished();", 0);
+}
+
+function removeTheDiv() {
+	var element = window.document.getElementById("thediv");
+	element.parentNode.removeChild(element);
+	element = null;
+	setTimeout("forceGC();", 0);
+}
+
+setTimeout("removeTheDiv();", 0);
+
+</script>

Added: trunk/LayoutTests/http/tests/loading/image-load-outlives-gc-without-crashing-expected.txt (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/image-load-outlives-gc-without-crashing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/image-load-outlives-gc-without-crashing-expected.txt	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didFailLoadWithError
+This test has an image inside a div. It removes the div, forces garbage collection, and makes sure that the window load event does not fire. It also makes sure there is no crash.
+

Added: trunk/LayoutTests/http/tests/loading/image-load-outlives-gc-without-crashing.html (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/image-load-outlives-gc-without-crashing.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/image-load-outlives-gc-without-crashing.html	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,52 @@
+<script>
+
+if (window.layoutTestController) {
+	layoutTestController.waitUntilDone();
+	layoutTestController.dumpAsText();
+}
+
+function loaded() {
+	// If the garbage collection causes the image load to stop and therefore causes the load event to fire on the main frame, we failed.
+	alert("FAIL: The load event fired");
+	
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+</script>
+<body _onload_="loaded();">
+
+This test has an image inside a div.  It removes the div, forces garbage collection, and makes sure that the window load event does not fire.  It also makes sure there is no crash.<br>
+<div id="thediv">
+<img src=""
+</div>
+</body>
+<script>
+
+function finished() {
+	window.stop()
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+function forceGC() {
+    if (this.GCController)
+        GCController.collect();
+    else {
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+	}
+
+	setTimeout("finished();", 0);
+}
+
+function removeTheDiv() {
+	var element = window.document.getElementById("thediv");
+	element.parentNode.removeChild(element);
+	element = null;
+	setTimeout("forceGC();", 0);
+}
+
+setTimeout("removeTheDiv();", 0);
+
+</script>

Added: trunk/LayoutTests/http/tests/loading/object-image-load-outlives-gc-without-crashing-expected.txt (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/object-image-load-outlives-gc-without-crashing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/object-image-load-outlives-gc-without-crashing-expected.txt	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didFailLoadWithError
+This has an object element representing an image. That object element is wrapped in a div. It removes the div, forces garbage collection, and makes sure that the window load event does not fire. It also makes sure there is no crash.
+

Added: trunk/LayoutTests/http/tests/loading/object-image-load-outlives-gc-without-crashing.html (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/object-image-load-outlives-gc-without-crashing.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/object-image-load-outlives-gc-without-crashing.html	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,53 @@
+<script>
+
+if (window.layoutTestController) {
+	layoutTestController.waitUntilDone();
+	layoutTestController.dumpAsText();
+}
+
+function loaded() {
+	// If the garbage collection causes the image load to stop and therefore causes the load event to fire on the main frame, we failed.
+	alert("FAIL: The load event fired");
+	
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+</script>
+<body _onload_="loaded();">
+
+This has an object element representing an image.  That object element is wrapped in a div.  It removes the div, forces garbage collection, and makes sure that the window load event does not fire.  It also makes sure there is no crash.<br>
+<div id="thediv">
+<object type="image/gif" data=""
+</object>
+</div>
+</body>
+<script>
+
+function finished() {
+	window.stop()
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+function forceGC() {
+    if (this.GCController)
+        GCController.collect();
+    else {
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+	}
+
+	setTimeout("finished();", 0);
+}
+
+function removeTheDiv() {
+	var element = window.document.getElementById("thediv");
+	element.parentNode.removeChild(element);
+	element = null;
+	setTimeout("forceGC();", 0);
+}
+
+setTimeout("removeTheDiv();", 0);
+
+</script>

Added: trunk/LayoutTests/http/tests/loading/resources/slowimage.php (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/resources/slowimage.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/resources/slowimage.php	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,5 @@
+<?php
+sleep(30);
+header('Cache-Control: no-cache, must-revalidate');
+header('Location: data:image/gif;base64,R0lGODlhAQABAJAAAMjIyAAAACwAAAAAAQABAAACAgQBADs%3D');
+?>

Added: trunk/LayoutTests/http/tests/loading/svg-image-load-outlives-gc-without-crashing-expected.txt (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/svg-image-load-outlives-gc-without-crashing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/svg-image-load-outlives-gc-without-crashing-expected.txt	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didFailLoadWithError
+This has an svg element that contains an svg image element. That svg element is wrapped in a div. It removes the div, forces garbage collection, and makes sure that the window load event does not fire. It also makes sure there is no crash.
+

Added: trunk/LayoutTests/http/tests/loading/svg-image-load-outlives-gc-without-crashing.html (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/svg-image-load-outlives-gc-without-crashing.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/svg-image-load-outlives-gc-without-crashing.html	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,54 @@
+<script>
+
+if (window.layoutTestController) {
+	layoutTestController.waitUntilDone();
+	layoutTestController.dumpAsText();
+}
+
+function loaded() {
+	// If the garbage collection causes the image load to stop and therefore causes the load event to fire on the main frame, we failed.
+	alert("FAIL: The load event fired");
+	
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+</script>
+<body _onload_="loaded();">
+
+This has an svg element that contains an svg image element.  That svg element is wrapped in a div.  It removes the div, forces garbage collection, and makes sure that the window load event does not fire.  It also makes sure there is no crash.<br>
+<div id="thediv">
+<svg>
+<image xlink:href="" />
+</svg>
+</div>
+</body>
+<script>
+
+function finished() {
+	window.stop()
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+function forceGC() {
+    if (this.GCController)
+        GCController.collect();
+    else {
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+	}
+
+	setTimeout("finished();", 0);
+}
+
+function removeTheDiv() {
+	var element = window.document.getElementById("thediv");
+	element.parentNode.removeChild(element);
+	element = null;
+	setTimeout("forceGC();", 0);
+}
+
+setTimeout("removeTheDiv();", 0);
+
+</script>

Added: trunk/LayoutTests/http/tests/loading/video-poster-image-load-outlives-gc-without-crashing-expected.txt (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/video-poster-image-load-outlives-gc-without-crashing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/video-poster-image-load-outlives-gc-without-crashing-expected.txt	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didFailLoadWithError
+This has a video element with an image for its poster frame. That video element is wrapped in a div. It removes the div, forces garbage collection, and makes sure that the window load event does not fire. It also makes sure there is no crash.
+

Added: trunk/LayoutTests/http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html (0 => 120096)


--- trunk/LayoutTests/http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html	2012-06-12 18:24:27 UTC (rev 120096)
@@ -0,0 +1,53 @@
+<script>
+
+if (window.layoutTestController) {
+	layoutTestController.waitUntilDone();
+	layoutTestController.dumpAsText();
+}
+
+function loaded() {
+	// If the garbage collection causes the image load to stop and therefore causes the load event to fire on the main frame, we failed.
+	alert("FAIL: The load event fired");
+	
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+</script>
+<body _onload_="loaded();">
+
+This has a video element with an image for its poster frame.  That video element is wrapped in a div.  It removes the div, forces garbage collection, and makes sure that the window load event does not fire.  It also makes sure there is no crash.<br>
+<div id="thediv">
+<video poster="resources/slowimage.php">
+</video>
+</div>
+</body>
+<script>
+
+function finished() {
+	window.stop()
+	if (window.layoutTestController)
+		layoutTestController.notifyDone();
+}
+
+function forceGC() {
+    if (this.GCController)
+        GCController.collect();
+    else {
+        for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC.
+            ({});
+	}
+
+	setTimeout("finished();", 0);
+}
+
+function removeTheDiv() {
+	var element = window.document.getElementById("thediv");
+	element.parentNode.removeChild(element);
+	element = null;
+	setTimeout("forceGC();", 0);
+}
+
+setTimeout("removeTheDiv();", 0);
+
+</script>

Modified: trunk/Source/WebCore/ChangeLog (120095 => 120096)


--- trunk/Source/WebCore/ChangeLog	2012-06-12 18:21:32 UTC (rev 120095)
+++ trunk/Source/WebCore/ChangeLog	2012-06-12 18:24:27 UTC (rev 120096)
@@ -1,3 +1,36 @@
+2012-06-12  Brady Eidson  <beid...@apple.com>
+
+        <rdar://problem/11593686> and https://bugs.webkit.org/show_bug.cgi?id=88683
+        Garbage collection of an <img> element can cause reentrant event dispatch.
+
+        Reviewed by Darin Adler.
+
+        The most straightforward solution is for ImageLoader to keep its Element alive
+        with ref/deref any time the Image is actually loading.
+
+        ImageLoader should always do this for all Elements, and if those Elements want/need
+        different behavior for when they are detached then they need to manually stop their
+        loads.
+
+        Tests: http/tests/loading/embed-image-load-outlives-gc-without-crashing.html
+               http/tests/loading/image-input-type-outlives-gc-without-crashing.html
+               http/tests/loading/image-load-outlives-gc-without-crashing.html
+               http/tests/loading/object-image-load-outlives-gc-without-crashing.html
+               http/tests/loading/svg-image-load-outlives-gc-without-crashing.html
+               http/tests/loading/video-poster-image-load-outlives-gc-without-crashing.html
+
+        * loader/ImageLoader.cpp:
+        (WebCore::ImageLoader::ImageLoader):
+        (WebCore::ImageLoader::~ImageLoader):
+        (WebCore::ImageLoader::setImage):
+        (WebCore::ImageLoader::updateFromElement):
+        (WebCore::ImageLoader::notifyFinished):
+        (WebCore::ImageLoader::updatedHasPendingLoadEvent):
+        (WebCore::ImageLoader::dispatchPendingBeforeLoadEvent):
+        (WebCore::ImageLoader::dispatchPendingLoadEvent):
+        * loader/ImageLoader.h:
+        (ImageLoader):
+
 2012-06-12  Shawn Singh  <shawnsi...@chromium.org>
 
         [chromium] Make damage tracking more robust to early exits

Modified: trunk/Source/WebCore/loader/ImageLoader.cpp (120095 => 120096)


--- trunk/Source/WebCore/loader/ImageLoader.cpp	2012-06-12 18:21:32 UTC (rev 120095)
+++ trunk/Source/WebCore/loader/ImageLoader.cpp	2012-06-12 18:24:27 UTC (rev 120096)
@@ -89,6 +89,7 @@
     , m_hasPendingErrorEvent(false)
     , m_imageComplete(true)
     , m_loadManually(false)
+    , m_elementIsProtected(false)
 {
 }
 
@@ -108,6 +109,11 @@
     ASSERT(m_hasPendingErrorEvent || !errorEventSender().hasPendingEvents(this));
     if (m_hasPendingErrorEvent)
         errorEventSender().cancelEvent(this);
+
+    // If the ImageLoader is being destroyed but it is still protecting its image-loading Element,
+    // remove that protection here.
+    if (m_elementIsProtected)
+        m_element->deref();
 }
 
 void ImageLoader::setImage(CachedImage* newImage)
@@ -137,6 +143,10 @@
 
     if (RenderImageResource* imageResource = renderImageResource())
         imageResource->resetAnimation();
+
+    // Only consider updating the protection ref-count of the Element immediately before returning
+    // from this function as doing so might result in the destruction of this ImageLoader.
+    updatedHasPendingLoadEvent();
 }
 
 void ImageLoader::updateFromElement()
@@ -218,6 +228,10 @@
 
     if (RenderImageResource* imageResource = renderImageResource())
         imageResource->resetAnimation();
+
+    // Only consider updating the protection ref-count of the Element immediately before returning
+    // from this function as doing so might result in the destruction of this ImageLoader.
+    updatedHasPendingLoadEvent();
 }
 
 void ImageLoader::updateFromElementIgnoringPreviousError()
@@ -257,6 +271,9 @@
 
     if (resource->wasCanceled()) {
         m_hasPendingLoadEvent = false;
+        // Only consider updating the protection ref-count of the Element immediately before returning
+        // from this function as doing so might result in the destruction of this ImageLoader.
+        updatedHasPendingLoadEvent();
         return;
     }
 
@@ -303,6 +320,24 @@
         imageResource->setCachedImage(m_image.get());
 }
 
+void ImageLoader::updatedHasPendingLoadEvent()
+{
+    // If an Element that does image loading is removed from the DOM the load event for the image is still observable.
+    // As long as the ImageLoader is actively loading, the Element itself needs to be ref'ed to keep it from being
+    // destroyed by DOM manipulation or garbage collection.
+    // If such an Element wishes for the load to stop when removed from the DOM it needs to stop the ImageLoader explicitly.
+
+    if (m_hasPendingLoadEvent == m_elementIsProtected)
+        return;
+
+    m_elementIsProtected = m_hasPendingLoadEvent;
+
+    if (m_elementIsProtected)
+        m_element->ref();
+    else
+        m_element->deref();
+}
+
 void ImageLoader::dispatchPendingEvent(ImageEventSender* eventSender)
 {
     ASSERT(eventSender == &beforeLoadEventSender() || eventSender == &loadEventSender() || eventSender == &errorEventSender());
@@ -338,6 +373,10 @@
     
     if (m_element->hasTagName(HTMLNames::objectTag))
         static_cast<HTMLObjectElement*>(m_element)->renderFallbackContent();
+
+    // Only consider updating the protection ref-count of the Element immediately before returning
+    // from this function as doing so might result in the destruction of this ImageLoader.
+    updatedHasPendingLoadEvent();
 }
 
 void ImageLoader::dispatchPendingLoadEvent()
@@ -350,6 +389,10 @@
         return;
     m_hasPendingLoadEvent = false;
     dispatchLoadEvent();
+
+    // Only consider updating the protection ref-count of the Element immediately before returning
+    // from this function as doing so might result in the destruction of this ImageLoader.
+    updatedHasPendingLoadEvent();
 }
 
 void ImageLoader::dispatchPendingErrorEvent()

Modified: trunk/Source/WebCore/loader/ImageLoader.h (120095 => 120096)


--- trunk/Source/WebCore/loader/ImageLoader.h	2012-06-12 18:21:32 UTC (rev 120095)
+++ trunk/Source/WebCore/loader/ImageLoader.h	2012-06-12 18:24:27 UTC (rev 120096)
@@ -75,6 +75,8 @@
     virtual void dispatchLoadEvent() = 0;
     virtual String sourceURI(const AtomicString&) const = 0;
 
+    void updatedHasPendingLoadEvent();
+
     void dispatchPendingBeforeLoadEvent();
     void dispatchPendingLoadEvent();
     void dispatchPendingErrorEvent();
@@ -90,6 +92,7 @@
     bool m_hasPendingErrorEvent : 1;
     bool m_imageComplete : 1;
     bool m_loadManually : 1;
+    bool m_elementIsProtected : 1;
 };
 
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to