When <img crossorigin> fails the CORS check, we should fire the error event

https://bugs.webkit.org/show_bug.cgi?id=81998

Reviewed by Nate Chapin.

Source/WebCore:

The spec says we're supposed to fire the error event when the CORS
check fails, but we haven't been.  This patch is larger than it might
otherwise be because we're firing the event asynchronously, but that
seems like the right thing to do.

Tests: http/tests/security/img-with-failed-cors-check-fails-to-load.html

* dom/Document.cpp:
(WebCore::Document::implicitClose):
* loader/ImageLoader.cpp:
(WebCore::errorEventSender):
(WebCore):
(WebCore::ImageLoader::ImageLoader):
(WebCore::ImageLoader::~ImageLoader):
(WebCore::ImageLoader::setImage):
(WebCore::ImageLoader::updateFromElement):
(WebCore::ImageLoader::notifyFinished):
(WebCore::ImageLoader::dispatchPendingEvent):
(WebCore::ImageLoader::dispatchPendingErrorEvent):
(WebCore::ImageLoader::dispatchPendingErrorEvents):
* loader/ImageLoader.h:
(ImageLoader):

LayoutTests:

Check that we're firing the error event in this case.

* http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt:
* http/tests/security/img-with-failed-cors-check-fails-to-load.html:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@112190 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d2b7d89..124acee 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2012-03-26  Adam Barth  <abarth@webkit.org>
+
+        When <img crossorigin> fails the CORS check, we should fire the error event
+        https://bugs.webkit.org/show_bug.cgi?id=81998
+
+        Reviewed by Nate Chapin.
+
+        Check that we're firing the error event in this case.
+
+        * http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt:
+        * http/tests/security/img-with-failed-cors-check-fails-to-load.html:
+
 2012-03-26  Ryosuke Niwa  <rniwa@webkit.org>
 
         Rebaseline after r112177.

diff --git a/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt b/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt
index 21666ca..02a25c9 100644 (file)
--- a/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt
+++ b/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt
@@ -1,2 +1,3 @@
 CONSOLE MESSAGE: Cross-origin image load denied by Cross-Origin Resource Sharing policy.
+ALERT: PASS: The error event was called.
 This test passes if the image below does not load. 

diff --git a/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load.html b/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load.html
index 8c67a18..485c693 100644 (file)
--- a/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load.html
+++ b/LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load.html
@@ -10,6 +10,10 @@ img.addEventListener('load', function(event) {
     alert('FAIL: The image loaded.');
 }, false);
 
+img.addEventListener('error', function(event) {
+    alert('PASS: The error event was called.');
+}, false);
+
 img.crossOrigin = "";
 img.src = "http://localhost:8080/security/resources/red200x100.png";
 document.body.appendChild(img);

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6171c52..a5c3de8 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2012-03-26  Adam Barth  <abarth@webkit.org>
+
+        When <img crossorigin> fails the CORS check, we should fire the error event
+        https://bugs.webkit.org/show_bug.cgi?id=81998
+
+        Reviewed by Nate Chapin.
+
+        The spec says we're supposed to fire the error event when the CORS
+        check fails, but we haven't been.  This patch is larger than it might
+        otherwise be because we're firing the event asynchronously, but that
+        seems like the right thing to do.
+
+        Tests: http/tests/security/img-with-failed-cors-check-fails-to-load.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::implicitClose):
+        * loader/ImageLoader.cpp:
+        (WebCore::errorEventSender):
+        (WebCore):
+        (WebCore::ImageLoader::ImageLoader):
+        (WebCore::ImageLoader::~ImageLoader):
+        (WebCore::ImageLoader::setImage):
+        (WebCore::ImageLoader::updateFromElement):
+        (WebCore::ImageLoader::notifyFinished):
+        (WebCore::ImageLoader::dispatchPendingEvent):
+        (WebCore::ImageLoader::dispatchPendingErrorEvent):
+        (WebCore::ImageLoader::dispatchPendingErrorEvents):
+        * loader/ImageLoader.h:
+        (ImageLoader):
+
 2012-03-26  Stephen White  <senorblanco@chromium.org>
 
         Make filters and the threaded compositor play well together.

diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index eb05795..15b307f 100644 (file)
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -2317,6 +2317,7 @@ void Document::implicitClose()
 
     ImageLoader::dispatchPendingBeforeLoadEvents();
     ImageLoader::dispatchPendingLoadEvents();
+    ImageLoader::dispatchPendingErrorEvents();
 
     HTMLLinkElement::dispatchPendingLoadEvents();
     HTMLStyleElement::dispatchPendingLoadEvents();

diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
index 842175b..19da987 100644 (file)
--- a/Source/WebCore/loader/ImageLoader.cpp
+++ b/Source/WebCore/loader/ImageLoader.cpp
@@ -75,11 +75,18 @@ static ImageEventSender& loadEventSender()
     return sender;
 }
 
+static ImageEventSender& errorEventSender()
+{
+    DEFINE_STATIC_LOCAL(ImageEventSender, sender, (eventNames().errorEvent));
+    return sender;
+}
+
 ImageLoader::ImageLoader(Element* element)
     : m_element(element)
     , m_image(0)
     , m_firedBeforeLoad(true)
     , m_firedLoad(true)
