Bug 1445836 - finalize topSites api on platform APIs that will be stable, r?Mardak, rpl
MozReview-Commit-ID: Esj5sJweJ4K
--- a/toolkit/components/extensions/parent/ext-topSites.js
+++ b/toolkit/components/extensions/parent/ext-topSites.js
@@ -4,34 +4,27 @@
ChromeUtils.defineModuleGetter(this, "NewTabUtils",
"resource://gre/modules/NewTabUtils.jsm");
this.topSites = class extends ExtensionAPI {
getAPI(context) {
return {
topSites: {
- get: function(options) {
- return new Promise((resolve) => {
- NewTabUtils.links.populateCache(async () => {
- let urls;
-
- // The placesProvider is a superset of activityStream if sites are blocked, etc.,
- // there is no need to attempt a merge of multiple provider lists.
- if (options.providers.includes("places")) {
- urls = NewTabUtils.getProviderLinks(NewTabUtils.placesProvider).slice();
- } else {
- urls = await NewTabUtils.activityStreamLinks.getTopSites();
- }
- resolve(urls.filter(link => !!link)
- .map(link => {
- return {
- url: link.url,
- title: link.title,
- };
- }));
- }, false);
+ get: async function(options) {
+ let links = await NewTabUtils.activityStreamLinks.getTopSites({
+ ignoreBlocked: options.includeBlocked,
+ onePerDomain: options.onePerDomain,
+ numItems: options.limit,
+ includeFavicon: options.includeFavicon,
+ });
+ return links.map(link => {
+ return {
+ url: link.url,
+ title: link.title,
+ favicon: link.favicon,
+ };
});
},
},
};
}
};
--- a/toolkit/components/extensions/schemas/top_sites.json
+++ b/toolkit/components/extensions/schemas/top_sites.json
@@ -30,16 +30,21 @@
"url": {
"type": "string",
"description": "The most visited URL."
},
"title": {
"type": "string",
"optional": true,
"description": "The title of the page."
+ },
+ "favicon": {
+ "type": "string",
+ "optional": true,
+ "description": "Data URL for the favicon, if available."
}
}
}
],
"functions": [
{
"name": "get",
"type": "function",
@@ -48,19 +53,45 @@
"parameters": [
{
"type": "object",
"name": "options",
"properties": {
"providers": {
"type": "array",
"items": { "type": "string" },
- "description": "Which providers to get top sites from. Possible values are \"places\" and \"activityStream\".",
+ "deprecated": "Please use the other options to tune the results received from topSites.",
"default": [],
"optional": true
+ },
+ "limit": {
+ "type": "integer",
+ "default": 12,
+ "maximum": 100,
+ "minimum": 1,
+ "optional": true,
+ "description": "The number of top sites to return, defaults to the value used by Firefox"
+ },
+ "onePerDomain": {
+ "type": "boolean",
+ "default": true,
+ "optional": true,
+ "description": "Limit the result to a single top site link per domain"
+ },
+ "includeBlocked": {
+ "type": "boolean",
+ "default": false,
+ "optional": true,
+ "description": "Include sites that the user has blocked from appearing on the Firefox new tab."
+ },
+ "includeFavicon": {
+ "type": "boolean",
+ "default": false,
+ "optional": true,
+ "description": "Include sites favicon if available."
}
},
"default": {},
"optional": true
},
{
"name": "callback",
"type": "function",
--- a/toolkit/components/extensions/test/xpcshell/test_ext_topSites.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_topSites.js
@@ -1,72 +1,104 @@
"use strict";
ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
ChromeUtils.import("resource://gre/modules/NewTabUtils.jsm");
+ChromeUtils.import("resource://testing-common/PlacesTestUtils.jsm");
+
+// A small 1x1 test png
+const IMAGE_1x1 = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==";
add_task(async function test_topSites() {
let visits = [];
const numVisits = 15; // To make sure we get frecency.
let visitDate = new Date(1999, 9, 9, 9, 9).getTime();
- // Stick a couple sites into history.
- for (let i = 0; i < 2; ++i) {
- let visit = {
- url: `http://example.com${i}/`,
- title: `visit${i}`,
- visits: [],
- };
+ function setVisit(visit) {
for (let j = 0; j < numVisits; ++j) {
visitDate -= 1000;
visit.visits.push({date: new Date(visitDate)});
}
visits.push(visit);
}
+ // Stick a couple sites into history.
+ for (let i = 0; i < 2; ++i) {
+ setVisit({
+ url: `http://example${i}.com/`,
+ title: `visit${i}`,
+ visits: [],
+ });
+ setVisit({
+ url: `http://www.example${i}.com/foobar`,
+ title: `visit${i}-www`,
+ visits: [],
+ });
+ }
NewTabUtils.init();
-
await PlacesUtils.history.insertMany(visits);
+ // Insert a favicon to show that favicons are not returned by default.
+ let faviconData = new Map();
+ faviconData.set("http://example0.com", IMAGE_1x1);
+ await PlacesTestUtils.addFavicons(faviconData);
+
// Ensure our links show up in activityStream.
- let links = await NewTabUtils.activityStreamLinks.getTopSites();
+ let links = await NewTabUtils.activityStreamLinks.getTopSites({onePerDomain: false, topsiteFrecency: 1});
+
equal(links.length, visits.length, "Top sites has been successfully initialized");
// Drop the visits.visits for later testing.
- visits = visits.map(v => { return {url: v.url, title: v.title}; });
+ visits = visits.map(v => { return {url: v.url, title: v.title, favicon: undefined}; });
// Test that results from all providers are returned by default.
let extension = ExtensionTestUtils.loadExtension({
manifest: {
"permissions": [
"topSites",
],
},
background() {
- // Tests consistent behaviour when no providers are specified.
- browser.test.onMessage.addListener(async providers => {
+ browser.test.onMessage.addListener(async options => {
let sites;
- if (typeof providers !== undefined) {
- sites = await browser.topSites.get(providers);
+ if (typeof options !== undefined) {
+ sites = await browser.topSites.get(options);
} else {
sites = await browser.topSites.get();
}
browser.test.sendMessage("sites", sites);
});
},
});
await extension.startup();
- function getSites(providers) {
- extension.sendMessage(providers);
+ function getSites(options) {
+ extension.sendMessage(options);
return extension.awaitMessage("sites");
}
- Assert.deepEqual(visits, await getSites(), "got topSites");
- Assert.deepEqual(visits, await getSites({}), "got topSites");
- Assert.deepEqual(visits, await getSites({providers: ["places"]}), "got topSites");
- Assert.deepEqual(visits, await getSites({providers: ["activityStream"]}), "got topSites");
- Assert.deepEqual(visits, await getSites({providers: ["places", "activityStream"]}), "got topSites");
+ Assert.deepEqual([visits[0], visits[2]],
+ await getSites(),
+ "got topSites default");
+ Assert.deepEqual(visits,
+ await getSites({onePerDomain: false}),
+ "got topSites all links");
+
+ NewTabUtils.activityStreamLinks.blockURL(visits[0]);
+ ok(NewTabUtils.blockedLinks.isBlocked(visits[0]), `link ${visits[0].url} is blocked`);
+
+ Assert.deepEqual([visits[2], visits[1]],
+ await getSites(),
+ "got topSites with blocked links filtered out");
+ Assert.deepEqual([visits[0], visits[2]],
+ await getSites({includeBlocked: true}),
+ "got topSites with blocked links included");
+
+ // Test favicon result
+ let topSites = await getSites({includeBlocked: true, includeFavicon: true});
+ equal(topSites[0].favicon, IMAGE_1x1, "received favicon");
+
+ equal(1, (await getSites({limit: 1, includeBlocked: true})).length, "limit 1 topSite");
NewTabUtils.uninit();
await extension.unload();
await PlacesUtils.history.clear();
});
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -1084,57 +1084,65 @@ var ActivityStreamProvider = {
/*
* Gets the top frecent sites for Activity Stream.
*
* @param {Object} aOptions
* {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.
+ * {bool} onePerDomain: Dedupe the resulting list.
+ * {bool} includeFavicon: Include favicons if available.
*
* @returns {Promise} Returns a promise with the array of links as payload.
*/
async getTopFrecentSites(aOptions) {
const options = Object.assign({
ignoreBlocked: false,
numItems: ACTIVITY_STREAM_DEFAULT_LIMIT,
- topsiteFrecency: ACTIVITY_STREAM_DEFAULT_FRECENCY
+ topsiteFrecency: ACTIVITY_STREAM_DEFAULT_FRECENCY,
+ onePerDomain: true,
+ includeFavicon: true,
}, aOptions || {});
// 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;
+ if (options.onePerDomain) {
+ 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
${this._commonBookmarkGuidSelect},
frecency,
guid,
last_visit_date / 1000 AS lastVisitDate,
rev_host,
title,
- url
+ url,
+ "history" as type
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"
+ "url",
+ "type"
],
params: this._getCommonParams(options, {
frecencyThreshold: options.topsiteFrecency
})
});
// Determine if the other link is "better" (larger frecency, more recent,
// lexicographically earlier url)
@@ -1152,42 +1160,47 @@ var ActivityStreamProvider = {
if (isOtherBetter(link, other)) {
link = other;
}
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 (!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(/:\/\/([^\/]+)/));
+ // Remove any blocked links.
+ if (!options.ignoreBlocked) {
+ links = links.filter(link => !BlockedLinks.isBlocked(link));
}
- // Clean up exact hosts to dedupe as non-www hosts
- const hosts = new Map();
- for (const link of exactHosts.values()) {
- setBetterLink(hosts, link, url => url.match(/:\/\/(?:www\.)?([^\/]+)/),
- // Combine frecencies when deduping these links
- (targetLink, otherLink) => {
- targetLink.frecency = link.frecency + otherLink.frecency;
- });
+ if (options.onePerDomain) {
+ // De-dup the links.
+ const exactHosts = new Map();
+ for (const link of links) {
+ // First we want to find the best link for an exact host
+ setBetterLink(exactHosts, link, url => url.match(/:\/\/([^\/]+)/));
+ }
+
+ // Clean up exact hosts to dedupe as non-www hosts
+ const hosts = new Map();
+ for (const link of exactHosts.values()) {
+ setBetterLink(hosts, link, url => url.match(/:\/\/(?:www\.)?([^\/]+)/),
+ // Combine frecencies when deduping these links
+ (targetLink, otherLink) => {
+ targetLink.frecency = link.frecency + otherLink.frecency;
+ });
+ }
+
+ links = [...hosts.values()];
}
+ // Pick out the top links using the same comparer as before
+ links = links.sort(isOtherBetter).slice(0, origNumItems);
- // Pick out the top links using the same comparer as before
- links = [...hosts.values()].sort(isOtherBetter).slice(0, origNumItems);
-
+ if (!options.includeFavicon) {
+ return 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 some info about it
*
* @param {Obj} aInfo
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -674,16 +674,41 @@ add_task(async function getTopFrecentSit
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");
});
+add_task(async function getTopFrecentSites_no_dedup() {
+ await setUpActivityStreamTest();
+
+ let provider = NewTabUtils.activityStreamLinks;
+ let links = await provider.getTopSites({topsiteFrecency: 100});
+ Assert.equal(links.length, 0, "empty history yields empty links");
+
+ // Add a visits in reverse order they will be returned in when not deduped.
+ let testURIs = [{uri: "http://www.mozilla.com/"}, {uri: "http://mozilla.com/"}];
+ await PlacesTestUtils.addVisits(testURIs);
+
+ 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");
+ // Plain domain is returned when deduped.
+ Assert.equal(links[0].url, testURIs[1].uri, "added visit corresponds to added url");
+
+ links = await provider.getTopSites({topsiteFrecency: 100, onePerDomain: false});
+ Assert.equal(links.length, 2, "adding a visit yields a link");
+ Assert.equal(links[0].url, testURIs[1].uri, "added visit corresponds to added url");
+ Assert.equal(links[1].url, testURIs[0].uri, "added visit corresponds to added url");
+});
+
add_task(async function getTopFrecentSites_dedupeWWW() {
await setUpActivityStreamTest();
let provider = NewTabUtils.activityStreamLinks;
let links = await provider.getTopSites({topsiteFrecency: 100});
Assert.equal(links.length, 0, "empty history yields empty links");