image-inline: don't inline shortcut images

Because shortcut images don't block page rendering and generally aren't that important the tradeoffs are different for them, and we shouldn't inline them.

Fixes https://github.com/pagespeed/mod_pagespeed/issues/1022

install/mod_pagespeed_test/shortcut_icons.html

@@ -0,0 +1,9 @@
+<link rel="icon"
+      href="../mod_pagespeed_example/images/Cuppa.png">
+<link rel="apple-touch-icon"
+      href="../mod_pagespeed_example/images/Cuppa.png">
+<link rel="apple-touch-icon-precomposed"
+      href="../mod_pagespeed_example/images/Cuppa.png">
+<link rel="apple-touch-startup-image"
+      href="../mod_pagespeed_example/images/Cuppa.png">
+<img src="../mod_pagespeed_example/images/Cuppa.png">

net/instaweb/rewriter/image_rewrite_filter.cc

@@ -365,6 +365,9 @@ const char* MessageForInlineResult(InlineResult inline_result) {
       // image will be deleted before the user sees it, so message won't be
       // useful.
       break;
+    case INLINE_SHORTCUT:
+      message = "The image was not inlined because it is a shortcut icon.";
+      break;
     case INLINE_INTERNAL_ERROR:
       message = "The image was not inlined because the internal data was "
         "corrupted.";
@@ -1606,6 +1609,15 @@ bool ImageRewriteFilter::FinishRewriteImageUrl(
       StringPiece(responsive_attr) !=
       ResponsiveImageFirstFilter::kInlinableVirtualImage) {
     *inline_result = INLINE_RESPONSIVE;
+  } else if (element->keyword() == HtmlName::kLink) {
+    // Don't inline shortcut images.  All shortcut images are on link tags, and
+    // no non-shortcut images are on link tags, so we can just check if this is
+    // a link tag.  This is to exclude inlining on:
+    // * <link rel=icon ...>
+    // * <link rel=apple-touch-icon ...>
+    // * <link rel=apple-touch-icon-precomposed ...>
+    // * <link rel=apple-touch-startup-image ...>
+    *inline_result = INLINE_SHORTCUT;
   } else {
     // See if we have a data URL, and if so use it if the browser can handle it
     // TODO(jmaessen): get rid of a string copy here. Tricky because

net/instaweb/rewriter/public/image_rewrite_filter.h

@@ -44,6 +44,7 @@
 
 namespace net_instaweb {
 
+// See MessageForInlineResult for enum meanings.
 enum InlineResult {
   INLINE_SUCCESS,
   INLINE_UNSUPPORTED_DEVICE,
@@ -53,6 +54,7 @@ enum InlineResult {
   INLINE_CACHE_SMALL_IMAGES_UNREWRITTEN,
   // Image should not be inlined because it is part of a responsive image.
   INLINE_RESPONSIVE,
+  INLINE_SHORTCUT,
   INLINE_INTERNAL_ERROR,
 };
 

pagespeed/automatic/shortcut_icons.sh

@@ -0,0 +1,29 @@
+start_test "shortcut icons aren't inlined"
+
+URL="$TEST_ROOT/shortcut_icons.html?PageSpeedFilters=debug,rewrite_images"
+
+# We expect to see just one inline image, because of the img tag.
+fetch_until -save $URL 'grep -c data:image/png' 1
+
+OUT=$(cat $FETCH_UNTIL_OUTFILE)
+
+# All four icon link tags should be optimized but not inlined.
+check_from "$OUT" grep \
+  '<link rel="icon" href="[^<]*xCuppa.png.pagespeed.ic'
+check_from "$OUT" grep \
+  '<link rel="apple-touch-icon" href="[^<]*xCuppa.png.pagespeed.ic'
+check_from "$OUT" grep \
+  '<link rel="apple-touch-icon-precomposed" href="[^<]*xCuppa.png.pagespeed.ic'
+check_from "$OUT" grep \
+  '<link rel="apple-touch-startup-image" href="[^<]*xCuppa.png.pagespeed.ic'
+
+# The image tag should have been inlined instead.
+check_not_from "$OUT" grep \
+  '<img src="[^<]*xCuppa.png.pagespeed.ic'
+
+function appears_exactly_four_times() {
+  test $(grep -c "$@") = 4
+}
+
+check_from "$OUT" appears_exactly_four_times \
+  "The image was not inlined because it is a shortcut icon."

pagespeed/automatic/system_test.sh

@@ -58,6 +58,7 @@ run_test broken_images
 run_test make_show_ads_async
 run_test mobilizer
 run_test responsive_images
+run_test shortcut_icons
 
 # These have to run after image_rewrite tests. Otherwise it causes some images
 # to be loaded into memory before they should be.