Post a microtask to load <img> elements.

Source/core/loader/ImageLoader.cpp

Index: Source/core/loader/ImageLoader.cpp
diff --git a/Source/core/loader/ImageLoader.cpp b/Source/core/loader/ImageLoader.cpp
index ed661094767f513861c8a0ed2fe47766108525cf..375a06ad281a23fc4300120771e7d963dd23736b 100644
--- a/Source/core/loader/ImageLoader.cpp
+++ b/Source/core/loader/ImageLoader.cpp
@@ -22,15 +22,19 @@
 #include "config.h"
 #include "core/loader/ImageLoader.h"
 
+#include "bindings/v8/ScriptController.h"
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
+#include "core/dom/IncrementLoadEventDelayCount.h"
+#include "core/dom/Microtask.h"
 #include "core/events/Event.h"
 #include "core/events/EventSender.h"
 #include "core/fetch/CrossOriginAccessControl.h"
 #include "core/fetch/FetchRequest.h"
 #include "core/fetch/MemoryCache.h"
 #include "core/fetch/ResourceFetcher.h"
-#include "core/html/HTMLObjectElement.h"
+#include "core/frame/LocalFrame.h"
+#include "core/html/HTMLImageElement.h"
 #include "core/html/parser/HTMLParserIdioms.h"
 #include "core/rendering/RenderImage.h"
 #include "core/rendering/RenderVideo.h"
@@ -56,6 +60,45 @@ static inline bool pageIsBeingDismissed(Document* document)
     return document->pageDismissalEventBeingDispatched() != Document::NoDismissal;
 }
 
+class ImageLoader::Task : public blink::WebThread::Task {
+public:
+    Task(ImageLoader* loader)
+        : m_loader(loader)
+        , m_shouldBypassMainWorldContentSecurityPolicy(false)
+        , m_weakFactory(this)
+    {
+        LocalFrame* frame = loader->m_element->document().frame();
+        m_shouldBypassMainWorldContentSecurityPolicy = frame->script().shouldBypassMainWorldContentSecurityPolicy();
+    }
+
+    virtual ~Task()
+    {
+        clearLoader();

[abarth-chromium, 2014/05/03 01:22:31]

Why do we need to clear the loader from the destructor?  It's just setting a
private member to zero.

[cbiesinger, 2014/05/05 22:39:30]

Oh, yeah, a leftover. Let me remove that.

+    }
+
+    virtual void run() OVERRIDE
+    {
+        if (m_loader) {
+            m_loader->doUpdateFromElement(m_shouldBypassMainWorldContentSecurityPolicy);
+        }
+    }
+
+    void clearLoader()
+    {
+        m_loader = 0;
+    }
+
+    WeakPtr<Task> createWeakPtr()
+    {
+        return m_weakFactory.createWeakPtr();
+    }
+
+private:
+    ImageLoader* m_loader;
+    bool m_shouldBypassMainWorldContentSecurityPolicy;
+    WeakPtrFactory<Task> m_weakFactory;
+};
+
 ImageLoader::ImageLoader(Element* element)
     : m_element(element)
     , m_image(0)
