--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -758,16 +758,32 @@ Engine.prototype = {
},
};
this.SyncEngine = function SyncEngine(name, service) {
Engine.call(this, name || "SyncEngine", service);
this.loadToFetch();
this.loadPreviousFailed();
+ // The set of records needing a weak reupload.
+ // The difference between this and a "normal" reupload is that these records
+ // are only tracked in memory, and if the reupload attempt fails (shutdown,
+ // 412, etc), we abort uploading the "weak" set.
+ //
+ // The rationale here is for the cases where we receive a record from the
+ // server that we know is wrong in some (small) way. For example, the
+ // dateAdded field on bookmarks -- maybe we have a better date, or the server
+ // record is entirely missing the date, etc.
+ //
+ // In these cases, we fix our local copy of the record, and mark it for
+ // weak reupload. A normal ("strong") reupload is problematic here because
+ // in the case of a conflict from the server, there's a window where our
+ // record would be marked as modified more recently than a change that occurs
+ // on another device change, and we lose data from the user.
+ this._needWeakReupload = new Set();
}
// Enumeration to define approaches to handling bad records.
// Attached to the constructor to allow use as a kind of static enumeration.
SyncEngine.kRecoveryStrategy = {
ignore: "ignore",
retry: "retry",
error: "error"
@@ -1003,16 +1019,17 @@ SyncEngine.prototype = {
// to upload back.
this._tracker.clearChangedIDs();
this._log.info(this._modified.count() +
" outgoing items pre-reconciliation");
// Keep track of what to delete at the end of sync
this._delete = {};
+ this._needWeakReupload.clear();
},
/**
* A tiny abstraction to make it easier to test incoming record
* application.
*/
itemSource() {
return new Collection(this.engineURL, this._recordObj, this.service);
@@ -1546,25 +1563,25 @@ SyncEngine.prototype = {
item.id);
return remoteIsNewer;
},
// Upload outgoing records.
_uploadOutgoing() {
this._log.trace("Uploading local changes to server.");
+ // collection we'll upload
+ let up = new Collection(this.engineURL, null, this.service);
let modifiedIDs = this._modified.ids();
+ let counts = { failed: 0, sent: 0 };
if (modifiedIDs.length) {
this._log.trace("Preparing " + modifiedIDs.length +
" outgoing records");
- let counts = { sent: modifiedIDs.length, failed: 0 };
-
- // collection we'll upload
- let up = new Collection(this.engineURL, null, this.service);
+ counts.sent = modifiedIDs.length;
let failed = [];
let successful = [];
let handleResponse = (resp, batchOngoing = false) => {
// Note: We don't want to update this.lastSync, or this._modified until
// the batch is complete, however we want to remember success/failure
// indicators for when that happens.
if (!resp.success) {
@@ -1620,34 +1637,95 @@ SyncEngine.prototype = {
} catch (ex) {
this._log.warn("Error creating record", ex);
++counts.failed;
if (Async.isShutdownException(ex) || !this.allowSkippedRecord) {
Observers.notify("weave:engine:sync:uploaded", counts, this.name);
throw ex;
}
}
+ this._needWeakReupload.delete(id);
if (ok) {
let { enqueued, error } = postQueue.enqueue(out);
if (!enqueued) {
++counts.failed;
if (!this.allowSkippedRecord) {
throw error;
}
this._modified.delete(id);
this._log.warn(`Failed to enqueue record "${id}" (skipping)`, error);
}
}
this._store._sleep(0);
}
postQueue.flush(true);
+ }
+
+ if (this._needWeakReupload.size) {
+ try {
+ const { sent, failed } = this._weakReupload(up);
+ counts.sent += sent;
+ counts.failed += failed;
+ } catch (e) {
+ if (Async.isShutdownException(e)) {
+ throw e;
+ }
+ this._log.warn("Weak reupload failed", e);
+ }
+ }
+ if (counts.sent || counts.failed) {
Observers.notify("weave:engine:sync:uploaded", counts, this.name);
}
},
+ _weakReupload(collection) {
+ const counts = { sent: 0, failed: 0 };
+ let pendingSent = 0;
+ let postQueue = collection.newPostQueue(this._log, this.lastSync, (resp, batchOngoing = false) => {
+ if (!resp.success) {
+ this._needWeakReupload.clear();
+ this._log.warn("Uploading records (weak) failed: " + resp);
+ resp.failureCode = resp.status == 412 ? ENGINE_BATCH_INTERRUPTED : ENGINE_UPLOAD_FAIL;
+ throw resp;
+ }
+ if (!batchOngoing) {
+ counts.sent += pendingSent;
+ pendingSent = 0;
+ }
+ });
+
+ let pendingWeakReupload = this.buildWeakReuploadMap(this._needWeakReupload);
+ for (let [id, encodedRecord] of pendingWeakReupload) {
+ try {
+ this._log.trace("Outgoing (weak)", encodedRecord);
+ encodedRecord.encrypt(this.service.collectionKeys.keyForCollection(this.name));
+ } catch (ex) {
+ if (Async.isShutdownException(ex)) {
+ throw ex;
+ }
+ this._log.warn(`Failed to encrypt record "${id}" during weak reupload`, ex);
+ ++counts.failed;
+ continue;
+ }
+ // Note that general errors (network error, 412, etc.) will throw here.
+ // `enqueued` is only false if the specific item failed to enqueue, but
+ // other items should be/are fine. For example, enqueued will be false if
+ // it is larger than the max post or WBO size.
+ let { enqueued } = postQueue.enqueue(encodedRecord);
+ if (!enqueued) {
+ ++counts.failed;
+ } else {
+ ++pendingSent;
+ }
+ this._store._sleep(0);
+ }
+ postQueue.flush(true);
+ return counts;
+ },
+
_onRecordsWritten(succeeded, failed) {
// Implement this method to take specific actions against successfully
// uploaded records and failed records.
},
// Any cleanup necessary.
// Save the current snapshot so as to calculate changes at next sync
_syncFinish() {
@@ -1673,16 +1751,17 @@ SyncEngine.prototype = {
doDelete(key, val.slice(0, 100));
val = val.slice(100);
}
}
}
},
_syncCleanup() {
+ this._needWeakReupload.clear();
if (!this._modified) {
return;
}
try {
// Mark failed WBOs as changed again so they are reuploaded next time.
this.trackRemainingChanges();
} finally {
@@ -1809,16 +1888,37 @@ SyncEngine.prototype = {
* items that failed to upload. This method is called at the end of each sync.
*
*/
trackRemainingChanges() {
for (let [id, change] of this._modified.entries()) {
this._tracker.addChangedID(id, change);
}
},
+
+ /**
+ * Returns a map of (id, unencrypted record) that will be used to perform
+ * the weak reupload. Subclasses may override this to filter out items we
+ * shouldn't upload as part of a weak reupload (items that have changed,
+ * for example).
+ */
+ buildWeakReuploadMap(idSet) {
+ let result = new Map();
+ for (let id of idSet) {
+ try {
+ result.set(id, this._createRecord(id));
+ } catch (ex) {
+ if (Async.isShutdownException(ex)) {
+ throw ex;
+ }
+ this._log.warn("createRecord failed during weak reupload", ex);
+ }
+ }
+ return result;
+ }
};
/**
* A changeset is created for each sync in `Engine::get{Changed, All}IDs`,
* and stores opaque change data for tracked IDs. The default implementation
* only records timestamps, though engines can extend this to store additional
* data for each entry.
*/
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -115,34 +115,43 @@ PlacesItem.prototype = {
},
__proto__: CryptoWrapper.prototype,
_logName: "Sync.Record.PlacesItem",
// Converts the record to a Sync bookmark object that can be passed to
// `PlacesSyncUtils.bookmarks.{insert, update}`.
toSyncBookmark() {
- return {
+ let result = {
kind: this.type,
syncId: this.id,
parentSyncId: this.parentid,
};
+ let dateAdded = PlacesSyncUtils.bookmarks.ratchetTimestampBackwards(
+ this.dateAdded, +this.modified * 1000);
+ if (dateAdded !== undefined) {
+ result.dateAdded = dateAdded;
+ }
+ return result;
},
// Populates the record from a Sync bookmark object returned from
// `PlacesSyncUtils.bookmarks.fetch`.
fromSyncBookmark(item) {
this.parentid = item.parentSyncId;
this.parentName = item.parentTitle;
+ if (item.dateAdded) {
+ this.dateAdded = item.dateAdded;
+ }
},
};
Utils.deferGetSet(PlacesItem,
"cleartext",
- ["hasDupe", "parentid", "parentName", "type"]);
+ ["hasDupe", "parentid", "parentName", "type", "dateAdded"]);
this.Bookmark = function Bookmark(collection, id, type) {
PlacesItem.call(this, collection, id, type || "bookmark");
}
Bookmark.prototype = {
__proto__: PlacesItem.prototype,
_logName: "Sync.Record.Bookmark",
@@ -538,16 +547,43 @@ BookmarksEngine.prototype = {
// Make sure deleted items are marked as tombstones. We do this here
// in addition to the `isTombstone` call above because it's possible
// a changed bookmark might be deleted during a sync (bug 1313967).
this._modified.setTombstone(record.id);
}
return record;
},
+ buildWeakReuploadMap(idSet) {
+ // We want to avoid uploading records which have changed, since that could
+ // cause an inconsistent state on the server.
+ //
+ // Strictly speaking, it would be correct to just call getChangedIds() after
+ // building the initial weak reupload map, however this is quite slow, since
+ // we might end up doing createRecord() (which runs at least one, and
+ // sometimes multiple database queries) for a potentially large number of
+ // items.
+ //
+ // Since the call to getChangedIds is relatively cheap, we do it once before
+ // building the weakReuploadMap (which is where the calls to createRecord()
+ // occur) as an optimization, and once after for correctness, to handle the
+ // unlikely case that a record was modified while we were building the map.
+ let initialChanges = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.getChangedIds());
+ for (let changed of initialChanges) {
+ idSet.delete(changed);
+ }
+
+ let map = SyncEngine.prototype.buildWeakReuploadMap.call(this, idSet);
+ let changes = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.getChangedIds());
+ for (let id of changes) {
+ map.delete(id);
+ }
+ return map;
+ },
+
_findDupe: function _findDupe(item) {
this._log.trace("Finding dupe for " + item.id +
" (already duped: " + item.hasDupe + ").");
// Don't bother finding a dupe if the incoming item has duplicates.
if (item.hasDupe) {
this._log.trace(item.id + " already a dupe: not finding one.");
return null;
@@ -661,30 +697,36 @@ BookmarksStore.prototype = {
let info = record.toSyncBookmark();
// This can throw if we're inserting an invalid or incomplete bookmark.
// That's fine; the exception will be caught by `applyIncomingBatch`
// without aborting further processing.
let item = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
if (item) {
this._log.debug(`Created ${item.kind} ${item.syncId} under ${
item.parentSyncId}`, item);
+ if (item.dateAdded != record.dateAdded) {
+ this.engine._needWeakReupload.add(item.syncId);
+ }
}
},
remove: function BStore_remove(record) {
this._log.trace(`Buffering removal of item "${record.id}".`);
this._itemsToDelete.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 ${
item.parentSyncId}`, item);
+ if (item.dateAdded != record.dateAdded) {
+ this.engine._needWeakReupload.add(item.syncId);
+ }
}
},
_orderChildren: function _orderChildren() {
let promises = Object.keys(this._childrenToOrder).map(syncID => {
let children = this._childrenToOrder[syncID];
return PlacesSyncUtils.bookmarks.order(syncID, children).catch(ex => {
this._log.debug(`Could not order children for ${syncID}`, ex);
--- a/services/sync/tests/unit/test_bookmark_duping.js
+++ b/services/sync/tests/unit/test_bookmark_duping.js
@@ -118,16 +118,17 @@ async function validate(collection, expe
for (let elt of summary) {
(isInExpectedFailures(elt) ? expected : unexpected).push(elt);
}
if (unexpected.length || expected.length != expectedFailures.length) {
do_print("Validation failed:");
do_print(JSON.stringify(summary));
// print the entire validator output as it has IDs etc.
do_print(JSON.stringify(problems, undefined, 2));
+ do_print("Expected: " + JSON.stringify(expectedFailures, undefined, 2));
// All server records and the entire bookmark tree.
do_print("Server records:\n" + JSON.stringify(collection.payloads(), undefined, 2));
let tree = await PlacesUtils.promiseBookmarksTree("", { includeItemIds: true });
do_print("Local bookmark tree:\n" + JSON.stringify(tree, undefined, 2));
ok(false);
}
}
@@ -155,17 +156,17 @@ add_task(async function test_dupe_bookma
type: "bookmark",
title: "Get Firefox!",
parentName: "Folder 1",
parentid: folder1_guid,
};
collection.insert(newGUID, encryptPayload(to_apply), Date.now() / 1000 + 500);
_("Syncing so new dupe record is processed");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// We should have logically deleted the dupe record.
equal(collection.count(), 7);
ok(getServerRecord(collection, bmk1_guid).deleted);
// and physically removed from the local store.
await promiseNoLocalItem(bmk1_guid);
// Parent should still only have 1 item.
@@ -213,17 +214,17 @@ add_task(async function test_dupe_repare
title: "Get Firefox!",
parentName: "Folder 1",
parentid: folder2_guid,
};
collection.insert(newGUID, encryptPayload(to_apply), Date.now() / 1000 + 500);
_("Syncing so new dupe record is processed");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// We should have logically deleted the dupe record.
equal(collection.count(), 8);
ok(getServerRecord(collection, bmk1_guid).deleted);
// and physically removed from the local store.
await promiseNoLocalItem(bmk1_guid);
// The original folder no longer has the item
@@ -290,17 +291,17 @@ add_task(async function test_dupe_repare
// to *not* apply the incoming record.
await PlacesTestUtils.setBookmarkSyncFields({
guid: bmk1_guid,
syncChangeCounter: 1,
lastModified: Date.now() + (deltaSeconds + 10) * 1000,
});
_("Syncing so new dupe record is processed");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// We should have logically deleted the dupe record.
equal(collection.count(), 8);
ok(getServerRecord(collection, bmk1_guid).deleted);
// and physically removed from the local store.
await promiseNoLocalItem(bmk1_guid);
// The original folder still longer has the item
@@ -381,17 +382,17 @@ add_task(async function test_dupe_repare
title: "Get Firefox!",
parentName: "Folder 1",
parentid: newParentGUID,
tags: [],
}), Date.now() / 1000 + 500);
_("Syncing so new records are processed.");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// Everything should be parented correctly.
equal((await getFolderChildrenIDs(folder1_id)).length, 0);
let newParentID = store.idForGUID(newParentGUID);
let newID = store.idForGUID(newGUID);
deepEqual(await getFolderChildrenIDs(newParentID), [newID]);
@@ -457,17 +458,17 @@ add_task(async function test_dupe_repare
title: "A second folder",
parentName: "Bookmarks Toolbar",
parentid: "toolbar",
children: [newParentGUID],
tags: [],
}), Date.now() / 1000 + 500);
_("Syncing so out-of-order records are processed.");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// The intended parent did end up existing, so it should be parented
// correctly after de-duplication.
equal((await getFolderChildrenIDs(folder1_id)).length, 0);
let newParentID = store.idForGUID(newParentGUID);
let newID = store.idForGUID(newGUID);
deepEqual(await getFolderChildrenIDs(newParentID), [newID]);
@@ -509,20 +510,21 @@ add_task(async function test_dupe_repare
collection.insert(newGUID, encryptPayload({
id: newGUID,
bmkUri: "http://getfirefox.com/",
type: "bookmark",
title: "Get Firefox!",
parentName: "Folder 1",
parentid: newParentGUID,
tags: [],
+ dateAdded: Date.now() - 10000
}), Date.now() / 1000 + 500);
_("Syncing so new dupe record is processed");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// We should have logically deleted the dupe record.
equal(collection.count(), 8);
ok(getServerRecord(collection, bmk1_guid).deleted);
// and physically removed from the local store.
await promiseNoLocalItem(bmk1_guid);
// The intended parent doesn't exist, so it remains in the original folder
@@ -552,30 +554,33 @@ add_task(async function test_dupe_repare
collection.insert(newParentGUID, encryptPayload({
id: newParentGUID,
type: "folder",
title: "Folder 1",
parentName: "A second folder",
parentid: folder2_guid,
children: [newGUID],
tags: [],
+ dateAdded: Date.now() - 10000,
}), Date.now() / 1000 + 500);
// We also queue an update to "folder 2" that references the new parent.
collection.insert(folder2_guid, encryptPayload({
id: folder2_guid,
type: "folder",
title: "A second folder",
parentName: "Bookmarks Toolbar",
parentid: "toolbar",
children: [newParentGUID],
tags: [],
+ dateAdded: Date.now() - 11000,
}), Date.now() / 1000 + 500);
+
_("Syncing so missing parent appears");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
// The intended parent now does exist, so it should have been reparented.
equal((await getFolderChildrenIDs(folder1_id)).length, 0);
let newParentID = store.idForGUID(newParentGUID);
let newID = store.idForGUID(newGUID);
deepEqual(await getFolderChildrenIDs(newParentID), [newID]);
@@ -621,17 +626,17 @@ add_task(async function test_dupe_empty_
type: "folder",
title: "Folder 1",
parentName: "Bookmarks Toolbar",
parentid: "toolbar",
children: [],
}), Date.now() / 1000 + 500);
_("Syncing so new dupe records are processed");
- engine.lastSync = engine.lastSync - 0.01;
+ engine.lastSync = engine.lastSync - 5;
engine.sync();
await validate(collection);
// Collection now has one additional record - the logically deleted dupe.
equal(collection.count(), 6);
// original folder should be logically deleted.
ok(getServerRecord(collection, folder1_guid).deleted);
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -738,13 +738,172 @@ add_task(async function test_misreconcil
do_check_eq(store.GUIDForId(toolbarIDBefore), "toolbar");
do_check_eq(parentGUIDBefore, parentGUIDAfter);
do_check_eq(parentIDBefore, parentIDAfter);
await PlacesSyncUtils.bookmarks.reset();
await promiseStopServer(server);
});
+add_task(async function test_sync_dateAdded() {
+ await Service.recordManager.clearCache();
+ await PlacesSyncUtils.bookmarks.reset();
+ let engine = new BookmarksEngine(Service);
+ let store = engine._store;
+ let server = serverForFoo(engine);
+ await SyncTestingInfrastructure(server);
+
+ let collection = server.user("foo").collection("bookmarks");
+
+ Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup...
+
+ // Just matters that it's in the past, not how far.
+ let now = Date.now();
+ let oneYearMS = 365 * 24 * 60 * 60 * 1000;
+
+ try {
+ let item1GUID = "abcdefabcdef";
+ let item1 = new Bookmark("bookmarks", item1GUID);
+ item1.bmkUri = "https://example.com";
+ item1.title = "asdf";
+ item1.parentName = "Bookmarks Toolbar";
+ item1.parentid = "toolbar";
+ item1.dateAdded = now - oneYearMS;
+ collection.insert(item1GUID, encryptPayload(item1.cleartext));
+
+ let item2GUID = "aaaaaaaaaaaa";
+ let item2 = new Bookmark("bookmarks", item2GUID);
+ item2.bmkUri = "https://example.com/2";
+ item2.title = "asdf2";
+ item2.parentName = "Bookmarks Toolbar";
+ item2.parentid = "toolbar";
+ item2.dateAdded = now + oneYearMS;
+ const item2LastModified = now / 1000 - 100;
+ collection.insert(item2GUID, encryptPayload(item2.cleartext), item2LastModified);
+
+ let item3GUID = "bbbbbbbbbbbb";
+ let item3 = new Bookmark("bookmarks", item3GUID);
+ item3.bmkUri = "https://example.com/3";
+ item3.title = "asdf3";
+ item3.parentName = "Bookmarks Toolbar";
+ item3.parentid = "toolbar";
+ // no dateAdded
+ collection.insert(item3GUID, encryptPayload(item3.cleartext));
+
+ let item4GUID = "cccccccccccc";
+ let item4 = new Bookmark("bookmarks", item4GUID);
+ item4.bmkUri = "https://example.com/4";
+ item4.title = "asdf4";
+ item4.parentName = "Bookmarks Toolbar";
+ item4.parentid = "toolbar";
+ // no dateAdded, but lastModified in past
+ const item4LastModified = (now - oneYearMS) / 1000;
+ collection.insert(item4GUID, encryptPayload(item4.cleartext), item4LastModified);
+
+ let item5GUID = "dddddddddddd";
+ let item5 = new Bookmark("bookmarks", item5GUID);
+ item5.bmkUri = "https://example.com/5";
+ item5.title = "asdf5";
+ item5.parentName = "Bookmarks Toolbar";
+ item5.parentid = "toolbar";
+ // no dateAdded, lastModified in (near) future.
+ const item5LastModified = (now + 60000) / 1000;
+ collection.insert(item5GUID, encryptPayload(item5.cleartext), item5LastModified);
+
+ let item6GUID = "eeeeeeeeeeee";
+ let item6 = new Bookmark("bookmarks", item6GUID);
+ item6.bmkUri = "https://example.com/6";
+ item6.title = "asdf6";
+ item6.parentName = "Bookmarks Toolbar";
+ item6.parentid = "toolbar";
+ const item6LastModified = (now - oneYearMS) / 1000;
+ collection.insert(item6GUID, encryptPayload(item6.cleartext), item6LastModified);
+
+ let origBuildWeakReuploadMap = engine.buildWeakReuploadMap;
+ engine.buildWeakReuploadMap = set => {
+ let fullMap = origBuildWeakReuploadMap.call(engine, set);
+ fullMap.delete(item6GUID);
+ return fullMap;
+ };
+
+ await sync_engine_and_validate_telem(engine, false);
+
+ let record1 = await store.createRecord(item1GUID);
+ let record2 = await store.createRecord(item2GUID);
+
+ equal(item1.dateAdded, record1.dateAdded, "dateAdded in past should be synced");
+ equal(record2.dateAdded, item2LastModified * 1000, "dateAdded in future should be ignored in favor of last modified");
+
+ let record3 = await store.createRecord(item3GUID);
+
+ ok(record3.dateAdded);
+ // Make sure it's within 24 hours of the right timestamp... This is a little
+ // dodgey but we only really care that it's basically accurate and has the
+ // right day.
+ ok(Math.abs(Date.now() - record3.dateAdded) < 24 * 60 * 60 * 1000);
+
+ let record4 = await store.createRecord(item4GUID);
+ equal(record4.dateAdded, item4LastModified * 1000,
+ "If no dateAdded is provided, lastModified should be used");
+
+ let record5 = await store.createRecord(item5GUID);
+ equal(record5.dateAdded, item5LastModified * 1000,
+ "If no dateAdded is provided, lastModified should be used (even if it's in the future)");
+
+ let item6WBO = JSON.parse(JSON.parse(collection._wbos[item6GUID].payload).ciphertext);
+ ok(!item6WBO.dateAdded,
+ "If we think an item has been modified locally, we don't upload it to the server");
+
+ let record6 = await store.createRecord(item6GUID);
+ equal(record6.dateAdded, item6LastModified * 1000,
+ "We still remember the more accurate dateAdded if we don't upload a record due to local changes");
+ engine.buildWeakReuploadMap = origBuildWeakReuploadMap;
+
+ // Update item2 and try resyncing it.
+ item2.dateAdded = now - 100000;
+ collection.insert(item2GUID, encryptPayload(item2.cleartext), now / 1000 - 50);
+
+
+ // Also, add a local bookmark and make sure it's date added makes it up to the server
+ let bzid = PlacesUtils.bookmarks.insertBookmark(
+ PlacesUtils.bookmarksMenuFolderId, Utils.makeURI("https://bugzilla.mozilla.org/"),
+ PlacesUtils.bookmarks.DEFAULT_INDEX, "Bugzilla");
+
+ let bzguid = await PlacesUtils.promiseItemGuid(bzid);
+
+
+ await sync_engine_and_validate_telem(engine, false);
+
+ let newRecord2 = await store.createRecord(item2GUID);
+ equal(newRecord2.dateAdded, item2.dateAdded, "dateAdded update should work for earlier date");
+
+ let bzWBO = JSON.parse(JSON.parse(collection._wbos[bzguid].payload).ciphertext);
+ ok(bzWBO.dateAdded, "Locally added dateAdded lost");
+
+ let localRecord = await store.createRecord(bzguid);
+ equal(bzWBO.dateAdded, localRecord.dateAdded, "dateAdded should not change during upload");
+
+ item2.dateAdded += 10000;
+ collection.insert(item2GUID, encryptPayload(item2.cleartext), now / 1000 - 10);
+ engine.lastSync = now / 1000 - 20;
+
+ await sync_engine_and_validate_telem(engine, false);
+
+ let newerRecord2 = await store.createRecord(item2GUID);
+ equal(newerRecord2.dateAdded, newRecord2.dateAdded,
+ "dateAdded update should be ignored for later date if we know an earlier one ");
+
+
+
+ } finally {
+ store.wipe();
+ Svc.Prefs.resetBranch("");
+ Service.recordManager.clearCache();
+ await PlacesSyncUtils.bookmarks.reset();
+ await promiseStopServer(server);
+ }
+});
+
function run_test() {
initTestLogging("Trace");
generateNewKeys(Service.collectionKeys);
run_next_test();
}
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -37,16 +37,17 @@ async function verifyTrackerEmpty() {
async function resetTracker() {
await PlacesTestUtils.markBookmarksAsSynced();
tracker.resetScore();
}
async function cleanup() {
engine.lastSync = 0;
+ engine._needWeakReupload.clear()
store.wipe();
await resetTracker();
await stopTracking();
}
// startTracking is a signal that the test wants to notice things that happen
// after this is called (ie, things already tracked should be discarded.)
async function startTracking() {
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -154,33 +154,33 @@ var Bookmarks = Object.freeze({
* object representing a bookmark-item.
*
* @return {Promise} resolved when the creation is complete.
* @resolves to an object representing the created bookmark.
* @rejects if it's not possible to create the requested bookmark.
* @throws if the arguments are invalid.
*/
insert(info) {
- // Ensure to use the same date for dateAdded and lastModified, even if
- // dateAdded may be imposed by the caller.
- let time = (info && info.dateAdded) || new Date();
+ let now = new Date();
+ let addedTime = (info && info.dateAdded) || now;
+ let modTime = addedTime;
+ if (addedTime > now) {
+ modTime = now;
+ }
let insertInfo = validateBookmarkObject(info,
{ type: { defaultValue: this.TYPE_BOOKMARK }
, index: { defaultValue: this.DEFAULT_INDEX }
, url: { requiredIf: b => b.type == this.TYPE_BOOKMARK
, validIf: b => b.type == this.TYPE_BOOKMARK }
, parentGuid: { required: true }
, title: { validIf: b => [ this.TYPE_BOOKMARK
, this.TYPE_FOLDER ].includes(b.type) }
- , dateAdded: { defaultValue: time
- , validIf: b => !b.lastModified ||
- b.dateAdded <= b.lastModified }
- , lastModified: { defaultValue: time,
- validIf: b => (!b.dateAdded && b.lastModified >= time) ||
- (b.dateAdded && b.lastModified >= b.dateAdded) }
+ , dateAdded: { defaultValue: addedTime }
+ , lastModified: { defaultValue: modTime,
+ validIf: b => b.lastModified >= now || (b.dateAdded && b.lastModified >= b.dateAdded) }
, source: { defaultValue: this.SOURCES.DEFAULT }
});
return Task.spawn(function* () {
// Ensure the parent exists.
let parent = yield fetchBookmark({ guid: insertInfo.parentGuid });
if (!parent)
throw new Error("parentGuid must be valid");
@@ -262,34 +262,33 @@ var Bookmarks = Object.freeze({
return Task.spawn(function* () {
// Ensure the item exists.
let item = yield fetchBookmark(updateInfo);
if (!item)
throw new Error("No bookmarks found for the provided GUID");
if (updateInfo.hasOwnProperty("type") && updateInfo.type != item.type)
throw new Error("The bookmark type cannot be changed");
- if (updateInfo.hasOwnProperty("dateAdded") &&
- updateInfo.dateAdded.getTime() != item.dateAdded.getTime())
- throw new Error("The bookmark dateAdded cannot be changed");
// Remove any property that will stay the same.
removeSameValueProperties(updateInfo, item);
// Check if anything should still be updated.
if (Object.keys(updateInfo).length < 3) {
// Remove non-enumerable properties.
return Object.assign({}, item);
}
-
+ const now = new Date();
updateInfo = validateBookmarkObject(updateInfo,
{ url: { validIf: () => item.type == this.TYPE_BOOKMARK }
, title: { validIf: () => [ this.TYPE_BOOKMARK
, this.TYPE_FOLDER ].includes(item.type) }
- , lastModified: { defaultValue: new Date()
- , validIf: b => b.lastModified >= item.dateAdded }
+ , lastModified: { defaultValue: now
+ , validIf: b => b.lastModified >= now ||
+ b.lastModified >= (b.dateAdded || item.dateAdded) }
+ , dateAdded: { defaultValue: item.dateAdded }
});
return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: update",
Task.async(function*(db) {
let parent;
if (updateInfo.hasOwnProperty("parentGuid")) {
if (item.type == this.TYPE_FOLDER) {
// Make sure we are not moving a folder into itself or one of its
@@ -837,16 +836,18 @@ function notify(observers, notification,
function updateBookmark(info, item, newParent) {
return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: updateBookmark",
Task.async(function*(db) {
let tuples = new Map();
tuples.set("lastModified", { value: PlacesUtils.toPRTime(info.lastModified) });
if (info.hasOwnProperty("title"))
tuples.set("title", { value: info.title });
+ if (info.hasOwnProperty("dateAdded"))
+ tuples.set("dateAdded", { value: PlacesUtils.toPRTime(info.dateAdded) });
yield db.executeTransaction(function* () {
let isTagging = item._grandParentId == PlacesUtils.tagsFolderId;
let syncChangeDelta =
PlacesSyncUtils.bookmarks.determineSyncChangeDelta(info.source);
if (info.hasOwnProperty("url")) {
// Ensure a page exists in moz_places for this URL.
@@ -894,22 +895,27 @@ function updateBookmark(info, item, newP
yield setAncestorsLastModified(db, item.parentGuid, info.lastModified,
syncChangeDelta);
}
yield setAncestorsLastModified(db, newParent.guid, info.lastModified,
syncChangeDelta);
}
if (syncChangeDelta) {
- let isChangingIndex = info.hasOwnProperty("index") &&
- info.index != item.index;
// Sync stores child indices in the parent's record, so we only bump the
// item's counter if we're updating at least one more property in
- // addition to the index and last modified time.
- let needsSyncChange = isChangingIndex ? tuples.size > 2 : tuples.size > 1;
+ // addition to the index, last modified time, and dateAdded.
+ let sizeThreshold = 1;
+ if (info.hasOwnProperty("index") && info.index != item.index) {
+ ++sizeThreshold;
+ }
+ if (tuples.has("dateAdded")) {
+ ++sizeThreshold;
+ }
+ let needsSyncChange = tuples.size > sizeThreshold;
if (needsSyncChange) {
tuples.set("syncChangeDelta", { value: syncChangeDelta
, fragment: "syncChangeCounter = syncChangeCounter + :syncChangeDelta" });
}
}
if (isTagging) {
// If we're updating a tag entry, bump the sync change counter for
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -75,16 +75,21 @@ const HistorySyncUtils = PlacesSyncUtils
const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
SMART_BOOKMARKS_ANNO: "Places/SmartBookmark",
DESCRIPTION_ANNO: "bookmarkProperties/description",
SIDEBAR_ANNO: "bookmarkProperties/loadInSidebar",
SYNC_PARENT_ANNO: "sync/parent",
SYNC_MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
+ // Jan 23, 1993 in milliseconds since 1970. Corresponds roughly to the release
+ // of the original NCSA Mosiac. We can safely assume that any dates before
+ // this time are invalid.
+ EARLIEST_BOOKMARK_TIMESTAMP: Date.UTC(1993, 0, 23),
+
KINDS: {
BOOKMARK: "bookmark",
// Microsummaries were removed from Places in bug 524091. For now, Sync
// treats them identically to bookmarks. Bug 745410 tracks removing them
// entirely.
MICROSUMMARY: "microsummary",
QUERY: "query",
FOLDER: "folder",
@@ -107,16 +112,29 @@ const BookmarkSyncUtils = PlacesSyncUtil
/**
* Converts a Sync record ID to a Places GUID.
*/
syncIdToGuid(syncId) {
return ROOT_SYNC_ID_TO_GUID[syncId] || syncId;
},
/**
+ * Resolves to an array of the syncIDs of bookmarks that have a nonzero change
+ * counter
+ */
+ getChangedIds: Task.async(function* () {
+ let db = yield PlacesUtils.promiseDBConnection();
+ let result = yield db.executeCached(`
+ SELECT guid FROM moz_bookmarks
+ WHERE syncChangeCounter >= 1`);
+ return result.map(row =>
+ BookmarkSyncUtils.guidToSyncId(row.getResultByName("guid")));
+ }),
+
+ /**
* Fetches the sync IDs for a folder's children, ordered by their position
* within the folder.
*/
fetchChildSyncIds: Task.async(function* (parentSyncId) {
PlacesUtils.SYNC_BOOKMARK_VALIDATORS.syncId(parentSyncId);
let parentGuid = BookmarkSyncUtils.syncIdToGuid(parentSyncId);
let db = yield PlacesUtils.promiseDBConnection();
@@ -627,16 +645,18 @@ const BookmarkSyncUtils = PlacesSyncUtil
* Fetches a Sync bookmark object for an item in the tree. The object contains
* the following properties, depending on the item's kind:
*
* - kind (all): A string representing the item's kind.
* - syncId (all): The item's sync ID.
* - parentSyncId (all): The sync ID of the item's parent.
* - parentTitle (all): The title of the item's parent, used for de-duping.
* Omitted for the Places root and parents with empty titles.
+ * - dateAdded (all): Timestamp in milliseconds, when the bookmark was added
+ * or created on a remote device if known.
* - title ("bookmark", "folder", "livemark", "query"): The item's title.
* Omitted if empty.
* - url ("bookmark", "query"): The item's URL.
* - tags ("bookmark", "query"): An array containing the item's tags.
* - keyword ("bookmark"): The bookmark's keyword, if one exists.
* - description ("bookmark", "folder", "livemark"): The item's description.
* Omitted if one isn't set.
* - loadInSidebar ("bookmark", "query"): Whether to load the bookmark in
@@ -774,16 +794,29 @@ const BookmarkSyncUtils = PlacesSyncUtil
UPDATE moz_bookmarks
SET syncChangeCounter = syncChangeCounter + :syncChangeDelta
WHERE type = :type AND
fk = (SELECT id FROM moz_places WHERE url_hash = hash(:url) AND
url = :url)`,
{ syncChangeDelta, type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: url.href });
},
+
+ /**
+ * Returns `undefined` if no sensible timestamp could be found.
+ * Otherwise, returns the earliest sensible timestamp between `existingMillis`
+ * and `serverMillis`.
+ */
+ ratchetTimestampBackwards(existingMillis, serverMillis, lowerBound = BookmarkSyncUtils.EARLIEST_BOOKMARK_TIMESTAMP) {
+ const possible = [+existingMillis, +serverMillis].filter(n => !isNaN(n) && n > lowerBound);
+ if (!possible.length) {
+ return undefined;
+ }
+ return Math.min(...possible);
+ }
});
XPCOMUtils.defineLazyGetter(this, "BookmarkSyncLog", () => {
return Log.repository.getLogger("BookmarkSyncUtils");
});
function validateSyncBookmarkObject(input, behavior) {
return PlacesUtils.validateItemProperties(
@@ -1124,16 +1157,26 @@ var shouldReinsertLivemark = Task.async(
var updateSyncBookmark = Task.async(function* (updateInfo) {
let guid = BookmarkSyncUtils.syncIdToGuid(updateInfo.syncId);
let oldBookmarkItem = yield PlacesUtils.bookmarks.fetch(guid);
if (!oldBookmarkItem) {
throw new Error(`Bookmark with sync ID ${
updateInfo.syncId} does not exist`);
}
+ if (updateInfo.hasOwnProperty("dateAdded")) {
+ let newDateAdded = BookmarkSyncUtils.ratchetTimestampBackwards(
+ oldBookmarkItem.dateAdded, updateInfo.dateAdded);
+ if (!newDateAdded || newDateAdded === oldBookmarkItem.dateAdded) {
+ delete updateInfo.dateAdded;
+ } else {
+ updateInfo.dateAdded = newDateAdded;
+ }
+ }
+
let shouldReinsert = false;
let oldKind = yield getKindForItem(oldBookmarkItem);
if (updateInfo.hasOwnProperty("kind") && updateInfo.kind != oldKind) {
// If the item's aren't the same kind, we can't update the record;
// we must remove and reinsert.
shouldReinsert = true;
if (BookmarkSyncLog.level <= Log.Level.Warn) {
let oldSyncId = BookmarkSyncUtils.guidToSyncId(oldBookmarkItem.guid);
@@ -1150,16 +1193,19 @@ var updateSyncBookmark = Task.async(func
let oldSyncId = BookmarkSyncUtils.guidToSyncId(oldBookmarkItem.guid);
BookmarkSyncLog.debug(`updateSyncBookmark: Local ${
oldSyncId} and remote ${
updateInfo.syncId} livemarks have different URLs`);
}
}
if (shouldReinsert) {
+ if (!updateInfo.hasOwnProperty("dateAdded")) {
+ updateInfo.dateAdded = oldBookmarkItem.dateAdded.getTime();
+ }
let newInfo = validateNewBookmark(updateInfo);
yield PlacesUtils.bookmarks.remove({
guid,
source: SOURCE_SYNC,
});
// A reinsertion likely indicates a confused client, since there aren't
// public APIs for changing livemark URLs or an item's kind (e.g., turning
// a folder into a separator while preserving its annos and position).
@@ -1315,16 +1361,17 @@ function validateNewBookmark(info) {
, BookmarkSyncUtils.KINDS.QUERY
, BookmarkSyncUtils.KINDS.FOLDER
, BookmarkSyncUtils.KINDS.LIVEMARK ].includes(b.kind) }
, loadInSidebar: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
, BookmarkSyncUtils.KINDS.MICROSUMMARY
, BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
, feed: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
, site: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
+ , dateAdded: { required: false }
});
return insertInfo;
}
// Returns an array of GUIDs for items that have an `anno` with the given `val`.
var fetchGuidsWithAnno = Task.async(function* (anno, val) {
let db = yield PlacesUtils.promiseDBConnection();
@@ -1434,16 +1481,20 @@ var placesBookmarkToSyncBookmark = Task.
item.kind = yield getKindForItem(bookmarkItem);
break;
case "title":
case "url":
item[prop] = bookmarkItem[prop];
break;
+ case "dateAdded":
+ item[prop] = new Date(bookmarkItem[prop]).getTime();
+ break;
+
// Livemark objects contain additional properties. The feed URL is
// required; the site URL is optional.
case "feedURI":
item.feed = new URL(bookmarkItem.feedURI.spec);
break;
case "siteURI":
if (bookmarkItem.siteURI) {
@@ -1472,16 +1523,20 @@ function syncBookmarkToPlacesBookmark(in
bookmarkInfo.type = getTypeForKind(info.kind);
break;
// Convert sync IDs to Places GUIDs for roots.
case "syncId":
bookmarkInfo.guid = BookmarkSyncUtils.syncIdToGuid(info.syncId);
break;
+ case "dateAdded":
+ bookmarkInfo.dateAdded = new Date(info.dateAdded);
+ break;
+
case "parentSyncId":
bookmarkInfo.parentGuid =
BookmarkSyncUtils.syncIdToGuid(info.parentSyncId);
// Instead of providing an index, Sync reorders children at the end of
// the sync using `BookmarkSyncUtils.order`. We explicitly specify the
// default index here to prevent `PlacesUtils.bookmarks.update` and
// `PlacesUtils.livemarks.addLivemark` from throwing.
bookmarkInfo.index = PlacesUtils.bookmarks.DEFAULT_INDEX;
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -277,16 +277,18 @@ const SYNC_BOOKMARK_VALIDATORS = Object.
throw new Error(`Invalid tag: ${tag}`);
}
}
return v;
},
keyword: simpleValidateFunc(v => v === null || typeof v == "string"),
description: simpleValidateFunc(v => v === null || typeof v == "string"),
loadInSidebar: simpleValidateFunc(v => v === true || v === false),
+ dateAdded: simpleValidateFunc(v => typeof v === "number"
+ && v > PlacesSyncUtils.bookmarks.EARLIEST_BOOKMARK_TIMESTAMP),
feed: v => v === null ? v : BOOKMARK_VALIDATORS.url(v),
site: v => v === null ? v : BOOKMARK_VALIDATORS.url(v),
title: BOOKMARK_VALIDATORS.title,
url: BOOKMARK_VALIDATORS.url,
});
// Sync change records are passed between `PlacesSyncUtils` and the Sync
// bookmarks engine, and are used to update an item's sync status and change
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_insert.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_insert.js
@@ -37,20 +37,17 @@ add_task(function* invalid_input_throws(
Assert.throws(() => PlacesUtils.bookmarks.insert({ lastModified: -10 }),
/Invalid value for property 'lastModified'/);
Assert.throws(() => PlacesUtils.bookmarks.insert({ lastModified: "today" }),
/Invalid value for property 'lastModified'/);
Assert.throws(() => PlacesUtils.bookmarks.insert({ lastModified: Date.now() }),
/Invalid value for property 'lastModified'/);
let time = new Date();
- let future = new Date(time + 86400000);
- Assert.throws(() => PlacesUtils.bookmarks.insert({ dateAdded: future,
- lastModified: time }),
- /Invalid value for property 'dateAdded'/);
+
let past = new Date(time - 86400000);
Assert.throws(() => PlacesUtils.bookmarks.insert({ lastModified: past }),
/Invalid value for property 'lastModified'/);
Assert.throws(() => PlacesUtils.bookmarks.insert({ type: -1 }),
/Invalid value for property 'type'/);
Assert.throws(() => PlacesUtils.bookmarks.insert({ type: 100 }),
/Invalid value for property 'type'/);
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_update.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_update.js
@@ -98,32 +98,16 @@ add_task(function* invalid_properties_fo
type: PlacesUtils.bookmarks.TYPE_FOLDER });
Assert.ok(false, "Should have thrown");
} catch (ex) {
Assert.ok(/The bookmark type cannot be changed/.test(ex));
}
try {
yield PlacesUtils.bookmarks.update({ guid: bm.guid,
- dateAdded: new Date() });
- Assert.ok(false, "Should have thrown");
- } catch (ex) {
- Assert.ok(/The bookmark dateAdded cannot be changed/.test(ex));
- }
-
- try {
- yield PlacesUtils.bookmarks.update({ guid: bm.guid,
- dateAdded: new Date() });
- Assert.ok(false, "Should have thrown");
- } catch (ex) {
- Assert.ok(/The bookmark dateAdded cannot be changed/.test(ex));
- }
-
- try {
- yield PlacesUtils.bookmarks.update({ guid: bm.guid,
parentGuid: "123456789012",
index: 1 });
Assert.ok(false, "Should have thrown");
} catch (ex) {
Assert.ok(/No bookmarks found for the provided parentGuid/.test(ex));
}
let past = new Date(Date.now() - 86400000);
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/unit/test_sync_utils.js
@@ -1737,41 +1737,42 @@ add_task(function* test_fetch() {
let item = yield PlacesSyncUtils.bookmarks.fetch(folder.syncId);
deepEqual(item, {
syncId: folder.syncId,
kind: "folder",
parentSyncId: "menu",
description: "Folder description",
childSyncIds: [folderBmk.syncId, folderSep.syncId],
parentTitle: "Bookmarks Menu",
+ dateAdded: item.dateAdded,
title: "",
}, "Should include description, children, title, and parent title in folder");
}
do_print("Fetch bookmark with description, sidebar anno, and tags");
{
let item = yield PlacesSyncUtils.bookmarks.fetch(bmk.syncId);
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
- "url", "tags", "description", "loadInSidebar", "parentTitle", "title"].sort(),
+ "url", "tags", "description", "loadInSidebar", "parentTitle", "title", "dateAdded"].sort(),
"Should include bookmark-specific properties");
equal(item.syncId, bmk.syncId, "Sync ID should match");
equal(item.url.href, "https://example.com/", "Should return URL");
equal(item.parentSyncId, "menu", "Should return parent sync ID");
deepEqual(item.tags, ["taggy"], "Should return tags");
equal(item.description, "Bookmark description", "Should return bookmark description");
strictEqual(item.loadInSidebar, true, "Should return sidebar anno");
equal(item.parentTitle, "Bookmarks Menu", "Should return parent title");
strictEqual(item.title, "", "Should return empty title");
}
do_print("Fetch bookmark with keyword; without parent title or annos");
{
let item = yield PlacesSyncUtils.bookmarks.fetch(folderBmk.syncId);
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
- "url", "keyword", "tags", "loadInSidebar", "parentTitle", "title"].sort(),
+ "url", "keyword", "tags", "loadInSidebar", "parentTitle", "title", "dateAdded"].sort(),
"Should omit blank bookmark-specific properties");
strictEqual(item.loadInSidebar, false, "Should not load bookmark in sidebar");
deepEqual(item.tags, [], "Tags should be empty");
equal(item.keyword, "kw", "Should return keyword");
strictEqual(item.parentTitle, "", "Should include parent title even if empty");
strictEqual(item.title, "", "Should include bookmark title even if empty");
}
@@ -1780,27 +1781,27 @@ add_task(function* test_fetch() {
let item = yield PlacesSyncUtils.bookmarks.fetch(folderSep.syncId);
strictEqual(item.index, 1, "Should return separator position");
}
do_print("Fetch tag query");
{
let item = yield PlacesSyncUtils.bookmarks.fetch(tagQuery.syncId);
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
- "url", "title", "folder", "parentTitle"].sort(),
+ "url", "title", "folder", "parentTitle", "dateAdded"].sort(),
"Should include query-specific properties");
equal(item.url.href, `place:type=7&folder=${tagFolderId}`, "Should not rewrite outgoing tag queries");
equal(item.folder, "taggy", "Should return tag name for tag queries");
}
do_print("Fetch smart bookmark");
{
let item = yield PlacesSyncUtils.bookmarks.fetch(smartBmk.syncId);
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
- "url", "title", "query", "parentTitle"].sort(),
+ "url", "title", "query", "parentTitle", "dateAdded"].sort(),
"Should include smart bookmark-specific properties");
equal(item.query, "BookmarksToolbar", "Should return query name for smart bookmarks");
}
yield PlacesUtils.bookmarks.eraseEverything();
yield PlacesSyncUtils.bookmarks.reset();
});
@@ -1816,17 +1817,17 @@ add_task(function* test_fetch_livemark()
index: PlacesUtils.bookmarks.DEFAULT_INDEX,
});
PlacesUtils.annotations.setItemAnnotation(livemark.id, DESCRIPTION_ANNO,
"Livemark description", 0, PlacesUtils.annotations.EXPIRE_NEVER);
do_print("Fetch livemark");
let item = yield PlacesSyncUtils.bookmarks.fetch(livemark.guid);
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
- "description", "feed", "site", "parentTitle", "title"].sort(),
+ "description", "feed", "site", "parentTitle", "title", "dateAdded"].sort(),
"Should include livemark-specific properties");
equal(item.description, "Livemark description", "Should return description");
equal(item.feed.href, site + "/feed/1", "Should return feed URL");
equal(item.site.href, site + "/", "Should return site URL");
strictEqual(item.title, "", "Should include livemark title even if empty");
} finally {
yield stopServer();
}