core(icons): Add PNG check to icon validation

lighthouse-core/gather/computed/manifest-values.js

@@ -37,15 +37,15 @@ class ManifestValues extends ComputedArtifact {
       },
       {
         id: 'hasIconsAtLeast192px',
-        failureText: 'Manifest does not have icons at least 192px',
+        failureText: 'Manifest does not have a PNG icon of at least 192px',
         validate: manifestValue => icons.doExist(manifestValue) &&
-            icons.sizeAtLeast(192, manifestValue).length > 0,
+            icons.pngSizedAtLeast(192, manifestValue).length > 0,
       },
       {
         id: 'hasIconsAtLeast512px',
-        failureText: 'Manifest does not have icons at least 512px',
+        failureText: 'Manifest does not have a PNG icon of at least 512px',
         validate: manifestValue => icons.doExist(manifestValue) &&
-            icons.sizeAtLeast(512, manifestValue).length > 0,
+            icons.pngSizedAtLeast(512, manifestValue).length > 0,
       },
       {
         id: 'hasPWADisplayValue',

lighthouse-core/lib/icons.js

@@ -5,6 +5,8 @@
  */
 'use strict';
 
+const URL = require('./url-shim.js');
+
 /**
  * @param {NonNullable<LH.Artifacts.Manifest['value']>} manifest
  * @return {boolean} Does the manifest have any icons?
@@ -24,17 +26,26 @@ function doExist(manifest) {
  * @param {NonNullable<LH.Artifacts.Manifest['value']>} manifest
  * @return {Array<string>} Value of satisfactory sizes (eg. ['192x192', '256x256'])
  */
-function sizeAtLeast(sizeRequirement, manifest) {
+function pngSizedAtLeast(sizeRequirement, manifest) {
   // An icon can be provided for a single size, or for multiple sizes.
   // To handle both, we flatten all found sizes into a single array.
   const iconValues = manifest.icons.value;
   /** @type {Array<string>} */
   const flattenedSizes = [];
-  iconValues.forEach(icon => {
-    if (icon.value.sizes.value) {
-      flattenedSizes.push(...icon.value.sizes.value);
-    }
-  });
+  iconValues
+    // filter out icons with a typehint that is not 'image/png'
+    .filter(icon => (!icon.value.type.value) ||
+      (icon.value.type.value &&
+      icon.value.type.value === 'image/png'))
+    // filter out icons that are not png
+    .filter(icon => icon.value.src.value &&
+      new URL(icon.value.src.value).pathname.endsWith('.png'))
+    .forEach(icon => {
+      // check that the icon has a size
+      if (icon.value.sizes.value) {
+        flattenedSizes.push(...icon.value.sizes.value);
+      }
+    });
 
   return flattenedSizes
       // discard sizes that are not AAxBB (eg. "any")
@@ -54,5 +65,5 @@ function sizeAtLeast(sizeRequirement, manifest) {
 
 module.exports = {
   doExist,
-  sizeAtLeast,
+  pngSizedAtLeast,
 };

lighthouse-core/test/audits/splash-screen-test.js

@@ -10,30 +10,26 @@ const assert = require('assert');
 const manifestParser = require('../../lib/manifest-parser');
 
 const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
+const manifestDirtyJpgSrc = JSON.stringify(require('../fixtures/manifest-dirty-jpg.json'));
 const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
-const exampleManifest = noUrlManifestParser(manifestSrc);
 
 const Runner = require('../../runner.js');
 
-function generateMockArtifacts() {
+/**
+ * @param {string} src
+ * @return {!ManifestNode<(!Manifest|undefined)>}
+ */
+function generateMockArtifacts(src = manifestSrc) {
+  const exampleManifest = manifestParser(src, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+
   const computedArtifacts = Runner.instantiateComputedArtifacts();
   const mockArtifacts = Object.assign({}, computedArtifacts, {
     Manifest: exampleManifest,
   });
   return mockArtifacts;
 }
 
-/**
- * Simple manifest parsing helper when the manifest URLs aren't material to the
- * test. Uses example.com URLs for testing.
- * @param {string} manifestSrc
- * @return {!ManifestNode<(!Manifest|undefined)>}
- */
-function noUrlManifestParser(manifestSrc) {
-  return manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
-}
-
 /* eslint-env jest */
 describe('PWA: splash screen audit', () => {
   describe('basics', () => {
@@ -95,9 +91,8 @@ describe('PWA: splash screen audit', () => {
       });
     });
 
-    it('fails when a manifest contains no background color', () => {
-      const artifacts = generateMockArtifacts();
-      artifacts.Manifest = noUrlManifestParser(JSON.stringify({
+    it('fails when a manifest contains invalid background color', () => {
+      const artifacts = generateMockArtifacts(JSON.stringify({
         background_color: 'no',
       }));
 
@@ -123,7 +118,18 @@ describe('PWA: splash screen audit', () => {
 
       return SplashScreenAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
-        assert.ok(result.explanation.includes('icons'), result.explanation);
+        assert.ok(result.explanation.includes('PNG icon'), result.explanation);
+      });
+    });
+
+    it('fails if icons were present, but no valid PNG present', () => {
+      const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
+
+      return SplashScreenAudit.audit(artifacts).then(result => {
+        assert.strictEqual(result.rawValue, false);
+        assert.ok(result.explanation.includes('PNG icon'), result.explanation);
+        const failures = result.details.items[0].failures;
+        assert.strictEqual(failures.length, 1, failures);
       });
     });
   });

lighthouse-core/test/audits/webapp-install-banner-test.js

@@ -10,13 +10,15 @@ const assert = require('assert');
 const manifestParser = require('../../lib/manifest-parser');
 
 const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
+const manifestDirtyJpgSrc = JSON.stringify(require('../fixtures/manifest-dirty-jpg.json'));
 const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
-const exampleManifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
 
 const Runner = require('../../runner.js');
 
-function generateMockArtifacts() {
+function generateMockArtifacts(src = manifestSrc) {
+  const exampleManifest = manifestParser(src, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+
   const computedArtifacts = Runner.instantiateComputedArtifacts();
   const clonedArtifacts = JSON.parse(JSON.stringify({
     Manifest: exampleManifest,
@@ -118,13 +120,24 @@ describe('PWA: webapp install banner audit', () => {
 
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
-        assert.ok(result.explanation.includes('icons'), result.explanation);
+        assert.ok(result.explanation.includes('PNG icon'), result.explanation);
         const failures = result.details.items[0].failures;
         assert.strictEqual(failures.length, 1, failures);
       });
     });
   });
 
+  it('fails if icons were present, but no valid PNG present', () => {
+    const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
+
+    return WebappInstallBannerAudit.audit(artifacts).then(result => {
+      assert.strictEqual(result.rawValue, false);
+      assert.ok(result.explanation.includes('PNG icon'), result.explanation);
+      const failures = result.details.items[0].failures;
+      assert.strictEqual(failures.length, 1, failures);
+    });
+  });
+
   it('fails if page had no SW', () => {
     const artifacts = generateMockArtifacts();
     artifacts.ServiceWorker.versions = [];

lighthouse-core/test/fixtures/manifest-dirty-jpg.json

@@ -0,0 +1,31 @@
+{
+  "short_name": "ExApp",
+  "name": "Example App",
+  "start_url": "./",
+  "icons": [
+    {
+      "src": "/images/chrome-touch-icon-96x96.png",
+      "sizes": "96x96",
+      "type": "image/png"
+    },
+    {
+      "src": "/images/chrome-touch-icon-192x192.ico",
+      "sizes": "192x192",
+      "type": "image/x-icon"
+    },
+     {
+      "src": "/images/chrome-touch-icon-512x512.jpg",
+      "sizes": "512x512",
+      "type": "image/jpg"
+    },
+    {
+      "src": "/images/chrome-touch-icon-128x128.png",
+      "sizes": "64x64 128x128",
+      "type": "image/png"
+    }
+  ],
+  "background_color": "#FAFAFA",
+  "theme_color": "#123123",
+  "display": "standalone",
+  "orientation": "portrait"
+}