Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-12-01 08:42:13 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1810772 - Stop recalculating frecency immediately for bookmarks. r=daisuke

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D168779
This commit is contained in:
Marco Bonardo 2023-02-08 08:54:57 +00:00
parent bd7358d5b5
commit c0fd0ecffd
32 changed files with 265 additions and 265 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
browser/components/urlbar/tests/browser/browser_view_selectionByMouse.js
View File

@ -301,6 +301,7 @@ add_task(async function withDnsFirstForSingleWordsPref() {
url: "https://example.org/",
title: "example",
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await UrlbarTestUtils.promiseAutocompleteResultPopup({
value: "ex",

9
browser/components/urlbar/tests/browser/head.js
View File

@ -26,6 +26,13 @@ XPCOMUtils.defineLazyModuleGetters(this, {
ExperimentAPI: "resource://nimbus/ExperimentAPI.jsm",
ExperimentFakes: "resource://testing-common/NimbusTestUtils.jsm",
ObjectUtils: "resource://gre/modules/ObjectUtils.jsm",
sinon: "resource://testing-common/Sinon.jsm",
});
XPCOMUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => {
return Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
).wrappedJSObject;
});
let sandbox;
@ -35,8 +42,6 @@ Services.scriptloader.loadSubScript(
this
);
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
registerCleanupFunction(async () => {
// Ensure the Urlbar popup is always closed at the end of a test, to save having
// to do it within each test.

8
browser/components/urlbar/tests/unit/head.js
View File

@ -34,8 +34,8 @@ ChromeUtils.defineESModuleGetters(this, {
XPCOMUtils.defineLazyModuleGetters(this, {
AddonTestUtils: "resource://testing-common/AddonTestUtils.jsm",
HttpServer: "resource://testing-common/httpd.js",
sinon: "resource://testing-common/Sinon.jsm",
});
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
XPCOMUtils.defineLazyGetter(this, "QuickSuggestTestUtils", () => {
const { QuickSuggestTestUtils: module } = ChromeUtils.importESModule(
@ -53,6 +53,12 @@ XPCOMUtils.defineLazyGetter(this, "MerinoTestUtils", () => {
return module;
});
XPCOMUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => {
return Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
).wrappedJSObject;
});
SearchTestUtils.init(this);
AddonTestUtils.init(this, false);
AddonTestUtils.createAppInfo(

26
browser/components/urlbar/tests/unit/test_autofill_originsAndQueries.js
View File

@ -848,6 +848,7 @@ add_autofill_task(async function bookmarkBelowThreshold() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// Make sure the bookmarked origin and place frecencies are below the
// threshold so that the origin/URL otherwise would not be autofilled.
@ -897,6 +898,7 @@ add_autofill_task(async function bookmarkAboveThreshold() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// The frecencies of the place and origin should be >= the threshold. In
// fact they should be the same as the threshold since the place is the only
@ -935,6 +937,7 @@ add_autofill_task(async function zeroThreshold() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: pageUrl,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await PlacesUtils.history.clear();
await PlacesUtils.withConnectionWrapper("zeroThreshold", async db => {
@ -1106,6 +1109,7 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_0() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// Make the bookmark fall below the autofill frecency threshold so we ensure
// the bookmark is always autofilled in this case, even if it doesn't meet
@ -1159,6 +1163,8 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_1() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext(search, { isPrivate: false });
let matches = [
makeBookmarkResult(context, {
@ -1210,6 +1216,7 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_prefix_0() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// Make the bookmark fall below the autofill frecency threshold so we ensure
// the bookmark is always autofilled in this case, even if it doesn't meet
@ -1233,6 +1240,7 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_prefix_0() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext("http://" + search, { isPrivate: false });
await check_results({
context,
@ -1266,6 +1274,7 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_prefix_1() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
await check_results({
@ -1305,6 +1314,7 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_prefix_2() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
await check_results({
@ -1344,6 +1354,7 @@ add_autofill_task(async function suggestHistoryFalse_bookmark_prefix_3() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
await check_results({
@ -1609,6 +1620,7 @@ add_autofill_task(async function suggestBookmarkFalse_unvisitedBookmark() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext(search, { isPrivate: false });
await check_results({
context,
@ -1670,6 +1682,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let context = createContext("http://" + search, { isPrivate: false });
await check_results({
context,
@ -1720,6 +1733,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
@ -1757,6 +1771,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
@ -1794,6 +1809,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
@ -1831,6 +1847,7 @@ add_autofill_task(async function suggestBookmarkFalse_visitedBookmark_above() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext(search, { isPrivate: false });
await check_results({
@ -1866,6 +1883,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
await check_results({
@ -1902,6 +1920,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
@ -1945,6 +1964,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
@ -1988,6 +2008,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
let context = createContext("http://" + search, { isPrivate: false });
let prefixedUrl = origins ? `http://${search}/` : `http://${search}`;
@ -2067,6 +2088,7 @@ add_autofill_task(async function suggestBookmarkFalse_visitedBookmarkBelow() {
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
context = createContext(search, { isPrivate: false });
await check_results({
@ -2143,6 +2165,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
context = createContext("http://" + search, { isPrivate: false });
await check_results({
@ -2223,6 +2246,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
context = createContext("http://" + search, { isPrivate: false });
await check_results({
@ -2303,6 +2327,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "http://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
context = createContext("http://" + search, { isPrivate: false });
await check_results({
@ -2383,6 +2408,7 @@ add_autofill_task(
await PlacesTestUtils.addBookmarkWithDetails({
uri: "ftp://non-matching-" + url,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
context = createContext("http://" + search, { isPrivate: false });
await check_results({

1
browser/components/urlbar/tests/unit/test_download_embed_bookmarks.js
View File

@ -47,6 +47,7 @@ add_task(async function test_download_embed_bookmarks() {
uri: uri3,
title: "framed-bookmark",
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Searching for bookmarked download uri matches");
let context = createContext("download-bookmark", { isPrivate: false });

2
browser/components/urlbar/tests/unit/test_empty_search.js
View File

@ -44,6 +44,8 @@ add_task(async function test_empty_search() {
// Now remove page 6 from history, so it is an unvisited bookmark.
await PlacesUtils.history.remove(uri6);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// With the changes above, the sites in descending order of frecency are:
// uri2
// uri4

2
browser/components/urlbar/tests/unit/test_match_javascript.js
View File

@ -31,6 +31,8 @@ add_task(async function test_javascript_match() {
{ uri: uri1, title: "Title with javascript:" },
]);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Match non-javascript: with plain search");
let context = createContext("a", { isPrivate: false });
await check_results({

2
browser/components/urlbar/tests/unit/test_multi_word_search.js
View File

@ -39,6 +39,8 @@ add_task(async function test_match_beginning() {
title: "b(a)r b<a>z",
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Match 2 terms all in url");
let context = createContext("c d", { isPrivate: false });
await check_results({

4
browser/components/urlbar/tests/unit/test_providerPlaces.js
View File

@ -42,6 +42,7 @@ add_task(async function test_places() {
{ uri: "https://tab.mozilla.org/", title: "Test tab" },
]);
UrlbarProviderOpenTabs.registerOpenTab("https://tab.mozilla.org/", 0, false);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await controller.startQuery(context);
@ -119,6 +120,7 @@ add_task(async function test_bookmarkBehaviorDisabled_tagged() {
["mozilla", "org", "ham", "moz", "bacon"]
);
await PlacesTestUtils.addVisits("https://bookmark.mozilla.org/");
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await controller.startQuery(context);
@ -169,6 +171,7 @@ add_task(async function test_bookmarkBehaviorDisabled_untagged() {
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
});
await PlacesTestUtils.addVisits("https://bookmark.mozilla.org/");
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await controller.startQuery(context);
@ -216,6 +219,7 @@ add_task(async function test_diacritics() {
title: "Test bookmark with accents in path",
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await controller.startQuery(context);

2
browser/components/urlbar/tests/unit/test_search_engine_host.js
View File

@ -27,7 +27,7 @@ add_task(async function test_searchEngine_autoFill() {
uri,
title: "Example bookmark",
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
ok(
frecencyForUrl(uri) > 10000,
"Added URI should have expected high frecency"

1
browser/components/urlbar/tests/unit/test_search_suggestions_tail.js
View File

@ -251,6 +251,7 @@ add_task(async function mixed_results() {
url: "http://example.com/2",
title: "what time is",
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// Tail suggestions should not be shown.
const query = "what time is";

4
browser/components/urlbar/tests/unit/test_special_search.js
View File

@ -66,6 +66,8 @@ add_task(async function test_special_searches() {
await PlacesTestUtils.addBookmarkWithDetails({ uri: uri6, title: "foo.bar" });
await PlacesTestUtils.addBookmarkWithDetails({ uri: uri5, title: "title" });
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// Order of frecency when not restricting, descending:
// uri11
// uri1
@ -378,6 +380,7 @@ add_task(async function test_special_searches() {
uri: "http://bookmark.conflict.com/",
title: `conflict ${UrlbarTokenizer.RESTRICT.HISTORY}`,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
context = createContext(
`conflict ${UrlbarTokenizer.RESTRICT.HISTORY} ${UrlbarTokenizer.RESTRICT.BOOKMARK}`,
{ isPrivate: false }
@ -408,6 +411,7 @@ add_task(async function test_special_searches() {
uri: "http://nontag.conflict.com/",
title: `conflict ${UrlbarTokenizer.RESTRICT.BOOKMARK}`,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
context = createContext(
`conflict ${UrlbarTokenizer.RESTRICT.BOOKMARK} ${UrlbarTokenizer.RESTRICT.TAG}`,
{ isPrivate: false }

1
browser/components/urlbar/tests/unit/test_word_boundary_search.js
View File

@ -50,6 +50,7 @@ add_task(async function test_escape() {
title: "title1",
tags: ["dontmatchme3"],
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Match 'match' at the beginning or after / or on a CamelCase");
let context = createContext("match", { isPrivate: false });

105
toolkit/components/places/Bookmarks.sys.mjs
View File

@ -67,25 +67,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs",
});
// This is an helper to temporarily cover the need to know the tags folder
// itemId until bug 424160 is fixed. This exists so that startup paths won't
// pay the price to initialize the bookmarks service just to fetch this value.
// If the method is already initing the bookmarks service for other reasons
// (most of the writing methods will invoke getObservers() already) it can
// directly use the PlacesUtils.tagsFolderId property.
var gTagsFolderId;
async function promiseTagsFolderId() {
if (gTagsFolderId) {
return gTagsFolderId;
}
let db = await lazy.PlacesUtils.promiseDBConnection();
let rows = await db.execute(
"SELECT id FROM moz_bookmarks WHERE guid = :guid",
{ guid: Bookmarks.tagsGuid }
);
return (gTagsFolderId = rows[0].getResultByName("id"));
}
const MATCH_ANYWHERE_UNMODIFIED =
Ci.mozIPlacesAutoComplete.MATCH_ANYWHERE_UNMODIFIED;
const BEHAVIOR_BOOKMARK = Ci.mozIPlacesAutoComplete.BEHAVIOR_BOOKMARK;
@ -834,16 +815,6 @@ export var Bookmarks = Object.freeze({
return updatedItem;
});
if (
item.type == this.TYPE_BOOKMARK &&
item.url.href != updatedItem.url.href
) {
// ...though we don't wait for the calculation.
updateFrecency(db, [item.url, updatedItem.url]).catch(
console.error
);
}
const notifications = [];
// For lastModified, we only care about the original input, since we
@ -1373,7 +1344,6 @@ export var Bookmarks = Object.freeze({
"Bookmarks.jsm: eraseEverything",
async function(db) {
let urls;
await db.executeTransaction(async function() {
urls = await removeFoldersContents(
db,
@ -1400,10 +1370,8 @@ export var Bookmarks = Object.freeze({
);
});
// We don't wait for the frecency calculation.
if (urls && urls.length) {
if (urls?.length) {
await lazy.PlacesUtils.keywords.eraseEverything();
updateFrecency(db, urls).catch(console.error);
}
}
);
@ -2183,12 +2151,6 @@ function insertBookmark(item, parent) {
);
});
// If not a tag recalculate frecency...
if (item.type == Bookmarks.TYPE_BOOKMARK && !isTagging) {
// ...though we don't wait for the calculation.
updateFrecency(db, [item.url]).catch(console.error);
}
return item;
}
);
@ -2255,9 +2217,6 @@ function insertBookmarkTree(items, source, parent, urls, lastAddedForParent) {
);
});
// We don't wait for the frecency calculation.
updateFrecency(db, urls).catch(console.error);
return items;
}
);
@ -2324,7 +2283,7 @@ async function handleBookmarkItemSpecialData(itemId, item) {
async function queryBookmarks(info) {
let queryParams = {
tags_folder: await promiseTagsFolderId(),
tags_folder: lazy.PlacesUtils.tagsFolderId,
};
// We're searching for bookmarks, so exclude tags.
let queryString = "WHERE b.parent <> :tags_folder";
@ -2534,7 +2493,6 @@ async function fetchBookmarksByGUIDPrefix(info, options = {}) {
async function fetchBookmarksByURL(info, options = {}) {
let query = async function(db) {
let tagsFolderId = await promiseTagsFolderId();
let rows = await db.executeCached(
`/* do not warn (bug no): not worth to add an index */
SELECT b.guid, IFNULL(p.guid, '') AS parentGuid, b.position AS 'index',
@ -2554,7 +2512,11 @@ async function fetchBookmarksByURL(info, options = {}) {
AND _grandParentId <> :tagsFolderId
ORDER BY b.lastModified DESC
`,
{ url: info.url.href, tagsFolderId, tagsGuid: Bookmarks.tagsGuid }
{
url: info.url.href,
tagsFolderId: lazy.PlacesUtils.tagsFolderId,
tagsGuid: Bookmarks.tagsGuid,
}
);
return rows.length ? rowsToItemsArray(rows) : null;
@ -2607,7 +2569,6 @@ function fetchRecentBookmarks(numberOfItems) {
return lazy.PlacesUtils.withConnectionWrapper(
"Bookmarks.jsm: fetchRecentBookmarks",
async function(db) {
let tagsFolderId = await promiseTagsFolderId();
let rows = await db.executeCached(
`SELECT b.guid, IFNULL(p.guid, '') AS parentGuid, b.position AS 'index',
b.dateAdded, b.lastModified, b.type,
@ -2625,7 +2586,7 @@ function fetchRecentBookmarks(numberOfItems) {
LIMIT :numberOfItems
`,
{
tagsFolderId,
tagsFolderId: lazy.PlacesUtils.tagsFolderId,
type: Bookmarks.TYPE_BOOKMARK,
numberOfItems,
}
@ -2764,21 +2725,9 @@ function removeBookmarks(items, options) {
await insertTombstones(db, items, syncChangeDelta);
});
// Update the frecencies outside of the transaction, excluding tags, so that
// the updates can progress in the background.
urls = urls.concat(
items
.filter(item => {
let isUntagging =
item._grandParentId == lazy.PlacesUtils.tagsFolderId;
return !isUntagging && "url" in item;
})
.map(item => item.url)
);
urls = urls.concat(items.map(item => item.url).filter(item => item));
if (urls.length) {
await lazy.PlacesUtils.keywords.removeFromURLsIfNotBookmarked(urls);
updateFrecency(db, urls).catch(console.error);
}
}
);
@ -3021,37 +2970,6 @@ function validateBookmarkObject(name, input, behavior) {
);
}
/**
* Updates frecency for a list of URLs.
*
* @param db
* the Sqlite.sys.mjs connection handle.
* @param urls
* the array of URLs to update.
*/
var updateFrecency = async function(db, urls) {
let hrefs = urls.map(url => url.href);
// We just use the hashes, since updating a few additional urls won't hurt.
for (let chunk of lazy.PlacesUtils.chunkArray(hrefs, db.variableLimit)) {
await db.execute(
`UPDATE moz_places
SET hidden = (url_hash BETWEEN hash("place", "prefix_lo") AND hash("place", "prefix_hi")),
frecency = CALCULATE_FRECENCY(id)
WHERE url_hash IN (${lazy.PlacesUtils.sqlBindPlaceholders(
chunk,
"hash(",
")"
)})`,
chunk
);
}
// Trigger frecency updates for all affected origins.
await db.executeCached(`DELETE FROM moz_updateoriginsupdate_temp`);
PlacesObservers.notifyListeners([new PlacesRanking()]);
};
/**
* Removes any orphan annotation entries.
*
@ -3159,10 +3077,7 @@ var setAncestorsLastModified = async function(
* @param {Array} folderGuids
* array of folder guids.
* @return {Array}
* An array of urls that will need to be updated for frecency. These
* are returned rather than updated immediately so that the caller
* can decide when they need to be updated - they do not need to
* stop this function from completing.
* An array of the affected urls.
*/
var removeFoldersContents = async function(db, folderGuids, options) {
let syncChangeDelta = lazy.PlacesSyncUtils.bookmarks.determineSyncChangeDelta(

8
toolkit/components/places/PlacesUtils.sys.mjs
View File

@ -1957,7 +1957,9 @@ export var PlacesUtils = {
// The IGNORE conflict can trigger on `guid`.
await db.executeCached(
`INSERT OR IGNORE INTO moz_places (url, url_hash, rev_host, hidden, frecency, guid)
VALUES (:url, hash(:url), :rev_host, 0, :frecency,
VALUES (:url, hash(:url), :rev_host,
(CASE WHEN :url BETWEEN 'place:' AND 'place:' || X'FFFF' THEN 1 ELSE 0 END),
:frecency,
IFNULL((SELECT guid FROM moz_places WHERE url_hash = hash(:url) AND url = :url),
GENERATE_GUID()))
`,
@ -1986,7 +1988,9 @@ export var PlacesUtils = {
async maybeInsertManyPlaces(db, urls) {
await db.executeCached(
`INSERT OR IGNORE INTO moz_places (url, url_hash, rev_host, hidden, frecency, guid) VALUES
(:url, hash(:url), :rev_host, 0, :frecency,
(:url, hash(:url), :rev_host,
(CASE WHEN :url BETWEEN 'place:' AND 'place:' || X'FFFF' THEN 1 ELSE 0 END),
:frecency,
IFNULL((SELECT guid FROM moz_places WHERE url_hash = hash(:url) AND url = :url), :maybeguid))`,
urls.map(url => ({
url: url.href,

3
toolkit/components/places/bookmark_sync/src/store.rs
View File

@ -537,7 +537,8 @@ fn update_local_items_in_places<'t>(
let mut statement = db.prepare(format!(
"INSERT OR IGNORE INTO moz_places(url, url_hash, rev_host, hidden,
frecency, guid)
SELECT u.url, u.hash, u.revHost, 0,
SELECT u.url, u.hash, u.revHost,
(CASE WHEN u.url BETWEEN 'place:' AND 'place:' || X'FFFF' THEN 1 ELSE 0 END),
(CASE v.kind WHEN {} THEN 0 ELSE -1 END),
IFNULL((SELECT h.guid FROM moz_places h
WHERE h.url_hash = u.hash AND

24
toolkit/components/places/nsNavBookmarks.cpp
View File

@ -401,13 +401,6 @@ nsNavBookmarks::InsertBookmark(int64_t aFolder, nsIURI* aURI, int32_t aIndex,
aNewBookmarkId, guid);
NS_ENSURE_SUCCESS(rv, rv);
// If not a tag, recalculate frecency for this entry, since it changed.
int64_t tagsRootId = mDB->GetTagsFolderId();
if (grandParentId != tagsRootId) {
rv = history->UpdateFrecency(placeId);
NS_ENSURE_SUCCESS(rv, rv);
}
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
@ -418,6 +411,7 @@ nsNavBookmarks::InsertBookmark(int64_t aFolder, nsIURI* aURI, int32_t aIndex,
Sequence<OwningNonNull<PlacesEvent>> notifications;
nsAutoCString utf8spec;
aURI->GetSpec(utf8spec);
int64_t tagsRootId = mDB->GetTagsFolderId();
RefPtr<PlacesBookmarkAddition> bookmark = new PlacesBookmarkAddition();
bookmark->mItemType = TYPE_BOOKMARK;
@ -430,7 +424,7 @@ nsNavBookmarks::InsertBookmark(int64_t aFolder, nsIURI* aURI, int32_t aIndex,
bookmark->mGuid.Assign(guid);
bookmark->mParentGuid.Assign(folderGuid);
bookmark->mSource = aSource;
bookmark->mIsTagging = grandParentId == mDB->GetTagsFolderId();
bookmark->mIsTagging = grandParentId == tagsRootId;
bool success = !!notifications.AppendElement(bookmark.forget(), fallible);
MOZ_RELEASE_ASSERT(success);
@ -538,13 +532,6 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource) {
nsCOMPtr<nsIURI> uri;
if (bookmark.type == TYPE_BOOKMARK) {
// If not a tag, recalculate frecency for this entry, since it changed.
if (bookmark.grandParentId != tagsRootId) {
nsNavHistory* history = nsNavHistory::GetHistoryService();
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
rv = history->UpdateFrecency(bookmark.placeId);
NS_ENSURE_SUCCESS(rv, rv);
}
// A broken url should not interrupt the removal process.
(void)NS_NewURI(getter_AddRefs(uri), bookmark.url);
// We cannot assert since some automated tests are checking this path.
@ -866,13 +853,6 @@ nsresult nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId,
nsCOMPtr<nsIURI> uri;
if (child.type == TYPE_BOOKMARK) {
// If not a tag, recalculate frecency for this entry, since it changed.
if (child.grandParentId != tagsRootId) {
nsNavHistory* history = nsNavHistory::GetHistoryService();
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
rv = history->UpdateFrecency(child.placeId);
NS_ENSURE_SUCCESS(rv, rv);
}
// A broken url should not interrupt the removal process.
(void)NS_NewURI(getter_AddRefs(uri), child.url);
// We cannot assert since some automated tests are checking this path.

88
toolkit/components/places/nsPlacesTriggers.h
View File

@ -10,50 +10,47 @@
# define __nsPlacesTriggers_h__
/**
* Exclude these visit types:
* These visit types are excluded from visit_count:
* 0 - invalid
* 4 - EMBED
* 7 - DOWNLOAD
* 8 - FRAMED_LINK
* 9 - RELOAD
**/
# define EXCLUDED_VISIT_TYPES "0, 4, 7, 8, 9"
# define VISIT_COUNT_INC(field) \
"(SELECT CASE WHEN " field " IN (0, 4, 7, 8, 9) THEN 0 ELSE 1 END) "
/**
* This triggers update visit_count and last_visit_date based on historyvisits
* table changes.
*/
# define CREATE_HISTORYVISITS_AFTERINSERT_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_historyvisits_afterinsert_v2_trigger " \
"AFTER INSERT ON moz_historyvisits FOR EACH ROW " \
"BEGIN " \
"SELECT invalidate_days_of_history();" \
"SELECT store_last_inserted_id('moz_historyvisits', NEW.id); " \
"UPDATE moz_places SET " \
"visit_count = visit_count + (SELECT NEW.visit_type NOT IN " \
"(" EXCLUDED_VISIT_TYPES \
")), " \
"recalc_frecency = (frecency <> 0), " \
"last_visit_date = MAX(IFNULL(last_visit_date, 0), NEW.visit_date) " \
"WHERE id = NEW.place_id;" \
# define CREATE_HISTORYVISITS_AFTERINSERT_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_historyvisits_afterinsert_v2_trigger " \
"AFTER INSERT ON moz_historyvisits FOR EACH ROW " \
"BEGIN " \
"SELECT invalidate_days_of_history();" \
"SELECT store_last_inserted_id('moz_historyvisits', NEW.id); " \
"UPDATE moz_places SET " \
"visit_count = visit_count + " VISIT_COUNT_INC("NEW.visit_type") ", " \
"recalc_frecency = (frecency <> 0), " \
"last_visit_date = MAX(IFNULL(last_visit_date, 0), NEW.visit_date) " \
"WHERE id = NEW.place_id;" \
"END")
# define CREATE_HISTORYVISITS_AFTERDELETE_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_historyvisits_afterdelete_v2_trigger " \
"AFTER DELETE ON moz_historyvisits FOR EACH ROW " \
"BEGIN " \
"SELECT invalidate_days_of_history();" \
"UPDATE moz_places SET " \
"visit_count = visit_count - (SELECT OLD.visit_type NOT IN " \
"(" EXCLUDED_VISIT_TYPES \
")), " \
"recalc_frecency = (frecency <> 0), " \
"last_visit_date = (SELECT visit_date FROM moz_historyvisits " \
"WHERE place_id = OLD.place_id " \
"ORDER BY visit_date DESC LIMIT 1) " \
"WHERE id = OLD.place_id;" \
# define CREATE_HISTORYVISITS_AFTERDELETE_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_historyvisits_afterdelete_v2_trigger " \
"AFTER DELETE ON moz_historyvisits FOR EACH ROW " \
"BEGIN " \
"SELECT invalidate_days_of_history();" \
"UPDATE moz_places SET " \
"visit_count = visit_count - " VISIT_COUNT_INC("OLD.visit_type") ", " \
"recalc_frecency = (frecency <> 0), " \
"last_visit_date = (SELECT visit_date FROM moz_historyvisits " \
"WHERE place_id = OLD.place_id " \
"ORDER BY visit_date DESC LIMIT 1) " \
"WHERE id = OLD.place_id;" \
"END")
// This macro is a helper for the next several triggers. It updates the origin
@ -269,6 +266,14 @@
"AND userContextId = NEW.userContextId;" \
"END")
/**
* Any bookmark operation should recalculate frecency, apart from place:
* queries.
*/
# define NOT_A_QUERY \
" url_hash NOT BETWEEN hash('place', 'prefix_lo') " \
" AND hash('place', 'prefix_hi') "
# define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERDELETE_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterdelete_trigger " \
@ -276,10 +281,20 @@
"BEGIN " \
"UPDATE moz_places " \
"SET foreign_count = foreign_count - 1, " \
" recalc_frecency = (frecency <> 0) " \
" recalc_frecency = " NOT_A_QUERY \
"WHERE id = OLD.fk;" \
"END")
/**
* Currently expiration skips anything with frecency = -1, since that is the
* default value for new page insertions. Unfortunately adding and immediately
* removing a bookmark will generate a page with frecency = -1 that would never
* be expired until visited.
* As a temporary workaround we set frecency to 1 on bookmark addition if it was
* set to -1. This is not elegant, but it will be fixed by Bug 1475582 once
* removing bookmarks will immediately take care of removing orphan pages.
* Note setting frecency resets recalc_frecency, so do it first.
*/
# define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERINSERT_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterinsert_trigger " \
@ -288,8 +303,11 @@
"SELECT store_last_inserted_id('moz_bookmarks', NEW.id); " \
"SELECT note_sync_change() WHERE NEW.syncChangeCounter > 0; " \
"UPDATE moz_places " \
"SET frecency = 1 WHERE frecency = -1 AND " NOT_A_QUERY \
";" \
"UPDATE moz_places " \
"SET foreign_count = foreign_count + 1, " \
" recalc_frecency = (frecency <> 0) " \
" recalc_frecency = " NOT_A_QUERY \
"WHERE id = NEW.fk;" \
"END")
@ -302,11 +320,11 @@
"WHERE NEW.syncChangeCounter <> OLD.syncChangeCounter; " \
"UPDATE moz_places " \
"SET foreign_count = foreign_count + 1, " \
" recalc_frecency = (frecency <> 0) " \
" recalc_frecency = " NOT_A_QUERY \
"WHERE OLD.fk <> NEW.fk AND id = NEW.fk;" \
"UPDATE moz_places " \
"SET foreign_count = foreign_count - 1, " \
" recalc_frecency = (frecency <> 0) " \
" recalc_frecency = " NOT_A_QUERY \
"WHERE OLD.fk <> NEW.fk AND id = OLD.fk;" \
"END")

14
toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js
View File

@ -9,7 +9,7 @@ add_task(async function test_eraseEverything() {
uri: NetUtil.newURI("http://mozilla.org/"),
});
let frecencyForExample = frecencyForUrl("http://example.com/");
let frecencyForMozilla = frecencyForUrl("http://example.com/");
let frecencyForMozilla = frecencyForUrl("http://mozilla.org/");
Assert.ok(frecencyForExample > 0);
Assert.ok(frecencyForMozilla > 0);
let unfiledFolder = await PlacesUtils.bookmarks.insert({
@ -66,21 +66,13 @@ add_task(async function test_eraseEverything() {
});
checkBookmarkObject(toolbarBookmarkInFolder);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.ok(frecencyForUrl("http://example.com/") > frecencyForExample);
Assert.ok(frecencyForUrl("http://example.com/") > frecencyForMozilla);
const promise = PlacesTestUtils.waitForNotification(
"pages-rank-changed",
() => true,
"places"
);
await PlacesUtils.bookmarks.eraseEverything();
// Ensure we get an pages-rank-changed event.
await promise;
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.equal(frecencyForUrl("http://example.com/"), frecencyForExample);
Assert.equal(frecencyForUrl("http://example.com/"), frecencyForMozilla);
});

2
toolkit/components/places/tests/bookmarks/test_bookmarks_insert.js
View File

@ -386,7 +386,7 @@ add_task(async function create_bookmark_frecency() {
});
checkBookmarkObject(bm);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.greater(frecencyForUrl(bm.url), 0, "Check frecency has been updated");
});

4
toolkit/components/places/tests/bookmarks/test_bookmarks_insertTree.js
View File

@ -389,7 +389,7 @@ add_task(async function create_hierarchy() {
],
guid: PlacesUtils.bookmarks.unfiledGuid,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
PlacesUtils.observers.removeListener(["bookmark-added"], listener);
let parentFolder = null,
subFolder = null;
@ -465,7 +465,7 @@ add_task(async function insert_many_non_nested() {
],
guid: PlacesUtils.bookmarks.unfiledGuid,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
PlacesUtils.observers.removeListener(["bookmark-added"], listener);
let startIndex = -1;
for (let bm of bms) {

14
toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
View File

@ -114,14 +114,14 @@ add_task(async function remove_bookmark() {
title: "a bookmark",
});
checkBookmarkObject(bm1);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promise;
// This second one checks the frecency is changed when we remove the bookmark.
promise = promiseRankingChanged();
await PlacesUtils.bookmarks.remove(bm1.guid);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promise;
});
@ -136,7 +136,7 @@ add_task(async function remove_multiple_bookmarks_simple() {
title: "a bookmark",
});
checkBookmarkObject(bm1);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
const promise2 = promiseRankingChanged();
let bm2 = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
@ -145,7 +145,7 @@ add_task(async function remove_multiple_bookmarks_simple() {
title: "a bookmark",
});
checkBookmarkObject(bm2);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await Promise.all([promise1, promise2]);
// We should get a pages-rank-changed event with the removal of
@ -153,7 +153,7 @@ add_task(async function remove_multiple_bookmarks_simple() {
const promise3 = promiseRankingChanged();
await PlacesUtils.bookmarks.remove([bm1, bm2]);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promise3;
});
@ -315,7 +315,7 @@ add_task(async function test_contents_removed() {
await PlacesUtils.bookmarks.remove(folder1);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(folder1.guid), null);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(bm1.guid), null);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promise;
let expectedNotifications = [
@ -361,7 +361,7 @@ add_task(async function test_nested_contents_removed() {
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(folder1.guid), null);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(folder2.guid), null);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(bm1.guid), null);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promise;
});

2
toolkit/components/places/tests/bookmarks/test_bookmarks_update.js
View File

@ -307,6 +307,7 @@ add_task(async function update_url() {
});
checkBookmarkObject(bm);
let lastModified = bm.lastModified;
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
let frecency = frecencyForUrl(bm.url);
Assert.greater(frecency, 0, "Check frecency has been updated");
@ -322,6 +323,7 @@ add_task(async function update_url() {
Assert.equal(bm.url.href, "http://mozilla.org/");
Assert.ok(bm.lastModified >= lastModified);
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.equal(
frecencyForUrl("http://example.com/"),
frecency,

6
toolkit/components/places/tests/head_common.js
View File

@ -69,6 +69,12 @@ XPCOMUtils.defineLazyGetter(this, "SMALLSVG_DATA_URI", function() {
);
});
XPCOMUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => {
return Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
).wrappedJSObject;
});
var gTestDir = do_get_cwd();
// Initialize profile.

14
toolkit/components/places/tests/unit/test_412132.js
View File

@ -27,7 +27,7 @@ add_task(async function changeuri_unvisited_bookmark() {
url: TEST_URL1,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.notEqual(
frecencyForUrl(TEST_URL1),
@ -40,7 +40,7 @@ add_task(async function changeuri_unvisited_bookmark() {
url: TEST_URL2,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.equal(
frecencyForUrl(TEST_URL1),
@ -63,7 +63,7 @@ add_task(async function changeuri_visited_bookmark() {
url: TEST_URL1,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.notEqual(
frecencyForUrl(TEST_URL1),
@ -73,14 +73,14 @@ add_task(async function changeuri_visited_bookmark() {
await PlacesTestUtils.addVisits(TEST_URL1);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await PlacesUtils.bookmarks.update({
guid: bookmark.guid,
url: TEST_URL2,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.notEqual(
frecencyForUrl(TEST_URL1),
@ -110,7 +110,7 @@ add_task(async function changeuri_bookmark_still_bookmarked() {
url: TEST_URL1,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.notEqual(
frecencyForUrl(TEST_URL1),
@ -123,7 +123,7 @@ add_task(async function changeuri_bookmark_still_bookmarked() {
url: TEST_URL2,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("URI still bookmarked => frecency should != 0");
Assert.notEqual(frecencyForUrl(TEST_URL2), 0);

8
toolkit/components/places/tests/unit/test_frecency_decay.js
View File

@ -50,6 +50,7 @@ add_task(async function test_frecency_decay() {
url,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promiseOne;
let histogram = TelemetryTestUtils.getAndClearHistogram(
@ -59,13 +60,10 @@ add_task(async function test_frecency_decay() {
Assert.equal(PlacesUtils.history.isFrecencyDecaying, false);
let promiseRanking = promiseRankingChanged();
let svc = Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
);
svc.observe(null, "idle-daily", "");
PlacesFrecencyRecalculator.observe(null, "idle-daily", "");
Assert.equal(PlacesUtils.history.isFrecencyDecaying, true);
info("Wait for completion.");
await svc.wrappedJSObject.pendingFrecencyDecayPromise;
await PlacesFrecencyRecalculator.pendingFrecencyDecayPromise;
await promiseRanking;
Assert.equal(PlacesUtils.history.isFrecencyDecaying, false);

11
toolkit/components/places/tests/unit/test_frecency_observers.js
View File

@ -33,6 +33,7 @@ add_task(async function test_nsNavHistory_UpdateFrecency() {
url,
title: "test",
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await promise;
});
@ -58,20 +59,14 @@ add_task(async function test_clear() {
});
add_task(async function test_nsNavHistory_decayFrecency() {
let svc = Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
);
svc.observe(null, "idle-daily", "");
PlacesFrecencyRecalculator.observe(null, "idle-daily", "");
await Promise.all([onRankingChanged()]);
});
add_task(async function test_nsNavHistory_decayFrecency() {
let svc = Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
);
await Promise.all([
onRankingChanged(),
svc.wrappedJSObject.recalculateAnyOutdatedFrecencies(),
PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies(),
]);
});

81
toolkit/components/places/tests/unit/test_frecency_recalc_triggers.js
View File

@ -2,13 +2,15 @@
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Tests that all the operations affecting frecency are triggering a recalc.
* This currently includes:
* - adding visits (will recalc immediately)
* Tests that all the operations affecting frecency are either recalculating
* immediately or triggering a recalculation.
* Operations that should recalculate immediately:
* - adding visits
* Operations that should just trigger a recalculation:
* - removing visits
* - adding a bookmark
* - removing a bookmark
* - changing url of a bookmark+
* - changing url of a bookmark
*
* Also check setting a frecency resets recalc_frecency to 0.
**/
@ -22,8 +24,8 @@ const TEST_URL_2 = "https://example2.com/";
function insertVisit(url) {
return PlacesUtils.withConnectionWrapper("insertVisit", async db => {
await db.execute(
`INSERT INTO moz_historyvisits(place_id, visit_date)
VALUES ((SELECT id FROM moz_places WHERE url = :url), 1648226608386000)`,
`INSERT INTO moz_historyvisits(place_id, visit_date, visit_type)
VALUES ((SELECT id FROM moz_places WHERE url = :url), 1648226608386000, 1)`,
{ url }
);
});
@ -44,25 +46,7 @@ function resetFrecency(url) {
});
});
}
function changeBookmarkToUrl(guid, url) {
return PlacesUtils.withConnectionWrapper("insertVisit", async db => {
await db.execute(
`UPDATE moz_bookmarks SET fk = (SELECT id FROM moz_places WHERE url = :url )
WHERE guid = :guid`,
{
url,
guid,
}
);
});
}
function removeBookmark(guid) {
return PlacesUtils.withConnectionWrapper("insertVisit", async db => {
await db.execute(`DELETE FROM moz_bookmarks WHERE guid = :guid`, {
guid,
});
});
}
add_task(async function test_visit() {
// First add a bookmark so the page is not orphaned.
let bm = await PlacesUtils.bookmarks.insert({
@ -77,7 +61,7 @@ add_task(async function test_visit() {
let recalc = await PlacesTestUtils.fieldInDB(TEST_URL, "recalc_frecency");
Assert.equal(recalc, 0, "frecency doesn't need a recalc");
info("Add a visit (raw query) check frecency is not calculated immadiately");
info("Add a visit (raw query) check frecency is not calculated immediately");
await insertVisit(TEST_URL);
let frecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
Assert.equal(frecency, originalFrecency, "frecency is unchanged");
@ -102,22 +86,28 @@ add_task(async function test_visit() {
add_task(async function test_bookmark() {
// First add a visit so the page is not orphaned.
await PlacesTestUtils.addVisits(TEST_URL);
await PlacesTestUtils.addVisits(TEST_URL_2);
await PlacesTestUtils.addVisits([TEST_URL, TEST_URL_2]);
info("Check adding a bookmark sets frecency immediately");
let originalFrecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
Assert.greater(originalFrecency, 0);
info("Check adding a bookmark sets recalc_frecency");
let bm = await PlacesUtils.bookmarks.insert({
url: TEST_URL,
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
});
let originalFrecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
Assert.ok(originalFrecency > 0, "frecency was recalculated immediately");
let frecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
Assert.equal(frecency, originalFrecency, "frecency is unchanged");
let recalc = await PlacesTestUtils.fieldInDB(TEST_URL, "recalc_frecency");
Assert.equal(recalc, 0, "frecency doesn't need a recalc");
Assert.equal(recalc, 1, "frecency needs a recalc");
info("Check changing a bookmark url sets recalc_frecency on both urls");
await changeBookmarkToUrl(bm.guid, TEST_URL_2);
let frecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
await PlacesUtils.bookmarks.update({
guid: bm.guid,
url: TEST_URL_2,
});
frecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
Assert.equal(frecency, originalFrecency, "frecency is unchanged");
recalc = await PlacesTestUtils.fieldInDB(TEST_URL, "recalc_frecency");
Assert.equal(recalc, 1, "frecency needs a recalc");
@ -135,7 +125,7 @@ add_task(async function test_bookmark() {
Assert.equal(recalc, 0, "frecency doesn't need a recalc");
info("Removing a bookmark sets recalc_frecency");
await removeBookmark(bm.guid);
await PlacesUtils.bookmarks.remove(bm.guid);
frecency = await PlacesTestUtils.fieldInDB(TEST_URL, "frecency");
Assert.equal(frecency, -1, "frecency is unchanged");
recalc = await PlacesTestUtils.fieldInDB(TEST_URL, "recalc_frecency");
@ -147,3 +137,24 @@ add_task(async function test_bookmark() {
await PlacesUtils.history.clear();
});
add_task(async function test_bookmark_frecency_zero() {
info("A url with frecency 0 should be recalculated if bookmarked");
let url = "https://zerofrecency.org";
await PlacesTestUtils.addVisits({ url, transition: TRANSITION_FRAMED_LINK });
Assert.equal(await PlacesTestUtils.fieldInDB(url, "frecency"), 0);
Assert.equal(await PlacesTestUtils.fieldInDB(url, "recalc_frecency"), 0);
await PlacesUtils.bookmarks.insert({
url,
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
});
Assert.equal(await PlacesTestUtils.fieldInDB(url, "recalc_frecency"), 1);
info("place: uris should not be recalculated");
url = "place:test";
await PlacesUtils.bookmarks.insert({
url,
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
});
Assert.equal(await PlacesTestUtils.fieldInDB(url, "frecency"), 0);
Assert.equal(await PlacesTestUtils.fieldInDB(url, "recalc_frecency"), 0);
});

51
toolkit/components/places/tests/unit/test_frecency_recalculator.js
View File

@ -3,10 +3,6 @@
// Test PlacesFrecencyRecalculator scheduling.
var svc = Cc["@mozilla.org/places/frecency-recalculator;1"].getService(
Ci.nsIObserver
).wrappedJSObject;
async function getOriginFrecency(origin) {
let db = await PlacesUtils.promiseDBConnection();
return (
@ -60,10 +56,16 @@ async function addVisitsAndSetRecalc(urls) {
add_task(async function test() {
info("On startup a recalculation is always pending.");
Assert.ok(svc.isRecalculationPending, "Recalculation should be pending");
Assert.ok(
PlacesFrecencyRecalculator.isRecalculationPending,
"Recalculation should be pending"
);
// If everything gets recalculated, then it should not be pending anymore.
await svc.recalculateAnyOutdatedFrecencies();
Assert.ok(!svc.isRecalculationPending, "Recalculation should not be pending");
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.ok(
!PlacesFrecencyRecalculator.isRecalculationPending,
"Recalculation should not be pending"
);
// If after a recalculation there's outdated entries left, a new recalculation
// should be pending.
@ -74,10 +76,16 @@ add_task(async function test() {
await resetOriginFrecency(url1.host);
await resetOriginFrecency(url2.host);
await svc.recalculateSomeFrecencies({ chunkSize: 1 });
Assert.ok(svc.isRecalculationPending, "Recalculation should be pending");
await svc.recalculateSomeFrecencies({ chunkSize: 2 });
Assert.ok(!svc.isRecalculationPending, "Recalculation should not be pending");
await PlacesFrecencyRecalculator.recalculateSomeFrecencies({ chunkSize: 1 });
Assert.ok(
PlacesFrecencyRecalculator.isRecalculationPending,
"Recalculation should be pending"
);
await PlacesFrecencyRecalculator.recalculateSomeFrecencies({ chunkSize: 2 });
Assert.ok(
!PlacesFrecencyRecalculator.isRecalculationPending,
"Recalculation should not be pending"
);
Assert.greater(await getOriginFrecency(url1.host), 0);
Assert.greater(await getOriginFrecency(url2.host), 0);
@ -97,17 +105,26 @@ add_task(async function test() {
PlacesUtils.history.shouldStartFrecencyRecalculation,
"Should have set shouldStartFrecencyRecalculation"
);
svc.maybeStartFrecencyRecalculation();
Assert.ok(svc.isRecalculationPending, "Recalculation should be pending");
PlacesFrecencyRecalculator.maybeStartFrecencyRecalculation();
Assert.ok(
PlacesFrecencyRecalculator.isRecalculationPending,
"Recalculation should be pending"
);
});
add_task(async function test_idle_notifications() {
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
let spyStart = sinon.spy(svc, "startRecalculationCheckInterval");
let spyStop = sinon.spy(svc, "stopRecalculationCheckInterval");
svc.observe(null, "idle", "");
let spyStart = sinon.spy(
PlacesFrecencyRecalculator,
"startRecalculationCheckInterval"
);
let spyStop = sinon.spy(
PlacesFrecencyRecalculator,
"stopRecalculationCheckInterval"
);
PlacesFrecencyRecalculator.observe(null, "idle", "");
Assert.ok(!spyStart.calledOnce, "Start callback has not been invoked");
Assert.ok(spyStop.calledOnce, "Stop callback has been invoked");
svc.observe(null, "active", "");
PlacesFrecencyRecalculator.observe(null, "active", "");
Assert.ok(spyStop.calledOnce, "Start callback has been invoked");
});

4
toolkit/components/places/tests/unit/test_frecency_zero_updated.js
View File

@ -10,13 +10,13 @@ add_task(async function() {
url: TEST_URI,
title: "A title",
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.ok(frecencyForUrl(TEST_URI) > 0);
// Removing the bookmark should leave an orphan page with zero frecency.
// Note this would usually be expired later by expiration.
await PlacesUtils.bookmarks.remove(bookmark.guid);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
Assert.equal(frecencyForUrl(TEST_URI), 0);
// Now add a valid visit to the page, frecency should increase.

8
toolkit/components/places/tests/unit/test_origins.js
View File

@ -880,15 +880,18 @@ add_task(async function addRemoveBookmarks() {
})
);
}
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([
["http://", "example.com", ["http://example.com/"]],
["http://", "www.example.com", ["http://www.example.com/"]],
]);
await PlacesUtils.bookmarks.remove(bookmarks[0]);
await PlacesUtils.history.clear();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([["http://", "www.example.com", ["http://www.example.com/"]]]);
await PlacesUtils.bookmarks.remove(bookmarks[1]);
await PlacesUtils.history.clear();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([]);
await cleanUp();
});
@ -905,6 +908,7 @@ add_task(async function changeBookmarks() {
})
);
}
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([
["http://", "example.com", ["http://example.com/"]],
["http://", "www.example.com", ["http://www.example.com/"]],
@ -914,6 +918,7 @@ add_task(async function changeBookmarks() {
guid: bookmarks[0].guid,
});
await PlacesUtils.history.clear();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([["http://", "www.example.com", ["http://www.example.com/"]]]);
await cleanUp();
});
@ -973,7 +978,7 @@ add_task(async function moreOriginFrecencyStats() {
title: "A bookmark",
url: NetUtil.newURI("http://example.com/1"),
});
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([
[
"http://",
@ -997,6 +1002,7 @@ add_task(async function moreOriginFrecencyStats() {
// contributing to the frecency stats.
await PlacesUtils.bookmarks.remove(bookmark);
await PlacesUtils.history.remove("http://example.com/1");
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
await checkDB([["http://", "example.com", ["http://example.com/0"]]]);
// Remove URL 0.

20
toolkit/components/places/tests/unit/test_update_frecency_after_delete.js
View File

@ -24,13 +24,13 @@ add_task(async function removed_bookmark() {
url: TEST_URI,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Bookmarked => frecency of URI should be != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
await PlacesUtils.bookmarks.remove(bm);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Unvisited URI no longer bookmarked => frecency should = 0");
Assert.equal(frecencyForUrl(TEST_URI), 0);
@ -50,14 +50,14 @@ add_task(async function removed_but_visited_bookmark() {
url: TEST_URI,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Bookmarked => frecency of URI should be != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
await PlacesTestUtils.addVisits(TEST_URI);
await PlacesUtils.bookmarks.remove(bm);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("*Visited* URI no longer bookmarked => frecency should != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
@ -82,13 +82,13 @@ add_task(async function remove_bookmark_still_bookmarked() {
url: TEST_URI,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Bookmarked => frecency of URI should be != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
await PlacesUtils.bookmarks.remove(bm1);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("URI still bookmarked => frecency should != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
@ -108,14 +108,14 @@ add_task(async function cleared_parent_of_visited_bookmark() {
url: TEST_URI,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Bookmarked => frecency of URI should be != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
await PlacesTestUtils.addVisits(TEST_URI);
await PlacesUtils.bookmarks.eraseEverything();
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("*Visited* URI no longer bookmarked => frecency should != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
@ -147,12 +147,12 @@ add_task(async function cleared_parent_of_bookmark_still_bookmarked() {
url: TEST_URI,
});
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
info("Bookmarked => frecency of URI should be != 0");
Assert.notEqual(frecencyForUrl(TEST_URI), 0);
await PlacesUtils.bookmarks.remove(folder);
await PlacesTestUtils.promiseAsyncUpdates();
await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
// URI still bookmarked => frecency should != 0.
Assert.notEqual(frecencyForUrl(TEST_URI), 0);