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..5493173b29da1f9f0da41c8ba8216245ed0cfa4a 100644
--- a/Source/core/loader/ImageLoader.cpp
+++ b/Source/core/loader/ImageLoader.cpp
@@ -22,15 +22,18 @@
 #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/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,10 +59,60 @@ 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_document(&m_loader->m_element->document())
+        , m_shouldBypassMainWorldContentSecurityPolicy(false)
+    {
+        m_document->incrementLoadEventDelayCount();

[abarth-chromium, 2014/04/25 23:59:44]

Why is it the task's job to increment and decrement the delay load count?  That
seems more like the job of the loader.  I would think that the task's job is
just to call the loader back at the right time and to remember if we're supposed
to bypass CSP.  If we go that route, the Task doesn't need to hold onto the
document and we can remove the RefPtr<Document>.

[cbiesinger, 2014/05/01 00:09:08]

Well the entire reason was your request to have a refptr-like object to keep
track of the delay load count. If I go back to manually calling
increment/decrement, then sure, I can move these calls back to ImageLoader.

+
+        LocalFrame* frame = m_document->frame();
+        m_shouldBypassMainWorldContentSecurityPolicy =
+            frame && frame->script().shouldBypassMainWorldContentSecurityPolicy();

[abarth-chromium, 2014/04/25 23:59:44]

Can |frame| actually be zero here?

[cbiesinger, 2014/05/01 00:09:08]

This was copied from
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...,
which this function will eventually call. So, I assumed that it can be null, but
I'm happy to remove it if you think that's safe.

+    }
+
+    virtual ~Task()
+    {
+        clearLoader();
+    }
+
+    virtual void run() OVERRIDE
+    {
+        if (m_loader) {
+            m_loader->m_pendingTask = 0;

[abarth-chromium, 2014/04/25 23:59:44]

This works seems like it should be done by the loader instead of the task.  That
way the loader doesn't need to be friends with the task.

[cbiesinger, 2014/05/01 00:09:08]

Done. (Note that the task is only friends implicitly, by virtue of being an
inner class)

+            m_loader->updateFromElement(true, m_shouldBypassMainWorldContentSecurityPolicy);

[abarth-chromium, 2014/04/25 23:59:44]

Rather than passing a bool from the two different callers, can we refactor
updateFromElement into two different functions?  For example, one that schedules
the load (either immediately or delayed) and another that actually does the
load?

[cbiesinger, 2014/05/01 00:09:08]

Done.

+        }
+    }
+
+    void clearLoader()
+    {
+        if (m_loader) {
+            m_document->decrementLoadEventDelayCount();
+            m_document.clear();

[abarth-chromium, 2014/04/25 23:59:44]

Once it's the loaders job to increment/decrement the LoadEventDelayCount, we can
just use WeakPtr<ImageLoader> and we don't have to re-implement the weak pointer
pattern manually.

[cbiesinger, 2014/05/01 00:09:08]

Hm... WeakPtr requires refcounted objects?

+            m_loader = 0;
+        }
+    }
+
+    void documentChanged()
+    {
+        m_document->decrementLoadEventDelayCount();
+        m_document = &m_loader->m_element->document();
+        m_document->incrementLoadEventDelayCount();
+    }
+
+private:
+    ImageLoader* m_loader;
+    RefPtr<Document> m_document;
+    bool m_shouldBypassMainWorldContentSecurityPolicy;
+};
+
 ImageLoader::ImageLoader(Element* element)
     : m_element(element)
     , m_image(0)
     , m_derefElementTimer(this, &ImageLoader::timerFired)
+    , m_pendingTask(0)
     , m_hasPendingLoadEvent(false)
     , m_hasPendingErrorEvent(false)
     , m_imageComplete(true)
@@ -71,6 +124,9 @@ ImageLoader::ImageLoader(Element* element)
 
 ImageLoader::~ImageLoader()
 {
+    if (m_pendingTask)
+        m_pendingTask->clearLoader();
+
     if (m_image)
         m_image->removeClient(this);
 
@@ -123,7 +179,7 @@ void ImageLoader::setImageWithoutConsideringPendingLoadEvent(ImageResource* newI
         imageResource->resetAnimation();
 }
 
-void ImageLoader::updateFromElement()
+void ImageLoader::updateFromElement(bool calledFromTask, 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.
@@ -140,7 +196,25 @@ void ImageLoader::updateFromElement()
     // an empty string.
     ResourcePtr<ImageResource> newImage = 0;
     if (!attr.isNull() && !stripLeadingAndTrailingHTMLSpaces(attr).isEmpty()) {
-        FetchRequest request(ResourceRequest(document.completeURL(sourceURI(attr))), element()->localName());
+        KURL url = document.completeURL(sourceURI(attr));
+        if (!calledFromTask) {
+            // 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 = 0;
+            }
+            bool loadImmediately = shouldLoadImmediately(url);
+            if (!loadImmediately) {
+                m_pendingTask = new Task(this);
+                Microtask::enqueueMicrotask(adoptPtr(m_pendingTask));
+                return;
+            }
+        }
+
+        FetchRequest request(ResourceRequest(url), element()->localName());
+        if (bypassMainWorldCSP)
+            request.setContentSecurityCheck(DoNotCheckContentSecurityPolicy);
 
         AtomicString crossOriginMode = m_element->fastGetAttribute(HTMLNames::crossoriginAttr);
         if (!crossOriginMode.isNull())
@@ -224,6 +298,18 @@ void ImageLoader::updateFromElementIgnoringPreviousError()
     updateFromElement();
 }
 
+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;
+}
+
 void ImageLoader::notifyFinished(Resource* resource)
 {
     ASSERT(m_failedLoadURL.isEmpty());
@@ -389,6 +475,9 @@ void ImageLoader::dispatchPendingErrorEvents()
 
 void ImageLoader::elementDidMoveToNewDocument()
 {
+    if (m_pendingTask) {
+        m_pendingTask->documentChanged();
+    }
     clearFailedLoadURL();
     setImage(0);
 }