Adds support for multiple thumbnail files on Single iframe NTP, fallback to the best one that opens.

chrome/browser/resources/local_ntp/most_visited_single.js

 /* Copyright 2015 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. */
 
  // Single iframe for NTP tiles.
 (function() {
 'use strict';
 
 
 /**
@@ skipping 260 matching lines @@
 /**
  * Called when the host page wants to add a suggestion tile.
  * For Most Visited, it grabs the data from Chrome and pass on.
  * For host page generated it just passes the data.
  * @param {object} args Data for the tile to be rendered.
  */
 var addTile = function(args) {
   if (args.rid) {
     var data = chrome.embeddedSearch.searchBox.getMostVisitedItemData(args.rid);
     data.tid = data.rid;
+    data.thumbnailUrl = [data.thumbnailUrl];

[huangs, 2015/05/11 20:31:16]

This is hacky, please have:
  data.thumbnailUrlList = [data.thumbnailUrl];
and maybe
  data.thumbnailUrl = null
to prevent usage.

[fserb, 2015/05/11 20:39:56]

Done.

     data.faviconUrl = 'chrome-search://favicon/size/16@' +
         window.devicePixelRatio + 'x/' + data.renderViewId + '/' + data.tid;
     tiles.appendChild(renderTile(data));
     logEvent(LOG_TYPE.NTP_CLIENT_SIDE_SUGGESTION);
   } else if (args.id) {
     tiles.appendChild(renderTile(args));
     logEvent(LOG_TYPE.NTP_SERVER_SIDE_SUGGESTION);
   } else {
     tiles.appendChild(renderTile(null));
   }
@@ skipping 58 matching lines @@
     logEvent(LOG_TYPE.NTP_MOUSEOVER);
   });
 
   var title = tile.querySelector('.mv-title');
   title.innerText = data.title;
   title.style.direction = data.direction || 'ltr';
   if (NUM_TITLE_LINES > 1) {
     title.classList.add('multiline');
   }
 
-  var hasIcon = USE_ICONS && data.largeIconUrl;
-  var hasThumb = !USE_ICONS && data.thumbnailUrl;
-  var thumb = tile.querySelector('.mv-thumb');
-  if (hasIcon || hasThumb) {
-    var img = document.createElement('img');
-    img.title = data.title;
-    if (hasIcon) {
+  if (USE_ICONS) {
+    var thumb = tile.querySelector('.mv-thumb');
+    if (data.largeIconUrl) {
+      var img = document.createElement('img');
+      img.title = data.title;
       img.src = data.largeIconUrl;
       img.classList.add('large-icon');
-    } else {  // hasThumb
-      img.src = data.thumbnailUrl;
-      img.classList.add('thumbnail');
-    }
-    loadedCounter += 1;
-    img.addEventListener('load', countLoad);
-    if (data.largeIconUrl) {
+      loadedCounter += 1;
+      img.addEventListener('load', countLoad);
       img.addEventListener('load', function(ev) {
         thumb.classList.add('large-icon-outer');
       });
+      img.addEventListener('error', countLoad);
+      img.addEventListener('error', function(ev) {
+        thumb.classList.add('failed-img');
+        thumb.removeChild(img);
+        logEvent(LOG_TYPE.NTP_THUMBNAIL_ERROR);
+      });
+      thumb.appendChild(img);
+      logEvent(LOG_TYPE.NTP_THUMBNAIL_TILE);
+    } else {
+      thumb.classList.add('failed-img');
     }
+  } else { // THUMBNAILS
+    var thumb = tile.querySelector('.mv-thumb');
+

[Mathieu, 2015/05/11 20:27:24]

nit: extra newline

[fserb, 2015/05/11 20:39:56]

Done.

+    var img = document.createElement('img');
+    var loaded = false;
+    var results = [];

[Mathieu, 2015/05/11 20:27:24]

Add a comment like: // Used to keep track of loading outcomes for possible
thumbnails for this tile. Decision is taken in loadBestImage.

[fserb, 2015/05/11 20:39:56]

Done.

+
+    var loadBestImage = function() {
+      if (loaded) return;

[huangs, 2015/05/11 20:31:16]

Move return to next line.

[fserb, 2015/05/11 20:39:56]

Done.

+      for (i = 0; i < results.length; ++i) {
+        if (results[i] === null) return;

[huangs, 2015/05/11 20:31:16]

Move return to next line.

[fserb, 2015/05/11 20:39:56]

Done.

+        if (results[i] != false) {
+          img.src = results[i];
+          loaded = true;
+          return;
+        }
+      }
+      thumb.classList.add('failed-img');
+      thumb.removeChild(img);
+      logEvent(LOG_TYPE.NTP_THUMBNAIL_ERROR);
+    };
+
+    var acceptImage = function(idx, url) {
+      return function(ev) {
+        results[idx] = url;
+        loadBestImage();
+      };
+    };
+
+    var rejectImage = function(i) {

[huangs, 2015/05/11 20:31:16]

This |i| is a number.

[fserb, 2015/05/11 20:39:56]

Done.

+      return function(ev) {
+        results[i] = false;
+        loadBestImage();
+      };
+    };
+
+    var images = [];

[Mathieu, 2015/05/11 20:27:24]

remove

[fserb, 2015/05/11 20:39:56]

Done.

+
+    for (var t = 0; t < data.thumbnailUrl.length; ++t) {

[Mathieu, 2015/05/11 20:27:24]

could you add a comment above saying that the best thumbnail should be at the
beginning of the list?

[fserb, 2015/05/11 20:39:56]

Done.

+      results.push(null);
+      var i = new Image();

[huangs, 2015/05/11 20:31:16]

This |i| is an Image, and |t| is now the index.  Please make naming consistent.

[fserb, 2015/05/11 20:39:56]

Done.

+      i.src = data.thumbnailUrl[t];

[Mathieu, 2015/05/11 20:28:58]

does it completely poop out if data.thumbnailUrl is null/undefined?

[fserb, 2015/05/11 20:39:56]

it does get an onerror for undefined. But I added extra checks.

+      i.onload = acceptImage(t, data.thumbnailUrl[t]);
+      i.onerror = rejectImage(t);
+    }
+
+    img.title = data.title;
+    img.classList.add('thumbnail');
+    loadedCounter += 1;
+    img.addEventListener('load', countLoad);
     img.addEventListener('error', countLoad);
     img.addEventListener('error', function(ev) {
       thumb.classList.add('failed-img');
       thumb.removeChild(img);
       logEvent(LOG_TYPE.NTP_THUMBNAIL_ERROR);
     });
     thumb.appendChild(img);
     logEvent(LOG_TYPE.NTP_THUMBNAIL_TILE);
-  } else {
-    thumb.classList.add('failed-img');
-  }
 
-  if (!USE_ICONS) {
     var favicon = tile.querySelector('.mv-favicon');
     if (data.faviconUrl) {
       var fi = document.createElement('img');
       fi.src = data.faviconUrl;
       // Set the title to empty so screen readers won't say the image name.
       fi.title = '';
       loadedCounter += 1;
       fi.addEventListener('load', countLoad);
       fi.addEventListener('error', countLoad);
       fi.addEventListener('error', function(ev) {
@@ skipping 50 matching lines @@
     var html = document.querySelector('html');
     html.dir = 'rtl';
   }
 
   window.addEventListener('message', handlePostMessage);
 };
 
 
 window.addEventListener('DOMContentLoaded', init);
 })();