Bug 1435354 - Remove the keywords cache observer and implement cache invalidation. r=mak

MozReview-Commit-ID: dRGyz042JM

--HG--
extra : rebase_source : 06e6a38403f2aabe322520d74bbe0a178cd30684
This commit is contained in:
Kit Cambridge 2018-02-20 22:04:59 -08:00
parent d1ba474792
commit e5d15ed6c2
10 changed files with 609 additions and 353 deletions

View File

@ -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");

View File

@ -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);
}
});
}

View File

@ -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 =

View File

@ -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

View File

@ -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() {

View File

@ -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");
});

View File

@ -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");

View File

@ -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/");

View File

@ -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");
}

View File

@ -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