predictors: Support fonts in resource_prefetch_predictors.

chrome/browser/predictors/resource_prefetch_predictor.cc

Index: chrome/browser/predictors/resource_prefetch_predictor.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.cc b/chrome/browser/predictors/resource_prefetch_predictor.cc
index cbabd8d6c1eb69d8a0c373446944e4a88a17dfe9..861f3e875e748540913821a0bec933e93dfae90e 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 
+#include <algorithm>
+#include <array>
 #include <map>
 #include <set>
 #include <utility>
@@ -45,7 +47,7 @@ enum ResourceStatus {
   RESOURCE_STATUS_HANDLED = 0,
   RESOURCE_STATUS_NOT_HTTP_OR_HTTPS_PAGE = 1,
   RESOURCE_STATUS_NOT_HTTP_OR_HTTPS_RESOURCE = 2,
-  RESOURCE_STATUS_UNSUPPORTED_MIME_TYPE = 4,
+  RESOURCE_STATUS_UNSUPPORTED_RESOURCE_TYPE = 4,
   RESOURCE_STATUS_NOT_GET = 8,
   RESOURCE_STATUS_URL_TOO_LONG = 16,
   RESOURCE_STATUS_NOT_CACHEABLE = 32,
@@ -211,8 +213,10 @@ bool ResourcePrefetchPredictor::ShouldRecordResponse(
   if (!request_info->IsMainFrame())
     return false;
 
-  return request_info->GetResourceType() == content::RESOURCE_TYPE_MAIN_FRAME ?
-      IsHandledMainPage(response) : IsHandledSubresource(response);
+  content::ResourceType resource_type = request_info->GetResourceType();
+  return resource_type == content::RESOURCE_TYPE_MAIN_FRAME
+             ? IsHandledMainPage(response)
+             : IsHandledSubresource(response, resource_type);
 }
 
 // static
@@ -237,7 +241,8 @@ bool ResourcePrefetchPredictor::IsHandledMainPage(net::URLRequest* request) {
 
 // static
 bool ResourcePrefetchPredictor::IsHandledSubresource(
-    net::URLRequest* response) {
+    net::URLRequest* response,
+    content::ResourceType resource_type) {
   int resource_status = 0;
 
   if (!response->first_party_for_cookies().SchemeIsHTTPOrHTTPS())
@@ -248,11 +253,8 @@ bool ResourcePrefetchPredictor::IsHandledSubresource(
 
   std::string mime_type;
   response->GetMimeType(&mime_type);
-  if (!mime_type.empty() && !mime_util::IsSupportedImageMimeType(mime_type) &&
-      !mime_util::IsSupportedJavascriptMimeType(mime_type) &&
-      !net::MatchesMimeType("text/css", mime_type)) {
-    resource_status |= RESOURCE_STATUS_UNSUPPORTED_MIME_TYPE;
-  }
+  if (!IsHandledResourceType(resource_type, mime_type))
+    resource_status |= RESOURCE_STATUS_UNSUPPORTED_RESOURCE_TYPE;
 
   if (response->method() != "GET")
     resource_status |= RESOURCE_STATUS_NOT_GET;
@@ -276,6 +278,25 @@ bool ResourcePrefetchPredictor::IsHandledSubresource(
 }
 
 // static
+bool ResourcePrefetchPredictor::IsHandledResourceType(
+    content::ResourceType resource_type,
+    const std::string& mime_type) {
+  bool handled = resource_type == content::RESOURCE_TYPE_STYLESHEET ||
+                 resource_type == content::RESOURCE_TYPE_SCRIPT ||
+                 resource_type == content::RESOURCE_TYPE_IMAGE ||
+                 resource_type == content::RESOURCE_TYPE_FONT_RESOURCE;
+  // Need this because content::RESOURCE_TYPE_{PREFETCH,SUB_RESOURCE} can be
+  // anything.

[pasko, 2016/07/28 18:42:57]

This would be an easier to read comment:

// Restrict content::RESOURCE_TYPE_{PREFETCH,SUB_RESOURCE} to a small set of
mime types.

.. just because words like "anything" and "this" need extra pattern matching..

[Benoit L, 2016/07/29 08:56:27]

Done.

+  if (!handled && (resource_type == content::RESOURCE_TYPE_PREFETCH ||

[pasko, 2016/07/28 18:42:57]

one of these values for resource_type implies (!handled), checking for it looks
slightly confusing

[Benoit L, 2016/07/29 08:56:27]

Done.

+                   resource_type == content::RESOURCE_TYPE_SUB_RESOURCE)) {
+    content::ResourceType resource_type_from_mime = GetResourceTypeFromMimeType(
+        mime_type, content::RESOURCE_TYPE_LAST_TYPE);
+    handled = resource_type_from_mime != content::RESOURCE_TYPE_LAST_TYPE;
+  }
+  return handled;
+}
+
+// static
 bool ResourcePrefetchPredictor::IsCacheable(const net::URLRequest* response) {
   if (response->was_cached())
     return true;
@@ -296,14 +317,36 @@ bool ResourcePrefetchPredictor::IsCacheable(const net::URLRequest* response) {
 content::ResourceType ResourcePrefetchPredictor::GetResourceTypeFromMimeType(
     const std::string& mime_type,
     content::ResourceType fallback) {
-  if (mime_util::IsSupportedImageMimeType(mime_type))
+  // Sorted by decreasing likelihood according to HTTP archive.
+  static const std::array<std::string, 12> font_mime_types = {

[pasko, 2016/07/28 18:42:57]

nit: it would be less error prone to avoid indicating the array length constant
here (12).

One way is to replace std::array with std::vector.

Another is:

static std::string font_mime_types[] = {
  ...
};

if (std::any_of(std::begin(font_mime_types), std::end(font_mime_types),
    ...

[Benoit L, 2016/07/29 08:56:27]

Done.

+      "font/woff2",
+      "application/x-font-woff",
+      "application/font-woff",
+      "application/font-woff2",
+      "font/x-woff",
+      "application/x-font-ttf",
+      "font/woff",
+      "font/ttf",
+      "application/x-font-otf",
+      "x-font/woff",
+      "application/font-sfnt",
+      "application/font-ttf"};
+
+  if (mime_type.empty()) {
+    return fallback;
+  } else if (mime_util::IsSupportedImageMimeType(mime_type)) {
     return content::RESOURCE_TYPE_IMAGE;
-  else if (mime_util::IsSupportedJavascriptMimeType(mime_type))
+  } else if (mime_util::IsSupportedJavascriptMimeType(mime_type)) {
     return content::RESOURCE_TYPE_SCRIPT;
-  else if (net::MatchesMimeType("text/css", mime_type))
+  } else if (net::MatchesMimeType("text/css", mime_type)) {
     return content::RESOURCE_TYPE_STYLESHEET;
-  else
-    return fallback;
+  } else if (std::any_of(font_mime_types.begin(), font_mime_types.end(),
+                       [&mime_type](const std::string& mime) {
+                         return net::MatchesMimeType(mime, mime_type);
+                       })) {

[pasko, 2016/07/28 18:42:57]

is it the way git cl format prefers this?

[Benoit L, 2016/07/29 08:56:27]

Yes.

+    return content::RESOURCE_TYPE_FONT_RESOURCE;
+  }
+  return fallback;
 }
 
 ////////////////////////////////////////////////////////////////////////////////