Title: [194926] trunk/Source/WebCore
Revision
194926
Author
[email protected]
Date
2016-01-12 14:18:19 -0800 (Tue, 12 Jan 2016)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (194925 => 194926)


--- trunk/Source/WebCore/ChangeLog	2016-01-12 22:06:46 UTC (rev 194925)
+++ trunk/Source/WebCore/ChangeLog	2016-01-12 22:18:19 UTC (rev 194926)
@@ -1,3 +1,38 @@
+2016-01-12  Dave Hyatt  <[email protected]>
+
+        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  <[email protected]>
 
         Cleanup in font loading code

Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (194925 => 194926)


--- trunk/Source/WebCore/html/HTMLImageElement.cpp	2016-01-12 22:06:46 UTC (rev 194925)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp	2016-01-12 22:18:19 UTC (rev 194926)
@@ -53,6 +53,9 @@
 
 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 @@
 {
     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 @@
 
 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 @@
             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 @@
     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 @@
 
     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()) {

Modified: trunk/Source/WebCore/html/HTMLImageElement.h (194925 => 194926)


--- trunk/Source/WebCore/html/HTMLImageElement.h	2016-01-12 22:06:46 UTC (rev 194925)
+++ trunk/Source/WebCore/html/HTMLImageElement.h	2016-01-12 22:18:19 UTC (rev 194926)
@@ -87,6 +87,9 @@
     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 @@
     HTMLImageLoader m_imageLoader;
     HTMLFormElement* m_form;
     HTMLFormElement* m_formSetByParser;
+
     CompositeOperator m_compositeOperator;
     AtomicString m_bestFitImageURL;
     AtomicString m_currentSrc;

Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp (194925 => 194926)


--- trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp	2016-01-12 22:06:46 UTC (rev 194925)
+++ trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp	2016-01-12 22:18:19 UTC (rev 194926)
@@ -36,9 +36,11 @@
 #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 @@
     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();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to