Bug 1140496 - Only show a suggested tile url for some number of times or until clicked [r=adw, a=sylvestre]
authorEd Lee <edilee@mozilla.com>
Sun, 22 Mar 2015 00:46:26 -0700
changeset 258197 4afccec73fb9
parent 258196 60f350a6b8b8
child 258198 6892b485a7e0
push id4619
push useredilee@gmail.com
push date2015-04-02 05:50 +0000
treeherdermozilla-beta@daf8a9291a9b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, sylvestre
bugs1140496
milestone38.0
Bug 1140496 - Only show a suggested tile url for some number of times or until clicked [r=adw, a=sylvestre] Default to a hardcoded frequency cap that decreases by 1 per view or all for a click.
browser/modules/DirectoryLinksProvider.jsm
browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
--- a/browser/modules/DirectoryLinksProvider.jsm
+++ b/browser/modules/DirectoryLinksProvider.jsm
@@ -55,16 +55,19 @@ const ALLOWED_LINK_SCHEMES = new Set(["h
 const ALLOWED_IMAGE_SCHEMES = new Set(["https", "data"]);
 
 // The frecency of a directory link
 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;
 
 // 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.
@@ -84,16 +87,21 @@ let DirectoryLinksProvider = {
   _downloadIntervalMS: 86400000,
 
   /**
    * A mapping from eTLD+1 to an enhanced link objects
    */
   _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
    */
   _suggestedLinks: new Map(),
 
   /**
    * A set of top sites that we can provide suggested links for
    */
   _topSitesWithSuggestedLinks: new Set(),
@@ -320,16 +328,33 @@ let DirectoryLinksProvider = {
   /**
    * Report some action on a newtab page (view, click)
    * @param sites Array of sites shown on newtab page
    * @param action String of the behavior to report
    * @param triggeringSiteIndex optional Int index of the site triggering action
    * @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 {
       newtabEnhanced = Services.prefs.getBoolPref(PREF_NEWTAB_ENHANCED);
       pingEndPoint = Services.prefs.getCharPref(PREF_DIRECTORY_PING);
     }
     catch (ex) {}
 
@@ -410,16 +435,17 @@ let DirectoryLinksProvider = {
   /**
    * Gets the current set of directory links.
    * @param aCallback The function that the array of links is passed to.
    */
   getLinks: function DirectoryLinksProvider_getLinks(aCallback) {
     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) {
         // Make sure the link url is allowed and images too if they exist
         return this.isURLAllowed(link.url, ALLOWED_LINK_SCHEMES) &&
                this.isURLAllowed(link.imageURI, ALLOWED_IMAGE_SCHEMES) &&
                this.isURLAllowed(link.enhancedImageURI, ALLOWED_IMAGE_SCHEMES);
       }.bind(this);
@@ -433,23 +459,29 @@ let DirectoryLinksProvider = {
       }.bind(this);
 
       rawLinks.suggested.filter(validityFilter).forEach((link, position) => {
         setCommonProperties(link, rawLinks.suggested.length, position);
 
         // 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 [];
     }).then(links => {
       aCallback(links);
       this._populatePlacesLinks();
     });
   },
@@ -525,39 +557,53 @@ let DirectoryLinksProvider = {
   onManyLinksChanged: function () {
     // Make sure NewTabUtils.links handles the notification first.
     setTimeout(() => {
       this._handleManyLinksChanged();
     }, 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.
    *
    * @return the chosen suggested tile, or undefined if there isn't one
    */
   _updateSuggestedTile: function() {
     let sortedLinks = NewTabUtils.getProviderLinks(this);
 
     if (!sortedLinks) {
       // If NewTabUtils.links.resetCache() is called before getting here,
       // sortedLinks may be undefined.
       return;
     }
 
     // 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);
       }
     }
 
     if (this._topSitesWithSuggestedLinks.size == 0) {
       // There are no potential suggested links we can show.
@@ -569,30 +615,42 @@ let DirectoryLinksProvider = {
     // 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.
     let possibleLinks = new Map();
     let targetedSites = new Map();
     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
         // what site they visited to trigger this suggestion.
         if (!targetedSites.get(url)) {
           targetedSites.set(url, []);
         }
         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.
     this._callObservers("onLinkChanged", {
       url: chosenSuggestedLink.url,
       title: chosenSuggestedLink.title,
       frecency: SUGGESTED_FRECENCY,
       lastVisitDate: chosenSuggestedLink.lastVisitDate,
@@ -619,21 +677,21 @@ let DirectoryLinksProvider = {
   addObserver: function DirectoryLinksProvider_addObserver(aObserver) {
     this._observers.add(aObserver);
   },
 
   removeObserver: function DirectoryLinksProvider_removeObserver(aObserver) {
     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);
         }
       }
     }
   },
 
   _removeObservers: function() {
--- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
+++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
@@ -21,16 +21,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils",
   "resource://gre/modules/NewTabUtils.jsm");
 
 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);
 
 // DirectoryLinksProvider preferences
 const kLocalePref = DirectoryLinksProvider._observedPrefs.prefSelectedLocale;
 const kSourceUrlPref = DirectoryLinksProvider._observedPrefs.linksURL;
 const kPingUrlPref = "browser.newtabpage.directory.ping";
 const kNewtabEnhancedPref = "browser.newtabpage.enhanced";
@@ -242,17 +243,17 @@ add_task(function test_updateSuggestedTi
   function TestFirstRun() {
     this.promise = new Promise(resolve => {
       this.onLinkChanged = (directoryLinksProvider, link) => {
         links.unshift(link);
         let possibleLinks = [suggestedTile1.url, suggestedTile2.url, suggestedTile3.url];
 
         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();
       };
     });
   }
 
   function TestChangingSuggestedTile() {
     this.count = 0;
@@ -263,36 +264,36 @@ add_task(function test_updateSuggestedTi
 
         do_check_true(possibleLinks.indexOf(link.url) > -1);
         do_check_eq(link.type, "affiliate");
         do_check_true(this.count <= 2);
 
         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();
       }
     });
   }
 
   function TestRemovingSuggestedTile() {
     this.count = 0;
     this.promise = new Promise(resolve => {
       this.onLinkChanged = (directoryLinksProvider, link) => {
         this.count++;
 
         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();
       }
     });
   }
 
   // Test first call to '_updateSuggestedTile()', called when fetching directory links.
@@ -417,16 +418,152 @@ add_task(function test_topSitesWithSugge
   DirectoryLinksProvider._handleLinkChanged({url: "http://" + popped});
   isIdentical([...DirectoryLinksProvider._topSitesWithSuggestedLinks], expectedTopSitesWithSuggestedLinks);
 
   // Cleanup.
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
   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;
   let done = false;
   server.registerPrefixHandler(kPingPath, (aRequest, aResponse) => {
     if (done) {
       return;
     }