Backed out changesets 8522ea4f4621, c6d45a7a0eec, and 7d72517398ba (bug 1126184) for newtab mochitest-bc failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Fri, 13 Mar 2015 14:19:45 -0400
changeset 251875 14460ab11c3b5c56803931022b448607689cdd5a
parent 251874 697c416f8dc6e36bf094507f8feef689886906b6
child 251876 38154607d807c1b2ad4f83959c97603dd8b0b297
push id7860
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:46:02 +0000
treeherdermozilla-aurora@8ac636cd51f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1126184
milestone39.0a1
backs out8522ea4f4621ac0e02ae9784fed8fe14654be7ac
c6d45a7a0eecf410d2929e95febad67a57a4d134
7d72517398ba412a672a2f12a0cab4c38cea7e83
Backed out changesets 8522ea4f4621, c6d45a7a0eec, and 7d72517398ba (bug 1126184) for newtab mochitest-bc failures. CLOSED TREE
browser/base/content/test/newtab/browser_newtab_drag_drop.js
browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js
browser/modules/DirectoryLinksProvider.jsm
browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/browser/base/content/test/newtab/browser_newtab_drag_drop.js
+++ b/browser/base/content/test/newtab/browser_newtab_drag_drop.js
@@ -4,17 +4,16 @@
 /*
  * These tests make sure that dragging and dropping sites works as expected.
  * Sites contained in the grid need to shift around to indicate the result
  * of the drag-and-drop operation. If the grid is full and we're dragging
  * a new site into it another one gets pushed out.
  */
 function runTests() {
   requestLongerTimeout(2);
-  yield addNewTabPageTab();
 
   // test a simple drag-and-drop scenario
   yield setLinks("0,1,2,3,4,5,6,7,8");
   setPinnedLinks("");
 
   yield addNewTabPageTab();
   checkGrid("0,1,2,3,4,5,6,7,8");
 
--- a/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js
+++ b/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js
@@ -8,17 +8,16 @@ const PREF_NEWTAB_COLUMNS = "browser.new
  * Sites contained in the grid need to shift around to indicate the result
  * of the drag-and-drop operation. If the grid is full and we're dragging
  * a new site into it another one gets pushed out.
  * This is a continuation of browser_newtab_drag_drop.js
  * to decrease test run time, focusing on external sites.
  */
 function runTests() {
   registerCleanupFunction(_ => Services.prefs.clearUserPref(PREF_NEWTAB_COLUMNS));
-  yield addNewTabPageTab();
 
   // drag a new site onto the very first cell
   yield setLinks("0,1,2,3,4,5,6,7,8");
   setPinnedLinks(",,,,,,,7,8");
 
   yield addNewTabPageTab();
   checkGrid("0,1,2,3,4,5,6,7p,8p");
 
--- a/browser/modules/DirectoryLinksProvider.jsm
+++ b/browser/modules/DirectoryLinksProvider.jsm
@@ -10,17 +10,16 @@ const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 const XMLHttpRequest =
   Components.Constructor("@mozilla.org/xmlextras/xmlhttprequest;1", "nsIXMLHttpRequest");
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils",
   "resource://gre/modules/NewTabUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
   "resource://gre/modules/osfile.jsm")
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
@@ -52,19 +51,16 @@ const PREF_NEWTAB_ENHANCED = "browser.ne
 const ALLOWED_LINK_SCHEMES = new Set(["http", "https"]);
 
 // Only allow link image urls that are https or data
 const ALLOWED_IMAGE_SCHEMES = new Set(["https", "data"]);
 
 // The frecency of a directory link
 const DIRECTORY_FRECENCY = 1000;
 
-// The frecency of a related link
-const RELATED_FRECENCY = Infinity;
-
 // 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.
@@ -88,21 +84,16 @@ let DirectoryLinksProvider = {
    */
   _enhancedLinks: new Map(),
 
   /**
    * A mapping from site to a list of related link objects
    */
   _relatedLinks: new Map(),
 
-  /**
-   * A set of top sites that we can provide related links for
-   */
-  _topSitesWithRelatedLinks: new Set(),
-
   get _observedPrefs() Object.freeze({
     enhanced: PREF_NEWTAB_ENHANCED,
     linksURL: PREF_DIRECTORY_SOURCE,
     matchOSLocale: PREF_MATCH_OS_LOCALE,
     prefSelectedLocale: PREF_SELECTED_LOCALE,
   }),
 
   get _linksURL() {
@@ -436,160 +427,37 @@ let DirectoryLinksProvider = {
         }
         link.frecency = DIRECTORY_FRECENCY;
         links.push(link);
       });
       return links;
     }).catch(ex => {
       Cu.reportError(ex);
       return [];
-    }).then(links => {
-      aCallback(links);
-      this._populatePlacesLinks();
-    });
+    }).then(aCallback);
   },
 
   init: function DirectoryLinksProvider_init() {
     this._setDefaultEnhanced();
     this._addPrefsObserver();
     // setup directory file path and last download timestamp
     this._directoryFilePath = OS.Path.join(OS.Constants.Path.localProfileDir, DIRECTORY_LINKS_FILE);
     this._lastDownloadMS = 0;
-
-    NewTabUtils.placesProvider.addObserver(this);
-
     return Task.spawn(function() {
       // get the last modified time of the links file if it exists
       let doesFileExists = yield OS.File.exists(this._directoryFilePath);
       if (doesFileExists) {
         let fileInfo = yield OS.File.stat(this._directoryFilePath);
         this._lastDownloadMS = Date.parse(fileInfo.lastModificationDate);
       }
       // fetch directory on startup without force
       yield this._fetchAndCacheLinksIfNecessary();
     }.bind(this));
   },
 
