When <img crossorigin> fails the CORS check, we should fire the error event
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Mar 2012 01:59:06 +0000 (01:59 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Mar 2012 01:59:06 +0000 (01:59 +0000)
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: https://svn.webkit.org/repository/webkit/trunk@112190 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt
LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/ImageLoader.cpp
Source/WebCore/loader/ImageLoader.h

index d2b7d89b64371daed3d938325f7199f2c4f2898d..124acee5be88d238f92e16fe32d8af89b0eeb046 100644 (file)
@@ -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.
index 21666ca04c731eabdc9e86cf4b3b9509b2d07784..02a25c9ee51ad03e26577a054202774e8648caab 100644 (file)
@@ -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. 
index 8c67a18cb5324a4b497738d40abcfcd93c8a74ec..485c6935afdac2f995cc100b33c24958d97842f8 100644 (file)
@@ -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);
index 6171c521e9f7160929c7e9df25b7841d243b8821..a5c3de8891bb14b4ecd17322150499a6a3345bd2 100644 (file)
@@ -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.
index eb05795cd466eb5877f8bae3ed183dac6ea9fe2b..15b307fafda4fad3550760cd9a08e57b18f3d94f 100644 (file)
@@ -2317,6 +2317,7 @@ void Document::implicitClose()
 
     ImageLoader::dispatchPendingBeforeLoadEvents();
     ImageLoader::dispatchPendingLoadEvents();
+    ImageLoader::dispatchPendingErrorEvents();
 
     HTMLLinkElement::dispatchPendingLoadEvents();
     HTMLStyleElement::dispatchPendingLoadEvents();
index 842175b5e47607057cbc13f2e8ec83a6580a36fd..19da9876d8dd048c49c8d6782f6170c28b66d650 100644 (file)
@@ -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);
index 0667b3dc4f80da2ab26427b105a6c7c2d98fc44c..e31bc4b4cc890c8837693a453f22dfd3a4d21840 100644 (file)
@@ -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;
 };