Changeset 194926 in webkit

Timestamp:

Jan 12, 2016 2:18:19 PM (8 years ago)

Author:

hyatt@apple.com

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLImageElement.cpp (modified) (6 diffs)
• html/HTMLImageElement.h (modified) (2 diffs)
• html/parser/HTMLConstructionSite.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/html/HTMLImageElement.cpp

@@ -54 +54 @@
 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)
@@ -83 +86 @@
     if (m_form)
         m_form->removeImgElement(this);
+    setPictureNode(nullptr);
 }
 
@@ -141 +145 @@
 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;
@@ -164 +168 @@
                 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())
@@ -314 +318 @@
         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,
@@ -332 +338 @@
     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)
 {

trunk/Source/WebCore/html/HTMLImageElement.h

@@ -88 +88 @@
 
     bool hasShadowControls() const { return m_experimentalImageMenuEnabled; }
+    
+    Node* pictureNode() const;
+    void setPictureNode(Node*);
 
 protected:
@@ -128 +131 @@
     HTMLFormElement* m_form;
     HTMLFormElement* m_formSetByParser;
+
     CompositeOperator m_compositeOperator;
     AtomicString m_bestFitImageURL;

trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp

@@ -37 +37 @@
 #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"
@@ -641 +643 @@
     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());