зеркало из https://github.com/mozilla/gecko-dev.git
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 --HG-- extra : rebase_source : fb4bb13969b47c28c1a137075304efb23c254182
This commit is contained in:
Ed Lee
Родитель
2ec05bb44f
Коммит
85b1aa47dd
|
|
@ -53,11 +53,11 @@ 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.
|
||||
|
|
@ -824,19 +824,56 @@ var PlacesProvider = {
|
|||
* 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);
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
@ -905,58 +942,45 @@ var ActivityStreamProvider = {
|
|||
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: [
|
||||
|
|
@ -966,7 +990,10 @@ var ActivityStreamProvider = {
|
|||
"lastVisitDate",
|
||||
"title",
|
||||
"url"
|
||||
]
|
||||
],
|
||||
params: this._getCommonParams(options, {
|
||||
frecencyThreshold: options.topsiteFrecency
|
||||
})
|
||||
});
|
||||
|
||||
// Determine if the other link is "better" (larger frecency, more recent,
|
||||
|
|
@ -993,7 +1020,7 @@ var ActivityStreamProvider = {
|
|||
// 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;
|
||||
}
|
||||
|
||||
|
|
@ -1014,10 +1041,10 @@ var ActivityStreamProvider = {
|
|||
}
|
||||
|
||||
// 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));
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
@ -1039,31 +1066,6 @@ var ActivityStreamProvider = {
|
|||
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
|
||||
*
|
||||
|
|
|
|||
|
|
@ -3,6 +3,17 @@
|
|||
|
||||
// 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);
|
||||
|
||||
|
|
@ -357,7 +368,7 @@ 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
|
||||
|
|
@ -365,9 +376,11 @@ add_task(async function getTopFrecentSites() {
|
|||
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() {
|
||||
|
|
@ -375,7 +388,7 @@ add_task(async function getTopFrecentSites_dedupeWWW() {
|
|||
|
||||
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
|
||||
|
|
@ -387,7 +400,7 @@ add_task(async function getTopFrecentSites_dedupeWWW() {
|
|||
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");
|
||||
|
||||
|
|
@ -396,13 +409,13 @@ add_task(async function getTopFrecentSites_dedupeWWW() {
|
|||
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");
|
||||
|
|
@ -410,7 +423,7 @@ add_task(async function getTopFrecentSites_dedupeWWW() {
|
|||
// 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");
|
||||
|
|
@ -428,7 +441,7 @@ add_task(async function getTopFrencentSites_maxLimit() {
|
|||
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");
|
||||
});
|
||||
|
|
@ -442,28 +455,28 @@ add_task(async function getTopFrencentSites_allowedProtocols() {
|
|||
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");
|
||||
});
|
||||
|
||||
|
|
@ -487,7 +500,7 @@ add_task(async function getTopFrecentSites_order() {
|
|||
{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" +
|
||||
|
|
@ -500,7 +513,7 @@ add_task(async function getTopFrecentSites_order() {
|
|||
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
|
||||
|
|
@ -538,12 +551,12 @@ add_task(async function activitySteamProvider_deleteHistoryLink() {
|
|||
{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
|
||||
|
|
@ -551,7 +564,7 @@ add_task(async function activitySteamProvider_deleteHistoryLink() {
|
|||
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
|
||||
|
|
@ -564,7 +577,7 @@ add_task(async function activitySteamProvider_deleteHistoryLink() {
|
|||
|
||||
// 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");
|
||||
|
|
@ -575,18 +588,18 @@ add_task(async function activityStream_deleteBookmark() {
|
|||
|
||||
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(
|
||||
|
|
@ -594,7 +607,7 @@ add_task(async function activityStream_deleteBookmark() {
|
|||
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");
|
||||
});
|
||||
|
||||
|
|
@ -616,12 +629,12 @@ add_task(async function activityStream_blockedURLs() {
|
|||
{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");
|
||||
});
|
||||
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче