Bug 418643 - Bookmark folder deletion executes too many SQL statements (r=dietrich)

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -55,7 +55,7 @@
 
 const PRInt32 nsNavBookmarks::kFindBookmarksIndex_ID = 0;
 const PRInt32 nsNavBookmarks::kFindBookmarksIndex_Type = 1;
-const PRInt32 nsNavBookmarks::kFindBookmarksIndex_ForeignKey = 2;
+const PRInt32 nsNavBookmarks::kFindBookmarksIndex_PlaceID = 2;
 const PRInt32 nsNavBookmarks::kFindBookmarksIndex_Parent = 3;
 const PRInt32 nsNavBookmarks::kFindBookmarksIndex_Position = 4;
 const PRInt32 nsNavBookmarks::kFindBookmarksIndex_Title = 5;
@@ -63,7 +63,8 @@ const PRInt32 nsNavBookmarks::kFindBookmarksIndex_Title = 5;
 // These columns sit to the right of the kGetInfoIndex_* columns.
 const PRInt32 nsNavBookmarks::kGetChildrenIndex_Position = 11;
 const PRInt32 nsNavBookmarks::kGetChildrenIndex_Type = 12;
-const PRInt32 nsNavBookmarks::kGetChildrenIndex_ForeignKey = 13;
+const PRInt32 nsNavBookmarks::kGetChildrenIndex_PlaceID = 13;
+const PRInt32 nsNavBookmarks::kGetChildrenIndex_ServiceContractId = 14;
 
 const PRInt32 nsNavBookmarks::kGetItemPropertiesIndex_ID = 0;
 const PRInt32 nsNavBookmarks::kGetItemPropertiesIndex_URI = 1;
@@ -238,7 +239,7 @@ nsNavBookmarks::InitStatements()
         "h.rev_host, h.visit_count, "
           SQL_STR_FRAGMENT_MAX_VISIT_DATE( "h.id" )
           ", f.url, null, b.id, b.dateAdded, b.lastModified, "
-          "b.position, b.type, b.fk "
+          "b.position, b.type, b.fk, b.folder_type "
         "FROM moz_bookmarks b "
         "JOIN moz_places_temp h ON b.fk = h.id "
         "LEFT JOIN moz_favicons f ON h.favicon_id = f.id "
@@ -248,7 +249,7 @@ nsNavBookmarks::InitStatements()
         "h.rev_host, h.visit_count, "
           SQL_STR_FRAGMENT_MAX_VISIT_DATE( "h.id" )
           ", f.url, null, b.id, b.dateAdded, b.lastModified, "
-          "b.position, b.type, b.fk "
+          "b.position, b.type, b.fk, b.folder_type "
         "FROM moz_bookmarks b "
         "LEFT JOIN moz_places h ON b.fk = h.id "
         "LEFT JOIN moz_favicons f ON h.favicon_id = f.id "
@@ -1054,10 +1055,10 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder, nsIURI *aItem, PRInt32 aIndex,
   // If the bookmark has been added to a tag container, notify all
   // bookmark-folder result nodes which contain a bookmark for the new
   // bookmark's url
-  PRInt64 grandParent;
+  PRInt64 grandParentId;
-  rv = GetFolderIdForItem(aFolder, &grandParent);
+  rv = GetFolderIdForItem(aFolder, &grandParentId);
   NS_ENSURE_SUCCESS(rv, rv);
