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

Index: chrome/browser/resources/local_ntp/most_visited_single.js
diff --git a/chrome/browser/resources/local_ntp/most_visited_single.js b/chrome/browser/resources/local_ntp/most_visited_single.js
index 8590dca1fa174462c9bcac6e96c8ec4492db88e3..916261a7526d06634dcfe8820b52bcb9ff827d5a 100644
--- a/chrome/browser/resources/local_ntp/most_visited_single.js
+++ b/chrome/browser/resources/local_ntp/most_visited_single.js
@@ -278,6 +278,7 @@ var addTile = function(args) {
   if (args.rid) {

[huangs, 2015/05/11 21:27:11]

Should there be some code to allow server NTP to populate more items into
data.thumbnailUrls?

[fserb, 2015/05/19 19:02:15]

Acknowledged.

     var data = chrome.embeddedSearch.searchBox.getMostVisitedItemData(args.rid);
     data.tid = data.rid;
+    data.thumbnailUrls = [data.thumbnailUrl];
     data.faviconUrl = 'chrome-search://favicon/size/16@' +
         window.devicePixelRatio + 'x/' + data.renderViewId + '/' + data.tid;
     tiles.appendChild(renderTile(data));
@@ -356,26 +357,96 @@ var renderTile = function(data) {
     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);

[huangs, 2015/05/11 21:07:30]

Remove this, and just call countLoad() from within the handler?

[fserb, 2015/05/19 19:02:15]

Acknowledged.

+      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');
+    var img = document.createElement('img');
+    var loaded = false;
+
+    // We keep track of the outcome of loading possible thumbnails for this
+    // tile. Possible values:
+    //   - null: waiting for load/error
+    //   - false: error
+    //   - a string: URL that loaded correctly.
+    // This is populated by acceptImage/rejectImage and loadBestImage
+    // decides the best one to load.
+    var results = [];
+
+    var loadBestImage = function() {

[huangs, 2015/05/11 21:07:30]

It seems the first image loaded will just dominate, so if there are multiple
successes then we'd have a race condition?

[huangs, 2015/05/11 21:27:11]

Ah NVM; the "null" ensures first best image gets loaded.

[fserb, 2015/05/19 19:02:15]

Acknowledged.

[fserb, 2015/05/19 19:02:15]

Acknowledged.

+      if (loaded) {
+        return;
+      }
+      for (i = 0; i < results.length; ++i) {
+        if (results[i] === null) {
+          return;
+        }
+        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) {

[huangs, 2015/05/11 21:27:11]

I'd suggest removing acceptImage() and rejectImage(), but have

var loadBestImage = function(idx, value) {
  results[idx] = value;
  ...
};

then later, have

image.onload = loadBestImage.bind(loadBestImage, i, data.thumbnailUrls[i]);
image.onerror = loadBestImage.bind(loadBestImage, i, null);

and for the "else" case have
loadBestImage(i, null);

[fserb, 2015/05/19 19:02:15]

Done.

+      return function(ev) {
+        results[idx] = url;
+        loadBestImage();
+      };
+    };
+
+    var rejectImage = function(idx) {
+      return function(ev) {
+        results[idx] = false;
+        loadBestImage();
+      };
+    };
+
+    // Get all thumbnailUrls for the tile.
+    // They are ordered from best one to be used to worst.
+    for (var i = 0; i < data.thumbnailUrls.length; ++i) {
+      results.push(null);
+    }
+    for (var i = 0; i < data.thumbnailUrls.length; ++i) {
+      if (data.thumbnailUrls[i]) {
+        var image = new Image();
+        image.src = data.thumbnailUrls[i];
+        image.onload = acceptImage(i, data.thumbnailUrls[i]);
+        image.onerror = rejectImage(i);
+      } else {
+        rejectImage(i)(null);
+      }
+    }
+
+    img.title = data.title;
+    img.classList.add('thumbnail');
+    loadedCounter += 1;
+    img.addEventListener('load', countLoad);
     img.addEventListener('error', countLoad);

[huangs, 2015/05/11 21:07:30]

Remove this, and just call countLoad() from within the handler?

[fserb, 2015/05/19 19:02:15]

Acknowledged.

     img.addEventListener('error', function(ev) {
       thumb.classList.add('failed-img');
@@ -384,11 +455,7 @@ var renderTile = function(data) {
     });
     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');