Downloads: detect failed file icons and show default thumbnail

On the downloads page, we consult the system for a themed image for the
type of file downloads. Ex: zip files -> archive icon.

When a system doesn't know why type of icon to show for a specific file
type, it can either return a default or nothing. Right now, Linux
returns nothing, so we show nothing.  chrome://fileicon tries to warn
us by giving a network error (which has been ignored until now).

So, when a chrome://fileicon fails to load, show a default filetype
icon in respond.

Before & after screenshots: https://imgur.com/a/txRbfIG

R=dpapad@chromium.org
BUG=855259

Change-Id: I370fd8ea119b194ad8e1ba7e0dfad32c6cea1bbe
Reviewed-on: https://chromium-review.googlesource.com/c/1132560
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614581}

chrome/browser/browser_resources.grd

@@ -179,6 +179,8 @@
             <include name="IDR_MD_DOWNLOADS_CONSTANTS_JS" file="resources\md_downloads\constants.js" type="BINDATA" />
             <include name="IDR_MD_DOWNLOADS_DOWNLOADS_JS" file="resources\md_downloads\downloads.js" type="BINDATA" />
             <include name="IDR_MD_DOWNLOADS_I18N_SETUP_HTML" file="resources\md_downloads\i18n_setup.html" type="BINDATA" />
+            <include name="IDR_MD_DOWNLOADS_ICON_LOADER_HTML" file="resources\md_downloads\icon_loader.html" type="BINDATA" />
+            <include name="IDR_MD_DOWNLOADS_ICON_LOADER_JS" file="resources\md_downloads\icon_loader.js" type="BINDATA" />
             <include name="IDR_MD_DOWNLOADS_ICONS_HTML" file="resources\md_downloads\icons.html" type="BINDATA" />
             <include name="IDR_MD_DOWNLOADS_ITEM_HTML" file="resources\md_downloads\item.html" type="BINDATA" />
             <include name="IDR_MD_DOWNLOADS_ITEM_JS" file="resources\md_downloads\item.js" type="BINDATA" />

chrome/browser/resources/md_downloads/BUILD.gn

@@ -61,11 +61,20 @@ js_library("externs") {
   ]
 }
 
+js_library("icon_loader") {
+  deps = [
+    "//ui/webui/resources/js:assert",
+    "//ui/webui/resources/js:cr",
+    "//ui/webui/resources/js:icon",
+  ]
+}
+
 js_library("item") {
   deps = [
     ":browser_proxy",
     ":constants",
     ":externs",
+    ":icon_loader",
     "//third_party/polymer/v1_0/components-chromium/paper-behaviors:paper-inky-focus-behavior-extracted",
     "//ui/webui/resources/js:cr",
     "//ui/webui/resources/js:load_time_data",

chrome/browser/resources/md_downloads/icon_loader.html

+<link rel="import" href="chrome://resources/html/assert.html">
+<link rel="import" href="chrome://resources/html/cr.html">
+<link rel="import" href="chrome://resources/html/icon.html">
+<script src="chrome://downloads/icon_loader.js"></script>

chrome/browser/resources/md_downloads/icon_loader.js

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cr.define('downloads', function() {
+  class IconLoader {
+    constructor() {
+      /** @private {!Object<!PromiseResolver<boolean>>} */
+      this.iconResolvers_ = {};
+
+      /** @private {!Set<!HTMLImageElement>} */
+      this.listeningImages_ = new Set();
+    }
+
+    /**
+     * @param {!HTMLImageElement} imageEl
+     * @param {string} filePath
+     * @return {!Promise<boolean>} Whether or not the icon loaded successfully.
+     */
+    loadIcon(imageEl, filePath) {
+      const url = cr.icon.getFileIconUrl(filePath);
+
+      if (!this.iconResolvers_[url]) {
+        this.iconResolvers_[url] = new PromiseResolver();
+      }
+
+      if (!this.listeningImages_.has(imageEl)) {
+        imageEl.addEventListener('load', this.finishedLoading_.bind(this));
+        imageEl.addEventListener('error', this.finishedLoading_.bind(this));
+        this.listeningImages_.add(imageEl);
+      }
+
+      imageEl.src = url;
+
+      return assert(this.iconResolvers_[url]).promise;
+    }
+
+    /**
+     * @param {!Event} e
+     * @private
+     */
+    finishedLoading_(e) {
+      const resolver = assert(this.iconResolvers_[e.currentTarget.src]);
+      if (!resolver.isFulfilled)
+        resolver.resolve(e.type == 'load');
+    }
+  }
+
+  cr.addSingletonGetter(IconLoader);
+
+  return {IconLoader: IconLoader};
+});

chrome/browser/resources/md_downloads/item.html

@@ -16,6 +16,7 @@
 <link rel="import" href="chrome://downloads/browser_proxy.html">
 <link rel="import" href="chrome://downloads/constants.html">
 <link rel="import" href="chrome://downloads/i18n_setup.html">
+<link rel="import" href="chrome://downloads/icon_loader.html">
 <link rel="import" href="chrome://downloads/icons.html">
 
 <dom-module id="downloads-item">
@@ -93,11 +94,16 @@
         margin: 0 24px;
       }
 
-      .icon {
+      .icon,
+      #file-icon-wrapper {
         height: 32px;
         width: 32px;
       }
 
+      #file-icon-wrapper {
+        overflow: hidden;  /* Reduces file icon flicker on initial load. */
+      }
+
       #content:-webkit-any(.show-progress, .dangerous) #file-icon-wrapper {
         /* TODO(dbeam): animate from top-aligned to centered when items finish?
          */
