Avoid downloading unsupported `<source>` types in HTMLPreloadScanner

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

Index: third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
diff --git a/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp b/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
index e3ffd2fb0983ad5e8d71ec9b855c0007d27b35b7..a5ac57a5307519caf2b2565b276607f1622806b3 100644
--- a/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
+++ b/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
@@ -143,7 +143,7 @@ public:
         , m_linkIsPreconnect(false)
         , m_linkIsPreload(false)
         , m_linkIsImport(false)
-        , m_matchedMediaAttribute(true)
+        , m_matched(true)
         , m_inputIsImage(false)
         , m_sourceSize(0)
         , m_sourceSizeSet(false)
@@ -192,7 +192,7 @@ public:
 
     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;
@@ -211,7 +211,7 @@ public:
             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;
             }
         }
@@ -307,7 +307,7 @@ private:
             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)) {
@@ -341,7 +341,9 @@ private:
             }
         } 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());
         }
     }
 
@@ -460,7 +462,7 @@ private:
     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;