diff --git a/browser/base/content/browser-places.js b/browser/base/content/browser-places.js index 843921ec0c38..6b4697ec03a7 100644 --- a/browser/base/content/browser-places.js +++ b/browser/base/content/browser-places.js @@ -1674,7 +1674,7 @@ var BookmarkingUI = { // returning any result. let pendingUpdate = this._pendingUpdate = {}; - PlacesUtils.bookmarks.fetch({url: this._uri}, b => guids.add(b.guid)) + PlacesUtils.bookmarks.fetch({url: this._uri}, b => guids.add(b.guid), { concurrent: true }) .catch(Components.utils.reportError) .then(() => { if (pendingUpdate != this._pendingUpdate) { diff --git a/toolkit/components/places/Bookmarks.jsm b/toolkit/components/places/Bookmarks.jsm index 263c40be06b9..b78a947cdbc6 100644 --- a/toolkit/components/places/Bookmarks.jsm +++ b/toolkit/components/places/Bookmarks.jsm @@ -761,6 +761,11 @@ var Bookmarks = Object.freeze({ * object representing it, as defined above. * @param onResult [optional] * Callback invoked for each found bookmark. + * @param options [optional] + * an optional object whose properties describe options for the fetch: + * - concurrent: fetches concurrently to any writes, returning results + * faster. On the negative side, it may return stale + * information missing the currently ongoing write. * * @return {Promise} resolved when the fetch is complete. * @resolves to an object representing the found item, as described above, or @@ -772,7 +777,7 @@ var Bookmarks = Object.freeze({ * @note Any unknown property in the info object is ignored. Known properties * may be overwritten. */ - fetch(guidOrInfo, onResult = null) { + fetch(guidOrInfo, onResult = null, options = {}) { if (onResult && typeof onResult != "function") throw new Error("onResult callback must be a valid function"); let info = guidOrInfo; @@ -812,11 +817,11 @@ var Bookmarks = Object.freeze({ return (async function() { let results; if (fetchInfo.hasOwnProperty("url")) - results = await fetchBookmarksByURL(fetchInfo); + results = await fetchBookmarksByURL(fetchInfo, options && options.concurrent); else if (fetchInfo.hasOwnProperty("guid")) - results = await fetchBookmark(fetchInfo); + results = await fetchBookmark(fetchInfo, options && options.concurrent); else if (fetchInfo.hasOwnProperty("parentGuid") && fetchInfo.hasOwnProperty("index")) - results = await fetchBookmarkByPosition(fetchInfo); + results = await fetchBookmarkByPosition(fetchInfo, options && options.concurrent); if (!results) return null; @@ -1386,10 +1391,8 @@ async function queryBookmarks(info) { // Fetch implementation. -function fetchBookmark(info) { - return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmark", - async function(db) { - +async function fetchBookmark(info, concurrent) { + let query = async function(db) { let rows = await db.executeCached( `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index', b.dateAdded, b.lastModified, b.type, b.title, h.url AS url, @@ -1403,14 +1406,18 @@ function fetchBookmark(info) { `, { guid: info.guid }); return rows.length ? rowsToItemsArray(rows)[0] : null; - }); + }; + if (concurrent) { + let db = await PlacesUtils.promiseDBConnection(); + return query(db); + } + return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmark", + query); } -function fetchBookmarkByPosition(info) { - return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarkByPosition", - async function(db) { +async function fetchBookmarkByPosition(info, concurrent) { + let query = async function(db) { let index = info.index == Bookmarks.DEFAULT_INDEX ? null : info.index; - let rows = await db.executeCached( `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index', b.dateAdded, b.lastModified, b.type, b.title, h.url AS url, @@ -1427,31 +1434,42 @@ function fetchBookmarkByPosition(info) { `, { parentGuid: info.parentGuid, index }); return rows.length ? rowsToItemsArray(rows)[0] : null; - }); + }; + if (concurrent) { + let db = await PlacesUtils.promiseDBConnection(); + return query(db); + } + return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarkByPosition", + query); } -function fetchBookmarksByURL(info) { - return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarksByURL", - async function(db) { - let tagsFolderId = await promiseTagsFolderId(); - let rows = await db.executeCached( - `/* do not warn (bug no): not worth to add an index */ - SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index', - b.dateAdded, b.lastModified, b.type, b.title, h.url AS url, - b.id AS _id, b.parent AS _parentId, - NULL AS _childCount, /* Unused for now */ - p.parent AS _grandParentId, b.syncStatus AS _syncStatus - FROM moz_bookmarks b - JOIN moz_bookmarks p ON p.id = b.parent - JOIN moz_places h ON h.id = b.fk - WHERE h.url_hash = hash(:url) AND h.url = :url - AND _grandParentId <> :tagsFolderId - ORDER BY b.lastModified DESC - `, { url: info.url.href, tagsFolderId }); +async function fetchBookmarksByURL(info, concurrent) { + let query = async function(db) { + let tagsFolderId = await promiseTagsFolderId(); + let rows = await db.executeCached( + `/* do not warn (bug no): not worth to add an index */ + SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index', + b.dateAdded, b.lastModified, b.type, b.title, h.url AS url, + b.id AS _id, b.parent AS _parentId, + NULL AS _childCount, /* Unused for now */ + p.parent AS _grandParentId, b.syncStatus AS _syncStatus + FROM moz_bookmarks b + JOIN moz_bookmarks p ON p.id = b.parent + JOIN moz_places h ON h.id = b.fk + WHERE h.url_hash = hash(:url) AND h.url = :url + AND _grandParentId <> :tagsFolderId + ORDER BY b.lastModified DESC + `, { url: info.url.href, tagsFolderId }); - return rows.length ? rowsToItemsArray(rows) : null; - } - ); + return rows.length ? rowsToItemsArray(rows) : null; + }; + + if (concurrent) { + let db = await PlacesUtils.promiseDBConnection(); + return query(db); + } + return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: fetchBookmarksByURL", + query); } function fetchRecentBookmarks(numberOfItems) { diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js b/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js index e202ea391180..c9768178d8b1 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js @@ -307,6 +307,30 @@ add_task(async function fetch_byurl() { PlacesUtils.tagging.untagURI(uri(bm1.url.href), ["Test Tag"]); }); -function run_test() { - run_next_test(); -} +add_task(async function fetch_concurrent() { + let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: "http://concurrent.url.com/" }); + checkBookmarkObject(bm1); + + let bm2 = await PlacesUtils.bookmarks.fetch({ url: bm1.url }, + gAccumulator.callback, + { concurrent: true }); + checkBookmarkObject(bm2); + Assert.equal(gAccumulator.results.length, 1); + Assert.deepEqual(gAccumulator.results[0], bm1); + Assert.deepEqual(bm1, bm2); + let bm3 = await PlacesUtils.bookmarks.fetch({ url: bm1.url }, + gAccumulator.callback, + { concurrent: false }); + checkBookmarkObject(bm3); + Assert.equal(gAccumulator.results.length, 1); + Assert.deepEqual(gAccumulator.results[0], bm1); + Assert.deepEqual(bm1, bm3); + let bm4 = await PlacesUtils.bookmarks.fetch({ url: bm1.url }, + gAccumulator.callback, + {}); + checkBookmarkObject(bm4); + Assert.equal(gAccumulator.results.length, 1); + Assert.deepEqual(gAccumulator.results[0], bm1); + Assert.deepEqual(bm1, bm4); +}); diff --git a/toolkit/modules/Sqlite.jsm b/toolkit/modules/Sqlite.jsm index 30014e8a2b14..c53b066504ab 100644 --- a/toolkit/modules/Sqlite.jsm +++ b/toolkit/modules/Sqlite.jsm @@ -90,17 +90,16 @@ function logScriptError(message) { } /** - * Gets connection identifier from its database file path. + * Gets connection identifier from its database file name. * - * @param path - * A file string path pointing to a database file. + * @param fileName + * A database file string name. * @return the connection identifier. */ -function getIdentifierByPath(path) { - let basename = OS.Path.basename(path); - let number = connectionCounters.get(basename) || 0; - connectionCounters.set(basename, number + 1); - return basename + "#" + number; +function getIdentifierByFileName(fileName) { + let number = connectionCounters.get(fileName) || 0; + connectionCounters.set(fileName, number + 1); + return fileName + "#" + number; } /** @@ -215,7 +214,7 @@ function ConnectionData(connection, identifier, options = {}) { this._dbConn = connection; // This is a unique identifier for the connection, generated through - // getIdentifierByPath. It may be used for logging or as a key in Maps. + // getIdentifierByFileName. It may be used for logging or as a key in Maps. this._identifier = identifier; this._open = true; @@ -932,7 +931,7 @@ function openConnection(options) { } let file = FileUtils.File(path); - let identifier = getIdentifierByPath(path); + let identifier = getIdentifierByFileName(OS.Path.basename(path)); log.info("Opening database: " + path + " (" + identifier + ")"); @@ -1028,7 +1027,7 @@ function cloneStorageConnection(options) { } let path = source.databaseFile.path; - let identifier = getIdentifierByPath(path); + let identifier = getIdentifierByFileName(OS.Path.basename(path)); log.info("Cloning database: " + path + " (" + identifier + ")"); @@ -1081,10 +1080,9 @@ function wrapStorageConnection(options) { throw new Error("Sqlite.jsm has been shutdown. Cannot wrap connection to: " + connection.database.path); } - let path = connection.databaseFile.path; - let identifier = getIdentifierByPath(path); + let identifier = getIdentifierByFileName(connection.databaseFile.leafName); - log.info("Wrapping database: " + path + " (" + identifier + ")"); + log.info("Wrapping database: " + identifier); return new Promise(resolve => { try { let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);