Changeset 195064 in webkit

Timestamp:

Jan 14, 2016 11:13:26 AM (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.

Source/WebCore:

Added test in fast/picture.

• html/HTMLImageElement.cpp:

(WebCore::HTMLImageElement::HTMLImageElement):
(WebCore::HTMLImageElement::~HTMLImageElement):
(WebCore::HTMLImageElement::createForJSConstructor):
(WebCore::HTMLImageElement::bestFitSourceFromPictureElement):
(WebCore::HTMLImageElement::insertedInto):
(WebCore::HTMLImageElement::removedFrom):
(WebCore::HTMLImageElement::pictureElement):
(WebCore::HTMLImageElement::setPictureElement):
(WebCore::HTMLImageElement::width):

• html/HTMLImageElement.h:

(WebCore::HTMLImageElement::hasShadowControls):

• html/HTMLPictureElement.h:
• html/parser/HTMLConstructionSite.cpp:

(WebCore::HTMLConstructionSite::createHTMLElement):

• html/parser/HTMLPreloadScanner.cpp:

(WebCore::TokenPreloadScanner::StartTagScanner::processAttribute):

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().

LayoutTests:

• fast/picture/image-picture-loads-1x-expected.txt: Added.
• fast/picture/image-picture-loads-1x.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/picture/hidpi-image-picture-loads-expected.txt (added)
• LayoutTests/fast/picture/hidpi-image-picture-loads.html (added)
• LayoutTests/platform/ios-simulator/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac-yosemite/TestExpectations (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLImageElement.cpp (modified) (6 diffs)
• Source/WebCore/html/HTMLImageElement.h (modified) (2 diffs)
• Source/WebCore/html/HTMLPictureElement.h (modified) (1 diff)
• Source/WebCore/html/parser/HTMLConstructionSite.cpp (modified) (2 diffs)
• Source/WebCore/html/parser/HTMLPreloadScanner.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-01-14  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.
+
+        * fast/picture/image-picture-loads-1x-expected.txt: Added.
+        * fast/picture/image-picture-loads-1x.html: Added.
+
 2016-01-14  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 

trunk/LayoutTests/platform/ios-simulator/TestExpectations

@@ -2840 +2840 @@
 webkit.org/b/152141 fast/picture/image-picture-invalid.html [ Skip ]
 webkit.org/b/152141 fast/picture/image-picture-nested.html [ Skip ]
-webkit.org/b/153043 fast/picture/image-picture-loads-1x.html [ Skip ]
 webkit.org/b/152992 http/tests/loading/preload-picture-invalid.html [ Skip ]
 webkit.org/b/152992 http/tests/loading/preload-picture-nested.html [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-01-14  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.
+
+        Added test in fast/picture.
+
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::HTMLImageElement):
+        (WebCore::HTMLImageElement::~HTMLImageElement):
+        (WebCore::HTMLImageElement::createForJSConstructor):
+        (WebCore::HTMLImageElement::bestFitSourceFromPictureElement):
+        (WebCore::HTMLImageElement::insertedInto):
+        (WebCore::HTMLImageElement::removedFrom):
+        (WebCore::HTMLImageElement::pictureElement):
+        (WebCore::HTMLImageElement::setPictureElement):
+        (WebCore::HTMLImageElement::width):
+        * html/HTMLImageElement.h:
+        (WebCore::HTMLImageElement::hasShadowControls):
+        * html/HTMLPictureElement.h:
+        * html/parser/HTMLConstructionSite.cpp:
+        (WebCore::HTMLConstructionSite::createHTMLElement):
+        * html/parser/HTMLPreloadScanner.cpp:
+        (WebCore::TokenPreloadScanner::StartTagScanner::processAttribute):
+
+        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().
+
 2016-01-14  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebCore/html/HTMLImageElement.cpp

@@ -54 +54 @@
 using namespace HTMLNames;
 
+typedef HashMap<const HTMLImageElement*, WeakPtr<HTMLPictureElement>> 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);
+    setPictureElement(nullptr);
 }
 
