Bug 1389125 - Refactor ActivityStreamProvider in preparation for Highlights. r=ursula draft
authorEd Lee <edilee@mozilla.com>
Sat, 02 Sep 2017 13:13:52 -0700
changeset 660214 f7ee0500c8b2fab1e3fcd704556311d930a0cffd
parent 659873 c959327c6b75cd4930a6ea087583c38b805e7524
child 660215 bb80258ea64ff8ee9d9a0c30b5f4cfc86bd05333
push id78329
push userbmo:edilee@mozilla.com
push dateWed, 06 Sep 2017 18:51:30 +0000
reviewersursula
bugs1389125
milestone57.0a1
Bug 1389125 - Refactor ActivityStreamProvider in preparation for Highlights. r=ursula Add helpers for shared adjusting limit, bookmarkGuid sub-SELECT, WHERE and params. More efficiently select https and correctly select bookmarks. Remove _addETLD, getHistorySize and getBookmarksSize. Allow for activity stream caller to customize more options. MozReview-Commit-ID: Lj9AhoFJar
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -48,21 +48,21 @@ const PREF_NEWTAB_COLUMNS = "browser.new
 const HISTORY_RESULTS_LIMIT = 100;
 
 // The maximum number of links Links.getLinks will return.
 const LINKS_GET_LINKS_LIMIT = 100;
 
 // The gather telemetry topic.
 const TOPIC_GATHER_TELEMETRY = "gather-telemetry";
 
-// The number of top sites to display on Activity Stream page
-const TOP_SITES_LENGTH = 6;
+// Some default frecency threshold for Activity Stream requests
+const ACTIVITY_STREAM_DEFAULT_FRECENCY = 150;
 
