Backed out changeset e17a2bca5391 (bug 1411891) for failing xpcshell in toolkit/modules/tests/xpcshell/test_NewTabUtils.js r=backout on CLOSED TREE
authorMargareta Eliza Balazs <ebalazs@mozilla.com>
Tue, 31 Oct 2017 14:19:52 +0200
changeset 389296 88525938b9bc30c2f2040b02d0e3b1c6bfbc5788
parent 389295 13234a73f3a30a89c9c94b5fb9a4247617b98ed3
child 389297 2078ffcbf566f9fde2e4632ece87d5e099006a94
push id32784
push userarchaeopteryx@coole-files.de
push dateTue, 31 Oct 2017 23:34:29 +0000
treeherdermozilla-central@ae1d655ea7c7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1411891
milestone58.0a1
backs oute17a2bca539148946f045088e18b5674e6f8a0f7
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
Backed out changeset e17a2bca5391 (bug 1411891) for failing xpcshell in toolkit/modules/tests/xpcshell/test_NewTabUtils.js r=backout on CLOSED TREE
browser/components/extensions/ext-bookmarks.js
browser/components/places/content/controller.js
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
--- 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,