Avoid downloading unsupported `<source>` types in HTMLPreloadScanner

third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  * Copyright (C) 2009 Torch Mobile, Inc. http://www.torchmobile.com/
  * Copyright (C) 2010 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
@@ skipping 125 matching lines @@
 class TokenPreloadScanner::StartTagScanner {
     STACK_ALLOCATED();
 public:
     StartTagScanner(const StringImpl* tagImpl, MediaValuesCached* mediaValues)
         : m_tagImpl(tagImpl)
         , m_linkIsStyleSheet(false)
         , m_linkTypeIsMissingOrSupportedStyleSheet(true)
         , m_linkIsPreconnect(false)
         , m_linkIsPreload(false)
         , m_linkIsImport(false)
-        , m_matchedMediaAttribute(true)
+        , m_matched(true)
         , m_inputIsImage(false)
         , m_sourceSize(0)
         , m_sourceSizeSet(false)
         , m_defer(FetchRequest::NoDefer)
         , m_crossOrigin(CrossOriginAttributeNotSet)
         , m_mediaValues(mediaValues)
         , m_referrerPolicySet(false)
         , m_referrerPolicy(ReferrerPolicyDefault)
     {
         if (match(m_tagImpl, imgTag)
@@ skipping 28 matching lines @@
     void processAttributes(const Vector<CompactHTMLToken::Attribute>& attributes)
     {
         if (!m_tagImpl)
             return;
         for (const CompactHTMLToken::Attribute& htmlTokenAttribute : attributes)
             processAttribute(htmlTokenAttribute.name(), htmlTokenAttribute.value());
     }
 
     void handlePictureSourceURL(PictureData& pictureData)
     {
-        if (match(m_tagImpl, sourceTag) && m_matchedMediaAttribute && pictureData.sourceURL.isEmpty()) {
+        if (match(m_tagImpl, sourceTag) && m_matched && pictureData.sourceURL.isEmpty()) {
             pictureData.sourceURL = m_srcsetImageCandidate.toString();
             pictureData.sourceSizeSet = m_sourceSizeSet;
             pictureData.sourceSize = m_sourceSize;
             pictureData.picked = true;
         } else if (match(m_tagImpl, imgTag) && !pictureData.sourceURL.isEmpty()) {
             setUrlToLoad(pictureData.sourceURL, AllowURLReplacement);
         }
     }
 
     PassOwnPtr<PreloadRequest> createPreloadRequest(const KURL& predictedBaseURL, const SegmentedString& source, const ClientHintsPreferences& clientHintsPreferences, const PictureData& pictureData, const ReferrerPolicy documentReferrerPolicy)
     {
         PreloadRequest::RequestType requestType = PreloadRequest::RequestTypePreload;
         if (shouldPreconnect()) {
             requestType = PreloadRequest::RequestTypePreconnect;
         } else {
             if (isLinkRelPreload()) {
                 requestType = PreloadRequest::RequestTypeLinkRelPreload;
             }
-            if (!shouldPreload() || !m_matchedMediaAttribute) {
+            if (!shouldPreload() || !m_matched) {

[kouhei (in TOK), 2016/05/30 10:44:31]

!m_matched check should go into shouldPreload()

[Yoav Weiss, 2016/05/30 11:17:36]

fair enough

                 return nullptr;
             }
         }
 
 
         TextPosition position = TextPosition(source.currentLine(), source.currentColumn());
         FetchRequest::ResourceWidth resourceWidth;
         float sourceSize = m_sourceSize;
         bool sourceSizeSet = m_sourceSizeSet;
         if (pictureData.picked) {
@@ skipping 75 matching lines @@
         // FIXME - Don't set rel/media/crossorigin multiple times.
         if (match(attributeName, hrefAttr)) {
             setUrlToLoad(attributeValue, DisallowURLReplacement);
         } else if (match(attributeName, relAttr)) {
             LinkRelAttribute rel(attributeValue);
             m_linkIsStyleSheet = rel.isStyleSheet() && !rel.isAlternate() && rel.getIconType() == InvalidIcon && !rel.isDNSPrefetch();
             m_linkIsPreconnect = rel.isPreconnect();
             m_linkIsPreload = rel.isLinkPreload();
             m_linkIsImport = rel.isImport();
         } else if (match(attributeName, mediaAttr)) {
-            m_matchedMediaAttribute = mediaAttributeMatches(*m_mediaValues, attributeValue);
+            m_matched = mediaAttributeMatches(*m_mediaValues, attributeValue);
         } else if (match(attributeName, crossoriginAttr)) {
             setCrossOrigin(attributeValue);
         } else if (match(attributeName, asAttr)) {
             m_asAttributeValue = attributeValue;
         } else if (match(attributeName, typeAttr)) {
             m_linkTypeIsMissingOrSupportedStyleSheet = MIMETypeRegistry::isSupportedStyleSheetMIMEType(ContentType(attributeValue).type());
         }
     }
 
     template<typename NameType>
@@ skipping 13 matching lines @@
             m_srcsetAttributeValue = attributeValue;
             m_srcsetImageCandidate = bestFitSourceForSrcsetAttribute(m_mediaValues->devicePixelRatio(), m_sourceSize, attributeValue);
         } else if (match(attributeName, sizesAttr) && !m_sourceSizeSet) {
             m_sourceSize = SizesAttributeParser(m_mediaValues, attributeValue).length();
             m_sourceSizeSet = true;
             if (!m_srcsetImageCandidate.isEmpty()) {
                 m_srcsetImageCandidate = bestFitSourceForSrcsetAttribute(m_mediaValues->devicePixelRatio(), m_sourceSize, m_srcsetAttributeValue);
             }
         } else if (match(attributeName, mediaAttr)) {
             // FIXME - Don't match media multiple times.
-            m_matchedMediaAttribute = mediaAttributeMatches(*m_mediaValues, attributeValue);
+            m_matched &= mediaAttributeMatches(*m_mediaValues, attributeValue);
+        } else if (match(attributeName, typeAttr)) {
+            m_matched &= MIMETypeRegistry::isSupportedImagePrefixedMIMEType(ContentType(attributeValue).type());
         }
     }
 
     template<typename NameType>
     void processVideoAttribute(const NameType& attributeName, const String& attributeValue)
     {
         if (match(attributeName, posterAttr))
             setUrlToLoad(attributeValue, DisallowURLReplacement);
     }
 
@@ skipping 98 matching lines @@
 
     const StringImpl* m_tagImpl;
     String m_urlToLoad;
     ImageCandidate m_srcsetImageCandidate;
     String m_charset;
     bool m_linkIsStyleSheet;
     bool m_linkTypeIsMissingOrSupportedStyleSheet;
     bool m_linkIsPreconnect;
     bool m_linkIsPreload;
     bool m_linkIsImport;
-    bool m_matchedMediaAttribute;
+    bool m_matched;

[kouhei (in TOK), 2016/05/30 10:44:31]

Can we use separate flags for those two. Instead of sharing the same bool?

[Yoav Weiss, 2016/05/30 11:17:36]

We could but why? Both `media` and `type` impact whether this resource is a
match, and if it isn't, it shouldn't be preloaded. What would breaking it up
give us?

[kouhei (in TOK), 2016/05/30 11:26:22]

I thought it was more verbose.
The current processLinkAttribute() implementation worried me a bit, as it used
"m_matched = " which is correct, but inconsistent w/ lines 34{4,6}.
I may be worrying too much. I'm ok with your original here.

     bool m_inputIsImage;
     String m_imgSrcUrl;
     String m_srcsetAttributeValue;
     String m_asAttributeValue;
     float m_sourceSize;
     bool m_sourceSizeSet;
     FetchRequest::DeferOption m_defer;
     CrossOriginAttributeValue m_crossOrigin;
     Member<MediaValuesCached> m_mediaValues;
     bool m_referrerPolicySet;
@@ skipping 329 matching lines @@
     ASSERT(document);
     doHtmlPreloadScanning = !document->settings() || document->settings()->doHtmlPreloadScanning();
     doDocumentWritePreloadScanning = doHtmlPreloadScanning && document->frame() && document->frame()->isMainFrame();
     defaultViewportMinWidth = document->viewportDefaultMinWidth();
     viewportMetaZeroValuesQuirk = document->settings() && document->settings()->viewportMetaZeroValuesQuirk();
     viewportMetaEnabled = document->settings() && document->settings()->viewportMetaEnabled();
     referrerPolicy = document->getReferrerPolicy();
 }
 
 } // namespace blink