-// Use double the number to allow for immediate display when blocking sites
-const TOP_SITES_LIMIT = TOP_SITES_LENGTH * 2;
+// Some default query limit for Activity Stream requests
+const ACTIVITY_STREAM_DEFAULT_LIMIT = 12;
 
 /**
  * Calculate the MD5 hash for a string.
  * @param aValue
  *        The string to convert.
  * @return The base64 representation of the MD5 hash.
  */
 function toHash(aValue) {
@@ -819,29 +819,66 @@ var PlacesProvider = {
                                          Ci.nsISupportsWeakReference]),
 };
 
 /**
  * Queries history to retrieve the most frecent sites. Emits events when the
  * history changes.
  */
 var ActivityStreamProvider = {
+  /**
+   * Shared adjustment for selecting potentially blocked links.
+   */
+  _adjustLimitForBlocked({ignoreBlocked, numItems}) {
+    // Just use the usual number if blocked links won't be filtered out
+    if (ignoreBlocked) {
+      return numItems;
+    }
+    // Additionally select the number of blocked links in case they're removed
+    return Object.keys(BlockedLinks.links).length + numItems;
+  },
 
   /**
-   * Process links after getting them from the database.
-   *
-   * @param {Array} aLinks
-   *          an array containing link objects
-   *
-   * @returns {Array} an array of checked links with favicons and eTLDs added
+   * Shared sub-SELECT to get the guid of a bookmark of the current url while
+   * avoiding LEFT JOINs on moz_bookmarks. This avoids gettings tags. The guid
+   * could be one of multiple possible guids. Assumes `moz_places h` is in FROM.
    */
-  _processLinks(aLinks) {
-    let links_ = aLinks.filter(link => LinkChecker.checkLoadURI(link.url));
-    links_ = this._faviconBytesToDataURI(links_);
-    return this._addETLD(links_);
+  _commonBookmarkGuidSelect: `(
+    SELECT guid
+    FROM moz_bookmarks b
+    WHERE fk = h.id
+      AND type = :bookmarkType
+      AND (
+        SELECT id
+        FROM moz_bookmarks p
+        WHERE p.id = b.parent
+          AND p.parent <> :tagsFolderId
+      ) NOTNULL
+    ) AS bookmarkGuid`,
+
+  /**
+   * Shared WHERE expression filtering out undesired pages, e.g., hidden,
+   * unvisited, and non-http/s urls. Assumes moz_places is in FROM / JOIN.
+   */
+  _commonPlacesWhere: `
+    AND hidden = 0
+    AND last_visit_date > 0
+    AND (SUBSTR(url, 1, 6) == "https:"
+      OR SUBSTR(url, 1, 5) == "http:")
+  `,
+
+  /**
+   * Shared parameters for getting correct bookmarks and LIMITed queries.
+   */
+  _getCommonParams(aOptions, aParams = {}) {
+    return Object.assign({
+      bookmarkType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+      limit: this._adjustLimitForBlocked(aOptions),
+      tagsFolderId: PlacesUtils.tagsFolderId
+    }, aParams);
   },
 
   /**
    * From an Array of links, if favicons are present, convert to data URIs
    *
    * @param {Array} aLinks
    *          an array containing objects with favicon data and mimeTypes
    *
@@ -900,78 +937,68 @@ var ActivityStreamProvider = {
           link.mimeType = null;
           return link;
         })
       ));
     }
     return aLinks;
   },
 
-  /**
-   * Add the eTLD to each link in the array of links.
-   *
-   * @param {Array} aLinks
-   *          an array containing objects with urls
-   *
-   * @returns {Array} an array of links with eTLDs added
-   */
-  _addETLD(aLinks) {
-    return aLinks.map(link => {
-      try {
-        link.eTLD = Services.eTLD.getPublicSuffix(Services.io.newURI(link.url));
-      } catch (e) {
-        link.eTLD = "";
-      }
-      return link;
-    });
-  },
-
   /*
    * Gets the top frecent sites for Activity Stream.
    *
    * @param {Object} aOptions
-   *          options.ignoreBlocked: Do not filter out blocked links .
+   *   {bool} ignoreBlocked: Do not filter out blocked links.
+   *   {int}  numItems: Maximum number of items to return.
+   *   {int}  topsiteFrecency: Minimum amount of frecency for a site.
    *
    * @returns {Promise} Returns a promise with the array of links as payload.
    */
-  async getTopFrecentSites(aOptions = {}) {
-    let {ignoreBlocked} = aOptions;
+  async getTopFrecentSites(aOptions) {
+    const options = Object.assign({
+      ignoreBlocked: false,
+      numItems: ACTIVITY_STREAM_DEFAULT_LIMIT,
+      topsiteFrecency: ACTIVITY_STREAM_DEFAULT_FRECENCY
+    }, aOptions || {});
 
-    // Get extra links in case they're blocked and double the usual limit in
-    // case the host is deduped between with www or not-www (i.e., 2 hosts) as
-    // well as an extra buffer for multiple pages from the same exact host.
-    const limit = Object.keys(BlockedLinks.links).length + TOP_SITES_LIMIT * 2 * 10;
+    // Double the item count in case the host is deduped between with www or
+    // not-www (i.e., 2 hosts) and an extra buffer for multiple pages per host.
+    const origNumItems = options.numItems;
+    options.numItems *= 2 * 10;
 
     // Keep this query fast with frecency-indexed lookups (even with excess
     // rows) and shift the more complex logic to post-processing afterwards
-    const sqlQuery = `SELECT
-                        moz_bookmarks.guid AS bookmarkGuid,
-                        frecency,
-                        moz_places.guid,
-                        last_visit_date / 1000 AS lastVisitDate,
-                        rev_host,
-                        moz_places.title,
-                        url
-                      FROM moz_places
-                      LEFT JOIN moz_bookmarks
-                      ON moz_places.id = moz_bookmarks.fk
-                      WHERE hidden = 0 AND last_visit_date NOTNULL
-                      AND (SUBSTR(moz_places.url, 1, 6) == "https:" OR SUBSTR(moz_places.url, 1, 5) == "http:")
-                      ORDER BY frecency DESC
-                      LIMIT ${limit}`;
+    const sqlQuery = `
+      SELECT
+        ${this._commonBookmarkGuidSelect},
+        frecency,
+        guid,
+        last_visit_date / 1000 AS lastVisitDate,
+        rev_host,
+        title,
+        url
+      FROM moz_places h
+      WHERE frecency >= :frecencyThreshold
+        ${this._commonPlacesWhere}
+      ORDER BY frecency DESC
+      LIMIT :limit
+    `;
 
     let links = await this.executePlacesQuery(sqlQuery, {
       columns: [
         "bookmarkGuid",
         "frecency",
         "guid",
         "lastVisitDate",
         "title",
         "url"
-      ]
+      ],
+      params: this._getCommonParams(options, {
+        frecencyThreshold: options.topsiteFrecency
+      })
     });
 
     // Determine if the other link is "better" (larger frecency, more recent,
     // lexicographically earlier url)
     function isOtherBetter(link, other) {
       return other.frecency > link.frecency ||
         (other.frecency === link.frecency && other.lastVisitDate > link.lastVisitDate ||
          other.lastVisitDate === link.lastVisitDate && other.url < link.url);
@@ -988,17 +1015,17 @@ var ActivityStreamProvider = {
         combiner(link, other);
       }
       map.set(host, link);
     }
 
     // Clean up the returned links by removing blocked, deduping, etc.
     const exactHosts = new Map();
     for (const link of links) {
-      if (!ignoreBlocked && BlockedLinks.isBlocked(link)) {
+      if (!options.ignoreBlocked && BlockedLinks.isBlocked(link)) {
         continue;
       }
 
       link.type = "history";
 
       // First we want to find the best link for an exact host
       setBetterLink(exactHosts, link, url => url.match(/:\/\/([^\/]+)/));
     }
@@ -1009,20 +1036,20 @@ var ActivityStreamProvider = {
       setBetterLink(hosts, link, url => url.match(/:\/\/(?:www\.)?([^\/]+)/),
         // Combine frecencies when deduping these links
         (targetLink, otherLink) => {
           targetLink.frecency = link.frecency + otherLink.frecency;
         });
     }
 
     // Pick out the top links using the same comparer as before
-    links = [...hosts.values()].sort(isOtherBetter).slice(0, TOP_SITES_LIMIT);
+    links = [...hosts.values()].sort(isOtherBetter).slice(0, origNumItems);
 
-    links = await this._addFavicons(links);
-    return this._processLinks(links);
+    // Get the favicons as data URI for now (until we use the favicon protocol)
+    return this._faviconBytesToDataURI(await this._addFavicons(links));
   },
 
   /**
    * Gets a specific bookmark given an id
    *
    * @param {String} aGuid
    *          A bookmark guid to use as a refrence to fetch the bookmark
    */
@@ -1035,41 +1062,16 @@ var ActivityStreamProvider = {
     result.bookmarkGuid = bookmark.guid;
     result.bookmarkTitle = bookmark.title;
     result.lastModified = bookmark.lastModified.getTime();
     result.url = bookmark.url.href;
     return result;
   },
 
   /**
-   * Gets History size
-   *
-   * @returns {Promise} Returns a promise with the count of moz_places records
-   */
-  async getHistorySize() {
-    let sqlQuery = `SELECT count(*) FROM moz_places
-                    WHERE hidden = 0 AND last_visit_date NOT NULL`;
-
-    let result = await this.executePlacesQuery(sqlQuery);
-    return result;
-  },
-
-  /**
-   * Gets Bookmarks count
-   *
-   * @returns {Promise} Returns a promise with the count of bookmarks
-   */
-  async getBookmarksSize() {
-    let sqlQuery = `SELECT count(*) FROM moz_bookmarks WHERE type = :type`;
-
-    let result = await this.executePlacesQuery(sqlQuery, {params: {type: PlacesUtils.bookmarks.TYPE_BOOKMARK}});
-    return result;
-  },
-
-  /**
    * Executes arbitrary query against places database
    *
    * @param {String} aQuery
    *        SQL query to execute
    * @param {Object} [optional] aOptions
    *          aOptions.columns - an array of column names. if supplied the return
    *          items will consists of objects keyed on column names. Otherwise
    *          array of raw values is returned in the select order
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -1,13 +1,24 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // See also browser/base/content/test/newtab/.
 
+function getBookmarksSize() {
+  return NewTabUtils.activityStreamProvider.executePlacesQuery(
+    "SELECT count(*) FROM moz_bookmarks WHERE type = :type",
+    {params: {type: PlacesUtils.bookmarks.TYPE_BOOKMARK}});
+}
+
+function getHistorySize() {
+  return NewTabUtils.activityStreamProvider.executePlacesQuery(
+    "SELECT count(*) FROM moz_places WHERE hidden = 0 AND last_visit_date NOT NULL");
+}
+
 add_task(async function validCacheMidPopulation() {
   let expectedLinks = makeLinks(0, 3, 1);
 
   let provider = new TestProvider(done => done(expectedLinks));
   provider.maxNumLinks = expectedLinks.length;
 
   NewTabUtils.initWithoutProviders();
   NewTabUtils.links.addProvider(provider);
@@ -352,70 +363,72 @@ add_task(async function addFavicons() {
   Assert.equal(links[0].faviconLength, links[0].favicon.length, "Got the right length for the byte array");
   Assert.equal(provider._faviconBytesToDataURI(links)[0].favicon, base64URL, "Got the right favicon");
 });
 
 add_task(async function getTopFrecentSites() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
-  let links = await provider.getTopSites();
+  let links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 0, "empty history yields empty links");
 
   // add a visit
   let testURI = "http://mozilla.com/";
   await PlacesTestUtils.addVisits(testURI);
 
   links = await provider.getTopSites();
+  Assert.equal(links.length, 0, "adding a single visit doesn't exceed default threshold");
+
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "adding a visit yields a link");
   Assert.equal(links[0].url, testURI, "added visit corresponds to added url");
-  Assert.equal(links[0].eTLD, "com", "added visit mozilla.com has 'com' eTLD");
 });
 
 add_task(async function getTopFrecentSites_dedupeWWW() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
 
-  let links = await provider.getTopSites();
+  let links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 0, "empty history yields empty links");
 
   // add a visit without www
   let testURI = "http://mozilla.com";
   await PlacesTestUtils.addVisits(testURI);
 
   // add a visit with www
   testURI = "http://www.mozilla.com";
   await PlacesTestUtils.addVisits(testURI);
 
   // Test combined frecency score
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
   Assert.equal(links[0].frecency, 200, "frecency scores are combined");
 
   // add another page visit with www and without www
   let noWWW = "http://mozilla.com/page";
   await PlacesTestUtils.addVisits(noWWW);
   let withWWW = "http://www.mozilla.com/page";
   await PlacesTestUtils.addVisits(withWWW);
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
   Assert.equal(links[0].frecency, 200, "frecency scores are combined ignoring extra pages");
 
   // add another visit with www
   await PlacesTestUtils.addVisits(withWWW);
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "still yields one link");
   Assert.equal(links[0].url, withWWW, "more frecent www link is used");
   Assert.equal(links[0].frecency, 300, "frecency scores are combined ignoring extra pages");
 
   // add a couple more visits to the no-www page
   await PlacesTestUtils.addVisits(noWWW);
   await PlacesTestUtils.addVisits(noWWW);
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "still yields one link");
   Assert.equal(links[0].url, noWWW, "now more frecent no-www link is used");
   Assert.equal(links[0].frecency, 500, "frecency scores are combined ignoring extra pages");
 });
 
 add_task(async function getTopFrencentSites_maxLimit() {
   await setUpActivityStreamTest();
 
@@ -423,52 +436,52 @@ add_task(async function getTopFrencentSi
 
   // add many visits
   const MANY_LINKS = 20;
   for (let i = 0; i < MANY_LINKS; i++) {
     let testURI = `http://mozilla${i}.com`;
     await PlacesTestUtils.addVisits(testURI);
   }
 
-  let links = await provider.getTopSites();
+  let links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.ok(links.length < MANY_LINKS, "query default limited to less than many");
   Assert.ok(links.length > 6, "query default to more than visible count");
 });
 
 add_task(async function getTopFrencentSites_allowedProtocols() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
 
   // add a visit from a file:// site
   let testURI = "file:///some/file/path.png";
   await PlacesTestUtils.addVisits(testURI);
 
-  let links = await provider.getTopSites();
+  let links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 0, "don't get sites with the file:// protocol");
 
   // now add a site with an allowed protocol
   testURI = "http://www.mozilla.com";
   await PlacesTestUtils.addVisits(testURI);
 
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "http:// is an allowed protocol");
 
   // and just to be sure, add a visit to a site with ftp:// protocol
   testURI = "ftp://bad/example";
   await PlacesTestUtils.addVisits(testURI);
 
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 1, "we still only accept http:// and https:// for top sites");
 
   // add a different allowed protocol
   testURI = "https://https";
   await PlacesTestUtils.addVisits(testURI);
 
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 2, "we now accept both http:// and https:// for top sites");
 });
 
 add_task(async function getTopFrecentSites_order() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
   let {TRANSITION_TYPED} = PlacesUtils.history;
@@ -482,30 +495,30 @@ add_task(async function getTopFrecentSit
     // sort by url, frecency 200
     {uri: "https://mozilla2.com/1", visitDate: timeEarlier, transition: TRANSITION_TYPED},
     // sort by last visit date, frecency 200
     {uri: "https://mozilla3.com/2", visitDate: timeLater, transition: TRANSITION_TYPED},
     // sort by frecency, frecency 10
     {uri: "https://mozilla4.com/3", visitDate: timeLater}
   ];
 
-  let links = await provider.getTopSites();
+  let links = await provider.getTopSites({topsiteFrecency: 0});
   Assert.equal(links.length, 0, "empty history yields empty links");
 
   let base64URL = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAA" +
     "AAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==";
 
   // map of page url to favicon url
   let faviconData = new Map();
   faviconData.set("https://mozilla3.com/2", base64URL);
 
   await PlacesTestUtils.addVisits(visits);
   await PlacesTestUtils.addFavicons(faviconData);
 
-  links = await provider.getTopSites();
+  links = await provider.getTopSites({topsiteFrecency: 0});
   Assert.equal(links.length, visits.length, "number of links added is the same as obtain by getTopFrecentSites");
 
   // first link doesn't have a favicon
   Assert.equal(links[0].url, visits[0].uri, "links are obtained in the expected order");
   Assert.equal(null, links[0].favicon, "favicon data is stored as expected");
   Assert.ok(isVisitDateOK(links[0].lastVisitDate), "visit date within expected range");
 
   // second link doesn't have a favicon
@@ -533,73 +546,73 @@ add_task(async function activitySteamPro
 
   let visits = [
     // frecency 200
     {uri: "https://mozilla1.com/0", visitDate: timeDaysAgo(1), transition: TRANSITION_TYPED},
     // sort by url, frecency 200
     {uri: "https://mozilla2.com/1", visitDate: timeDaysAgo(0)}
   ];
 
-  let size = await NewTabUtils.activityStreamProvider.getHistorySize();
+  let size = await getHistorySize();
   Assert.equal(size, 0, "empty history has size 0");
 
   await PlacesTestUtils.addVisits(visits);
 
-  size = await NewTabUtils.activityStreamProvider.getHistorySize();
+  size = await getHistorySize();
   Assert.equal(size, 2, "expected history size");
 
   // delete a link
   let deleted = await provider.deleteHistoryEntry("https://mozilla2.com/1");
   Assert.equal(deleted, true, "link is deleted");
 
   // ensure that there's only one link left
-  size = await NewTabUtils.activityStreamProvider.getHistorySize();
+  size = await getHistorySize();
   Assert.equal(size, 1, "expected history size");
 
   // pin the link and delete it
   const linkToPin = {"url": "https://mozilla1.com/0"};
   NewTabUtils.pinnedLinks.pin(linkToPin, 0);
 
   // sanity check that the correct link was pinned
   Assert.equal(NewTabUtils.pinnedLinks.links.length, 1, "added a link to pinned sites");
   Assert.equal(NewTabUtils.pinnedLinks.isPinned(linkToPin), true, "pinned the correct link");
 
   // delete the pinned link and ensure it was both deleted from history and unpinned
   deleted = await provider.deleteHistoryEntry("https://mozilla1.com/0");
-  size = await NewTabUtils.activityStreamProvider.getHistorySize();
+  size = await getHistorySize();
   Assert.equal(deleted, true, "link is deleted");
   Assert.equal(size, 0, "expected history size");
   Assert.equal(NewTabUtils.pinnedLinks.links.length, 0, "unpinned the deleted link");
 });
 
 add_task(async function activityStream_deleteBookmark() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
   let bookmarks = [
-    {url: "https://mozilla1.com/0", parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK},
-    {url: "https://mozilla1.com/1", parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK}
+    {url: "https://mozilla1.com/0", parentGuid: PlacesUtils.bookmarks.unfiledGuid},
+    {url: "https://mozilla1.com/1", parentGuid: PlacesUtils.bookmarks.unfiledGuid}
   ];
 
-  let bookmarksSize = await NewTabUtils.activityStreamProvider.getBookmarksSize();
+  let bookmarksSize = await getBookmarksSize();
   Assert.equal(bookmarksSize, 0, "empty bookmarks yields 0 size");
 
   for (let placeInfo of bookmarks) {
     await PlacesUtils.bookmarks.insert(placeInfo);
   }
 
-  bookmarksSize = await NewTabUtils.activityStreamProvider.getBookmarksSize();
+  bookmarksSize = await getBookmarksSize();
   Assert.equal(bookmarksSize, 2, "size 2 for 2 bookmarks added");
 
   let bookmarkGuid = await new Promise(resolve => PlacesUtils.bookmarks.fetch(
     {url: bookmarks[0].url}, bookmark => resolve(bookmark.guid)));
   let deleted = await provider.deleteBookmark(bookmarkGuid);
   Assert.equal(deleted.guid, bookmarkGuid, "the correct bookmark was deleted");
 
-  bookmarksSize = await NewTabUtils.activityStreamProvider.getBookmarksSize();
+  bookmarksSize = await getBookmarksSize();
   Assert.equal(bookmarksSize, 1, "size 1 after deleting");
 });
 
 add_task(async function activityStream_blockedURLs() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
   NewTabUtils.blockedLinks.addObserver(provider);
@@ -611,22 +624,22 @@ add_task(async function activityStream_b
 
   let visits = [
     {uri: "https://example1.com/", visitDate: timeToday, transition: TRANSITION_TYPED},
     {uri: "https://example2.com/", visitDate: timeToday, transition: TRANSITION_TYPED},
     {uri: "https://example3.com/", visitDate: timeEarlier, transition: TRANSITION_TYPED},
     {uri: "https://example4.com/", visitDate: timeEarlier, transition: TRANSITION_TYPED}
   ];
   await PlacesTestUtils.addVisits(visits);
-  await PlacesUtils.bookmarks.insert({url: "https://example5.com/", parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK});
+  await PlacesUtils.bookmarks.insert({url: "https://example5.com/", parentGuid: PlacesUtils.bookmarks.unfiledGuid});
 
   let sizeQueryResult;
 
   // bookmarks
-  sizeQueryResult = await NewTabUtils.activityStreamProvider.getBookmarksSize();
+  sizeQueryResult = await getBookmarksSize();
   Assert.equal(sizeQueryResult, 1, "got the correct bookmark size");
 });
 
 function TestProvider(getLinksFn) {
   this.getLinks = getLinksFn;
   this._observers = new Set();
 }