Avoid downloading the wrong image for <picture> elements.
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jan 2016 22:18:19 +0000 (22:18 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jan 2016 22:18:19 +0000 (22:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=153027

Reviewed by Dean Jackson.

I was unable to write a reliable test for this feature (I welcome suggestions regarding
how this could be tested).

* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::HTMLImageElement):
(WebCore::HTMLImageElement::~HTMLImageElement):
(WebCore::HTMLImageElement::bestFitSourceFromPictureElement):
(WebCore::HTMLImageElement::insertedInto):
(WebCore::HTMLImageElement::removedFrom):
(WebCore::HTMLImageElement::pictureNode):
(WebCore::HTMLImageElement::setPictureNode):
* html/HTMLImageElement.h:
* html/parser/HTMLConstructionSite.cpp:
(WebCore::HTMLConstructionSite::createHTMLElement):

Images that are built underneath a <picture> element are now connected
to that picture element via a setPictureNode call from the parser. This
ensures that the correct <source> elements are examined before checking the image.

This connection between images and their picture owners is handled using a static
HashMap in HTMLImageElement. This connection is made both from the parser and from
DOM insertions, and the map is queried now instead of looking directly at the
image's parentNode().

Also note the change to pass the document element's computed style in for media
query evaluation. Just as with the preload scanner, the image's style can't be
used as it has not been determined yet.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194926 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/HTMLImageElement.h
Source/WebCore/html/parser/HTMLConstructionSite.cpp

index cb2f39e..e26ffd3 100644 (file)
@@ -1,3 +1,38 @@
+2016-01-12  Dave Hyatt  <hyatt@apple.com>
+
+        Avoid downloading the wrong image for <picture> elements.
+        https://bugs.webkit.org/show_bug.cgi?id=153027
+
+        Reviewed by Dean Jackson.
+
+        I was unable to write a reliable test for this feature (I welcome suggestions regarding
+        how this could be tested).
+
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::HTMLImageElement):
+        (WebCore::HTMLImageElement::~HTMLImageElement):
+        (WebCore::HTMLImageElement::bestFitSourceFromPictureElement):
+        (WebCore::HTMLImageElement::insertedInto):
+        (WebCore::HTMLImageElement::removedFrom):
+        (WebCore::HTMLImageElement::pictureNode):
+        (WebCore::HTMLImageElement::setPictureNode):
+        * html/HTMLImageElement.h:
+        * html/parser/HTMLConstructionSite.cpp:
+        (WebCore::HTMLConstructionSite::createHTMLElement):
+
+        Images that are built underneath a <picture> element are now connected
+        to that picture element via a setPictureNode call from the parser. This
+        ensures that the correct <source> elements are examined before checking the image.
+
+        This connection between images and their picture owners is handled using a static
+        HashMap in HTMLImageElement. This connection is made both from the parser and from
+        DOM insertions, and the map is queried now instead of looking directly at the
+        image's parentNode().
+
+        Also note the change to pass the document element's computed style in for media
+        query evaluation. Just as with the preload scanner, the image's style can't be
+        used as it has not been determined yet.
+
 2016-01-12  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Cleanup in font loading code
index b611f07..743fd40 100644 (file)
@@ -53,6 +53,9 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
+typedef HashMap<const Node*, Node*> PictureOwnerMap;
+static PictureOwnerMap* gPictureOwnerMap = nullptr;
+
 HTMLImageElement::HTMLImageElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
     : HTMLElement(tagName, document)
     , m_imageLoader(*this)
@@ -82,6 +85,7 @@ HTMLImageElement::~HTMLImageElement()
 {
     if (m_form)
         m_form->removeImgElement(this);
+    setPictureNode(nullptr);
 }
 
 Ref<HTMLImageElement> HTMLImageElement::createForJSConstructor(Document& document, const int* optionalWidth, const int* optionalHeight)
