Diff
Modified: branches/safari-534-branch/LayoutTests/ChangeLog (87649 => 87650)
--- branches/safari-534-branch/LayoutTests/ChangeLog 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/LayoutTests/ChangeLog 2011-05-29 21:01:09 UTC (rev 87650)
@@ -1,3 +1,18 @@
+2011-05-29 Mark Rowe <[email protected]>
+
+ Merge r87628.
+
+ 2011-05-28 Alexey Proskuryakov <[email protected]>
+
+ Reviewed by Geoff Garen.
+
+ REGRESSION (r85375): Load event is sometimes lost when multiple image elements use the same URL
+ https://bugs.webkit.org/show_bug.cgi?id=61692
+ <rdar://problem/9488628>
+
+ * fast/dom/gc-image-element-2-expected.txt: Added.
+ * fast/dom/gc-image-element-2.html: Added.
+
2011-05-27 Mark Rowe <[email protected]>
Merge r87322.
Added: branches/safari-534-branch/LayoutTests/fast/dom/gc-image-element-2-expected.txt (0 => 87650)
--- branches/safari-534-branch/LayoutTests/fast/dom/gc-image-element-2-expected.txt (rev 0)
+++ branches/safari-534-branch/LayoutTests/fast/dom/gc-image-element-2-expected.txt 2011-05-29 21:01:09 UTC (rev 87650)
@@ -0,0 +1,14 @@
+Tests for image elements firing their load events even when they're not in the document. Should say "onload fired" ten times, and then "PASS".
+
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+onload fired...
+PASS
+
Added: branches/safari-534-branch/LayoutTests/fast/dom/gc-image-element-2.html (0 => 87650)
--- branches/safari-534-branch/LayoutTests/fast/dom/gc-image-element-2.html (rev 0)
+++ branches/safari-534-branch/LayoutTests/fast/dom/gc-image-element-2.html 2011-05-29 21:01:09 UTC (rev 87650)
@@ -0,0 +1,45 @@
+<p>Tests for image elements firing their load events even when they're not in the document. Should say "onload fired" ten times, and then "PASS".</p>
+<pre id="console"></pre>
+
+<script src=""
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+
+function $(id)
+{
+ return document.getElementById(id);
+}
+
+function log(s)
+{
+ $("console").appendChild(document.createTextNode(s + "\n"));
+}
+
+var imageCount = 0;
+
+function createImage()
+{
+ ++imageCount;
+ var image = new Image;
+ image.src = ""
+ image._onload_ = function () {
+ log("onload fired...");
+ gc();
+
+ --imageCount;
+ if (!imageCount) {
+ log("PASS");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+ };
+ image = null;
+}
+
+for (var i = 0; i < 10; ++i)
+ createImage();
+
+</script>
Modified: branches/safari-534-branch/Source/WebCore/ChangeLog (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/ChangeLog 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/ChangeLog 2011-05-29 21:01:09 UTC (rev 87650)
@@ -1,5 +1,62 @@
2011-05-29 Mark Rowe <[email protected]>
+ Merge r87628.
+
+ 2011-05-28 Alexey Proskuryakov <[email protected]>
+
+ Reviewed by Geoff Garen.
+
+ REGRESSION (r85375): Load event is sometimes lost when multiple image elements use the same URL
+ https://bugs.webkit.org/show_bug.cgi?id=61692
+ <rdar://problem/9488628>
+
+ Test: fast/dom/gc-image-element-2.html
+
+ Manually verified that tests from bug 59604 and from bug 40926 still pass.
+
+ The problem here was that HTMLImageElement::hasPendingActivity() could return false when
+ a load (or error) event was still expected to fire.
+
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::setRequest):
+ * loader/cache/CachedResource.h:
+ (WebCore::CachedResource::wasCanceled):
+ (WebCore::CachedResource::errorOccurred):
+ Track whether the load was canceled. We want to always notify clients of load outcome,
+ as that's the only way they could make intelligent decisions.
+
+ * dom/ScriptElement.cpp: (WebCore::ScriptElement::execute): Cached resource clients now
+ get a notifyFinished call on cancellation. Handle this case, where we don't need the
+ execute the script, but also don't need to fire an error event.
+
+ * html/HTMLImageElement.cpp: Moved hasPendingActivity() to header, since it's just a single
+ function call now.
+
+ * html/HTMLImageElement.h: (WebCore::HTMLImageElement::hasPendingActivity): There is a large
+ window between when CachedResource::isLoading() becomes false and events are queued.
+ ImageLoader::haveFiredLoadEvent() is a much better indication of whether we are expecting
+ an event to fire.
+
+ * html/HTMLLinkElement.cpp: (WebCore::HTMLLinkElement::onloadTimerFired): Again, don't do
+ anything on cancellation.
+
+ * loader/ImageLoader.cpp:
+ (WebCore::ImageEventSender::hasPendingEvents): Made it debug-only again, and fixed to
+ give an accurate result while looping over the list of events to dispatch.
+ (WebCore::ImageLoader::notifyFinished): Don't do anything when cancelled. We don't want to
+ switch to a broken image icon, or to dispatch events.
+ (WebCore::ImageEventSender::dispatchPendingEvents): Clear the current loader from dispatching
+ list, as the event is no longer pending when it's being dispatched.
+
+ * loader/ImageLoader.h: Removed unnecessary hasPendingLoadEvent(). We don't care whether one
+ is already pending, we only care if one is expected at some time in the future, and
+ !haveFiredLoadEvent() is our best idea of that.
+
+ * dom/XMLDocumentParser.cpp: (WebCore::XMLDocumentParser::notifyFinished): Another place to
+ handle cancellation.
+
+2011-05-29 Mark Rowe <[email protected]>
+
Merge r87634.
2011-05-28 Steve Falkenburg <[email protected]>
Modified: branches/safari-534-branch/Source/WebCore/dom/ScriptElement.cpp (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/dom/ScriptElement.cpp 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/dom/ScriptElement.cpp 2011-05-29 21:01:09 UTC (rev 87650)
@@ -295,7 +295,7 @@
ASSERT(cachedScript);
if (cachedScript->errorOccurred())
dispatchErrorEvent();
- else {
+ else if (!cachedScript->wasCanceled()) {
executeScript(ScriptSourceCode(cachedScript));
dispatchLoadEvent();
}
Modified: branches/safari-534-branch/Source/WebCore/dom/XMLDocumentParser.cpp (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/dom/XMLDocumentParser.cpp 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/dom/XMLDocumentParser.cpp 2011-05-29 21:01:09 UTC (rev 87650)
@@ -324,6 +324,7 @@
ScriptSourceCode sourceCode(m_pendingScript.get());
bool errorOccurred = m_pendingScript->errorOccurred();
+ bool wasCanceled = m_pendingScript->wasCanceled();
m_pendingScript->removeClient(this);
m_pendingScript = 0;
@@ -339,7 +340,7 @@
if (errorOccurred)
scriptElement->dispatchErrorEvent();
- else {
+ else if (!wasCanceled) {
scriptElement->executeScript(sourceCode);
scriptElement->dispatchLoadEvent();
}
Modified: branches/safari-534-branch/Source/WebCore/html/HTMLImageElement.cpp (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/html/HTMLImageElement.cpp 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/html/HTMLImageElement.cpp 2011-05-29 21:01:09 UTC (rev 87650)
@@ -385,11 +385,6 @@
return m_imageLoader.imageComplete();
}
-bool HTMLImageElement::hasPendingActivity()
-{
- return (cachedImage() && cachedImage()->isLoading()) || m_imageLoader.hasPendingLoadEvent();
-}
-
void HTMLImageElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const
{
HTMLElement::addSubresourceAttributeURLs(urls);
Modified: branches/safari-534-branch/Source/WebCore/html/HTMLImageElement.h (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/html/HTMLImageElement.h 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/html/HTMLImageElement.h 2011-05-29 21:01:09 UTC (rev 87650)
@@ -73,7 +73,7 @@
bool complete() const;
bool haveFiredLoadEvent() const { return m_imageLoader.haveFiredLoadEvent(); }
- bool hasPendingActivity();
+ bool hasPendingActivity() const { return !m_imageLoader.haveFiredLoadEvent(); }
virtual bool canContainRangeEndPoint() const { return false; }
Modified: branches/safari-534-branch/Source/WebCore/html/HTMLLinkElement.cpp (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/html/HTMLLinkElement.cpp 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/html/HTMLLinkElement.cpp 2011-05-29 21:01:09 UTC (rev 87650)
@@ -432,7 +432,7 @@
ASSERT_UNUSED(timer, timer == &m_onloadTimer);
if (m_cachedLinkResource->errorOccurred())
dispatchEvent(Event::create(eventNames().errorEvent, false, false));
- else
+ else if (!m_cachedLinkResource->wasCanceled())
dispatchEvent(Event::create(eventNames().loadEvent, false, false));
m_cachedLinkResource->removeClient(this);
Modified: branches/safari-534-branch/Source/WebCore/loader/ImageLoader.cpp (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/loader/ImageLoader.cpp 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/loader/ImageLoader.cpp 2011-05-29 21:01:09 UTC (rev 87650)
@@ -67,7 +67,12 @@
void dispatchPendingEvents();
- bool hasPendingEvents(ImageLoader* loader) { return m_dispatchSoonList.find(loader) != notFound; }
+#ifndef NDEBUG
+ bool hasPendingEvents(ImageLoader* loader) const
+ {
+ return m_dispatchSoonList.find(loader) != notFound || m_dispatchingList.find(loader) != notFound;
+ }
+#endif
private:
void timerFired(Timer<ImageEventSender>*);
@@ -208,9 +213,10 @@
updateFromElement();
}
-void ImageLoader::notifyFinished(CachedResource*)
+void ImageLoader::notifyFinished(CachedResource* resource)
{
ASSERT(m_failedLoadURL.isEmpty());
+ ASSERT_UNUSED(m_image, resource == m_image.get());
m_imageComplete = true;
if (haveFiredBeforeLoadEvent())
@@ -219,6 +225,9 @@
if (m_firedLoad)
return;
+ if (resource->wasCanceled())
+ return;
+
loadEventSender().dispatchEventSoon(this);
}
@@ -310,11 +319,6 @@
setImage(0);
}
-bool ImageLoader::hasPendingLoadEvent()
-{
- return loadEventSender().hasPendingEvents(this);
-}
-
ImageEventSender::ImageEventSender(const AtomicString& eventType)
: m_eventType(eventType)
, m_timer(this, &ImageEventSender::timerFired)
@@ -362,6 +366,7 @@
size_t size = m_dispatchingList.size();
for (size_t i = 0; i < size; ++i) {
if (ImageLoader* loader = m_dispatchingList[i]) {
+ m_dispatchingList[i] = 0;
if (m_eventType == eventNames().beforeloadEvent)
loader->dispatchPendingBeforeLoadEvent();
else
Modified: branches/safari-534-branch/Source/WebCore/loader/ImageLoader.h (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/loader/ImageLoader.h 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/loader/ImageLoader.h 2011-05-29 21:01:09 UTC (rev 87650)
@@ -58,7 +58,6 @@
bool haveFiredBeforeLoadEvent() const { return m_firedBeforeLoad; }
bool haveFiredLoadEvent() const { return m_firedLoad; }
- bool hasPendingLoadEvent();
static void dispatchPendingBeforeLoadEvents();
static void dispatchPendingLoadEvents();
Modified: branches/safari-534-branch/Source/WebCore/loader/cache/CachedResource.cpp (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/loader/cache/CachedResource.cpp 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/loader/cache/CachedResource.cpp 2011-05-29 21:01:09 UTC (rev 87650)
@@ -251,11 +251,13 @@
m_request = request;
// All loads finish with data(allDataReceived = true) or error(), except for
- // canceled loads, which silently set our request to 0. Be sure to set our
- // loading flag to false in that case, so we don't seem to continue loading
- // forever.
- if (!m_request)
+ // canceled loads, which silently set our request to 0. Be sure to notify our
+ // client in that case, so we don't seem to continue loading forever.
+ if (!m_request && isLoading()) {
setLoading(false);
+ setStatus(Canceled);
+ checkNotify();
+ }
if (canDelete() && !inCache())
delete this;
Modified: branches/safari-534-branch/Source/WebCore/loader/cache/CachedResource.h (87649 => 87650)
--- branches/safari-534-branch/Source/WebCore/loader/cache/CachedResource.h 2011-05-29 20:57:26 UTC (rev 87649)
+++ branches/safari-534-branch/Source/WebCore/loader/cache/CachedResource.h 2011-05-29 21:01:09 UTC (rev 87650)
@@ -73,6 +73,7 @@
Unknown, // let cache decide what to do with it
Pending, // only partially loaded
Cached, // regular case
+ Canceled,
LoadError,
DecodeError
};
@@ -182,7 +183,8 @@
String accept() const { return m_accept; }
void setAccept(const String& accept) { m_accept = accept; }
- bool errorOccurred() const { return (status() == LoadError || status() == DecodeError); }
+ bool wasCanceled() const { return m_status == Canceled; }
+ bool errorOccurred() const { return (m_status == LoadError || m_status == DecodeError); }
bool sendResourceLoadCallbacks() const { return m_sendResourceLoadCallbacks; }
