Most visited iframe now postMessage to signal the iframing page that the link has been displayed

chrome/browser/resources/local_ntp/most_visited_thumbnail.js

Index: chrome/browser/resources/local_ntp/most_visited_thumbnail.js
diff --git a/chrome/browser/resources/local_ntp/most_visited_thumbnail.js b/chrome/browser/resources/local_ntp/most_visited_thumbnail.js
index 9201259e31e8c30aa82b6f54fe21481bd88085a8..e1e14d9ad8557a042c65978dae7f2359a0b7a2d8 100644
--- a/chrome/browser/resources/local_ntp/most_visited_thumbnail.js
+++ b/chrome/browser/resources/local_ntp/most_visited_thumbnail.js
@@ -17,86 +17,65 @@ window.addEventListener('DOMContentLoaded', function() {
     function logImpression(tileIndex, provider) {
       chrome.embeddedSearch.newTabPage.logImpression(tileIndex, provider);
     }
+    function displayLink(link) {
+      document.body.appendChild(link);
+      window.parent.postMessage('linkDisplayed', '*');

[Jered, 2014/01/10 15:12:36]

If this is always going to happen, can you just use the iframe onload event
instead?

[Jered, 2014/01/10 15:12:36]

'*' makes me nervous. While I think this message is harmless, it would be better
to restrict to just the new tab page origin.

[beaudoin, 2014/01/15 23:39:56]

This event is fired after the onload, specifically when the dynamically created
<img> tiles have loaded. The timing is important, otherwise we could display the
fallback image (which sits behind the regular tile) before the tile has
completed loading, which results in flashing.

[beaudoin, 2014/01/15 23:39:56]

The problem here is that I cannot know the URL of the iframing page.
document.referrer wont work because they are on different domains. Looks like
chrome://newtab doesn't work, I probably need the real URL from
window.location.orign, which I can't bake in the JS because it's not a constant
(it can use different TLD, it's different when debugging, etc.)

Think I can go with '*' here?

[Jered, 2014/01/16 18:07:55]

That makes sense. We should probably also be using this message to wait to show
most visited tiles so that we don't flash empty tiles before showing the images.

[Jered, 2014/01/16 18:07:55]

I solved this problem once before for suggestion iframes but put up a mental
block about that code. If you send this JS via SendJSWithOrigin(), and include a
string like "{{ORIGIN}}", it should be the origin you want. :-)

[beaudoin, 2014/01/17 03:51:46]

Good point, I'll add that as a TODO on my side. :)

[beaudoin, 2014/01/17 03:51:46]

This was significantly harder than expected, you'll see my various changes: one
to account for the fact that we now have the render_frame_id not the
render_view_id, one to make sure we get the real URL not the "chrome://newtab"
one. Nobody else seemed to be using SendJSWithOrigin() save from the test so I
believe the changes are fine.

Please take a thorough look at these changes, in /c/b/search/*

+    }
     function showDomainElement() {
-      logEvent(NTP_LOGGING_EVENT_TYPE.NTP_THUMBNAIL_ERROR);
       var link = createMostVisitedLink(
           params, data.url, data.title, undefined, data.ping, data.provider);
       var domain = document.createElement('div');
       domain.textContent = data.domain;
       link.appendChild(domain);
-      document.body.appendChild(link);
+      displayLink(link);
     }
     // Called on intentionally empty tiles for which the visuals are handled
     // externally by the page itself.
     function showEmptyTile() {
-      var link = createMostVisitedLink(
-          params, data.url, data.title, undefined, data.ping, data.provider);
-      document.body.appendChild(link);
-      logEvent(NTP_LOGGING_EVENT_TYPE.NTP_EXTERNAL_TILE);
+      displayLink(createMostVisitedLink(
+          params, data.url, data.title, undefined, data.ping, data.provider));
     }
-    function createAndAppendThumbnail(isVisible) {
+

[Mathieu, 2014/01/09 18:37:18]

nit: remove space here to be consistent

[beaudoin, 2014/01/15 23:39:56]

Done.

+    // Creates and add an image.

[Mathieu, 2014/01/09 18:37:18]

nit: adds

[beaudoin, 2014/01/15 23:39:56]

Done.

+    function createThumbnail(src) {
       var image = new Image();
       image.onload = function() {
         var shadow = document.createElement('span');
         shadow.classList.add('shadow');
         var link = createMostVisitedLink(
-            params, data.url, data.title, undefined, data.ping, data.provider);
+            params, data.url, data.title, undefined, data.ping,
+            data.provider);
         link.appendChild(shadow);
         link.appendChild(image);
-        // We add 'position: absolute' in anticipation that there could be more
-        // than one thumbnail. This will superpose the elements.
-        link.style.position = 'absolute';
-        document.body.appendChild(link);
+        displayLink(link);
+      };
+      image.onerror = function() {
+        logEvent(NTP_LOGGING_EVENT_TYPE.NTP_THUMBNAIL_ERROR);
+        if (data.domain) {
+          logEvent(NTP_LOGGING_EVENT_TYPE.NTP_GRAY_TILE_FALLBACK);
+          showDomainElement();
+        } else {
+          logEvent(NTP_LOGGING_EVENT_TYPE.NTP_EXTERNAL_FALLBACK);
+          showEmptyTile();
+        }
       };
-      if (!isVisible) {
-        image.style.visibility = 'hidden';
-      }
-      return image;
+      image.src = src;
     }
+
     // Log an impression if we know the position of the tile.
     if (isFinite(params.pos) && data.provider) {
       logImpression(parseInt(params.pos, 10), data.provider);
     }
 
+    logEvent(NTP_LOGGING_EVENT_TYPE.NTP_TILE);
     if (data.thumbnailUrl) {
-      var image = createAndAppendThumbnail(true);
-      // If a backup thumbnail URL was provided, preload it in case the first
-      // thumbnail errors. The backup thumbnail is always preloaded so that the
-      // server can't gain knowledge on the local thumbnail DB by specifying a
-      // second URL that is only sometimes fetched.
-      if (data.thumbnailUrl2) {
-        var image2 = createAndAppendThumbnail(false);
-        var imageFailed = false;
-        var image2Failed = false;
-        image2.onerror = function() {
-          image2Failed = true;
-          image2.style.visibility = 'hidden';
-          if (imageFailed) {
-            showDomainElement();
-          }
-        };
-        image2.src = data.thumbnailUrl2;
-        // The first thumbnail's onerror function will swap the visibility of
-        // the two thumbnails.
-        image.onerror = function() {
-          logEvent(NTP_LOGGING_EVENT_TYPE.NTP_FALLBACK_THUMBNAIL_USED);
-          imageFailed = true;
-          image.style.visibility = 'hidden';
-          if (image2Failed) {
-            showDomainElement();
-          } else {
-            image2.style.visibility = 'visible';
-          }
-        };
-        logEvent(NTP_LOGGING_EVENT_TYPE.NTP_FALLBACK_THUMBNAIL_REQUESTED);
-      } else {
-        image.onerror = showDomainElement;
-      }
-      image.src = data.thumbnailUrl;
       logEvent(NTP_LOGGING_EVENT_TYPE.NTP_THUMBNAIL_ATTEMPT);
+      createThumbnail(data.thumbnailUrl);
     } else if (data.domain) {
+      logEvent(NTP_LOGGING_EVENT_TYPE.NTP_GRAY_TILE);
       showDomainElement();
     } else {
+      logEvent(NTP_LOGGING_EVENT_TYPE.NTP_EXTERNAL_TILE);
       showEmptyTile();
     }
   });