@@ -110,12 +116,11 @@
         opacity: .5;
       }
 
-      #danger-icon {
-        height: 32px;
-        width: 32px;
+      #file-icon-wrapper iron-icon[icon='cr:insert-drive-file'] {
+        color: var(--paper-grey-400);
       }
 
-      #danger-icon[icon='cr:warning'],
+      #file-icon-wrapper iron-icon[icon='cr:warning'],
       .dangerous #description {
         color: var(--google-red-700);
       }
@@ -233,10 +238,10 @@
     <div id="content" on-dragstart="onDragStart_"
         class$="[[computeClass_(isActive_, isDangerous_, showProgress_)]]">
       <div id="file-icon-wrapper" class="icon-wrapper">
-        <img class="icon" id="file-icon" alt="" hidden="[[isDangerous_]]">
-        <iron-icon id="danger-icon"
-            icon$="[[computeDangerIcon_(isDangerous_, data.dangerType)]]"
-            hidden="[[!isDangerous_]]"></iron-icon>
+        <img class="icon" id="file-icon" alt="" hidden="[[!useFileIcon_]]">
+        <iron-icon class="icon" icon$="[[computeIcon_(
+            isDangerous_, data.dangerType, useFileIcon_)]]"
+            hidden="[[useFileIcon_]]"></iron-icon>
       </div>
 
       <div id="details">

chrome/browser/resources/md_downloads/item.js