@@ -140,13 +144,13 @@ void HTMLImageElement::setBestFitURLAndDPRFromImageCandidate(const ImageCandidat
 
 ImageCandidate HTMLImageElement::bestFitSourceFromPictureElement()
 {
-    auto* parent = parentNode();
-    if (!is<HTMLPictureElement>(parent))
+    auto* pictureOwner = pictureNode();
+    if (!pictureOwner)
         return { };
-    auto* picture = downcast<HTMLPictureElement>(parent);
+    auto* picture = downcast<HTMLPictureElement>(pictureOwner);
     picture->clearViewportDependentResults();
     document().removeViewportDependentPicture(*picture);
-    for (Node* child = parent->firstChild(); child && child != this; child = child->nextSibling()) {
+    for (Node* child = picture->firstChild(); child && child != this; child = child->nextSibling()) {
         if (!is<HTMLSourceElement>(*child))
             continue;
         auto& source = downcast<HTMLSourceElement>(*child);
@@ -163,7 +167,7 @@ ImageCandidate HTMLImageElement::bestFitSourceFromPictureElement()
             if (!type.isEmpty() && !MIMETypeRegistry::isSupportedImageMIMEType(type) && type != "image/svg+xml")
                 continue;
         }
-        MediaQueryEvaluator evaluator(document().printing() ? "print" : "screen", document().frame(), computedStyle());
+        MediaQueryEvaluator evaluator(document().printing() ? "print" : "screen", document().frame(), document().documentElement()->computedStyle());
         bool evaluation = evaluator.evalCheckingViewportDependentResults(source.mediaQuerySet(), picture->viewportDependentResults());
         if (picture->hasViewportDependentResults())
             document().addViewportDependentPicture(*picture);
@@ -313,8 +317,10 @@ Node::InsertionNotificationRequest HTMLImageElement::insertedInto(ContainerNode&
     if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
         document().addImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
 
-    if (is<HTMLPictureElement>(parentNode()))
+    if (is<HTMLPictureElement>(parentNode())) {
+        setPictureNode(parentNode());
         selectImageSource();
+    }
 
     // If we have been inserted from a renderer-less document,
     // our loader may have not fetched the image, so do it now.
@@ -331,11 +337,34 @@ void HTMLImageElement::removedFrom(ContainerNode& insertionPoint)
 
     if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
         document().removeImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
-
+    
+    if (is<HTMLPictureElement>(parentNode()))
+        setPictureNode(nullptr);
+    
     m_form = nullptr;
     HTMLElement::removedFrom(insertionPoint);
 }
 
+Node* HTMLImageElement::pictureNode() const
+{
+    if (!gPictureOwnerMap)
+        return nullptr;
+    return gPictureOwnerMap->get(this);
+}
+    
+void HTMLImageElement::setPictureNode(Node* node)
+{
+    if (!node) {
+        if (gPictureOwnerMap)
+            gPictureOwnerMap->remove(this);
+        return;
+    }
+    
+    if (!gPictureOwnerMap)
+        gPictureOwnerMap = new PictureOwnerMap();
+    gPictureOwnerMap->add(this, node);
+}
+    
 int HTMLImageElement::width(bool ignorePendingStylesheets)
 {
     if (!renderer()) {
index 199e8c0..e63cc64 100644 (file)
@@ -87,6 +87,9 @@ public:
     virtual const AtomicString& imageSourceURL() const override;
 
     bool hasShadowControls() const { return m_experimentalImageMenuEnabled; }
+    
+    Node* pictureNode() const;
+    void setPictureNode(Node*);
 
 protected:
     HTMLImageElement(const QualifiedName&, Document&, HTMLFormElement* = 0);
@@ -127,6 +130,7 @@ private:
     HTMLImageLoader m_imageLoader;
     HTMLFormElement* m_form;
     HTMLFormElement* m_formSetByParser;
+
     CompositeOperator m_compositeOperator;
     AtomicString m_bestFitImageURL;
     AtomicString m_currentSrc;
index 8f283da..8ba3e81 100644 (file)
 #include "HTMLElementFactory.h"
 #include "HTMLFormElement.h"
 #include "HTMLHtmlElement.h"
+#include "HTMLImageElement.h"
 #include "HTMLOptGroupElement.h"
 #include "HTMLOptionElement.h"
 #include "HTMLParserIdioms.h"
+#include "HTMLPictureElement.h"
 #include "HTMLScriptElement.h"
 #include "HTMLTemplateElement.h"
 #include "NotImplemented.h"
@@ -640,6 +642,13 @@ PassRefPtr<Element> HTMLConstructionSite::createHTMLElement(AtomicHTMLToken* tok
     Document& ownerDocument = ownerDocumentForCurrentNode();
     bool insideTemplateElement = !ownerDocument.frame();
     RefPtr<Element> element = HTMLElementFactory::createElement(tagName, ownerDocument, insideTemplateElement ? nullptr : form(), true);
+    
+    // FIXME: This is a hack to connect images to pictures before the image has
+    // been inserted into the document. It can be removed once asynchronous image
+    // loading is working.
+    if (is<HTMLPictureElement>(currentNode()) && is<HTMLImageElement>(*element.get()))
+        downcast<HTMLImageElement>(*element.get()).setPictureNode(&currentNode());
+
     setAttributes(element.get(), token, m_parserContentPolicy);
     ASSERT(element->isHTMLElement());
     return element.release();