Bug 1240245 - Disable data collection for new-tab page r=jaws
authorOlivier Yiptong <olivier@olivieryiptong.com>
Wed, 20 Jan 2016 16:58:01 -0500
changeset 280888 1fc5f691e48258d3f9be7f857d0a081b6a473790
parent 280887 7b9c54abd2fe8932e19ad57e3b334684a8c7f77d
child 280889 9e771bb631ce8d732af7d389837aed8561ac2bf6
push id29922
push usercbook@mozilla.com
push dateThu, 21 Jan 2016 10:51:00 +0000
treeherdermozilla-central@977d78a8dd78 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1240245
milestone46.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 1240245 - Disable data collection for new-tab page r=jaws
browser/modules/DirectoryLinksProvider.jsm
browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
--- a/browser/modules/DirectoryLinksProvider.jsm
+++ b/browser/modules/DirectoryLinksProvider.jsm
@@ -91,19 +91,16 @@ const DEFAULT_TOTAL_FREQUENCY_CAP = 10;
 const DEFAULT_PRUNE_TIME_DELTA = 10*24*60*60*1000;
 
 // The min number of visible (not blocked) history tiles to have before showing suggested tiles
 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"];
 
 // Location of inadjacent sites json
 const INADJACENCY_SOURCE = "chrome://browser/content/newtab/newTab.inadjacent.json";
 
 // Fake URL to keep track of last block of a suggested tile in the frequency cap object
 const FAKE_SUGGESTED_BLOCK_URL = "ignore://suggested_block";
@@ -561,60 +558,23 @@ var DirectoryLinksProvider = {
     let newtabEnhanced = false;
     let pingEndPoint = "";
     try {
       newtabEnhanced = Services.prefs.getBoolPref(PREF_NEWTAB_ENHANCED);
       pingEndPoint = Services.prefs.getCharPref(PREF_DIRECTORY_PING);
     }
     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();
       // Use this as an opportunity to potentially fetch new links
       yield this._fetchAndCacheLinksIfNecessary();
     }.bind(this));
   },
 
--- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
+++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
@@ -711,123 +711,16 @@ add_task(function* test_frequencyCappedS
   // Cleanup.
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
   DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
   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;
-    }
-
-    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();
   // Trigger cache of data or chrome uri files in profD
   yield DirectoryLinksProvider._fetchAndCacheLinks(kTestURL);
   let data = yield readJsonFile();
   isIdentical(data, kURLData);
 });
@@ -1896,120 +1789,16 @@ add_task(function* test_inadjecentSites(
   gLinks.removeProvider(DirectoryLinksProvider);
   DirectoryLinksProvider._inadjacentSitesUrl = origInadjacentSitesUrl;
   NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
   NewTabUtils.getProviderLinks = origGetProviderLinks;
   DirectoryLinksProvider._getCurrentTopSiteCount = origCurrentTopSiteCount;
   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;
   let topSites = ["site0.com", "1040.com", "site2.com", "hrblock.com", "site4.com", "freetaxusa.com", "site6.com"];
   let data = {"suggested": [suggestedTile1, suggestedTile2, suggestedTile3], "directory": [someOtherSite]};
   let dataURI = 'data:application/json,' + JSON.stringify(data);
 
   yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});