Fix HTMLPreloadScanner handling of type in link preload.

Handling of the `type` attribute in `<link rel=preload>` was not
properly added, which meant preloads that included a valid type were
dismissed due to rel=stylesheet related logic.

This CL adds proper handling for the `type` attribute in
`<link rel=preload>` for fonts, images and styles.

BUG=659640

Review-Url: https://codereview.chromium.org/2620993002
Cr-Commit-Position: refs/heads/master@{#442887}

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

@@ -51,6 +51,7 @@
 #include "platform/instrumentation/tracing/TraceEvent.h"
 #include "platform/network/mime/ContentType.h"
 #include "platform/network/mime/MIMETypeRegistry.h"
+#include "wtf/Optional.h"
 #include <memory>
 
 namespace blink {
@@ -120,7 +121,7 @@ static String initiatorFor(const StringImpl* tagImpl) {
     return scriptTag.localName();
   if (match(tagImpl, videoTag))
     return videoTag.localName();
-  ASSERT_NOT_REACHED();
+  NOTREACHED();
   return emptyString();
 }
 
@@ -205,13 +206,17 @@ class TokenPreloadScanner::StartTagScanner {
       const ReferrerPolicy documentReferrerPolicy) {
     PreloadRequest::RequestType requestType =
         PreloadRequest::RequestTypePreload;
+    WTF::Optional<Resource::Type> type;
     if (shouldPreconnect()) {
       requestType = PreloadRequest::RequestTypePreconnect;
     } else {
       if (isLinkRelPreload()) {
         requestType = PreloadRequest::RequestTypeLinkRelPreload;
+        type = resourceTypeForLinkPreload();
+        if (type == WTF::nullopt)
+          return nullptr;
       }
-      if (!shouldPreload()) {
+      if (!shouldPreload(type)) {
         return nullptr;
       }
     }
@@ -230,9 +235,8 @@ class TokenPreloadScanner::StartTagScanner {
       resourceWidth.isSet = true;
     }
 
-    Resource::Type type;
-    if (!resourceType(type))
-      return nullptr;
+    if (type == WTF::nullopt)
+      type = resourceType();
 
     // The element's 'referrerpolicy' attribute (if present) takes precedence
     // over the document's referrer policy.
@@ -240,8 +244,9 @@ class TokenPreloadScanner::StartTagScanner {
                                         ? m_referrerPolicy
                                         : documentReferrerPolicy;
     auto request = PreloadRequest::createIfNeeded(
-        initiatorFor(m_tagImpl), position, m_urlToLoad, predictedBaseURL, type,
-        referrerPolicy, resourceWidth, clientHintsPreferences, requestType);
+        initiatorFor(m_tagImpl), position, m_urlToLoad, predictedBaseURL,
+        type.value(), referrerPolicy, resourceWidth, clientHintsPreferences,
+        requestType);
     if (!request)
       return nullptr;
 
@@ -349,8 +354,7 @@ class TokenPreloadScanner::StartTagScanner {
     } else if (match(attributeName, asAttr)) {
       m_asAttributeValue = attributeValue.lower();
     } else if (match(attributeName, typeAttr)) {
-      m_matched &= MIMETypeRegistry::isSupportedStyleSheetMIMEType(
-          ContentType(attributeValue).type());
+      m_typeAttributeValue = attributeValue;
     } else if (!m_referrerPolicySet &&
                match(attributeName, referrerpolicyAttr) &&
                !attributeValue.isNull()) {
@@ -443,25 +447,26 @@ class TokenPreloadScanner::StartTagScanner {
     return m_charset;
   }
 
-  bool resourceType(Resource::Type& type) const {
+  WTF::Optional<Resource::Type> resourceTypeForLinkPreload() const {
+    DCHECK(m_linkIsPreload);
+    return LinkLoader::getResourceTypeFromAsAttribute(m_asAttributeValue);
+  }
+
+  Resource::Type resourceType() const {
     if (match(m_tagImpl, scriptTag)) {
-      type = Resource::Script;
+      return Resource::Script;
     } else if (match(m_tagImpl, imgTag) || match(m_tagImpl, videoTag) ||
                (match(m_tagImpl, inputTag) && m_inputIsImage)) {
-      type = Resource::Image;
+      return Resource::Image;
     } else if (match(m_tagImpl, linkTag) && m_linkIsStyleSheet) {
-      type = Resource::CSSStyleSheet;
+      return Resource::CSSStyleSheet;
     } else if (m_linkIsPreconnect) {
-      type = Resource::Raw;
-    } else if (m_linkIsPreload) {
-      if (!LinkLoader::getResourceTypeFromAsAttribute(m_asAttributeValue, type))
-        return false;
+      return Resource::Raw;
     } else if (match(m_tagImpl, linkTag) && m_linkIsImport) {
-      type = Resource::ImportResource;
-    } else {
-      ASSERT_NOT_REACHED();
+      return Resource::ImportResource;
     }
-    return true;
+    NOTREACHED();
+    return Resource::Raw;
   }
 
   bool shouldPreconnect() const {
@@ -474,14 +479,39 @@ class TokenPreloadScanner::StartTagScanner {
            !m_urlToLoad.isEmpty();
   }
 
-  bool shouldPreload() const {
+  bool shouldPreloadLink(WTF::Optional<Resource::Type>& type) const {
+    if (m_linkIsStyleSheet) {
+      return m_typeAttributeValue.isEmpty() ||
+             MIMETypeRegistry::isSupportedStyleSheetMIMEType(
+                 ContentType(m_typeAttributeValue).type());
+    } else if (m_linkIsPreload) {
+      if (m_typeAttributeValue.isEmpty())
+        return true;
+      String typeFromAttribute = ContentType(m_typeAttributeValue).type();
+      if ((type == Resource::Font &&
+           !MIMETypeRegistry::isSupportedFontMIMEType(typeFromAttribute)) ||
+          (type == Resource::Image &&
+           !MIMETypeRegistry::isSupportedImagePrefixedMIMEType(
+               typeFromAttribute)) ||
+          (type == Resource::CSSStyleSheet &&
+           !MIMETypeRegistry::isSupportedStyleSheetMIMEType(
+               typeFromAttribute))) {
+        return false;
+      }
+    } else if (!m_linkIsImport) {
+      return false;
+    }
+
+    return true;
+  }
+
+  bool shouldPreload(WTF::Optional<Resource::Type>& type) const {
     if (m_urlToLoad.isEmpty())
       return false;
     if (!m_matched)
       return false;
-    if (match(m_tagImpl, linkTag) && !m_linkIsStyleSheet && !m_linkIsImport &&
-        !m_linkIsPreload)
-      return false;
+    if (match(m_tagImpl, linkTag))
+      return shouldPreloadLink(type);
     if (match(m_tagImpl, inputTag) && !m_inputIsImage)
       return false;
     if (match(m_tagImpl, scriptTag) &&

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

@@ -723,12 +723,33 @@ TEST_F(HTMLPreloadScannerTest, testLinkRelPreload) {
        "http://example.test/", Resource::Raw, 0},
       {"http://example.test", "<link rel=preload href=bla as=script>", "bla",
        "http://example.test/", Resource::Script, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=script type='script/foo'>", "bla",
+       "http://example.test/", Resource::Script, 0},
       {"http://example.test", "<link rel=preload href=bla as=style>", "bla",
        "http://example.test/", Resource::CSSStyleSheet, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=style type='text/css'>", "bla",
+       "http://example.test/", Resource::CSSStyleSheet, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=style type='text/bla'>", nullptr,
+       "http://example.test/", Resource::CSSStyleSheet, 0},
       {"http://example.test", "<link rel=preload href=bla as=image>", "bla",
        "http://example.test/", Resource::Image, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=image type='image/webp'>", "bla",
+       "http://example.test/", Resource::Image, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=image type='image/bla'>", nullptr,
+       "http://example.test/", Resource::Image, 0},
       {"http://example.test", "<link rel=preload href=bla as=font>", "bla",
        "http://example.test/", Resource::Font, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=font type='font/woff2'>", "bla",
+       "http://example.test/", Resource::Font, 0},
+      {"http://example.test",
+       "<link rel=preload href=bla as=font type='font/bla'>", nullptr,
+       "http://example.test/", Resource::Font, 0},
       {"http://example.test", "<link rel=preload href=bla as=media>", "bla",
        "http://example.test/", Resource::Media, 0},
       {"http://example.test", "<link rel=preload href=bla as=track>", "bla",

third_party/WebKit/Source/core/loader/LinkLoader.cpp

@@ -182,27 +182,25 @@ static void preconnectIfNeeded(
   }
 }
 
-bool LinkLoader::getResourceTypeFromAsAttribute(const String& as,
-                                                Resource::Type& type) {
+WTF::Optional<Resource::Type> LinkLoader::getResourceTypeFromAsAttribute(
+    const String& as) {
   DCHECK_EQ(as.lower(), as);
   if (as == "image") {
-    type = Resource::Image;
+    return Resource::Image;
   } else if (as == "script") {
-    type = Resource::Script;
+    return Resource::Script;
   } else if (as == "style") {
-    type = Resource::CSSStyleSheet;
+    return Resource::CSSStyleSheet;
   } else if (as == "media") {
-    type = Resource::Media;
+    return Resource::Media;
   } else if (as == "font") {
-    type = Resource::Font;
+    return Resource::Font;
   } else if (as == "track") {
-    type = Resource::TextTrack;
-  } else {
-    type = Resource::Raw;
-    if (!as.isEmpty())
-      return false;
+    return Resource::TextTrack;
+  } else if (as.isEmpty()) {
+    return Resource::Raw;
   }
-  return true;
+  return WTF::nullopt;
 }
 
 void LinkLoader::createLinkPreloadResourceClient(Resource* resource) {
@@ -300,8 +298,9 @@ static Resource* preloadIfNeeded(const LinkRelAttribute& relAttribute,
   }
   if (caller == LinkCalledFromHeader)
     UseCounter::count(document, UseCounter::LinkHeaderPreload);
-  Resource::Type resourceType;
-  if (!LinkLoader::getResourceTypeFromAsAttribute(as, resourceType)) {
+  Optional<Resource::Type> resourceType =
+      LinkLoader::getResourceTypeFromAsAttribute(as);
+  if (resourceType == WTF::nullopt) {
     document.addConsoleMessage(ConsoleMessage::create(
         OtherMessageSource, WarningMessageLevel,
         String("<link rel=preload> must have a valid `as` value")));
@@ -309,15 +308,15 @@ static Resource* preloadIfNeeded(const LinkRelAttribute& relAttribute,
     return nullptr;
   }
 
-  if (!isSupportedType(resourceType, mimeType)) {
+  if (!isSupportedType(resourceType.value(), mimeType)) {
     document.addConsoleMessage(ConsoleMessage::create(
         OtherMessageSource, WarningMessageLevel,
         String("<link rel=preload> has an unsupported `type` value")));
     return nullptr;
   }
   ResourceRequest resourceRequest(document.completeURL(href));
-  ResourceFetcher::determineRequestContext(resourceRequest, resourceType,
-                                           false);
+  ResourceFetcher::determineRequestContext(resourceRequest,
+                                           resourceType.value(), false);
 
   if (referrerPolicy != ReferrerPolicyDefault) {
     resourceRequest.setHTTPReferrer(SecurityPolicy::generateReferrer(
@@ -339,7 +338,7 @@ static Resource* preloadIfNeeded(const LinkRelAttribute& relAttribute,
   }
   linkRequest.setForPreload(true, monotonicallyIncreasingTime());
   linkRequest.setLinkPreload(true);
-  return document.loader()->startPreload(resourceType, linkRequest);
+  return document.loader()->startPreload(resourceType.value(), linkRequest);
 }
 
 static Resource* prefetchIfNeeded(Document& document,

third_party/WebKit/Source/core/loader/LinkLoader.h

@@ -41,6 +41,7 @@
 #include "platform/PrerenderClient.h"
 #include "platform/Timer.h"
 #include "platform/heap/Handle.h"
+#include "wtf/Optional.h"
 
 namespace blink {
 
@@ -101,7 +102,8 @@ class CORE_EXPORT LinkLoader final
                                   CanLoadResources,
                                   MediaPreloadPolicy,
                                   ViewportDescriptionWrapper*);
-  static bool getResourceTypeFromAsAttribute(const String& as, Resource::Type&);
+  static WTF::Optional<Resource::Type> getResourceTypeFromAsAttribute(
+      const String& as);
 
   DECLARE_TRACE();