Bug 1372496 - Speed up bookmarks.eraseEverything and bookmarks.remove APIs by handling updating of frencencies outside of the main transaction. r=mak

MozReview-Commit-ID: FuSLHB23yNt

--HG--
extra : rebase_source : 8b08d11ba04845711335ac72d1974c93705b67bd
This commit is contained in:
Mark Banner 2017-06-15 11:38:22 +01:00
Родитель 5704197fbf
Коммит 3309e57749
2 изменённых файлов: 67 добавлений и 29 удалений

Просмотреть файл

@ -701,8 +701,11 @@ var Bookmarks = Object.freeze({
const folderGuids = [this.toolbarGuid, this.menuGuid, this.unfiledGuid,
this.mobileGuid];
return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: eraseEverything",
db => db.executeTransaction(async function() {
await removeFoldersContents(db, folderGuids, options);
async function(db) {
let urls;
await db.executeTransaction(async function() {
urls = await removeFoldersContents(db, folderGuids, options);
const time = PlacesUtils.toPRTime(new Date());
const syncChangeDelta =
PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source);
@ -713,7 +716,13 @@ var Bookmarks = Object.freeze({
WHERE id IN (SELECT id FROM moz_bookmarks WHERE guid = :folderGuid )
`, { folderGuid, time, syncChangeDelta });
}
})
});
// We don't wait for the frecency calculation.
if (urls && urls.length) {
updateFrecency(db, urls, true).then(null, Cu.reportError);
}
}
);
},
@ -1541,6 +1550,7 @@ function fetchBookmarksByParent(info) {
function removeBookmark(item, options) {
return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: removeBookmark",
async function(db) {
let urls;
let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
await db.executeTransaction(async function transaction() {
@ -1549,7 +1559,7 @@ function removeBookmark(item, options) {
if (options.preventRemovalOfNonEmptyFolders && item._childCount > 0) {
throw new Error("Cannot remove a non-empty folder.");
}
await removeFoldersContents(db, [item.guid], options);
urls = await removeFoldersContents(db, [item.guid], options);
}
// Remove annotations first. If it's a tag, we can avoid paying that cost.
@ -1596,6 +1606,10 @@ function removeBookmark(item, options) {
updateFrecency(db, [item.url]).catch(Cu.reportError);
}
if (urls && urls.length && item.type == Bookmarks.TYPE_FOLDER) {
updateFrecency(db, urls, true).catch(Cu.reportError);
}
return item;
});
}
@ -1950,10 +1964,15 @@ var setAncestorsLastModified = async function(db, folderGuid, time, syncChangeDe
/**
* Remove all descendants of one or more bookmark folders.
*
* @param db
* @param {Object} db
* the Sqlite.jsm connection handle.
* @param folderGuids
* @param {Array} folderGuids
* array of folder guids.
* @return {Array}
* An array of urls that will need to be updated for frecency. These
* are returned rather than updated immediately so that the caller
* can decide when they need to be updated - they do not need to
* stop this function from completing.
*/
var removeFoldersContents =
async function(db, folderGuids, options) {
@ -2008,9 +2027,6 @@ async function(db, folderGuids, options) {
// TODO (Bug 1087576): this may leave orphan tags behind.
let urls = itemsRemoved.filter(item => "url" in item).map(item => item.url);
updateFrecency(db, urls, true).then(null, Cu.reportError);
// Send onItemRemoved notifications to listeners.
// TODO (Bug 1087580): for the case of eraseEverything, this should send a
// single clear bookmarks notification rather than notifying for each
@ -2040,6 +2056,7 @@ async function(db, folderGuids, options) {
}
}
}
return itemsRemoved.filter(item => "url" in item).map(item => item.url);
};
/**

Просмотреть файл

@ -174,8 +174,6 @@ add_task(async function remove_folder() {
title: "a folder" });
checkBookmarkObject(bm1);
let manyFrencenciesPromise = promiseManyFrecenciesChanged();
let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
checkBookmarkObject(bm2);
@ -187,11 +185,30 @@ add_task(async function remove_folder() {
Assert.equal(bm2.title, "a folder");
Assert.ok(!("url" in bm2));
// We should get an onManyFrecenciesChanged notification with the remove of
// a folder.
// No promiseManyFrecenciesChanged in this test as the folder doesn't have
// any children that would need updating.
});
add_task(async function test_contents_removed() {
let folder1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "a folder" });
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: folder1.guid,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example.com/",
title: "" });
let manyFrencenciesPromise = promiseManyFrecenciesChanged();
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;
});
add_task(async function test_nested_contents_removed() {
let folder1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
@ -199,12 +216,20 @@ add_task(async function test_nested_contents_removed() {
let folder2 = await PlacesUtils.bookmarks.insert({ parentGuid: folder1.guid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "a folder" });
let sep = await PlacesUtils.bookmarks.insert({ parentGuid: folder2.guid,
type: PlacesUtils.bookmarks.TYPE_SEPARATOR });
let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: folder2.guid,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example.com/",
title: "" });
let manyFrencenciesPromise = promiseManyFrecenciesChanged();
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(sep.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;
});
add_task(async function remove_folder_empty_title() {
@ -248,7 +273,3 @@ add_task(async function test_nested_content_fails_when_not_allowed() {
await Assert.rejects(PlacesUtils.bookmarks.remove(folder1, {preventRemovalOfNonEmptyFolders: true}),
/Cannot remove a non-empty folder./);
});
function run_test() {
run_next_test();
}