-  _handleManyLinksChanged: function() {
-    this._topSitesWithRelatedLinks.clear();
-    this._relatedLinks.forEach((relatedLinks, site) => {
-      if (NewTabUtils.isTopPlacesSite(site)) {
-        this._topSitesWithRelatedLinks.add(site);
-      }
-    });
-    this._updateRelatedTile();
-  },
-
-  /**
-   * Updates _topSitesWithRelatedLinks based on the link that was changed.
-   *
-   * @return true if _topSitesWithRelatedLinks was modified, false otherwise.
-   */
-  _handleLinkChanged: function(aLink) {
-    let changedLinkSite = NewTabUtils.extractSite(aLink.url);
-    let linkStored = this._topSitesWithRelatedLinks.has(changedLinkSite);
-
-    if (!NewTabUtils.isTopPlacesSite(changedLinkSite) && linkStored) {
-      this._topSitesWithRelatedLinks.delete(changedLinkSite);
-      return true;
-    }
-
-    if (this._relatedLinks.has(changedLinkSite) &&
-        NewTabUtils.isTopPlacesSite(changedLinkSite) && !linkStored) {
-      this._topSitesWithRelatedLinks.add(changedLinkSite);
-      return true;
-    }
-    return false;
-  },
-
-  _populatePlacesLinks: function () {
-    NewTabUtils.links.populateProviderCache(NewTabUtils.placesProvider, () => {
-      this._handleManyLinksChanged();
-    });
-  },
-
-  onLinkChanged: function (aProvider, aLink) {
-    // Make sure NewTabUtils.links handles the notification first.
-    setTimeout(() => {
-      if (this._handleLinkChanged(aLink)) {
-        this._updateRelatedTile();
-      }
-    }, 0);
-  },
-
-  onManyLinksChanged: function () {
-    // Make sure NewTabUtils.links handles the notification first.
-    setTimeout(() => {
-      this._handleManyLinksChanged();
-    }, 0);
-  },
-
-  /**
-   * Chooses and returns a related tile based on a user's top sites
-   * that we have an available related tile for.
-   *
-   * @return the chosen related tile, or undefined if there isn't one
-   */
-  _updateRelatedTile: 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 related tile, if one exists.
-    let initialLength = sortedLinks.length;
-    this.maxNumLinks = initialLength;
-    if (initialLength) {
-      let mostFrecentLink = sortedLinks[0];
-      if ("related" == mostFrecentLink.type) {
-        this._callObservers("onLinkChanged", {
-          url: mostFrecentLink.url,
-          frecency: 0,
-          lastVisitDate: mostFrecentLink.lastVisitDate,
-          type: "related",
-        }, 0, true);
-      }
-    }
-
-    if (this._topSitesWithRelatedLinks.size == 0) {
-      // There are no potential related links we can show.
-      return;
-    }
-
-    // Create a flat list of all possible links we can show as related.
-    // Note that many top sites may map to the same related links, but we only
-    // want to count each related link once (based on url), thus possibleLinks is a map
-    // from url to relatedLink. Thus, each link has an equal chance of being chosen at
-    // random from flattenedLinks if it appears only once.
-    let possibleLinks = new Map();
-    this._topSitesWithRelatedLinks.forEach(topSiteWithRelatedLink => {
-      let relatedLinksMap = this._relatedLinks.get(topSiteWithRelatedLink);
-      relatedLinksMap.forEach((relatedLink, url) => {
-        possibleLinks.set(url, relatedLink);
-      })
-    });
-    let flattenedLinks = [...possibleLinks.values()];
-
-    // Choose our related link at random
-    let relatedIndex = Math.floor(Math.random() * flattenedLinks.length);
-    let chosenRelatedLink = flattenedLinks[relatedIndex];
-
-    // Show the new directory tile.
-    this._callObservers("onLinkChanged", {
-      url: chosenRelatedLink.url,
-      frecency: RELATED_FRECENCY,
-      lastVisitDate: chosenRelatedLink.lastVisitDate,
-      type: "related",
-    });
-    return chosenRelatedLink;
-   },
-
   /**
    * Return the object to its pre-init state
    */
   reset: function DirectoryLinksProvider_reset() {
     delete this.__linksURL;
     this._removePrefsObserver();
     this._removeObservers();
   },
--- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
+++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
@@ -14,18 +14,16 @@ Cu.import("resource://gre/modules/Promis
 Cu.import("resource://gre/modules/Http.jsm");
 Cu.import("resource://testing-common/httpd.js");
 Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
   "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 kURLData = {"en-US": [{"url":"http://example.com","title":"LocalSource"}]};
 const kTestURL = 'data:application/json,' + JSON.stringify(kURLData);
 
@@ -55,52 +53,16 @@ Services.prefs.setBoolPref(kNewtabEnhanc
 const kHttpHandlerData = {};
 kHttpHandlerData[kExamplePath] = {"en-US": [{"url":"http://example.com","title":"RemoteSource"}]};
 
 const BinaryInputStream = CC("@mozilla.org/binaryinputstream;1",
                               "nsIBinaryInputStream",
                               "setInputStream");
 
 let gLastRequestPath;
-
-let relatedTile1 = {
-  url: "http://turbotax.com",
-  type: "related",
-  lastVisitDate: 4,
-  related: [
-    "taxact.com",
-    "hrblock.com",
-    "1040.com",
-    "taxslayer.com"
-  ]
-};
-let relatedTile2 = {
-  url: "http://irs.gov",
-  type: "related",
-  lastVisitDate: 3,
-  related: [
-    "taxact.com",
-    "hrblock.com",
-    "freetaxusa.com",
-    "taxslayer.com"
-  ]
-};
-let relatedTile3 = {
-  url: "http://hrblock.com",
-  type: "related",
-  lastVisitDate: 2,
-  related: [
-    "taxact.com",
-    "freetaxusa.com",
-    "1040.com",
-    "taxslayer.com"
-  ]
-};
-let someOtherSite = {url: "http://someothersite.com", title: "Not_A_Related_Site"};
-
 function getHttpHandler(path) {
   let code = 200;
   let body = JSON.stringify(kHttpHandlerData[path]);
   if (path == kFailPath) {
     code = 204;
   }
   return function(aRequest, aResponse) {
     gLastRequestPath = aRequest.path;
@@ -194,157 +156,65 @@ function promiseCleanDirectoryLinksProvi
 }
 
 function run_test() {
   // Set up a mock HTTP server to serve a directory page
   server = new HttpServer();
   server.registerPrefixHandler(kExamplePath, getHttpHandler(kExamplePath));
   server.registerPrefixHandler(kFailPath, getHttpHandler(kFailPath));
   server.start(kDefaultServerPort);
-  NewTabUtils.init();
 
   run_next_test();
 
   // Teardown.
   do_register_cleanup(function() {
     server.stop(function() { });
     DirectoryLinksProvider.reset();
     Services.prefs.clearUserPref(kLocalePref);
     Services.prefs.clearUserPref(kSourceUrlPref);
     Services.prefs.clearUserPref(kPingUrlPref);
     Services.prefs.clearUserPref(kNewtabEnhancedPref);
   });
 }
 
-add_task(function test_updateRelatedTile() {
-  let topSites = ["site0.com", "1040.com", "site2.com", "hrblock.com", "site4.com", "freetaxusa.com", "site6.com"];
-
-  // Initial setup
-  let data = {"en-US": [relatedTile1, relatedTile2, relatedTile3, someOtherSite]};
-  let dataURI = 'data:application/json,' + JSON.stringify(data);
-
-  let testObserver = new TestFirstRun();
-  DirectoryLinksProvider.addObserver(testObserver);
-
-  yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});
-  let links = yield fetchData();
-
-  let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
-  NewTabUtils.isTopPlacesSite = function(site) {
-    return topSites.indexOf(site) >= 0;
-  }
-
-  let origGetProviderLinks = NewTabUtils.getProviderLinks;
-  NewTabUtils.getProviderLinks = function(provider) {
-    return links;
-  }
-
-  do_check_eq(DirectoryLinksProvider._updateRelatedTile(), undefined);
-
-  function TestFirstRun() {
-    this.promise = new Promise(resolve => {
-      this.onLinkChanged = (directoryLinksProvider, link) => {
-        links.unshift(link);
-        let possibleLinks = [relatedTile1.url, relatedTile2.url, relatedTile3.url];
-
-        isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], ["hrblock.com", "1040.com", "freetaxusa.com"]);
-        do_check_true(possibleLinks.indexOf(link.url) > -1);
-        do_check_eq(link.frecency, Infinity);
-        do_check_eq(link.type, "related");
-        resolve();
-      };
-    });
-  }
-
-  function TestChangingRelatedTile() {
-    this.count = 0;
-    this.promise = new Promise(resolve => {
-      this.onLinkChanged = (directoryLinksProvider, link) => {
-        this.count++;
-        let possibleLinks = [relatedTile1.url, relatedTile2.url, relatedTile3.url];
-
-        do_check_true(possibleLinks.indexOf(link.url) > -1);
-        do_check_eq(link.type, "related");
-        do_check_true(this.count <= 2);
-
-        if (this.count == 1) {
-          // The removed related link is the one we added initially.
-          do_check_eq(link.url, links.shift().url);
-          do_check_eq(link.frecency, 0);
-        } else {
-          links.unshift(link);
-          do_check_eq(link.frecency, Infinity);
-        }
-        isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], ["hrblock.com", "freetaxusa.com"]);
-        resolve();
-      }
-    });
-  }
-
-  function TestRemovingRelatedTile() {
-    this.count = 0;
-    this.promise = new Promise(resolve => {
-      this.onLinkChanged = (directoryLinksProvider, link) => {
-        this.count++;
-
-        do_check_eq(link.type, "related");
-        do_check_eq(this.count, 1);
-        do_check_eq(link.frecency, 0);
-        do_check_eq(link.url, links.shift().url);
-        isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], []);
-        resolve();
-      }
-    });
-  }
-
-  // Test first call to '_updateRelatedTile()', called when fetching directory links.
-  yield testObserver.promise;
-  DirectoryLinksProvider.removeObserver(testObserver);
-
-  // Removing a top site that doesn't have a related link should
-  // not change the current related tile.
-  let removedTopsite = topSites.shift();
-  do_check_eq(removedTopsite, "site0.com");
-  do_check_false(NewTabUtils.isTopPlacesSite(removedTopsite));
-  let updateRelatedTile = DirectoryLinksProvider._handleLinkChanged({
-    url: "http://" + removedTopsite,
-    type: "history",
-  });
-  do_check_false(updateRelatedTile);
-
-  // Removing a top site that has a related link should
-  // remove any current related tile and add a new one.
-  testObserver = new TestChangingRelatedTile();
-  DirectoryLinksProvider.addObserver(testObserver);
-  removedTopsite = topSites.shift();
-  do_check_eq(removedTopsite, "1040.com");
-  do_check_false(NewTabUtils.isTopPlacesSite(removedTopsite));
-  DirectoryLinksProvider.onLinkChanged(DirectoryLinksProvider, {
-    url: "http://" + removedTopsite,
-    type: "history",
-  });
-  yield testObserver.promise;
-  do_check_eq(testObserver.count, 2);
-  DirectoryLinksProvider.removeObserver(testObserver);
-
-  // Removing all top sites with related links should remove
-  // the current related link and not replace it.
-  topSites = [];
-  testObserver = new TestRemovingRelatedTile();
-  DirectoryLinksProvider.addObserver(testObserver);
-  DirectoryLinksProvider.onManyLinksChanged();
-  yield testObserver.promise;
-
-  // Cleanup
-  yield promiseCleanDirectoryLinksProvider();
-  NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
-  NewTabUtils.getProviderLinks = origGetProviderLinks;
-});
-
 add_task(function test_relatedLinksMap() {
+  let relatedTile1 = {
+    url: "http://turbotax.com",
+    type: "related",
+    lastVisitDate: 4,
+    related: [
+      "taxact.com",
+      "hrblock.com",
+      "1040.com",
+      "taxslayer.com"
+    ]
+  };
+  let relatedTile2 = {
+    url: "http://irs.gov",
+    type: "related",
+    lastVisitDate: 3,
+    related: [
+      "taxact.com",
+      "hrblock.com",
+      "freetaxusa.com",
+      "taxslayer.com"
+    ]
+  };
+  let relatedTile3 = {
+    url: "http://hrblock.com",
+    type: "related",
+    lastVisitDate: 2,
+    related: [
+      "taxact.com",
+      "freetaxusa.com",
+      "1040.com",
+      "taxslayer.com"
+    ]
+  };
+  let someOtherSite = {url: "http://someothersite.com", title: "Not_A_Related_Site"};
   let data = {"en-US": [relatedTile1, relatedTile2, relatedTile3, someOtherSite]};
   let dataURI = 'data:application/json,' + JSON.stringify(data);
 
   yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});
   let links = yield fetchData();
 
   // Ensure the related tiles were not considered directory tiles.
   do_check_eq(links.length, 1);