@@ -85,6 +85,8 @@ cr.define('downloads', function() {
         type: Boolean,
         value: false,
       },
+
+      useFileIcon_: Boolean,
     },
 
     observers: [
@@ -103,6 +105,11 @@ cr.define('downloads', function() {
       this.content = this.$.content;
     },
 
+    /** @return {!HTMLElement} */
+    getFileIcon: function() {
+      return assert(this.$['file-icon']);
+    },
+
     /**
      * @param {string} url
      * @return {string} A reasonably long URL.
@@ -159,14 +166,6 @@ cr.define('downloads', function() {
           'controlRemoveFromListAriaLabel', this.data.fileName);
     },
 
-    /**
-     * @return {string}
-     * @private
-     */
-    computeDangerIcon_: function() {
-      return this.isDangerous_ ? 'cr:warning' : '';
-    },
-
     /**
      * @return {string}
      * @private
@@ -213,6 +212,18 @@ cr.define('downloads', function() {
       return '';
     },
 
+    /**
+     * @return {string}
+     * @private
+     */
+    computeIcon_: function() {
+      if (this.isDangerous_)
+        return 'cr:warning';
+      if (!this.useFileIcon_)
+        return 'cr:insert-drive-file';
+      return '';
+    },
+
     /**
      * @return {boolean}
      * @private
@@ -345,11 +356,16 @@ cr.define('downloads', function() {
 
       if (this.isDangerous_) {
         this.$.url.removeAttribute('href');
+        this.useFileIcon_ = false;
       } else {
         this.$.url.href = assert(this.data.url);
-        const filePath = encodeURIComponent(this.data.filePath);
-        const scaleFactor = `?scale=${window.devicePixelRatio}x`;
-        this.$['file-icon'].src = `chrome://fileicon/${filePath}${scaleFactor}`;
+        const path = this.data.filePath;
+        downloads.IconLoader.getInstance()
+            .loadIcon(this.$['file-icon'], path)
+            .then(success => {
+              if (path == this.data.filePath)
+                this.useFileIcon_ = success;
+            });
       }
     },
 

chrome/browser/resources/print_preview/data/destination.js

@@ -666,7 +666,7 @@ cr.define('print_preview', function() {
         return 'print-preview:save-to-drive';
       }
       if (this.id_ == Destination.GooglePromotedId.SAVE_AS_PDF) {
-        return 'print-preview:insert-drive-file';
+        return 'cr:insert-drive-file';
       }
       if (this.isEnterprisePrinter) {
         return 'print-preview:business';

chrome/browser/resources/print_preview/icons.html

@@ -27,7 +27,6 @@
       See http://goo.gl/Y1OdAq for instructions on adding additional icons.
       -->
       <g id="business"><path d="M12 7V3H2v18h20V7H12zM6 19H4v-2h2v2zm0-4H4v-2h2v2zm0-4H4V9h2v2zm0-4H4V5h2v2zm4 12H8v-2h2v2zm0-4H8v-2h2v2zm0-4H8V9h2v2zm0-4H8V5h2v2zm10 12h-8v-2h2v-2h-2v-2h2v-2h-2V9h8v10zm-2-8h-2v2h2v-2zm0 4h-2v2h2v-2z"></path></g>
-      <g id="insert-drive-file"><path d="M6 2c-1.1 0-1.99.9-1.99 2L4 20c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6H6zm7 7V3.5L18.5 9H13z"></path></g>
       <g id="smartphone"><path d="M17 1.01L7 1c-1.1 0-2 .9-2 2v18c0 1.1.9 2 2 2h10c1.1 0 2-.9 2-2V3c0-1.1-.9-1.99-2-1.99zM17 19H7V5h10v14z"></path></g>
       <g id="cloud-queue"><path d="M19.35 10.04C18.67 6.59 15.64 4 12 4 9.11 4 6.6 5.64 5.35 8.04 2.34 8.36 0 10.91 0 14c0 3.31 2.69 6 6 6h13c2.76 0 5-2.24 5-5 0-2.64-2.05-4.78-4.65-4.96zM19 18H6c-2.21 0-4-1.79-4-4s1.79-4 4-4h.71C7.37 7.69 9.48 6 12 6c3.04 0 5.5 2.46 5.5 5.5v.5H19c1.66 0 3 1.34 3 3s-1.34 3-3 3z"></path></g>
       <g id="print"><path d="M19 8H5c-1.66 0-3 1.34-3 3v6h4v4h12v-4h4v-6c0-1.66-1.34-3-3-3zm-3 11H8v-5h8v5zm3-7c-.55 0-1-.45-1-1s.45-1 1-1 1 .45 1 1-.45 1-1 1zm-1-9H6v4h12V3z"></path></g>

chrome/browser/resources/print_preview/new/app.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
 <link rel="import" href="chrome://resources/cr_elements/cr_container_shadow_behavior.html">
+<link rel="import" href="chrome://resources/cr_elements/icons.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 <link rel="import" href="chrome://resources/html/cr.html">

chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc

@@ -127,6 +127,9 @@ content::WebUIDataSource* CreateDownloadsUIHTMLSource(Profile* profile) {
   source->AddResourcePath("constants.js", IDR_MD_DOWNLOADS_CONSTANTS_JS);
   source->AddResourcePath("downloads.js", IDR_MD_DOWNLOADS_DOWNLOADS_JS);
   source->AddResourcePath("i18n_setup.html", IDR_MD_DOWNLOADS_I18N_SETUP_HTML);
+  source->AddResourcePath("icon_loader.html",
+                          IDR_MD_DOWNLOADS_ICON_LOADER_HTML);
+  source->AddResourcePath("icon_loader.js", IDR_MD_DOWNLOADS_ICON_LOADER_JS);
   source->AddResourcePath("icons.html", IDR_MD_DOWNLOADS_ICONS_HTML);
   source->AddResourcePath("item.html", IDR_MD_DOWNLOADS_ITEM_HTML);
   source->AddResourcePath("item.js", IDR_MD_DOWNLOADS_ITEM_JS);

chrome/test/data/webui/icon_test.html

@@ -33,6 +33,12 @@ function testGetFavicon_IconUrl() {
   assertEquals(expected, cr.icon.getFavicon(url));
 }
 
+function testGetFileIconUrl() {
+  assertEquals(cr.icon.getFileIconUrl('file path'),
+               'chrome://fileicon/file%20path?scale=' +
+                   window.devicePixelRatio + 'x');
+}
+
 </script>
 </body>
 </html>

chrome/test/data/webui/md_downloads/downloads_browsertest.js

@@ -47,6 +47,9 @@ DownloadsItemTest.prototype = {
 
   /** @override */
   extraLibraries: DownloadsTest.prototype.extraLibraries.concat([
+    ROOT_PATH + 'ui/webui/resources/js/cr.js',
+    '../test_browser_proxy.js',
+    'test_support.js',
     'item_tests.js',
   ]),
 };

chrome/test/data/webui/md_downloads/item_tests.js

@@ -6,23 +6,52 @@ suite('item tests', function() {
   /** @type {!downloads.Item} */
   let item;
 
+  /** @type {!TestIconLoader} */
+  let testIconLoader;
+
   setup(function() {
     PolymerTest.clearBody();
+
+    // This isn't strictly necessary, but is a probably good idea.
+    downloads.BrowserProxy.instance_ = new TestDownloadsProxy;
+
+    testIconLoader = new TestIconLoader;
+    downloads.IconLoader.instance_ = testIconLoader;
+
     item = document.createElement('downloads-item');
     document.body.appendChild(item);
   });
 
-  test('dangerous downloads aren\'t linkable', function() {
-    item.set('data', {
-      dangerType: downloads.DangerType.DANGEROUS_FILE,
-      fileExternallyRemoved: false,
-      hideDate: true,
-      state: downloads.States.DANGEROUS,
-      url: 'http://evil.com'
-    });
+  test('dangerous downloads aren\'t linkable', () => {
+    item.set('data', createDownload({
+               dangerType: downloads.DangerType.DANGEROUS_FILE,
+               fileExternallyRemoved: false,
+               hideDate: true,
+               state: downloads.States.DANGEROUS,
+               url: 'http://evil.com'
+             }));
     Polymer.dom.flush();
 
     assertTrue(item.$['file-link'].hidden);
     assertFalse(item.$.url.hasAttribute('href'));
   });
+
+  test('icon loads successfully', async () => {
+    testIconLoader.setShouldIconsLoad(true);
+    item.set('data', createDownload({filePath: 'unique1', hideDate: false}));
+    const loadedPath = await testIconLoader.whenCalled('loadIcon');
+    assertEquals(loadedPath, 'unique1');
+    Polymer.dom.flush();
+    assertFalse(item.getFileIcon().hidden);
+  });
+
+  test('icon fails to load', async () => {
+    testIconLoader.setShouldIconsLoad(false);
+    item.set('data', createDownload({filePath: 'unique2', hideDate: false}));
+    item.set('data', createDownload({hideDate: false}));
+    const loadedPath = await testIconLoader.whenCalled('loadIcon');
+    assertEquals(loadedPath, 'unique2');
+    Polymer.dom.flush();
+    assertTrue(item.getFileIcon().hidden);
+  });
 });

chrome/test/data/webui/md_downloads/test_support.js

@@ -81,6 +81,29 @@ class TestDownloadsMojoHandler {
   openDownloadsFolderRequiringGesture() {}
 }
 
+class TestIconLoader extends TestBrowserProxy {
+  constructor() {
+    super(['loadIcon']);
+
+    /** @private */
+    this.shouldIconsLoad_ = true;
+  }
+
+  /** @param {boolean} shouldIconsLoad */
+  setShouldIconsLoad(shouldIconsLoad) {
+    this.shouldIconsLoad_ = shouldIconsLoad;
+  }
+
+  /**
+   * @param {!HTMLImageElement} imageEl
+   * @param {string} filePath
+   */
+  loadIcon(imageEl, filePath) {
+    this.methodCalled('loadIcon', filePath);
+    return Promise.resolve(this.shouldIconsLoad_);
+  }
+}
+
 /**
  * @param {Object=} config
  * @return {!downloads.Data}

ui/webui/resources/cr_elements/icons.html

@@ -52,6 +52,7 @@ blurry at 20 px). Please use 20 px icons when available.
       <g id="fullscreen"><path d="M7 14H5v5h5v-2H7v-3zm-2-4h2V7h3V5H5v5zm12 7h-3v2h5v-5h-2v3zM14 5v2h3v3h2V5h-5z"></path></g>
       <g id="group"><path d="M16 11c1.66 0 2.99-1.34 2.99-3S17.66 5 16 5c-1.66 0-3 1.34-3 3s1.34 3 3 3zm-8 0c1.66 0 2.99-1.34 2.99-3S9.66 5 8 5C6.34 5 5 6.34 5 8s1.34 3 3 3zm0 2c-2.33 0-7 1.17-7 3.5V19h14v-2.5c0-2.33-4.67-3.5-7-3.5zm8 0c-.29 0-.62.02-.97.05 1.16.84 1.97 1.97 1.97 3.45V19h6v-2.5c0-2.33-4.67-3.5-7-3.5z"></path></g>
       <g id="info"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z"></path></g>
+      <g id="insert-drive-file"><path d="M6 2c-1.1 0-1.99.9-1.99 2L4 20c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6H6zm7 7V3.5L18.5 9H13z"></path></g>
       <g id="more-vert"><path d="M12 8c1.1 0 2-.9 2-2s-.9-2-2-2-2 .9-2 2 .9 2 2 2zm0 2c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2zm0 6c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2z"></path></g>
       <g id="open-in-new"><path d="M19 19H5V5h7V3H5c-1.11 0-2 .9-2 2v14c0 1.1.89 2 2 2h14c1.1 0 2-.9 2-2v-7h-2v7zM14 3v2h3.59l-9.83 9.83 1.41 1.41L19 6.41V10h2V3h-7z"></path></g>
       <g id="person"><path d="M12 12c2.21 0 4-1.79 4-4s-1.79-4-4-4-4 1.79-4 4 1.79 4 4 4zm0 2c-2.67 0-8 1.34-8 4v2h16v-2c0-2.66-5.33-4-8-4z"></path></g>

ui/webui/resources/js/icon.js

@@ -30,6 +30,16 @@ cr.define('cr.icon', function() {
     return supportedScaleFactors;
   }
 
+  /**
+   * A URL for the filetype icon for |filePath|. OS and theme dependent.
+   * @param {string} filePath
+   * @return {string}
+   */
+  function getFileIconUrl(filePath) {
+    return 'chrome://fileicon/' + encodeURIComponent(filePath) +
+        '?scale=' + window.devicePixelRatio + 'x';
+  }
+
   /**
    * Generates a CSS -webkit-image-set for a chrome:// url.
    * An entry in the image set is added for each of getSupportedScaleFactors().
@@ -100,5 +110,6 @@ cr.define('cr.icon', function() {
   return {
     getImage: getImage,
     getFavicon: getFavicon,
+    getFileIconUrl: getFileIconUrl,
   };
 });