@@ -141 +145 @@
 ImageCandidate HTMLImageElement::bestFitSourceFromPictureElement()
 {
-    auto* parent = parentNode();
-    if (!is<HTMLPictureElement>(parent))
+    auto* picture = pictureElement();
+    if (!picture)
         return { };
-    auto* picture = downcast<HTMLPictureElement>(parent);
     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 +167 @@
                 continue;
         }
-        MediaQueryEvaluator evaluator(document().printing() ? "print" : "screen", document().frame(), computedStyle());
+        MediaQueryEvaluator evaluator(document().printing() ? "print" : "screen", document().frame(), document().documentElement() ? document().documentElement()->computedStyle() : nullptr);
         bool evaluation = evaluator.evalCheckingViewportDependentResults(source.mediaQuerySet(), picture->viewportDependentResults());
         if (picture->hasViewportDependentResults())
@@ -314 +317 @@
         document().addImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
 
-    if (is<HTMLPictureElement>(parentNode()))
+    if (is<HTMLPictureElement>(parentNode())) {
+        setPictureElement(&downcast<HTMLPictureElement>(*parentNode()));
         selectImageSource();
+    }
 
     // If we have been inserted from a renderer-less document,
@@ -332 +337 @@
     if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
         document().removeImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
-
+    
+    if (is<HTMLPictureElement>(parentNode()))
+        setPictureElement(nullptr);
+    
     m_form = nullptr;
     HTMLElement::removedFrom(insertionPoint);
 }
 
+HTMLPictureElement* HTMLImageElement::pictureElement() const
+{
+    if (!gPictureOwnerMap || !gPictureOwnerMap->contains(this))
+        return nullptr;
+    HTMLPictureElement* result = gPictureOwnerMap->get(this).get();
+    if (!result)
+        gPictureOwnerMap->remove(this);
+    return result;
+}
+    
+void HTMLImageElement::setPictureElement(HTMLPictureElement* pictureElement)
+{
+    if (!pictureElement) {
+        if (gPictureOwnerMap)
+            gPictureOwnerMap->remove(this);
+        return;
+    }
+    
+    if (!gPictureOwnerMap)
+        gPictureOwnerMap = new PictureOwnerMap();
+    gPictureOwnerMap->add(this, pictureElement->createWeakPtr());
+}
+    
 int HTMLImageElement::width(bool ignorePendingStylesheets)
 {

trunk/Source/WebCore/html/HTMLImageElement.h

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

trunk/Source/WebCore/html/HTMLPictureElement.h

@@ -47 +47 @@
     bool viewportChangeAffectedPicture();
 
+    WeakPtr<HTMLPictureElement> createWeakPtr() { return m_weakFactory.createWeakPtr(); }
+
 private:
     HTMLPictureElement(const QualifiedName&, Document&);
     
+    WeakPtrFactory<HTMLPictureElement> m_weakFactory { this };
     Vector<std::unique_ptr<MediaQueryResult>> m_viewportDependentMediaQueryResults;
 

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()).setPictureElement(&downcast<HTMLPictureElement>(currentNode()));
+
     setAttributes(element.get(), token, m_parserContentPolicy);
     ASSERT(element->isHTMLElement());

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

@@ -209 +209 @@
                 Ref<MediaQuerySet> mediaSet = MediaQuerySet::createAllowingDescriptionSyntax(attributeValue);
                 Vector<std::unique_ptr<MediaQueryResult>> viewportDependentMediaQueryResults;
-                MediaQueryEvaluator evaluator(document.printing() ? "print" : "screen", document.frame(), document.documentElement()->computedStyle());
+                MediaQueryEvaluator evaluator(document.printing() ? "print" : "screen", document.frame(), document.documentElement() ? document.documentElement()->computedStyle() : nullptr);
                 m_mediaMatched = evaluator.evalCheckingViewportDependentResults(mediaSet.ptr(), viewportDependentMediaQueryResults);
             }