Bug 1140496 - Only show a suggested tile url for some number of times or until clicked [r=adw]
authorEd Lee <edilee@mozilla.com>
Sun, 22 Mar 2015 00:46:26 -0700
changeset 266266 ac13bb5f374caff54cf6f71368232a24141c7e16
parent 266265 a752e16a5251cd9b047f41dcef33f1c8d8187517
child 266267 8b514f389bf700fd6488469db296b5be70aa7abf
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1140496
milestone39.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 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
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;
     }