diff --git a/browser/modules/DirectoryLinksProvider.jsm b/browser/modules/DirectoryLinksProvider.jsm index 0998e5695d1..b955bb0d29a 100644 --- a/browser/modules/DirectoryLinksProvider.jsm +++ b/browser/modules/DirectoryLinksProvider.jsm @@ -96,9 +96,6 @@ const MIN_VISIBLE_HISTORY_TILES = 8; // The max number of visible (not blocked) history tiles to test for inadjacency const MAX_VISIBLE_HISTORY_TILES = 15; -// 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"]; @@ -566,50 +563,13 @@ var DirectoryLinksProvider = { } catch (ex) {} - // Only send pings when enhancing tiles with an endpoint and valid action + // Bug 1240245 - We no longer send pings, but frequency capping and fetching + // tests depend on the following actions, so references to PING remain. let invalidAction = PING_ACTIONS.indexOf(action) == -1; if (!newtabEnhanced || pingEndPoint == "" || invalidAction) { return Promise.resolve(); } - let actionIndex; - let data = { - locale: this.locale, - tiles: sites.reduce((tiles, site, pos) => { - // Only add data for non-empty tiles - if (site) { - // Remember which tiles data triggered the action - let {link} = site; - let tilesIndex = tiles.length; - if (triggeringSiteIndex == pos) { - actionIndex = tilesIndex; - } - - // Make the payload in a way so keys can be excluded when stringified - let id = link.directoryId; - tiles.push({ - id: id || site.enhancedId, - pin: site.isPinned() ? 1 : undefined, - pos: pos != tilesIndex ? pos : undefined, - past_impressions: pos == triggeringSiteIndex ? pastImpressions : undefined, - score: Math.round(link.frecency / PING_SCORE_DIVISOR) || undefined, - url: site.enhancedId && "", - }); - } - return tiles; - }, []), - }; - - // Provide a direct index to the tile triggering the action - if (actionIndex !== undefined) { - data[action] = actionIndex; - } - - // Package the data to be sent with the ping - let ping = this._newXHR(); - ping.open("POST", pingEndPoint + (action == "view" ? "view" : "click")); - ping.send(JSON.stringify(data)); - return Task.spawn(function* () { // since we updated views/clicks we need write _frequencyCaps to disk yield this._writeFrequencyCapFile(); diff --git a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js index 803cf10e607..f0d64d419a6 100644 --- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js +++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js @@ -716,113 +716,6 @@ add_task(function* test_frequencyCappedSites_click() { 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; - } - - do_check_eq(aRequest.path, expectedPath); - - let bodyStream = new BinaryInputStream(aRequest.bodyInputStream); - let bodyObject = JSON.parse(NetUtil.readInputStreamToString(bodyStream, bodyStream.available())); - isIdentical(bodyObject, expectedPost); - - deferred.resolve(); - }); - - function sendPingAndTest(path, action, index) { - deferred = Promise.defer(); - expectedPath = kPingPath + path; - DirectoryLinksProvider.reportSitesAction(sites, action, index); - return deferred.promise; - } - - // Start with a single pinned link at position 3 - let sites = [,,{ - isPinned: _ => true, - link: { - directoryId: 1, - frecency: 30000, - url: "http://directory1/", - }, - }]; - - // Make sure we get the click ping for the directory link with fields we want - // and unwanted fields removed by stringify/parse - expectedPost = JSON.parse(JSON.stringify({ - click: 0, - locale: "en-US", - tiles: [{ - id: 1, - pin: 1, - pos: 2, - score: 3, - url: undefined, - }], - })); - yield sendPingAndTest("click", "click", 2); - - // Try a pin click ping - delete expectedPost.click; - expectedPost.pin = 0; - yield sendPingAndTest("click", "pin", 2); - - // Try a block click ping - delete expectedPost.pin; - expectedPost.block = 0; - yield sendPingAndTest("click", "block", 2); - - // A view ping has no actions - delete expectedPost.block; - expectedPost.view = 0; - yield sendPingAndTest("view", "view", 2); - - // Remove the identifier that makes it a directory link so just plain history - delete sites[2].link.directoryId; - delete expectedPost.tiles[0].id; - yield sendPingAndTest("view", "view", 2); - - // Add directory link at position 0 - sites[0] = { - isPinned: _ => false, - link: { - directoryId: 1234, - frecency: 1000, - url: "http://directory/", - } - }; - expectedPost.tiles.unshift(JSON.parse(JSON.stringify({ - id: 1234, - pin: undefined, - pos: undefined, - score: undefined, - url: undefined, - }))); - expectedPost.view = 1; - yield sendPingAndTest("view", "view", 2); - - // Make the history tile enhanced so it reports both id and url - sites[2].enhancedId = "id from enhanced"; - expectedPost.tiles[1].id = "id from enhanced"; - expectedPost.tiles[1].url = ""; - yield sendPingAndTest("view", "view", 2); - - // Click the 0th site / 0th tile - delete expectedPost.view; - expectedPost.click = 0; - yield sendPingAndTest("click", "click", 0); - - // Click the 2th site / 1th tile - expectedPost.click = 1; - yield sendPingAndTest("click", "click", 2); - - done = true; -}); - add_task(function* test_fetchAndCacheLinks_local() { yield DirectoryLinksProvider.init(); yield cleanJsonFile(); @@ -1901,110 +1794,6 @@ add_task(function* test_inadjecentSites() { yield promiseCleanDirectoryLinksProvider(); }); -add_task(function* test_reportPastImpressions() { - 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, - adgroup_name: "Test" - }] - }; - let dataURI = "data:application/json," + JSON.stringify(data); - yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); - - // make DirectoryLinksProvider load json - let loadPromise = Promise.defer(); - DirectoryLinksProvider.getLinks(_ => {loadPromise.resolve();}); - yield loadPromise.promise; - - // setup ping handler - let deferred, expectedPath, expectedAction, expectedImpressions; - let done = false; - server.registerPrefixHandler(kPingPath, (aRequest, aResponse) => { - if (done) { - return; - } - - do_check_eq(aRequest.path, expectedPath); - - let bodyStream = new BinaryInputStream(aRequest.bodyInputStream); - let bodyObject = JSON.parse(NetUtil.readInputStreamToString(bodyStream, bodyStream.available())); - let expectedActionIndex = bodyObject[expectedAction]; - if (bodyObject.unpin) { - // unpin should not report past_impressions - do_check_false(bodyObject.tiles[expectedActionIndex].hasOwnProperty("past_impressions")); - } - else if (expectedImpressions) { - do_check_eq(bodyObject.tiles[expectedActionIndex].past_impressions.total, expectedImpressions); - do_check_eq(bodyObject.tiles[expectedActionIndex].past_impressions.daily, expectedImpressions); - } - else { - do_check_eq(expectedPath, "/ping/view"); - do_check_false(bodyObject.tiles[expectedActionIndex].hasOwnProperty("past_impressions")); - } - - deferred.resolve(); - }); - - // setup ping sender - function sendPingAndTest(path, action, index) { - deferred = Promise.defer(); - expectedPath = kPingPath + path; - expectedAction = action; - DirectoryLinksProvider.reportSitesAction(sites, action, index); - return deferred.promise; - } - - // Start with a view ping first - let site = { - isPinned: _ => false, - link: { - directoryId: 1, - frecency: 30000, - frecent_sites: targets, - targetedSite: targets[0], - url: testUrl - } - }; - let sites = [, - { - isPinned: _ => false, - link: {type: "history", url: "https://foo.com"} - }, - site - ]; - - yield sendPingAndTest("view", "view", 2); - yield sendPingAndTest("view", "view", 2); - yield sendPingAndTest("view", "view", 2); - - expectedImpressions = DirectoryLinksProvider._frequencyCaps[testUrl].totalViews; - do_check_eq(expectedImpressions, 3); - - // now report pin, unpin, block and click - sites.isPinned = _ => true; - yield sendPingAndTest("click", "pin", 2); - sites.isPinned = _ => false; - yield sendPingAndTest("click", "unpin", 2); - sites.isPinned = _ => false; - yield sendPingAndTest("click", "click", 2); - sites.isPinned = _ => false; - yield sendPingAndTest("click", "block", 2); - - // Cleanup. - done = true; - NewTabUtils.isTopPlacesSite = origIsTopPlacesSite; - DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount; -}); - add_task(function* test_blockSuggestedTiles() { // Initial setup let suggestedTile = suggestedTile1;