@@ -71,6 +114,9 @@ ImageLoader::ImageLoader(Element* element)
 
 ImageLoader::~ImageLoader()
 {
+    if (m_pendingTask)
+        m_pendingTask->clearLoader();
+
     if (m_image)
         m_image->removeClient(this);
 
@@ -123,24 +169,23 @@ void ImageLoader::setImageWithoutConsideringPendingLoadEvent(ImageResource* newI
         imageResource->resetAnimation();
 }
 
-void ImageLoader::updateFromElement()
+void ImageLoader::doUpdateFromElement(bool bypassMainWorldCSP)
 {
-    // Don't load images for inactive documents. We don't want to slow down the
-    // raw HTML parsing case by loading images we don't intend to display.
+    m_pendingTask.clear();

[abarth-chromium, 2014/05/03 01:22:31]

Do we need to call clearLoader() on the m_pendingTask?

[cbiesinger, 2014/05/05 22:39:30]

No - let me add a comment to this effect:
    // We don't need to call clearLoader here: Either we were called from the
    // task, or our caller updateFromElement cleared the task's loader (and set
    // m_pendingTask to null).

+    // Make sure to only decrement the count when we exit this function
+    OwnPtr<IncrementLoadEventDelayCount> delayLoad;
+    delayLoad.swap(m_delayLoad);
+
     Document& document = m_element->document();
-    if (!document.isActive())
-        return;
 
     AtomicString attr = m_element->imageSourceURL();
 
-    if (!m_failedLoadURL.isEmpty() && attr == m_failedLoadURL)
-        return;
-
-    // Do not load any image if the 'src' attribute is missing or if it is
-    // an empty string.
+    KURL url = imageURL();
     ResourcePtr<ImageResource> newImage = 0;
-    if (!attr.isNull() && !stripLeadingAndTrailingHTMLSpaces(attr).isEmpty()) {
-        FetchRequest request(ResourceRequest(document.completeURL(sourceURI(attr))), element()->localName());
+    if (!url.isNull()) {
+        FetchRequest request(ResourceRequest(url), element()->localName());
+        if (bypassMainWorldCSP)
+            request.setContentSecurityCheck(DoNotCheckContentSecurityPolicy);
 
         AtomicString crossOriginMode = m_element->fastGetAttribute(HTMLNames::crossoriginAttr);
         if (!crossOriginMode.isNull())
@@ -218,12 +263,74 @@ void ImageLoader::updateFromElement()
     updatedHasPendingEvent();
 }
 
+void ImageLoader::updateFromElement()
+{
+    AtomicString attr = m_element->imageSourceURL();
+
+    if (!m_failedLoadURL.isEmpty() && attr == m_failedLoadURL)
+        return;
+
+    KURL url = imageURL();
+    if (!attr.isNull() && !url.isNull()) {
+        // If we have a pending task, we have to clear it -- either we're
+        // now loading immediately, or we need to reset the task's state.
+        if (m_pendingTask) {
+            m_pendingTask->clearLoader();
+            m_pendingTask.clear();
+        }
+        bool loadImmediately = shouldLoadImmediately(url);
+        if (loadImmediately) {
+            doUpdateFromElement(false);
+        } else {
+            OwnPtr<Task> task = adoptPtr(new Task(this));
+            m_pendingTask = task->createWeakPtr();
+            Microtask::enqueueMicrotask(task.release());
+            m_delayLoad = adoptPtr(new IncrementLoadEventDelayCount(m_element->document()));
+            return;
+        }
+    } else {
+        doUpdateFromElement(false);
+    }
+}
+
 void ImageLoader::updateFromElementIgnoringPreviousError()
 {
     clearFailedLoadURL();
     updateFromElement();
 }
 
+KURL ImageLoader::imageURL() const
+{
+    KURL url;
+
+    // Don't load images for inactive documents. We don't want to slow down the
+    // raw HTML parsing case by loading images we don't intend to display.
+    Document& document = m_element->document();
+    if (!document.isActive())
+        return url;
+
+    AtomicString attr = m_element->imageSourceURL();
+
+    // Do not load any image if the 'src' attribute is missing or if it is
+    // an empty string.
+    if (!attr.isNull() && !stripLeadingAndTrailingHTMLSpaces(attr).isEmpty()) {

[Erik Dahlström (inactive), 2014/05/05 08:41:01]

technically this may not always be a 'src' attribute (in svg it is usually
xlink:href), so applying html-specific handling here might possibly be wrong.
It's probably fine though, and svg and html should be the same here IMHO.

Feel free to tweak the comment to reflect this, but no need to change the code.

[cbiesinger, 2014/05/05 22:39:30]

I'm not changing this code (and comment) - it's identical to what used to be on
line 142.

+        url = document.completeURL(sourceURI(attr));
+    }
+    return url;
+}
+
+bool ImageLoader::shouldLoadImmediately(const KURL& url) const
+{
+    if (m_loadManually)
+        return true;
+
+    if (url.protocolIsData())
+        return true;
+    if (memoryCache()->resourceForURL(url))
+        return true;
+    return false;

[Erik Dahlström (inactive), 2014/05/05 08:41:01]

should we delay loads for <svg:image> elements?

[cbiesinger, 2014/05/05 22:39:30]

I think so. Do you disagree?

+}
+
 void ImageLoader::notifyFinished(Resource* resource)
 {
     ASSERT(m_failedLoadURL.isEmpty());
@@ -389,6 +496,9 @@ void ImageLoader::dispatchPendingErrorEvents()
 
 void ImageLoader::elementDidMoveToNewDocument()
 {
+    if (m_delayLoad) {
+        m_delayLoad->documentChanged(m_element->document());
+    }
     clearFailedLoadURL();
     setImage(0);
 }