Bug 817232 - Don't apply incoming deletions for special folders. r=gps
authorRichard Newman <rnewman@mozilla.com>
Tue, 04 Dec 2012 16:49:55 -0800
changeset 114987 44799b79143d94adc47eb8a1acc1cee050ba80cd
parent 114986 5aafc6d684f056cf1df0780143ce5c1afd44220e
child 114988 95e6c6b6e939569554982b31cb70ada58e16febe
push id19095
push userrnewman@mozilla.com
push dateWed, 05 Dec 2012 00:50:04 +0000
treeherdermozilla-inbound@44799b79143d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs817232
milestone20.0a1
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
Bug 817232 - Don't apply incoming deletions for special folders. r=gps
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.");