author | Margareta Eliza Balazs <ebalazs@mozilla.com> |
Tue, 31 Oct 2017 14:19:52 +0200 | |
changeset 389296 | 88525938b9bc30c2f2040b02d0e3b1c6bfbc5788 |
parent 389295 | 13234a73f3a30a89c9c94b5fb9a4247617b98ed3 |
child 389297 | 2078ffcbf566f9fde2e4632ece87d5e099006a94 |
push id | 32784 |
push user | archaeopteryx@coole-files.de |
push date | Tue, 31 Oct 2017 23:34:29 +0000 |
treeherder | mozilla-central@ae1d655ea7c7 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | backout |
bugs | 1411891 |
milestone | 58.0a1 |
backs out | e17a2bca539148946f045088e18b5674e6f8a0f7 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/components/extensions/ext-bookmarks.js +++ b/browser/components/extensions/ext-bookmarks.js @@ -313,30 +313,30 @@ this.bookmarks = class extends Extension remove: function(id) { let info = { guid: id, }; // The API doesn't give you the old bookmark at the moment try { - return PlacesUtils.bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true}) + return PlacesUtils.bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true}).then(result => {}) .catch(error => Promise.reject({message: error.message})); } catch (e) { return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`}); } }, removeTree: function(id) { let info = { guid: id, }; try { - return PlacesUtils.bookmarks.remove(info) + return PlacesUtils.bookmarks.remove(info).then(result => {}) .catch(error => Promise.reject({message: error.message})); } catch (e) { return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`}); } }, onCreated: new EventManager(context, "bookmarks.onCreated", fire => { let listener = (event, bookmark) => {
--- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -868,33 +868,27 @@ PlacesController.prototype = { * Creates a set of transactions for the removal of a range of items. * A range is an array of adjacent nodes in a view. * @param [in] range * An array of nodes to remove. Should all be adjacent. * @param [out] transactions * An array of transactions. * @param [optional] removedFolders * An array of folder nodes that have already been removed. - * @return {Integer} The total number of items affected. */ async _removeRange(range, transactions, removedFolders) { NS_ASSERT(transactions instanceof Array, "Must pass a transactions array"); if (!removedFolders) removedFolders = []; - let bmGuidsToRemove = []; - let totalItems = 0; - for (var i = 0; i < range.length; ++i) { var node = range[i]; if (this._shouldSkipNode(node, removedFolders)) continue; - totalItems++; - if (PlacesUtils.nodeIsTagQuery(node.parent)) { // This is a uri node inside a tag container. It needs a special // untag transaction. var tagItemId = PlacesUtils.getConcreteItemId(node.parent); var uri = NetUtil.newURI(node.uri); if (PlacesUIUtils.useAsyncTransactions) { let tag = node.parent.title; if (!tag) { @@ -943,47 +937,43 @@ PlacesController.prototype = { } else { // This is a common bookmark item. if (PlacesUtils.nodeIsFolder(node)) { // If this is a folder we add it to our array of folders, used // to skip nodes that are children of an already removed folder. removedFolders.push(node); } if (PlacesUIUtils.useAsyncTransactions) { - bmGuidsToRemove.push(node.bookmarkGuid); + transactions.push( + PlacesTransactions.Remove({ guid: node.bookmarkGuid })); } else { let txn = new PlacesRemoveItemTransaction(node.itemId); transactions.push(txn); } } } - if (bmGuidsToRemove.length) { - transactions.push(PlacesTransactions.Remove({ guids: bmGuidsToRemove })); - } - return totalItems; }, /** * Removes the set of selected ranges from bookmarks. * @param txnName * See |remove|. */ async _removeRowsFromBookmarks(txnName) { - let ranges = this._view.removableSelectionRanges; - let transactions = []; - let removedFolders = []; - let totalItems = 0; + var ranges = this._view.removableSelectionRanges; + var transactions = []; + var removedFolders = []; for (let range of ranges) { - totalItems += await this._removeRange(range, transactions, removedFolders); + await this._removeRange(range, transactions, removedFolders); } if (transactions.length > 0) { if (PlacesUIUtils.useAsyncTransactions) { - await PlacesUIUtils.batchUpdatesForNode(this._view.result, totalItems, async () => { + await PlacesUIUtils.batchUpdatesForNode(this._view.result, transactions.length, async () => { await PlacesTransactions.batch(transactions); }); } else { var txn = new PlacesAggregatedTransaction(txnName, transactions); PlacesUtils.transactionManager.doTransaction(txn); } } },
--- a/toolkit/components/places/Bookmarks.jsm +++ b/toolkit/components/places/Bookmarks.jsm @@ -733,100 +733,84 @@ var Bookmarks = Object.freeze({ // Remove non-enumerable properties. delete updatedItem.source; return Object.assign({}, updatedItem); }); })(); }, /** - * Removes one or more bookmark-items. + * Removes a bookmark-item. * - * @param guidOrInfo This may be: - * - The globally unique identifier of the item to remove - * - an object representing the item, as defined above - * - an array of objects representing the items to be removed + * @param guidOrInfo + * The globally unique identifier of the item to remove, or an + * object representing it, as defined above. * @param {Object} [options={}] * Additional options that can be passed to the function. * Currently supports the following properties: * - preventRemovalOfNonEmptyFolders: Causes an exception to be * thrown when attempting to remove a folder that is not empty. * - source: The change source, forwarded to all bookmark observers. * Defaults to nsINavBookmarksService::SOURCE_DEFAULT. * - * @return {Promise} - * @resolves when the removal is complete + * @return {Promise} resolved when the removal is complete. + * @resolves to an object representing the removed bookmark. * @rejects if the provided guid doesn't match any existing bookmark. * @throws if the arguments are invalid. */ remove(guidOrInfo, options = {}) { - let infos = guidOrInfo; - if (!infos) + let info = guidOrInfo; + if (!info) throw new Error("Input should be a valid object"); - if (!Array.isArray(guidOrInfo)) { - if (typeof(guidOrInfo) != "object") { - infos = [{ guid: guidOrInfo }]; - } else { - infos = [guidOrInfo]; - } + if (typeof(guidOrInfo) != "object") + info = { guid: guidOrInfo }; + + // Disallow removing the root folders. + if ([this.rootGuid, this.menuGuid, this.toolbarGuid, this.unfiledGuid, + this.tagsGuid, this.mobileGuid].includes(info.guid)) { + throw new Error("It's not possible to remove Places root folders."); } if (!("source" in options)) { options.source = Bookmarks.SOURCES.DEFAULT; } - let removeInfos = []; - for (let info of infos) { - // Disallow removing the root folders. - if ([ - Bookmarks.rootGuid, Bookmarks.menuGuid, Bookmarks.toolbarGuid, - Bookmarks.unfiledGuid, Bookmarks.tagsGuid, Bookmarks.mobileGuid - ].includes(info.guid)) { - throw new Error("It's not possible to remove Places root folders."); - } - - // Even if we ignore any other unneeded property, we still validate any - // known property to reduce likelihood of hidden bugs. - let removeInfo = validateBookmarkObject("Bookmarks.jsm: remove", info); - removeInfos.push(removeInfo); - } + // Even if we ignore any other unneeded property, we still validate any + // known property to reduce likelihood of hidden bugs. + let removeInfo = validateBookmarkObject("Bookmarks.jsm: remove", info); return (async function() { - let removeItems = []; - for (let info of removeInfos) { - let item = await fetchBookmark(info); - if (!item) - throw new Error("No bookmarks found for the provided GUID."); + let item = await fetchBookmark(removeInfo); + if (!item) + throw new Error("No bookmarks found for the provided GUID."); - removeItems.push(item); - } - - await removeBookmarks(removeItems, options); + item = await removeBookmark(item, options); // Notify onItemRemoved to listeners. - for (let item of removeItems) { - let observers = PlacesUtils.bookmarks.getObservers(); - let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null; - let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId; - notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index, - item.type, uri, item.guid, - item.parentGuid, - options.source ], - { isTagging: isUntagging }); + let observers = PlacesUtils.bookmarks.getObservers(); + let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null; + let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId; + notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index, + item.type, uri, item.guid, + item.parentGuid, + options.source ], + { isTagging: isUntagging }); - if (isUntagging) { - for (let entry of (await fetchBookmarksByURL(item, true))) { - notify(observers, "onItemChanged", [ entry._id, "tags", false, "", - PlacesUtils.toPRTime(entry.lastModified), - entry.type, entry._parentId, - entry.guid, entry.parentGuid, - "", options.source ]); - } + if (isUntagging) { + for (let entry of (await fetchBookmarksByURL(item, true))) { + notify(observers, "onItemChanged", [ entry._id, "tags", false, "", + PlacesUtils.toPRTime(entry.lastModified), + entry.type, entry._parentId, + entry.guid, entry.parentGuid, + "", options.source ]); } } + + // Remove non-enumerable properties. + return Object.assign({}, item); })(); }, /** * Removes ALL bookmarks, resetting the bookmarks storage to an empty tree. * * Note that roots are preserved, only their children will be removed. * @@ -1521,17 +1505,17 @@ function insertBookmarkTree(items, sourc * * @param {Object} item Livemark item that need to be added. */ async function insertLivemarkData(item) { // Delete the placeholder but note the index of it, so that we can insert the // livemark item at the right place. let placeholder = await Bookmarks.fetch(item.guid); let index = placeholder.index; - await removeBookmarks([item], {source: item.source}); + await removeBookmark(item, {source: item.source}); let feedURI = null; let siteURI = null; item.annos = item.annos.filter(function(aAnno) { switch (aAnno.name) { case PlacesUtils.LMANNO_FEEDURI: feedURI = NetUtil.newURI(aAnno.value); return false; @@ -1798,93 +1782,80 @@ async function fetchBookmarksByParent(db ORDER BY b.position ASC `, { parentGuid: info.parentGuid }); return rowsToItemsArray(rows); } // Remove implementation. -function removeBookmarks(items, options) { - return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: removeBookmarks", +function removeBookmark(item, options) { + return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: removeBookmark", async function(db) { - let urls = []; + let urls; + let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId; await db.executeTransaction(async function transaction() { - let parentGuids = new Set(); - let syncChangeDelta = - PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source); - - for (let item of items) { - parentGuids.add(item.parentGuid); - - // If it's a folder, remove its contents first. - if (item.type == Bookmarks.TYPE_FOLDER) { - if (options.preventRemovalOfNonEmptyFolders && item._childCount > 0) { - throw new Error("Cannot remove a non-empty folder."); - } - urls = urls.concat(await removeFoldersContents(db, [item.guid], options)); + // If it's a folder, remove its contents first. + if (item.type == Bookmarks.TYPE_FOLDER) { + if (options.preventRemovalOfNonEmptyFolders && item._childCount > 0) { + throw new Error("Cannot remove a non-empty folder."); } + urls = await removeFoldersContents(db, [item.guid], options); } - for (let chunk of chunkArray(items, SQLITE_MAX_VARIABLE_NUMBER)) { + // Remove annotations first. If it's a tag, we can avoid paying that cost. + if (!isUntagging) { // We don't go through the annotations service for this cause otherwise // we'd get a pointless onItemChanged notification and it would also // set lastModified to an unexpected value. - await removeAnnotationsForItems(db, chunk); - - // Remove the bookmarks. - await db.executeCached( - `DELETE FROM moz_bookmarks WHERE guid IN (${ - new Array(chunk.length).fill("?").join(",")})`, - chunk.map(item => item.guid) - ); + await removeAnnotationsForItem(db, item._id); } - for (let item of items) { - // Fix indices in the parent. - await db.executeCached( - `UPDATE moz_bookmarks SET position = position - 1 WHERE - parent = :parentId AND position > :index - `, { parentId: item._parentId, index: item.index }); + // Remove the bookmark from the database. + await db.executeCached( + `DELETE FROM moz_bookmarks WHERE guid = :guid`, { guid: item.guid }); + + // Fix indices in the parent. + await db.executeCached( + `UPDATE moz_bookmarks SET position = position - 1 WHERE + parent = :parentId AND position > :index + `, { parentId: item._parentId, index: item.index }); - if (item._grandParentId == PlacesUtils.tagsFolderId) { - // If we're removing a tag entry, increment the change counter for all - // bookmarks with the tagged URL. - await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL( - db, item.url, syncChangeDelta); - } + let syncChangeDelta = + PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source); + + // Mark all affected separators as changed + await adjustSeparatorsSyncCounter(db, item._parentId, item.index, syncChangeDelta); - await adjustSeparatorsSyncCounter(db, item._parentId, item.index, syncChangeDelta); - } - - for (let guid of parentGuids) { - // Mark all affected parents as changed. - await setAncestorsLastModified(db, guid, new Date(), syncChangeDelta); + if (isUntagging) { + // If we're removing a tag entry, increment the change counter for all + // bookmarks with the tagged URL. + await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL( + db, item.url, syncChangeDelta); } // Write a tombstone for the removed item. - await insertTombstones(db, items, syncChangeDelta); + await insertTombstone(db, item, syncChangeDelta); + + await setAncestorsLastModified(db, item.parentGuid, new Date(), + syncChangeDelta); }); - // Update the frecencies outside of the transaction, so that the updates - // can progress in the background. - for (let item of items) { - 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); - } + // 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); } - if (urls.length) { + if (urls && urls.length && item.type == Bookmarks.TYPE_FOLDER) { updateFrecency(db, urls, true).catch(Cu.reportError); } + + return item; }); } // Reorder implementation. function reorderChildren(parent, orderedChildrenGuids, options) { return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: reorderChildren", db => db.executeTransaction(async function() { @@ -2177,25 +2148,24 @@ var removeOrphanAnnotations = async func `); }; /** * Removes annotations for a given item. * * @param db * the Sqlite.jsm connection handle. - * @param items - * The items for which to remove annotations. + * @param itemId + * internal id of the item for which to remove annotations. */ -var removeAnnotationsForItems = async function(db, items) { - // Remove the annotations. - let ids = sqlList(items.map(item => item._id)); +var removeAnnotationsForItem = async function(db, itemId) { await db.executeCached( - `DELETE FROM moz_items_annos WHERE item_id IN (${ids})`, - ); + `DELETE FROM moz_items_annos + WHERE item_id = :id + `, { id: itemId }); await db.executeCached( `DELETE FROM moz_anno_attributes WHERE id IN (SELECT n.id from moz_anno_attributes n LEFT JOIN moz_annos a1 ON a1.anno_attribute_id = n.id LEFT JOIN moz_items_annos a2 ON a2.anno_attribute_id = n.id WHERE a1.id ISNULL AND a2.id ISNULL) `); }; @@ -2477,16 +2447,8 @@ function* chunkArray(array, chunkLength) yield array; return; } let startIndex = 0; while (startIndex < array.length) { yield array.slice(startIndex, startIndex += chunkLength); } } - -/** - * Convert a list of strings or numbers to its SQL - * representation as a string. - */ -function sqlList(list) { - return list.map(JSON.stringify).join(); -}
--- a/toolkit/components/places/PlacesTransactions.jsm +++ b/toolkit/components/places/PlacesTransactions.jsm @@ -1484,43 +1484,39 @@ PT.SortByName.prototype = { /** * Transaction for removing an item (any type). * * Required Input Properties: guids. */ PT.Remove = DefineTransaction(["guids"]); PT.Remove.prototype = { async execute({ guids }) { - let removedItems = []; - - for (let guid of guids) { + let promiseBookmarksTree = async function(guid) { + let tree; try { - // Although we don't strictly need to get this information for the remove, - // we do need it for the possibility of undo(). - removedItems.push(await PlacesUtils.promiseBookmarksTree(guid)); + tree = await PlacesUtils.promiseBookmarksTree(guid); } catch (ex) { throw new Error("Failed to get info for the specified item (guid: " + guid + "): " + ex); } + return tree; + }; + let removedItems = []; + for (let guid of guids) { + removedItems.push(await promiseBookmarksTree(guid)); } - let removeThem = async function() { - let bmsToRemove = []; for (let info of removedItems) { if (info.annos && info.annos.some(anno => anno.name == PlacesUtils.LMANNO_FEEDURI)) { await PlacesUtils.livemarks.removeLivemark({ guid: info.guid }); } else { - bmsToRemove.push({guid: info.guid}); + await PlacesUtils.bookmarks.remove({ guid: info.guid }); } } - - if (bmsToRemove.length) { - await PlacesUtils.bookmarks.remove(bmsToRemove); - } }; await removeThem(); this.undo = async function() { for (let info of removedItems) { await createItemsFromBookmarksTree(info, true); } };
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js @@ -225,61 +225,34 @@ add_task(async function update_move_diff add_task(async function remove_bookmark() { let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK, parentGuid: PlacesUtils.bookmarks.unfiledGuid, url: new URL("http://remove.example.com/") }); let itemId = await PlacesUtils.promiseItemId(bm.guid); let parentId = await PlacesUtils.promiseItemId(bm.parentGuid); let observer = expectNotifications(); - await PlacesUtils.bookmarks.remove(bm.guid); + bm = await PlacesUtils.bookmarks.remove(bm.guid); // TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no // annotations. observer.check([ { name: "onItemRemoved", arguments: [ itemId, parentId, bm.index, bm.type, bm.url, bm.guid, bm.parentGuid, Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); }); -add_task(async function remove_multiple_bookmarks() { - let bm1 = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK, - parentGuid: PlacesUtils.bookmarks.unfiledGuid, - url: "http://remove.example.com/" }); - let bm2 = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK, - parentGuid: PlacesUtils.bookmarks.unfiledGuid, - url: "http://remove1.example.com/" }); - let itemId1 = await PlacesUtils.promiseItemId(bm1.guid); - let parentId1 = await PlacesUtils.promiseItemId(bm1.parentGuid); - let itemId2 = await PlacesUtils.promiseItemId(bm2.guid); - let parentId2 = await PlacesUtils.promiseItemId(bm2.parentGuid); - - let observer = expectNotifications(); - await PlacesUtils.bookmarks.remove([bm1, bm2]); - // TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no - // annotations. - observer.check([ { name: "onItemRemoved", - arguments: [ itemId1, parentId1, bm1.index, bm1.type, bm1.url, - bm1.guid, bm1.parentGuid, - Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }, - { name: "onItemRemoved", - arguments: [ itemId2, parentId2, bm2.index, bm2.type, bm2.url, - bm2.guid, bm2.parentGuid, - Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } - ]); -}); - add_task(async function remove_folder() { let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_FOLDER, parentGuid: PlacesUtils.bookmarks.unfiledGuid }); let itemId = await PlacesUtils.promiseItemId(bm.guid); let parentId = await PlacesUtils.promiseItemId(bm.parentGuid); let observer = expectNotifications(); - await PlacesUtils.bookmarks.remove(bm.guid); + bm = await PlacesUtils.bookmarks.remove(bm.guid); observer.check([ { name: "onItemRemoved", arguments: [ itemId, parentId, bm.index, bm.type, null, bm.guid, bm.parentGuid, Ci.nsINavBookmarksService.SOURCE_DEFAULT ] } ]); }); add_task(async function remove_bookmark_tag_notification() {
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js @@ -62,25 +62,26 @@ add_task(async function remove_roots_fai let guids = [PlacesUtils.bookmarks.rootGuid, PlacesUtils.bookmarks.unfiledGuid, PlacesUtils.bookmarks.menuGuid, PlacesUtils.bookmarks.toolbarGuid, PlacesUtils.bookmarks.tagsGuid, PlacesUtils.bookmarks.mobileGuid]; for (let guid of guids) { Assert.throws(() => PlacesUtils.bookmarks.remove(guid), - /It's not possible to remove Places root folders\./); + /It's not possible to remove Places root folders/); } }); add_task(async function remove_normal_folder_under_root_succeeds() { let folder = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.rootGuid, type: PlacesUtils.bookmarks.TYPE_FOLDER }); checkBookmarkObject(folder); - await PlacesUtils.bookmarks.remove(folder); + let removed_folder = await PlacesUtils.bookmarks.remove(folder); + Assert.deepEqual(folder, removed_folder); Assert.strictEqual((await PlacesUtils.bookmarks.fetch(folder.guid)), null); }); add_task(async function remove_bookmark() { // When removing a bookmark we need to check the frecency. First we confirm // that there is a normal update when it is inserted. let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", UNVISITED_BOOKMARK_BONUS); @@ -90,61 +91,42 @@ add_task(async function remove_bookmark( title: "a bookmark" }); checkBookmarkObject(bm1); await frecencyChangedPromise; // This second one checks the frecency is changed when we remove the bookmark. frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0); - await PlacesUtils.bookmarks.remove(bm1.guid); + let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid); + checkBookmarkObject(bm2); + + Assert.deepEqual(bm1, bm2); + Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid); + Assert.equal(bm2.index, 0); + Assert.deepEqual(bm2.dateAdded, bm2.lastModified); + Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_BOOKMARK); + Assert.equal(bm2.url.href, "http://example.com/"); + Assert.equal(bm2.title, "a bookmark"); await frecencyChangedPromise; }); -add_task(async function remove_multiple_bookmarks() { - // When removing a bookmark we need to check the frecency. First we confirm - // that there is a normal update when it is inserted. - let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", - UNVISITED_BOOKMARK_BONUS); - let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, - type: PlacesUtils.bookmarks.TYPE_BOOKMARK, - url: "http://example.com/", - title: "a bookmark" }); - checkBookmarkObject(bm1); - - let frecencyChangedPromise1 = promiseFrecencyChanged("http://example1.com/", - UNVISITED_BOOKMARK_BONUS); - let bm2 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, - type: PlacesUtils.bookmarks.TYPE_BOOKMARK, - url: "http://example1.com/", - title: "a bookmark" }); - checkBookmarkObject(bm2); - - 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); - - await PlacesUtils.bookmarks.remove([bm1, bm2]); - - await Promise.all([frecencyChangedPromise, frecencyChangedPromise1]); -}); add_task(async function remove_bookmark_orphans() { let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK, url: "http://example.com/", title: "a bookmark" }); checkBookmarkObject(bm1); PlacesUtils.annotations.setItemAnnotation((await PlacesUtils.promiseItemId(bm1.guid)), "testanno", "testvalue", 0, 0); - await PlacesUtils.bookmarks.remove(bm1.guid); + let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid); + checkBookmarkObject(bm2); // Check there are no orphan annotations. let conn = await PlacesUtils.promiseDBConnection(); let annoAttrs = await conn.execute(`SELECT id, name FROM moz_anno_attributes`); // Bug 1306445 will eventually remove the mobile root anno. Assert.equal(annoAttrs.length, 1); Assert.equal(annoAttrs[0].getResultByName("name"), PlacesUtils.MOBILE_ROOT_ANNO); let annos = await conn.execute(`SELECT item_id, anno_attribute_id FROM moz_items_annos`); @@ -155,28 +137,40 @@ add_task(async function remove_bookmark_ add_task(async function remove_bookmark_empty_title() { let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK, url: "http://example.com/", title: "" }); checkBookmarkObject(bm1); - await PlacesUtils.bookmarks.remove(bm1.guid); - Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null); + let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid); + checkBookmarkObject(bm2); + + Assert.deepEqual(bm1, bm2); + Assert.equal(bm2.index, 0); + Assert.strictEqual(bm2.title, ""); }); add_task(async function remove_folder() { let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_FOLDER, title: "a folder" }); checkBookmarkObject(bm1); - await PlacesUtils.bookmarks.remove(bm1.guid); - Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null); + let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid); + checkBookmarkObject(bm2); + + Assert.deepEqual(bm1, bm2); + Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid); + Assert.equal(bm2.index, 0); + Assert.deepEqual(bm2.dateAdded, bm2.lastModified); + Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_FOLDER); + Assert.equal(bm2.title, "a folder"); + Assert.ok(!("url" in bm2)); // No wait for onManyFrecenciesChanged 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, @@ -247,27 +241,39 @@ add_task(async function test_nested_cont }); add_task(async function remove_folder_empty_title() { let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_FOLDER, title: "" }); checkBookmarkObject(bm1); - await PlacesUtils.bookmarks.remove(bm1.guid); - Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null); + let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid); + checkBookmarkObject(bm2); + + Assert.deepEqual(bm1, bm2); + Assert.equal(bm2.index, 0); + Assert.strictEqual(bm2.title, ""); }); add_task(async function remove_separator() { let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_SEPARATOR }); checkBookmarkObject(bm1); - await PlacesUtils.bookmarks.remove(bm1.guid); - Assert.strictEqual((await PlacesUtils.bookmarks.fetch(bm1.guid)), null); + let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid); + checkBookmarkObject(bm2); + + Assert.deepEqual(bm1, bm2); + Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid); + Assert.equal(bm2.index, 0); + Assert.deepEqual(bm2.dateAdded, bm2.lastModified); + Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_SEPARATOR); + Assert.ok(!("url" in bm2)); + Assert.strictEqual(bm2.title, ""); }); add_task(async function test_nested_content_fails_when_not_allowed() { let folder1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_FOLDER, title: "a folder" }); await PlacesUtils.bookmarks.insert({ parentGuid: folder1.guid, type: PlacesUtils.bookmarks.TYPE_FOLDER,