Bug 1140496 - Only show a suggested tile url for some number of times or until clicked [r=adw]

Default to a hardcoded frequency cap that decreases by 1 per view or all for a click.

browser/modules/DirectoryLinksProvider.jsm

@@ -60,6 +60,9 @@ 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;
+
 // Divide frecency by this amount for pings
 const PING_SCORE_DIVISOR = 10000;
 
@@ -88,6 +91,11 @@ let DirectoryLinksProvider = {
    */
   _enhancedLinks: new Map(),
 
+  /**
+   * A mapping from site to remaining number of views
+   */
+  _frequencyCaps: new Map(),
+
   /**
    * A mapping from site to a list of suggested link objects
    */
@@ -325,6 +333,23 @@ let DirectoryLinksProvider = {
    * @return download promise
    */
   reportSitesAction: function DirectoryLinksProvider_reportSitesAction(sites, action, triggeringSiteIndex) {
+    // Check if the suggested tile was shown
+    if (action == "view") {
+      sites.slice(0, triggeringSiteIndex + 1).forEach(site => {
+        let {targetedSite, url} = site.link;
+        if (targetedSite) {
+          this._decreaseFrequencyCap(url, 1);
+        }
+      });
+    }
+    // Use up all views if the user clicked on a frequency capped tile
+    else if (action == "click") {
+      let {targetedSite, url} = sites[triggeringSiteIndex].link;
+      if (targetedSite) {
+        this._decreaseFrequencyCap(url, DEFAULT_FREQUENCY_CAP);
+      }
+    }
+
     let newtabEnhanced = false;
     let pingEndPoint = "";
     try {
@@ -415,6 +440,7 @@ let DirectoryLinksProvider = {
     this._readDirectoryLinksFile().then(rawLinks => {
       // Reset the cache of suggested tiles and enhanced images for this new set of links
       this._enhancedLinks.clear();
+      this._frequencyCaps.clear();
       this._suggestedLinks.clear();
 
       let validityFilter = function(link) {
@@ -438,13 +464,19 @@ let DirectoryLinksProvider = {
         // We cache suggested tiles here but do not push any of them in the links list yet.
         // The decision for which suggested tile to include will be made separately.
         this._cacheSuggestedLinks(link);
+        this._frequencyCaps.set(link.url, DEFAULT_FREQUENCY_CAP);
       });
 
-      return rawLinks.directory.filter(validityFilter).map((link, position) => {
+      let links = rawLinks.directory.filter(validityFilter).map((link, position) => {
         setCommonProperties(link, rawLinks.directory.length, position);
         link.frecency = DIRECTORY_FRECENCY;
         return link;
       });
+
+      // Allow for one link suggestion on top of the default directory links
+      this.maxNumLinks = links.length + 1;
+
+      return links;
     }).catch(ex => {
       Cu.reportError(ex);
       return [];
@@ -529,6 +561,21 @@ let DirectoryLinksProvider = {
     }, 0);
   },
 
+  /**
+   * Record for a url that some number of views have been used
+   * @param url String url of the suggested link
+   * @param amount Number of equivalent views to decrease
+   */
+  _decreaseFrequencyCap(url, amount) {
+    let remainingViews = this._frequencyCaps.get(url) - amount;
+    this._frequencyCaps.set(url, remainingViews);
+
+    // Reached the number of views, so pick a new one.
+    if (remainingViews <= 0) {
+      this._updateSuggestedTile();
+    }
+  },
+
   /**
    * Chooses and returns a suggested tile based on a user's top sites
    * that we have an available suggested tile for.
@@ -546,13 +593,12 @@ let DirectoryLinksProvider = {
 
     // Delete the current suggested tile, if one exists.
     let initialLength = sortedLinks.length;
-    this.maxNumLinks = initialLength;
     if (initialLength) {
       let mostFrecentLink = sortedLinks[0];
       if (mostFrecentLink.targetedSite) {
         this._callObservers("onLinkChanged", {
           url: mostFrecentLink.url,
-          frecency: 0,
+          frecency: SUGGESTED_FRECENCY,
           lastVisitDate: mostFrecentLink.lastVisitDate,
           type: mostFrecentLink.type,
         }, 0, true);
@@ -574,6 +620,11 @@ let DirectoryLinksProvider = {
     this._topSitesWithSuggestedLinks.forEach(topSiteWithSuggestedLink => {
       let suggestedLinksMap = this._suggestedLinks.get(topSiteWithSuggestedLink);
       suggestedLinksMap.forEach((suggestedLink, url) => {
+        // Skip this link if we've shown it too many times already
+        if (this._frequencyCaps.get(url) <= 0) {
+          return;
+        }
+
         possibleLinks.set(url, suggestedLink);
 
         // Keep a map of URL to targeted sites. We later use this to show the user
@@ -584,10 +635,17 @@ let DirectoryLinksProvider = {
         targetedSites.get(url).push(topSiteWithSuggestedLink);
       })
     });
+
+    // We might have run out of possible links to show
+    let numLinks = possibleLinks.size;
+    if (numLinks == 0) {
+      return;
+    }
+
     let flattenedLinks = [...possibleLinks.values()];
 
     // Choose our suggested link at random
-    let suggestedIndex = Math.floor(Math.random() * flattenedLinks.length);
+    let suggestedIndex = Math.floor(Math.random() * numLinks);
     let chosenSuggestedLink = flattenedLinks[suggestedIndex];
 
     // Show the new directory tile.
@@ -624,11 +682,11 @@ let DirectoryLinksProvider = {
     this._observers.delete(aObserver);
   },
 
-  _callObservers: function DirectoryLinksProvider__callObservers(aMethodName, aArg) {
+  _callObservers(methodName, ...args) {
     for (let obs of this._observers) {
-      if (typeof(obs[aMethodName]) == "function") {
+      if (typeof(obs[methodName]) == "function") {
         try {
-          obs[aMethodName](this, aArg);
+          obs[methodName](this, ...args);
         } catch (err) {
           Cu.reportError(err);
         }

browser/modules/test/xpcshell/test_DirectoryLinksProvider.js

@@ -26,6 +26,7 @@ do_get_profile();
 
 const DIRECTORY_LINKS_FILE = "directoryLinks.json";
 const DIRECTORY_FRECENCY = 1000;
+const SUGGESTED_FRECENCY = Infinity;
 const kURLData = {"directory": [{"url":"http://example.com","title":"LocalSource"}]};
 const kTestURL = 'data:application/json,' + JSON.stringify(kURLData);
 
@@ -247,7 +248,7 @@ add_task(function test_updateSuggestedTile() {
 
         isIdentical([...DirectoryLinksProvider._topSitesWithSuggestedLinks], ["hrblock.com", "1040.com", "freetaxusa.com"]);
         do_check_true(possibleLinks.indexOf(link.url) > -1);
-        do_check_eq(link.frecency, Infinity);
+        do_check_eq(link.frecency, SUGGESTED_FRECENCY);
         do_check_eq(link.type, "affiliate");
         resolve();
       };
@@ -268,10 +269,10 @@ add_task(function test_updateSuggestedTile() {
         if (this.count == 1) {
           // The removed suggested link is the one we added initially.
           do_check_eq(link.url, links.shift().url);
-          do_check_eq(link.frecency, 0);
+          do_check_eq(link.frecency, SUGGESTED_FRECENCY);
         } else {
           links.unshift(link);
-          do_check_eq(link.frecency, Infinity);
+          do_check_eq(link.frecency, SUGGESTED_FRECENCY);
         }
         isIdentical([...DirectoryLinksProvider._topSitesWithSuggestedLinks], ["hrblock.com", "freetaxusa.com"]);
         resolve();
@@ -287,7 +288,7 @@ add_task(function test_updateSuggestedTile() {
 
         do_check_eq(link.type, "affiliate");
         do_check_eq(this.count, 1);
-        do_check_eq(link.frecency, 0);
+        do_check_eq(link.frecency, SUGGESTED_FRECENCY);
         do_check_eq(link.url, links.shift().url);
         isIdentical([...DirectoryLinksProvider._topSitesWithSuggestedLinks], []);
         resolve();
@@ -422,6 +423,142 @@ add_task(function test_topSitesWithSuggestedLinks() {
   NewTabUtils.getProviderLinks = origGetProviderLinks;
 });
 
+add_task(function test_frequencyCappedSites_views() {
+  Services.prefs.setCharPref(kPingUrlPref, "");
+  let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
+  NewTabUtils.isTopPlacesSite = () => true;
+
+  let testUrl = "http://frequency.capped/link";
+  let targets = ["top.site.com"];
+  let data = {
+    suggested: [{
+      type: "sponsored",
+      frecent_sites: targets,
+      url: testUrl
+    }],
+    directory: [{
+      type: "organic",
+      url: "http://directory.site/"
+    }]
+  };
+  let dataURI = "data:application/json," + JSON.stringify(data);
+
+  yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});
+
+  // Wait for links to get loaded
+  let gLinks = NewTabUtils.links;
+  gLinks.addProvider(DirectoryLinksProvider);
+  gLinks.populateCache();
+  yield new Promise(resolve => {
+    NewTabUtils.allPages.register({
+      observe: _ => _,
+      update() {
+        NewTabUtils.allPages.unregister(this);
+        resolve();
+      }
+    });
+  });
+
+  function synthesizeAction(action) {
+    DirectoryLinksProvider.reportSitesAction([{
+      link: {
+        targetedSite: targets[0],
+        url: testUrl
+      }
+    }], action, 0);
+  }
+
+  function checkFirstTypeAndLength(type, length) {
+    let links = gLinks.getLinks();
+    do_check_eq(links[0].type, type);
+    do_check_eq(links.length, length);
+  }
+
+  // Make sure we get 5 views of the link before it is removed
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("view");
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("view");
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("view");
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("view");
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("view");
+  checkFirstTypeAndLength("organic", 1);
+
+  // Cleanup.
+  NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
+  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 testUrl = "http://frequency.capped/link";
+  let targets = ["top.site.com"];
+  let data = {
+    suggested: [{
+      type: "sponsored",
+      frecent_sites: targets,
+      url: testUrl
+    }],
+    directory: [{
+      type: "organic",
+      url: "http://directory.site/"
+    }]
+  };
+  let dataURI = "data:application/json," + JSON.stringify(data);
+
+  yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});
+
+  // Wait for links to get loaded
+  let gLinks = NewTabUtils.links;
+  gLinks.addProvider(DirectoryLinksProvider);
+  gLinks.populateCache();
+  yield new Promise(resolve => {
+    NewTabUtils.allPages.register({
+      observe: _ => _,
+      update() {
+        NewTabUtils.allPages.unregister(this);
+        resolve();
+      }
+    });
+  });
+
+  function synthesizeAction(action) {
+    DirectoryLinksProvider.reportSitesAction([{
+      link: {
+        targetedSite: targets[0],
+        url: testUrl
+      }
+    }], action, 0);
+  }
+
+  function checkFirstTypeAndLength(type, length) {
+    let links = gLinks.getLinks();
+    do_check_eq(links[0].type, type);
+    do_check_eq(links.length, length);
+  }
+
+  // Make sure the link disappears after the first click
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("view");
+  checkFirstTypeAndLength("sponsored", 2);
+  synthesizeAction("click");
+  checkFirstTypeAndLength("organic", 1);
+
+  // Cleanup.
+  NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
+  gLinks.removeProvider(DirectoryLinksProvider);
+  DirectoryLinksProvider.removeObserver(gLinks);
+  Services.prefs.setCharPref(kPingUrlPref, kPingUrl);
+});
+
 add_task(function test_reportSitesAction() {
   yield DirectoryLinksProvider.init();
   let deferred, expectedPath, expectedPost;