Bug 1139052 - Don't show suggested tiles for users with fewer than 8 history tiles. r=adw
authorMarina Samuel <msamuel@mozilla.com>
Tue, 14 Apr 2015 14:39:50 -0400
changeset 270495 9b3b216d2e98792ac2bfc464fb69efdf5c1d3484
parent 270494 3c1b43d1b703746a212b9050ddbad6dd0f5e7579
child 270496 16958c3e86dab77a301918df0d77400674f794d1
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1139052
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1139052 - Don't show suggested tiles for users with fewer than 8 history tiles. r=adw
browser/base/content/test/newtab/browser_newtab_enhanced.js
browser/modules/DirectoryLinksProvider.jsm
browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
--- a/browser/base/content/test/newtab/browser_newtab_enhanced.js
+++ b/browser/base/content/test/newtab/browser_newtab_enhanced.js
@@ -104,27 +104,28 @@ function runTests() {
 
   yield unpinCell(0);
 
 
 
   // Test that a suggested tile is not enhanced by a directory tile
   let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
   NewTabUtils.isTopPlacesSite = () => true;
-  yield setLinks("-1");
+  yield setLinks("-1,2,3,4,5,6,7,8");
 
   // Test with enhanced = false
   yield addNewTabPageTab();
   yield customizeNewTabPage("classic");
   ({type, enhanced, title} = getData(0));
   isnot(type, "enhanced", "history link is not enhanced");
   is(enhanced, "", "history link has no enhanced image");
   is(title, "site#-1");
 
-  is(getData(1), null, "there is only one link and it's a history link");
+  isnot(getData(7), null, "there are 8 history links");
+  is(getData(8), null, "there are 8 history links");
 
 
   // Test with enhanced = true
   yield addNewTabPageTab();
   yield customizeNewTabPage("enhanced");
 
   // Suggested link was not enhanced by directory link with same domain
   ({type, enhanced, title} = getData(0));
@@ -133,10 +134,10 @@ function runTests() {
   is(title, "title2");
 
   // Enhanced history link shows up second
   ({type, enhanced, title} = getData(1));
   is(type, "enhanced", "pinned history link is enhanced");
   isnot(enhanced, "", "pinned history link has enhanced image");
   is(title, "title");
 
-  is(getData(2), null, "there is only a suggested link followed by an enhanced history link");
+  is(getData(9), null, "there is a suggested link followed by an enhanced history link and the remaining history links");
 }
--- a/browser/modules/DirectoryLinksProvider.jsm
+++ b/browser/modules/DirectoryLinksProvider.jsm
@@ -60,16 +60,19 @@ const ALLOWED_IMAGE_SCHEMES = new Set(["
 const DIRECTORY_FRECENCY = 1000;
 
 // The frecency of a suggested link
 const SUGGESTED_FRECENCY = Infinity;
 
 // Default number of times to show a link
 const DEFAULT_FREQUENCY_CAP = 5;
 
+// The min number of visible (not blocked) history tiles to have before showing suggested tiles
+const MIN_VISIBLE_HISTORY_TILES = 8;
+
 // Divide frecency by this amount for pings
 const PING_SCORE_DIVISOR = 10000;
 
 // Allowed ping actions remotely stored as columns: case-insensitive [a-z0-9_]
 const PING_ACTIONS = ["block", "click", "pin", "sponsored", "sponsored_link", "unpin", "view"];
 
 /**
  * Singleton that serves as the provider of directory links.
@@ -559,17 +562,17 @@ let DirectoryLinksProvider = {
     NewTabUtils.links.populateProviderCache(NewTabUtils.placesProvider, () => {
       this._handleManyLinksChanged();
     });
   },
 
   onLinkChanged: function (aProvider, aLink) {
     // Make sure NewTabUtils.links handles the notification first.
     setTimeout(() => {
-      if (this._handleLinkChanged(aLink)) {
+      if (this._handleLinkChanged(aLink) || this._shouldUpdateSuggestedTile()) {
         this._updateSuggestedTile();
       }
     }, 0);
   },
 
   onManyLinksChanged: function () {
     // Make sure NewTabUtils.links handles the notification first.
     setTimeout(() => {
@@ -587,16 +590,48 @@ let DirectoryLinksProvider = {
     this._frequencyCaps.set(url, remainingViews);
 
     // Reached the number of views, so pick a new one.
     if (remainingViews <= 0) {
       this._updateSuggestedTile();
     }
   },
 
+  _getCurrentTopSiteCount: function() {
+    let visibleTopSiteCount = 0;
+    for (let link of NewTabUtils.links.getLinks().slice(0, MIN_VISIBLE_HISTORY_TILES)) {
+      if (link && (link.type == "history" || link.type == "enhanced")) {
+        visibleTopSiteCount++;
+      }
+    }
+    return visibleTopSiteCount;
+  },
+
+  _shouldUpdateSuggestedTile: function() {
+    let sortedLinks = NewTabUtils.getProviderLinks(this);
+
+    let mostFrecentLink = {};
+    if (sortedLinks && sortedLinks.length) {
+      mostFrecentLink = sortedLinks[0]
+    }
+
+    let currTopSiteCount = this._getCurrentTopSiteCount();
+    if ((!mostFrecentLink.targetedSite && currTopSiteCount >= MIN_VISIBLE_HISTORY_TILES) ||
+        (mostFrecentLink.targetedSite && currTopSiteCount < MIN_VISIBLE_HISTORY_TILES)) {
+      // If mostFrecentLink has a targetedSite then mostFrecentLink is a suggested link.
+      // If we have enough history links (8+) to show a suggested tile and we are not
+      // already showing one, then we should update (to *attempt* to add a suggested tile).
+      // OR if we don't have enough history to show a suggested tile (<8) and we are
+      // currently showing one, we should update (to remove it).
+      return true;
+    }
+
+    return false;
+  },
+
   /**
    * Chooses and returns a suggested tile based on a user's top sites
    * that we have an available suggested tile for.
    *
    * @return the chosen suggested tile, or undefined if there isn't one
    */
   _updateSuggestedTile: function() {
     let sortedLinks = NewTabUtils.getProviderLinks(this);
@@ -616,18 +651,19 @@ let DirectoryLinksProvider = {
           url: mostFrecentLink.url,
           frecency: SUGGESTED_FRECENCY,
           lastVisitDate: mostFrecentLink.lastVisitDate,
           type: mostFrecentLink.type,
         }, 0, true);
       }
     }
 
-    if (this._topSitesWithSuggestedLinks.size == 0) {
-      // There are no potential suggested links we can show.
+    if (this._topSitesWithSuggestedLinks.size == 0 || !this._shouldUpdateSuggestedTile()) {
+      // There are no potential suggested links we can show or not
+      // enough history for a suggested tile.
       return;
     }
 
     // Create a flat list of all possible links we can show as suggested.
     // Note that many top sites may map to the same suggested links, but we only
     // want to count each suggested link once (based on url), thus possibleLinks is a map
     // from url to suggestedLink. Thus, each link has an equal chance of being chosen at
     // random from flattenedLinks if it appears only once.
--- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
+++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
@@ -218,16 +218,52 @@ function run_test() {
     DirectoryLinksProvider.reset();
     Services.prefs.clearUserPref(kLocalePref);
     Services.prefs.clearUserPref(kSourceUrlPref);
     Services.prefs.clearUserPref(kPingUrlPref);
     Services.prefs.clearUserPref(kNewtabEnhancedPref);
   });
 }
 