+    , m_firedError(true)
     , m_imageComplete(true)
     , m_loadManually(false)
 {
@@ -97,6 +104,10 @@ ImageLoader::~ImageLoader()
     ASSERT(!m_firedLoad || !loadEventSender().hasPendingEvents(this));
     if (!m_firedLoad)
         loadEventSender().cancelEvent(this);
+
+    ASSERT(!m_firedError || !errorEventSender().hasPendingEvents(this));
+    if (!m_firedError)
+        errorEventSender().cancelEvent(this);
 }
 
 void ImageLoader::setImage(CachedImage* newImage)
@@ -113,6 +124,10 @@ void ImageLoader::setImage(CachedImage* newImage)
             loadEventSender().cancelEvent(this);
             m_firedLoad = true;
         }
+        if (!m_firedError) {
+            errorEventSender().cancelEvent(this);
+            m_firedError = true;
+        }
         m_imageComplete = true;
         if (newImage)
             newImage->addClient(this);
@@ -163,8 +178,11 @@ void ImageLoader::updateFromElement()
         // If we do not have an image here, it means that a cross-site
         // violation occurred.
         m_failedLoadURL = !newImage ? attr : AtomicString();
-    } else if (!attr.isNull()) // Fire an error event if the url is empty.
+    } else if (!attr.isNull()) {
+        // Fire an error event if the url is empty.
+        // FIXME: Should we fire this event asynchronoulsy via errorEventSender()?
         m_element->dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+    }
     
     CachedImage* oldImage = m_image.get();
     if (newImage != oldImage) {
@@ -172,6 +190,8 @@ void ImageLoader::updateFromElement()
             beforeLoadEventSender().cancelEvent(this);
         if (!m_firedLoad)
             loadEventSender().cancelEvent(this);
+        if (!m_firedError)
+            errorEventSender().cancelEvent(this);
 
         m_image = newImage;
         m_firedBeforeLoad = !newImage;
@@ -222,6 +242,9 @@ void ImageLoader::notifyFinished(CachedResource* resource)
 
         setImage(0);
 
+        m_firedError = false;
+        errorEventSender().dispatchEventSoon(this);
+
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Cross-origin image load denied by Cross-Origin Resource Sharing policy."));
         m_element->document()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage);
 
@@ -279,12 +302,14 @@ void ImageLoader::updateRenderer()
 
 void ImageLoader::dispatchPendingEvent(ImageEventSender* eventSender)
 {
-    ASSERT(eventSender == &beforeLoadEventSender() || eventSender == &loadEventSender());
+    ASSERT(eventSender == &beforeLoadEventSender() || eventSender == &loadEventSender() || eventSender == &errorEventSender());
     const AtomicString& eventType = eventSender->eventType();
     if (eventType == eventNames().beforeloadEvent)
         dispatchPendingBeforeLoadEvent();
     if (eventType == eventNames().loadEvent)
         dispatchPendingLoadEvent();
+    if (eventType == eventNames().errorEvent)
+        dispatchPendingErrorEvent();
 }
 
 void ImageLoader::dispatchPendingBeforeLoadEvent()
@@ -324,6 +349,16 @@ void ImageLoader::dispatchPendingLoadEvent()
     dispatchLoadEvent();
 }
 
+void ImageLoader::dispatchPendingErrorEvent()
+{
+    if (m_firedError)
+        return;
+    if (!m_element->document()->attached())
+        return;
+    m_firedError = true;
+    m_element->dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+}
+
 void ImageLoader::dispatchPendingBeforeLoadEvents()
 {
     beforeLoadEventSender().dispatchPendingEvents();
@@ -334,6 +369,11 @@ void ImageLoader::dispatchPendingLoadEvents()
     loadEventSender().dispatchPendingEvents();
 }
 
+void ImageLoader::dispatchPendingErrorEvents()
+{
+    errorEventSender().dispatchPendingEvents();
+}
+
 void ImageLoader::elementDidMoveToNewDocument()
 {
     setImage(0);

diff --git a/Source/WebCore/loader/ImageLoader.h b/Source/WebCore/loader/ImageLoader.h
index 0667b3d..e31bc4b 100644 (file)
--- a/Source/WebCore/loader/ImageLoader.h
+++ b/Source/WebCore/loader/ImageLoader.h
@@ -66,6 +66,7 @@ public:
 
     static void dispatchPendingBeforeLoadEvents();
     static void dispatchPendingLoadEvents();
+    static void dispatchPendingErrorEvents();
 
 protected:
     virtual void notifyFinished(CachedResource*);
@@ -76,6 +77,7 @@ private:
 
     void dispatchPendingBeforeLoadEvent();
     void dispatchPendingLoadEvent();
+    void dispatchPendingErrorEvent();
 
     RenderImageResource* renderImageResource();
     void updateRenderer();
@@ -85,6 +87,7 @@ private:
     AtomicString m_failedLoadURL;
     bool m_firedBeforeLoad : 1;
     bool m_firedLoad : 1;
+    bool m_firedError : 1;
     bool m_imageComplete : 1;
     bool m_loadManually : 1;
 };