--- 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();
}