@@ -365,68 +235,16 @@ add_task(function test_relatedLinksMap()
     for (let link of expected_data[site]) {
       isIdentical(relatedLinksItr.next().value, link);
     }
   })
 
   yield promiseCleanDirectoryLinksProvider();
 });
 
-add_task(function test_topSitesWithRelatedLinks() {
-  let topSites = ["site0.com", "1040.com", "site2.com", "hrblock.com", "site4.com", "freetaxusa.com", "site6.com"];
-  let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite;
-  NewTabUtils.isTopPlacesSite = function(site) {
-    return topSites.indexOf(site) >= 0;
-  }
-
-  // Mock out getProviderLinks() so we don't have to populate cache in NewTabUtils
-  let origGetProviderLinks = NewTabUtils.getProviderLinks;
-  NewTabUtils.getProviderLinks = function(provider) {
-    return [];
-  }
-
-  // We start off with no top sites with related links.
-  do_check_eq(DirectoryLinksProvider._topSitesWithRelatedLinks.size, 0);
-
-  let data = {"en-US": [relatedTile1, relatedTile2, relatedTile3, someOtherSite]};
-  let dataURI = 'data:application/json,' + JSON.stringify(data);
-
-  yield promiseSetupDirectoryLinksProvider({linksURL: dataURI});
-  let links = yield fetchData();
-
-  // Check we've populated related links as expected.
-  do_check_eq(DirectoryLinksProvider._relatedLinks.size, 5);
-
-  // When many sites change, we update _topSitesWithRelatedLinks as expected.
-  let expectedTopSitesWithRelatedLinks = ["hrblock.com", "1040.com", "freetaxusa.com"];
-  DirectoryLinksProvider._handleManyLinksChanged();
-  isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], expectedTopSitesWithRelatedLinks);
-
-  // Removing site6.com as a topsite has no impact on _topSitesWithRelatedLinks.
-  let popped = topSites.pop();
-  DirectoryLinksProvider._handleLinkChanged({url: "http://" + popped});
-  isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], expectedTopSitesWithRelatedLinks);
-
-  // Removing freetaxusa.com as a topsite will remove it from _topSitesWithRelatedLinks.
-  popped = topSites.pop();
-  expectedTopSitesWithRelatedLinks.pop();
-  DirectoryLinksProvider._handleLinkChanged({url: "http://" + popped});
-  isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], expectedTopSitesWithRelatedLinks);
-
-  // Re-adding freetaxusa.com as a topsite will add it to _topSitesWithRelatedLinks.
-  topSites.push(popped);
-  expectedTopSitesWithRelatedLinks.push(popped);
-  DirectoryLinksProvider._handleLinkChanged({url: "http://" + popped});
-  isIdentical([...DirectoryLinksProvider._topSitesWithRelatedLinks], expectedTopSitesWithRelatedLinks);
-
-  // Cleanup.
-  NewTabUtils.isTopPlacesSite = origIsTopPlacesSite;
-  NewTabUtils.getProviderLinks = origGetProviderLinks;
-});
-
 add_task(function test_reportSitesAction() {
   yield DirectoryLinksProvider.init();
   let deferred, expectedPath, expectedPost;
   let done = false;
   server.registerPrefixHandler(kPingPath, (aRequest, aResponse) => {
     if (done) {
       return;
     }
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -7,17 +7,16 @@
 this.EXPORTED_SYMBOLS = ["NewTabUtils"];
 
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Task.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
   "resource://gre/modules/PlacesUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PageThumbs",
   "resource://gre/modules/PageThumbs.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "BinarySearch",
@@ -711,22 +710,27 @@ let PlacesProvider = {
  */
 let Links = {
   /**
    * The maximum number of links returned by getLinks.
    */
   maxNumLinks: LINKS_GET_LINKS_LIMIT,
 
   /**
+   * The link providers.
+   */
+  _providers: new Set(),
+
+  /**
    * A mapping from each provider to an object { sortedLinks, siteMap, linkMap }.
    * sortedLinks is the cached, sorted array of links for the provider.
    * siteMap is a mapping from base domains to URL count associated with the domain.
    * linkMap is a Map from link URLs to link objects.
    */
-  _providers: new Map(),
+  _providerLinks: new Map(),
 
   /**
    * The properties of link objects used to sort them.
    */
   _sortProperties: [
     "frecency",
     "lastVisitDate",
     "url",
@@ -737,27 +741,28 @@ let Links = {
    */
   _populateCallbacks: [],
 
   /**
    * Adds a link provider.
    * @param aProvider The link provider.
    */
   addProvider: function Links_addProvider(aProvider) {
-    this._providers.set(aProvider, null);
+    this._providers.add(aProvider);
     aProvider.addObserver(this);
   },
 
   /**
    * Removes a link provider.
    * @param aProvider The link provider.
    */
   removeProvider: function Links_removeProvider(aProvider) {
     if (!this._providers.delete(aProvider))
       throw new Error("Unknown provider");
+    this._providerLinks.delete(aProvider);
   },
 
   /**
    * Populates the cache with fresh links from the providers.
    * @param aCallback The callback to call when finished (optional).
    * @param aForce When true, populates the cache even when it's already filled.
    */
   populateCache: function Links_populateCache(aCallback, aForce) {
@@ -780,17 +785,17 @@ let Links = {
           } catch (e) {
             // We want to proceed even if a callback fails.
           }
         }
       }
     }
 
     let numProvidersRemaining = this._providers.size;
-    for (let [provider, links] of this._providers) {
+    for (let provider of this._providers) {
       this._populateProviderCache(provider, () => {
         if (--numProvidersRemaining == 0)
           executeCallbacks();
       }, aForce);
     }
 
     this._addObserver();
   },
@@ -830,19 +835,17 @@ let Links = {
 
     return pinnedLinks;
   },
 
   /**
    * Resets the links cache.
    */
   resetCache: function Links_resetCache() {
-    for (let provider of this._providers.keys()) {
-      this._providers.set(provider, null);
-    }
+    this._providerLinks.clear();
   },
 
   /**
    * Compares two links.
    * @param aLink1 The first link.
    * @param aLink2 The second link.
    * @return A negative number if aLink1 is ordered before aLink2, zero if
    *         aLink1 and aLink2 have the same ordering, or a positive number if
@@ -870,80 +873,56 @@ let Links = {
     let previousURLCount = map.get(site);
     if (previousURLCount === 1) {
       map.delete(site);
     } else {
       map.set(site, previousURLCount - 1);
     }
   },
 
-  populateProviderCache: function(provider, callback) {
-    if (!this._providers.has(provider)) {
-      throw new Error("Can only populate provider cache for existing provider.");
-    }
-
-    return this._populateProviderCache(provider, callback, false);
-  },
-
   /**
    * Calls getLinks on the given provider and populates our cache for it.
    * @param aProvider The provider whose cache will be populated.
    * @param aCallback The callback to call when finished.
    * @param aForce When true, populates the provider's cache even when it's
    *               already filled.
    */
-  _populateProviderCache: function (aProvider, aCallback, aForce) {
-    let cache = this._providers.get(aProvider);
-    let createCache = !cache;
-    if (createCache) {
-      cache = {
-        // Start with a resolved promise.
-        populatePromise: new Promise(resolve => resolve()),
-      };
-      this._providers.set(aProvider, cache);
-    }
-    // Chain the populatePromise so that calls are effectively queued.
-    cache.populatePromise = cache.populatePromise.then(() => {
-      return new Promise(resolve => {
-        if (!createCache && !aForce) {
-          aCallback();
-          resolve();
-          return;
-        }
-        aProvider.getLinks(links => {
-          // Filter out null and undefined links so we don't have to deal with
-          // them in getLinks when merging links from providers.
-          links = links.filter((link) => !!link);
-          cache.sortedLinks = links;
-          cache.siteMap = links.reduce((map, link) => {
+  _populateProviderCache: function Links_populateProviderCache(aProvider, aCallback, aForce) {
+    if (this._providerLinks.has(aProvider) && !aForce) {
+      aCallback();
+    } else {
+      aProvider.getLinks(links => {
+        // Filter out null and undefined links so we don't have to deal with
+        // them in getLinks when merging links from providers.
+        links = links.filter((link) => !!link);
+        this._providerLinks.set(aProvider, {
+          sortedLinks: links,
+          siteMap: links.reduce((map, link) => {
             this._incrementSiteMap(map, link);
             return map;
-          }, new Map());
-          cache.linkMap = links.reduce((map, link) => {
+          }, new Map()),
+          linkMap: links.reduce((map, link) => {
             map.set(link.url, link);
             return map;
-          }, new Map());
-          aCallback();
-          resolve();
+          }, new Map()),
         });
+        aCallback();
       });
-    });
+    }
   },
 
   /**
    * Merges the cached lists of links from all providers whose lists are cached.
    * @return The merged list.
    */
   _getMergedProviderLinks: function Links__getMergedProviderLinks() {
     // Build a list containing a copy of each provider's sortedLinks list.
     let linkLists = [];
-    for (let links of this._providers.values()) {
-      if (links && links.sortedLinks) {
-        linkLists.push(links.sortedLinks.slice());
-      }
+    for (let links of this._providerLinks.values()) {
+      linkLists.push(links.sortedLinks.slice());
     }
 
     function getNextLink() {
       let minLinks = null;
       for (let links of linkLists) {
         if (links.length &&
             (!minLinks || Links.compareLinks(links[0], minLinks[0]) < 0))
           minLinks = links;
@@ -962,67 +941,50 @@ let Links = {
   },
 
   /**
    * Called by a provider to notify us when a single link changes.
    * @param aProvider The provider whose link changed.
    * @param aLink The link that changed.  If the link is new, it must have all
    *              of the _sortProperties.  Otherwise, it may have as few or as
    *              many as is convenient.
-   * @param aIndex The current index of the changed link in the sortedLinks
-                   cache in _providers. Defaults to -1 if the provider doesn't know the index
-   * @param aDeleted Boolean indicating if the provider has deleted the link.
    */
-  onLinkChanged: function Links_onLinkChanged(aProvider, aLink, aIndex=-1, aDeleted=false) {
+  onLinkChanged: function Links_onLinkChanged(aProvider, aLink) {
     if (!("url" in aLink))
       throw new Error("Changed links must have a url property");
 
-    let links = this._providers.get(aProvider);
+    let links = this._providerLinks.get(aProvider);
     if (!links)
       // This is not an error, it just means that between the time the provider
       // was added and the future time we call getLinks on it, it notified us of
       // a change.
       return;
 
     let { sortedLinks, siteMap, linkMap } = links;
     let existingLink = linkMap.get(aLink.url);
     let insertionLink = null;
     let updatePages = false;
 
     if (existingLink) {
       // Update our copy's position in O(lg n) by first removing it from its
       // list.  It's important to do this before modifying its properties.
       if (this._sortProperties.some(prop => prop in aLink)) {
-        let idx = aIndex;
-        if (idx < 0) {
-          idx = this._indexOf(sortedLinks, existingLink);
-        } else if (this.compareLinks(aLink, sortedLinks[idx]) != 0) {
-          throw new Error("aLink should be the same as sortedLinks[idx]");
-        }
-
+        let idx = this._indexOf(sortedLinks, existingLink);
         if (idx < 0) {
           throw new Error("Link should be in _sortedLinks if in _linkMap");
         }
         sortedLinks.splice(idx, 1);
-
-        if (aDeleted) {
-          updatePages = true;
-          linkMap.delete(existingLink.url);
-          this._decrementSiteMap(siteMap, existingLink);
-        } else {
-          // Update our copy's properties.
-          for (let prop of this._sortProperties) {
-            if (prop in aLink) {
-              existingLink[prop] = aLink[prop];
-            }
+        // Update our copy's properties.
+        for (let prop of this._sortProperties) {
+          if (prop in aLink) {
+            existingLink[prop] = aLink[prop];
           }
-
-          // Finally, reinsert our copy below.
-          insertionLink = existingLink;
         }
+        // Finally, reinsert our copy below.
+        insertionLink = existingLink;
       }
       // Update our copy's title in O(1).
       if ("title" in aLink && aLink.title != existingLink.title) {
         existingLink.title = aLink.title;
         updatePages = true;
       }
     }
     else if (this._sortProperties.every(prop => prop in aLink)) {
@@ -1242,22 +1204,18 @@ this.NewTabUtils = {
       this._initialized = true;
       ExpirationFilter.init();
       Telemetry.init();
       return true;
     }
     return false;
   },
 
-  getProviderLinks: function(aProvider) {
-    return Links._providers.get(aProvider).sortedLinks;
-  },
-
   isTopSiteGivenProvider: function(aSite, aProvider) {
-    return Links._providers.get(aProvider).siteMap.has(aSite);
+    return Links._providerLinks.get(aProvider).siteMap.has(aSite);
   },
 
   isTopPlacesSite: function(aSite) {
     return this.isTopSiteGivenProvider(aSite, PlacesProvider);
   },
 
   /**
    * Restores all sites that have been removed from the grid.
@@ -1285,11 +1243,10 @@ this.NewTabUtils = {
     Links.populateCache(aCallback, true);
   },
 
   links: Links,
   allPages: AllPages,
   linkChecker: LinkChecker,
   pinnedLinks: PinnedLinks,
   blockedLinks: BlockedLinks,
-  gridPrefs: GridPrefs,
-  placesProvider: PlacesProvider
+  gridPrefs: GridPrefs
 };
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -1,93 +1,33 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // See also browser/base/content/test/newtab/.
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 Cu.import("resource://gre/modules/NewTabUtils.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
-Cu.import("resource://gre/modules/Task.jsm");
 
 function run_test() {
   run_next_test();
 }
 
-add_task(function notifyLinkDelete() {
-  let expectedLinks = makeLinks(0, 3, 1);
-
-  let provider = new TestProvider(done => done(expectedLinks));
-  provider.maxNumLinks = expectedLinks.length;
-
-  NewTabUtils.initWithoutProviders();
-  NewTabUtils.links.addProvider(provider);
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
-
-  do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
-
-  // Remove a link.
-  let removedLink = expectedLinks[2];
-  provider.notifyLinkChanged(removedLink, 2, true);
-  let links = NewTabUtils.links._providers.get(provider);
-
-  // Check that sortedLinks is correctly updated.
-  do_check_links(NewTabUtils.links.getLinks(), expectedLinks.slice(0, 2));
-
-  // Check that linkMap is accurately updated.
-  do_check_eq(links.linkMap.size, 2);
-  do_check_true(links.linkMap.get(expectedLinks[0].url));
-  do_check_true(links.linkMap.get(expectedLinks[1].url));
-  do_check_false(links.linkMap.get(removedLink.url));
-
-  // Check that siteMap is correctly updated.
-  do_check_eq(links.siteMap.size, 2);
-  do_check_true(links.siteMap.has(NewTabUtils.extractSite(expectedLinks[0].url)));
-  do_check_true(links.siteMap.has(NewTabUtils.extractSite(expectedLinks[1].url)));
-  do_check_false(links.siteMap.has(NewTabUtils.extractSite(removedLink.url)));
-
-  NewTabUtils.links.removeProvider(provider);
-});
-
-add_task(function populatePromise() {
-  let count = 0;
-  let expectedLinks = makeLinks(0, 10, 2);
-
-  let getLinksFcn = Task.async(function* (callback) {
-    //Should not be calling getLinksFcn twice
-    count++;
-    do_check_eq(count, 1);
-    yield Promise.resolve();
-    callback(expectedLinks);
-  });
-
-  let provider = new TestProvider(getLinksFcn);
-
-  NewTabUtils.initWithoutProviders();
-  NewTabUtils.links.addProvider(provider);
-
-  NewTabUtils.links.populateProviderCache(provider, () => {});
-  NewTabUtils.links.populateProviderCache(provider, () => {
-    do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
-    NewTabUtils.links.removeProvider(provider);
-  });
-});
-
 add_task(function isTopSiteGivenProvider() {
   let expectedLinks = makeLinks(0, 10, 2);
 
   // The lowest 2 frecencies have the same base domain.
   expectedLinks[expectedLinks.length - 2].url = expectedLinks[expectedLinks.length - 1].url + "Test";
 
   let provider = new TestProvider(done => done(expectedLinks));
   provider.maxNumLinks = expectedLinks.length;
 
   NewTabUtils.initWithoutProviders();
   NewTabUtils.links.addProvider(provider);
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
+  NewTabUtils.links.populateCache(function () {}, false);
 
   do_check_eq(NewTabUtils.isTopSiteGivenProvider("example2.com", provider), true);
   do_check_eq(NewTabUtils.isTopSiteGivenProvider("example1.com", provider), false);
 
   // Push out frecency 2 because the maxNumLinks is reached when adding frecency 3
   let newLink = makeLink(3);
   provider.notifyLinkChanged(newLink);
 
@@ -117,17 +57,18 @@ add_task(function multipleProviders() {
   let evenProvider = new TestProvider(done => done(evenLinks));
   let oddLinks = makeLinks(0, 2 * NewTabUtils.links.maxNumLinks - 1, 2);
   let oddProvider = new TestProvider(done => done(oddLinks));
 
   NewTabUtils.initWithoutProviders();
   NewTabUtils.links.addProvider(evenProvider);
   NewTabUtils.links.addProvider(oddProvider);
 
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
+  // This is sync since the providers' getLinks are sync.
+  NewTabUtils.links.populateCache(function () {}, false);
 
   let links = NewTabUtils.links.getLinks();
   let expectedLinks = makeLinks(NewTabUtils.links.maxNumLinks,
                                 2 * NewTabUtils.links.maxNumLinks,
                                 1);
   do_check_eq(links.length, NewTabUtils.links.maxNumLinks);
   do_check_links(links, expectedLinks);
 
@@ -137,17 +78,18 @@ add_task(function multipleProviders() {
 
 add_task(function changeLinks() {
   let expectedLinks = makeLinks(0, 20, 2);
   let provider = new TestProvider(done => done(expectedLinks));
 
   NewTabUtils.initWithoutProviders();
   NewTabUtils.links.addProvider(provider);
 
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
+  // This is sync since the provider's getLinks is sync.
+  NewTabUtils.links.populateCache(function () {}, false);
 
   do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
 
   // Notify of a new link.
   let newLink = makeLink(19);
   expectedLinks.splice(1, 0, newLink);
   provider.notifyLinkChanged(newLink);
   do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
@@ -177,58 +119,56 @@ add_task(function changeLinks() {
   expectedLinks.pop();
   do_check_eq(expectedLinks.length, provider.maxNumLinks); // Sanity check.
   provider.notifyLinkChanged(newLink);
   do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
 
   // Notify of many links changed.
   expectedLinks = makeLinks(0, 3, 1);
   provider.notifyManyLinksChanged();
-
-  // Since _populateProviderCache() is async, we must wait until the provider's
-  // populate promise has been resolved.
-  yield NewTabUtils.links._providers.get(provider).populatePromise;
-
-  // NewTabUtils.links will now repopulate its cache
+  // NewTabUtils.links will now repopulate its cache, which is sync since
+  // the provider's getLinks is sync.
   do_check_links(NewTabUtils.links.getLinks(), expectedLinks);
 
   NewTabUtils.links.removeProvider(provider);
 });
 
 add_task(function oneProviderAlreadyCached() {
   let links1 = makeLinks(0, 10, 1);
   let provider1 = new TestProvider(done => done(links1));
 
   NewTabUtils.initWithoutProviders();
   NewTabUtils.links.addProvider(provider1);
 
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
+  // This is sync since the provider's getLinks is sync.
+  NewTabUtils.links.populateCache(function () {}, false);
   do_check_links(NewTabUtils.links.getLinks(), links1);
 
   let links2 = makeLinks(10, 20, 1);
   let provider2 = new TestProvider(done => done(links2));
   NewTabUtils.links.addProvider(provider2);
 
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
+  NewTabUtils.links.populateCache(function () {}, false);
   do_check_links(NewTabUtils.links.getLinks(), links2.concat(links1));
 
   NewTabUtils.links.removeProvider(provider1);
   NewTabUtils.links.removeProvider(provider2);
 });
 
 add_task(function newLowRankedLink() {
   // Init a provider with 10 links and make its maximum number also 10.
   let links = makeLinks(0, 10, 1);
   let provider = new TestProvider(done => done(links));
   provider.maxNumLinks = links.length;
 
   NewTabUtils.initWithoutProviders();
   NewTabUtils.links.addProvider(provider);
 
-  yield new Promise(resolve => NewTabUtils.links.populateCache(resolve));
+  // This is sync since the provider's getLinks is sync.
+  NewTabUtils.links.populateCache(function () {}, false);
   do_check_links(NewTabUtils.links.getLinks(), links);
 
   // Notify of a new link that's low-ranked enough not to make the list.
   let newLink = makeLink(0);
   provider.notifyLinkChanged(newLink);
   do_check_links(NewTabUtils.links.getLinks(), links);
 
   // Notify about the new link's title change.
@@ -297,29 +237,26 @@ function TestProvider(getLinksFn) {
   this.getLinks = getLinksFn;
   this._observers = new Set();
 }
 
 TestProvider.prototype = {
   addObserver: function (observer) {
     this._observers.add(observer);
   },
-  notifyLinkChanged: function (link, index=-1, deleted=false) {
-    this._notifyObservers("onLinkChanged", link, index, deleted);
+  notifyLinkChanged: function (link) {
+    this._notifyObservers("onLinkChanged", link);
   },
   notifyManyLinksChanged: function () {
     this._notifyObservers("onManyLinksChanged");
   },
-  _notifyObservers: function () {
-    let observerMethodName = arguments[0];
-    let args = Array.prototype.slice.call(arguments, 1);
-    args.unshift(this);
+  _notifyObservers: function (observerMethodName, arg) {
     for (let obs of this._observers) {
       if (obs[observerMethodName])
-        obs[observerMethodName].apply(NewTabUtils.links, args);
+        obs[observerMethodName](this, arg);
     }
   },
 };
 
 function do_check_links(actualLinks, expectedLinks) {
   do_check_true(Array.isArray(actualLinks));
   do_check_eq(actualLinks.length, expectedLinks.length);
   for (let i = 0; i < expectedLinks.length; i++) {