Bug 817232 - Don't apply incoming deletions for special folders. r=gps, a=akeybl
authorRichard Newman <rnewman@mozilla.com>
Tue, 04 Dec 2012 16:49:55 -0800
changeset 118661 b89a1215566a2b2f82539ad688d15f58f5900bcd
parent 118660 fe20163490eca8d585406455e230ccb963cb1cd8
child 118662 308b1b0be4042862e361a30098556928d2dee79d
push id2879
push userrnewman@mozilla.com
push dateFri, 07 Dec 2012 08:35:49 +0000
treeherdermozilla-aurora@b89a1215566a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps, akeybl
bugs817232
milestone19.0a2
Bug 817232 - Don't apply incoming deletions for special folders. r=gps, a=akeybl
services/sync/modules/engines/bookmarks.js
services/sync/tests/unit/test_bookmark_store.js
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -480,24 +480,31 @@ BookmarksStore.prototype = {
       }
     }
     finally {
       tags.containerOpen = false;
     }
   },
   
   applyIncoming: function BStore_applyIncoming(record) {
-    // Don't bother with pre and post-processing for deletions.
+    let isSpecial = record.id in kSpecialIds;
+
     if (record.deleted) {
+      if (isSpecial) {
+        this._log.warn("Ignoring deletion for special record " + record.id);
+        return;
+      }
+
+      // Don't bother with pre and post-processing for deletions.
       Store.prototype.applyIncoming.call(this, record);
       return;
     }
 
     // For special folders we're only interested in child ordering.
-    if ((record.id in kSpecialIds) && record.children) {
+    if (isSpecial && record.children) {
       this._log.debug("Processing special node: " + record.id);
       // Reorder children later
       this._childrenToOrder[record.id] = record.children;
       return;
     }
 
     // Skip malformed records. (Bug 806460.)
     if (record.type == "query" &&
@@ -763,16 +770,21 @@ BookmarksStore.prototype = {
       break;
     default:
       this._log.error("remove: Unknown item type: " + type);
       break;
     }
   },
 
   remove: function BStore_remove(record) {
+    if (kSpecialIds.isSpecialGUID(record.id)) {
+      this._log.warn("Refusing to remove special folder " + record.id);
+      return;
+    }
+
     let itemId = this.idForGUID(record.id);
     if (itemId <= 0) {
       this._log.debug("Item " + record.id + " already removed");
       return;
     }
     this.removeById(itemId, record.id);
   },
 
--- a/services/sync/tests/unit/test_bookmark_store.js
+++ b/services/sync/tests/unit/test_bookmark_store.js
@@ -9,16 +9,40 @@ Cu.import("resource://services-sync/util
 const PARENT_ANNO = "sync/parent";
 
 Service.engineManager.register(BookmarksEngine);
 let engine = Service.engineManager.get("bookmarks");
 let store = engine._store;
 let fxuri = Utils.makeURI("http://getfirefox.com/");
 let tburi = Utils.makeURI("http://getthunderbird.com/");
 
+add_test(function test_ignore_specials() {
+  _("Ensure that we can't delete bookmark roots.");
+
+  // Belt...
+  let record = new BookmarkFolder("bookmarks", "toolbar", "folder");
+  record.deleted = true;
+  do_check_neq(null, store.idForGUID("toolbar"));
+
+  store.applyIncoming(record);
+
+  // Ensure that the toolbar exists.
+  do_check_neq(null, store.idForGUID("toolbar"));
+
+  // This will fail painfully in getItemType if the deletion worked.
+  engine._buildGUIDMap();
+
+  // Braces...
+  store.remove(record);
+  do_check_neq(null, store.idForGUID("toolbar"));
+  engine._buildGUIDMap();
+
+  store.wipe();
+  run_next_test();
+});
 
 add_test(function test_bookmark_create() {
   try {
     _("Ensure the record isn't present yet.");
     let ids = PlacesUtils.bookmarks.getBookmarkIdsForURI(fxuri, {});
     do_check_eq(ids.length, 0);
 
     _("Let's create a new record.");