Changeset 252828 in webkit
- Timestamp:
- Nov 23, 2019 2:19:42 AM (4 years ago)
- Location:
- trunk
- Files:
-
- 4 added
- 14 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r252826 r252828 1 2019-11-23 Antti Koivisto <antti@apple.com> 2 3 Media queries in img sizes attribute don't evaluate dynamically 4 https://bugs.webkit.org/show_bug.cgi?id=204521 5 6 Reviewed by Simon Fraser. 7 8 * fast/images/sizes-dynamic-001-expected.html: Added. 9 * fast/images/sizes-dynamic-001.html: Added. 10 * fast/images/sizes-dynamic-002-expected.html: Added. 11 * fast/images/sizes-dynamic-002.html: Added. 12 13 Copied and modified from imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-* 14 because we don't support reftest-wait (webkit.org/b/186045). 15 1 16 2019-11-22 Per Arne Vollan <pvollan@apple.com> 2 17 -
trunk/Source/WebCore/ChangeLog
r252824 r252828 1 2019-11-23 Antti Koivisto <antti@apple.com> 2 3 Media queries in img sizes attribute don't evaluate dynamically 4 https://bugs.webkit.org/show_bug.cgi?id=204521 5 6 Reviewed by Simon Fraser. 7 8 Tests: fast/images/sizes-dynamic-001.html 9 fast/images/sizes-dynamic-002.html 10 11 * css/MediaQueryEvaluator.cpp: 12 (WebCore::MediaQueryEvaluator::evaluateForChanges const): 13 14 Add a helper function for evaluating dynamic results for changes. 15 16 * css/MediaQueryEvaluator.h: 17 (WebCore::MediaQueryDynamicResults::isEmpty const): 18 * css/parser/SizesAttributeParser.cpp: 19 (WebCore::SizesAttributeParser::SizesAttributeParser): 20 (WebCore::SizesAttributeParser::mediaConditionMatches): 21 22 Gather MediaQueryDynamicResults 23 24 * css/parser/SizesAttributeParser.h: 25 * dom/Document.cpp: 26 (WebCore::Document::updateElementsAffectedByMediaQueries): 27 (WebCore::Document::addDynamicMediaQueryDependentImage): 28 (WebCore::Document::removeDynamicMediaQueryDependentImage): 29 30 Replace viewport/appearance specific mechanism with a unified one (they were all invoked together anyway). 31 Make it about HTMLImageElement rather than HTMLPictureElement since that is the only client in this patch. 32 33 (WebCore::Document::checkViewportDependentPictures): Deleted. 34 (WebCore::Document::checkAppearanceDependentPictures): Deleted. 35 (WebCore::Document::addViewportDependentPicture): Deleted. 36 (WebCore::Document::removeViewportDependentPicture): Deleted. 37 (WebCore::Document::addAppearanceDependentPicture): Deleted. 38 (WebCore::Document::removeAppearanceDependentPicture): Deleted. 39 * dom/Document.h: 40 * html/HTMLImageElement.cpp: 41 (WebCore::HTMLImageElement::~HTMLImageElement): 42 (WebCore::HTMLImageElement::bestFitSourceFromPictureElement): 43 (WebCore::HTMLImageElement::evaluateDynamicMediaQueryDependencies): 44 (WebCore::HTMLImageElement::selectImageSource): 45 46 Gather MediaQueryDynamicResults from all paths. 47 48 (WebCore::HTMLImageElement::didMoveToNewDocument): 49 * html/HTMLImageElement.h: 50 * html/HTMLPictureElement.cpp: 51 (WebCore::HTMLPictureElement::~HTMLPictureElement): 52 (WebCore::HTMLPictureElement::didMoveToNewDocument): 53 (WebCore::HTMLPictureElement::viewportChangeAffectedPicture const): Deleted. 54 (WebCore::HTMLPictureElement::appearanceChangeAffectedPicture const): Deleted. 55 56 Move the media query dependency code to HTMLImageElement since thats where it is actually needed. 57 58 * html/HTMLPictureElement.h: 59 * rendering/RenderImage.cpp: 60 (WebCore::RenderImage::setImageDevicePixelRatio): 61 62 Update rendering also if only the pixel ratio changes. 63 64 * rendering/RenderImage.h: 65 (WebCore::RenderImage::setImageDevicePixelRatio): Deleted. 66 1 67 2019-11-21 Ryosuke Niwa <rniwa@webkit.org> 2 68 -
trunk/Source/WebCore/css/MediaQueryEvaluator.cpp
r252762 r252828 193 193 } 194 194 195 bool MediaQueryEvaluator::evaluateForChanges(const MediaQueryDynamicResults& dynamicResults) const 196 { 197 auto hasChanges = [&](auto& dynamicResultsVector) { 198 for (auto& dynamicResult : dynamicResultsVector) { 199 if (evaluate(dynamicResult.expression) != dynamicResult.result) 200 return true; 201 } 202 return false; 203 }; 204 205 return hasChanges(dynamicResults.viewport) || hasChanges(dynamicResults.appearance) || hasChanges(dynamicResults.accessibilitySettings); 206 } 207 195 208 template<typename T, typename U> bool compareValue(T a, U b, MediaFeaturePrefix op) 196 209 { -
trunk/Source/WebCore/css/MediaQueryEvaluator.h
r252736 r252828 58 58 accessibilitySettings.appendVector(other.accessibilitySettings); 59 59 } 60 bool isEmpty() const { return viewport.isEmpty() && appearance.isEmpty() && accessibilitySettings.isEmpty(); } 60 61 }; 61 62 … … 81 82 // Evaluates media query subexpression, ie "and (media-feature: value)" part. 82 83 bool evaluate(const MediaQueryExpression&) const; 84 bool evaluateForChanges(const MediaQueryDynamicResults&) const; 83 85 84 86 WEBCORE_EXPORT bool evaluate(const MediaQuerySet&, MediaQueryDynamicResults* = nullptr) const; -
trunk/Source/WebCore/css/parser/SizesAttributeParser.cpp
r252736 r252828 70 70 } 71 71 72 SizesAttributeParser::SizesAttributeParser(const String& attribute, const Document& document )72 SizesAttributeParser::SizesAttributeParser(const String& attribute, const Document& document, MediaQueryDynamicResults* mediaQueryDynamicResults) 73 73 : m_document(document) 74 , m_length(0) 75 , m_lengthWasSet(false) 74 , m_mediaQueryDynamicResults(mediaQueryDynamicResults) 76 75 { 77 76 m_isValid = parse(CSSTokenizer(attribute).tokenRange()); … … 116 115 return false; 117 116 auto& style = renderer->style(); 118 return MediaQueryEvaluator { "screen", m_document, &style }.evaluate(mediaCondition );117 return MediaQueryEvaluator { "screen", m_document, &style }.evaluate(mediaCondition, m_mediaQueryDynamicResults); 119 118 } 120 119 -
trunk/Source/WebCore/css/parser/SizesAttributeParser.h
r252392 r252828 39 39 class Document; 40 40 class MediaQuerySet; 41 struct MediaQueryDynamicResults; 41 42 42 43 class SizesAttributeParser { 43 44 public: 44 explicit SizesAttributeParser(const String&, const Document&);45 SizesAttributeParser(const String&, const Document&, MediaQueryDynamicResults* = nullptr); 45 46 46 47 float length(); … … 58 59 const Document& m_document; 59 60 RefPtr<MediaQuerySet> m_mediaCondition; 60 float m_length; 61 bool m_lengthWasSet; 62 bool m_isValid; 61 MediaQueryDynamicResults* m_mediaQueryDynamicResults { nullptr }; 62 float m_length { 0 }; 63 bool m_lengthWasSet { false }; 64 bool m_isValid { false }; 63 65 }; 64 66 -
trunk/Source/WebCore/dom/Document.cpp
r252824 r252828 3947 3947 { 3948 3948 ScriptDisallowedScope::InMainThread scriptDisallowedScope; 3949 checkViewportDependentPictures(); 3950 checkAppearanceDependentPictures(); 3949 3950 // FIXME: copyToVector doesn't work with WeakHashSet 3951 Vector<Ref<HTMLImageElement>> images; 3952 images.reserveInitialCapacity(m_dynamicMediaQueryDependentImages.computeSize()); 3953 for (auto& image : m_dynamicMediaQueryDependentImages) 3954 images.append(image); 3955 3956 for (auto& image : images) 3957 image->evaluateDynamicMediaQueryDependencies(); 3951 3958 } 3952 3959 … … 3957 3964 3958 3965 m_mediaQueryMatcher->evaluateAll(); 3959 }3960 3961 void Document::checkViewportDependentPictures()3962 {3963 Vector<HTMLPictureElement*, 16> changedPictures;3964 HashSet<HTMLPictureElement*>::iterator end = m_viewportDependentPictures.end();3965 for (HashSet<HTMLPictureElement*>::iterator it = m_viewportDependentPictures.begin(); it != end; ++it) {3966 if ((*it)->viewportChangeAffectedPicture())3967 changedPictures.append(*it);3968 }3969 for (auto* picture : changedPictures)3970 picture->sourcesChanged();3971 }3972 3973 void Document::checkAppearanceDependentPictures()3974 {3975 Vector<HTMLPictureElement*, 16> changedPictures;3976 for (auto* picture : m_appearanceDependentPictures) {3977 if (picture->appearanceChangeAffectedPicture())3978 changedPictures.append(picture);3979 }3980 3981 for (auto* picture : changedPictures)3982 picture->sourcesChanged();3983 3966 } 3984 3967 … … 7433 7416 } 7434 7417 7435 void Document::addViewportDependentPicture(HTMLPictureElement& picture) 7436 { 7437 m_viewportDependentPictures.add(&picture); 7438 } 7439 7440 void Document::removeViewportDependentPicture(HTMLPictureElement& picture) 7441 { 7442 m_viewportDependentPictures.remove(&picture); 7443 } 7444 7445 void Document::addAppearanceDependentPicture(HTMLPictureElement& picture) 7446 { 7447 m_appearanceDependentPictures.add(&picture); 7448 } 7449 7450 void Document::removeAppearanceDependentPicture(HTMLPictureElement& picture) 7451 { 7452 m_appearanceDependentPictures.remove(&picture); 7418 void Document::addDynamicMediaQueryDependentImage(HTMLImageElement& element) 7419 { 7420 m_dynamicMediaQueryDependentImages.add(element); 7421 } 7422 7423 void Document::removeDynamicMediaQueryDependentImage(HTMLImageElement& element) 7424 { 7425 m_dynamicMediaQueryDependentImages.remove(element); 7453 7426 } 7454 7427 -
trunk/Source/WebCore/dom/Document.h
r252824 r252828 145 145 class HTMLMediaElement; 146 146 class HTMLVideoElement; 147 class HTMLPictureElement;148 147 class HTMLScriptElement; 149 148 class HitTestLocation; … … 1394 1393 void applyContentDispositionAttachmentSandbox(); 1395 1394 1396 void addViewportDependentPicture(HTMLPictureElement&); 1397 void removeViewportDependentPicture(HTMLPictureElement&); 1398 1399 void addAppearanceDependentPicture(HTMLPictureElement&); 1400 void removeAppearanceDependentPicture(HTMLPictureElement&); 1395 void addDynamicMediaQueryDependentImage(HTMLImageElement&); 1396 void removeDynamicMediaQueryDependentImage(HTMLImageElement&); 1401 1397 1402 1398 void scheduleTimedRenderingUpdate(); … … 1655 1651 void didLoadResourceSynchronously() final; 1656 1652 1657 void checkViewportDependentPictures();1658 void checkAppearanceDependentPictures();1659 1660 1653 bool canNavigateInternal(Frame& targetFrame); 1661 1654 bool isNavigationBlockedByThirdPartyIFrameRedirectBlocking(Frame& targetFrame, const URL& destinationURL); … … 1832 1825 #endif 1833 1826 1834 HashSet<HTMLPictureElement*> m_viewportDependentPictures; 1835 HashSet<HTMLPictureElement*> m_appearanceDependentPictures; 1827 WeakHashSet<HTMLImageElement> m_dynamicMediaQueryDependentImages; 1836 1828 1837 1829 #if ENABLE(INTERSECTION_OBSERVER) -
trunk/Source/WebCore/html/HTMLImageElement.cpp
r252736 r252828 96 96 HTMLImageElement::~HTMLImageElement() 97 97 { 98 document().removeDynamicMediaQueryDependentImage(*this); 99 98 100 if (m_form) 99 101 m_form->removeImgElement(this); … … 161 163 return { }; 162 164 163 document().removeViewportDependentPicture(*picture);164 document().removeAppearanceDependentPicture(*picture);165 166 MediaQueryDynamicResults mediaQueryDynamicResults;167 165 ImageCandidate candidate; 168 166 … … 190 188 LOG(MediaQueries, "HTMLImageElement %p bestFitSourceFromPictureElement evaluating media queries", this); 191 189 192 auto evaluation = !queries || evaluator.evaluate(*queries, &m ediaQueryDynamicResults);190 auto evaluation = !queries || evaluator.evaluate(*queries, &m_mediaQueryDynamicResults); 193 191 if (!evaluation) 194 192 continue; 195 193 196 auto sourceSize = SizesAttributeParser(source.attributeWithoutSynchronization(sizesAttr).string(), document()).length(); 194 SizesAttributeParser sizesParser(source.attributeWithoutSynchronization(sizesAttr).string(), document(), &m_mediaQueryDynamicResults); 195 auto sourceSize = sizesParser.length(); 196 197 197 candidate = bestFitSourceForImageAttributes(document().deviceScaleFactor(), nullAtom(), srcset, sourceSize); 198 198 if (!candidate.isEmpty()) … … 200 200 } 201 201 202 picture->setMediaQueryDynamicResults(WTFMove(mediaQueryDynamicResults));203 204 // FIXME: This is not handling accessibility settings dependent media queries.205 if (picture->hasViewportDependentResults())206 document().addViewportDependentPicture(*picture);207 if (picture->hasAppearanceDependentResults())208 document().addAppearanceDependentPicture(*picture);209 210 202 return candidate; 211 203 } 212 204 205 void HTMLImageElement::evaluateDynamicMediaQueryDependencies() 206 { 207 auto documentElement = makeRefPtr(document().documentElement()); 208 MediaQueryEvaluator evaluator { document().printing() ? "print" : "screen", document(), documentElement ? documentElement->computedStyle() : nullptr }; 209 210 if (!evaluator.evaluateForChanges(m_mediaQueryDynamicResults)) 211 return; 212 213 selectImageSource(); 214 } 215 213 216 void HTMLImageElement::selectImageSource() 214 217 { 218 m_mediaQueryDynamicResults = { }; 219 document().removeDynamicMediaQueryDependentImage(*this); 220 215 221 // First look for the best fit source from our <picture> parent if we have one. 216 222 ImageCandidate candidate = bestFitSourceFromPictureElement(); 217 223 if (candidate.isEmpty()) { 218 224 // If we don't have a <picture> or didn't find a source, then we use our own attributes. 219 auto sourceSize = SizesAttributeParser(attributeWithoutSynchronization(sizesAttr).string(), document()).length(); 225 SizesAttributeParser sizesParser(attributeWithoutSynchronization(sizesAttr).string(), document(), &m_mediaQueryDynamicResults); 226 auto sourceSize = sizesParser.length(); 220 227 candidate = bestFitSourceForImageAttributes(document().deviceScaleFactor(), attributeWithoutSynchronization(srcAttr), attributeWithoutSynchronization(srcsetAttr), sourceSize); 221 228 } 222 229 setBestFitURLAndDPRFromImageCandidate(candidate); 223 230 m_imageLoader->updateFromElementIgnoringPreviousError(); 231 232 if (!m_mediaQueryDynamicResults.isEmpty()) 233 document().addDynamicMediaQueryDependentImage(*this); 224 234 } 225 235 … … 672 682 void HTMLImageElement::didMoveToNewDocument(Document& oldDocument, Document& newDocument) 673 683 { 684 oldDocument.removeDynamicMediaQueryDependentImage(*this); 685 674 686 m_imageLoader->elementDidMoveToNewDocument(); 675 687 HTMLElement::didMoveToNewDocument(oldDocument, newDocument); -
trunk/Source/WebCore/html/HTMLImageElement.h
r251708 r252828 29 29 #include "GraphicsTypes.h" 30 30 #include "HTMLElement.h" 31 #include "MediaQueryEvaluator.h" 32 #include <wtf/WeakPtr.h> 31 33 32 34 namespace WebCore { … … 131 133 bool isDroppedImagePlaceholder() const { return m_isDroppedImagePlaceholder; } 132 134 void setIsDroppedImagePlaceholder() { m_isDroppedImagePlaceholder = true; } 135 136 void evaluateDynamicMediaQueryDependencies(); 133 137 134 138 protected: … … 199 203 RefPtr<EditableImageReference> m_editableImage; 200 204 WeakPtr<HTMLPictureElement> m_pictureElement; 205 MediaQueryDynamicResults m_mediaQueryDynamicResults; 201 206 202 207 #if ENABLE(ATTACHMENT_ELEMENT) -
trunk/Source/WebCore/html/HTMLPictureElement.cpp
r252736 r252828 45 45 HTMLPictureElement::~HTMLPictureElement() 46 46 { 47 document().removeViewportDependentPicture(*this);48 document().removeAppearanceDependentPicture(*this);49 47 } 50 48 51 49 void HTMLPictureElement::didMoveToNewDocument(Document& oldDocument, Document& newDocument) 52 50 { 53 oldDocument.removeViewportDependentPicture(*this);54 oldDocument.removeAppearanceDependentPicture(*this);55 51 HTMLElement::didMoveToNewDocument(oldDocument, newDocument); 56 52 sourcesChanged(); … … 66 62 for (auto& element : childrenOfType<HTMLImageElement>(*this)) 67 63 element.selectImageSource(); 68 }69 70 bool HTMLPictureElement::viewportChangeAffectedPicture() const71 {72 auto documentElement = makeRefPtr(document().documentElement());73 MediaQueryEvaluator evaluator { document().printing() ? "print" : "screen", document(), documentElement ? documentElement->computedStyle() : nullptr };74 for (auto& result : m_mediaQueryDynamicResults.viewport) {75 LOG(MediaQueries, "HTMLPictureElement %p viewportChangeAffectedPicture evaluating media queries", this);76 if (evaluator.evaluate(result.expression) != result.result)77 return true;78 }79 return false;80 }81 82 bool HTMLPictureElement::appearanceChangeAffectedPicture() const83 {84 auto documentElement = makeRefPtr(document().documentElement());85 MediaQueryEvaluator evaluator { document().printing() ? "print" : "screen", document(), documentElement ? documentElement->computedStyle() : nullptr };86 for (auto& result : m_mediaQueryDynamicResults.appearance) {87 LOG(MediaQueries, "HTMLPictureElement %p appearanceChangeAffectedPicture evaluating media queries", this);88 if (evaluator.evaluate(result.expression) != result.result)89 return true;90 }91 return false;92 64 } 93 65 -
trunk/Source/WebCore/html/HTMLPictureElement.h
r252736 r252828 27 27 28 28 #include "HTMLElement.h" 29 #include "MediaQueryEvaluator.h"30 29 31 30 namespace WebCore { … … 39 38 void sourcesChanged(); 40 39 41 42 void setMediaQueryDynamicResults(MediaQueryDynamicResults&& results) { m_mediaQueryDynamicResults = results; }43 44 bool hasViewportDependentResults() const { return m_mediaQueryDynamicResults.viewport.size(); }45 bool hasAppearanceDependentResults() const { return m_mediaQueryDynamicResults.appearance.size(); }46 47 bool viewportChangeAffectedPicture() const;48 bool appearanceChangeAffectedPicture() const;49 50 40 #if USE(SYSTEM_PREVIEW) 51 41 WEBCORE_EXPORT bool isSystemPreviewImage() const; … … 56 46 57 47 void didMoveToNewDocument(Document& oldDocument, Document& newDocument) final; 58 59 MediaQueryDynamicResults m_mediaQueryDynamicResults;60 48 }; 61 49 -
trunk/Source/WebCore/rendering/RenderImage.cpp
r251708 r252828 381 381 } 382 382 383 void RenderImage::setImageDevicePixelRatio(float factor) 384 { 385 if (m_imageDevicePixelRatio == factor) 386 return; 387 388 m_imageDevicePixelRatio = factor; 389 intrinsicSizeChanged(); 390 } 391 383 392 bool RenderImage::isShowingMissingOrImageError() const 384 393 { -
trunk/Source/WebCore/rendering/RenderImage.h
r250100 r252828 67 67 void setAltText(const String& altText) { m_altText = altText; } 68 68 69 inline void setImageDevicePixelRatio(float factor) { m_imageDevicePixelRatio = factor; }69 void setImageDevicePixelRatio(float factor); 70 70 float imageDevicePixelRatio() const { return m_imageDevicePixelRatio; } 71 71
Note: See TracChangeset
for help on using the changeset viewer.