+add_task(function test_shouldUpdateSuggestedTile() {
+  let suggestedLink = {
+    targetedSite: "somesite.com"
+  };
+
+  // DirectoryLinksProvider has no suggested tile and no top sites => no need to update
+  do_check_eq(DirectoryLinksProvider._getCurrentTopSiteCount(), 0);
+  isIdentical(NewTabUtils.getProviderLinks(), []);
+  do_check_eq(DirectoryLinksProvider._shouldUpdateSuggestedTile(), false);
+
+  // DirectoryLinksProvider has a suggested tile and no top sites => need to update
+  let origGetProviderLinks = NewTabUtils.getProviderLinks;
+  NewTabUtils.getProviderLinks = (provider) => [suggestedLink];
+
+  do_check_eq(DirectoryLinksProvider._getCurrentTopSiteCount(), 0);
+  isIdentical(NewTabUtils.getProviderLinks(), [suggestedLink]);
+  do_check_eq(DirectoryLinksProvider._shouldUpdateSuggestedTile(), true);
+
+  // DirectoryLinksProvider has a suggested tile and 8 top sites => no need to update
+  let origCurrentTopSiteCount = DirectoryLinksProvider._getCurrentTopSiteCount;
+  DirectoryLinksProvider._getCurrentTopSiteCount = () => 8;
+
+  do_check_eq(DirectoryLinksProvider._getCurrentTopSiteCount(), 8);
+  isIdentical(NewTabUtils.getProviderLinks(), [suggestedLink]);
+  do_check_eq(DirectoryLinksProvider._shouldUpdateSuggestedTile(), false);
+
+  // DirectoryLinksProvider has no suggested tile and 8 top sites => need to update
+  NewTabUtils.getProviderLinks = origGetProviderLinks;
+  do_check_eq(DirectoryLinksProvider._getCurrentTopSiteCount(), 8);
+  isIdentical(NewTabUtils.getProviderLinks(), []);
+  do_check_eq(DirectoryLinksProvider._shouldUpdateSuggestedTile(), true);
+
+  // Cleanup
+  DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
+});
+
 add_task(function test_updateSuggestedTile() {
   let topSites = ["site0.com", "1040.com", "site2.com", "hrblock.com", "site4.com", "freetaxusa.com", "site6.com"];
 
   // Initial setup
   let data = {"suggested": [suggestedTile1, suggestedTile2, suggestedTile3], "directory": [someOtherSite]};
   let dataURI = 'data:application/json,' + JSON.stringify(data);
 
   let testObserver = new TestFirstRun();
@@ -241,16 +277,19 @@ add_task(function test_updateSuggestedTi
     return topSites.indexOf(site) >= 0;
   }
 
   let origGetProviderLinks = NewTabUtils.getProviderLinks;
   NewTabUtils.getProviderLinks = function(provider) {
     return links;
   }
 
+  let origCurrentTopSiteCount = DirectoryLinksProvider._getCurrentTopSiteCount;
+  DirectoryLinksProvider._getCurrentTopSiteCount = () => 8;
+
   do_check_eq(DirectoryLinksProvider._updateSuggestedTile(), undefined);
 
   function TestFirstRun() {
     this.promise = new Promise(resolve => {
       this.onLinkChanged = (directoryLinksProvider, link) => {
         links.unshift(link);
         let possibleLinks = [suggestedTile1.url, suggestedTile2.url, suggestedTile3.url];
 
@@ -341,16 +380,17 @@ add_task(function test_updateSuggestedTi
   DirectoryLinksProvider.addObserver(testObserver);
   DirectoryLinksProvider.onManyLinksChanged();
   yield testObserver.promise;
 
   // Cleanup
   yield promiseCleanDirectoryLinksProvider();
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
   NewTabUtils.getProviderLinks = origGetProviderLinks;
+  DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
 });
 
 add_task(function test_suggestedLinksMap() {
   let data = {"suggested": [suggestedTile1, suggestedTile2, suggestedTile3, suggestedTile4], "directory": [someOtherSite]};
   let dataURI = 'data:application/json,' + JSON.stringify(data);
 
   yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});
   let links = yield fetchData();
@@ -431,16 +471,19 @@ add_task(function test_topSitesWithSugge
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
   NewTabUtils.getProviderLinks = origGetProviderLinks;
 });
 
 add_task(function test_suggestedAttributes() {
   let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
   NewTabUtils.isTopPlacesSite = () => true;
 
+  let origCurrentTopSiteCount = DirectoryLinksProvider._getCurrentTopSiteCount;
+  DirectoryLinksProvider._getCurrentTopSiteCount = () => 8;
+
   let frecent_sites = ["top.site.com"];
   let imageURI = "https://image/";
   let title = "the title";
   let type = "affiliate";
   let url = "http://test.url/";
   let data = {
     suggested: [{
       frecent_sites,
@@ -473,25 +516,29 @@ add_task(function test_suggestedAttribut
   do_check_eq(link.imageURI, imageURI);
   do_check_eq(link.targetedSite, frecent_sites[0]);
   do_check_eq(link.title, title);
   do_check_eq(link.type, type);
   do_check_eq(link.url, url);
 
   // Cleanup.
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
+  DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
   gLinks.removeProvider(DirectoryLinksProvider);
   DirectoryLinksProvider.removeObserver(gLinks);
 });
 
 add_task(function test_frequencyCappedSites_views() {
   Services.prefs.setCharPref(kPingUrlPref, "");
   let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
   NewTabUtils.isTopPlacesSite = () => true;
 
+  let origCurrentTopSiteCount = DirectoryLinksProvider._getCurrentTopSiteCount;
+  DirectoryLinksProvider._getCurrentTopSiteCount = () => 8;
+
   let testUrl = "http://frequency.capped/link";
   let targets = ["top.site.com"];
   let data = {
     suggested: [{
       type: "affiliate",
       frecent_sites: targets,
       url: testUrl
     }],
@@ -543,26 +590,30 @@ add_task(function test_frequencyCappedSi
   checkFirstTypeAndLength("affiliate", 2);
   synthesizeAction("view");
   checkFirstTypeAndLength("affiliate", 2);
   synthesizeAction("view");
   checkFirstTypeAndLength("organic", 1);
 
   // Cleanup.
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
+  DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
   gLinks.removeProvider(DirectoryLinksProvider);
   DirectoryLinksProvider.removeObserver(gLinks);
   Services.prefs.setCharPref(kPingUrlPref, kPingUrl);
 });
 
 add_task(function test_frequencyCappedSites_click() {
   Services.prefs.setCharPref(kPingUrlPref, "");
   let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
   NewTabUtils.isTopPlacesSite = () => true;
 
+  let origCurrentTopSiteCount = DirectoryLinksProvider._getCurrentTopSiteCount;
+  DirectoryLinksProvider._getCurrentTopSiteCount = () => 8;
+
   let testUrl = "http://frequency.capped/link";
   let targets = ["top.site.com"];
   let data = {
     suggested: [{
       type: "affiliate",
       frecent_sites: targets,
       url: testUrl
     }],
@@ -608,16 +659,17 @@ add_task(function test_frequencyCappedSi
   checkFirstTypeAndLength("affiliate", 2);
   synthesizeAction("view");
   checkFirstTypeAndLength("affiliate", 2);
   synthesizeAction("click");
   checkFirstTypeAndLength("organic", 1);
 
   // Cleanup.
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
+  DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
   gLinks.removeProvider(DirectoryLinksProvider);
   DirectoryLinksProvider.removeObserver(gLinks);
   Services.prefs.setCharPref(kPingUrlPref, kPingUrl);
 });
 
 add_task(function test_reportSitesAction() {
   yield DirectoryLinksProvider.init();
   let deferred, expectedPath, expectedPost;
@@ -1084,16 +1136,18 @@ add_task(function test_DirectoryLinksPro
   do_check_eq(links.length, 0); // There are no directory links.
   checkEnhanced("http://example.net", undefined);
   checkEnhanced("http://example.com", "data:,fresh");
 });
 
 add_task(function test_DirectoryLinksProvider_enhancedURIs() {
   let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
   NewTabUtils.isTopPlacesSite = () => true;
+  let origCurrentTopSiteCount = DirectoryLinksProvider._getCurrentTopSiteCount;
+  DirectoryLinksProvider._getCurrentTopSiteCount = () => 8;
 
   let data = {
     "suggested": [
       {url: "http://example.net", enhancedImageURI: "data:,net1", title:"SuggestedTitle", frecent_sites: ["test.com"]}
     ],
     "directory": [
       {url: "http://example.net", enhancedImageURI: "data:,net2", title:"DirectoryTitle"}
     ]
@@ -1124,16 +1178,17 @@ add_task(function test_DirectoryLinksPro
   // Check that the suggested tile with the same URL replaces the directory tile.
   links = gLinks.getLinks();
   do_check_eq(links.length, 1);
   do_check_eq(links[0].title, "SuggestedTitle");
   do_check_eq(links[0].enhancedImageURI, "data:,net1");
 
   // Cleanup.
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
+  DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
   gLinks.removeProvider(DirectoryLinksProvider);
 });
 
 add_task(function test_DirectoryLinksProvider_setDefaultEnhanced() {
   function checkDefault(expected) {
     Services.prefs.clearUserPref(kNewtabEnhancedPref);
     do_check_eq(Services.prefs.getBoolPref(kNewtabEnhancedPref), expected);
   }