Bug 990322 - Don't show multiple thumbnails from the same base domain. r=adw a=sylvestre
authorDão Gottwald <dao@mozilla.com>
Sat, 09 Aug 2014 00:57:46 +0200
changeset 216556 7095ec4aa45e4e2ed272b047cb4a936dc3b68112
parent 216555 56ca563b495a4ebd07858ec668ef591ff939c6f7
child 216557 f5d8b8bf02ffbf2b7890eec4bf991b7b30c0dc8b
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, sylvestre
bugs990322
milestone33.0a2
Bug 990322 - Don't show multiple thumbnails from the same base domain. r=adw a=sylvestre
browser/base/content/test/newtab/browser_newtab_background_captures.js
browser/base/content/test/newtab/browser_newtab_bug721442.js
browser/base/content/test/newtab/browser_newtab_bug725996.js
browser/base/content/test/newtab/browser_newtab_bug765628.js
browser/base/content/test/newtab/browser_newtab_bug991111.js
browser/base/content/test/newtab/browser_newtab_enhanced.js
browser/base/content/test/newtab/browser_newtab_update.js
browser/base/content/test/newtab/head.js
toolkit/components/thumbnails/test/browser_thumbnails_privacy.js
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/browser/base/content/test/newtab/browser_newtab_background_captures.js
+++ b/browser/base/content/test/newtab/browser_newtab_background_captures.js
@@ -13,28 +13,27 @@ function runTests() {
   Cu.import("resource://gre/modules/PageThumbs.jsm", imports);
   Cu.import("resource:///modules/BrowserNewTabPreloader.jsm", imports);
 
   // Disable captures.
   let originalDisabledState = Services.prefs.getBoolPref(CAPTURE_PREF);
   Services.prefs.setBoolPref(CAPTURE_PREF, true);
 
   // Make sure the thumbnail doesn't exist yet.
-  let siteName = "newtab_background_captures";
-  let url = "http://example.com/#" + siteName;
+  let url = "http://example.com/";
   let path = imports.PageThumbsStorage.getFilePathForURL(url);
   let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
   file.initWithPath(path);
   try {
     file.remove(false);
   }
   catch (err) {}
 
   // Add a top site.
-  yield setLinks(siteName);
+  yield setLinks("-1");
 
   // We need a handle to a hidden, pre-loaded newtab so we can verify that it
   // doesn't allow background captures.  Add a newtab, which triggers creation
   // of a hidden newtab, and then keep calling BrowserNewTabPreloader.newTab
   // until it returns true, meaning that it swapped the passed-in tab's docshell
   // for the hidden newtab docshell.
   let tab = gWindow.gBrowser.addTab("about:blank");
   yield addNewTabPageTab();
--- a/browser/base/content/test/newtab/browser_newtab_bug721442.js
+++ b/browser/base/content/test/newtab/browser_newtab_bug721442.js
@@ -1,23 +1,23 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 function runTests() {
   yield setLinks("0,1,2,3,4,5,6,7,8");
   setPinnedLinks([
-    {url: "http://example.com/#7", title: ""},
-    {url: "http://example.com/#8", title: "title"},
-    {url: "http://example.com/#9", title: "http://example.com/#9"}
+    {url: "http://example7.com/", title: ""},
+    {url: "http://example8.com/", title: "title"},
+    {url: "http://example9.com/", title: "http://example9.com/"}
   ]);
 
   yield addNewTabPageTab();
   checkGrid("7p,8p,9p,0,1,2,3,4,5");
 
-  checkTooltip(0, "http://example.com/#7", "1st tooltip is correct");
-  checkTooltip(1, "title\nhttp://example.com/#8", "2nd tooltip is correct");
-  checkTooltip(2, "http://example.com/#9", "3rd tooltip is correct");
+  checkTooltip(0, "http://example7.com/", "1st tooltip is correct");
+  checkTooltip(1, "title\nhttp://example8.com/", "2nd tooltip is correct");
+  checkTooltip(2, "http://example9.com/", "3rd tooltip is correct");
 }
 
 function checkTooltip(aIndex, aExpected, aMessage) {
   let link = getCell(aIndex).node.querySelector(".newtab-link");
   is(link.getAttribute("title"), aExpected, aMessage);
 }
--- a/browser/base/content/test/newtab/browser_newtab_bug725996.js
+++ b/browser/base/content/test/newtab/browser_newtab_bug725996.js
@@ -5,19 +5,19 @@ function runTests() {
   yield setLinks("0,1,2,3,4,5,6,7,8");
   setPinnedLinks("");
 
   yield addNewTabPageTab();
   checkGrid("0,1,2,3,4,5,6,7,8");
 
   let cell = getCell(0).node;
 
-  sendDragEvent("drop", cell, "http://example.com/#99\nblank");
-  is(NewTabUtils.pinnedLinks.links[0].url, "http://example.com/#99",
+  sendDragEvent("drop", cell, "http://example99.com/\nblank");
+  is(NewTabUtils.pinnedLinks.links[0].url, "http://example99.com/",
      "first cell is pinned and contains the dropped site");
 
   yield whenPagesUpdated();
   checkGrid("99p,0,1,2,3,4,5,6,7");
 
   sendDragEvent("drop", cell, "");
-  is(NewTabUtils.pinnedLinks.links[0].url, "http://example.com/#99",
+  is(NewTabUtils.pinnedLinks.links[0].url, "http://example99.com/",
      "first cell is still pinned with the site we dropped before");
 }
--- a/browser/base/content/test/newtab/browser_newtab_bug765628.js
+++ b/browser/base/content/test/newtab/browser_newtab_bug765628.js
@@ -1,13 +1,13 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 const BAD_DRAG_DATA = "javascript:alert('h4ck0rz');\nbad stuff";
-const GOOD_DRAG_DATA = "http://example.com/#99\nsite 99";
+const GOOD_DRAG_DATA = "http://example99.com/\nsite 99";
 
 function runTests() {
   yield setLinks("0,1,2,3,4,5,6,7,8");
   setPinnedLinks("");
 
   yield addNewTabPageTab();
   checkGrid("0,1,2,3,4,5,6,7,8");
 
--- a/browser/base/content/test/newtab/browser_newtab_bug991111.js
+++ b/browser/base/content/test/newtab/browser_newtab_bug991111.js
@@ -1,13 +1,13 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 function runTests() {
-  yield setLinks("0");
+  yield setLinks("-1");
   yield addNewTabPageTab();
 
   // Remember if the click handler was triggered
   let cell = getCell(0);
   let clicked = false;
   cell.site.onClick = e => {
     clicked = true;
     executeSoon(TestRunner.next);
--- a/browser/base/content/test/newtab/browser_newtab_enhanced.js
+++ b/browser/base/content/test/newtab/browser_newtab_enhanced.js
@@ -16,40 +16,48 @@ function runTests() {
   registerCleanupFunction(() => {
     Services.prefs.clearUserPref(PRELOAD_PREF);
     NewTabUtils.allPages.enhanced = origEnhanced;
   });
 
   Services.prefs.setBoolPref(PRELOAD_PREF, false);
 
   function getData(cellNum) {
-    let siteNode = getCell(cellNum).site.node;
+    let cell = getCell(cellNum);
+    if (!cell.site)
+      return null;
+    let siteNode = cell.site.node;
     return {
       type: siteNode.getAttribute("type"),
       enhanced: siteNode.querySelector(".enhanced-content").style.backgroundImage,
     };
   }
 
   // Make the page have a directory link followed by a history link
-  yield setLinks("1");
+  yield setLinks("-1");
 
   // Test with enhanced = false
   NewTabUtils.allPages.enhanced = false;
   yield addNewTabPageTab();
   let {type, enhanced} = getData(0);
   is(type, "organic", "directory link is organic");
   isnot(enhanced, "", "directory link has enhanced image");
 
-  let {type, enhanced} = getData(1);
-  is(type, "history", "history link is history");
-  is(enhanced, "", "history link has no enhanced image");
+  is(getData(1), null, "history link pushed out by directory link");
 
   // Test with enhanced = true
   NewTabUtils.allPages.enhanced = true;
   yield addNewTabPageTab();
   let {type, enhanced} = getData(0);
   is(type, "organic", "directory link is still organic");
   isnot(enhanced, "", "directory link still has enhanced image");
 
-  let {type, enhanced} = getData(1);
-  is(type, "enhanced", "history link now is enhanced");
-  isnot(enhanced, "", "history link now has enhanced image");
+  is(getData(1), null, "history link still pushed out by directory link");
+
+  // Test with a pinned link
+  setPinnedLinks("-1");
+  yield addNewTabPageTab();
+  let {type, enhanced} = getData(0);
+  is(type, "enhanced", "pinned history link is enhanced");
+  isnot(enhanced, "", "pinned history link has enhanced image");
+
+  is(getData(1), null, "directory link pushed out by pinned history link");
 }
--- a/browser/base/content/test/newtab/browser_newtab_update.js
+++ b/browser/base/content/test/newtab/browser_newtab_update.js
@@ -42,10 +42,10 @@ function runTests() {
   yield addNewTabPageTab();
   checkGrid("2,1,3,4,,,,,");
 
   // Make sure these added links have the right type
   is(getCell(1).site.link.type, "history", "added link is history");
 }
 
 function link(id) {
-  return { url: "http://example.com/#" + id, title: "site#" + id };
+  return { url: "http://example" + id + ".com/", title: "site#" + id };
 }
--- a/browser/base/content/test/newtab/head.js
+++ b/browser/base/content/test/newtab/head.js
@@ -198,27 +198,30 @@ function getGrid() {
 function getCell(aIndex) {
   return getGrid().cells[aIndex];
 }
 
 /**
  * Allows to provide a list of links that is used to construct the grid.
  * @param aLinksPattern the pattern (see below)
  *
- * Example: setLinks("1,2,3")
- * Result: [{url: "http://example.com/#1", title: "site#1"},
- *          {url: "http://example.com/#2", title: "site#2"}
- *          {url: "http://example.com/#3", title: "site#3"}]
+ * Example: setLinks("-1,0,1,2,3")
+ * Result: [{url: "http://example.com/", title: "site#-1"},
+ *          {url: "http://example0.com/", title: "site#0"},
+ *          {url: "http://example1.com/", title: "site#1"},
+ *          {url: "http://example2.com/", title: "site#2"},
+ *          {url: "http://example3.com/", title: "site#3"}]
  */
 function setLinks(aLinks) {
   let links = aLinks;
 
   if (typeof links == "string") {
     links = aLinks.split(/\s*,\s*/).map(function (id) {
-      return {url: "http://example.com/#" + id, title: "site#" + id};
+      return {url: "http://example" + (id != "-1" ? id : "") + ".com/",
+              title: "site#" + id};
     });
   }
 
   // Call populateCache() once to make sure that all link fetching that is
   // currently in progress has ended. We clear the history, fill it with the
   // given entries and call populateCache() now again to make sure the cache
   // has the desired contents.
   NewTabUtils.links.populateCache(function () {
@@ -279,26 +282,27 @@ function fillHistory(aLinks, aCallback) 
 }
 
 /**
  * Allows to specify the list of pinned links (that have a fixed position in
  * the grid.
  * @param aLinksPattern the pattern (see below)
  *
  * Example: setPinnedLinks("3,,1")
- * Result: 'http://example.com/#3' is pinned in the first cell. 'http://example.com/#1' is
+ * Result: 'http://example3.com/' is pinned in the first cell. 'http://example1.com/' is
  *         pinned in the third cell.
  */
 function setPinnedLinks(aLinks) {
   let links = aLinks;
 
   if (typeof links == "string") {
     links = aLinks.split(/\s*,\s*/).map(function (id) {
       if (id)
-        return {url: "http://example.com/#" + id, title: "site#" + id};
+        return {url: "http://example" + (id != "-1" ? id : "") + ".com/",
+                title: "site#" + id};
     });
   }
 
   let string = Cc["@mozilla.org/supports-string;1"]
                  .createInstance(Ci.nsISupportsString);
   string.data = JSON.stringify(links);
   Services.prefs.setComplexValue("browser.newtabpage.pinned",
                                  Ci.nsISupportsString, string);
@@ -350,34 +354,34 @@ function addNewTabPageTab() {
 }
 
 /**
  * Compares the current grid arrangement with the given pattern.
  * @param the pattern (see below)
  * @param the array of sites to compare with (optional)
  *
  * Example: checkGrid("3p,2,,1p")
- * Result: We expect the first cell to contain the pinned site 'http://example.com/#3'.
- *         The second cell contains 'http://example.com/#2'. The third cell is empty.
- *         The fourth cell contains the pinned site 'http://example.com/#4'.
+ * Result: We expect the first cell to contain the pinned site 'http://example3.com/'.
+ *         The second cell contains 'http://example2.com/'. The third cell is empty.
+ *         The fourth cell contains the pinned site 'http://example4.com/'.
  */
 function checkGrid(aSitesPattern, aSites) {
   let length = aSitesPattern.split(",").length;
   let sites = (aSites || getGrid().sites).slice(0, length);
   let current = sites.map(function (aSite) {
     if (!aSite)
       return "";
 
     let pinned = aSite.isPinned();
     let hasPinnedAttr = aSite.node.hasAttribute("pinned");
 
     if (pinned != hasPinnedAttr)
       ok(false, "invalid state (site.isPinned() != site[pinned])");
 
-    return aSite.url.replace(/^http:\/\/example\.com\/#(\d+)$/, "$1") + (pinned ? "p" : "");
+    return aSite.url.replace(/^http:\/\/example(\d+)\.com\/$/, "$1") + (pinned ? "p" : "");
   });
 
   is(current, aSitesPattern, "grid status = " + aSitesPattern);
 }
 
 /**
  * Blocks a site from the grid.
  * @param aIndex The cell index.
@@ -484,17 +488,17 @@ function startAndCompleteDragOperation(a
 
 /**
  * Helper function that creates a temporary iframe in the about:newtab
  * document. This will contain a link we can drag to the test the dropping
  * of links from external documents.
  */
 function createExternalDropIframe() {
   const url = "data:text/html;charset=utf-8," +
-              "<a id='link' href='http://example.com/%2399'>link</a>";
+              "<a id='link' href='http://example99.com/'>link</a>";
 
   let deferred = Promise.defer();
   let doc = getContentDocument();
   let iframe = doc.createElement("iframe");
   iframe.setAttribute("src", url);
   iframe.style.width = "50px";
   iframe.style.height = "50px";
   iframe.style.position = "absolute";
--- a/toolkit/components/thumbnails/test/browser_thumbnails_privacy.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_privacy.js
@@ -31,40 +31,39 @@ function runTests() {
     {scheme: "https", cacheControl: "no-store", diskCacheSSL: true},
 
     // Don't capture HTTPS pages by default.
     {scheme: "https", cacheControl: null, diskCacheSSL: false},
     {scheme: "https", cacheControl: "public", diskCacheSSL: false},
     {scheme: "https", cacheControl: "private", diskCacheSSL: false}
   ];
 
-  let urls = positive.map((combi) => {
-    let url = combi.scheme + URL;
-    if (combi.cacheControl)
-      url += "?" + combi.cacheControl;
-    return url;
-  });
-  yield addVisitsAndRepopulateNewTabLinks(urls, next);
-
   yield checkCombinations(positive, true);
   yield checkCombinations(negative, false);
 }
 
 function checkCombinations(aCombinations, aResult) {
   let combi = aCombinations.shift();
   if (!combi) {
     next();
     return;
   }
 
   let url = combi.scheme + URL;
   if (combi.cacheControl)
     url += "?" + combi.cacheControl;
   Services.prefs.setBoolPref(PREF_DISK_CACHE_SSL, combi.diskCacheSSL);
 
+  // Add the test page as a top link so it has a chance to be thumbnailed
+  addVisitsAndRepopulateNewTabLinks(url, _ => {
+    testCombination(combi, url, aCombinations, aResult);
+  });
+}
+
+function testCombination(combi, url, aCombinations, aResult) {
   let tab = gBrowser.selectedTab = gBrowser.addTab(url);
   let browser = gBrowser.selectedBrowser;
 
   whenLoaded(browser, function () {
     let msg = JSON.stringify(combi) + " == " + aResult;
     is(gBrowserThumbnails._shouldCapture(browser), aResult, msg);
     gBrowser.removeTab(tab);
 
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -824,18 +824,44 @@ let Links = {
   /**
    * Gets the current set of links contained in the grid.
    * @return The links in the grid.
    */
   getLinks: function Links_getLinks() {
     let pinnedLinks = Array.slice(PinnedLinks.links);
     let links = this._getMergedProviderLinks();
 
-    // Filter blocked and pinned links.
+    function getBaseDomain(url) {
+      let uri;
+      try {
+        uri = Services.io.newURI(url, null, null);
+      } catch (e) {
+        return null;
+      }
+
+      try {
+        return Services.eTLD.getBaseDomain(uri);
+      } catch (e) {
+        return uri.asciiHost;
+      }
+    }
+
+    let baseDomains = new Set();
+    for (let link of pinnedLinks) {
+      if (link)
+        baseDomains.add(getBaseDomain(link.url));
+    }
+
+    // Filter blocked and pinned links and duplicate base domains.
     links = links.filter(function (link) {
+      let baseDomain = getBaseDomain(link.url);
+      if (baseDomain == null || baseDomains.has(baseDomain))
+        return false;
+      baseDomains.add(baseDomain);
+
       return !BlockedLinks.isBlocked(link) && !PinnedLinks.isPinned(link);
     });
 
     // Try to fill the gaps between pinned links.
     for (let i = 0; i < pinnedLinks.length && links.length; i++)
       if (!pinnedLinks[i])
         pinnedLinks[i] = links.shift();
 
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -186,14 +186,14 @@ function makeLinks(frecRangeStart, frecR
   for (let i = frecRangeEnd; i > frecRangeStart; i -= step) {
     links.push(makeLink(i));
   }
   return links;
 }
 
 function makeLink(frecency) {
   return {
-    url: "http://example.com/" + frecency,
+    url: "http://example" + frecency + ".com/",
     title: "My frecency is " + frecency,
     frecency: frecency,
     lastVisitDate: 0,
   };
 }