From e5d15ed6c250bb52dc1567c2389f77c2761915eb Mon Sep 17 00:00:00 2001 From: Kit Cambridge Date: Tue, 20 Feb 2018 22:04:59 -0800 Subject: [PATCH] Bug 1435354 - Remove the keywords cache observer and implement cache invalidation. r=mak MozReview-Commit-ID: dRGyz042JM --HG-- extra : rebase_source : 06e6a38403f2aabe322520d74bbe0a178cd30684 --- .../sync/tests/unit/test_bookmark_tracker.js | 32 -- toolkit/components/places/Bookmarks.jsm | 23 +- .../places/PlacesCategoriesStarter.js | 4 - toolkit/components/places/PlacesUtils.jsm | 381 +++++++++------ .../tests/bookmarks/test_bookmarks_remove.js | 23 +- .../places/tests/bookmarks/test_keywords.js | 452 ++++++++++++++---- .../places/tests/legacy/test_bookmarks.js | 14 +- .../places/tests/unit/test_keywords.js | 24 - .../places/tests/unit/test_sync_utils.js | 8 - .../components/places/toolkitplaces.manifest | 1 - 10 files changed, 609 insertions(+), 353 deletions(-) diff --git a/services/sync/tests/unit/test_bookmark_tracker.js b/services/sync/tests/unit/test_bookmark_tracker.js index 5bbcd7245e18..da06d0fa7a6d 100644 --- a/services/sync/tests/unit/test_bookmark_tracker.js +++ b/services/sync/tests/unit/test_bookmark_tracker.js @@ -774,38 +774,6 @@ add_task(async function test_async_onItemTagged() { } }); -add_task(async function test_onItemKeywordChanged() { - _("Keyword changes via the synchronous API should be tracked"); - - try { - await tracker.stop(); - let folder = PlacesUtils.bookmarks.createFolder( - PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent", - PlacesUtils.bookmarks.DEFAULT_INDEX); - _("Track changes to keywords"); - let uri = CommonUtils.makeURI("http://getfirefox.com"); - let b = PlacesUtils.bookmarks.insertBookmark( - folder, uri, - PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!"); - let bGUID = await PlacesUtils.promiseItemGuid(b); - _("New item is " + b); - _("GUID: " + bGUID); - - await startTracking(); - - _("Give the item a keyword"); - PlacesUtils.bookmarks.setKeywordForBookmark(b, "the_keyword"); - - // bookmark should be tracked, folder should not be. - await verifyTrackedItems([bGUID]); - Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); - - } finally { - _("Clean up."); - await cleanup(); - } -}); - add_task(async function test_async_onItemKeywordChanged() { _("Keyword changes via the asynchronous API should be tracked"); diff --git a/toolkit/components/places/Bookmarks.jsm b/toolkit/components/places/Bookmarks.jsm index 9de339531428..9eb5a8ed5b29 100644 --- a/toolkit/components/places/Bookmarks.jsm +++ b/toolkit/components/places/Bookmarks.jsm @@ -765,6 +765,8 @@ var Bookmarks = Object.freeze({ } } if (updateInfo.hasOwnProperty("url")) { + await PlacesUtils.keywords.reassign(item.url, updatedItem.url, + updatedItem.source); notify(observers, "onItemChanged", [ updatedItem._id, "uri", false, updatedItem.url.href, PlacesUtils.toPRTime(updatedItem.lastModified), @@ -919,6 +921,7 @@ var Bookmarks = Object.freeze({ // We don't wait for the frecency calculation. if (urls && urls.length) { + await PlacesUtils.keywords.eraseEverything(); updateFrecency(db, urls, true).catch(Cu.reportError); } } @@ -1950,24 +1953,20 @@ function removeBookmarks(items, options) { await setAncestorsLastModified(db, guid, new Date(), syncChangeDelta); } - // Write a tombstone for the removed item. + // Write tombstones for the removed items. await insertTombstones(db, items, syncChangeDelta); }); - // Update the frecencies outside of the transaction, so that the updates - // can progress in the background. - for (let item of items) { + // Update the frecencies outside of the transaction, excluding tags, so that + // the updates can progress in the background. + urls = urls.concat(items.filter(item => { let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId; - - // If not a tag recalculate frecency... - if (item.type == Bookmarks.TYPE_BOOKMARK && !isUntagging) { - // ...though we don't wait for the calculation. - updateFrecency(db, [item.url]).catch(Cu.reportError); - } - } + return !isUntagging && "url" in item; + }).map(item => item.url)); if (urls.length) { - updateFrecency(db, urls, true).catch(Cu.reportError); + await PlacesUtils.keywords.removeFromURLsIfNotBookmarked(urls); + updateFrecency(db, urls, urls.length > 1).catch(Cu.reportError); } }); } diff --git a/toolkit/components/places/PlacesCategoriesStarter.js b/toolkit/components/places/PlacesCategoriesStarter.js index 29c871931427..273084c70fe0 100644 --- a/toolkit/components/places/PlacesCategoriesStarter.js +++ b/toolkit/components/places/PlacesCategoriesStarter.js @@ -39,10 +39,6 @@ PlacesCategoriesStarter.prototype = { case TOPIC_GATHER_TELEMETRY: PlacesDBUtils.telemetry(); break; - case PlacesUtils.TOPIC_INIT_COMPLETE: - // Init keywords so it starts listening to bookmarks notifications. - PlacesUtils.keywords; - break; case "idle-daily": // Once a week run places.sqlite maintenance tasks. let lastMaintenance = diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index b5ae6c31e4a7..5ddecdfa66ed 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -78,13 +78,11 @@ async function notifyKeywordChange(url, keyword, source) { // Notify bookmarks about the removal. let bookmarks = []; await PlacesUtils.bookmarks.fetch({ url }, b => bookmarks.push(b)); - // We don't want to yield in the gIgnoreKeywordNotifications section. for (let bookmark of bookmarks) { bookmark.id = await PlacesUtils.promiseItemId(bookmark.guid); bookmark.parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid); } let observers = PlacesUtils.bookmarks.getObservers(); - gIgnoreKeywordNotifications = true; for (let bookmark of bookmarks) { notify(observers, "onItemChanged", [ bookmark.id, "keyword", false, keyword, @@ -95,7 +93,6 @@ async function notifyKeywordChange(url, keyword, source) { "", source ]); } - gIgnoreKeywordNotifications = false; } /** @@ -1889,11 +1886,6 @@ XPCOMUtils.defineLazyServiceGetter(PlacesUtils, "livemarks", "@mozilla.org/browser/livemark-service;2", "mozIAsyncLivemarks"); -XPCOMUtils.defineLazyGetter(PlacesUtils, "keywords", () => { - gKeywordsCachePromise.catch(Cu.reportError); - return Keywords; -}); - XPCOMUtils.defineLazyGetter(this, "bundle", function() { const PLACES_STRING_BUNDLE_URI = "chrome://places/locale/places.properties"; return Services.strings.createBundle(PLACES_STRING_BUNDLE_URI); @@ -1984,7 +1976,7 @@ XPCOMUtils.defineLazyGetter(this, "gAsyncDBWrapperPromised", * - 1 keyword can only point to 1 URL * - 1 URL can have multiple keywords, iff they differ by POST data (included the empty one). */ -var Keywords = { +PlacesUtils.keywords = { /** * Fetches a keyword entry based on keyword or URL. * @@ -2029,7 +2021,7 @@ var Keywords = { } }; - return gKeywordsCachePromise.then(cache => { + return promiseKeywordsCache().then(cache => { let entries = []; if (hasKeyword) { let entry = cache.get(keywordOrEntry.keyword); @@ -2091,8 +2083,8 @@ var Keywords = { // This also checks href for validity url = new URL(url); - return PlacesUtils.withConnectionWrapper("Keywords.insert", async db => { - let cache = await gKeywordsCachePromise; + return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.insert", async db => { + let cache = await promiseKeywordsCache(); // Trying to set the same keyword is a no-op. let oldEntry = cache.get(keyword); @@ -2184,8 +2176,8 @@ var Keywords = { let { keyword, source = Ci.nsINavBookmarksService.SOURCE_DEFAULT } = keywordOrEntry; keyword = keywordOrEntry.keyword.trim().toLowerCase(); - return PlacesUtils.withConnectionWrapper("Keywords.remove", async function(db) { - let cache = await gKeywordsCachePromise; + return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.remove", async db => { + let cache = await promiseKeywordsCache(); if (!cache.has(keyword)) return; let { url } = cache.get(keyword); @@ -2200,145 +2192,238 @@ var Keywords = { // Notify bookmarks about the removal. await notifyKeywordChange(url.href, "", source); }); - } + }, + + /** + * Moves all (keyword, POST data) pairs from one URL to another, and fires + * observer notifications for all affected bookmarks. If the destination URL + * already has keywords, they will be removed and replaced with the source + * URL's keywords. + * + * @param oldURL + * The source URL. + * @param newURL + * The destination URL. + * @param source + * The change source, forwarded to all bookmark observers. + * @return {Promise} + * @resolves when all keywords have been moved to the destination URL. + */ + reassign(oldURL, newURL, source = PlacesUtils.bookmarks.SOURCES.DEFAULT) { + try { + oldURL = BOOKMARK_VALIDATORS.url(oldURL); + } catch (ex) { + throw new Error(oldURL + " is not a valid source URL"); + } + try { + newURL = BOOKMARK_VALIDATORS.url(newURL); + } catch (ex) { + throw new Error(oldURL + " is not a valid destination URL"); + } + return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.reassign", + async function(db) { + let keywordsToReassign = []; + let keywordsToRemove = []; + let cache = await promiseKeywordsCache(); + for (let [keyword, entry] of cache) { + if (entry.url.href == oldURL.href) { + keywordsToReassign.push(keyword); + } + if (entry.url.href == newURL.href) { + keywordsToRemove.push(keyword); + } + } + if (!keywordsToReassign.length) { + return; + } + + await db.executeTransaction(async function() { + // Remove existing keywords from the new URL. + await db.executeCached( + `DELETE FROM moz_keywords WHERE keyword = :keyword`, + keywordsToRemove.map(keyword => ({ keyword }))); + + // Move keywords from the old URL to the new URL. + await db.executeCached(` + UPDATE moz_keywords SET + place_id = (SELECT id FROM moz_places + WHERE url_hash = hash(:newURL) AND + url = :newURL) + WHERE place_id = (SELECT id FROM moz_places + WHERE url_hash = hash(:oldURL) AND + url = :oldURL)`, + { newURL: newURL.href, oldURL: oldURL.href }); + }); + for (let keyword of keywordsToReassign) { + let entry = cache.get(keyword); + entry.url = newURL; + } + for (let keyword of keywordsToRemove) { + cache.delete(keyword); + } + + if (keywordsToReassign.length) { + // If we moved any keywords, notify that we removed all keywords from + // the old and new URLs, then notify for each moved keyword. + await notifyKeywordChange(oldURL, "", source); + await notifyKeywordChange(newURL, "", source); + for (let keyword of keywordsToReassign) { + await notifyKeywordChange(newURL, keyword, source); + } + } else if (keywordsToRemove.length) { + // If the old URL didn't have any keywords, but the new URL did, just + // notify that we removed all keywords from the new URL. + await notifyKeywordChange(oldURL, "", source); + } + }); + }, + + /** + * Removes all orphaned keywords from the given URLs. Orphaned keywords are + * associated with URLs that are no longer bookmarked. If a given URL is still + * bookmarked, its keywords will not be removed. + * + * @param urls + * A list of URLs to check for orphaned keywords. + * @return {Promise} + * @resolves when all keywords have been removed from URLs that are no longer + * bookmarked. + */ + removeFromURLsIfNotBookmarked(urls) { + let hrefs = new Set(); + for (let url of urls) { + try { + url = BOOKMARK_VALIDATORS.url(url); + } catch (ex) { + throw new Error(url + " is not a valid URL"); + } + hrefs.add(url.href); + } + return PlacesUtils.withConnectionWrapper( + "PlacesUtils.keywords.removeFromURLsIfNotBookmarked", + async function(db) { + let keywordsByHref = new Map(); + let cache = await promiseKeywordsCache(); + for (let [keyword, entry] of cache) { + let href = entry.url.href; + if (!hrefs.has(href)) { + continue; + } + if (!keywordsByHref.has(href)) { + keywordsByHref.set(href, [keyword]); + continue; + } + let existingKeywords = keywordsByHref.get(href); + existingKeywords.push(keyword); + } + if (!keywordsByHref.size) { + return; + } + + let placeInfosToRemove = []; + let rows = await db.execute(` + SELECT h.id, h.url + FROM moz_places h + JOIN moz_keywords k ON k.place_id = h.id + GROUP BY h.id + HAVING h.foreign_count = COUNT(*)`); + for (let row of rows) { + placeInfosToRemove.push({ + placeId: row.getResultByName("id"), + href: row.getResultByName("url"), + }); + } + if (!placeInfosToRemove.length) { + return; + } + + await db.execute(`DELETE FROM moz_keywords WHERE place_id IN (${ + Array.from(placeInfosToRemove.map(info => info.placeId)).join()})`); + for (let { href } of placeInfosToRemove) { + let keywords = keywordsByHref.get(href); + for (let keyword of keywords) { + cache.delete(keyword); + } + } + }); + }, + + /** + * Removes all keywords from all URLs. + * + * @return {Promise} + * @resolves when all keywords have been removed. + */ + eraseEverything() { + return PlacesUtils.withConnectionWrapper( + "PlacesUtils.keywords.eraseEverything", + async function(db) { + let cache = await promiseKeywordsCache(); + if (!cache.size) { + return; + } + await db.executeCached(`DELETE FROM moz_keywords`); + cache.clear(); + } + ); + }, + + /** + * Invalidates the keywords cache, leaving all existing keywords in place. + * The cache will be repopulated on the next `PlacesUtils.keywords.*` call. + * + * @return {Promise} + * @resolves when the cache has been cleared. + */ + invalidateCachedKeywords() { + gKeywordsCachePromise = gKeywordsCachePromise.then(_ => null); + return gKeywordsCachePromise; + }, }; -// Set by the keywords API to distinguish notifications fired by the old API. -// Once the old API will be gone, we can remove this and stop observing. -var gIgnoreKeywordNotifications = false; - -XPCOMUtils.defineLazyGetter(this, "gKeywordsCachePromise", () => - PlacesUtils.withConnectionWrapper("PlacesUtils: gKeywordsCachePromise", - async function(db) { - let cache = new Map(); - - // Start observing changes to bookmarks. For now we are going to keep that - // relation for backwards compatibility reasons, but mostly because we are - // lacking a UI to manage keywords directly. - let observer = { - QueryInterface: XPCOMUtils.generateQI(Ci.nsINavBookmarkObserver), - onBeginUpdateBatch() {}, - onEndUpdateBatch() {}, - onItemAdded() {}, - onItemVisited() {}, - onItemMoved() {}, - - onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid, - source) { - if (itemType != PlacesUtils.bookmarks.TYPE_BOOKMARK) - return; - - let keywords = keywordsForHref(uri.spec); - // This uri has no keywords associated, so there's nothing to do. - if (keywords.length == 0) - return; - - (async function() { - // If the uri is not bookmarked anymore, we can remove this keyword. - let bookmark = await PlacesUtils.bookmarks.fetch({ url: uri }); - if (!bookmark) { - for (let keyword of keywords) { - await PlacesUtils.keywords.remove({ keyword, source }); - } - } - })().catch(Cu.reportError); - }, - - onItemChanged(id, prop, isAnno, val, lastMod, itemType, parentId, guid, - parentGuid, oldVal, source) { - if (gIgnoreKeywordNotifications) { - return; - } - - if (prop == "keyword") { - this._onKeywordChanged(guid, val, oldVal); - } else if (prop == "uri") { - this._onUrlChanged(guid, val, oldVal, source).catch(Cu.reportError); - } - }, - - _onKeywordChanged(guid, keyword, href) { - if (keyword.length == 0) { - // We are removing a keyword. - let keywords = keywordsForHref(href); - for (let kw of keywords) { - cache.delete(kw); - } - } else { - // We are adding a new keyword. - cache.set(keyword, { keyword, url: new URL(href) }); - } - }, - - async _onUrlChanged(guid, url, oldUrl, source) { - // Check if the old url is associated with keywords. - let entries = []; - await PlacesUtils.keywords.fetch({ url: oldUrl }, e => entries.push(e)); - if (entries.length == 0) { - return; - } - - // Move the keywords to the new url. - for (let entry of entries) { - await PlacesUtils.keywords.remove({ - keyword: entry.keyword, - source, - }); - await PlacesUtils.keywords.insert({ - keyword: entry.keyword, - url, - postData: entry.postData, - source, - }); - } - }, - }; - - PlacesUtils.bookmarks.addObserver(observer); - PlacesUtils.registerShutdownFunction(() => { - PlacesUtils.bookmarks.removeObserver(observer); - }); - - let rows = await db.execute( - `SELECT keyword, url, post_data - FROM moz_keywords k - JOIN moz_places h ON h.id = k.place_id - `); - let brokenKeywords = []; - for (let row of rows) { - let keyword = row.getResultByName("keyword"); - try { - let entry = { keyword, - url: new URL(row.getResultByName("url")), - postData: row.getResultByName("post_data") || null }; - cache.set(keyword, entry); - } catch (ex) { - // The url is invalid, don't load the keyword and remove it, or it - // would break the whole keywords API. - brokenKeywords.push(keyword); - } - } - - if (brokenKeywords.length) { - await db.execute( - `DELETE FROM moz_keywords - WHERE keyword IN (${brokenKeywords.map(JSON.stringify).join(",")}) - `); - } - - // Helper to get a keyword from an href. - function keywordsForHref(href) { - let keywords = []; - for (let [ key, val ] of cache) { - if (val.url.href == href) - keywords.push(key); - } - return keywords; - } +var gKeywordsCachePromise = Promise.resolve(); +function promiseKeywordsCache() { + let promise = gKeywordsCachePromise.then(function(cache) { + if (cache) { return cache; } -)); + return PlacesUtils.withConnectionWrapper( + "PlacesUtils: promiseKeywordsCache", + async db => { + let cache = new Map(); + let rows = await db.execute( + `SELECT keyword, url, post_data + FROM moz_keywords k + JOIN moz_places h ON h.id = k.place_id + `); + let brokenKeywords = []; + for (let row of rows) { + let keyword = row.getResultByName("keyword"); + try { + let entry = { keyword, + url: new URL(row.getResultByName("url")), + postData: row.getResultByName("post_data") || null }; + cache.set(keyword, entry); + } catch (ex) { + // The url is invalid, don't load the keyword and remove it, or it + // would break the whole keywords API. + brokenKeywords.push(keyword); + } + } + if (brokenKeywords.length) { + await db.execute( + `DELETE FROM moz_keywords + WHERE keyword IN (${brokenKeywords.map(JSON.stringify).join(",")}) + `); + } + return cache; + } + ); + }); + gKeywordsCachePromise = promise.catch(_ => {}); + return promise; +} // Sometime soon, likely as part of the transition to mozIAsyncBookmarks, // itemIds will be deprecated in favour of GUIDs, which play much better diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js index 0206a8b3a1f8..e91790cf665b 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js @@ -121,13 +121,14 @@ add_task(async function remove_multiple_bookmarks() { await Promise.all([frecencyChangedPromise, frecencyChangedPromise1]); - // This second one checks the frecency is changed when we remove the bookmark. - frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0); - frecencyChangedPromise1 = promiseFrecencyChanged("http://example1.com/", 0); + // We should get an onManyFrecenciesChanged notification with the removal of + // multiple bookmarks. + let manyFrencenciesPromise = + PlacesTestUtils.waitForNotification("onManyFrecenciesChanged", () => true, "history"); await PlacesUtils.bookmarks.remove([bm1, bm2]); - await Promise.all([frecencyChangedPromise, frecencyChangedPromise1]); + await manyFrencenciesPromise; }); add_task(async function remove_bookmark_orphans() { @@ -188,15 +189,12 @@ add_task(async function test_contents_removed() { let skipDescendantsObserver = expectNotifications(true); let receiveAllObserver = expectNotifications(false); - let manyFrencenciesPromise = - PlacesTestUtils.waitForNotification("onManyFrecenciesChanged", () => true, "history"); + let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0); await PlacesUtils.bookmarks.remove(folder1); Assert.strictEqual((await PlacesUtils.bookmarks.fetch(folder1.guid)), null); Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null); - // We should get an onManyFrecenciesChanged notification with the removal of - // a folder with children. - await manyFrencenciesPromise; + await frecencyChangedPromise; let expectedNotifications = [{ name: "onItemRemoved", @@ -234,16 +232,13 @@ add_task(async function test_nested_contents_removed() { url: "http://example.com/", title: "" }); - let manyFrencenciesPromise = - PlacesTestUtils.waitForNotification("onManyFrecenciesChanged", () => true, "history"); + let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0); await PlacesUtils.bookmarks.remove(folder1); Assert.strictEqual((await PlacesUtils.bookmarks.fetch(folder1.guid)), null); Assert.strictEqual((await PlacesUtils.bookmarks.fetch(folder2.guid)), null); Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null); - // We should get an onManyFrecenciesChanged notification with the removal of - // a folder with children. - await manyFrencenciesPromise; + await frecencyChangedPromise; }); add_task(async function remove_folder_empty_title() { diff --git a/toolkit/components/places/tests/bookmarks/test_keywords.js b/toolkit/components/places/tests/bookmarks/test_keywords.js index 3eafadee0342..bf4919e94fdd 100644 --- a/toolkit/components/places/tests/bookmarks/test_keywords.js +++ b/toolkit/components/places/tests/bookmarks/test_keywords.js @@ -1,9 +1,12 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + ChromeUtils.defineModuleGetter(this, "Preferences", "resource://gre/modules/Preferences.jsm"); -const URI1 = NetUtil.newURI("http://test1.mozilla.org/"); -const URI2 = NetUtil.newURI("http://test2.mozilla.org/"); -const URI3 = NetUtil.newURI("http://test3.mozilla.org/"); +const URI1 = "http://test1.mozilla.org/"; +const URI2 = "http://test2.mozilla.org/"; +const URI3 = "http://test3.mozilla.org/"; async function check_keyword(aURI, aKeyword) { if (aKeyword) @@ -15,7 +18,7 @@ async function check_keyword(aURI, aKeyword) { let itemId = await PlacesUtils.promiseItemId(bm.guid); let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId); if (keyword && !aKeyword) { - throw (`${aURI.spec} should not have a keyword`); + throw (`${aURI} should not have a keyword, found keyword "${keyword}"`); } else if (aKeyword && keyword == aKeyword) { Assert.equal(keyword, aKeyword); } @@ -23,10 +26,10 @@ async function check_keyword(aURI, aKeyword) { if (aKeyword) { let uri = await PlacesUtils.keywords.fetch(aKeyword); - Assert.equal(uri.url, aURI.spec); + Assert.equal(uri.url, aURI); // Check case insensitivity. uri = await PlacesUtils.keywords.fetch(aKeyword.toUpperCase()); - Assert.equal(uri.url, aURI.spec); + Assert.equal(uri.url, aURI); } } @@ -94,28 +97,22 @@ add_task(async function test_addBookmarkAndKeyword() { let fc = await foreign_count(URI1); let observer = expectNotifications(); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI1, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test"); - - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword"); - let bookmark = await PlacesUtils.bookmarks.fetch({ url: URI1 }); + let bookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI1, + title: "test" + }); + await PlacesUtils.keywords.insert({url: URI1, keyword: "keyword"}); + let itemId = await PlacesUtils.promiseItemId(bookmark.guid); observer.check([ { name: "onItemChanged", arguments: [ itemId, "keyword", false, "keyword", bookmark.lastModified * 1000, bookmark.type, (await PlacesUtils.promiseItemId(bookmark.parentGuid)), - bookmark.guid, bookmark.parentGuid, - bookmark.url.href, + bookmark.guid, bookmark.parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); - await PlacesTestUtils.promiseAsyncUpdates(); - await check_keyword(URI1, "keyword"); Assert.equal((await foreign_count(URI1)), fc + 2); // + 1 bookmark + 1 keyword - - await PlacesTestUtils.promiseAsyncUpdates(); await check_orphans(); }); @@ -124,16 +121,14 @@ add_task(async function test_addBookmarkToURIHavingKeyword() { await check_keyword(URI1, "keyword"); let fc = await foreign_count(URI1); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI1, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test"); + let bookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI1, + title: "test" + }); await check_keyword(URI1, "keyword"); Assert.equal((await foreign_count(URI1)), fc + 1); // + 1 bookmark - - PlacesUtils.bookmarks.removeItem(itemId); - await PlacesTestUtils.promiseAsyncUpdates(); + await PlacesUtils.bookmarks.remove(bookmark); await check_orphans(); }); @@ -149,43 +144,38 @@ add_task(async function test_sameKeywordDifferentURI() { let fc2 = await foreign_count(URI2); let observer = expectNotifications(); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI2, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test2"); + let bookmark2 = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI2, + title: "test2" + }); await check_keyword(URI1, "keyword"); await check_keyword(URI2, null); - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "kEyWoRd"); + await PlacesUtils.keywords.insert({ url: URI2, keyword: "kEyWoRd" }); let bookmark1 = await PlacesUtils.bookmarks.fetch({ url: URI1 }); - let bookmark2 = await PlacesUtils.bookmarks.fetch({ url: URI2 }); observer.check([ { name: "onItemChanged", arguments: [ (await PlacesUtils.promiseItemId(bookmark1.guid)), "keyword", false, "", bookmark1.lastModified * 1000, bookmark1.type, (await PlacesUtils.promiseItemId(bookmark1.parentGuid)), - bookmark1.guid, bookmark1.parentGuid, - bookmark1.url.href, + bookmark1.guid, bookmark1.parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, { name: "onItemChanged", - arguments: [ itemId, "keyword", false, "keyword", + arguments: [ (await PlacesUtils.promiseItemId(bookmark2.guid)), + "keyword", false, "keyword", bookmark2.lastModified * 1000, bookmark2.type, (await PlacesUtils.promiseItemId(bookmark2.parentGuid)), - bookmark2.guid, bookmark2.parentGuid, - bookmark2.url.href, + bookmark2.guid, bookmark2.parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); - await PlacesTestUtils.promiseAsyncUpdates(); // The keyword should have been "moved" to the new URI. await check_keyword(URI1, null); Assert.equal((await foreign_count(URI1)), fc1 - 1); // - 1 keyword await check_keyword(URI2, "keyword"); Assert.equal((await foreign_count(URI2)), fc2 + 2); // + 1 bookmark + 1 keyword - - await PlacesTestUtils.promiseAsyncUpdates(); await check_orphans(); }); @@ -193,58 +183,49 @@ add_task(async function test_sameURIDifferentKeyword() { let fc = await foreign_count(URI2); let observer = expectNotifications(); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI2, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test2"); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI2, + title: "test2" + }); await check_keyword(URI2, "keyword"); - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword2"); - + await PlacesUtils.keywords.insert({ url: URI2, keyword: "keyword2" }); let bookmarks = []; - await PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark)); + await PlacesUtils.bookmarks.fetch({ url: URI2 }, bm => bookmarks.push(bm)); observer.check([ { name: "onItemChanged", arguments: [ (await PlacesUtils.promiseItemId(bookmarks[0].guid)), "keyword", false, "keyword2", bookmarks[0].lastModified * 1000, bookmarks[0].type, (await PlacesUtils.promiseItemId(bookmarks[0].parentGuid)), - bookmarks[0].guid, bookmarks[0].parentGuid, - bookmarks[0].url.href, + bookmarks[0].guid, bookmarks[0].parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, { name: "onItemChanged", arguments: [ (await PlacesUtils.promiseItemId(bookmarks[1].guid)), "keyword", false, "keyword2", bookmarks[1].lastModified * 1000, bookmarks[1].type, (await PlacesUtils.promiseItemId(bookmarks[1].parentGuid)), - bookmarks[1].guid, bookmarks[1].parentGuid, - bookmarks[0].url.href, + bookmarks[1].guid, bookmarks[1].parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); - await PlacesTestUtils.promiseAsyncUpdates(); - await check_keyword(URI2, "keyword2"); Assert.equal((await foreign_count(URI2)), fc + 1); // + 1 bookmark - 1 keyword + 1 keyword - - await PlacesTestUtils.promiseAsyncUpdates(); await check_orphans(); }); add_task(async function test_removeBookmarkWithKeyword() { let fc = await foreign_count(URI2); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI2, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test"); + let bookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI2, + title: "test" + }); // The keyword should not be removed, since there are other bookmarks yet. - PlacesUtils.bookmarks.removeItem(itemId); + await PlacesUtils.bookmarks.remove(bookmark); await check_keyword(URI2, "keyword2"); Assert.equal((await foreign_count(URI2)), fc); // + 1 bookmark - 1 bookmark - - await PlacesTestUtils.promiseAsyncUpdates(); await check_orphans(); }); @@ -252,80 +233,357 @@ add_task(async function test_unsetKeyword() { let fc = await foreign_count(URI2); let observer = expectNotifications(); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI2, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test"); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI2, + title: "test" + }); // The keyword should be removed from any bookmark. - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, null); + await PlacesUtils.keywords.remove("keyword2"); let bookmarks = []; await PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark)); - info(bookmarks.length); + Assert.equal(bookmarks.length, 3, "Check number of bookmarks"); observer.check([ { name: "onItemChanged", arguments: [ (await PlacesUtils.promiseItemId(bookmarks[0].guid)), "keyword", false, "", bookmarks[0].lastModified * 1000, bookmarks[0].type, (await PlacesUtils.promiseItemId(bookmarks[0].parentGuid)), - bookmarks[0].guid, bookmarks[0].parentGuid, - bookmarks[0].url.href, + bookmarks[0].guid, bookmarks[0].parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, { name: "onItemChanged", arguments: [ (await PlacesUtils.promiseItemId(bookmarks[1].guid)), "keyword", false, "", bookmarks[1].lastModified * 1000, bookmarks[1].type, (await PlacesUtils.promiseItemId(bookmarks[1].parentGuid)), - bookmarks[1].guid, bookmarks[1].parentGuid, - bookmarks[1].url.href, + bookmarks[1].guid, bookmarks[1].parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, { name: "onItemChanged", arguments: [ (await PlacesUtils.promiseItemId(bookmarks[2].guid)), "keyword", false, "", bookmarks[2].lastModified * 1000, bookmarks[2].type, (await PlacesUtils.promiseItemId(bookmarks[2].parentGuid)), - bookmarks[2].guid, bookmarks[2].parentGuid, - bookmarks[2].url.href, + bookmarks[2].guid, bookmarks[2].parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); await check_keyword(URI1, null); await check_keyword(URI2, null); Assert.equal((await foreign_count(URI2)), fc); // + 1 bookmark - 1 keyword - - await PlacesTestUtils.promiseAsyncUpdates(); await check_orphans(); }); add_task(async function test_addRemoveBookmark() { + let fc = await foreign_count(URI3); let observer = expectNotifications(); - let itemId = - PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - URI3, - PlacesUtils.bookmarks.DEFAULT_INDEX, - "test3"); - - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword"); - let bookmark = await PlacesUtils.bookmarks.fetch({ url: URI3 }); - let parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid); - PlacesUtils.bookmarks.removeItem(itemId); + let bookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: URI3, + title: "test3" + }); + let itemId = (await PlacesUtils.promiseItemId(bookmark.guid)); + await PlacesUtils.keywords.insert({ url: URI3, keyword: "keyword" }); + await PlacesUtils.bookmarks.remove(bookmark); observer.check([ { name: "onItemChanged", arguments: [ itemId, "keyword", false, "keyword", bookmark.lastModified * 1000, bookmark.type, - parentId, - bookmark.guid, bookmark.parentGuid, - bookmark.url.href, + (await PlacesUtils.promiseItemId(bookmark.parentGuid)), + bookmark.guid, bookmark.parentGuid, "", Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); await check_keyword(URI3, null); - // Don't check the foreign count since the process is async. - // The new test_keywords.js in unit is checking this though. + Assert.equal((await foreign_count(URI3)), fc); // +- 1 bookmark +- 1 keyword + await check_orphans(); +}); + +add_task(async function test_reassign() { + // Should move keywords from old URL to new URL. + info("Old URL with keywords; new URL without keywords"); + { + let oldURL = "http://example.com/1/kw"; + let oldBmk = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + url: oldURL, + }); + await PlacesUtils.keywords.insert({ + url: oldURL, + keyword: "kw1-1", + postData: "a=b", + }); + await PlacesUtils.keywords.insert({ + url: oldURL, + keyword: "kw1-2", + postData: "c=d", + }); + let oldFC = await foreign_count(oldURL); + equal(oldFC, 3); + + let newURL = "http://example.com/2/no-kw"; + let newBmk = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + url: newURL, + }); + let newFC = await foreign_count(newURL); + equal(newFC, 1); + + let observer = expectNotifications(); + await PlacesUtils.keywords.reassign(oldURL, newURL); + observer.check([ { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(oldBmk.guid)), + "keyword", false, "", + oldBmk.lastModified * 1000, oldBmk.type, + (await PlacesUtils.promiseItemId(oldBmk.parentGuid)), + oldBmk.guid, oldBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(newBmk.guid)), + "keyword", false, "", + newBmk.lastModified * 1000, newBmk.type, + (await PlacesUtils.promiseItemId(newBmk.parentGuid)), + newBmk.guid, newBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(newBmk.guid)), + "keyword", false, "kw1-1", + newBmk.lastModified * 1000, newBmk.type, + (await PlacesUtils.promiseItemId(newBmk.parentGuid)), + newBmk.guid, newBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(newBmk.guid)), + "keyword", false, "kw1-2", + newBmk.lastModified * 1000, newBmk.type, + (await PlacesUtils.promiseItemId(newBmk.parentGuid)), + newBmk.guid, newBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + ]); + + + await check_keyword(oldURL, null); + await check_keyword(newURL, "kw1-1"); + await check_keyword(newURL, "kw1-2"); + + equal(await foreign_count(oldURL), oldFC - 2); // Removed both keywords. + equal(await foreign_count(newURL), newFC + 2); // Added two keywords. + } + + // Should not remove any keywords from new URL. + info("Old URL without keywords; new URL with keywords"); + { + let oldURL = "http://example.com/3/no-kw"; + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + url: oldURL, + }); + let oldFC = await foreign_count(oldURL); + equal(oldFC, 1); + + let newURL = "http://example.com/4/kw"; + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + url: newURL, + }); + await PlacesUtils.keywords.insert({ + url: newURL, + keyword: "kw4-1", + }); + let newFC = await foreign_count(newURL); + equal(newFC, 2); + + let observer = expectNotifications(); + await PlacesUtils.keywords.reassign(oldURL, newURL); + observer.check([]); + + await check_keyword(newURL, "kw4-1"); + + equal(await foreign_count(oldURL), oldFC); + equal(await foreign_count(newURL), newFC); + } + + // Should remove all keywords from new URL, then move keywords from old URL. + info("Old URL with keywords; new URL with keywords"); + { + let oldURL = "http://example.com/8/kw"; + let oldBmk = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + url: oldURL, + }); + await PlacesUtils.keywords.insert({ + url: oldURL, + keyword: "kw8-1", + postData: "a=b", + }); + await PlacesUtils.keywords.insert({ + url: oldURL, + keyword: "kw8-2", + postData: "c=d", + }); + let oldFC = await foreign_count(oldURL); + equal(oldFC, 3); + + let newURL = "http://example.com/9/kw"; + let newBmk = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + url: newURL, + }); + await PlacesUtils.keywords.insert({ + url: newURL, + keyword: "kw9-1", + }); + let newFC = await foreign_count(newURL); + equal(newFC, 2); + + let observer = expectNotifications(); + await PlacesUtils.keywords.reassign(oldURL, newURL); + observer.check([ { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(oldBmk.guid)), + "keyword", false, "", + oldBmk.lastModified * 1000, oldBmk.type, + (await PlacesUtils.promiseItemId(oldBmk.parentGuid)), + oldBmk.guid, oldBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(newBmk.guid)), + "keyword", false, "", + newBmk.lastModified * 1000, newBmk.type, + (await PlacesUtils.promiseItemId(newBmk.parentGuid)), + newBmk.guid, newBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(newBmk.guid)), + "keyword", false, "kw8-1", + newBmk.lastModified * 1000, newBmk.type, + (await PlacesUtils.promiseItemId(newBmk.parentGuid)), + newBmk.guid, newBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + { name: "onItemChanged", + arguments: [ (await PlacesUtils.promiseItemId(newBmk.guid)), + "keyword", false, "kw8-2", + newBmk.lastModified * 1000, newBmk.type, + (await PlacesUtils.promiseItemId(newBmk.parentGuid)), + newBmk.guid, newBmk.parentGuid, "", + Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, + ]); + + await check_keyword(oldURL, null); + await check_keyword(newURL, "kw8-1"); + await check_keyword(newURL, "kw8-2"); + + equal(await foreign_count(oldURL), oldFC - 2); // Removed both keywords. + equal(await foreign_count(newURL), newFC + 1); // Removed old keyword; added two keywords. + } + + // Should do nothing. + info("Old URL without keywords; new URL without keywords"); + { + let oldURL = "http://example.com/10/no-kw"; + let oldFC = await foreign_count(oldURL); + + let newURL = "http://example.com/11/no-kw"; + let newFC = await foreign_count(newURL); + + let observer = expectNotifications(); + await PlacesUtils.keywords.reassign(oldURL, newURL); + observer.check([]); + + equal(await foreign_count(oldURL), oldFC); + equal(await foreign_count(newURL), newFC); + } + + await check_orphans(); +}); + +add_task(async function test_invalidation() { + info("Insert bookmarks"); + let fx = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + title: "Get Firefox!", + url: "http://getfirefox.com", + }); + let tb = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + title: "Get Thunderbird!", + url: "http://getthunderbird.com", + }); + + info("Set keywords for bookmarks"); + await PlacesUtils.keywords.insert({ url: fx.url, keyword: "fx" }); + await PlacesUtils.keywords.insert({ url: tb.url, keyword: "tb" }); + + info("Invalidate cached keywords"); + await PlacesUtils.keywords.invalidateCachedKeywords(); + + info("Change URL of bookmark with keyword"); + let promiseNotification = PlacesTestUtils.waitForNotification( + "onItemChanged", + (id, prop, isAnnoProp, newValue, lastModified, type, parentId, guid) => + guid == fx.guid && prop == "keyword" && newValue == "fx" + ); + await PlacesUtils.bookmarks.update({ + guid: fx.guid, + url: "https://www.mozilla.org/firefox", + }); + await promiseNotification; + + let entriesByKeyword = []; + await PlacesUtils.keywords.fetch({ keyword: "fx" }, e => entriesByKeyword.push(e.url.href)); + deepEqual(entriesByKeyword, ["https://www.mozilla.org/firefox"], + "Should return new URL for keyword"); + + ok(!(await PlacesUtils.keywords.fetch({ url: "http://getfirefox.com" })), + "Should not return keywords for old URL"); + + let entiresByURL = []; + await PlacesUtils.keywords.fetch({ url: "https://www.mozilla.org/firefox" }, + e => entiresByURL.push(e.keyword)); + deepEqual(entiresByURL, ["fx"], "Should return keyword for new URL"); + + info("Invalidate cached keywords"); + await PlacesUtils.keywords.invalidateCachedKeywords(); + + info("Remove bookmark with keyword"); + await PlacesUtils.bookmarks.remove(tb.guid); + + ok(!(await PlacesUtils.keywords.fetch({ url: "http://getthunderbird.com" })), + "Should not return keywords for removed bookmark URL"); + + ok(!(await PlacesUtils.keywords.fetch({ keyword: "tb" })), + "Should not return URL for removed bookmark keyword"); await PlacesTestUtils.promiseAsyncUpdates(); await check_orphans(); + + await PlacesUtils.bookmarks.eraseEverything(); +}); + +add_task(async function test_eraseAllBookmarks() { + info("Insert bookmarks"); + let fx = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + title: "Get Firefox!", + url: "http://getfirefox.com", + }); + let tb = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + title: "Get Thunderbird!", + url: "http://getthunderbird.com", + }); + + info("Set keywords for bookmarks"); + await PlacesUtils.keywords.insert({ url: fx.url, keyword: "fx" }); + await PlacesUtils.keywords.insert({ url: tb.url, keyword: "tb" }); + + info("Erase everything"); + await PlacesUtils.bookmarks.eraseEverything(); + + ok(!(await PlacesUtils.keywords.fetch({ keyword: "fx" })), + "Should remove Firefox keyword"); + + ok(!(await PlacesUtils.keywords.fetch({ keyword: "tb" })), + "Should remove Thunderbird keyword"); }); diff --git a/toolkit/components/places/tests/legacy/test_bookmarks.js b/toolkit/components/places/tests/legacy/test_bookmarks.js index 77b5ef8686f9..4223e5e6f284 100644 --- a/toolkit/components/places/tests/legacy/test_bookmarks.js +++ b/toolkit/components/places/tests/legacy/test_bookmarks.js @@ -272,18 +272,6 @@ add_task(async function test_bookmarks() { // test get folder's index let tmpFolder = bs.createFolder(testRoot, "tmp", 2); - // test setKeywordForBookmark - let kwTestItemId = bs.insertBookmark(testRoot, uri("http://keywordtest.com"), - bs.DEFAULT_INDEX, ""); - bs.setKeywordForBookmark(kwTestItemId, "bar"); - - // test getKeywordForBookmark - let k = bs.getKeywordForBookmark(kwTestItemId); - Assert.equal("bar", k); - - // test PlacesUtils.keywords.fetch() - let u = await PlacesUtils.keywords.fetch("bar"); - Assert.equal("http://keywordtest.com/", u.url); // test removeFolderChildren // 1) add/remove each child type (bookmark, separator, folder) tmpFolder = bs.createFolder(testRoot, "removeFolderChildren", @@ -402,7 +390,7 @@ add_task(async function test_bookmarks() { bs.DEFAULT_INDEX, ""); Assert.equal(bookmarksObserver._itemAddedId, newId13); Assert.equal(bookmarksObserver._itemAddedParent, testRoot); - Assert.equal(bookmarksObserver._itemAddedIndex, 10); + Assert.equal(bookmarksObserver._itemAddedIndex, 9); // set bookmark title bs.setItemTitle(newId13, "ZZZXXXYYY"); diff --git a/toolkit/components/places/tests/unit/test_keywords.js b/toolkit/components/places/tests/unit/test_keywords.js index ae2381a3fc0b..b81fece7f557 100644 --- a/toolkit/components/places/tests/unit/test_keywords.js +++ b/toolkit/components/places/tests/unit/test_keywords.js @@ -471,30 +471,6 @@ add_task(async function test_multipleKeywordsSamePostData() { await check_no_orphans(); }); -add_task(async function test_oldKeywordsAPI() { - let bookmark = await PlacesUtils.bookmarks.insert({ url: "http://example.com/", - parentGuid: PlacesUtils.bookmarks.unfiledGuid }); - await check_keyword(false, "http://example.com/", "keyword"); - let itemId = await PlacesUtils.promiseItemId(bookmark.guid); - - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword"); - await promiseKeyword("keyword", "http://example.com/"); - - // Remove the keyword. - PlacesUtils.bookmarks.setKeywordForBookmark(itemId, ""); - await promiseKeyword("keyword", null); - - await PlacesUtils.keywords.insert({ keyword: "keyword", url: "http://example.com" }); - Assert.equal(PlacesUtils.bookmarks.getKeywordForBookmark(itemId), "keyword"); - - let entry = await PlacesUtils.keywords.fetch("keyword"); - Assert.equal(entry.url, "http://example.com/"); - - await PlacesUtils.bookmarks.remove(bookmark); - - await check_no_orphans(); -}); - add_task(async function test_bookmarkURLChange() { let fc1 = await foreign_count("http://example1.com/"); let fc2 = await foreign_count("http://example2.com/"); diff --git a/toolkit/components/places/tests/unit/test_sync_utils.js b/toolkit/components/places/tests/unit/test_sync_utils.js index 6f738442e721..758df2ab5918 100644 --- a/toolkit/components/places/tests/unit/test_sync_utils.js +++ b/toolkit/components/places/tests/unit/test_sync_utils.js @@ -899,14 +899,6 @@ add_task(async function test_update_keyword() { keyword: "test5", }); equal(updatedItem2.keyword, "test5", "New update succeeds"); - await PlacesSyncUtils.bookmarks.update({ - recordId: item2.recordId, - url: item.url.href, - }); - updatedItem2 = await PlacesSyncUtils.bookmarks.fetch(item2.recordId); - equal(updatedItem2.keyword, "test4", "Item keyword has been updated"); - updatedItem = await PlacesSyncUtils.bookmarks.fetch(item.recordId); - equal(updatedItem.keyword, "test4", "Initial item still has keyword"); } diff --git a/toolkit/components/places/toolkitplaces.manifest b/toolkit/components/places/toolkitplaces.manifest index 4dd54c5475d0..451ac3bdf94c 100644 --- a/toolkit/components/places/toolkitplaces.manifest +++ b/toolkit/components/places/toolkitplaces.manifest @@ -17,7 +17,6 @@ category places-init-complete nsPlacesExpiration @mozilla.org/places/expiration; component {803938d5-e26d-4453-bf46-ad4b26e41114} PlacesCategoriesStarter.js contract @mozilla.org/places/categoriesStarter;1 {803938d5-e26d-4453-bf46-ad4b26e41114} category idle-daily PlacesCategoriesStarter @mozilla.org/places/categoriesStarter;1 -category places-init-complete PlacesCategoriesStarter @mozilla.org/places/categoriesStarter;1 # ColorAnalyzer.js component {d056186c-28a0-494e-aacc-9e433772b143} ColorAnalyzer.js