--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -1273,16 +1273,26 @@ SyncEngine.prototype = {
* to have a different GUID
*
* @return GUID of the similar item; falsy otherwise
*/
_findDupe: function (item) {
// By default, assume there's no dupe items for the engine
},
+ // Called when the server has a record marked as deleted, but locally we've
+ // changed it more recently than the deletion. If we return false, the
+ // record will be deleted locally. If we return true, we'll reupload the
+ // record to the server -- any extra work that's needed as part of this
+ // process should be done at this point (such as mark the record's parent
+ // for reuploading in the case of bookmarks).
+ _shouldReviveRemotelyDeletedRecord(remoteItem) {
+ return true;
+ },
+
_deleteId: function (id) {
this._tracker.removeChangedID(id);
// Remember this id to delete at the end of sync
if (this._delete.ids == null)
this._delete.ids = [id];
else
this._delete.ids.push(id);
@@ -1348,25 +1358,28 @@ SyncEngine.prototype = {
// ages. If the item is not modified locally, the remote side wins and
// the deletion is processed. If it is modified locally, we take the
// newer record.
if (!locallyModified) {
this._log.trace("Applying incoming delete because the local item " +
"exists and isn't modified.");
return true;
}
+ this._log.trace("Incoming record is deleted but we had local changes.");
- // TODO As part of bug 720592, determine whether we should do more here.
- // In the case where the local changes are newer, it is quite possible
- // that the local client will restore data a remote client had tried to
- // delete. There might be a good reason for that delete and it might be
- // enexpected for this client to restore that data.
- this._log.trace("Incoming record is deleted but we had local changes. " +
- "Applying the youngest record.");
- return remoteIsNewer;
+ if (remoteIsNewer) {
+ this._log.trace("Remote record is newer -- deleting local record.");
+ return true;
+ }
+ // If the local record is newer, we defer to individual engines for
+ // how to handle this. By default, we revive the record.
+ let willRevive = this._shouldReviveRemotelyDeletedRecord(item);
+ this._log.trace("Local record is newer -- reviving? " + willRevive);
+
+ return !willRevive;
}
// At this point the incoming record is not for a deletion and must have
// data. If the incoming record does not exist locally, we check for a local
// duplicate existing under a different ID. The default implementation of
// _findDupe() is empty, so engines have to opt in to this functionality.
//
// If we find a duplicate, we change the local ID to the incoming ID and we
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -467,25 +467,81 @@ BookmarksEngine.prototype = {
throw {code: Engine.prototype.eEngineAbortApplyIncoming,
cause: ex};
}
delete this._guidMap;
return this._guidMap = guidMap;
});
this._store._childrenToOrder = {};
+ this._store.clearPendingDeletions();
+ },
+
+ _deletePending() {
+ // Delete pending items -- See the comment above BookmarkStore's deletePending
+ let newlyModified = Async.promiseSpinningly(this._store.deletePending());
+ let now = this._tracker._now();
+ this._log.debug("Deleted pending items", newlyModified);
+ for (let modifiedSyncID of newlyModified) {
+ if (!this._modified.has(modifiedSyncID)) {
+ this._modified.set(modifiedSyncID, { timestamp: now, deleted: false });
+ }
+ }
+ },
+
+ // We avoid reviving folders since reviving them properly would require
+ // reviving their children as well. Unfortunately, this is the wrong choice
+ // in the case of a bookmark restore where wipeServer failed -- if the
+ // server has the folder as deleted, we *would* want to reupload this folder.
+ // This is mitigated by the fact that we move any undeleted children to the
+ // grandparent when deleting the parent.
+ _shouldReviveRemotelyDeletedRecord(item) {
+ let kind = Async.promiseSpinningly(
+ PlacesSyncUtils.bookmarks.getKindForSyncId(item.id));
+ if (kind === PlacesSyncUtils.bookmarks.KINDS.FOLDER) {
+ return false;
+ }
+
+ // In addition to preventing the deletion of this record (handled by the caller),
+ // we need to mark the parent of this record for uploading next sync, in order
+ // to ensure its children array is accurate.
+ let modifiedTimestamp = this._modified.getModifiedTimestamp(item.id);
+ if (!modifiedTimestamp) {
+ // We only expect this to be called with items locally modified, so
+ // something strange is going on - play it safe and don't revive it.
+ this._log.error("_shouldReviveRemotelyDeletedRecord called on unmodified item: " + item.id);
+ return false;
+ }
+
+ let localID = this._store.idForGUID(item.id);
+ let localParentID = PlacesUtils.bookmarks.getFolderIdForItem(localID);
+ let localParentSyncID = this._store.GUIDForId(localParentID);
+
+ this._log.trace(`Reviving item "${item.id}" and marking parent ${localParentSyncID} as modified.`);
+
+ if (!this._modified.has(localParentSyncID)) {
+ this._modified.set(localParentSyncID, {
+ timestamp: modifiedTimestamp,
+ deleted: false
+ });
+ }
+ return true
},
_processIncoming: function (newitems) {
try {
SyncEngine.prototype._processIncoming.call(this, newitems);
} finally {
- // Reorder children.
- this._store._orderChildren();
- delete this._store._childrenToOrder;
+ try {
+ this._deletePending();
+ } finally {
+ // Reorder children.
+ this._store._orderChildren();
+ delete this._store._childrenToOrder;
+ }
}
},
_syncFinish: function _syncFinish() {
SyncEngine.prototype._syncFinish.call(this);
this._tracker._ensureMobileQuery();
},
@@ -652,17 +708,18 @@ BookmarksEngine.prototype = {
},
getValidator() {
return new BookmarkValidator();
}
};
function BookmarksStore(name, engine) {
Store.call(this, name, engine);
-
+ this._foldersToDelete = new Set();
+ this._atomsToDelete = new Set();
// Explicitly nullify our references to our cached services so we don't leak
Svc.Obs.add("places-shutdown", function() {
for (let query in this._stmts) {
let stmt = this._stmts[query];
stmt.finalize();
}
this._stmts = {};
}, this);
@@ -727,24 +784,28 @@ BookmarksStore.prototype = {
let item = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
if (item) {
this._log.debug(`Created ${item.kind} ${item.syncId} under ${
item.parentSyncId}`, item);
}
},
remove: function BStore_remove(record) {
- try {
- let info = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.remove(record.id));
- if (info) {
- this._log.debug(`Removed item ${record.id} with type ${record.type}`);
- }
- } catch (ex) {
- // Likely already removed.
- this._log.debug(`Error removing ${record.id}`, ex);
+ if (PlacesSyncUtils.bookmarks.isRootSyncID(record.id)) {
+ this._log.warn("Refusing to remove special folder " + record.id);
+ return;
+ }
+ let recordKind = Async.promiseSpinningly(
+ PlacesSyncUtils.bookmarks.getKindForSyncId(record.id));
+ let isFolder = recordKind === PlacesSyncUtils.bookmarks.KINDS.FOLDER;
+ this._log.trace(`Buffering removal of item "${record.id}" of type "${recordKind}".`);
+ if (isFolder) {
+ this._foldersToDelete.add(record.id);
+ } else {
+ this._atomsToDelete.add(record.id);
}
},
update: function BStore_update(record) {
let info = record.toSyncBookmark();
let item = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.update(info));
if (item) {
this._log.debug(`Updated ${item.kind} ${item.syncId} under ${
@@ -757,16 +818,142 @@ BookmarksStore.prototype = {
let children = this._childrenToOrder[syncID];
return PlacesSyncUtils.bookmarks.order(syncID, children).catch(ex => {
this._log.debug(`Could not order children for ${syncID}`, ex);
});
});
Async.promiseSpinningly(Promise.all(promises));
},
+ // There's some complexity here around pending deletions. Our goals:
+ //
+ // - Don't delete any bookmarks a user has created but not explicitly deleted
+ // (This includes any bookmark that was not a child of the folder at the
+ // time the deletion was recorded, and also bookmarks restored from a backup).
+ // - Don't undelete any bookmark without ensuring the server structure
+ // includes it (see `BookmarkEngine.prototype._shouldReviveRemotelyDeletedRecord`)
+ //
+ // This leads the following approach:
+ //
+ // - Additions, moves, and updates are processed before deletions.
+ // - To do this, all deletion operations are buffered during a sync. Folders
+ // we plan on deleting have their sync id's stored in `this._foldersToDelete`,
+ // and non-folders we plan on deleting have their sync id's stored in
+ // `this._atomsToDelete`.
+ // - The exception to this is the moves that occur to fix the order of bookmark
+ // children, which are performed after we process deletions.
+ // - Non-folders are deleted before folder deletions, so that when we process
+ // folder deletions we know the correct state.
+ // - Remote deletions always win for folders, but do not result in recursive
+ // deletion of children. This is a hack because we're not able to distinguish
+ // between value changes and structural changes to folders, and we don't even
+ // have the old server record to compare to. See `BookmarkEngine`'s
+ // `_shouldReviveRemotelyDeletedRecord` method.
+ // - When a folder is deleted, its remaining children are moved in order to
+ // their closest living ancestor. If this is interrupted (unlikely, but
+ // possible given that we don't perform this operation in a transaction),
+ // we revive the folder.
+ // - Remote deletions can lose for non-folders, but only until we handle
+ // bookmark restores correctly (removing stale state from the server -- this
+ // is to say, if bug 1230011 is fixed, we should never revive bookmarks).
+
+ deletePending: Task.async(function* deletePending() {
+ yield this._deletePendingAtoms();
+ let guidsToUpdate = yield this._deletePendingFolders();
+ this.clearPendingDeletions();
+ return guidsToUpdate;
+ }),
+
+ clearPendingDeletions() {
+ this._foldersToDelete.clear();
+ this._atomsToDelete.clear();
+ },
+
+ _deleteAtom: Task.async(function* _deleteAtom(syncID) {
+ try {
+ let info = yield PlacesSyncUtils.bookmarks.remove(syncID, {
+ preventRemovalOfNonEmptyFolders: true
+ });
+ this._log.trace(`Removed item ${syncID} with type ${info.type}`);
+ } catch (ex) {
+ // Likely already removed.
+ this._log.trace(`Error removing ${syncID}`, ex);
+ }
+ }),
+
+ _deletePendingAtoms() {
+ return Promise.all(
+ [...this._atomsToDelete.values()]
+ .map(syncID => this._deleteAtom(syncID)));
+ },
+
+ // Returns an array of sync ids that need updates.
+ _deletePendingFolders: Task.async(function* _deletePendingFolders() {
+ // To avoid data loss, we don't want to just delete the folder outright,
+ // so we buffer folder deletions and process them at the end (now).
+ //
+ // At this point, any member in the folder that remains is either a folder
+ // pending deletion (which we'll get to in this function), or an item that
+ // should not be deleted. To avoid deleting these items, we first move them
+ // to the parent of the folder we're about to delete.
+ let needUpdate = new Set();
+ for (let syncId of this._foldersToDelete) {
+ let childSyncIds = yield PlacesSyncUtils.bookmarks.fetchChildSyncIds(syncId);
+ if (!childSyncIds.length) {
+ // No children -- just delete the folder.
+ yield this._deleteAtom(syncId)
+ continue;
+ }
+ // We could avoid some redundant work here by finding the nearest
+ // grandparent who isn't present in `this._toDelete`...
+
+ let grandparentSyncId = this.GUIDForId(
+ PlacesUtils.bookmarks.getFolderIdForItem(
+ this.idForGUID(PlacesSyncUtils.bookmarks.syncIdToGuid(syncId))));
+
+ this._log.trace(`Moving ${childSyncIds.length} children of "${syncId}" to ` +
+ `grandparent "${grandparentSyncId}" before deletion.`);
+
+ // Move children out of the parent and into the grandparent
+ yield Promise.all(childSyncIds.map(child => PlacesSyncUtils.bookmarks.update({
+ syncId: child,
+ parentSyncId: grandparentSyncId
+ })));
+
+ // Delete the (now empty) parent
+ try {
+ yield PlacesSyncUtils.bookmarks.remove(syncId, {
+ preventRemovalOfNonEmptyFolders: true
+ });
+ } catch (e) {
+ // We failed, probably because someone added something to this folder
+ // between when we got the children and now (or the database is corrupt,
+ // or something else happened...) This is unlikely, but possible. To
+ // avoid corruption in this case, we need to reupload the record to the
+ // server.
+ //
+ // (Ideally this whole operation would be done in a transaction, and this
+ // wouldn't be possible).
+ needUpdate.add(syncId);
+ }
+
+ // Add children (for parentid) and grandparent (for children list) to set
+ // of records needing an update, *unless* they're marked for deletion.
+ if (!this._foldersToDelete.has(grandparentSyncId)) {
+ needUpdate.add(grandparentSyncId);
+ }
+ for (let childSyncId of childSyncIds) {
+ if (!this._foldersToDelete.has(childSyncId)) {
+ needUpdate.add(childSyncId);
+ }
+ }
+ }
+ return [...needUpdate];
+ }),
+
changeItemID: function BStore_changeItemID(oldID, newID) {
this._log.debug("Changing GUID " + oldID + " to " + newID);
Async.promiseSpinningly(PlacesSyncUtils.bookmarks.changeGuid(oldID, newID));
},
// Create a record starting from the weave id (places guid)
createRecord: function createRecord(id, collection) {
@@ -869,16 +1056,17 @@ BookmarksStore.prototype = {
let syncID = PlacesSyncUtils.bookmarks.guidToSyncId(guid);
items[syncID] = { modified: 0, deleted: false };
}
return items;
},
wipe: function BStore_wipe() {
+ this.clearPendingDeletions();
Async.promiseSpinningly(Task.spawn(function* () {
// Save a backup before clearing out all bookmarks.
yield PlacesBackups.create(null, true);
yield PlacesUtils.bookmarks.eraseEverything({
source: SOURCE_SYNC,
});
}));
}
--- a/services/sync/tests/tps/all_tests.json
+++ b/services/sync/tests/tps/all_tests.json
@@ -1,9 +1,10 @@
{ "tests": [
+ "test_bookmark_conflict.js",
"test_sync.js",
"test_prefs.js",
"test_tabs.js",
"test_passwords.js",
"test_history.js",
"test_formdata.js",
"test_bug530717.js",
"test_bug531489.js",
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/tps/test_bookmark_conflict.js
@@ -0,0 +1,143 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * The list of phases mapped to their corresponding profiles. The object
+ * here must be in strict JSON format, as it will get parsed by the Python
+ * testrunner (no single quotes, extra comma's, etc).
+ */
+EnableEngines(["bookmarks"]);
+
+var phases = { "phase1": "profile1",
+ "phase2": "profile2",
+ "phase3": "profile1",
+ "phase4": "profile2" };
+
+
+// the initial list of bookmarks to add to the browser
+var bookmarksInitial = {
+ "menu": [
+ { folder: "foldera" },
+ { folder: "folderb" },
+ { folder: "folderc" },
+ { folder: "folderd" },
+ ],
+
+ "menu/foldera": [{ uri: "http://www.cnn.com", title: "CNN" }],
+ "menu/folderb": [{ uri: "http://www.apple.com", title: "Apple", tags: [] }],
+ "menu/folderc": [{ uri: "http://www.yahoo.com", title: "Yahoo" }],
+
+ "menu/folderd": []
+};
+
+// a list of bookmarks to delete during a 'delete' action on P2
+var bookmarksToDelete = {
+ "menu": [
+ { folder: "foldera" },
+ { folder: "folderb" },
+ ],
+ "menu/folderc": [{ uri: "http://www.yahoo.com", title: "Yahoo" }],
+};
+
+
+// the modifications to make on P1, after P2 has synced, but before P1 has gotten
+// P2's changes
+var bookmarkMods = {
+ "menu": [
+ { folder: "foldera" },
+ { folder: "folderb" },
+ { folder: "folderc" },
+ { folder: "folderd" },
+ ],
+
+ // we move this child out of its folder (p1), after deleting the folder (p2)
+ // and expect the child to come back to p2 after sync.
+ "menu/foldera": [{
+ uri: "http://www.cnn.com",
+ title: "CNN",
+ changes: { location: "menu/folderd" }
+ }],
+
+ // we rename this child (p1) after deleting the folder (p2), and expect the child
+ // to be moved into great grandparent (menu)
+ "menu/folderb": [{
+ uri: "http://www.apple.com",
+ title: "Apple",
+ tags: [],
+ changes: { title: "Mac" }
+ }],
+
+
+ // we move this child (p1) after deleting the child (p2) and expect it to survive
+ "menu/folderc": [{
+ uri: "http://www.yahoo.com",
+ title: "Yahoo",
+ changes: { location: "menu/folderd" }
+ }],
+
+ "menu/folderd": []
+};
+
+// a list of bookmarks to delete during a 'delete' action
+var bookmarksToDelete = {
+ "menu": [
+ { folder: "foldera" },
+ { folder: "folderb" },
+ ],
+ "menu/folderc": [
+ { uri: "http://www.yahoo.com", title: "Yahoo" },
+ ],
+};
+
+
+
+// expected bookmark state after conflict resolution
+var bookmarksExpected = {
+ "menu": [
+ { folder: "folderc" },
+ { folder: "folderd" },
+ { uri: "http://www.apple.com", title: "Mac", },
+ ],
+
+ "menu/folderc": [],
+
+ "menu/folderd": [
+ { uri: "http://www.cnn.com", title: "CNN" },
+ { uri: "http://www.yahoo.com", title: "Yahoo" }
+ ]
+};
+
+// Add bookmarks to profile1 and sync.
+Phase("phase1", [
+ [Bookmarks.add, bookmarksInitial],
+ [Bookmarks.verify, bookmarksInitial],
+ [Sync],
+ [Bookmarks.verify, bookmarksInitial],
+]);
+
+// Sync to profile2 and verify that the bookmarks are present. Delete
+// bookmarks/folders, verify that it's not present, and sync
+Phase("phase2", [
+ [Sync],
+ [Bookmarks.verify, bookmarksInitial],
+ [Bookmarks.delete, bookmarksToDelete],
+ [Bookmarks.verifyNot, bookmarksToDelete],
+ [Sync]
+]);
+
+// Using profile1, modify the bookmarks, and sync *after* the modification,
+// and then sync again to propagate the reconciliation changes.
+Phase("phase3", [
+ [Bookmarks.verify, bookmarksInitial],
+ [Bookmarks.modify, bookmarkMods],
+ [Sync],
+ [Bookmarks.verify, bookmarksExpected],
+ [Bookmarks.verifyNot, bookmarksToDelete],
+]);
+
+// Back in profile2, do a sync and verify that we're in the expected state
+Phase("phase4", [
+ [Sync],
+ [Bookmarks.verify, bookmarksExpected],
+ [Bookmarks.verifyNot, bookmarksToDelete],
+]);
--- a/services/sync/tests/unit/test_bookmark_store.js
+++ b/services/sync/tests/unit/test_bookmark_store.js
@@ -16,39 +16,40 @@ var store = engine._store;
var tracker = engine._tracker;
// Don't write some persistence files asynchronously.
tracker.persistChangedIDs = false;
var fxuri = Utils.makeURI("http://getfirefox.com/");
var tburi = Utils.makeURI("http://getthunderbird.com/");
-add_test(function test_ignore_specials() {
+add_task(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);
+ yield store.deletePending();
// 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);
+ yield store.deletePending();
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);
@@ -238,45 +239,44 @@ add_test(function test_folder_createReco
} finally {
_("Clean up.");
store.wipe();
run_next_test();
}
});
-add_test(function test_deleted() {
+add_task(function* test_deleted() {
try {
_("Create a bookmark that will be deleted.");
let bmk1_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.toolbarFolder, fxuri,
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
let bmk1_guid = store.GUIDForId(bmk1_id);
_("Delete the bookmark through the store.");
let record = new PlacesItem("bookmarks", bmk1_guid);
record.deleted = true;
store.applyIncoming(record);
-
+ yield store.deletePending();
_("Ensure it has been deleted.");
let error;
try {
PlacesUtils.bookmarks.getBookmarkURI(bmk1_id);
} catch(ex) {
error = ex;
}
do_check_eq(error.result, Cr.NS_ERROR_ILLEGAL_VALUE);
let newrec = store.createRecord(bmk1_guid);
do_check_eq(newrec.deleted, true);
} finally {
_("Clean up.");
store.wipe();
- run_next_test();
}
});
add_test(function test_move_folder() {
try {
_("Create two folders and a bookmark in one of them.");
let folder1_id = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.toolbarFolder, "Folder1", 0);
@@ -423,12 +423,112 @@ add_test(function test_empty_query_doesn
store.applyIncoming(record);
delete record.folderName;
store.applyIncoming(record);
run_next_test();
});
+function assertDeleted(id) {
+ let error;
+ try {
+ PlacesUtils.bookmarks.getItemType(id);
+ } catch (e) {
+ error = e;
+ }
+ equal(error.result, Cr.NS_ERROR_ILLEGAL_VALUE)
+}
+
+add_task(function* test_delete_buffering() {
+ store.wipe();
+ try {
+ _("Create a folder with two bookmarks.");
+ let folder = new BookmarkFolder("bookmarks", "testfolder-1");
+ folder.parentName = "Bookmarks Toolbar";
+ folder.parentid = "toolbar";
+ folder.title = "Test Folder";
+ store.applyIncoming(folder);
+
+
+ let fxRecord = new Bookmark("bookmarks", "get-firefox1");
+ fxRecord.bmkUri = fxuri.spec;
+ fxRecord.title = "Get Firefox!";
+ fxRecord.parentName = "Test Folder";
+ fxRecord.parentid = "testfolder-1";
+
+ let tbRecord = new Bookmark("bookmarks", "get-tndrbrd1");
+ tbRecord.bmkUri = tburi.spec;
+ tbRecord.title = "Get Thunderbird!";
+ tbRecord.parentName = "Test Folder";
+ tbRecord.parentid = "testfolder-1";
+
+ store.applyIncoming(fxRecord);
+ store.applyIncoming(tbRecord);
+
+ let folderId = store.idForGUID(folder.id);
+ let fxRecordId = store.idForGUID(fxRecord.id);
+ let tbRecordId = store.idForGUID(tbRecord.id);
+
+ _("Check everything was created correctly.");
+
+ equal(PlacesUtils.bookmarks.getItemType(fxRecordId),
+ PlacesUtils.bookmarks.TYPE_BOOKMARK);
+ equal(PlacesUtils.bookmarks.getItemType(tbRecordId),
+ PlacesUtils.bookmarks.TYPE_BOOKMARK);
+ equal(PlacesUtils.bookmarks.getItemType(folderId),
+ PlacesUtils.bookmarks.TYPE_FOLDER);
+
+ equal(PlacesUtils.bookmarks.getFolderIdForItem(fxRecordId), folderId);
+ equal(PlacesUtils.bookmarks.getFolderIdForItem(tbRecordId), folderId);
+ equal(PlacesUtils.bookmarks.getFolderIdForItem(folderId),
+ PlacesUtils.bookmarks.toolbarFolder);
+
+ _("Delete the folder and one bookmark.");
+
+ let deleteFolder = new PlacesItem("bookmarks", "testfolder-1");
+ deleteFolder.deleted = true;
+
+ let deleteFxRecord = new PlacesItem("bookmarks", "get-firefox1");
+ deleteFxRecord.deleted = true;
+
+ store.applyIncoming(deleteFolder);
+ store.applyIncoming(deleteFxRecord);
+
+ _("Check that we haven't deleted them yet, but that the deletions are queued");
+ // these will throw if we've deleted them
+ equal(PlacesUtils.bookmarks.getItemType(fxRecordId),
+ PlacesUtils.bookmarks.TYPE_BOOKMARK);
+
+ equal(PlacesUtils.bookmarks.getItemType(folderId),
+ PlacesUtils.bookmarks.TYPE_FOLDER);
+
+ equal(PlacesUtils.bookmarks.getFolderIdForItem(fxRecordId), folderId);
+
+ ok(store._foldersToDelete.has(folder.id));
+ ok(store._atomsToDelete.has(fxRecord.id));
+ ok(!store._atomsToDelete.has(tbRecord.id));
+
+ _("Process pending deletions and ensure that the right things are deleted.");
+ let updatedGuids = yield store.deletePending();
+
+ deepEqual(updatedGuids.sort(), ["get-tndrbrd1", "toolbar"]);
+
+ assertDeleted(fxRecordId);
+ assertDeleted(folderId);
+
+ ok(!store._foldersToDelete.has(folder.id));
+ ok(!store._atomsToDelete.has(fxRecord.id));
+
+ equal(PlacesUtils.bookmarks.getFolderIdForItem(tbRecordId),
+ PlacesUtils.bookmarks.toolbarFolder);
+
+ } finally {
+ _("Clean up.");
+ store.wipe();
+ }
+});
+
+
function run_test() {
initTestLogging('Trace');
run_next_test();
}
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -158,30 +158,38 @@ const BookmarkSyncUtils = PlacesSyncUtil
// Update positions.
let orderedChildrenGuids = children.map(({ guid }) => guid);
yield PlacesUtils.bookmarks.reorder(parentGuid, orderedChildrenGuids);
})
);
}),
/**
- * Removes an item from the database.
+ * Removes an item from the database. Options are passed through to
+ * PlacesUtils.bookmarks.remove.
*/
- remove: Task.async(function* (syncId) {
+ remove: Task.async(function* (syncId, options = {}) {
let guid = BookmarkSyncUtils.syncIdToGuid(syncId);
if (guid in ROOT_GUID_TO_SYNC_ID) {
BookmarkSyncLog.warn(`remove: Refusing to remove root ${syncId}`);
return null;
}
- return PlacesUtils.bookmarks.remove(guid, {
+ return PlacesUtils.bookmarks.remove(guid, Object.assign({}, options, {
source: SOURCE_SYNC,
- });
+ }));
}),
/**
+ * Returns true for sync IDs that are considered roots.
+ */
+ isRootSyncID(syncID) {
+ return ROOT_SYNC_ID_TO_GUID.hasOwnProperty(syncID);
+ },
+
+ /**
* Changes the GUID of an existing item. This method only allows Places GUIDs
* because root sync IDs cannot be changed.
*
* @return {Promise} resolved once the GUID has been changed.
* @resolves to the new GUID.
* @rejects if the old GUID does not exist.
*/
changeGuid: Task.async(function* (oldGuid, newGuid) {
@@ -336,16 +344,40 @@ const BookmarkSyncUtils = PlacesSyncUtil
let parent = yield PlacesUtils.bookmarks.fetch(bookmarkItem.parentGuid);
if ("title" in parent) {
item.parentTitle = parent.title;
}
}
return item;
}),
+
+ /**
+ * Get the sync record kind for the record with provided sync id.
+ *
+ * @param syncId
+ * Sync ID for the item in question
+ *
+ * @returns {Promise} A promise that resolves with the sync record kind (e.g.
+ * something under `PlacesSyncUtils.bookmarks.KIND`), or
+ * with `null` if no item with that guid exists.
+ * @throws if `guid` is invalid.
+ */
+ getKindForSyncId(syncId) {
+ PlacesUtils.SYNC_BOOKMARK_VALIDATORS.syncId(syncId);
+ let guid = BookmarkSyncUtils.syncIdToGuid(syncId);
+ return PlacesUtils.bookmarks.fetch(guid)
+ .then(item => {
+ if (!item) {
+ return null;
+ }
+ return getKindForItem(item)
+ });
+ },
+
});
XPCOMUtils.defineLazyGetter(this, "BookmarkSyncLog", () => {
return Log.repository.getLogger("BookmarkSyncUtils");
});
function validateSyncBookmarkObject(input, behavior) {
return PlacesUtils.validateItemProperties(