-  if (grandParent == mTagRoot) {
+  if (grandParentId == mTagRoot) {
     // query for all bookmarks for that URI, notify for each
     nsTArray<PRInt64> bookmarks;
 
@@ -1081,7 +1082,7 @@ nsNavBookmarks::RemoveItem(PRInt64 aItemId)
   nsresult rv;
   PRInt32 childIndex;
   PRInt64 placeId, folderId;
-  PRInt32 itemType;
+  PRUint16 itemType;
   nsCAutoString buffer;
   nsCAutoString spec;
 
@@ -1099,7 +1100,7 @@ nsNavBookmarks::RemoveItem(PRInt64 aItemId)
     childIndex = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Position);
     placeId = mDBGetItemProperties->AsInt64(kGetItemPropertiesIndex_PlaceID);
     folderId = mDBGetItemProperties->AsInt64(kGetItemPropertiesIndex_Parent);
-    itemType = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
+    itemType = (PRUint16) mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
     if (itemType == TYPE_BOOKMARK) {
       rv = mDBGetItemProperties->GetUTF8String(kGetItemPropertiesIndex_URI, spec);
       NS_ENSURE_SUCCESS(rv, rv);
@@ -1158,10 +1159,10 @@ nsNavBookmarks::RemoveItem(PRInt64 aItemId)
     // If the removed bookmark was a child of a tag container, notify all
     // bookmark-folder result nodes which contain a bookmark for the removed
     // bookmark's url.
-    PRInt64 grandParent;
+    PRInt64 grandParentId;
-    rv = GetFolderIdForItem(folderId, &grandParent);
+    rv = GetFolderIdForItem(folderId, &grandParentId);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (grandParent == mTagRoot) {
+    if (grandParentId == mTagRoot) {
       nsCOMPtr<nsIURI> uri;
       rv = NS_NewURI(getter_AddRefs(uri), spec);
       NS_ENSURE_SUCCESS(rv, rv);
@@ -1568,7 +1569,8 @@ nsNavBookmarks::RemoveFolder(PRInt64 aFolderId)
   }
 
   // Remove all of the folder's children
-  RemoveFolderChildren(aFolderId);
+  rv = RemoveFolderChildren(aFolderId);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   // Remove the folder from its parent
   nsCAutoString buffer;
@@ -1613,48 +1615,197 @@ nsNavBookmarks::GetRemoveFolderTransaction(PRInt64 aFolder, nsITransaction** aRe
   return NS_OK;
 }
 
-NS_IMETHODIMP
+nsresult
-nsNavBookmarks::RemoveFolderChildren(PRInt64 aFolder)
+nsNavBookmarks::GetDescendantChildren(PRInt64 aFolderId,
-{
+                                      PRInt64 aGrandParentId,
-  mozStorageTransaction transaction(mDBConn, PR_FALSE);
+                                      nsTArray<folderChildrenInfo>& aFolderChildrenArray) {
-
+  // New children will be added from this index on.
-  nsTArray<PRInt64> folderChildren;
+  PRUint32 startIndex = aFolderChildrenArray.Length();
-  nsTArray<PRInt64> itemChildren; // bookmarks / separators
   nsresult rv;
   {
+    // Collect children informations.
     mozStorageStatementScoper scope(mDBGetChildren);
-    rv = mDBGetChildren->BindInt64Parameter(0, aFolder);
+    rv = mDBGetChildren->BindInt64Parameter(0, aFolderId);
     NS_ENSURE_SUCCESS(rv, rv);
 
     PRBool hasMore;
     while (NS_SUCCEEDED(mDBGetChildren->ExecuteStep(&hasMore)) && hasMore) {
-      PRInt32 type = mDBGetChildren->AsInt32(kGetChildrenIndex_Type);
+      folderChildrenInfo child;
-      if (type == TYPE_FOLDER) {
+      child.itemId = mDBGetChildren->AsInt64(nsNavHistory::kGetInfoIndex_ItemId);
-        // folder
+      child.parentId = aFolderId;
-        folderChildren.AppendElement(
+      child.grandParentId = aGrandParentId;
-            mDBGetChildren->AsInt64(nsNavHistory::kGetInfoIndex_ItemId));
+      child.itemType = (PRUint16) mDBGetChildren->AsInt32(kGetChildrenIndex_Type);
-      } else {
+      child.placeId = mDBGetChildren->AsInt64(kGetChildrenIndex_PlaceID);
-        // bookmarks / separators
+      child.index = mDBGetChildren->AsInt32(kGetChildrenIndex_Position);
-        itemChildren.AppendElement(mDBGetChildren->AsInt64(nsNavHistory::kGetInfoIndex_ItemId));
+
+      if (child.itemType == TYPE_BOOKMARK) {
+        nsCAutoString URIString;
+        rv = mDBGetChildren->GetUTF8String(nsNavHistory::kGetInfoIndex_URL,
+                                           URIString);
+        NS_ENSURE_SUCCESS(rv, rv);
+        child.url = URIString;
+      }
+      else if (child.itemType == TYPE_FOLDER) {
+        nsCAutoString folderType;
+        rv = mDBGetChildren->GetUTF8String(kGetChildrenIndex_ServiceContractId,
+                                           folderType);
+        NS_ENSURE_SUCCESS(rv, rv);
+        child.folderType = folderType;
+      }
+      // Append item to children's array.
+      aFolderChildrenArray.AppendElement(child);
+    }
+  }
+
+  // Recursively call GetDescendantChildren for added folders.
+  // We start at startIndex since previous folders are checked
+  // by previous calls to this method.
+  PRUint32 childCount = aFolderChildrenArray.Length();
+  for (PRUint32 i = startIndex; i < childCount; i++) {
+    if (aFolderChildrenArray[i].itemType == TYPE_FOLDER) {
+      GetDescendantChildren(aFolderChildrenArray[i].itemId,
+                            aFolderId,
+                            aFolderChildrenArray);
+    }
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsNavBookmarks::RemoveFolderChildren(PRInt64 aFolderId)
+{
+  nsresult rv;
+  PRUint16 itemType;
+  PRInt64 grandParentId;
+  {
+    mozStorageStatementScoper scope(mDBGetItemProperties);
+    rv = mDBGetItemProperties->BindInt64Parameter(0, aFolderId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    // Sanity check: ensure that item exists.
+    PRBool folderExists;
+    if (NS_FAILED(mDBGetItemProperties->ExecuteStep(&folderExists)) || !folderExists)
+      return NS_ERROR_INVALID_ARG;
+
+    // Sanity check: ensure that this is a folder.
+    itemType = (PRUint16) mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
+    if (itemType != TYPE_FOLDER)
+      return NS_ERROR_INVALID_ARG;
+
+    // Get the grandParent.
+    // We have to do this only once since recursion will give us other
+    // grandParents without the need of additional queries.
+    grandParentId = mDBGetItemProperties->AsInt64(kGetItemPropertiesIndex_Parent);
+  }
+
+  // Fill folder children array recursively.
+  nsTArray<folderChildrenInfo> folderChildrenArray;
+  rv = GetDescendantChildren(aFolderId, grandParentId, folderChildrenArray);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Build a string of folders whose children will be removed.
+  nsCString foldersToRemove;
+  for (PRUint32 i = 0; i < folderChildrenArray.Length(); i++) {
+    folderChildrenInfo child = folderChildrenArray[i];
+    if (child.itemType == TYPE_FOLDER) {
+      foldersToRemove.AppendLiteral(",");
+      foldersToRemove.AppendInt(child.itemId);
+
+      // If this is a dynamic container, try to notify its service that we
+      // are going to remove it.
+      if (child.folderType.Length() > 0) {
+        nsCOMPtr<nsIDynamicContainer> bmcServ =
+          do_GetService(child.folderType.get());
+        if (bmcServ) {
+          rv = bmcServ->OnContainerRemoving(child.itemId);
+          if (NS_FAILED(rv))
+            NS_WARNING("Remove folder container notification failed.");
+        }
       }
     }
   }
 
-  PRUint32 i;
+  // Delete items from the database now.
+  mozStorageTransaction transaction(mDBConn, PR_FALSE);
 
-  // remove folders
+  rv = mDBConn->ExecuteSimpleSQL(
-  for (i = 0; i < folderChildren.Length(); ++i) {
+    NS_LITERAL_CSTRING(
-    rv = RemoveFolder(folderChildren[i]);
+      "DELETE FROM moz_bookmarks "
+      "WHERE parent IN (") +
+        nsPrintfCString("%d", aFolderId) +
+        foldersToRemove +
+      NS_LITERAL_CSTRING(")"));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Clean up orphan items annotations.
+  rv = mDBConn->ExecuteSimpleSQL(
+    NS_LITERAL_CSTRING(
+      "DELETE FROM moz_items_annos "
+      "WHERE id IN ("
+        "SELECT a.id from moz_items_annos a "
+        "LEFT JOIN moz_bookmarks b ON a.item_id = b.id "
+        "WHERE b.id ISNULL)"));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Set the lastModified date.
+  rv = SetItemDateInternal(mDBSetItemLastModified, aFolderId, PR_Now());
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  for (PRUint32 i = 0; i < folderChildrenArray.Length(); i++) {
+    folderChildrenInfo child = folderChildrenArray[i];
+    if (child.itemType == TYPE_BOOKMARK) {
+      PRInt64 placeId = child.placeId;
+      UpdateBookmarkHashOnRemove(placeId);
+
+      // XXX is this too expensive when updating livemarks?
+      // UpdateBookmarkHashOnRemove() does a sanity check using
+      // IsBookmarkedInDatabase(),  so it might not have actually
+      // removed the bookmark.  should we have a boolean out param
+      // for if we actually removed it, and use that to decide if we call
+      // UpdateFrecency() and the rest of this code?
+      rv = History()->UpdateFrecency(placeId, PR_FALSE /* isBookmark */);
       NS_ENSURE_SUCCESS(rv, rv);
     }
-
-  // remove items
-  for (i = 0; i < itemChildren.Length(); ++i) {
-    rv = RemoveItem(itemChildren[i]);
-    NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  transaction.Commit();
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Call observers in reverse order to serve children before their parent.
+  for (PRInt32 i = folderChildrenArray.Length() - 1; i >= 0 ; i--) {
+    folderChildrenInfo child = folderChildrenArray[i];
+
+    ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
+                        OnItemRemoved(child.itemId,
+                                      child.parentId,
+                                      child.index));
+
+    if (child.itemType == TYPE_BOOKMARK) {
+      // If the removed bookmark was a child of a tag container, notify all
+      // bookmark-folder result nodes which contain a bookmark for the removed
+      // bookmark's url.
+
+      if (child.grandParentId == mTagRoot) {
+        nsCOMPtr<nsIURI> uri;
+        rv = NS_NewURI(getter_AddRefs(uri), child.url);
+        NS_ENSURE_SUCCESS(rv, rv);
+
+        nsTArray<PRInt64> bookmarks;
+        rv = GetBookmarkIdsForURITArray(uri, &bookmarks);
+        NS_ENSURE_SUCCESS(rv, rv);
+
+        if (bookmarks.Length()) {
+          for (PRUint32 i = 0; i < bookmarks.Length(); i++) {
+            ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
+                                OnItemChanged(bookmarks[i], NS_LITERAL_CSTRING("tags"),
+                                              PR_FALSE, EmptyCString()))
+          }
+        }
+      }
+    }
+  }
+
   return NS_OK;
 }
 
@@ -1674,7 +1825,8 @@ nsNavBookmarks::MoveItem(PRInt64 aItemId, PRInt64 aNewParent, PRInt32 aIndex)
   // get item properties
   nsresult rv;
   PRInt64 oldParent;
-  PRInt32 oldIndex, itemType;
+  PRInt32 oldIndex;
+  PRUint16 itemType;
   nsCAutoString folderType;
   {
     mozStorageStatementScoper scope(mDBGetItemProperties);
@@ -1690,7 +1842,7 @@ nsNavBookmarks::MoveItem(PRInt64 aItemId, PRInt64 aNewParent, PRInt32 aIndex)
 
     oldParent = mDBGetItemProperties->AsInt64(kGetItemPropertiesIndex_Parent);
     oldIndex = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Position);
-    itemType = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
+    itemType = (PRUint16) mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
     if (itemType == TYPE_FOLDER) {
       rv = mDBGetItemProperties->GetUTF8String(kGetItemPropertiesIndex_ServiceContractId,
                                                folderType);
@@ -2140,7 +2292,7 @@ nsNavBookmarks::ResultNodeForContainer(PRInt64 aID,
   nsCAutoString title;
   rv = mDBGetItemProperties->GetUTF8String(kGetItemPropertiesIndex_Title, title);
 
-  PRInt32 itemType = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
+  PRUint16 itemType = (PRUint16) mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Type);
   if (itemType == TYPE_DYNAMIC_CONTAINER) {
     *aNode = new nsNavHistoryContainerResultNode(EmptyCString(), title, EmptyCString(),
                                                  nsINavHistoryResultNode::RESULT_TYPE_DYNAMIC_CONTAINER,
@@ -2187,7 +2339,7 @@ nsNavBookmarks::QueryFolderChildren(PRInt64 aFolderId,
     // it will start counting at 0 the first time through the loop.
     index ++;
 
-    PRInt32 itemType = mDBGetChildren->AsInt32(kGetChildrenIndex_Type);
+    PRUint16 itemType = (PRUint16) mDBGetChildren->AsInt32(kGetChildrenIndex_Type);
     PRInt64 id = mDBGetChildren->AsInt64(nsNavHistory::kGetInfoIndex_ItemId);
     nsRefPtr<nsNavHistoryResultNode> node;
     if (itemType == TYPE_BOOKMARK) {

toolkit/components/places/src/nsNavBookmarks.h

@@ -165,18 +165,36 @@ private:
 
   nsresult SetItemDateInternal(mozIStorageStatement* aStatement, PRInt64 aItemId, PRTime aValue);
 
+  // Structure to hold folder's children informations
+  typedef struct folderChildrenInfo
+  {
+    PRInt64 itemId;
+    PRUint16 itemType;
+    PRInt64 placeId;
+    PRInt64 parentId;
+    PRInt64 grandParentId;
+    PRInt32 index;
+    nsCString url;
+    nsCString folderType;
+  };
+
+  // Recursive method to build an array of folder's children
+  nsresult GetDescendantChildren(PRInt64 aFolderId,
+                                 PRInt64 aGrandParentId,
+                                 nsTArray<folderChildrenInfo>& aFolderChildrenArray);
+
   // kGetInfoIndex_* results + kGetChildrenIndex_* results
   nsCOMPtr<mozIStorageStatement> mDBGetChildren;
   static const PRInt32 kGetChildrenIndex_Position;
   static const PRInt32 kGetChildrenIndex_Type;
-  static const PRInt32 kGetChildrenIndex_ForeignKey;
+  static const PRInt32 kGetChildrenIndex_PlaceID;
   static const PRInt32 kGetChildrenIndex_FolderTitle;
-  static const PRInt32 kGetChildrenIndex_ID;
+  static const PRInt32 kGetChildrenIndex_ServiceContractId;
 
   nsCOMPtr<mozIStorageStatement> mDBFindURIBookmarks;  // kFindBookmarksIndex_* results
   static const PRInt32 kFindBookmarksIndex_ID;
   static const PRInt32 kFindBookmarksIndex_Type;
-  static const PRInt32 kFindBookmarksIndex_ForeignKey;
+  static const PRInt32 kFindBookmarksIndex_PlaceID;
   static const PRInt32 kFindBookmarksIndex_Parent;
   static const PRInt32 kFindBookmarksIndex_Position;
   static const PRInt32 kFindBookmarksIndex_Title;

toolkit/components/places/tests/unit/test_418643_removeFolderChildren.js

@@ -0,0 +1,169 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Places unit test code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2008
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Marco Bonardo <mak77@bonardo.net> (Original Author)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// Get services.
+try {
+  var histSvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+                getService(Ci.nsINavHistoryService);
+  var bmSvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+              getService(Ci.nsINavBookmarksService);
+  var annoSvc = Cc["@mozilla.org/browser/annotation-service;1"]
+                  .getService(Ci.nsIAnnotationService);
+} catch(ex) {
+  do_throw("Could not get services\n");
+}
+
+var validAnnoName = "validAnno";
+var validItemName = "validItem";
+var deletedAnnoName = "deletedAnno";
+var deletedItemName = "deletedItem";
+var bookmarkedURI = uri("http://www.mozilla.org/");
+// set lastModified to the past to prevent VM timing bugs
+var pastDate = Date.now() * 1000 - 1;
+var deletedBookmarkIds = [];
+
+// bookmarks observer
+var observer = {
+  // cached ordered array of notified items
+  _onItemRemovedItemIds: [],
+  onItemRemoved: function(aItemId, aParentId, aIndex) {
+    // We should first get notifications for children, then for their parent
+    do_check_eq(this._onItemRemovedItemIds.indexOf(aParentId), -1);
+    // Ensure we are not wrongly removing 1 level up
+    do_check_neq(aParentId, bmSvc.toolbarFolder);
+    // Removed item must be one of those we have manually deleted
+    do_check_neq(deletedBookmarkIds.indexOf(aItemId), -1);
+    this._onItemRemovedItemIds.push(aItemId);
+  },
+
+  QueryInterface: function(aIID) {
+    if (aIID.equals(Ci.nsINavBookmarkObserver) ||
+        aIID.equals(Ci.nsISupports)) {
+      return this;
+    }
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  }
+};
+
+bmSvc.addObserver(observer, false);
+
+function add_bookmarks() {
+  // This is the folder we will cleanup
+  var validFolderId = bmSvc.createFolder(bmSvc.toolbarFolder,
+                                         validItemName,
+                                         bmSvc.DEFAULT_INDEX);
+  annoSvc.setItemAnnotation(validFolderId, validAnnoName,
+                            "annotation", 0,
+                            annoSvc.EXPIRE_NEVER);
+  bmSvc.setItemLastModified(validFolderId, pastDate);
+
+  // This bookmark should not be deleted
+  var validItemId = bmSvc.insertBookmark(bmSvc.toolbarFolder,
+                                         bookmarkedURI,
+                                         bmSvc.DEFAULT_INDEX,
+                                         validItemName);
+  annoSvc.setItemAnnotation(validItemId, validAnnoName,
+                            "annotation", 0, annoSvc.EXPIRE_NEVER);
+
+  // The following contents should be deleted
+  var deletedItemId = bmSvc.insertBookmark(validFolderId,
+                                           bookmarkedURI,
+                                           bmSvc.DEFAULT_INDEX,
+                                           deletedItemName);
+  annoSvc.setItemAnnotation(deletedItemId, deletedAnnoName,
+                            "annotation", 0, annoSvc.EXPIRE_NEVER);
+  deletedBookmarkIds.push(deletedItemId);
+
+  var internalFolderId = bmSvc.createFolder(validFolderId,
+                                           deletedItemName,
+                                           bmSvc.DEFAULT_INDEX);
+  annoSvc.setItemAnnotation(internalFolderId, deletedAnnoName,
+                            "annotation", 0, annoSvc.EXPIRE_NEVER);
+  deletedBookmarkIds.push(internalFolderId);
+
+  deletedItemId = bmSvc.insertBookmark(internalFolderId,
+                                       bookmarkedURI,
+                                       bmSvc.DEFAULT_INDEX,
+                                       deletedItemName);
+  annoSvc.setItemAnnotation(deletedItemId, deletedAnnoName,
+                            "annotation", 0, annoSvc.EXPIRE_NEVER);
+  deletedBookmarkIds.push(deletedItemId);
+
+  return validFolderId;
+}
+
+function check_bookmarks(aFolderId) {
+  // check that we still have valid bookmarks
+  var bookmarks = bmSvc.getBookmarkIdsForURI(bookmarkedURI, {});
+  for(var i = 0; i < bookmarks.length; i++) {
+    do_check_eq(bmSvc.getItemTitle(bookmarks[i]), validItemName);
+    do_check_true(annoSvc.itemHasAnnotation(bookmarks[i],validAnnoName));
+  }
+
+  // check that folder exists and has still its annotation
+  do_check_eq(bmSvc.getItemTitle(aFolderId), validItemName);
+  do_check_true(annoSvc.itemHasAnnotation(aFolderId, validAnnoName));
+
+  // check that folder is empty
+  var options = histSvc.getNewQueryOptions();
+  var query = histSvc.getNewQuery();
+  query.setFolders([aFolderId], 1);
+  var result = histSvc.executeQuery(query, options);
+  var root = result.root;
+  root.containerOpen = true;
+  do_check_eq(root.childCount, 0);
+  root.containerOpen = false;
+
+  // test that lastModified got updated
+  do_check_true(pastDate < bmSvc.getItemLastModified(aFolderId));
+
+  // test that all children have been deleted, we use annos for that
+  var deletedItems = annoSvc.getItemsWithAnnotation(deletedAnnoName, {});
+  do_check_eq(deletedItems.length, 0);
+
+  // test that observer has been called for (and only for) deleted items
+  do_check_eq(observer._onItemRemovedItemIds.length, deletedBookmarkIds.length);
+}
+
+// main
+function run_test() {
+  var folderId = add_bookmarks();
+  bmSvc.removeFolderChildren(folderId);
+  check_bookmarks(folderId);
+}