diff options
author | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-04-19 01:13:27 +0000 |
---|---|---|
committer | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-04-19 01:13:27 +0000 |
commit | 40a355a42d4a9444dc753c04c6608dade2f06a23 (patch) | |
tree | 871fc667d2de662f171103ce5ec067014ef85e61 /toolkit/components/places | |
parent | Adding upstream version 124.0.1. (diff) | |
download | firefox-adbda400be353e676059e335c3c0aaf99e719475.tar.xz firefox-adbda400be353e676059e335c3c0aaf99e719475.zip |
Adding upstream version 125.0.1.upstream/125.0.1
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'toolkit/components/places')
82 files changed, 1368 insertions, 487 deletions
diff --git a/toolkit/components/places/BookmarkHTMLUtils.sys.mjs b/toolkit/components/places/BookmarkHTMLUtils.sys.mjs index 559d8b9a8e..81bd791730 100644 --- a/toolkit/components/places/BookmarkHTMLUtils.sys.mjs +++ b/toolkit/components/places/BookmarkHTMLUtils.sys.mjs @@ -428,7 +428,7 @@ BookmarkImporter.prototype = { * We also don't import ADD_DATE or LAST_MODIFIED for separators because * pre-Places bookmarks did not support them. */ - _handleSeparator: function handleSeparator(aElt) { + _handleSeparator: function handleSeparator() { let frame = this._curFrame; let separator = { diff --git a/toolkit/components/places/BookmarkJSONUtils.sys.mjs b/toolkit/components/places/BookmarkJSONUtils.sys.mjs index 29967b5395..0a27db3696 100644 --- a/toolkit/components/places/BookmarkJSONUtils.sys.mjs +++ b/toolkit/components/places/BookmarkJSONUtils.sys.mjs @@ -21,26 +21,6 @@ const OLD_BOOKMARK_QUERY_TRANSLATIONS = { MOBILE_BOOKMARKS: PlacesUtils.bookmarks.mobileGuid, }; -/** - * Generates an hash for the given string. - * - * @note The generated hash is returned in base64 form. Mind the fact base64 - * is case-sensitive if you are going to reuse this code. - */ -function generateHash(aString) { - let cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance( - Ci.nsICryptoHash - ); - cryptoHash.init(Ci.nsICryptoHash.MD5); - let stringStream = Cc["@mozilla.org/io/string-input-stream;1"].createInstance( - Ci.nsIStringInputStream - ); - stringStream.setUTF8Data(aString); - cryptoHash.updateFromStream(stringStream, -1); - // base64 allows the '/' char, but we can't use it for filenames. - return cryptoHash.finish(true).replace(/\//g, "-"); -} - export var BookmarkJSONUtils = Object.freeze({ /** * Import bookmarks from a url. @@ -164,7 +144,8 @@ export var BookmarkJSONUtils = Object.freeze({ console.error("Unable to report telemetry."); } - let hash = generateHash(jsonString); + // Use "base64url" as this may be part of a filename. + let hash = PlacesUtils.sha256(jsonString, { format: "base64url" }); if (hash === aOptions.failIfHashIs) { let e = new Error("Hash conflict"); diff --git a/toolkit/components/places/BookmarkList.sys.mjs b/toolkit/components/places/BookmarkList.sys.mjs new file mode 100644 index 0000000000..3465948b52 --- /dev/null +++ b/toolkit/components/places/BookmarkList.sys.mjs @@ -0,0 +1,262 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const lazy = {}; + +ChromeUtils.defineESModuleGetters(lazy, { + DeferredTask: "resource://gre/modules/DeferredTask.sys.mjs", + PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs", +}); + +const OBSERVER_DEBOUNCE_RATE_MS = 500; +const OBSERVER_DEBOUNCE_TIMEOUT_MS = 5000; + +/** + * A collection of bookmarks that internally stays up-to-date in order to + * efficiently query whether certain URLs are bookmarked. + */ +export class BookmarkList { + /** + * The set of hashed URLs that need to be fetched from the database. + * + * @type {Set<string>} + */ + #urlsToFetch = new Set(); + + /** + * The function to call when changes are made. + * + * @type {function} + */ + #observer; + + /** + * Cached mapping of hashed URLs to how many bookmarks they are used in. + * + * @type {Map<string, number>} + */ + #bookmarkCount = new Map(); + + /** + * Cached mapping of bookmark GUIDs to their respective URL hashes. + * + * @type {Map<string, string>} + */ + #guidToUrl = new Map(); + + /** + * @type {DeferredTask} + */ + #observerTask; + + /** + * Construct a new BookmarkList. + * + * @param {string[]} urls + * The initial set of URLs to track. + * @param {function} [observer] + * The function to call when changes are made. + * @param {number} [debounceRate] + * Time between observer executions, in milliseconds. + * @param {number} [debounceTimeout] + * The maximum time to wait for an idle callback, in milliseconds. + */ + constructor(urls, observer, debounceRate, debounceTimeout) { + this.setTrackedUrls(urls); + this.#observer = observer; + this.handlePlacesEvents = this.handlePlacesEvents.bind(this); + this.addListeners(debounceRate, debounceTimeout); + } + + /** + * Add places listeners to this bookmark list. The observer (if one was + * provided) will be called after processing any events. + * + * @param {number} [debounceRate] + * Time between observer executions, in milliseconds. + * @param {number} [debounceTimeout] + * The maximum time to wait for an idle callback, in milliseconds. + */ + addListeners( + debounceRate = OBSERVER_DEBOUNCE_RATE_MS, + debounceTimeout = OBSERVER_DEBOUNCE_TIMEOUT_MS + ) { + lazy.PlacesUtils.observers.addListener( + ["bookmark-added", "bookmark-removed", "bookmark-url-changed"], + this.handlePlacesEvents + ); + this.#observerTask = new lazy.DeferredTask( + () => this.#observer?.(), + debounceRate, + debounceTimeout + ); + } + + /** + * Update the set of URLs to track. + * + * @param {string[]} urls + */ + async setTrackedUrls(urls) { + const updatedBookmarkCount = new Map(); + for (const url of urls) { + // Use cached value if possible. Otherwise, it must be fetched from db. + const urlHash = lazy.PlacesUtils.history.hashURL(url); + const count = this.#bookmarkCount.get(urlHash); + if (count != undefined) { + updatedBookmarkCount.set(urlHash, count); + } else { + this.#urlsToFetch.add(urlHash); + } + } + this.#bookmarkCount = updatedBookmarkCount; + + const updateGuidToUrl = new Map(); + for (const [guid, urlHash] of this.#guidToUrl.entries()) { + if (updatedBookmarkCount.has(urlHash)) { + updateGuidToUrl.set(guid, urlHash); + } + } + this.#guidToUrl = updateGuidToUrl; + } + + /** + * Check whether the given URL is bookmarked. + * + * @param {string} url + * @returns {boolean} + * The result, or `undefined` if the URL is not tracked. + */ + async isBookmark(url) { + if (this.#urlsToFetch.size) { + await this.#fetchTrackedUrls(); + } + const urlHash = lazy.PlacesUtils.history.hashURL(url); + const count = this.#bookmarkCount.get(urlHash); + return count != undefined ? Boolean(count) : count; + } + + /** + * Run the database query and populate the bookmarks cache with the URLs + * that are waiting to be fetched. + */ + async #fetchTrackedUrls() { + const urls = [...this.#urlsToFetch]; + this.#urlsToFetch = new Set(); + for (const urlHash of urls) { + this.#bookmarkCount.set(urlHash, 0); + } + const db = await lazy.PlacesUtils.promiseDBConnection(); + for (const chunk of lazy.PlacesUtils.chunkArray(urls, db.variableLimit)) { + // Note that this query does not *explicitly* filter out tags, but we + // should not expect to find any, unless the db is somehow malformed. + const sql = `SELECT b.guid, p.url_hash + FROM moz_bookmarks b + JOIN moz_places p + ON b.fk = p.id + WHERE p.url_hash IN (${Array(chunk.length).fill("?").join(",")})`; + const rows = await db.executeCached(sql, chunk); + for (const row of rows) { + this.#cacheBookmark( + row.getResultByName("guid"), + row.getResultByName("url_hash") + ); + } + } + } + + /** + * Handle bookmark events and update the cache accordingly. + * + * @param {PlacesEvent[]} events + */ + async handlePlacesEvents(events) { + let cacheUpdated = false; + let needsFetch = false; + for (const { guid, type, url } of events) { + const urlHash = lazy.PlacesUtils.history.hashURL(url); + if (this.#urlsToFetch.has(urlHash)) { + needsFetch = true; + continue; + } + const isUrlTracked = this.#bookmarkCount.has(urlHash); + switch (type) { + case "bookmark-added": + if (isUrlTracked) { + this.#cacheBookmark(guid, urlHash); + cacheUpdated = true; + } + break; + case "bookmark-removed": + if (isUrlTracked) { + this.#removeCachedBookmark(guid, urlHash); + cacheUpdated = true; + } + break; + case "bookmark-url-changed": { + const oldUrlHash = this.#guidToUrl.get(guid); + if (oldUrlHash) { + this.#removeCachedBookmark(guid, oldUrlHash); + cacheUpdated = true; + } + if (isUrlTracked) { + this.#cacheBookmark(guid, urlHash); + cacheUpdated = true; + } + break; + } + } + } + if (needsFetch) { + await this.#fetchTrackedUrls(); + cacheUpdated = true; + } + if (cacheUpdated) { + this.#observerTask.arm(); + } + } + + /** + * Remove places listeners from this bookmark list. URLs are no longer + * tracked. + * + * In order to resume tracking, you must call `setTrackedUrls()` followed by + * `addListeners()`. + */ + removeListeners() { + lazy.PlacesUtils.observers.removeListener( + ["bookmark-added", "bookmark-removed", "bookmark-url-changed"], + this.handlePlacesEvents + ); + if (!this.#observerTask.isFinalized) { + this.#observerTask.disarm(); + this.#observerTask.finalize(); + } + this.setTrackedUrls([]); + } + + /** + * Store a bookmark in the cache. + * + * @param {string} guid + * @param {string} urlHash + */ + #cacheBookmark(guid, urlHash) { + const count = this.#bookmarkCount.get(urlHash); + this.#bookmarkCount.set(urlHash, count + 1); + this.#guidToUrl.set(guid, urlHash); + } + + /** + * Remove a bookmark from the cache. + * + * @param {string} guid + * @param {string} urlHash + */ + #removeCachedBookmark(guid, urlHash) { + const count = this.#bookmarkCount.get(urlHash); + this.#bookmarkCount.set(urlHash, count - 1); + this.#guidToUrl.delete(guid); + } +} diff --git a/toolkit/components/places/Bookmarks.sys.mjs b/toolkit/components/places/Bookmarks.sys.mjs index 7f9a397c34..2ca2cb6b5d 100644 --- a/toolkit/components/places/Bookmarks.sys.mjs +++ b/toolkit/components/places/Bookmarks.sys.mjs @@ -230,7 +230,7 @@ export var Bookmarks = Object.freeze({ if (addedTime > now) { modTime = now; } - let insertInfo = validateBookmarkObject("Bookmarks.jsm: insert", info, { + let insertInfo = validateBookmarkObject("Bookmarks.sys.mjs: insert", info, { type: { defaultValue: this.TYPE_BOOKMARK }, index: { defaultValue: this.DEFAULT_INDEX }, url: { @@ -488,7 +488,7 @@ export var Bookmarks = Object.freeze({ } try { insertInfo = validateBookmarkObject( - "Bookmarks.jsm: insertTree", + "Bookmarks.sys.mjs: insertTree", info, insertInfo ); @@ -681,7 +681,7 @@ export var Bookmarks = Object.freeze({ // The info object is first validated here to ensure it's consistent, then // it's compared to the existing item to remove any properties that don't // need to be updated. - let updateInfo = validateBookmarkObject("Bookmarks.jsm: update", info, { + let updateInfo = validateBookmarkObject("Bookmarks.sys.mjs: update", info, { guid: { required: true }, index: { requiredIf: b => b.hasOwnProperty("parentGuid"), @@ -722,23 +722,27 @@ export var Bookmarks = Object.freeze({ Math.max(item.lastModified, updateInfo.dateAdded) ); } - updateInfo = validateBookmarkObject("Bookmarks.jsm: update", updateInfo, { - url: { validIf: () => item.type == this.TYPE_BOOKMARK }, - title: { - validIf: () => - [this.TYPE_BOOKMARK, this.TYPE_FOLDER].includes(item.type), - }, - lastModified: { - defaultValue: lastModifiedDefault, - validIf: b => - b.lastModified >= now || - b.lastModified >= (b.dateAdded || item.dateAdded), - }, - dateAdded: { defaultValue: item.dateAdded }, - }); + updateInfo = validateBookmarkObject( + "Bookmarks.sys.mjs: update", + updateInfo, + { + url: { validIf: () => item.type == this.TYPE_BOOKMARK }, + title: { + validIf: () => + [this.TYPE_BOOKMARK, this.TYPE_FOLDER].includes(item.type), + }, + lastModified: { + defaultValue: lastModifiedDefault, + validIf: b => + b.lastModified >= now || + b.lastModified >= (b.dateAdded || item.dateAdded), + }, + dateAdded: { defaultValue: item.dateAdded }, + } + ); return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: update", + "Bookmarks.sys.mjs: update", async db => { let parent; if (updateInfo.hasOwnProperty("parentGuid")) { @@ -1031,7 +1035,7 @@ export var Bookmarks = Object.freeze({ lazy.PlacesSyncUtils.bookmarks.determineSyncChangeDelta(source); await lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: moveToFolder", + "Bookmarks.sys.mjs: moveToFolder", async db => { const lastModified = new Date(); @@ -1283,7 +1287,10 @@ export var Bookmarks = Object.freeze({ // Even if we ignore any other unneeded property, we still validate any // known property to reduce likelihood of hidden bugs. - let removeInfo = validateBookmarkObject("Bookmarks.jsm: remove", info); + let removeInfo = validateBookmarkObject( + "Bookmarks.sys.mjs: remove", + info + ); removeInfos.push(removeInfo); } @@ -1372,7 +1379,7 @@ export var Bookmarks = Object.freeze({ } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: eraseEverything", + "Bookmarks.sys.mjs: eraseEverything", async function (db) { let urls; await db.executeTransaction(async function () { @@ -1552,7 +1559,7 @@ export var Bookmarks = Object.freeze({ // Even if we ignore any other unneeded property, we still validate any // known property to reduce likelihood of hidden bugs. let fetchInfo = validateBookmarkObject( - "Bookmarks.jsm: fetch", + "Bookmarks.sys.mjs: fetch", info, behavior ); @@ -1681,7 +1688,7 @@ export var Bookmarks = Object.freeze({ */ // TODO must implement these methods yet: // PlacesUtils.promiseBookmarksTree() - fetchTree(guid = "", options = {}) { + fetchTree() { throw new Error("Not yet implemented"); }, @@ -1695,7 +1702,7 @@ export var Bookmarks = Object.freeze({ * } */ async fetchTags() { - // TODO: Once the tagging API is implemented in Bookmarks.jsm, we can cache + // TODO: Once the tagging API is implemented in Bookmarks.sys.mjs, we can cache // the list of tags, instead of querying every time. let db = await lazy.PlacesUtils.promiseDBConnection(); let rows = await db.executeCached( @@ -1742,7 +1749,7 @@ export var Bookmarks = Object.freeze({ */ reorder(parentGuid, orderedChildrenGuids, options = {}) { let info = { guid: parentGuid }; - info = validateBookmarkObject("Bookmarks.jsm: reorder", info, { + info = validateBookmarkObject("Bookmarks.sys.mjs: reorder", info, { guid: { required: true }, }); @@ -2104,7 +2111,7 @@ async function updateBookmark( function insertBookmark(item, parent) { return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: insertBookmark", + "Bookmarks.sys.mjs: insertBookmark", async function (db) { // If a guid was not provided, generate one, so we won't need to fetch the // bookmark just after having created it. @@ -2202,7 +2209,7 @@ function insertBookmark(item, parent) { function insertBookmarkTree(items, source, parent, urls, lastAddedForParent) { return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: insertBookmarkTree", + "Bookmarks.sys.mjs: insertBookmarkTree", async function (db) { await db.executeTransaction(async function transaction() { await lazy.PlacesUtils.maybeInsertManyPlaces(db, urls); @@ -2353,7 +2360,7 @@ async function queryBookmarks(info) { } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: queryBookmarks", + "Bookmarks.sys.mjs: queryBookmarks", async function (db) { // _id, _childCount, _grandParentId and _parentId fields // are required to be in the result by the converting function @@ -2422,7 +2429,7 @@ async function fetchBookmark(info, options = {}) { return query(options.db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchBookmark", + "Bookmarks.sys.mjs: fetchBookmark", query ); } @@ -2454,7 +2461,7 @@ async function fetchBookmarkByPosition(info, options = {}) { return query(db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchBookmarkByPosition", + "Bookmarks.sys.mjs: fetchBookmarkByPosition", query ); } @@ -2502,7 +2509,7 @@ async function fetchBookmarksByTags(info, options = {}) { return query(db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchBookmarksByTags", + "Bookmarks.sys.mjs: fetchBookmarksByTags", query ); } @@ -2532,7 +2539,7 @@ async function fetchBookmarksByGUIDPrefix(info, options = {}) { return query(db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchBookmarksByGUIDPrefix", + "Bookmarks.sys.mjs: fetchBookmarksByGUIDPrefix", query ); } @@ -2574,7 +2581,7 @@ async function fetchBookmarksByURL(info, options = {}) { return query(db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchBookmarksByURL", + "Bookmarks.sys.mjs: fetchBookmarksByURL", query ); } @@ -2607,14 +2614,14 @@ async function fetchBookmarksByParentGUID(info, options = {}) { return query(db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchBookmarksByParentGUID", + "Bookmarks.sys.mjs: fetchBookmarksByParentGUID", query ); } function fetchRecentBookmarks(numberOfItems) { return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: fetchRecentBookmarks", + "Bookmarks.sys.mjs: fetchRecentBookmarks", async function (db) { let rows = await db.executeCached( `SELECT b.guid, IFNULL(p.guid, '') AS parentGuid, b.position AS 'index', @@ -2667,7 +2674,7 @@ async function fetchBookmarksByParent(db, info) { function removeBookmarks(items, options) { return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: removeBookmarks", + "Bookmarks.sys.mjs: removeBookmarks", async function (db) { let urls = []; @@ -2780,7 +2787,7 @@ function removeBookmarks(items, options) { function reorderChildren(parent, orderedChildrenGuids, options) { return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: reorderChildren", + "Bookmarks.sys.mjs: reorderChildren", db => db.executeTransaction(async function () { // Fetch old indices for the notifications. @@ -3314,7 +3321,7 @@ async function retrieveFullBookmarkPath(guid, options = {}) { return query(db); } return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.jsm: retrieveFullBookmarkPath", + "Bookmarks.sys.mjs: retrieveFullBookmarkPath", query ); } @@ -3328,42 +3335,38 @@ async function retrieveFullBookmarkPath(guid, options = {}) { */ async function getBookmarkDetailMap(aGuids) { return lazy.PlacesUtils.withConnectionWrapper( - "Bookmarks.geBookmarkDetails", + "Bookmarks.geBookmarkDetailMap", async db => { - const rows = await db.executeCached( - ` - SELECT - b.guid, - b.id, - b.parent, - IFNULL(h.frecency, 0), - IFNULL(h.hidden, 0), - IFNULL(h.visit_count, 0), - h.last_visit_date, - ( - SELECT group_concat(pp.title ORDER BY pp.title) - FROM moz_bookmarks bb - JOIN moz_bookmarks pp ON pp.id = bb.parent - JOIN moz_bookmarks gg ON gg.id = pp.parent - WHERE bb.fk = h.id - AND gg.guid = '${Bookmarks.tagsGuid}' - ), - t.guid, t.id, t.title - FROM moz_bookmarks b - LEFT JOIN moz_places h ON h.id = b.fk - LEFT JOIN moz_bookmarks t ON t.guid = target_folder_guid(h.url) - WHERE b.guid IN (${lazy.PlacesUtils.sqlBindPlaceholders(aGuids)}) - `, - aGuids - ); - - return new Map( - rows.map(row => { - const lastVisitDate = row.getResultByIndex(6); - - return [ - row.getResultByIndex(0), - { + let entries = new Map(); + for (let chunk of lazy.PlacesUtils.chunkArray(aGuids, db.variableLimit)) { + await db.executeCached( + ` + SELECT + b.guid, + b.id, + b.parent, + IFNULL(h.frecency, 0), + IFNULL(h.hidden, 0), + IFNULL(h.visit_count, 0), + h.last_visit_date, + ( + SELECT group_concat(pp.title ORDER BY pp.title) + FROM moz_bookmarks bb + JOIN moz_bookmarks pp ON pp.id = bb.parent + JOIN moz_bookmarks gg ON gg.id = pp.parent + WHERE bb.fk = h.id + AND gg.guid = '${Bookmarks.tagsGuid}' + ), + t.guid, t.id, t.title + FROM moz_bookmarks b + LEFT JOIN moz_places h ON h.id = b.fk + LEFT JOIN moz_bookmarks t ON t.guid = target_folder_guid(h.url) + WHERE b.guid IN (${lazy.PlacesUtils.sqlBindPlaceholders(chunk)}) + `, + chunk, + row => { + const lastVisitDate = row.getResultByIndex(6); + entries.set(row.getResultByIndex(0), { id: row.getResultByIndex(1), parentId: row.getResultByIndex(2), frecency: row.getResultByIndex(3), @@ -3376,10 +3379,11 @@ async function getBookmarkDetailMap(aGuids) { targetFolderGuid: row.getResultByIndex(8), targetFolderItemId: row.getResultByIndex(9), targetFolderTitle: row.getResultByIndex(10), - }, - ]; - }) - ); + }); + } + ); + } + return entries; } ); } diff --git a/toolkit/components/places/Database.cpp b/toolkit/components/places/Database.cpp index 891c53156d..56da4094c7 100644 --- a/toolkit/components/places/Database.cpp +++ b/toolkit/components/places/Database.cpp @@ -1260,6 +1260,15 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) { // Firefox 118 uses schema version 75 + // Version 76 was not correctly invoked and thus removed. + + if (currentSchemaVersion < 77) { + rv = MigrateV77Up(); + NS_ENSURE_SUCCESS(rv, rv); + } + + // Firefox 125 uses schema version 77 + // Schema Upgrades must add migration code here. // >>> IMPORTANT! <<< // NEVER MIX UP SYNC AND ASYNC EXECUTION IN MIGRATORS, YOU MAY LOCK THE @@ -1615,7 +1624,7 @@ nsresult Database::InitFunctions() { NS_ENSURE_SUCCESS(rv, rv); rv = InvalidateDaysOfHistoryFunction::create(mMainConn); NS_ENSURE_SUCCESS(rv, rv); - rv = MD5HexFunction::create(mMainConn); + rv = SHA256HexFunction::create(mMainConn); NS_ENSURE_SUCCESS(rv, rv); rv = SetShouldStartFrecencyRecalculationFunction::create(mMainConn); NS_ENSURE_SUCCESS(rv, rv); @@ -2041,6 +2050,15 @@ nsresult Database::MigrateV75Up() { return NS_OK; } +nsresult Database::MigrateV77Up() { + // Recalculate origins frecency. + nsCOMPtr<mozIStorageStatement> stmt; + nsresult rv = mMainConn->ExecuteSimpleSQL( + "UPDATE moz_origins SET recalc_frecency = 1"_ns); + NS_ENSURE_SUCCESS(rv, rv); + return NS_OK; +} + int64_t Database::CreateMobileRoot() { MOZ_ASSERT(NS_IsMainThread()); diff --git a/toolkit/components/places/Database.h b/toolkit/components/places/Database.h index 1798f73eec..755ef26393 100644 --- a/toolkit/components/places/Database.h +++ b/toolkit/components/places/Database.h @@ -308,6 +308,7 @@ class Database final : public nsIObserver, public nsSupportsWeakReference { nsresult MigrateV73Up(); nsresult MigrateV74Up(); nsresult MigrateV75Up(); + nsresult MigrateV77Up(); nsresult UpdateBookmarkRootTitles(); diff --git a/toolkit/components/places/Helpers.cpp b/toolkit/components/places/Helpers.cpp index 90ab263ec1..8cd019b6e1 100644 --- a/toolkit/components/places/Helpers.cpp +++ b/toolkit/components/places/Helpers.cpp @@ -10,6 +10,7 @@ #include "nsReadableUtils.h" #include "nsString.h" #include "nsNavHistory.h" +#include "nsNetUtil.h" #include "mozilla/Base64.h" #include "mozilla/HashFunctions.h" #include "mozilla/RandomNum.h" @@ -378,5 +379,26 @@ nsresult BackupDatabaseFile(nsIFile* aDBFile, const nsAString& aBackupFileName, return aDBFile->CopyTo(parentDir, fileName); } +already_AddRefed<nsIURI> GetExposableURI(nsIURI* aURI) { + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(aURI); + + nsresult rv; + nsCOMPtr<nsIIOService> ioService = do_GetIOService(&rv); + if (NS_FAILED(rv)) { + NS_WARNING("Failed to get nsIIOService"); + return nsCOMPtr<nsIURI>(aURI).forget(); + } + + nsCOMPtr<nsIURI> uri; + rv = ioService->CreateExposableURI(aURI, getter_AddRefs(uri)); + if (NS_FAILED(rv)) { + NS_WARNING("Failed to create exposable URI"); + return nsCOMPtr<nsIURI>(aURI).forget(); + } + + return uri.forget(); +} + } // namespace places } // namespace mozilla diff --git a/toolkit/components/places/Helpers.h b/toolkit/components/places/Helpers.h index 6719fdded2..814322e042 100644 --- a/toolkit/components/places/Helpers.h +++ b/toolkit/components/places/Helpers.h @@ -170,6 +170,14 @@ PRTime RoundedPRNow(); nsresult HashURL(const nsACString& aSpec, const nsACString& aMode, uint64_t* _hash); +/** + * Return exposable URL from given URI. + * + * @param aURI The URI to be converted. + * @return already_AddRefed<nsIURI> The converted, exposable URI. + */ +already_AddRefed<nsIURI> GetExposableURI(nsIURI* aURI); + class QueryKeyValuePair final { public: QueryKeyValuePair(const nsACString& aKey, const nsACString& aValue) { diff --git a/toolkit/components/places/History.cpp b/toolkit/components/places/History.cpp index 81cccdcb55..6c066f47be 100644 --- a/toolkit/components/places/History.cpp +++ b/toolkit/components/places/History.cpp @@ -1434,6 +1434,17 @@ void NotifyEmbedVisit(VisitData& aPlace, (void)NS_DispatchToMainThread(event); } +void NotifyVisitIfHavingUserPass(nsIURI* aURI) { + MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread!"); + + bool hasUserPass; + if (NS_SUCCEEDED(aURI->GetHasUserPass(&hasUserPass)) && hasUserPass) { + nsCOMPtr<nsIRunnable> event = + new NotifyManyVisitsObservers(VisitData(aURI)); + (void)NS_DispatchToMainThread(event); + } +} + //////////////////////////////////////////////////////////////////////////////// //// History @@ -1925,13 +1936,6 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI, NS_ENSURE_SUCCESS(rv, rv); } - nsTArray<VisitData> placeArray(1); - placeArray.AppendElement(VisitData(aURI, aLastVisitedURI)); - VisitData& place = placeArray.ElementAt(0); - NS_ENSURE_FALSE(place.spec.IsEmpty(), NS_ERROR_INVALID_ARG); - - place.visitTime = PR_Now(); - // Assigns a type to the edge in the visit linked list. Each type will be // considered differently when weighting the frecency of a location. uint32_t recentFlags = navHistory->GetRecentFlags(aURI); @@ -1966,18 +1970,8 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI, transitionType = nsINavHistoryService::TRANSITION_FRAMED_LINK; } - place.SetTransitionType(transitionType); bool isRedirect = aFlags & IHistory::REDIRECT_SOURCE; - if (isRedirect) { - place.useFrecencyRedirectBonus = - (aFlags & (IHistory::REDIRECT_SOURCE_PERMANENT | - IHistory::REDIRECT_SOURCE_UPGRADED)) || - transitionType != nsINavHistoryService::TRANSITION_TYPED; - } - place.hidden = GetHiddenState(isRedirect, place.transitionType); - - // Error pages should never be autocompleted. - place.isUnrecoverableError = aFlags & IHistory::UNRECOVERABLE_ERROR; + bool isHidden = GetHiddenState(isRedirect, transitionType); // Do not save a reloaded uri if we have visited the same URI recently. if (reload) { @@ -1988,17 +1982,43 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI, bool wasHidden = entry->mHidden; // Regardless of whether we store the visit or not, we must update the // stored visit time. - AppendToRecentlyVisitedURIs(aURI, place.hidden); + AppendToRecentlyVisitedURIs(aURI, isHidden); // We always want to store an unhidden visit, if the previous visits were // hidden, because otherwise the page may not appear in the history UI. // This can happen for example at a page redirecting to itself. - if (!wasHidden || place.hidden) { + if (!wasHidden || isHidden) { // We can skip this visit. return NS_OK; } } } + // Never store the URL having userpass to database. + nsCOMPtr<nsIURI> visitedURI = GetExposableURI(aURI); + nsCOMPtr<nsIURI> lastVisitedURI; + if (aLastVisitedURI) { + lastVisitedURI = GetExposableURI(aLastVisitedURI); + } + + nsTArray<VisitData> placeArray(1); + placeArray.AppendElement(VisitData(visitedURI, lastVisitedURI)); + VisitData& place = placeArray.ElementAt(0); + NS_ENSURE_FALSE(place.spec.IsEmpty(), NS_ERROR_INVALID_ARG); + + place.visitTime = PR_Now(); + place.SetTransitionType(transitionType); + place.hidden = isHidden; + + if (isRedirect) { + place.useFrecencyRedirectBonus = + (aFlags & (IHistory::REDIRECT_SOURCE_PERMANENT | + IHistory::REDIRECT_SOURCE_UPGRADED)) || + transitionType != nsINavHistoryService::TRANSITION_TYPED; + } + + // Error pages should never be autocompleted. + place.isUnrecoverableError = aFlags & IHistory::UNRECOVERABLE_ERROR; + nsCOMPtr<nsIBrowserWindowTracker> bwt = do_ImportESModule("resource:///modules/BrowserWindowTracker.sys.mjs", "BrowserWindowTracker", &rv); @@ -2067,6 +2087,15 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI, NS_ENSURE_SUCCESS(rv, rv); } + // URIs with a userpass component are not stored in the database for security + // reasons, we store the exposable URI version of them instead. + // The original link pointing at the URI with userpass must still be marked as + // visited, to properly react to the user interaction, so we notify a visit. + // The visited status is not going to survive a reload of course, though the + // alternative of marking any userpass URI as visited if the exposable URI is + // visited also feels wrong from the user point of view. + NotifyVisitIfHavingUserPass(aURI); + return NS_OK; } @@ -2099,8 +2128,9 @@ History::SetURITitle(nsIURI* aURI, const nsAString& aTitle) { // NS_ENSURE_TRUE(navHistory, NS_ERROR_FAILURE); + nsCOMPtr<nsIURI> uri = GetExposableURI(aURI); bool canAdd; - nsresult rv = navHistory->CanAddURI(aURI, &canAdd); + nsresult rv = navHistory->CanAddURI(uri, &canAdd); NS_ENSURE_SUCCESS(rv, rv); if (!canAdd) { return NS_OK; @@ -2109,7 +2139,7 @@ History::SetURITitle(nsIURI* aURI, const nsAString& aTitle) { mozIStorageConnection* dbConn = GetDBConn(); NS_ENSURE_STATE(dbConn); - return SetPageTitle::Start(dbConn, aURI, aTitle); + return SetPageTitle::Start(dbConn, uri, aTitle); } //////////////////////////////////////////////////////////////////////////////// @@ -2135,6 +2165,10 @@ History::UpdatePlaces(JS::Handle<JS::Value> aPlaceInfos, NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr<nsIURI> uri = GetURIFromJSObject(aCtx, info, "uri"); + if (uri) { + uri = GetExposableURI(uri); + } + nsCString guid; { nsString fatGUID; diff --git a/toolkit/components/places/History.sys.mjs b/toolkit/components/places/History.sys.mjs index 3f43ca5a53..fe13043600 100644 --- a/toolkit/components/places/History.sys.mjs +++ b/toolkit/components/places/History.sys.mjs @@ -251,8 +251,9 @@ export var History = Object.freeze({ insert(pageInfo) { let info = lazy.PlacesUtils.validatePageInfo(pageInfo); - return lazy.PlacesUtils.withConnectionWrapper("History.jsm: insert", db => - insert(db, info) + return lazy.PlacesUtils.withConnectionWrapper( + "History.sys.mjs: insert", + db => insert(db, info) ); }, @@ -322,7 +323,7 @@ export var History = Object.freeze({ } return lazy.PlacesUtils.withConnectionWrapper( - "History.jsm: insertMany", + "History.sys.mjs: insertMany", db => insertMany(db, infos, onResult, onError) ); }, @@ -400,7 +401,7 @@ export var History = Object.freeze({ let pages = { guids: guidsSlice, urls: urlsSlice }; let result = await lazy.PlacesUtils.withConnectionWrapper( - "History.jsm: remove", + "History.sys.mjs: remove", db => remove(db, pages, onResult) ); @@ -499,7 +500,7 @@ export var History = Object.freeze({ } return lazy.PlacesUtils.withConnectionWrapper( - "History.jsm: removeVisitsByFilter", + "History.sys.mjs: removeVisitsByFilter", db => removeVisitsByFilter(db, filter, onResult) ); }, @@ -592,7 +593,7 @@ export var History = Object.freeze({ } return lazy.PlacesUtils.withConnectionWrapper( - "History.jsm: removeByFilter", + "History.sys.mjs: removeByFilter", db => removeByFilter(db, filter, onResult) ); }, @@ -644,7 +645,10 @@ export var History = Object.freeze({ * A promise resolved once the operation is complete. */ clear() { - return lazy.PlacesUtils.withConnectionWrapper("History.jsm: clear", clear); + return lazy.PlacesUtils.withConnectionWrapper( + "History.sys.mjs: clear", + clear + ); }, /** @@ -705,7 +709,7 @@ export var History = Object.freeze({ * 1). A null `previewImageURL` will clear the existing value in the * database. * 2). It throws if its length is greater than DB_URL_LENGTH_MAX - * defined in PlacesUtils.jsm. + * defined in PlacesUtils.sys.mjs. * * If a property `annotations` is provided, the annotations will be * updated. Note that: @@ -728,7 +732,7 @@ export var History = Object.freeze({ * If `pageInfo` has neither `description` nor `previewImageURL`. * @throws (Error) * If the length of `pageInfo.previewImageURL` is greater than - * DB_URL_LENGTH_MAX defined in PlacesUtils.jsm. + * DB_URL_LENGTH_MAX defined in PlacesUtils.sys.mjs. */ update(pageInfo) { let info = lazy.PlacesUtils.validatePageInfo(pageInfo, false); @@ -744,8 +748,9 @@ export var History = Object.freeze({ ); } - return lazy.PlacesUtils.withConnectionWrapper("History.jsm: update", db => - update(db, info) + return lazy.PlacesUtils.withConnectionWrapper( + "History.sys.mjs: update", + db => update(db, info) ); }, diff --git a/toolkit/components/places/PlacesBackups.sys.mjs b/toolkit/components/places/PlacesBackups.sys.mjs index 066eef4ec9..02b105e054 100644 --- a/toolkit/components/places/PlacesBackups.sys.mjs +++ b/toolkit/components/places/PlacesBackups.sys.mjs @@ -15,7 +15,7 @@ ChromeUtils.defineLazyGetter( lazy, "filenamesRegex", () => - /^bookmarks-([0-9-]+)(?:_([0-9]+)){0,1}(?:_([a-z0-9=+-]{24})){0,1}\.(json(lz4)?)$/i + /^bookmarks-([0-9-]+)(?:_([0-9]+)){0,1}(?:_([a-z0-9=_+-]{24,})){0,1}\.(json(lz4)?)$/i ); async function limitBackups(aMaxBackups, backupFiles) { diff --git a/toolkit/components/places/PlacesDBUtils.sys.mjs b/toolkit/components/places/PlacesDBUtils.sys.mjs index 5cc2bfc631..e9a13ac10c 100644 --- a/toolkit/components/places/PlacesDBUtils.sys.mjs +++ b/toolkit/components/places/PlacesDBUtils.sys.mjs @@ -874,7 +874,7 @@ export var PlacesDBUtils = { ); let returnPromise = new Promise(res => { - let observer = (subject, topic, data) => { + let observer = (subject, topic) => { Services.obs.removeObserver(observer, topic); logs.push("Database cleaned up"); res(logs); @@ -1377,7 +1377,7 @@ async function integrity(dbName) { export function PlacesDBUtilsIdleMaintenance() {} PlacesDBUtilsIdleMaintenance.prototype = { - observe(subject, topic, data) { + observe(subject, topic) { switch (topic) { case "idle-daily": // Once a week run places.sqlite maintenance tasks. diff --git a/toolkit/components/places/PlacesExpiration.sys.mjs b/toolkit/components/places/PlacesExpiration.sys.mjs index 4db494d63e..2621decdc5 100644 --- a/toolkit/components/places/PlacesExpiration.sys.mjs +++ b/toolkit/components/places/PlacesExpiration.sys.mjs @@ -435,7 +435,7 @@ export function nsPlacesExpiration() { ); this._dbInitializedPromise = lazy.PlacesUtils.withConnectionWrapper( - "PlacesExpiration.jsm: setup", + "PlacesExpiration.sys.mjs: setup", async db => { await db.execute( `CREATE TEMP TABLE expiration_notify ( @@ -758,7 +758,7 @@ nsPlacesExpiration.prototype = { try { let notifications = []; await lazy.PlacesUtils.withConnectionWrapper( - "PlacesExpiration.jsm: expire", + "PlacesExpiration.sys.mjs: expire", async db => { await db.executeTransaction(async () => { for (let queryType in EXPIRATION_QUERIES) { diff --git a/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs b/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs index 2de558af34..7c52c508ee 100644 --- a/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs +++ b/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs @@ -51,6 +51,14 @@ XPCOMUtils.defineLazyPreferenceGetter( 90 ); +// For origins frecency calculation only sample pages visited recently. +XPCOMUtils.defineLazyPreferenceGetter( + lazy, + "originsFrecencyCutOffDays", + "places.frecency.originsCutOffDays", + 90 +); + // Time between deferred task executions. const DEFERRED_TASK_INTERVAL_MS = 2 * 60000; // Maximum time to wait for an idle before the task is executed anyway. @@ -218,16 +226,19 @@ export class PlacesFrecencyRecalculator { let affectedCount = 0; let db = await lazy.PlacesUtils.promiseUnsafeWritableDBConnection(); await db.executeTransaction(async () => { + // NULL frecencies are normalized to 1.0 (to avoid confusion with pages + // 0 frecency special meaning), as the table doesn't support NULL values. let affected = await db.executeCached( ` UPDATE moz_origins - SET frecency = CAST( - (SELECT total(frecency) - FROM moz_places h - WHERE origin_id = moz_origins.id AND frecency > 0) - AS INT - ), - recalc_frecency = 0 + SET frecency = IFNULL(( + SELECT sum(frecency) + FROM moz_places h + WHERE origin_id = moz_origins.id + AND last_visit_date > + strftime('%s','now','localtime','start of day', + '-${lazy.originsFrecencyCutOffDays} day','utc') * 1000000 + ), 1.0), recalc_frecency = 0 WHERE id IN ( SELECT id FROM moz_origins WHERE recalc_frecency = 1 @@ -238,25 +249,19 @@ export class PlacesFrecencyRecalculator { ); affectedCount += affected.length; - // Calculate and store the frecency statistics used to calculate a - // thredhold. Origins above that threshold will be considered meaningful - // and autofilled. + // Calculate and store the frecency threshold. Origins whose frecency is + // above this value will be considered meaningful and autofilled. // While it may be tempting to do this only when some frecency was // updated, that won't catch the edge case of the moz_origins table being // emptied. - let row = ( - await db.executeCached(` - SELECT count(*), total(frecency), total(pow(frecency,2)) - FROM moz_origins - WHERE frecency > 0 - `) - )[0]; - await lazy.PlacesUtils.metadata.setMany( - new Map([ - ["origin_frecency_count", row.getResultByIndex(0)], - ["origin_frecency_sum", row.getResultByIndex(1)], - ["origin_frecency_sum_of_squares", row.getResultByIndex(2)], - ]) + // In case of NULL, the default threshold is 2, that is higher than the + // default frecency set above. + let threshold = ( + await db.executeCached(`SELECT avg(frecency) FROM moz_origins`) + )[0].getResultByIndex(0); + await lazy.PlacesUtils.metadata.set( + "origin_frecency_threshold", + threshold ?? 2 ); }); @@ -343,7 +348,7 @@ export class PlacesFrecencyRecalculator { } } - observe(subject, topic, data) { + observe(subject, topic) { lazy.logger.trace(`Got ${topic} topic`); switch (topic) { case "idle-daily": @@ -524,7 +529,7 @@ class AlternativeFrecencyHelper { return affected; } - async #recalculateSomePagesAlternativeFrecencies({ chunkSize, variables }) { + async #recalculateSomePagesAlternativeFrecencies({ chunkSize }) { lazy.logger.trace( `Recalculate ${chunkSize} alternative pages frecency values` ); diff --git a/toolkit/components/places/PlacesPreviews.sys.mjs b/toolkit/components/places/PlacesPreviews.sys.mjs index 08ee94664a..e19bf2a55c 100644 --- a/toolkit/components/places/PlacesPreviews.sys.mjs +++ b/toolkit/components/places/PlacesPreviews.sys.mjs @@ -14,13 +14,8 @@ ChromeUtils.defineESModuleGetters(lazy, { setTimeout: "resource://gre/modules/Timer.sys.mjs", }); -ChromeUtils.defineLazyGetter(lazy, "logConsole", function () { - return console.createInstance({ - prefix: "PlacesPreviews", - maxLogLevel: Services.prefs.getBoolPref("places.previews.log", false) - ? "Debug" - : "Warn", - }); +ChromeUtils.defineLazyGetter(lazy, "logger", function () { + return lazy.PlacesUtils.getLogger({ prefix: "Previews" }); }); // Toggling Places previews requires a restart, because a database trigger @@ -90,7 +85,7 @@ class DeletionHandler { constructor() { // Clear any pending timeouts on shutdown. lazy.PlacesUtils.history.shutdownClient.jsclient.addBlocker( - "PlacesPreviews.jsm::DeletionHandler", + "PlacesPreviews.sys.mjs::DeletionHandler", async () => { this.#shutdownProgress.shuttingDown = true; lazy.clearTimeout(this.#timeoutId); @@ -112,7 +107,7 @@ class DeletionHandler { this.#timeoutId = null; ChromeUtils.idleDispatch(() => { this.#deleteChunk().catch(ex => - lazy.logConsole.error("Error during previews deletion:" + ex) + lazy.logger.error("Error during previews deletion:" + ex) ); }); }, this.timeout); @@ -156,7 +151,7 @@ class DeletionHandler { if (DOMException.isInstance(ex) && ex.name == "NotFoundError") { deleted.push(hash); } else { - lazy.logConsole.error("Unable to delete file: " + filePath); + lazy.logger.error("Unable to delete file: " + filePath); } } if (this.#shutdownProgress.shuttingDown) { @@ -169,7 +164,7 @@ class DeletionHandler { return p; }, {}); await lazy.PlacesUtils.withConnectionWrapper( - "PlacesPreviews.jsm::ExpirePreviews", + "PlacesPreviews.sys.mjs::ExpirePreviews", async db => { await db.execute( `DELETE FROM moz_previews_tombstones WHERE hash in @@ -189,7 +184,7 @@ class DeletionHandler { /** * Handles previews for Places urls. - * Previews are stored in WebP format, using MD5 hash of the page url in hex + * Previews are stored in WebP format, using SHA256 hash of the page url in hex * format. All the previews are saved into a "places-previews" folder under * the roaming profile folder. */ @@ -260,13 +255,13 @@ export const PlacesPreviews = new (class extends EventEmitter { getPathForUrl(url) { return PathUtils.join( this.getPath(), - lazy.PlacesUtils.md5(url, { format: "hex" }) + this.fileExtension + lazy.PlacesUtils.sha256(url, { format: "hex" }) + this.fileExtension ); } /** * Returns the file path of the preview having the given hash. - * @param {string} hash md5 hash in hex format. + * @param {string} hash SHA256 hash in hex format. * @returns {string } File path of the preview having the given hash. */ getPathForHash(hash) { @@ -293,7 +288,7 @@ export const PlacesPreviews = new (class extends EventEmitter { * Updates the preview for the given page url. The update happens in * background, using a windowless browser with very conservative privacy * settings. Due to this, it may not look exactly like the page that the user - * is normally facing when logged in. See BackgroundPageThumbs.jsm for + * is normally facing when logged in. See BackgroundPageThumbs.sys.mjs for * additional details. * Unless `forceUpdate` is set, the preview is not updated if: * - It was already fetched recently @@ -310,7 +305,7 @@ export const PlacesPreviews = new (class extends EventEmitter { let filePath = this.getPathForUrl(url); if (!forceUpdate) { if (this.#recentlyUpdatedPreviews.has(filePath)) { - lazy.logConsole.debug("Skipping update because recently updated"); + lazy.logger.debug("Skipping update because recently updated"); return true; } try { @@ -321,13 +316,13 @@ export const PlacesPreviews = new (class extends EventEmitter { ) { // File is recent enough. this.#recentlyUpdatedPreviews.add(filePath); - lazy.logConsole.debug("Skipping update because file is recent"); + lazy.logger.debug("Skipping update because file is recent"); return true; } } catch (ex) { // If the file doesn't exist, we always update it. if (!DOMException.isInstance(ex) || ex.name != "NotFoundError") { - lazy.logConsole.error("Error while trying to stat() preview" + ex); + lazy.logger.error("Error while trying to stat() preview" + ex); return false; } } @@ -350,7 +345,7 @@ export const PlacesPreviews = new (class extends EventEmitter { }); }); if (!buffer) { - lazy.logConsole.error("Unable to fetch preview: " + url); + lazy.logger.error("Unable to fetch preview: " + url); return false; } try { @@ -359,9 +354,7 @@ export const PlacesPreviews = new (class extends EventEmitter { tmpPath: filePath + ".tmp", }); } catch (ex) { - lazy.logConsole.error( - lazy.logConsole.error("Unable to create preview: " + ex) - ); + lazy.logger.error("Unable to create preview: " + ex); return false; } this.#recentlyUpdatedPreviews.add(filePath); @@ -388,11 +381,11 @@ export const PlacesPreviews = new (class extends EventEmitter { let files = await IOUtils.getChildren(this.getPath()); let hashes = files .map(f => PathUtils.filename(f)) - .filter(n => /^[a-f0-9]{32}\.webp$/) + .filter(() => /^[a-f0-9]{32}\.webp$/) .map(n => n.substring(0, n.lastIndexOf("."))); await lazy.PlacesUtils.withConnectionWrapper( - "PlacesPreviews.jsm::deleteOrphans", + "PlacesPreviews.sys.mjs::deleteOrphans", async db => { await db.execute( ` @@ -402,7 +395,7 @@ export const PlacesPreviews = new (class extends EventEmitter { INSERT OR IGNORE INTO moz_previews_tombstones SELECT hash FROM files EXCEPT - SELECT md5hex(url) FROM moz_places + SELECT sha256hex(url) FROM moz_places ` ); } diff --git a/toolkit/components/places/PlacesSyncUtils.sys.mjs b/toolkit/components/places/PlacesSyncUtils.sys.mjs index 6da1b91243..caa5054885 100644 --- a/toolkit/components/places/PlacesSyncUtils.sys.mjs +++ b/toolkit/components/places/PlacesSyncUtils.sys.mjs @@ -11,7 +11,7 @@ ChromeUtils.defineESModuleGetters(lazy, { /** * This module exports functions for Sync to use when applying remote - * records. The calls are similar to those in `Bookmarks.jsm` and + * records. The calls are similar to those in `Bookmarks.sys.mjs` and * `nsINavBookmarksService`, with special handling for * tags, keywords, synced annotations, and missing parents. */ @@ -1405,7 +1405,7 @@ function validateChangeRecord(name, changeRecord, behavior) { } // Similar to the private `fetchBookmarksByParent` implementation in -// `Bookmarks.jsm`. +// `Bookmarks.sys.mjs`. var fetchChildGuids = async function (db, parentGuid) { let rows = await db.executeCached( ` diff --git a/toolkit/components/places/PlacesTransactions.sys.mjs b/toolkit/components/places/PlacesTransactions.sys.mjs index 0c9accd3ba..bd0b7654a3 100644 --- a/toolkit/components/places/PlacesTransactions.sys.mjs +++ b/toolkit/components/places/PlacesTransactions.sys.mjs @@ -177,13 +177,7 @@ function setTimeout(callback, ms) { const lazy = {}; ChromeUtils.defineLazyGetter(lazy, "logger", function () { - return console.createInstance({ - prefix: "PlacesTransactions", - maxLogLevel: Services.prefs.getCharPref( - "places.transactions.logLevel", - "Error" - ), - }); + return PlacesUtils.getLogger({ prefix: "Transactions" }); }); class TransactionsHistoryArray extends Array { diff --git a/toolkit/components/places/PlacesUtils.sys.mjs b/toolkit/components/places/PlacesUtils.sys.mjs index aeebedd31a..8eeb4f94d8 100644 --- a/toolkit/components/places/PlacesUtils.sys.mjs +++ b/toolkit/components/places/PlacesUtils.sys.mjs @@ -12,7 +12,6 @@ const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { Bookmarks: "resource://gre/modules/Bookmarks.sys.mjs", History: "resource://gre/modules/History.sys.mjs", - Log: "resource://gre/modules/Log.sys.mjs", PlacesSyncUtils: "resource://gre/modules/PlacesSyncUtils.sys.mjs", Sqlite: "resource://gre/modules/Sqlite.sys.mjs", }); @@ -817,7 +816,7 @@ export var PlacesUtils = { }, // nsIObserver - observe: function PU_observe(aSubject, aTopic, aData) { + observe: function PU_observe(aSubject, aTopic) { switch (aTopic) { case this.TOPIC_SHUTDOWN: Services.obs.removeObserver(this, this.TOPIC_SHUTDOWN); @@ -1181,7 +1180,7 @@ export var PlacesUtils = { { url: { requiredIf: b => !b.guid }, guid: { requiredIf: b => !b.url }, - visits: { requiredIf: b => validateVisits }, + visits: { requiredIf: () => validateVisits }, } ); }, @@ -1830,14 +1829,13 @@ export var PlacesUtils = { * Run some text through md5 and return the hash. * @param {string} data The string to hash. * @param {string} [format] Which format of the hash to return: - * - "ascii" for ascii format. + * - "base64" for ascii format. * - "hex" for hex format. - * @returns {string} hash of the input string in the required format. + * @returns {string} hash of the input data in the required format. * @deprecated use sha256 instead. */ - md5(data, { format = "ascii" } = {}) { + md5(data, { format = "base64" } = {}) { let hasher = new lazy.CryptoHash("md5"); - // Convert the data to a byte array for hashing. data = new TextEncoder().encode(data); hasher.update(data, data.length); @@ -1847,7 +1845,7 @@ export var PlacesUtils = { return Array.from(hash, (c, i) => hash.charCodeAt(i).toString(16).padStart(2, "0") ).join(""); - case "ascii": + case "base64": default: return hasher.finish(true); } @@ -1855,25 +1853,35 @@ export var PlacesUtils = { /** * Run some text through SHA256 and return the hash. - * @param {string} data The string to hash. + * @param {string|nsIStringInputStream} data The data to hash. * @param {string} [format] Which format of the hash to return: - * - "ascii" for ascii format. + * - "base64" (default) for ascii format, not safe for URIs or file names. * - "hex" for hex format. - * @returns {string} hash of the input string in the required format. + * - "base64url" for ascii format safe to be used in file names (RFC 4648). + * You should normally use the "hex" format for file names, but if the + * user may manipulate the file, it would be annoying to have very long + * and unreadable file names, thus this provides a shorter alternative. + * Note padding "=" are untouched and may have to be encoded in URIs. + * @returns {string} hash of the input data in the required format. */ - sha256(data, { format = "ascii" } = {}) { + sha256(data, { format = "base64" } = {}) { let hasher = new lazy.CryptoHash("sha256"); - - // Convert the data to a byte array for hashing. - data = new TextEncoder().encode(data); - hasher.update(data, data.length); + if (data instanceof Ci.nsIStringInputStream) { + hasher.updateFromStream(data, -1); + } else { + // Convert the data string to a byte array for hashing. + data = new TextEncoder().encode(data); + hasher.update(data, data.length); + } switch (format) { case "hex": let hash = hasher.finish(false); return Array.from(hash, (c, i) => hash.charCodeAt(i).toString(16).padStart(2, "0") ).join(""); - case "ascii": + case "base64url": + return hasher.finish(true).replaceAll("+", "-").replaceAll("/", "_"); + case "base64": default: return hasher.finish(true); } @@ -1950,31 +1958,26 @@ export var PlacesUtils = { }, /** - * Creates a logger. - * Logging level can be controlled through places.loglevel. + * Creates a console logger. + * Logging level can be controlled through the `places.loglevel` preference. * - * @param {string} [prefix] Prefix to use for the logged messages, "::" will - * be appended automatically to the prefix. - * @returns {object} The logger. + * @param {object} options + * @param {string} [options.prefix] Prefix to use for the logged messages. + * @returns {ConsoleInstance} The console logger. */ getLogger({ prefix = "" } = {}) { - if (!this._logger) { - this._logger = lazy.Log.repository.getLogger("places"); - this._logger.manageLevelFromPref("places.loglevel"); - this._logger.addAppender( - new lazy.Log.ConsoleAppender(new lazy.Log.BasicFormatter()) - ); - } - if (prefix) { - // This is not an early return because it is necessary to invoke getLogger - // at least once before getLoggerWithMessagePrefix; it replaces a - // method of the original logger, rather than using an actual Proxy. - return lazy.Log.repository.getLoggerWithMessagePrefix( - "places", - prefix + " :: " - ); + if (!this._loggers) { + this._loggers = new Map(); + } + let logger = this._loggers.get(prefix); + if (!logger) { + logger = console.createInstance({ + prefix: `Places${prefix ? " - " + prefix : ""}`, + maxLogLevelPref: "places.loglevel", + }); + this._loggers.set(prefix, logger); } - return this._logger; + return logger; }, }; @@ -2300,7 +2303,7 @@ PlacesUtils.metadata = { } return true; }) - .map(([key, value]) => key); + .map(([key]) => key); if (keysToDelete.length) { await this.deleteWithConnection(db, ...keysToDelete); if (keysToDelete.length == pairs.size) { diff --git a/toolkit/components/places/SQLFunctions.cpp b/toolkit/components/places/SQLFunctions.cpp index e625f3fa09..85c5cc8d17 100644 --- a/toolkit/components/places/SQLFunctions.cpp +++ b/toolkit/components/places/SQLFunctions.cpp @@ -1135,7 +1135,7 @@ GetQueryParamFunction::OnFunctionCall(mozIStorageValueArray* aArguments, RefPtr<nsVariant> result = new nsVariant(); if (!queryString.IsEmpty() && !paramName.IsEmpty()) { URLParams::Parse( - queryString, + queryString, true, [¶mName, &result](const nsAString& aName, const nsAString& aValue) { NS_ConvertUTF16toUTF8 name(aName); if (!paramName.Equals(name)) { @@ -1191,19 +1191,19 @@ HashFunction::OnFunctionCall(mozIStorageValueArray* aArguments, } //////////////////////////////////////////////////////////////////////////////// -//// MD5 Function +//// SHA256Hex Function /* static */ -nsresult MD5HexFunction::create(mozIStorageConnection* aDBConn) { - RefPtr<MD5HexFunction> function = new MD5HexFunction(); - return aDBConn->CreateFunction("md5hex"_ns, -1, function); +nsresult SHA256HexFunction::create(mozIStorageConnection* aDBConn) { + RefPtr<SHA256HexFunction> function = new SHA256HexFunction(); + return aDBConn->CreateFunction("sha256hex"_ns, -1, function); } -NS_IMPL_ISUPPORTS(MD5HexFunction, mozIStorageFunction) +NS_IMPL_ISUPPORTS(SHA256HexFunction, mozIStorageFunction) NS_IMETHODIMP -MD5HexFunction::OnFunctionCall(mozIStorageValueArray* aArguments, - nsIVariant** _result) { +SHA256HexFunction::OnFunctionCall(mozIStorageValueArray* aArguments, + nsIVariant** _result) { // Must have non-null function arguments. MOZ_ASSERT(aArguments); @@ -1217,8 +1217,8 @@ MD5HexFunction::OnFunctionCall(mozIStorageValueArray* aArguments, nsCOMPtr<nsICryptoHash> hasher = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); - // MD5 is not a secure hash function, but it's ok for this use. - rv = hasher->Init(nsICryptoHash::MD5); + // SHA256 is not super strong but fine for our mapping needs. + rv = hasher->Init(nsICryptoHash::SHA256); NS_ENSURE_SUCCESS(rv, rv); rv = hasher->Update(reinterpret_cast<const uint8_t*>(str.BeginReading()), diff --git a/toolkit/components/places/SQLFunctions.h b/toolkit/components/places/SQLFunctions.h index 0b0dbca970..923172a2d7 100644 --- a/toolkit/components/places/SQLFunctions.h +++ b/toolkit/components/places/SQLFunctions.h @@ -412,17 +412,17 @@ class HashFunction final : public mozIStorageFunction { }; //////////////////////////////////////////////////////////////////////////////// -//// MD5 Function +//// SHA256Hex Function /** - * Calculates md5 hash for a given string. + * Calculates SHA256 hash for a given string in hex format. * * @param string * A string. * @return * The hash for the string. */ -class MD5HexFunction final : public mozIStorageFunction { +class SHA256HexFunction final : public mozIStorageFunction { public: NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_MOZISTORAGEFUNCTION @@ -436,7 +436,7 @@ class MD5HexFunction final : public mozIStorageFunction { static nsresult create(mozIStorageConnection* aDBConn); private: - ~MD5HexFunction() = default; + ~SHA256HexFunction() = default; }; //////////////////////////////////////////////////////////////////////////////// diff --git a/toolkit/components/places/Shutdown.h b/toolkit/components/places/Shutdown.h index 3e5612a454..0a9674bf06 100644 --- a/toolkit/components/places/Shutdown.h +++ b/toolkit/components/places/Shutdown.h @@ -28,8 +28,8 @@ class Database; * PHASE 2 (Modern clients shutdown) * Modern clients should instead register as a blocker by passing a promise to - * nsINavHistoryService::shutdownClient (for example see Sanitizer.jsm), so they - * block Places shutdown until the promise is resolved. + * nsINavHistoryService::shutdownClient (for example see Sanitizer.sys.mjs), so + * they block Places shutdown until the promise is resolved. * When profile-change-teardown is observed by async shutdown, it calls * ClientsShutdownBlocker::BlockShutdown. This class is registered as a teardown * phase blocker in Database::Init (see Database::mClientsShutdown). diff --git a/toolkit/components/places/SyncedBookmarksMirror.sys.mjs b/toolkit/components/places/SyncedBookmarksMirror.sys.mjs index 31688928b3..a09e1b7eb3 100644 --- a/toolkit/components/places/SyncedBookmarksMirror.sys.mjs +++ b/toolkit/components/places/SyncedBookmarksMirror.sys.mjs @@ -666,7 +666,7 @@ export class SyncedBookmarksMirror { "mozISyncedBookmarksMirrorCallback", ]), // `mozISyncedBookmarksMirrorProgressListener` methods. - onFetchLocalTree: (took, itemCount, deleteCount, problemsBag) => { + onFetchLocalTree: (took, itemCount, deleteCount) => { let counts = [ { name: "items", diff --git a/toolkit/components/places/TaggingService.sys.mjs b/toolkit/components/places/TaggingService.sys.mjs index e27c84f844..0c038e6b0a 100644 --- a/toolkit/components/places/TaggingService.sys.mjs +++ b/toolkit/components/places/TaggingService.sys.mjs @@ -321,7 +321,7 @@ TaggingService.prototype = { }, // nsIObserver - observe: function TS_observe(aSubject, aTopic, aData) { + observe: function TS_observe(aSubject, aTopic) { if (aTopic == TOPIC_SHUTDOWN) { PlacesUtils.observers.removeListener( [ diff --git a/toolkit/components/places/moz.build b/toolkit/components/places/moz.build index a0513ec341..28a7421fba 100644 --- a/toolkit/components/places/moz.build +++ b/toolkit/components/places/moz.build @@ -60,6 +60,7 @@ if CONFIG["MOZ_PLACES"]: EXTRA_JS_MODULES += [ "BookmarkHTMLUtils.sys.mjs", "BookmarkJSONUtils.sys.mjs", + "BookmarkList.sys.mjs", "Bookmarks.sys.mjs", "ExtensionSearchHandler.sys.mjs", "History.sys.mjs", diff --git a/toolkit/components/places/mozIAsyncHistory.idl b/toolkit/components/places/mozIAsyncHistory.idl index 90c32b1090..88690b4798 100644 --- a/toolkit/components/places/mozIAsyncHistory.idl +++ b/toolkit/components/places/mozIAsyncHistory.idl @@ -128,11 +128,11 @@ interface mozIVisitedStatusCallback : nsISupports /** * This interface contains APIs for cpp consumers. - * Javascript consumers should look at History.jsm instead, + * Javascript consumers should look at History.sys.mjs instead, * that is exposed through PlacesUtils.history. * * If you're evaluating adding a new history API, it should - * usually go to History.jsm, unless it needs to do long and + * usually go to History.sys.mjs, unless it needs to do long and * expensive work in a batch, then it could be worth doing * that in History.cpp. */ diff --git a/toolkit/components/places/nsFaviconService.cpp b/toolkit/components/places/nsFaviconService.cpp index 51d20236c3..c236d7a680 100644 --- a/toolkit/components/places/nsFaviconService.cpp +++ b/toolkit/components/places/nsFaviconService.cpp @@ -269,6 +269,9 @@ nsFaviconService::SetAndFetchFaviconForPage( NS_ENSURE_ARG(aFaviconURI); NS_ENSURE_ARG_POINTER(_canceler); + nsCOMPtr<nsIURI> pageURI = GetExposableURI(aPageURI); + nsCOMPtr<nsIURI> faviconURI = GetExposableURI(aFaviconURI); + nsCOMPtr<nsIPrincipal> loadingPrincipal = aLoadingPrincipal; MOZ_ASSERT(loadingPrincipal, "please provide aLoadingPrincipal for this favicon"); @@ -291,33 +294,33 @@ nsFaviconService::SetAndFetchFaviconForPage( // Build page data. PageData page; - nsresult rv = aPageURI->GetSpec(page.spec); + nsresult rv = pageURI->GetSpec(page.spec); NS_ENSURE_SUCCESS(rv, rv); // URIs can arguably lack a host. - Unused << aPageURI->GetHost(page.host); + Unused << pageURI->GetHost(page.host); if (StringBeginsWith(page.host, "www."_ns)) { page.host.Cut(0, 4); } bool canAddToHistory; nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); NS_ENSURE_TRUE(navHistory, NS_ERROR_OUT_OF_MEMORY); - rv = navHistory->CanAddURI(aPageURI, &canAddToHistory); + rv = navHistory->CanAddURI(pageURI, &canAddToHistory); NS_ENSURE_SUCCESS(rv, rv); page.canAddToHistory = !!canAddToHistory && !loadPrivate; // Build icon data. IconData icon; // If we have an in-memory icon payload, it overwrites the actual request. - UnassociatedIconHashKey* iconKey = mUnassociatedIcons.GetEntry(aFaviconURI); + UnassociatedIconHashKey* iconKey = mUnassociatedIcons.GetEntry(faviconURI); if (iconKey) { icon = iconKey->iconData; mUnassociatedIcons.RemoveEntry(iconKey); } else { icon.fetchMode = aForceReload ? FETCH_ALWAYS : FETCH_IF_MISSING; - rv = aFaviconURI->GetSpec(icon.spec); + rv = faviconURI->GetSpec(icon.spec); NS_ENSURE_SUCCESS(rv, rv); // URIs can arguably lack a host. - Unused << aFaviconURI->GetHost(icon.host); + Unused << faviconURI->GetHost(icon.host); if (StringBeginsWith(icon.host, "www."_ns)) { icon.host.Cut(0, 4); } @@ -327,9 +330,8 @@ nsFaviconService::SetAndFetchFaviconForPage( // is just /favicon.ico. These icons are considered valid for the whole // origin and expired with the origin through a trigger. nsAutoCString path; - if (NS_SUCCEEDED(aFaviconURI->GetPathQueryRef(path)) && - !icon.host.IsEmpty() && icon.host.Equals(page.host) && - path.EqualsLiteral("/favicon.ico")) { + if (NS_SUCCEEDED(faviconURI->GetPathQueryRef(path)) && !icon.host.IsEmpty() && + icon.host.Equals(page.host) && path.EqualsLiteral("/favicon.ico")) { icon.rootIcon = 1; } @@ -370,13 +372,15 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI, NS_ENSURE_ARG(imgLoader::SupportImageWithMimeType( aMimeType, AcceptedMimeTypes::IMAGES_AND_DOCUMENTS)); + nsCOMPtr<nsIURI> faviconURI = GetExposableURI(aFaviconURI); + PRTime now = PR_Now(); if (aExpiration < now + MIN_FAVICON_EXPIRATION) { // Invalid input, just use the default. aExpiration = now + MAX_FAVICON_EXPIRATION; } - UnassociatedIconHashKey* iconKey = mUnassociatedIcons.PutEntry(aFaviconURI); + UnassociatedIconHashKey* iconKey = mUnassociatedIcons.PutEntry(faviconURI); if (!iconKey) { return NS_ERROR_OUT_OF_MEMORY; } @@ -397,10 +401,10 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI, iconData->expiration = aExpiration; iconData->status = ICON_STATUS_CACHED; iconData->fetchMode = FETCH_NEVER; - nsresult rv = aFaviconURI->GetSpec(iconData->spec); + nsresult rv = faviconURI->GetSpec(iconData->spec); NS_ENSURE_SUCCESS(rv, rv); // URIs can arguably lack a host. - Unused << aFaviconURI->GetHost(iconData->host); + Unused << faviconURI->GetHost(iconData->host); if (StringBeginsWith(iconData->host, "www."_ns)) { iconData->host.Cut(0, 4); } @@ -428,7 +432,7 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI, if ((*iconData).payloads.Length() == 0) { // We cannot optimize this favicon size and we are over the maximum size // allowed, so we will not save data to the db to avoid bloating it. - mUnassociatedIcons.RemoveEntry(aFaviconURI); + mUnassociatedIcons.RemoveEntry(faviconURI); return NS_ERROR_FAILURE; } @@ -544,8 +548,10 @@ nsFaviconService::GetFaviconURLForPage(nsIURI* aPageURI, aPreferredWidth = mDefaultIconURIPreferredSize; } + nsCOMPtr<nsIURI> pageURI = GetExposableURI(aPageURI); + nsAutoCString pageSpec; - nsresult rv = aPageURI->GetSpec(pageSpec); + nsresult rv = pageURI->GetSpec(pageSpec); NS_ENSURE_SUCCESS(rv, rv); nsAutoCString pageHost; // It's expected that some domains may not have a host. @@ -573,12 +579,14 @@ nsFaviconService::GetFaviconDataForPage(nsIURI* aPageURI, aPreferredWidth = mDefaultIconURIPreferredSize; } + nsCOMPtr<nsIURI> pageURI = GetExposableURI(aPageURI); + nsAutoCString pageSpec; - nsresult rv = aPageURI->GetSpec(pageSpec); + nsresult rv = pageURI->GetSpec(pageSpec); NS_ENSURE_SUCCESS(rv, rv); nsAutoCString pageHost; // It's expected that some domains may not have a host. - Unused << aPageURI->GetHost(pageHost); + Unused << pageURI->GetHost(pageHost); RefPtr<AsyncGetFaviconDataForPage> event = new AsyncGetFaviconDataForPage( pageSpec, pageHost, aPreferredWidth, aCallback); @@ -601,17 +609,20 @@ nsFaviconService::CopyFavicons(nsIURI* aFromPageURI, nsIURI* aToPageURI, aFaviconLoadType <= nsIFaviconService::FAVICON_LOAD_NON_PRIVATE, NS_ERROR_INVALID_ARG); + nsCOMPtr<nsIURI> fromPageURI = GetExposableURI(aFromPageURI); + nsCOMPtr<nsIURI> toPageURI = GetExposableURI(aToPageURI); + PageData fromPage; - nsresult rv = aFromPageURI->GetSpec(fromPage.spec); + nsresult rv = fromPageURI->GetSpec(fromPage.spec); NS_ENSURE_SUCCESS(rv, rv); PageData toPage; - rv = aToPageURI->GetSpec(toPage.spec); + rv = toPageURI->GetSpec(toPage.spec); NS_ENSURE_SUCCESS(rv, rv); bool canAddToHistory; nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); NS_ENSURE_TRUE(navHistory, NS_ERROR_OUT_OF_MEMORY); - rv = navHistory->CanAddURI(aToPageURI, &canAddToHistory); + rv = navHistory->CanAddURI(toPageURI, &canAddToHistory); NS_ENSURE_SUCCESS(rv, rv); toPage.canAddToHistory = !!canAddToHistory && @@ -636,6 +647,8 @@ nsresult nsFaviconService::GetFaviconLinkForIcon(nsIURI* aFaviconURI, nsAutoCString spec; if (aFaviconURI) { + nsCOMPtr<nsIURI> faviconURI = GetExposableURI(aFaviconURI); + // List of protocols for which it doesn't make sense to generate a favicon // uri since they can be directly loaded from disk or memory. static constexpr nsLiteralCString sDirectRequestProtocols[] = { @@ -651,15 +664,15 @@ nsresult nsFaviconService::GetFaviconLinkForIcon(nsIURI* aFaviconURI, // clang-format on }; nsAutoCString iconURIScheme; - if (NS_SUCCEEDED(aFaviconURI->GetScheme(iconURIScheme)) && + if (NS_SUCCEEDED(faviconURI->GetScheme(iconURIScheme)) && std::find(std::begin(sDirectRequestProtocols), std::end(sDirectRequestProtocols), iconURIScheme) != std::end(sDirectRequestProtocols)) { // Just return the input URL. - *_retval = do_AddRef(aFaviconURI).take(); + *_retval = do_AddRef(faviconURI).take(); return NS_OK; } - nsresult rv = aFaviconURI->GetSpec(spec); + nsresult rv = faviconURI->GetSpec(spec); NS_ENSURE_SUCCESS(rv, rv); } return GetFaviconLinkForIconString(spec, _retval); @@ -787,6 +800,11 @@ nsresult nsFaviconService::GetFaviconDataAsync( const nsCString& aFaviconSpec, mozIStorageStatementCallback* aCallback) { MOZ_ASSERT(aCallback, "Doesn't make sense to call this without a callback"); + nsCOMPtr<nsIURI> uri; + nsresult rv = NS_NewURI(getter_AddRefs(uri), aFaviconSpec); + NS_ENSURE_SUCCESS(rv, rv); + uri = GetExposableURI(uri); + nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement( "/*Do not warn (bug no: not worth adding an index */ " "SELECT data, width FROM moz_icons " @@ -794,7 +812,7 @@ nsresult nsFaviconService::GetFaviconDataAsync( "ORDER BY width DESC"); NS_ENSURE_STATE(stmt); - nsresult rv = URIBinder::Bind(stmt, "url"_ns, aFaviconSpec); + rv = URIBinder::Bind(stmt, "url"_ns, uri); NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr<mozIStoragePendingStatement> pendingStatement; diff --git a/toolkit/components/places/nsINavHistoryService.idl b/toolkit/components/places/nsINavHistoryService.idl index 5b118911a1..cb9b301410 100644 --- a/toolkit/components/places/nsINavHistoryService.idl +++ b/toolkit/components/places/nsINavHistoryService.idl @@ -903,7 +903,7 @@ interface nsINavHistoryService : nsISupports // The current database schema version. // To migrate to a new version bump this, add a MigrateVXXUp function to // Database.cpp/h, and a test into tests/migration/ - const unsigned long DATABASE_SCHEMA_VERSION = 75; + const unsigned long DATABASE_SCHEMA_VERSION = 77; /** * System Notifications: diff --git a/toolkit/components/places/nsITaggingService.idl b/toolkit/components/places/nsITaggingService.idl index acda63b583..93ee82314a 100644 --- a/toolkit/components/places/nsITaggingService.idl +++ b/toolkit/components/places/nsITaggingService.idl @@ -58,9 +58,3 @@ interface nsITaggingService : nsISupports Array<AString> getTagsForURI(in nsIURI aURI); }; - -%{C++ - -#define TAGGING_SERVICE_CID "@mozilla.org/browser/tagging-service;1" - -%} diff --git a/toolkit/components/places/nsPlacesTriggers.h b/toolkit/components/places/nsPlacesTriggers.h index 0281054c16..c78ca3232c 100644 --- a/toolkit/components/places/nsPlacesTriggers.h +++ b/toolkit/components/places/nsPlacesTriggers.h @@ -112,7 +112,7 @@ "UPDATE moz_origins SET recalc_frecency = 1, recalc_alt_frecency = 1 " \ "WHERE id = OLD.origin_id; " \ "INSERT OR IGNORE INTO moz_previews_tombstones VALUES " \ - "(md5hex(OLD.url));" \ + "(sha256hex(OLD.url));" \ "END ") // This is the supporting table for the "AFTER DELETE ON moz_places" triggers. diff --git a/toolkit/components/places/tests/PlacesTestUtils.sys.mjs b/toolkit/components/places/tests/PlacesTestUtils.sys.mjs index acd1152b44..4e459d2e32 100644 --- a/toolkit/components/places/tests/PlacesTestUtils.sys.mjs +++ b/toolkit/components/places/tests/PlacesTestUtils.sys.mjs @@ -64,6 +64,9 @@ export var PlacesTestUtils = Object.freeze({ let info = { url: place.uri || place.url }; let spec = info.url instanceof Ci.nsIURI ? info.url.spec : new URL(info.url).href; + info.exposableURI = Services.io.createExposableURI( + Services.io.newURI(spec) + ); info.title = "title" in place ? place.title : "test visit for " + spec; let visitDate = place.visitDate; if (visitDate) { @@ -107,7 +110,7 @@ export var PlacesTestUtils = Object.freeze({ } if (lastStoredVisit) { await lazy.TestUtils.waitForCondition( - () => lazy.PlacesUtils.history.fetch(lastStoredVisit.url), + () => lazy.PlacesUtils.history.fetch(lastStoredVisit.exposableURI), "Ensure history has been updated and is visible to read-only connections" ); } @@ -556,7 +559,9 @@ export var PlacesTestUtils = Object.freeze({ * @param {string} field - The name of the field to retrieve a value from. * @param {Object} [conditions] - An object containing the conditions to * filter the query results. The keys represent the names of the columns to - * filter by, and the values represent the filter values. + * filter by, and the values represent the filter values. It's possible to + * pass an array as value where the first element is an operator + * (e.g. "<", ">") and the second element is the actual value. * @return {Promise} A Promise that resolves to the value of the specified * field from the database table, or null if the query returns no results. * @throws If more than one result is found for the given conditions. @@ -579,9 +584,11 @@ export var PlacesTestUtils = Object.freeze({ * conditions. * @param {string} table - The name of the database table to add to. * @param {string} fields - an object with field, value pairs - * @param {Object} [conditions] - An object containing the conditions to filter - * the query results. The keys represent the names of the columns to filter - * by, and the values represent the filter values. + * @param {Object} [conditions] - An object containing the conditions to + * filter the query results. The keys represent the names of the columns to + * filter by, and the values represent the filter values. It's possible to + * pass an array as value where the first element is an operator + * (e.g. "<", ">") and the second element is the actual value. * @return {Promise} A Promise that resolves to the number of affected rows. * @throws If no rows were affected. */ @@ -636,6 +643,11 @@ export var PlacesTestUtils = Object.freeze({ } if (column == "url" && table == "moz_places") { fragments.push("url_hash = hash(:url) AND url = :url"); + } else if (Array.isArray(value)) { + // First element is the operator, second element is the value. + let [op, actualValue] = value; + fragments.push(`${column} ${op} :${column}`); + value = actualValue; } else { fragments.push(`${column} = :${column}`); } diff --git a/toolkit/components/places/tests/bookmarks/test_818584-discard-duplicate-backups.js b/toolkit/components/places/tests/bookmarks/test_818584-discard-duplicate-backups.js index be5d53f8c6..71065425fd 100644 --- a/toolkit/components/places/tests/bookmarks/test_818584-discard-duplicate-backups.js +++ b/toolkit/components/places/tests/bookmarks/test_818584-discard-duplicate-backups.js @@ -17,7 +17,7 @@ add_task(async function () { oldBackup ); Assert.ok(count > 0); - Assert.equal(hash.length, 24); + Assert.equal(hash.length, 44); oldBackupName = oldBackupName.replace( /\.json/, "_" + count + "_" + hash + ".json" diff --git a/toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js b/toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js index 6d280e8cad..265cc5621d 100644 --- a/toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js +++ b/toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js @@ -28,7 +28,7 @@ add_task(async function test_saveBookmarksToJSONFile_and_create() { PlacesBackups.filenamesRegex ); Assert.equal(matches[2], nodeCount); - Assert.equal(matches[3].length, 24); + Assert.equal(matches[3].length, 44); // Clear all backups in our backups folder. await PlacesBackups.create(0); @@ -44,7 +44,7 @@ add_task(async function test_saveBookmarksToJSONFile_and_create() { PlacesBackups.filenamesRegex ); Assert.equal(matches[2], nodeCount); - Assert.equal(matches[3].length, 24); + Assert.equal(matches[3].length, 44); // Cleanup await IOUtils.remove(backupFile); diff --git a/toolkit/components/places/tests/bookmarks/test_insert_thousands_bookmarks.js b/toolkit/components/places/tests/bookmarks/test_insert_thousands_bookmarks.js new file mode 100644 index 0000000000..9d1823449f --- /dev/null +++ b/toolkit/components/places/tests/bookmarks/test_insert_thousands_bookmarks.js @@ -0,0 +1,24 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Test `insertTree()` with more bookmarks than the Sqlite variables limit. + +add_task(async function () { + const NUM_BOOKMARKS = 1000; + await PlacesUtils.withConnectionWrapper("test", async db => { + db.variableLimit = NUM_BOOKMARKS - 100; + Assert.greater( + NUM_BOOKMARKS, + db.variableLimit, + "Insert more bookmarks than the Sqlite variables limit." + ); + }); + let children = []; + for (let i = 0; i < NUM_BOOKMARKS; ++i) { + children.push({ url: "http://www.mozilla.org/" + i }); + } + await PlacesUtils.bookmarks.insertTree({ + guid: PlacesUtils.bookmarks.toolbarGuid, + children, + }); +}); diff --git a/toolkit/components/places/tests/bookmarks/xpcshell.toml b/toolkit/components/places/tests/bookmarks/xpcshell.toml index 0b989e5fbf..c2e6d7ff09 100644 --- a/toolkit/components/places/tests/bookmarks/xpcshell.toml +++ b/toolkit/components/places/tests/bookmarks/xpcshell.toml @@ -70,6 +70,8 @@ support-files = ["bookmarks_long_tag.json"] ["test_bookmarks_update.js"] +["test_insert_thousands_bookmarks.js"] + ["test_insertTree_fixupOrSkipInvalidEntries.js"] ["test_keywords.js"] diff --git a/toolkit/components/places/tests/browser/browser.toml b/toolkit/components/places/tests/browser/browser.toml index 022b929240..b7d688c980 100644 --- a/toolkit/components/places/tests/browser/browser.toml +++ b/toolkit/components/places/tests/browser/browser.toml @@ -44,6 +44,8 @@ support-files = [ "redirect_twice.sjs", ] +["browser_favicon.js"] + ["browser_favicon_privatebrowsing_perwindowpb.js"] ["browser_history_post.js"] @@ -94,9 +96,11 @@ support-files = [ https_first_disabled = true support-files = [ "begin.html", + "favicon.html", "final.html", "redirect_once.sjs", "redirect_twice.sjs", + "userpass.html", ] ["browser_visituri_nohistory.js"] diff --git a/toolkit/components/places/tests/browser/browser_double_redirect.js b/toolkit/components/places/tests/browser/browser_double_redirect.js index 435bd86f19..d5b2fca1fe 100644 --- a/toolkit/components/places/tests/browser/browser_double_redirect.js +++ b/toolkit/components/places/tests/browser/browser_double_redirect.js @@ -16,7 +16,7 @@ add_task(async function () { let promiseVisits = new Promise(resolve => { let observer = { _notified: [], - onVisit(uri, id, time, referrerId, transition) { + onVisit(uri) { info("Received onVisit: " + uri); this._notified.push(uri); diff --git a/toolkit/components/places/tests/browser/browser_favicon.js b/toolkit/components/places/tests/browser/browser_favicon.js new file mode 100644 index 0000000000..9b4a1b97fe --- /dev/null +++ b/toolkit/components/places/tests/browser/browser_favicon.js @@ -0,0 +1,74 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const lazy = {}; +ChromeUtils.defineESModuleGetters(lazy, { + PlacesTestUtils: "resource://testing-common/PlacesTestUtils.sys.mjs", +}); + +const { MockRegistrar } = ChromeUtils.importESModule( + "resource://testing-common/MockRegistrar.sys.mjs" +); + +add_task(async function test_userpass() { + // Setup the prompt to avoid showing it. + let mockPromptService = { + firstTimeCalled: false, + confirmExBC() { + if (!this.firstTimeCalled) { + this.firstTimeCalled = true; + return 0; + } + + return 1; + }, + QueryInterface: ChromeUtils.generateQI(["nsIPromptService"]), + }; + let mockPromptServiceCID = MockRegistrar.register( + "@mozilla.org/prompter;1", + mockPromptService + ); + registerCleanupFunction(() => { + MockRegistrar.unregister(mockPromptServiceCID); + }); + + const pageUrl = + "https://user:pass@example.org/tests/toolkit/components/places/tests/browser/favicon.html"; + const faviconUrl = + "https://user:pass@example.org/tests/toolkit/components/places/tests/browser/favicon-normal32.png"; + const exposableFaviconUrl = + "https://example.org/tests/toolkit/components/places/tests/browser/favicon-normal32.png"; + + let faviconPromise = lazy.PlacesTestUtils.waitForNotification( + "favicon-changed", + async () => { + let faviconForExposable = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_icons", + "icon_url", + { + icon_url: exposableFaviconUrl, + } + ); + Assert.ok(faviconForExposable, "Found the icon for exposable URL"); + + let faviconForOriginal = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_icons", + "icon_url", + { + icon_url: faviconUrl, + } + ); + Assert.ok(!faviconForOriginal, "Not found the icon for the original URL"); + return true; + } + ); + + await BrowserTestUtils.openNewForegroundTab(gBrowser, pageUrl); + await faviconPromise; + + // Clean up. + await PlacesUtils.history.clear(); + gBrowser.removeCurrentTab(); +}); diff --git a/toolkit/components/places/tests/browser/browser_favicon_privatebrowsing_perwindowpb.js b/toolkit/components/places/tests/browser/browser_favicon_privatebrowsing_perwindowpb.js index ab3e0a1ef1..090fe802ab 100644 --- a/toolkit/components/places/tests/browser/browser_favicon_privatebrowsing_perwindowpb.js +++ b/toolkit/components/places/tests/browser/browser_favicon_privatebrowsing_perwindowpb.js @@ -36,7 +36,7 @@ function test() { waitForTabLoad(win, function () { PlacesUtils.favicons.getFaviconURLForPage( NetUtil.newURI(pageURI), - function (uri, dataLen, data, mimeType) { + function (uri) { is(uri, null, "No result should be found"); finish(); } diff --git a/toolkit/components/places/tests/browser/browser_history_post.js b/toolkit/components/places/tests/browser/browser_history_post.js index a62592516f..dad988a624 100644 --- a/toolkit/components/places/tests/browser/browser_history_post.js +++ b/toolkit/components/places/tests/browser/browser_history_post.js @@ -12,7 +12,7 @@ add_task(async function () { let doc = content.document; let submit = doc.getElementById("submit"); let iframe = doc.getElementById("post_iframe"); - let p = new Promise((resolve, reject) => { + let p = new Promise(resolve => { iframe.addEventListener( "load", function () { diff --git a/toolkit/components/places/tests/browser/browser_notfound.js b/toolkit/components/places/tests/browser/browser_notfound.js index 22ac67de0a..84e4f2ab9b 100644 --- a/toolkit/components/places/tests/browser/browser_notfound.js +++ b/toolkit/components/places/tests/browser/browser_notfound.js @@ -23,7 +23,7 @@ add_task(async function () { gBrowser, url, }, - async browser => { + async () => { info("awaiting for the visit"); await promiseVisited; diff --git a/toolkit/components/places/tests/browser/browser_redirect_self.js b/toolkit/components/places/tests/browser/browser_redirect_self.js index 7ed7ee0af0..3f14cf4f7c 100644 --- a/toolkit/components/places/tests/browser/browser_redirect_self.js +++ b/toolkit/components/places/tests/browser/browser_redirect_self.js @@ -37,7 +37,7 @@ add_task(async function () { gBrowser, url, }, - async browser => { + async () => { await TestUtils.waitForCondition(() => visitCount == 2); // Check that the visit is not hidden in the database. Assert.ok( diff --git a/toolkit/components/places/tests/browser/browser_visited_notfound.js b/toolkit/components/places/tests/browser/browser_visited_notfound.js index 36d1764361..ca0638481b 100644 --- a/toolkit/components/places/tests/browser/browser_visited_notfound.js +++ b/toolkit/components/places/tests/browser/browser_visited_notfound.js @@ -29,7 +29,7 @@ add_task(async function test() { gBrowser, url, }, - async browser => { + async () => { info("awaiting for the visit"); Assert.equal( diff --git a/toolkit/components/places/tests/browser/browser_visituri.js b/toolkit/components/places/tests/browser/browser_visituri.js index 6633ac188b..529c010e63 100644 --- a/toolkit/components/places/tests/browser/browser_visituri.js +++ b/toolkit/components/places/tests/browser/browser_visituri.js @@ -1,86 +1,78 @@ -/** - * One-time observer callback. - */ -function promiseObserve(name, checkFn) { - return new Promise(resolve => { - Services.obs.addObserver(function observer(subject) { - if (checkFn(subject)) { - Services.obs.removeObserver(observer, name); - resolve(); - } - }, name); - }); -} - -var conn = PlacesUtils.history.DBConnection; - -/** - * Gets a single column value from either the places or historyvisits table. - */ -function getColumn(table, column, fromColumnName, fromColumnValue) { - let sql = `SELECT ${column} - FROM ${table} - WHERE ${fromColumnName} = :val - ${fromColumnName == "url" ? "AND url_hash = hash(:val)" : ""} - LIMIT 1`; - let stmt = conn.createStatement(sql); - try { - stmt.params.val = fromColumnValue; - ok(stmt.executeStep(), "Expect to get a row"); - return stmt.row[column]; - } finally { - stmt.reset(); - } -} +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; -add_task(async function () { +const lazy = {}; +ChromeUtils.defineESModuleGetters(lazy, { + PlacesTestUtils: "resource://testing-common/PlacesTestUtils.sys.mjs", + TestUtils: "resource://testing-common/TestUtils.sys.mjs", +}); + +add_task(async function test_basic() { // Make sure places visit chains are saved correctly with a redirect // transitions. // Part 1: observe history events that fire when a visit occurs. // Make sure visits appear in order, and that the visit chain is correct. - var expectedUrls = [ + const expectedUrls = [ "http://example.com/tests/toolkit/components/places/tests/browser/begin.html", "http://example.com/tests/toolkit/components/places/tests/browser/redirect_twice.sjs", "http://example.com/tests/toolkit/components/places/tests/browser/redirect_once.sjs", "http://test1.example.com/tests/toolkit/components/places/tests/browser/final.html", ]; - var currentIndex = 0; - - function checkObserver(subject) { - var uri = subject.QueryInterface(Ci.nsIURI); - var expected = expectedUrls[currentIndex]; - is(uri.spec, expected, "Saved URL visit " + uri.spec); - - var placeId = getColumn("moz_places", "id", "url", uri.spec); - var fromVisitId = getColumn( - "moz_historyvisits", - "from_visit", - "place_id", - placeId - ); - if (currentIndex == 0) { - is(fromVisitId, 0, "First visit has no from visit"); - } else { - var lastVisitId = getColumn( - "moz_historyvisits", - "place_id", + let currentIndex = 0; + let visitUriPromise = lazy.TestUtils.topicObserved( + "uri-visit-saved", + async subject => { + let uri = subject.QueryInterface(Ci.nsIURI); + let expected = expectedUrls[currentIndex]; + is(uri.spec, expected, "Saved URL visit " + uri.spec); + + let placeId = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_places", "id", - fromVisitId + { + url: uri.spec, + } ); - var fromVisitUrl = getColumn("moz_places", "url", "id", lastVisitId); - is( - fromVisitUrl, - expectedUrls[currentIndex - 1], - "From visit was " + expectedUrls[currentIndex - 1] + let fromVisitId = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_historyvisits", + "from_visit", + { + place_id: placeId, + } ); - } - currentIndex++; - return currentIndex >= expectedUrls.length; - } - let visitUriPromise = promiseObserve("uri-visit-saved", checkObserver); + if (currentIndex == 0) { + is(fromVisitId, 0, "First visit has no from visit"); + } else { + let lastVisitId = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_historyvisits", + "place_id", + { + id: fromVisitId, + } + ); + let fromVisitUrl = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_places", + "url", + { + id: lastVisitId, + } + ); + is( + fromVisitUrl, + expectedUrls[currentIndex - 1], + "From visit was " + expectedUrls[currentIndex - 1] + ); + } + + currentIndex++; + return currentIndex >= expectedUrls.length; + } + ); const testUrl = "http://example.com/tests/toolkit/components/places/tests/browser/begin.html"; @@ -94,7 +86,119 @@ add_task(async function () { ); await visitUriPromise; + // Clean up. await PlacesUtils.history.clear(); + gBrowser.removeCurrentTab(); +}); + +add_task(async function test_userpass() { + // Avoid showing the auth prompt. + await SpecialPowers.pushPrefEnv({ + set: [["network.auth.confirmAuth.enabled", false]], + }); + + // Open a html having test links. + await BrowserTestUtils.openNewForegroundTab( + gBrowser, + "https://example.org/tests/toolkit/components/places/tests/browser/userpass.html" + ); + + const clickedUrl = + "https://user:pass@example.org/tests/toolkit/components/places/tests/browser/favicon.html"; + const exposablePageUrl = + "https://example.org/tests/toolkit/components/places/tests/browser/favicon.html"; + + let visitUriPromise = lazy.TestUtils.topicObserved( + "uri-visit-saved", + async subject => { + let uri = subject.QueryInterface(Ci.nsIURI); + if (uri.spec !== exposablePageUrl) { + return false; + } + let placeForExposable = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_places", + "id", + { + url: exposablePageUrl, + } + ); + Assert.ok(placeForExposable, "Found the place for exposable URL"); + + let placeForOriginal = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_places", + "id", + { + url: clickedUrl, + } + ); + Assert.ok(!placeForOriginal, "Not found the place for the original URL"); + + return true; + } + ); + + // Open the target link as background. + await ContentTask.spawn(gBrowser.selectedBrowser, null, async args => { + let link = content.document.getElementById("target-userpass"); + EventUtils.synthesizeMouseAtCenter( + link, + { + ctrlKey: true, + metaKey: true, + }, + content + ); + return link.href; + }); + + // Wait for fireing visited event. + await visitUriPromise; + + // Check the title. + await BrowserTestUtils.waitForCondition(async () => { + let titleForExposable = await lazy.PlacesTestUtils.getDatabaseValue( + "moz_places", + "title", + { + url: exposablePageUrl, + } + ); + return titleForExposable == "favicon page"; + }, "Wait for the proper title is updated"); + + // Check the link status. + const expectedResults = { + "target-userpass": true, + "no-userpass": true, + "another-userpass": false, + "another-url": false, + }; + + for (const [key, value] of Object.entries(expectedResults)) { + await ContentTask.spawn( + gBrowser.selectedBrowser, + [key, value], + async ([k, v]) => { + // ElementState::VISITED + const VISITED_STATE = 1 << 18; + await ContentTaskUtils.waitForCondition(() => { + const isVisited = !!( + content.InspectorUtils.getContentState( + content.document.getElementById(k) + ) & VISITED_STATE + ); + return isVisited == v; + }); + } + ); + Assert.ok(true, `The status of ${key} is correct`); + } + + // Clean up. + await PlacesUtils.history.clear(); + // Remove the tab for userpass.html + gBrowser.removeCurrentTab(); + // Remove the tab for favicon.html gBrowser.removeCurrentTab(); }); diff --git a/toolkit/components/places/tests/browser/browser_visituri_privatebrowsing_perwindowpb.js b/toolkit/components/places/tests/browser/browser_visituri_privatebrowsing_perwindowpb.js index 746611d2ad..2d170115f6 100644 --- a/toolkit/components/places/tests/browser/browser_visituri_privatebrowsing_perwindowpb.js +++ b/toolkit/components/places/tests/browser/browser_visituri_privatebrowsing_perwindowpb.js @@ -13,7 +13,7 @@ var visitSavedPromise; add_setup(async function () { visitSavedPromise = new Promise(resolve => { observer = { - observe(subject, topic, data) { + observe(subject, topic) { // The uri-visit-saved topic should only work when on normal mode. if (topic == "uri-visit-saved") { Services.obs.removeObserver(observer, "uri-visit-saved"); diff --git a/toolkit/components/places/tests/browser/favicon.html b/toolkit/components/places/tests/browser/favicon.html index a0f5ea9594..789d0b89b8 100644 --- a/toolkit/components/places/tests/browser/favicon.html +++ b/toolkit/components/places/tests/browser/favicon.html @@ -5,9 +5,10 @@ <html> <head> - <link rel="shortcut icon" href="http://example.org/tests/toolkit/components/places/tests/browser/favicon-normal32.png"> + <link rel="shortcut icon" href="/tests/toolkit/components/places/tests/browser/favicon-normal32.png"> + <title>favicon page</title> </head> <body> - OK we're done! + <label>OK we're done!</label> </body> </html> diff --git a/toolkit/components/places/tests/browser/previews/browser.toml b/toolkit/components/places/tests/browser/previews/browser.toml index 10758a4803..f7f2c9b583 100644 --- a/toolkit/components/places/tests/browser/previews/browser.toml +++ b/toolkit/components/places/tests/browser/previews/browser.toml @@ -2,7 +2,7 @@ prefs = [ "browser.pagethumbnails.capturing_disabled=false", "places.previews.enabled=true", - "places.previews.log=true", + "places.loglevel='All'", ] ["browser_thumbnails.js"] diff --git a/toolkit/components/places/tests/browser/userpass.html b/toolkit/components/places/tests/browser/userpass.html new file mode 100644 index 0000000000..ceff388b79 --- /dev/null +++ b/toolkit/components/places/tests/browser/userpass.html @@ -0,0 +1,13 @@ +<!-- + Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ + --> + +<html> + <body> + <a href="https://user:pass@example.org/tests/toolkit/components/places/tests/browser/favicon.html" id="target-userpass">target userpass</a> + <a href="https://user2:pass2@example.org/tests/toolkit/components/places/tests/browser/favicon.html" id="another-userpass">another userpass</a> + <a href="https://example.org/tests/toolkit/components/places/tests/browser/favicon.html" id="no-userpass">no userpass</a> + <a href="https://example.org/" id="another-url">another url</a> + </body> +</html> diff --git a/toolkit/components/places/tests/chrome/test_371798.xhtml b/toolkit/components/places/tests/chrome/test_371798.xhtml index 33e866e51e..f66ac74aae 100644 --- a/toolkit/components/places/tests/chrome/test_371798.xhtml +++ b/toolkit/components/places/tests/chrome/test_371798.xhtml @@ -57,7 +57,7 @@ const TEST_URI = Services.io.newURI("http://foo.com"); let node = rootNode.getChild(i); // test that bm1 does not have new title if (node.bookmarkGuid == bm1.guid) - ok(node.title != "foo", + isnot(node.title, "foo", "Changing a bookmark's title did not affect the title of other bookmarks with the same URI"); } rootNode.containerOpen = false; diff --git a/toolkit/components/places/tests/expiration/test_notifications.js b/toolkit/components/places/tests/expiration/test_notifications.js index d52319a9c9..7ac7d769d6 100644 --- a/toolkit/components/places/tests/expiration/test_notifications.js +++ b/toolkit/components/places/tests/expiration/test_notifications.js @@ -14,7 +14,7 @@ var gObserver = { notifications: 0, - observe(aSubject, aTopic, aData) { + observe() { this.notifications++; }, }; diff --git a/toolkit/components/places/tests/favicons/head_favicons.js b/toolkit/components/places/tests/favicons/head_favicons.js index d8109c66e0..afd2c4924f 100644 --- a/toolkit/components/places/tests/favicons/head_favicons.js +++ b/toolkit/components/places/tests/favicons/head_favicons.js @@ -53,13 +53,10 @@ function checkFaviconDataForPage( * This function is called after the check finished. */ function checkFaviconMissingForPage(aPageURI, aCallback) { - PlacesUtils.favicons.getFaviconURLForPage( - aPageURI, - function (aURI, aDataLen, aData, aMimeType) { - Assert.ok(aURI === null); - aCallback(); - } - ); + PlacesUtils.favicons.getFaviconURLForPage(aPageURI, function (aURI) { + Assert.ok(aURI === null); + aCallback(); + }); } function promiseFaviconMissingForPage(aPageURI) { diff --git a/toolkit/components/places/tests/favicons/test_cached-favicon_mime_type.js b/toolkit/components/places/tests/favicons/test_cached-favicon_mime_type.js index 1c95d63f94..c1f6689a70 100644 --- a/toolkit/components/places/tests/favicons/test_cached-favicon_mime_type.js +++ b/toolkit/components/places/tests/favicons/test_cached-favicon_mime_type.js @@ -19,7 +19,7 @@ function streamListener(aExpectedContentType) { } streamListener.prototype = { onStartRequest() {}, - onStopRequest(aRequest, aContext, aStatusCode) { + onStopRequest(aRequest) { let channel = aRequest.QueryInterface(Ci.nsIChannel); Assert.equal( channel.contentType, @@ -28,7 +28,7 @@ streamListener.prototype = { ); this.done.resolve(); }, - onDataAvailable(aRequest, aInputStream, aOffset, aCount) { + onDataAvailable(aRequest) { aRequest.cancel(Cr.NS_ERROR_ABORT); throw Components.Exception("", Cr.NS_ERROR_ABORT); }, @@ -85,4 +85,60 @@ add_task(async function () { let listener = new streamListener("image/png"); channel.asyncOpen(listener); await listener.done.promise; + + await PlacesUtils.history.clear(); +}); + +add_task(async function test_userpass() { + info("Test whether can get favicon content regardless of user pass"); + + const PAGE_NORMAL = uri("http://mozilla.org/"); + const PAGE_USERPASS = uri("http://user:pass@mozilla.org/"); + const ICON_NORMAL = uri("http://mozilla.org/favicon.png"); + const ICON_USERPASS = uri("http://user:pass@mozilla.org/favicon.png"); + const CACHED_ICON_NORMAL = "cached-favicon:http://mozilla.org/favicon.png"; + const CACHED_ICON_USERPASS = + "cached-favicon:http://user:pass@mozilla.org/favicon.png"; + + const testData = [ + { + pageURI: PAGE_USERPASS, + iconURI: ICON_NORMAL, + }, + { + pageURI: PAGE_NORMAL, + iconURI: ICON_USERPASS, + }, + { + pageURI: PAGE_USERPASS, + iconURI: ICON_USERPASS, + }, + ]; + + for (const { pageURI, iconURI } of testData) { + for (const loadingIconURISpec of [ + CACHED_ICON_NORMAL, + CACHED_ICON_USERPASS, + ]) { + PlacesUtils.favicons.replaceFaviconDataFromDataURL( + iconURI, + testFaviconData, + 0, + systemPrincipal + ); + await PlacesTestUtils.addVisits(pageURI); + await setFaviconForPage(pageURI, iconURI); + + // Open the channel + let channel = NetUtil.newChannel({ + uri: loadingIconURISpec, + loadUsingSystemPrincipal: true, + contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_IMAGE_FAVICON, + }); + let listener = new streamListener("image/png"); + channel.asyncOpen(listener); + await listener.done.promise; + await PlacesUtils.history.clear(); + } + } }); diff --git a/toolkit/components/places/tests/favicons/test_page-icon_protocol.js b/toolkit/components/places/tests/favicons/test_page-icon_protocol.js index 932040bafb..287484868f 100644 --- a/toolkit/components/places/tests/favicons/test_page-icon_protocol.js +++ b/toolkit/components/places/tests/favicons/test_page-icon_protocol.js @@ -185,11 +185,11 @@ add_task(async function page_content_process() { let img = content.document.createElement("img"); img.src = url; let imgPromise = new Promise((resolve, reject) => { - img.addEventListener("error", e => { + img.addEventListener("error", () => { Assert.ok(true, "Got expected load error."); resolve(); }); - img.addEventListener("load", e => { + img.addEventListener("load", () => { Assert.ok(false, "Did not expect a successful load."); reject(); }); @@ -225,11 +225,11 @@ add_task(async function page_privileged_about_content_process() { let img = content.document.createElement("img"); img.src = url; let imgPromise = new Promise((resolve, reject) => { - img.addEventListener("error", e => { + img.addEventListener("error", () => { Assert.ok(false, "Did not expect an error. "); reject(); }); - img.addEventListener("load", e => { + img.addEventListener("load", () => { Assert.ok(true, "Got expected load event."); resolve(); }); @@ -241,3 +241,57 @@ add_task(async function page_privileged_about_content_process() { await contentPage.close(); }); + +add_task(async function test_with_user_pass() { + info("Test whether can get favicon content regardless of user pass"); + await PlacesUtils.history.clear(); + + const PAGE_NORMAL = uri("http://mozilla.org/"); + const PAGE_USERPASS = uri("http://user:pass@mozilla.org/"); + const ICON_NORMAL = uri("http://mozilla.org/favicon.png"); + const ICON_USERPASS = uri("http://user:pass@mozilla.org/favicon.png"); + const PAGE_ICON_NORMAL = "page-icon:http://mozilla.org/"; + const PAGE_ICON_USERPASS = "page-icon:http://user:pass@mozilla.org/"; + + const testData = [ + { + pageURI: PAGE_USERPASS, + iconURI: ICON_NORMAL, + }, + { + pageURI: PAGE_NORMAL, + iconURI: ICON_USERPASS, + }, + { + pageURI: PAGE_USERPASS, + iconURI: ICON_USERPASS, + }, + ]; + + for (const { pageURI, iconURI } of testData) { + for (const loadingIconURISpec of [PAGE_ICON_NORMAL, PAGE_ICON_USERPASS]) { + PlacesUtils.favicons.replaceFaviconDataFromDataURL( + iconURI, + ICON_DATAURL, + 0, + systemPrincipal + ); + await PlacesTestUtils.addVisits(pageURI); + await new Promise(resolve => { + PlacesUtils.favicons.setAndFetchFaviconForPage( + pageURI, + iconURI, + false, + PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, + resolve, + Services.scriptSecurityManager.getSystemPrincipal() + ); + }); + + let { data, contentType } = await fetchIconForSpec(loadingIconURISpec); + Assert.equal(contentType, gFavicon.contentType); + Assert.deepEqual(data, gFavicon.data, "Got the favicon data"); + await PlacesUtils.history.clear(); + } + } +}); diff --git a/toolkit/components/places/tests/favicons/test_setAndFetchFaviconForPage.js b/toolkit/components/places/tests/favicons/test_setAndFetchFaviconForPage.js index 1b4ea87ec0..b2d82e65b0 100644 --- a/toolkit/components/places/tests/favicons/test_setAndFetchFaviconForPage.js +++ b/toolkit/components/places/tests/favicons/test_setAndFetchFaviconForPage.js @@ -53,6 +53,17 @@ let gTests = [ Services.prefs.setBoolPref("places.history.enabled", true); }, }, + { + desc: "Visit URL with login info", + href: "http://user:pass@example.com/with_login_info", + loadType: PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, + async setup() { + await PlacesTestUtils.addVisits({ + uri: this.href, + transition: TRANSITION_TYPED, + }); + }, + }, ]; add_task(async function () { @@ -67,6 +78,7 @@ add_task(async function () { for (let test of gTests) { info(test.desc); let pageURI = PlacesUtils.toURI(test.href); + let exposableURI = Services.io.createExposableURI(pageURI); await test.setup(); @@ -75,7 +87,7 @@ add_task(async function () { "favicon-changed", events => events.some(e => { - if (e.url == pageURI.spec && e.faviconUrl == faviconURI.spec) { + if (e.url == exposableURI.spec && e.faviconUrl == faviconURI.spec) { pageGuid = e.pageGuid; return true; } @@ -97,7 +109,7 @@ add_task(async function () { Assert.equal( pageGuid, await PlacesTestUtils.getDatabaseValue("moz_places", "guid", { - url: pageURI, + url: exposableURI, }), "Page guid is correct" ); diff --git a/toolkit/components/places/tests/history/test_async_history_api.js b/toolkit/components/places/tests/history/test_async_history_api.js index ce0d96b306..cc645baaec 100644 --- a/toolkit/components/places/tests/history/test_async_history_api.js +++ b/toolkit/components/places/tests/history/test_async_history_api.js @@ -32,7 +32,7 @@ function VisitInfo(aTransitionType, aVisitTime) { } function promiseUpdatePlaces(aPlaces, aOptions = {}) { - return new Promise((resolve, reject) => { + return new Promise(resolve => { asyncHistory.updatePlaces( aPlaces, Object.assign( @@ -974,7 +974,7 @@ add_task(async function test_title_change_notifies() { place.title = "title 1"; let expectedNotification = false; let titleChangeObserver; - let titleChangePromise = new Promise((resolve, reject) => { + let titleChangePromise = new Promise(resolve => { titleChangeObserver = new TitleChangedObserver( place.uri, place.title, @@ -1032,8 +1032,8 @@ add_task(async function test_visit_notifies() { }; Assert.equal(false, await PlacesUtils.history.hasVisits(place.uri)); - function promiseVisitObserver(aPlace) { - return new Promise((resolve, reject) => { + function promiseVisitObserver() { + return new Promise(resolve => { let callbackCount = 0; let finisher = function () { if (++callbackCount == 2) { @@ -1137,7 +1137,7 @@ add_task(async function test_omit_frecency_notifications() { // we won't get a ranking changed notification until recalculation happens. await PlacesUtils.history.clear(); let notified = false; - let listener = events => { + let listener = () => { notified = true; PlacesUtils.observers.removeListener(["pages-rank-changed"], listener); }; diff --git a/toolkit/components/places/tests/history/test_insertMany.js b/toolkit/components/places/tests/history/test_insertMany.js index b2cf60ed91..3d0774cbf2 100644 --- a/toolkit/components/places/tests/history/test_insertMany.js +++ b/toolkit/components/places/tests/history/test_insertMany.js @@ -189,7 +189,7 @@ add_task(async function test_transitions() { await PlacesUtils.history.insertMany(places); // Check callbacks. let count = 0; - await PlacesUtils.history.insertMany(places, pageInfo => { + await PlacesUtils.history.insertMany(places, () => { ++count; }); Assert.equal(count, Object.keys(PlacesUtils.history.TRANSITIONS).length); @@ -246,3 +246,22 @@ add_task(async function test_guid() { "Record C is fetchable after insertMany" ); }); + +add_task(async function test_withUserPass() { + await PlacesUtils.history.insertMany([ + { + url: "http://user:pass@example.com/userpass", + visits: [{ date: new Date() }], + }, + ]); + + Assert.ok( + !(await PlacesUtils.history.fetch("http://user:pass@example.com/userpass")), + "The url with user and pass is not stored" + ); + + Assert.ok( + await PlacesUtils.history.fetch("http://example.com/userpass"), + "The url without user and pass is stored" + ); +}); diff --git a/toolkit/components/places/tests/history/test_removeByFilter.js b/toolkit/components/places/tests/history/test_removeByFilter.js index fb18bf8e74..fe90977bfd 100644 --- a/toolkit/components/places/tests/history/test_removeByFilter.js +++ b/toolkit/components/places/tests/history/test_removeByFilter.js @@ -174,13 +174,13 @@ add_task(async function test_removeByFilter() { for (let callbackUse of [true, false]) { // Case A Positives for (let bookmarkUse of [true, false]) { - let bookmarkedUri = arr => undefined; + let bookmarkedUri = () => undefined; let checkableArray = arr => arr; let checkClosure = assertNotInDB; if (bookmarkUse) { bookmarkedUri = arr => arr[0]; checkableArray = arr => arr.slice(1); - checkClosure = function (aUri) {}; + checkClosure = function () {}; } // Case A 1: Dates await removeByFilterTester( diff --git a/toolkit/components/places/tests/history/test_removeVisitsByFilter.js b/toolkit/components/places/tests/history/test_removeVisitsByFilter.js index 5681ab22bc..be01fcb901 100644 --- a/toolkit/components/places/tests/history/test_removeVisitsByFilter.js +++ b/toolkit/components/places/tests/history/test_removeVisitsByFilter.js @@ -117,7 +117,7 @@ add_task(async function test_removeVisitsByFilter() { } endIndex = Math.min( endIndex, - removedItems.findIndex((v, index) => v.uri.spec != rawURL) - 1 + removedItems.findIndex(v => v.uri.spec != rawURL) - 1 ); } removedItems.splice(endIndex + 1); diff --git a/toolkit/components/places/tests/history/test_updatePlaces_embed.js b/toolkit/components/places/tests/history/test_updatePlaces_embed.js index a2831f2f58..84efd11b2d 100644 --- a/toolkit/components/places/tests/history/test_updatePlaces_embed.js +++ b/toolkit/components/places/tests/history/test_updatePlaces_embed.js @@ -28,10 +28,10 @@ add_task(async function test_embed_visit() { asyncHistory.updatePlaces(place, { ignoreErrors: true, ignoreResults: true, - handleError(aResultCode, aPlace) { + handleError() { errors++; }, - handleResult(aPlace) { + handleResult() { results++; }, handleCompletion(resultCount) { @@ -64,10 +64,10 @@ add_task(async function test_misc_visits() { asyncHistory.updatePlaces(place, { ignoreErrors: true, ignoreResults: true, - handleError(aResultCode, aPlace) { + handleError() { errors++; }, - handleResult(aPlace) { + handleResult() { results++; }, handleCompletion(resultCount) { diff --git a/toolkit/components/places/tests/migration/places_v75.sqlite b/toolkit/components/places/tests/migration/places_v77.sqlite Binary files differindex 2dd624b945..d77d8233b0 100644 --- a/toolkit/components/places/tests/migration/places_v75.sqlite +++ b/toolkit/components/places/tests/migration/places_v77.sqlite diff --git a/toolkit/components/places/tests/migration/test_current_from_v74.js b/toolkit/components/places/tests/migration/test_current_from_v74.js index 82c535f78f..eeb862daf5 100644 --- a/toolkit/components/places/tests/migration/test_current_from_v74.js +++ b/toolkit/components/places/tests/migration/test_current_from_v74.js @@ -4,7 +4,16 @@ "use strict"; add_task(async function setup() { - await setupPlacesDatabase("places_v74.sqlite"); + let path = await setupPlacesDatabase("places_v74.sqlite"); + let db = await Sqlite.openConnection({ path }); + await db.execute(` + INSERT INTO moz_origins (id, prefix, host, frecency, recalc_frecency) + VALUES + (100, 'https://', 'test1.com', 0, 0), + (101, 'https://', 'test2.com', 0, 0), + (102, 'https://', 'test3.com', 0, 0) + `); + await db.close(); }); add_task(async function database_is_valid() { @@ -20,3 +29,18 @@ add_task(async function database_is_valid() { await db.execute("SELECT * FROM moz_places_extra"); await db.execute("SELECT * from moz_historyvisits_extra"); }); + +add_task(async function recalc_origins_frecency() { + const db = await PlacesUtils.promiseDBConnection(); + Assert.equal(await db.getSchemaVersion(), CURRENT_SCHEMA_VERSION); + + Assert.equal( + ( + await db.execute( + "SELECT count(*) FROM moz_origins WHERE recalc_frecency = 0" + ) + )[0].getResultByIndex(0), + 0, + "All entries should be set for recalculation" + ); +}); diff --git a/toolkit/components/places/tests/migration/xpcshell.toml b/toolkit/components/places/tests/migration/xpcshell.toml index b127fa501f..06dbb67fce 100644 --- a/toolkit/components/places/tests/migration/xpcshell.toml +++ b/toolkit/components/places/tests/migration/xpcshell.toml @@ -14,7 +14,7 @@ support-files = [ "places_v70.sqlite", "places_v72.sqlite", "places_v74.sqlite", - "places_v75.sqlite", + "places_v77.sqlite", ] ["test_current_from_downgraded.js"] diff --git a/toolkit/components/places/tests/moz.build b/toolkit/components/places/tests/moz.build index 60c57f53b8..a93b86a134 100644 --- a/toolkit/components/places/tests/moz.build +++ b/toolkit/components/places/tests/moz.build @@ -65,6 +65,7 @@ TEST_HARNESS_FILES.testing.mochitest.tests.toolkit.components.places.tests.brows "browser/redirect_twice_perma.sjs", "browser/title1.html", "browser/title2.html", + "browser/userpass.html", ] TEST_HARNESS_FILES.testing.mochitest.tests.toolkit.components.places.tests.chrome += [ diff --git a/toolkit/components/places/tests/queries/test_async.js b/toolkit/components/places/tests/queries/test_async.js index 8e895748ab..32a5c1a691 100644 --- a/toolkit/components/places/tests/queries/test_async.js +++ b/toolkit/components/places/tests/queries/test_async.js @@ -85,7 +85,7 @@ var tests = [ node.containerOpen = false; }, - opened(node, newState, oldState) { + opened() { do_throw("opened should not be called"); }, diff --git a/toolkit/components/places/tests/queries/test_containersQueries_sorting.js b/toolkit/components/places/tests/queries/test_containersQueries_sorting.js index 9cdc0f2a52..ff4bbe67bf 100644 --- a/toolkit/components/places/tests/queries/test_containersQueries_sorting.js +++ b/toolkit/components/places/tests/queries/test_containersQueries_sorting.js @@ -113,7 +113,7 @@ function cartProd(aSequences, aCallback) { // For each sequence in aSequences, we maintain a pointer (an array index, // really) to the element we're currently enumerating in that sequence - var seqEltPtrs = aSequences.map(i => 0); + var seqEltPtrs = aSequences.map(() => 0); var numProds = 0; var done = false; @@ -407,7 +407,7 @@ function check_children_sorting(aRootNode, aExpectedSortingMode) { var comparator; switch (aExpectedSortingMode) { case Ci.nsINavHistoryQueryOptions.SORT_BY_NONE: - comparator = function (a, b) { + comparator = function () { return 0; }; break; diff --git a/toolkit/components/places/tests/queries/test_querySerialization.js b/toolkit/components/places/tests/queries/test_querySerialization.js index 4c33854718..64a4c5a6e7 100644 --- a/toolkit/components/places/tests/queries/test_querySerialization.js +++ b/toolkit/components/places/tests/queries/test_querySerialization.js @@ -86,11 +86,11 @@ const querySwitches = [ desc: "nsINavHistoryQuery.hasBeginTime", matches: flagSwitchMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.beginTime = Date.now() * 1000; aQuery.beginTimeReference = Ci.nsINavHistoryQuery.TIME_RELATIVE_EPOCH; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.beginTime = Date.now() * 1000; aQuery.beginTimeReference = Ci.nsINavHistoryQuery.TIME_RELATIVE_TODAY; }, @@ -103,11 +103,11 @@ const querySwitches = [ desc: "nsINavHistoryQuery.hasEndTime", matches: flagSwitchMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.endTime = Date.now() * 1000; aQuery.endTimeReference = Ci.nsINavHistoryQuery.TIME_RELATIVE_EPOCH; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.endTime = Date.now() * 1000; aQuery.endTimeReference = Ci.nsINavHistoryQuery.TIME_RELATIVE_TODAY; }, @@ -120,10 +120,10 @@ const querySwitches = [ desc: "nsINavHistoryQuery.hasSearchTerms", matches: flagSwitchMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.searchTerms = "shrimp and white wine"; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.searchTerms = ""; }, ], @@ -135,15 +135,15 @@ const querySwitches = [ desc: "nsINavHistoryQuery.hasDomain", matches: flagSwitchMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.domain = "mozilla.com"; aQuery.domainIsHost = false; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.domain = "www.mozilla.com"; aQuery.domainIsHost = true; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.domain = ""; }, ], @@ -155,7 +155,7 @@ const querySwitches = [ desc: "nsINavHistoryQuery.hasUri", matches: flagSwitchMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.uri = uri("http://mozilla.com"); }, ], @@ -167,7 +167,7 @@ const querySwitches = [ desc: "nsINavHistoryQuery.minVisits", matches: simplePropertyMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.minVisits = 0x7fffffff; // 2^31 - 1 }, ], @@ -178,7 +178,7 @@ const querySwitches = [ desc: "nsINavHistoryQuery.maxVisits", matches: simplePropertyMatches, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.maxVisits = 0x7fffffff; // 2^31 - 1 }, ], @@ -205,13 +205,13 @@ const querySwitches = [ return true; }, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.setParents([]); }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.setParents([PlacesUtils.bookmarks.rootGuid]); }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.setParents([ PlacesUtils.bookmarks.rootGuid, PlacesUtils.bookmarks.tagsGuid, @@ -244,13 +244,13 @@ const querySwitches = [ return true; }, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.tags = []; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.tags = [""]; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.tags = [ "foo", "七難", @@ -263,7 +263,7 @@ const querySwitches = [ "あいうえお", ]; }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.tags = [ "foo", "七難", @@ -301,13 +301,13 @@ const querySwitches = [ return true; }, runs: [ - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.setTransitions([]); }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.setTransitions([Ci.nsINavHistoryService.TRANSITION_DOWNLOAD]); }, - function (aQuery, aQueryOptions) { + function (aQuery) { aQuery.setTransitions([ Ci.nsINavHistoryService.TRANSITION_TYPED, Ci.nsINavHistoryService.TRANSITION_BOOKMARK, @@ -455,7 +455,7 @@ function cartProd(aSequences, aCallback) { // For each sequence in aSequences, we maintain a pointer (an array index, // really) to the element we're currently enumerating in that sequence - var seqEltPtrs = aSequences.map(i => 0); + var seqEltPtrs = aSequences.map(() => 0); var numProds = 0; var done = false; diff --git a/toolkit/components/places/tests/queries/test_redirects.js b/toolkit/components/places/tests/queries/test_redirects.js index b0e7c9b421..6b122e3180 100644 --- a/toolkit/components/places/tests/queries/test_redirects.js +++ b/toolkit/components/places/tests/queries/test_redirects.js @@ -42,7 +42,7 @@ function check_results_callback(aSequence) { } // Build expectedData array. - let expectedData = visits.filter(function (aVisit, aIndex, aArray) { + let expectedData = visits.filter(function (aVisit) { // Embed visits never appear in results. if (aVisit.transType == Ci.nsINavHistoryService.TRANSITION_EMBED) { return false; @@ -154,7 +154,7 @@ function cartProd(aSequences, aCallback) { // For each sequence in aSequences, we maintain a pointer (an array index, // really) to the element we're currently enumerating in that sequence - let seqEltPtrs = aSequences.map(i => 0); + let seqEltPtrs = aSequences.map(() => 0); let numProds = 0; let done = false; diff --git a/toolkit/components/places/tests/queries/test_tags.js b/toolkit/components/places/tests/queries/test_tags.js index 17ad3478ce..23a332f20b 100644 --- a/toolkit/components/places/tests/queries/test_tags.js +++ b/toolkit/components/places/tests/queries/test_tags.js @@ -190,7 +190,7 @@ add_task(async function many_tags_no_bookmark() { "Querying on many tags associated with a URI and tags not associated " + "with that URI should not return that URI" ); - await task_doWithBookmark(["foo", "bar", "baz"], function (aURI) { + await task_doWithBookmark(["foo", "bar", "baz"], function () { var [query, opts] = makeQuery(["foo", "bogus"]); executeAndCheckQueryResults(query, opts, []); [query, opts] = makeQuery(["foo", "bar", "bogus"]); @@ -202,7 +202,7 @@ add_task(async function many_tags_no_bookmark() { add_task(async function nonexistent_tags() { info("Querying on nonexistent tag should return no results"); - await task_doWithBookmark(["foo", "bar", "baz"], function (aURI) { + await task_doWithBookmark(["foo", "bar", "baz"], function () { var [query, opts] = makeQuery(["bogus"]); executeAndCheckQueryResults(query, opts, []); [query, opts] = makeQuery(["bogus", "gnarly"]); @@ -449,7 +449,7 @@ function addBookmark(aURI) { /** * Asynchronous task that removes all pages from history and bookmarks. */ -async function task_cleanDatabase(aCallback) { +async function task_cleanDatabase() { await PlacesUtils.bookmarks.eraseEverything(); await PlacesUtils.history.clear(); } diff --git a/toolkit/components/places/tests/sync/head_sync.js b/toolkit/components/places/tests/sync/head_sync.js index 7dd69e275b..68221ce93f 100644 --- a/toolkit/components/places/tests/sync/head_sync.js +++ b/toolkit/components/places/tests/sync/head_sync.js @@ -116,7 +116,7 @@ function makeRecord(cleartext) { return new Proxy( { cleartext }, { - get(target, property, receiver) { + get(target, property) { if (property == "cleartext") { return target.cleartext; } @@ -125,7 +125,7 @@ function makeRecord(cleartext) { } return target.cleartext[property]; }, - set(target, property, value, receiver) { + set(target, property, value) { if (property == "cleartext") { target.cleartext = value; } else if (property != "cleartextToString") { @@ -135,7 +135,7 @@ function makeRecord(cleartext) { has(target, property) { return property == "cleartext" || property in target.cleartext; }, - deleteProperty(target, property) {}, + deleteProperty() {}, ownKeys(target) { return ["cleartext", ...Reflect.ownKeys(target)]; }, diff --git a/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js b/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js index 877feb99f4..f7a871e3cb 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js +++ b/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js @@ -95,8 +95,8 @@ add_task(async function test_blocker_state() { let buf = await SyncedBookmarksMirror.open({ path: "blocker_state_buf.sqlite", finalizeAt: barrier.client, - recordStepTelemetry(...args) {}, - recordValidationTelemetry(...args) {}, + recordStepTelemetry() {}, + recordValidationTelemetry() {}, }); await storeRecords(buf, [ { diff --git a/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js b/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js index 16d8ed746c..b4f3d4bd0b 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js +++ b/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js @@ -185,7 +185,7 @@ add_task(async function test_update_frecencies() { let frecencies = await promiseAllURLFrecencies(); let urlsWithFrecency = mapFilterIterator( frecencies.entries(), - ([href, { frecency, recalc }]) => (recalc == 0 ? href : null) + ([href, { recalc }]) => (recalc == 0 ? href : null) ); // A is unchanged, and we should recalculate frecency for three more @@ -236,7 +236,7 @@ add_task(async function test_update_frecencies() { let frecencies = await promiseAllURLFrecencies(); let urlsWithoutFrecency = mapFilterIterator( frecencies.entries(), - ([href, { frecency, recalc }]) => (recalc == 1 ? href : null) + ([href, { recalc }]) => (recalc == 1 ? href : null) ); deepEqual( urlsWithoutFrecency, diff --git a/toolkit/components/places/tests/unit/test_asyncExecuteLegacyQueries.js b/toolkit/components/places/tests/unit/test_asyncExecuteLegacyQueries.js index 084415fb37..02577f159e 100644 --- a/toolkit/components/places/tests/unit/test_asyncExecuteLegacyQueries.js +++ b/toolkit/components/places/tests/unit/test_asyncExecuteLegacyQueries.js @@ -30,7 +30,7 @@ add_task(async function test_history_query() { "Async execution error (" + aError.result + "): " + aError.message ); }, - handleCompletion(aReason) { + handleCompletion() { cleanupTest().then(resolve); }, }); @@ -69,7 +69,7 @@ add_task(async function test_bookmarks_query() { "Async execution error (" + aError.result + "): " + aError.message ); }, - handleCompletion(aReason) { + handleCompletion() { cleanupTest().then(resolve); }, }); diff --git a/toolkit/components/places/tests/unit/test_async_transactions.js b/toolkit/components/places/tests/unit/test_async_transactions.js index b0e9b292f3..9f96a9c040 100644 --- a/toolkit/components/places/tests/unit/test_async_transactions.js +++ b/toolkit/components/places/tests/unit/test_async_transactions.js @@ -1485,7 +1485,7 @@ add_task(async function test_edit_specific_keyword() { url: "http://test.edit.keyword/", }; bm_info.guid = await PT.NewBookmark(bm_info).transact(); - function ensureKeywordChange(aCurrentKeyword = "", aPreviousKeyword = "") { + function ensureKeywordChange(aCurrentKeyword = "") { ensureItemsKeywordChanged({ guid: bm_info.guid, keyword: aCurrentKeyword, diff --git a/toolkit/components/places/tests/unit/test_bookmark_list.js b/toolkit/components/places/tests/unit/test_bookmark_list.js new file mode 100644 index 0000000000..b743a1281d --- /dev/null +++ b/toolkit/components/places/tests/unit/test_bookmark_list.js @@ -0,0 +1,115 @@ +/* Any copyright is dedicated to the Public Domain. +http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const { sinon } = ChromeUtils.importESModule( + "resource://testing-common/Sinon.sys.mjs" +); +const { BookmarkList } = ChromeUtils.importESModule( + "resource://gre/modules/BookmarkList.sys.mjs" +); + +registerCleanupFunction( + async () => await PlacesUtils.bookmarks.eraseEverything() +); + +add_task(async function test_url_tracking() { + const firstBookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: "https://www.example.com/", + }); + const secondBookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: "https://www.reddit.com/", + }); + + let deferredUpdate; + const bookmarkList = new BookmarkList( + [ + "https://www.example.com/", + "https://www.reddit.com/", + "https://www.youtube.com/", + ], + () => deferredUpdate?.resolve() + ); + + async function waitForUpdateBookmarksTask(updateTask) { + deferredUpdate = Promise.withResolvers(); + await updateTask(); + return deferredUpdate.promise; + } + + info("Check bookmark status of tracked URLs."); + equal(await bookmarkList.isBookmark("https://www.example.com/"), true); + equal(await bookmarkList.isBookmark("https://www.reddit.com/"), true); + equal(await bookmarkList.isBookmark("https://www.youtube.com/"), false); + + info("Add a bookmark."); + await waitForUpdateBookmarksTask(() => + PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: "https://www.youtube.com/", + }) + ); + equal(await bookmarkList.isBookmark("https://www.youtube.com/"), true); + + info("Remove a bookmark."); + await waitForUpdateBookmarksTask(() => + PlacesUtils.bookmarks.remove(firstBookmark.guid) + ); + equal(await bookmarkList.isBookmark("https://www.example.com/"), false); + + info("Update a bookmark's URL."); + await waitForUpdateBookmarksTask(() => + PlacesUtils.bookmarks.update({ + guid: secondBookmark.guid, + url: "https://www.wikipedia.org/", + }) + ); + equal(await bookmarkList.isBookmark("https://www.reddit.com/"), false); + + info("Add a bookmark after removing listeners."); + bookmarkList.removeListeners(); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: "https://www.example.com/", + }); + + info("Reinitialize the list and validate bookmark status."); + bookmarkList.setTrackedUrls(["https://www.example.com/"]); + bookmarkList.addListeners(); + equal(await bookmarkList.isBookmark("https://www.example.com/"), true); + + info("Cleanup."); + bookmarkList.removeListeners(); + await PlacesUtils.bookmarks.eraseEverything(); +}); + +add_task(async function test_no_unnecessary_observer_notifications() { + const spy = sinon.spy(); + const bookmarkList = new BookmarkList( + ["https://www.example.com/"], + spy, + 0, + 0 + ); + + info("Add a bookmark with an untracked URL."); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: "https://www.reddit.com/", + }); + await new Promise(resolve => ChromeUtils.idleDispatch(resolve)); + ok(spy.notCalled, "Observer was not notified."); + equal(await bookmarkList.isBookmark("https://www.reddit.com"), undefined); + + info("Add a bookmark after removing listeners."); + bookmarkList.removeListeners(); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + url: "https://www.example.com/", + }); + await new Promise(resolve => ChromeUtils.idleDispatch(resolve)); + ok(spy.notCalled, "Observer was not notified."); +}); diff --git a/toolkit/components/places/tests/unit/test_bookmarks_html.js b/toolkit/components/places/tests/unit/test_bookmarks_html.js index 4b3f04b444..f1f8caa354 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_html.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_html.js @@ -246,7 +246,7 @@ add_task(async function test_import_chromefavicon() { let data = await new Promise(resolve => { PlacesUtils.favicons.getFaviconDataForPage( PAGE_URI, - (uri, dataLen, faviconData, mimeType) => resolve(faviconData) + (uri, dataLen, faviconData) => resolve(faviconData) ); }); diff --git a/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js b/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js index 061c8c0c5f..f7b366b309 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js @@ -112,7 +112,7 @@ var database_check = async function () { await new Promise(resolve => { PlacesUtils.favicons.getFaviconDataForPage( uri(TEST_FAVICON_PAGE_URL), - (aURI, aDataLen, aData, aMimeType) => { + (aURI, aDataLen) => { // aURI should never be null when aDataLen > 0. Assert.notEqual(aURI, null); // Favicon data is stored in the bookmarks file as a "data:" URI. For diff --git a/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js b/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js index 892b2d1d04..75b52aafc7 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js @@ -115,7 +115,7 @@ async function checkObservers(expectPromises, expectedData) { /** * Run after every test cases. */ -async function teardown(file, begin, success, fail) { +async function teardown(file) { // On restore failed, file may not exist, so wrap in try-catch. await IOUtils.remove(file, { ignoreAbsent: true }); diff --git a/toolkit/components/places/tests/unit/test_frecency_decay.js b/toolkit/components/places/tests/unit/test_frecency_decay.js index 8fbb08aecc..a9762209c1 100644 --- a/toolkit/components/places/tests/unit/test_frecency_decay.js +++ b/toolkit/components/places/tests/unit/test_frecency_decay.js @@ -78,5 +78,5 @@ add_task(async function test_frecency_decay() { ); let snapshot = histogram.snapshot(); - Assert.greater(snapshot.sum, 0); + Assert.greater(Object.values(snapshot.values).length, 0); }); diff --git a/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js b/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js index e98cdbac79..2cdcba60aa 100644 --- a/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js +++ b/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js @@ -6,11 +6,11 @@ var resultObserver = { insertedNode: null, - nodeInserted(parent, node, newIndex) { + nodeInserted(parent, node) { this.insertedNode = node; }, removedNode: null, - nodeRemoved(parent, node, oldIndex) { + nodeRemoved(parent, node) { this.removedNode = node; }, @@ -24,14 +24,14 @@ var resultObserver = { newAccessCount: 0, newTime: 0, nodeChangedByHistoryDetails: null, - nodeHistoryDetailsChanged(node, oldVisitDate, oldVisitCount) { + nodeHistoryDetailsChanged(node) { this.nodeChangedByHistoryDetails = node; this.newTime = node.time; this.newAccessCount = node.accessCount; }, movedNode: null, - nodeMoved(node, oldParent, oldIndex, newParent, newIndex) { + nodeMoved(node) { this.movedNode = node; }, openedContainer: null, diff --git a/toolkit/components/places/tests/unit/test_origins.js b/toolkit/components/places/tests/unit/test_origins.js index 67b6d59c7d..f74313a125 100644 --- a/toolkit/components/places/tests/unit/test_origins.js +++ b/toolkit/components/places/tests/unit/test_origins.js @@ -1005,6 +1005,33 @@ add_task(async function moreOriginFrecencyStats() { await cleanUp(); }); +add_task(async function test_cutoff() { + // Add first page with visit. + await PlacesTestUtils.addVisits([{ uri: "http://example.com/0" }]); + // Add a second page last visited before the cutoff, it should be ignored. + let visitDate = PlacesUtils.toPRTime( + new Date( + new Date().setDate( + -Services.prefs.getIntPref("places.frecency.originsCutOffDays", 90) + ) + ) + ); + await PlacesTestUtils.addVisits([{ uri: "http://example.com/1", visitDate }]); + // Add a third page with visit both before and after the cutoff, should count. + await PlacesTestUtils.addVisits([ + { uri: "http://example.com/2" }, + { uri: "http://example.com/2", visitDate }, + ]); + await checkDB([ + [ + "http://", + "example.com", + ["http://example.com/0", "http://example.com/2"], + ], + ]); + await cleanUp(); +}); + /** * Returns the expected frecency of the origin of the given URLs, i.e., the sum * of their frecencies. Each URL is expected to have the same origin. @@ -1017,12 +1044,15 @@ async function expectedOriginFrecency(urls) { let value = 0; for (let url of urls) { let v = Math.max( - await PlacesTestUtils.getDatabaseValue("moz_places", "frecency", { url }), + (await PlacesTestUtils.getDatabaseValue("moz_places", "frecency", { + url, + last_visit_date: [">", 0], + })) ?? 0, 0 ); value += v; } - return value; + return value || 1.0; } /** @@ -1064,49 +1094,34 @@ async function checkDB(expectedOrigins) { } Assert.deepEqual(actualOrigins, expected); if (checkFrecencies) { - await checkStats(expected.map(o => o[2]).filter(o => o > 0)); + info("Checking threshold"); + await PlacesTestUtils.dumpTable({ db, table: "moz_origins" }); + await checkThreshold(expected.map(o => o[2])); } } /** - * Asserts that the origin frecency stats are correct. + * Asserts that the origin frecency threshold is correct. * * @param expectedOriginFrecencies * An array of expected origin frecencies. */ -async function checkStats(expectedOriginFrecencies) { - let stats = await promiseStats(); - Assert.equal(stats.count, expectedOriginFrecencies.length); - Assert.equal( - stats.sum, - expectedOriginFrecencies.reduce((sum, f) => sum + f, 0) +async function checkThreshold(expectedOriginFrecencies) { + const DEFAULT_THRESHOLD = 2.0; + let threshold = await PlacesUtils.metadata.get( + "origin_frecency_threshold", + DEFAULT_THRESHOLD ); + Assert.equal( - stats.squares, - expectedOriginFrecencies.reduce((squares, f) => squares + f * f, 0) + threshold, + expectedOriginFrecencies.length + ? expectedOriginFrecencies.reduce((a, b) => a + b, 0) / + expectedOriginFrecencies.length + : DEFAULT_THRESHOLD ); } -/** - * Returns the origin frecency stats. - * - * @return An object: { count, sum, squares } - */ -async function promiseStats() { - let db = await PlacesUtils.promiseDBConnection(); - let rows = await db.execute(` - SELECT - IFNULL((SELECT value FROM moz_meta WHERE key = 'origin_frecency_count'), 0), - IFNULL((SELECT value FROM moz_meta WHERE key = 'origin_frecency_sum'), 0), - IFNULL((SELECT value FROM moz_meta WHERE key = 'origin_frecency_sum_of_squares'), 0) - `); - return { - count: rows[0].getResultByIndex(0), - sum: rows[0].getResultByIndex(1), - squares: rows[0].getResultByIndex(2), - }; -} - async function cleanUp() { await PlacesUtils.bookmarks.eraseEverything(); await PlacesUtils.history.clear(); diff --git a/toolkit/components/places/tests/unit/test_origins_parsing.js b/toolkit/components/places/tests/unit/test_origins_parsing.js index 35ba8bdd0d..bdeabce271 100644 --- a/toolkit/components/places/tests/unit/test_origins_parsing.js +++ b/toolkit/components/places/tests/unit/test_origins_parsing.js @@ -65,7 +65,16 @@ add_task(async function parsing() { // in the database are correct. for (let i = 0; i < uris.length; i++) { await PlacesUtils.history.remove(uris[i]); - await checkDB(expectedOrigins.slice(i + 1, expectedOrigins.length)); + + let uri = Services.io.newURI(uris[i]); + if (uri.hasUserPass) { + // The history cannot be deleted at a URL with a user path. + } else { + expectedOrigins = expectedOrigins.filter( + ([prefix, hostPort]) => !prefix.startsWith(uri.scheme + ":") + ); + } + await checkDB(expectedOrigins); } await cleanUp(); } diff --git a/toolkit/components/places/tests/unit/test_tag_autocomplete_search.js b/toolkit/components/places/tests/unit/test_tag_autocomplete_search.js index 43f899c237..06182b6dd3 100644 --- a/toolkit/components/places/tests/unit/test_tag_autocomplete_search.js +++ b/toolkit/components/places/tests/unit/test_tag_autocomplete_search.js @@ -33,7 +33,7 @@ AutoCompleteInput.prototype = { popupOpen: false, popup: { - setSelectedIndex(aIndex) {}, + setSelectedIndex() {}, invalidate() {}, // nsISupports implementation diff --git a/toolkit/components/places/tests/unit/xpcshell.toml b/toolkit/components/places/tests/unit/xpcshell.toml index 750e8ad9ea..8a56fcc370 100644 --- a/toolkit/components/places/tests/unit/xpcshell.toml +++ b/toolkit/components/places/tests/unit/xpcshell.toml @@ -78,6 +78,8 @@ skip-if = ["os == 'linux'"] # Bug 821781 ["test_bookmark-tags-changed_frequency.js"] +["test_bookmark_list.js"] + ["test_bookmarks_html.js"] ["test_bookmarks_html_corrupt.js"] |