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..2f0a6f68e803e491f4eb3727f93242cb8d05470b 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();
+    }
+
+    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,26 @@ 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 = WeakPtr<Task>();
+    // 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 (!attr.isNull() && !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 +266,78 @@ void ImageLoader::updateFromElement()
     updatedHasPendingEvent();
 }
 
+void ImageLoader::updateFromElement()
+{
+    AtomicString attr = m_element->imageSourceURL();
+
+    if (!m_failedLoadURL.isEmpty() && attr == m_failedLoadURL)
+        return;

[abarth-chromium, 2014/05/02 17:04:59]

Do we need to do this check both here and in doUpdateFromElement?

[cbiesinger, 2014/05/02 23:24:04]

Oh, no, can be removed from doUpdate. Done.

+
+    KURL url = imageURL();
+    if (!attr.isNull() && !url.isNull()) {

[abarth-chromium, 2014/05/02 17:04:59]

Won't imageURL() always return a null |url| if |attr| is null (care of line
323)?

[cbiesinger, 2014/05/02 23:24:04]

Indeed, done.

+        // 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 = WeakPtr<Task>();

[abarth-chromium, 2014/05/02 17:04:59]

We should add a clear() method to WeakPtr

[cbiesinger, 2014/05/02 23:24:04]

Turns out that method exists and I just missed it :/

+        }
+        bool loadImmediately = shouldLoadImmediately(url);
+        if (loadImmediately) {
+            doUpdateFromElement(false);
+        } else {
+            Task* task = new Task(this);

[abarth-chromium, 2014/05/02 17:04:59]

OwnPtr<Task> task = adoptPtr(new Task(this));

All calls to |new| should be wrapped immediately in either adoptPtr or adoptRef
so it's clear there's no leak.  To see that there's no leak in this code, you
need to look down a few lines.

[cbiesinger, 2014/05/02 23:24:04]

Done.

+            m_pendingTask = task->createWeakPtr();
+            Microtask::enqueueMicrotask(adoptPtr(task));

[abarth-chromium, 2014/05/02 17:04:59]

s/adoptPtr(task)/task.release()/

[cbiesinger, 2014/05/02 23:24:04]

Done.

+            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();
+
+    if (!m_failedLoadURL.isEmpty() && attr == m_failedLoadURL)
+        return url;

[abarth-chromium, 2014/05/02 17:04:59]

This is a third place this check appears.  Shouldn't we be able to just make
this check in one place?

[cbiesinger, 2014/05/02 23:24:04]

Yeah, we don't need it here, a leftover from an earlier version.

+
+    // Do not load any image if the 'src' attribute is missing or if it is
+    // an empty string.
+    ResourcePtr<ImageResource> newImage = 0;
+    if (!attr.isNull() && !stripLeadingAndTrailingHTMLSpaces(attr).isEmpty()) {
+        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;
+}
+
 void ImageLoader::notifyFinished(Resource* resource)
 {
     ASSERT(m_failedLoadURL.isEmpty());
@@ -389,6 +503,9 @@ void ImageLoader::dispatchPendingErrorEvents()
 
 void ImageLoader::elementDidMoveToNewDocument()
 {
+    if (m_delayLoad) {
+        m_delayLoad->documentChanged(m_element->document());
+    }
     clearFailedLoadURL();
     setImage(0);
 }