Bug 1349703 - (part 1) Fix issue where sync would fail misreconcile special folders in fresh syncs r?kitcambridge
MozReview-Commit-ID: Izy9JgMYpom
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -1355,16 +1355,23 @@ SyncEngine.prototype = {
* to have a different GUID
*
* @return GUID of the similar item; falsy otherwise
*/
_findDupe(item) {
// By default, assume there's no dupe items for the engine
},
+ /**
+ * Called before a remote record is discarded due to failed reconciliation.
+ * Used by bookmark sync to note the child ordering of special folders.
+ */
+ beforeRecordDiscard(record) {
+ },
+
// 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;
@@ -1564,16 +1571,19 @@ SyncEngine.prototype = {
}
// At this point, records are different and the local record is modified.
// We resolve conflicts by record age, where the newest one wins. This does
// result in data loss and should be handled by giving the engine an
// opportunity to merge the records. Bug 720592 tracks this feature.
this._log.warn("DATA LOSS: Both local and remote changes to record: " +
item.id);
+ if (!remoteIsNewer) {
+ this.beforeRecordDiscard(item);
+ }
return remoteIsNewer;
},
// Upload outgoing records.
_uploadOutgoing() {
this._log.trace("Uploading local changes to server.");
// collection we'll upload
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -625,16 +625,27 @@ BookmarksEngine.prototype = {
// Cleans up the Places root, reading list items (ignored in bug 762118,
// removed in bug 1155684), and pinned sites.
_shouldDeleteRemotely(incomingItem) {
return FORBIDDEN_INCOMING_IDS.includes(incomingItem.id) ||
FORBIDDEN_INCOMING_PARENT_IDS.includes(incomingItem.parentid);
},
+ beforeRecordDiscard(record) {
+ let isSpecial = PlacesSyncUtils.bookmarks.ROOTS.includes(record.id);
+ if (isSpecial && record.children && !this._store._childrenToOrder[record.id]) {
+ if (this._modified.getStatus(record.id) != PlacesUtils.bookmarks.SYNC_STATUS.NEW) {
+ return;
+ }
+ this._log.debug("Recording children of " + record.id + " as " + JSON.stringify(record.children));
+ this._store._childrenToOrder[record.id] = record.children;
+ }
+ },
+
getValidator() {
return new BookmarkValidator();
}
};
function BookmarksStore(name, engine) {
Store.call(this, name, engine);
this._itemsToDelete = new Set();
@@ -1117,16 +1128,24 @@ BookmarksTracker.prototype = {
class BookmarksChangeset extends Changeset {
constructor() {
super();
// Weak changes are part of the changeset, but don't bump the change
// counter, and aren't persisted anywhere.
this.weakChanges = {};
}
+ getStatus(id) {
+ let change = this.changes[id];
+ if (!change) {
+ return PlacesUtils.bookmarks.SYNC_STATUS.UNKNOWN;
+ }
+ return change.status;
+ }
+
getModifiedTimestamp(id) {
let change = this.changes[id];
if (change) {
// Pretend the change doesn't exist if we've already synced or
// reconciled it.
return change.synced ? Number.NaN : change.modified;
}
if (this.weakChanges[id]) {