author | Ed Lee <edilee@mozilla.com> |
Sun, 22 Mar 2015 00:46:26 -0700 | |
changeset 236113 | ac13bb5f374caff54cf6f71368232a24141c7e16 |
parent 236112 | a752e16a5251cd9b047f41dcef33f1c8d8187517 |
child 236114 | 8b514f389bf700fd6488469db296b5be70aa7abf |
push id | 57588 |
push user | ryanvm@gmail.com |
push date | Fri, 27 Mar 2015 15:17:31 +0000 |
treeherder | mozilla-inbound@44e454b5e93b [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | adw |
bugs | 1140496 |
milestone | 39.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
|
browser/modules/DirectoryLinksProvider.jsm | file | annotate | diff | comparison | revisions | |
browser/modules/test/xpcshell/test_DirectoryLinksProvider.js | file | annotate | diff | comparison | revisions |
--- 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; }