Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(972)

Unified Diff: Source/core/loader/ImageLoader.cpp

Issue 200923002: Post a microtask to load <img> elements. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: review comments (retry) Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 destru
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_pendingT
cbiesinger 2014/05/05 22:39:30 No - let me add a comment to this effect: // W
+ // 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' attribu
cbiesinger 2014/05/05 22:39:30 I'm not changing this code (and comment) - it's id
+ 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);
}
« Source/core/dom/IncrementLoadEventDelayCount.h ('K') | « Source/core/loader/ImageLoader.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698