--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -890,16 +890,22 @@ SyncEngine.prototype = {
get metaURL() {
return this.storageURL + "meta/global";
},
/*
* lastSync is a timestamp in server time.
*/
+ async getLastSync() {
+ return this.lastSync;
+ },
+ async setLastSync(lastSync) {
+ this.lastSync = lastSync;
+ },
get lastSync() {
return this._lastSync;
},
set lastSync(value) {
// Store the value as a string to keep floating point precision
Svc.Prefs.set(this.name + ".lastSync", value.toString());
},
resetLastSync() {
@@ -1024,36 +1030,39 @@ SyncEngine.prototype = {
// Save objects that need to be uploaded in this._modified. We also save
// the timestamp of this fetch in this.lastSyncLocal. As we successfully
// upload objects we remove them from this._modified. If an error occurs
// or any objects fail to upload, they will remain in this._modified. At
// the end of a sync, or after an error, we add all objects remaining in
// this._modified to the tracker.
this.lastSyncLocal = Date.now();
- let initialChanges;
- if (this.lastSync) {
- initialChanges = await this.pullNewChanges();
- } else {
- this._log.debug("First sync, uploading all items");
- initialChanges = await this.pullAllChanges();
- }
+ let initialChanges = await this.pullChanges();
this._modified.replace(initialChanges);
// Clear the tracker now. If the sync fails we'll add the ones we failed
// to upload back.
this._tracker.clearChangedIDs();
this._tracker.resetScore();
this._log.info(this._modified.count() +
" outgoing items pre-reconciliation");
// Keep track of what to delete at the end of sync
this._delete = {};
},
+ async pullChanges() {
+ let lastSync = await this.getLastSync();
+ if (lastSync) {
+ return this.pullNewChanges();
+ }
+ this._log.debug("First sync, uploading all items");
+ return this.pullAllChanges();
+ },
+
/**
* A tiny abstraction to make it easier to test incoming record
* application.
*/
itemSource() {
return new Collection(this.engineURL, this._recordObj, this.service);
},
@@ -1075,18 +1084,19 @@ SyncEngine.prototype = {
* request records for the IDs in chunks, to avoid exceeding URL length
* limits, then remove successfully applied records from the backlog, and
* record IDs of any records that failed to apply to retry on the next sync.
*/
async _processIncoming() {
this._log.trace("Downloading & applying server changes");
let newitems = this.itemSource();
+ let lastSync = await this.getLastSync();
- newitems.newer = this.lastSync;
+ newitems.newer = lastSync;
newitems.full = true;
let downloadLimit = Infinity;
if (this.downloadLimit) {
// Fetch new records up to the download limit. Currently, only the history
// engine sets a limit, since the history collection has the highest volume
// of changed records between syncs. The other engines fetch all records
// changed since the last sync.
@@ -1112,17 +1122,17 @@ SyncEngine.prototype = {
let count = {applied: 0, failed: 0, newFailed: 0, reconciled: 0};
let recordsToApply = [];
let failedInCurrentSync = new SerializableSet();
let oldestModified = this.lastModified;
let downloadedIDs = new Set();
// Stage 1: Fetch new records from the server, up to the download limit.
- if (this.lastModified == null || this.lastModified > this.lastSync) {
+ if (this.lastModified == null || this.lastModified > lastSync) {
let { response, records } = await newitems.getBatched(this.downloadBatchSize);
if (!response.success) {
response.failureCode = ENGINE_DOWNLOAD_FAIL;
throw response;
}
let maybeYield = Async.jankYielder();
for (let record of records) {
@@ -1159,17 +1169,17 @@ SyncEngine.prototype = {
// Stage 2: If we reached our download limit, we might still have records
// on the server that changed since the last sync. Fetch the IDs for the
// remaining records, and add them to the backlog. Note that this stage
// only runs for engines that set a download limit.
if (downloadedIDs.size == downloadLimit) {
let guidColl = this.itemSource();
- guidColl.newer = this.lastSync;
+ guidColl.newer = lastSync;
guidColl.older = oldestModified;
guidColl.sort = "oldest";
let guids = await guidColl.get();
if (!guids.success)
throw guids;
// Filtering out already downloaded IDs here isn't necessary. We only do
@@ -1177,18 +1187,19 @@ SyncEngine.prototype = {
let remainingIDs = guids.obj.filter(id => !downloadedIDs.has(id));
if (remainingIDs.length > 0) {
this.toFetch = Utils.setAddAll(this.toFetch, remainingIDs);
}
}
// Fast-foward the lastSync timestamp since we have backlogged the
// remaining items.
- if (this.lastSync < this.lastModified) {
- this.lastSync = this.lastModified;
+ if (lastSync < this.lastModified) {
+ lastSync = this.lastModified;
+ await this.setLastSync(lastSync);
}
// Stage 3: Backfill records from the backlog, and those that failed to
// decrypt or apply during the last sync. We only backfill up to the
// download limit, to prevent a large backlog for one engine from blocking
// the others. We'll keep processing the backlog on subsequent engine syncs.
let failedInPreviousSync = this.previousFailed;
let idsToBackfill = Array.from(
@@ -1240,18 +1251,19 @@ SyncEngine.prototype = {
failedInBackfill.push(...failedToApply);
count.failed += failedToApply.length;
count.applied += backfilledRecordsToApply.length;
this.toFetch = Utils.setDeleteAll(this.toFetch, ids);
this.previousFailed = Utils.setAddAll(this.previousFailed, failedInBackfill);
- if (this.lastSync < this.lastModified) {
- this.lastSync = this.lastModified;
+ if (lastSync < this.lastModified) {
+ lastSync = this.lastModified;
+ await this.setLastSync(lastSync);
}
}
count.newFailed = 0;
for (let item of this.previousFailed) {
if (!failedInPreviousSync.has(item)) {
++count.newFailed;
}
@@ -1619,16 +1631,17 @@ SyncEngine.prototype = {
if (modifiedIDs.size) {
this._log.trace("Preparing " + modifiedIDs.size +
" outgoing records");
counts.sent = modifiedIDs.size;
let failed = [];
let successful = [];
+ let lastSync = await this.getLastSync();
let handleResponse = async (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) {
this._log.debug("Uploading records failed: " + resp);
resp.failureCode = resp.status == 412 ? ENGINE_BATCH_INTERRUPTED : ENGINE_UPLOAD_FAIL;
throw resp;
@@ -1637,41 +1650,44 @@ SyncEngine.prototype = {
// Update server timestamp from the upload.
failed = failed.concat(Object.keys(resp.obj.failed));
successful = successful.concat(resp.obj.success);
if (batchOngoing) {
// Nothing to do yet
return;
}
- // Advance lastSync since we've finished the batch.
- let modified = resp.headers["x-weave-timestamp"];
- if (modified > this.lastSync) {
- this.lastSync = modified;
- }
+ let serverModifiedTime = parseFloat(resp.headers["x-weave-timestamp"]);
+
if (failed.length && this._log.level <= Log.Level.Debug) {
this._log.debug("Records that will be uploaded again because "
+ "the server couldn't store them: "
+ failed.join(", "));
}
counts.failed += failed.length;
for (let id of successful) {
this._modified.delete(id);
}
- await this._onRecordsWritten(successful, failed);
+ await this._onRecordsWritten(successful, failed, serverModifiedTime);
+
+ // Advance lastSync since we've finished the batch.
+ if (serverModifiedTime > lastSync) {
+ lastSync = serverModifiedTime;
+ await this.setLastSync(lastSync);
+ }
// clear for next batch
failed.length = 0;
successful.length = 0;
};
- let postQueue = up.newPostQueue(this._log, this.lastSync, handleResponse);
+ let postQueue = up.newPostQueue(this._log, lastSync, handleResponse);
for (let id of modifiedIDs) {
let out;
let ok = false;
try {
let { forceTombstone = false } = this._needWeakUpload.get(id) || {};
if (forceTombstone) {
out = await this._createTombstone(id);
@@ -1712,17 +1728,17 @@ SyncEngine.prototype = {
}
this._needWeakUpload.clear();
if (counts.sent || counts.failed) {
Observers.notify("weave:engine:sync:uploaded", counts, this.name);
}
},
- async _onRecordsWritten(succeeded, failed) {
+ async _onRecordsWritten(succeeded, failed, serverModifiedTime) {
// 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
async _syncFinish() {
this._log.trace("Finishing up sync");
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -1,57 +1,57 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
this.EXPORTED_SYMBOLS = ["BookmarksEngine", "PlacesItem", "Bookmark",
"BookmarkFolder", "BookmarkQuery",
- "Livemark", "BookmarkSeparator"];
+ "Livemark", "BookmarkSeparator",
+ "BufferedBookmarksEngine"];
var Cc = Components.classes;
var Ci = Components.interfaces;
var Cu = Components.utils;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://services-common/async.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/engines.js");
Cu.import("resource://services-sync/record.js");
Cu.import("resource://services-sync/util.js");
-XPCOMUtils.defineLazyModuleGetter(this, "BookmarkValidator",
- "resource://services-sync/bookmark_validator.js");
+XPCOMUtils.defineLazyModuleGetters(this, {
+ SyncedBookmarksMirror: "resource://gre/modules/SyncedBookmarksMirror.jsm",
+ BookmarkValidator: "resource://services-sync/bookmark_validator.js",
+ OS: "resource://gre/modules/osfile.jsm",
+ PlacesBackups: "resource://gre/modules/PlacesBackups.jsm",
+ PlacesSyncUtils: "resource://gre/modules/PlacesSyncUtils.jsm",
+ PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
+ Resource: "resource://services-sync/resource.js",
+});
+
XPCOMUtils.defineLazyGetter(this, "PlacesBundle", () => {
return Services.strings.createBundle("chrome://places/locale/places.properties");
});
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
- "resource://gre/modules/PlacesUtils.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesSyncUtils",
- "resource://gre/modules/PlacesSyncUtils.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesBackups",
- "resource://gre/modules/PlacesBackups.jsm");
-const ANNOS_TO_TRACK = [PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO,
- PlacesSyncUtils.bookmarks.SIDEBAR_ANNO,
- PlacesUtils.LMANNO_FEEDURI, PlacesUtils.LMANNO_SITEURI];
+XPCOMUtils.defineLazyGetter(this, "ANNOS_TO_TRACK", () => [
+ PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO,
+ PlacesSyncUtils.bookmarks.SIDEBAR_ANNO, PlacesUtils.LMANNO_FEEDURI,
+ PlacesUtils.LMANNO_SITEURI,
+]);
-const SERVICE_NOT_SUPPORTED = "Service not supported on this platform";
const FOLDER_SORTINDEX = 1000000;
const {
SOURCE_SYNC,
SOURCE_IMPORT,
SOURCE_IMPORT_REPLACE,
SOURCE_SYNC_REPARENT_REMOVED_FOLDER_CHILDREN,
} = Ci.nsINavBookmarksService;
-const ORGANIZERQUERY_ANNO = "PlacesOrganizer/OrganizerQuery";
-const ALLBOOKMARKS_ANNO = "AllBookmarks";
-const MOBILE_ANNO = "MobileBookmarks";
-
// Roots that should be deleted from the server, instead of applied locally.
// This matches `AndroidBrowserBookmarksRepositorySession::forbiddenGUID`,
// but allows tags because we don't want to reparent tag folders or tag items
// to "unfiled".
const FORBIDDEN_INCOMING_IDS = ["pinned", "places", "readinglist"];
// Items with these parents should be deleted from the server. We allow
// children of the Places root, to avoid orphaning left pane queries and other
@@ -268,30 +268,134 @@ BookmarkSeparator.prototype = {
fromSyncBookmark(item) {
PlacesItem.prototype.fromSyncBookmark.call(this, item);
this.pos = item.index;
},
};
Utils.deferGetSet(BookmarkSeparator, "cleartext", "pos");
-this.BookmarksEngine = function BookmarksEngine(service) {
+/**
+ * The rest of this file implements two different bookmarks engines and stores.
+ * The `services.sync.engine.bookmarks.buffer` pref controls which one we use.
+ * `BaseBookmarksEngine` and `BaseBookmarksStore` define a handful of methods
+ * shared between the two implementations.
+ *
+ * `BookmarksEngine` and `BookmarksStore` pull locally changed IDs before
+ * syncing, examine every incoming record, use the default record-level
+ * reconciliation to resolve merge conflicts, and update records in Places
+ * using public APIs. This is similar to how the other sync engines work.
+ *
+ * Unfortunately, this general approach doesn't serve bookmark sync well.
+ * Bookmarks form a tree locally, but they're stored as opaque, encrypted, and
+ * unordered records on the server. The records are interdependent, with a
+ * set of constraints: each parent must know the IDs and order of its children,
+ * and a child can't appear in multiple parents.
+ *
+ * This has two important implications.
+ *
+ * First, some changes require us to upload multiple records. For example,
+ * moving a bookmark into a different folder uploads the bookmark, old folder,
+ * and new folder.
+ *
+ * Second, conflict resolution, like adding a bookmark to a folder on one
+ * device, and moving a different bookmark out of the same folder on a different
+ * device, must account for the tree structure. Otherwise, we risk uploading an
+ * incomplete tree, and confuse other devices that try to sync.
+ *
+ * Historically, the lack of durable change tracking and atomic uploads meant
+ * that we'd miss these changes entirely, or leave the server in an inconsistent
+ * state after a partial sync. Another device would then sync, download and
+ * apply the partial state directly to Places, and upload its changes. This
+ * could easily result in Sync scrambling bookmarks on both devices, and user
+ * intervention to manually undo the damage would make things worse.
+ *
+ * `BufferedBookmarksEngine` and `BufferedBookmarksStore` mitigate this by
+ * mirroring incoming bookmarks in a separate database, constructing trees from
+ * the local and remote bookmarks, and merging the two trees into a single
+ * consistent tree that accounts for every bookmark. For more information about
+ * merging, please see the explanation above `SyncedBookmarksMirror`.
+ */
+function BaseBookmarksEngine(service) {
SyncEngine.call(this, "Bookmarks", service);
-};
-BookmarksEngine.prototype = {
+}
+BaseBookmarksEngine.prototype = {
__proto__: SyncEngine.prototype,
_recordObj: PlacesItem,
- _storeObj: BookmarksStore,
_trackerObj: BookmarksTracker,
version: 2,
_defaultSort: "index",
syncPriority: 4,
allowSkippedRecord: false,
+ async _syncFinish() {
+ await SyncEngine.prototype._syncFinish.call(this);
+ await PlacesSyncUtils.bookmarks.ensureMobileQuery();
+ },
+
+ async _createRecord(id) {
+ if (this._modified.isTombstone(id)) {
+ // If we already know a changed item is a tombstone, just create the
+ // record without dipping into Places.
+ return this._createTombstone(id);
+ }
+ let record = await SyncEngine.prototype._createRecord.call(this, id);
+ if (record.deleted) {
+ // 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;
+ },
+
+ async pullAllChanges() {
+ return this.pullNewChanges();
+ },
+
+ async trackRemainingChanges() {
+ let changes = this._modified.changes;
+ await PlacesSyncUtils.bookmarks.pushChanges(changes);
+ },
+
+ _deleteId(id) {
+ this._noteDeletedId(id);
+ },
+
+ async _resetClient() {
+ await super._resetClient();
+ await PlacesSyncUtils.bookmarks.reset();
+ },
+
+ // 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);
+ },
+
+ getValidator() {
+ return new BookmarkValidator();
+ }
+};
+
+/**
+ * The original bookmarks engine. Uses an in-memory GUID map for deduping, and
+ * the default implementation for reconciling changes. Handles child ordering
+ * and deletions at the end of a sync.
+ */
+this.BookmarksEngine = function BookmarksEngine(service) {
+ BaseBookmarksEngine.apply(this, arguments);
+};
+
+BookmarksEngine.prototype = {
+ __proto__: BaseBookmarksEngine.prototype,
+ _storeObj: BookmarksStore,
+
emptyChangeset() {
return new BookmarksChangeset();
},
async _buildGUIDMap() {
let guidMap = {};
let tree = await PlacesUtils.promiseBookmarksTree("");
@@ -517,44 +621,31 @@ BookmarksEngine.prototype = {
await PlacesSyncUtils.bookmarks.markChangesAsSyncing(changes);
},
async _orderChildren() {
await this._store._orderChildren();
this._store._childrenToOrder = {};
},
- async _syncFinish() {
- await SyncEngine.prototype._syncFinish.call(this);
- await PlacesSyncUtils.bookmarks.ensureMobileQuery();
- },
-
async _syncCleanup() {
await SyncEngine.prototype._syncCleanup.call(this);
delete this._guidMap;
},
async _createRecord(id) {
- if (this._modified.isTombstone(id)) {
- // If we already know a changed item is a tombstone, just create the
- // record without dipping into Places.
- return this._createTombstone(id);
+ let record = await super._createRecord(id);
+ if (record.deleted) {
+ return record;
}
- // Create the record as usual, but mark it as having dupes if necessary.
- let record = await SyncEngine.prototype._createRecord.call(this, id);
+ // Mark the record as having dupes if necessary.
let entry = await this._mapDupe(record);
if (entry != null && entry.hasDupe) {
record.hasDupe = true;
}
- if (record.deleted) {
- // 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;
},
async _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.
@@ -564,54 +655,29 @@ BookmarksEngine.prototype = {
}
let mapped = await this._mapDupe(item);
this._log.debug(item.id + " mapped to " + mapped);
// We must return a string, not an object, and the entries in the GUIDMap
// are created via "new String()" making them an object.
return mapped ? mapped.toString() : mapped;
},
- async pullAllChanges() {
- return this.pullNewChanges();
- },
-
async pullNewChanges() {
return this._tracker.promiseChangedIDs();
},
- async trackRemainingChanges() {
- let changes = this._modified.changes;
- await PlacesSyncUtils.bookmarks.pushChanges(changes);
- },
-
- _deleteId(id) {
- this._noteDeletedId(id);
- },
-
- async _resetClient() {
- await SyncEngine.prototype._resetClient.call(this);
- await PlacesSyncUtils.bookmarks.reset();
- },
-
// Called when _findDupe returns a dupe item and the engine has decided to
// switch the existing item to the new incoming item.
async _switchItemToDupe(localDupeGUID, incomingItem) {
let newChanges = await PlacesSyncUtils.bookmarks.dedupe(localDupeGUID,
incomingItem.id,
incomingItem.parentid);
this._modified.insert(newChanges);
},
- // 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(localRecord, remoteRecord, remoteIsNewer) {
if (localRecord.type != "folder" || remoteRecord.type != "folder") {
return;
}
// Resolve child order conflicts by taking the chronologically newer list,
// then appending any missing items from the older list. This preserves the
// order of those missing items relative to each other, but not relative to
// the items that appear in the newer list.
@@ -624,28 +690,238 @@ BookmarksEngine.prototype = {
// Some of the children in `order` might have been deleted, or moved to
// other folders. `PlacesSyncUtils.bookmarks.order` ignores them.
let order = newRecord.children ?
[...newRecord.children, ...missingChildren] : missingChildren;
this._log.debug("Recording children of " + localRecord.id, order);
this._store._childrenToOrder[localRecord.id] = order;
},
+};
- getValidator() {
- return new BookmarkValidator();
+/**
+ * The buffered bookmarks engine uses a different store that stages downloaded
+ * bookmarks in a separate database, instead of writing directly to Places. The
+ * buffer handles reconciliation, so we stub out `_reconcile`, and wait to pull
+ * changes until we're ready to upload.
+ */
+this.BufferedBookmarksEngine = function BufferedBookmarksEngine() {
+ BaseBookmarksEngine.apply(this, arguments);
+};
+
+BufferedBookmarksEngine.prototype = {
+ __proto__: BaseBookmarksEngine.prototype,
+ _storeObj: BufferedBookmarksStore,
+
+ async getLastSync() {
+ let mirror = await this._store.ensureOpenMirror();
+ return mirror.getCollectionHighWaterMark();
+ },
+
+ async setLastSync(lastSync) {
+ let mirror = await this._store.ensureOpenMirror();
+ await mirror.setCollectionLastModified(lastSync);
+ // Update the pref so that reverting to the original bookmarks engine
+ // doesn't download records we've already applied.
+ super.lastSync = lastSync;
+ },
+
+ get lastSync() {
+ throw new TypeError("Use getLastSync");
+ },
+
+ set lastSync(value) {
+ throw new TypeError("Use setLastSync");
+ },
+
+ emptyChangeset() {
+ return new BufferedBookmarksChangeset();
+ },
+
+ async _processIncoming(newitems) {
+ try {
+ await super._processIncoming(newitems);
+ } finally {
+ let buf = await this._store.ensureOpenMirror();
+ let recordsToUpload = await buf.apply({
+ remoteTimeSeconds: Resource.serverTime,
+ });
+ this._modified.replace(recordsToUpload);
+ }
+ },
+
+ async _reconcile(item) {
+ return true;
+ },
+
+ async _createRecord(id) {
+ if (this._needWeakUpload.has(id)) {
+ return this._store.createRecord(id, this.name);
+ }
+ let change = this._modified.changes[id];
+ if (!change) {
+ this._log.error("Creating record for item ${id} not in strong " +
+ "changeset", { id });
+ throw new TypeError("Can't create record for unchanged item");
+ }
+ let record = this._recordFromCleartext(id, change.cleartext);
+ record.sortindex = await this._store._calculateIndex(record);
+ return record;
+ },
+
+ _recordFromCleartext(id, cleartext) {
+ let recordObj = getTypeObject(cleartext.type);
+ if (!recordObj) {
+ this._log.warn("Creating record for item ${id} with unknown type ${type}",
+ { id, type: cleartext.type });
+ recordObj = PlacesItem;
+ }
+ let record = new recordObj(this.name, id);
+ record.cleartext = cleartext;
+ return record;
+ },
+
+ async pullChanges() {
+ return {};
+ },
+
+ /**
+ * Writes successfully uploaded records back to the mirror, so that the
+ * mirror matches the server. We update the mirror before updating Places,
+ * which has implications for interrupted syncs.
+ *
+ * 1. Sync interrupted during upload; server doesn't support atomic uploads.
+ * We'll download and reapply everything that we uploaded before the
+ * interruption. All locally changed items retain their change counters.
+ * 2. Sync interrupted during upload; atomic uploads enabled. The server
+ * discards the batch. All changed local items retain their change
+ * counters, so the next sync resumes cleanly.
+ * 3. Sync interrupted during upload; outgoing records can't fit in a single
+ * batch. We'll download and reapply all records through the most recent
+ * committed batch. This is a variation of (1).
+ * 4. Sync interrupted after we update the mirror, but before cleanup. The
+ * mirror matches the server, but locally changed items retain their change
+ * counters. Reuploading them on the next sync should be idempotent, though
+ * unnecessary. If another client makes a conflicting remote change before
+ * we sync again, we may incorrectly prefer the local state.
+ * 5. Sync completes successfully. We'll update the mirror, and reset the
+ * change counters for all items.
+ */
+ async _onRecordsWritten(succeeded, failed, serverModifiedTime) {
+ let records = [];
+ for (let id of succeeded) {
+ let change = this._modified.changes[id];
+ if (!change) {
+ // TODO (Bug 1433178): Write weakly uploaded records back to the mirror.
+ this._log.info("Uploaded record not in strong changeset", id);
+ continue;
+ }
+ if (!change.synced) {
+ this._log.info("Record in strong changeset not uploaded", id);
+ continue;
+ }
+ let cleartext = change.cleartext;
+ if (!cleartext) {
+ this._log.error("Missing Sync record cleartext for ${id} in ${change}",
+ { id, change });
+ throw new TypeError("Missing cleartext for uploaded Sync record");
+ }
+ let record = this._recordFromCleartext(id, cleartext);
+ record.modified = serverModifiedTime;
+ records.push(record);
+ }
+ let buf = await this._store.ensureOpenMirror();
+ await buf.store(records, { needsMerge: false });
+ },
+
+ async _resetClient() {
+ await super._resetClient();
+ let buf = await this._store.ensureOpenMirror();
+ await buf.reset();
+ },
+
+ async finalize() {
+ await super.finalize();
+ await this._store.finalize();
+ },
+};
+
+/**
+ * The only code shared between `BookmarksStore` and `BufferedBookmarksStore`
+ * is for creating Sync records from Places items. Everything else is
+ * different.
+ */
+function BaseBookmarksStore(name, engine) {
+ Store.call(this, name, engine);
+}
+
+BaseBookmarksStore.prototype = {
+ __proto__: Store.prototype,
+
+ // Create a record starting from the weave id (places guid)
+ async createRecord(id, collection) {
+ let item = await PlacesSyncUtils.bookmarks.fetch(id);
+ if (!item) { // deleted item
+ let record = new PlacesItem(collection, id);
+ record.deleted = true;
+ return record;
+ }
+
+ let recordObj = getTypeObject(item.kind);
+ if (!recordObj) {
+ this._log.warn("Unknown item type, cannot serialize: " + item.kind);
+ recordObj = PlacesItem;
+ }
+ let record = new recordObj(collection, id);
+ record.fromSyncBookmark(item);
+
+ record.sortindex = await this._calculateIndex(record);
+
+ return record;
+ },
+
+ async _calculateIndex(record) {
+ // Ensure folders have a very high sort index so they're not synced last.
+ if (record.type == "folder")
+ return FOLDER_SORTINDEX;
+
+ // For anything directly under the toolbar, give it a boost of more than an
+ // unvisited bookmark
+ let index = 0;
+ if (record.parentid == "toolbar")
+ index += 150;
+
+ // Add in the bookmark's frecency if we have something.
+ if (record.bmkUri != null) {
+ let frecency = await PlacesSyncUtils.history.fetchURLFrecency(record.bmkUri);
+ if (frecency != -1)
+ index += frecency;
+ }
+
+ return index;
+ },
+
+ async wipe() {
+ // Save a backup before clearing out all bookmarks.
+ await PlacesBackups.create(null, true);
+ await PlacesSyncUtils.bookmarks.wipe();
}
};
-function BookmarksStore(name, engine) {
- Store.call(this, name, engine);
+/**
+ * The original store updates Places during the sync, using public methods.
+ * `BookmarksStore` implements all abstract `Store` methods, and behaves like
+ * the other stores.
+ */
+function BookmarksStore() {
+ BaseBookmarksStore.apply(this, arguments);
this._itemsToDelete = new Set();
}
BookmarksStore.prototype = {
- __proto__: Store.prototype,
+ __proto__: BaseBookmarksStore.prototype,
async itemExists(id) {
return (await this.idForGUID(id)) > 0;
},
async applyIncoming(record) {
this._log.debug("Applying record " + record.id);
let isSpecial = PlacesSyncUtils.bookmarks.ROOTS.includes(record.id);
@@ -772,93 +1048,108 @@ BookmarksStore.prototype = {
this.clearPendingDeletions();
return guidsToUpdate;
},
clearPendingDeletions() {
this._itemsToDelete.clear();
},
- // Create a record starting from the weave id (places guid)
- async createRecord(id, collection) {
- let item = await PlacesSyncUtils.bookmarks.fetch(id);
- if (!item) { // deleted item
- let record = new PlacesItem(collection, id);
- record.deleted = true;
- return record;
- }
-
- let recordObj = getTypeObject(item.kind);
- if (!recordObj) {
- this._log.warn("Unknown item type, cannot serialize: " + item.kind);
- recordObj = PlacesItem;
- }
- let record = new recordObj(collection, id);
- record.fromSyncBookmark(item);
-
- record.sortindex = await this._calculateIndex(record);
-
- return record;
- },
-
-
async GUIDForId(id) {
let guid = await PlacesUtils.promiseItemGuid(id);
return PlacesSyncUtils.bookmarks.guidToRecordId(guid);
},
async idForGUID(guid) {
// guid might be a String object rather than a string.
guid = PlacesSyncUtils.bookmarks.recordIdToGuid(guid.toString());
try {
return await PlacesUtils.promiseItemId(guid);
} catch (ex) {
return -1;
}
},
- async _calculateIndex(record) {
- // Ensure folders have a very high sort index so they're not synced last.
- if (record.type == "folder")
- return FOLDER_SORTINDEX;
+ async wipe() {
+ this.clearPendingDeletions();
+ await super.wipe();
+ }
+};
- // For anything directly under the toolbar, give it a boost of more than an
- // unvisited bookmark
- let index = 0;
- if (record.parentid == "toolbar")
- index += 150;
+/**
+ * The buffered store delegates to the mirror for staging and applying
+ * records. Unlike `BookmarksStore`, `BufferedBookmarksStore` only
+ * implements `applyIncoming`, and `createRecord` via `BaseBookmarksStore`.
+ * These are the only two methods that `BufferedBookmarksEngine` calls during
+ * download and upload.
+ *
+ * The other `Store` methods intentionally remain abstract, so you can't use
+ * this store to create or update bookmarks in Places. All changes must go
+ * through the mirror, which takes care of merging and producing a valid tree.
+ */
+function BufferedBookmarksStore() {
+ BaseBookmarksStore.apply(this, arguments);
+}
- // Add in the bookmark's frecency if we have something.
- if (record.bmkUri != null) {
- let frecency = await PlacesSyncUtils.history.fetchURLFrecency(record.bmkUri);
- if (frecency != -1)
- index += frecency;
+BufferedBookmarksStore.prototype = {
+ __proto__: BaseBookmarksStore.prototype,
+ _openMirrorPromise: null,
+
+ ensureOpenMirror() {
+ if (!this._openMirrorPromise) {
+ this._openMirrorPromise = this._openMirror().catch(err => {
+ // We may have failed to open the mirror temporarily; for example, if
+ // the database is locked. Clear the promise so that subsequent
+ // `ensureOpenMirror` calls can try to open the mirror again.
+ this._openMirrorPromise = null;
+ throw err;
+ });
}
-
- return index;
+ return this._openMirrorPromise;
},
- async wipe() {
- this.clearPendingDeletions();
- // Save a backup before clearing out all bookmarks.
- await PlacesBackups.create(null, true);
- await PlacesSyncUtils.bookmarks.wipe();
- }
+ async _openMirror() {
+ let mirrorPath = OS.Path.join(OS.Constants.Path.profileDir, "weave",
+ "bookmarks.sqlite");
+ await OS.File.makeDir(OS.Path.dirname(mirrorPath), {
+ from: OS.Constants.Path.profileDir,
+ });
+
+ return SyncedBookmarksMirror.open({
+ path: mirrorPath,
+ recordTelemetryEvent: (object, method, value, extra) => {
+ this.engine.service.recordTelemetryEvent(object, method, value,
+ extra);
+ },
+ });
+ },
+
+ async applyIncoming(record) {
+ let buf = await this.ensureOpenMirror();
+ await buf.store([record]);
+ },
+
+ async finalize() {
+ if (!this._openMirrorPromise) {
+ return;
+ }
+ let buf = await this._openMirrorPromise;
+ await buf.finalize();
+ },
};
// The bookmarks tracker is a special flower. Instead of listening for changes
// via observer notifications, it queries Places for the set of items that have
// changed since the last sync. Because it's a "pull-based" tracker, it ignores
// all concepts of "add a changed ID." However, it still registers an observer
// to bump the score, so that changed bookmarks are synced immediately.
function BookmarksTracker(name, engine) {
this._batchDepth = 0;
this._batchSawScoreIncrement = false;
- this._migratedOldEntries = false;
Tracker.call(this, name, engine);
}
BookmarksTracker.prototype = {
__proto__: Tracker.prototype,
// `_ignore` checks the change source for each observer notification, so we
// don't want to let the engine ignore all changes during a sync.
get ignoreAll() {
@@ -907,70 +1198,21 @@ BookmarksTracker.prototype = {
get changedIDs() {
throw new Error("Use promiseChangedIDs");
},
set changedIDs(obj) {
throw new Error("Don't set initial changed bookmark IDs");
},
- // Migrates tracker entries from the old JSON-based tracker to Places. This
- // is called the first time we start tracking changes.
- async _migrateOldEntries() {
- let existingIDs = await Utils.jsonLoad("changes/" + this.file, this);
- if (existingIDs === null) {
- // If the tracker file doesn't exist, we don't need to migrate, even if
- // the engine is enabled. It's possible we're upgrading before the first
- // sync. In the worst case, getting this wrong has the same effect as a
- // restore: we'll reupload everything to the server.
- this._log.debug("migrateOldEntries: Missing bookmarks tracker file; " +
- "skipping migration");
- return null;
- }
-
- if (!this._needsMigration()) {
- // We have a tracker file, but bookmark syncing is disabled, or this is
- // the first sync. It's likely the tracker file is stale. Remove it and
- // skip migration.
- this._log.debug("migrateOldEntries: Bookmarks engine disabled or " +
- "first sync; skipping migration");
- return Utils.jsonRemove("changes/" + this.file, this);
- }
-
- // At this point, we know the engine is enabled, we have a tracker file
- // (though it may be empty), and we've synced before.
- this._log.debug("migrateOldEntries: Migrating old tracker entries");
- let entries = [];
- for (let id in existingIDs) {
- let change = existingIDs[id];
- // Allow raw timestamps for backward-compatibility with changed IDs
- // persisted before bug 1274496.
- let timestamp = typeof change == "number" ? change : change.modified;
- entries.push({
- recordId: id,
- modified: timestamp * 1000,
- });
- }
- await PlacesSyncUtils.bookmarks.migrateOldTrackerEntries(entries);
- return Utils.jsonRemove("changes/" + this.file, this);
- },
-
- _needsMigration() {
- return this.engine && this.engineIsEnabled() && this.engine.lastSync > 0;
- },
-
observe: function observe(subject, topic, data) {
Tracker.prototype.observe.call(this, subject, topic, data);
switch (topic) {
case "weave:engine:start-tracking":
- if (!this._migratedOldEntries) {
- this._migratedOldEntries = true;
- Async.promiseSpinningly(this._migrateOldEntries());
- }
break;
case "bookmarks-restore-begin":
this._log.debug("Ignoring changes from importing bookmarks.");
break;
case "bookmarks-restore-success":
this._log.debug("Tracking all items on successful import.");
if (data == "json") {
@@ -1073,26 +1315,56 @@ BookmarksTracker.prototype = {
if (--this._batchDepth === 0 && this._batchSawScoreIncrement) {
this.score += SCORE_INCREMENT_XLARGE;
this._batchSawScoreIncrement = false;
}
},
onItemVisited() {}
};
-class BookmarksChangeset extends Changeset {
+/**
+ * A changeset that stores extra metadata in a change record for each ID. The
+ * engine updates this metadata when uploading Sync records, and writes it back
+ * to Places in `BaseBookmarksEngine#trackRemainingChanges`.
+ *
+ * The `synced` property on a change record means its corresponding item has
+ * been uploaded, and we should pretend it doesn't exist in the changeset.
+ */
+class BufferedBookmarksChangeset extends Changeset {
+ // Only `_reconcile` calls `getModifiedTimestamp` and `has`, and the buffered
+ // engine does its own reconciliation.
+ getModifiedTimestamp(id) {
+ throw new Error("Don't use timestamps to resolve bookmark conflicts");
+ }
- getStatus(id) {
- let change = this.changes[id];
- if (!change) {
- return PlacesUtils.bookmarks.SYNC_STATUS.UNKNOWN;
- }
- return change.status;
+ has(id) {
+ throw new Error("Don't use the changeset to resolve bookmark conflicts");
}
+ delete(id) {
+ let change = this.changes[id];
+ if (change) {
+ // Mark the change as synced without removing it from the set. We do this
+ // so that we can update Places in `trackRemainingChanges`.
+ change.synced = true;
+ }
+ }
+
+ ids() {
+ let results = new Set();
+ for (let id in this.changes) {
+ if (!this.changes[id].synced) {
+ results.add(id);
+ }
+ }
+ return [...results];
+ }
+}
+
+class BookmarksChangeset extends BufferedBookmarksChangeset {
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;
}
return Number.NaN;
@@ -1108,35 +1380,16 @@ class BookmarksChangeset extends Changes
setTombstone(id) {
let change = this.changes[id];
if (change) {
change.tombstone = true;
}
}
- delete(id) {
- let change = this.changes[id];
- if (change) {
- // Mark the change as synced without removing it from the set. We do this
- // so that we can update Places in `trackRemainingChanges`.
- change.synced = true;
- }
- }
-
- ids() {
- let results = new Set();
- for (let id in this.changes) {
- if (!this.changes[id].synced) {
- results.add(id);
- }
- }
- return [...results];
- }
-
isTombstone(id) {
let change = this.changes[id];
if (change) {
return change.tombstone;
}
return false;
}
}
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -34,17 +34,16 @@ Cu.import("resource://services-sync/stag
Cu.import("resource://services-sync/stages/declined.js");
Cu.import("resource://services-sync/status.js");
Cu.import("resource://services-sync/telemetry.js");
Cu.import("resource://services-sync/util.js");
function getEngineModules() {
let result = {
Addons: {module: "addons.js", symbol: "AddonsEngine"},
- Bookmarks: {module: "bookmarks.js", symbol: "BookmarksEngine"},
Form: {module: "forms.js", symbol: "FormEngine"},
History: {module: "history.js", symbol: "HistoryEngine"},
Password: {module: "passwords.js", symbol: "PasswordEngine"},
Prefs: {module: "prefs.js", symbol: "PrefsEngine"},
Tab: {module: "tabs.js", symbol: "TabEngine"},
ExtensionStorage: {module: "extension-storage.js", symbol: "ExtensionStorageEngine"},
};
if (Svc.Prefs.get("engine.addresses.available", false)) {
@@ -54,16 +53,27 @@ function getEngineModules() {
};
}
if (Svc.Prefs.get("engine.creditcards.available", false)) {
result.CreditCards = {
module: "resource://formautofill/FormAutofillSync.jsm",
symbol: "CreditCardsEngine",
};
}
+ if (Svc.Prefs.get("engine.bookmarks.buffer", false)) {
+ result.Bookmarks = {
+ module: "bookmarks.js",
+ symbol: "BufferedBookmarksEngine",
+ };
+ } else {
+ result.Bookmarks = {
+ module: "bookmarks.js",
+ symbol: "BookmarksEngine",
+ };
+ }
return result;
}
// A unique identifier for this browser session. Used for logging so
// we can easily see whether 2 logs are in the same browser session or
// after the browser restarted.
XPCOMUtils.defineLazyGetter(this, "browserSessionID", Utils.makeGUID);
--- a/services/sync/services-sync.js
+++ b/services/sync/services-sync.js
@@ -20,16 +20,17 @@ pref("services.sync.errorhandler.network
// Note that new engines are typically added with a default of disabled, so
// when an existing sync user gets the Firefox upgrade that supports the engine
// it starts as disabled until the user has explicitly opted in.
// The sync "create account" process typically *will* offer these engines, so
// they may be flipped to enabled at that time.
pref("services.sync.engine.addons", true);
pref("services.sync.engine.addresses", false);
pref("services.sync.engine.bookmarks", true);
+pref("services.sync.engine.bookmarks.buffer", false);
pref("services.sync.engine.creditcards", false);
pref("services.sync.engine.history", true);
pref("services.sync.engine.passwords", true);
pref("services.sync.engine.prefs", true);
pref("services.sync.engine.tabs", true);
pref("services.sync.engine.tabs.filteredUrls", "^(about:.*|resource:.*|chrome:.*|wyciwyg:.*|file:.*|blob:.*|moz-extension:.*)$");
// The addresses and CC engines might not actually be available at all.
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -33,21 +33,41 @@ async function fetchAllRecordIds() {
}
return recordIds;
}
add_task(async function setup() {
await generateNewKeys(Service.collectionKeys);
await Service.engineManager.unregister("bookmarks");
});
-add_task(async function test_delete_invalid_roots_from_server() {
+function add_bookmark_test(task) {
+ add_task(async function() {
+ _(`Running test ${task.name} with legacy bookmarks engine`);
+ let legacyEngine = new BookmarksEngine(Service);
+ await legacyEngine.initialize();
+ try {
+ await task(legacyEngine);
+ } finally {
+ await legacyEngine.finalize();
+ }
+
+ _(`Running test ${task.name} with buffered bookmarks engine`);
+ let bufferedEngine = new BufferedBookmarksEngine(Service);
+ await bufferedEngine.initialize();
+ try {
+ await task(bufferedEngine);
+ } finally {
+ await bufferedEngine.finalize();
+ }
+ });
+}
+
+add_bookmark_test(async function test_delete_invalid_roots_from_server(engine) {
_("Ensure that we delete the Places and Reading List roots from the server.");
- let engine = new BookmarksEngine(Service);
- await engine.initialize();
let store = engine._store;
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
let collection = server.user("foo").collection("bookmarks");
Svc.Obs.notify("weave:engine:start-tracking");
@@ -76,19 +96,23 @@ add_task(async function test_delete_inva
newBmk.parentid = "toolbar";
collection.insert(newBmk.id, encryptPayload(newBmk.cleartext));
deepEqual(collection.keys().sort(), ["places", "readinglist", listBmk.id, newBmk.id].sort(),
"Should store Places root, reading list items, and new bookmark on server");
await sync_engine_and_validate_telem(engine, false);
- ok(!(await store.itemExists("readinglist")), "Should not apply Reading List root");
- ok(!(await store.itemExists(listBmk.id)), "Should not apply items in Reading List");
- ok((await store.itemExists(newBmk.id)), "Should apply new bookmark");
+ await Assert.rejects(PlacesUtils.promiseItemId("readinglist"),
+ /no item found for the given GUID/, "Should not apply Reading List root");
+ await Assert.rejects(PlacesUtils.promiseItemId(listBmk.id),
+ /no item found for the given GUID/,
+ "Should not apply items in Reading List");
+ ok((await PlacesUtils.promiseItemId(newBmk.id)) > 0,
+ "Should apply new bookmark");
deepEqual(collection.keys().sort(), ["menu", "mobile", "toolbar", "unfiled", newBmk.id].sort(),
"Should remove Places root and reading list items from server; upload local roots");
} finally {
await store.wipe();
Svc.Prefs.resetBranch("");
Service.recordManager.clearCache();
await promiseStopServer(server);
@@ -117,21 +141,19 @@ add_task(async function bad_record_allID
Assert.ok(all.has("toolbar"));
_("Clean up.");
await PlacesUtils.bookmarks.eraseEverything();
await PlacesSyncUtils.bookmarks.reset();
await promiseStopServer(server);
});
-add_task(async function test_processIncoming_error_orderChildren() {
+add_bookmark_test(async function test_processIncoming_error_orderChildren(engine) {
_("Ensure that _orderChildren() is called even when _processIncoming() throws an error.");
- let engine = new BookmarksEngine(Service);
- await engine.initialize();
let store = engine._store;
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
let collection = server.user("foo").collection("bookmarks");
try {
@@ -147,16 +169,25 @@ add_task(async function test_processInco
title: "Get Firefox!",
});
let bmk2 = await PlacesUtils.bookmarks.insert({
parentGuid: folder1.guid,
url: "http://getthunderbird.com/",
title: "Get Thunderbird!",
});
+ let toolbar_record = await store.createRecord("toolbar");
+ collection.insert("toolbar", encryptPayload(toolbar_record.cleartext));
+
+ let bmk1_record = await store.createRecord(bmk1.guid);
+ collection.insert(bmk1.guid, encryptPayload(bmk1_record.cleartext));
+
+ let bmk2_record = await store.createRecord(bmk2.guid);
+ collection.insert(bmk2.guid, encryptPayload(bmk2_record.cleartext));
+
// Create a server record for folder1 where we flip the order of
// the children.
let folder1_record = await store.createRecord(folder1.guid);
let folder1_payload = folder1_record.cleartext;
folder1_payload.children.reverse();
collection.insert(folder1.guid, encryptPayload(folder1_payload));
// Create a bogus record that when synced down will provoke a
@@ -164,17 +195,17 @@ add_task(async function test_processInco
const BOGUS_GUID = "zzzzzzzzzzzz";
let bogus_record = collection.insert(BOGUS_GUID, "I'm a bogus record!");
bogus_record.get = function get() {
throw new Error("Sync this!");
};
// Make the 10 minutes old so it will only be synced in the toFetch phase.
bogus_record.modified = Date.now() / 1000 - 60 * 10;
- engine.lastSync = Date.now() / 1000 - 60;
+ await engine.setLastSync(Date.now() / 1000 - 60);
engine.toFetch = new SerializableSet([BOGUS_GUID]);
let error;
try {
await sync_engine_and_validate_telem(engine, true);
} catch (ex) {
error = ex;
}
@@ -195,35 +226,33 @@ add_task(async function test_processInco
await engine.resetClient();
Svc.Prefs.resetBranch("");
Service.recordManager.clearCache();
await PlacesSyncUtils.bookmarks.reset();
await promiseStopServer(server);
}
});
-add_task(async function test_restorePromptsReupload() {
- await test_restoreOrImport(true);
+add_bookmark_test(async function test_restorePromptsReupload(engine) {
+ await test_restoreOrImport(engine, { replace: true });
});
-add_task(async function test_importPromptsReupload() {
- await test_restoreOrImport(false);
+add_bookmark_test(async function test_importPromptsReupload(engine) {
+ await test_restoreOrImport(engine, { replace: false });
});
-// Test a JSON restore or HTML import. Use JSON if `aReplace` is `true`, or
+// Test a JSON restore or HTML import. Use JSON if `replace` is `true`, or
// HTML otherwise.
-async function test_restoreOrImport(aReplace) {
- let verb = aReplace ? "restore" : "import";
- let verbing = aReplace ? "restoring" : "importing";
- let bookmarkUtils = aReplace ? BookmarkJSONUtils : BookmarkHTMLUtils;
+async function test_restoreOrImport(engine, { replace }) {
+ let verb = replace ? "restore" : "import";
+ let verbing = replace ? "restoring" : "importing";
+ let bookmarkUtils = replace ? BookmarkJSONUtils : BookmarkHTMLUtils;
_(`Ensure that ${verbing} from a backup will reupload all records.`);
- let engine = new BookmarksEngine(Service);
- await engine.initialize();
let store = engine._store;
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
let collection = server.user("foo").collection("bookmarks");
Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup...
@@ -273,51 +302,52 @@ async function test_restoreOrImport(aRep
// Of course, there's also the Bookmarks Toolbar and Bookmarks Menu...
let wbos = collection.keys(function(id) {
return ["menu", "toolbar", "mobile", "unfiled", folder1.guid].indexOf(id) == -1;
});
Assert.equal(wbos.length, 1);
Assert.equal(wbos[0], bmk2.guid);
_(`Now ${verb} from a backup.`);
- await bookmarkUtils.importFromFile(backupFilePath, aReplace);
+ await bookmarkUtils.importFromFile(backupFilePath, replace);
let bookmarksCollection = server.user("foo").collection("bookmarks");
- if (aReplace) {
+ if (replace) {
_("Verify that we wiped the server.");
Assert.ok(!bookmarksCollection);
} else {
_("Verify that we didn't wipe the server.");
Assert.ok(!!bookmarksCollection);
}
_("Ensure we have the bookmarks we expect locally.");
- let guids = await fetchAllRecordIds();
- _("GUIDs: " + JSON.stringify([...guids]));
- let bookmarkGuids = new Map();
+ let recordIds = await fetchAllRecordIds();
+ _("GUIDs: " + JSON.stringify([...recordIds]));
+
+ let bookmarkRecordIds = new Map();
let count = 0;
- for (let guid of guids) {
+ for (let recordId of recordIds) {
count++;
let info = await PlacesUtils.bookmarks.fetch(
- PlacesSyncUtils.bookmarks.recordIdToGuid(guid));
+ PlacesSyncUtils.bookmarks.recordIdToGuid(recordId));
// Only one bookmark, so _all_ should be Firefox!
if (info.type == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
- _(`Found URI ${info.url.href} for GUID ${guid}`);
- bookmarkGuids.set(info.url.href, guid);
+ _(`Found URI ${info.url.href} for record ID ${recordId}`);
+ bookmarkRecordIds.set(info.url.href, recordId);
}
}
- Assert.ok(bookmarkGuids.has("http://getfirefox.com/"));
- if (!aReplace) {
- Assert.ok(bookmarkGuids.has("http://getthunderbird.com/"));
+ Assert.ok(bookmarkRecordIds.has("http://getfirefox.com/"));
+ if (!replace) {
+ Assert.ok(bookmarkRecordIds.has("http://getthunderbird.com/"));
}
_("Have the correct number of IDs locally, too.");
let expectedResults = ["menu", "toolbar", "mobile", "unfiled", folder1.guid,
bmk1.guid];
- if (!aReplace) {
+ if (!replace) {
expectedResults.push("toolbar", folder1.guid, bmk2.guid);
}
Assert.equal(count, expectedResults.length);
_("Sync again. This'll wipe bookmarks from the server.");
try {
await sync_engine_and_validate_telem(engine, false);
} catch (ex) {
@@ -338,40 +368,40 @@ async function test_restoreOrImport(aRep
(wbo.id != "menu") &&
(wbo.id != "toolbar") &&
(wbo.id != "unfiled") &&
(wbo.id != "mobile") &&
(wbo.parentid != "menu"));
});
let expectedFX = {
- id: bookmarkGuids.get("http://getfirefox.com/"),
+ id: bookmarkRecordIds.get("http://getfirefox.com/"),
bmkUri: "http://getfirefox.com/",
title: "Get Firefox!"
};
let expectedTB = {
- id: bookmarkGuids.get("http://getthunderbird.com/"),
+ id: bookmarkRecordIds.get("http://getthunderbird.com/"),
bmkUri: "http://getthunderbird.com/",
title: "Get Thunderbird!"
};
let expectedBookmarks;
- if (aReplace) {
+ if (replace) {
expectedBookmarks = [expectedFX];
} else {
expectedBookmarks = [expectedTB, expectedFX];
}
doCheckWBOs(bookmarkWBOs, expectedBookmarks);
_("Our old friend Folder 1 is still in play.");
let expectedFolder1 = { title: "Folder 1" };
let expectedFolders;
- if (aReplace) {
+ if (replace) {
expectedFolders = [expectedFolder1];
} else {
expectedFolders = [expectedFolder1, expectedFolder1];
}
doCheckWBOs(folderWBOs, expectedFolders);
} finally {
@@ -498,17 +528,17 @@ add_task(async function test_bookmark_gu
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "Folder 1",
});
let itemRecord = await store.createRecord(item.guid);
let itemPayload = itemRecord.cleartext;
coll.insert(item.guid, encryptPayload(itemPayload));
- engine.lastSync = 1; // So we don't back up.
+ await engine.setLastSync(1); // So we don't back up.
// Make building the GUID map fail.
let pbt = PlacesUtils.promiseBookmarksTree;
PlacesUtils.promiseBookmarksTree = function() { return Promise.reject("Nooo"); };
// Ensure that we throw when calling getGuidMap().
await engine._syncStartup();
@@ -592,41 +622,38 @@ add_task(async function test_bookmark_ta
type: "folder"
});
await store.create(record);
record.tags = ["bar"];
await store.update(record);
});
-add_task(async function test_misreconciled_root() {
+add_bookmark_test(async function test_misreconciled_root(engine) {
_("Ensure that we don't reconcile an arbitrary record with a root.");
- let engine = new BookmarksEngine(Service);
- await engine.initialize();
let store = engine._store;
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
// Log real hard for this test.
store._log.trace = store._log.debug;
engine._log.trace = engine._log.debug;
await engine._syncStartup();
// Let's find out where the toolbar is right now.
let toolbarBefore = await store.createRecord("toolbar", "bookmarks");
let toolbarIDBefore = await PlacesUtils.promiseItemId(
PlacesUtils.bookmarks.toolbarGuid);
Assert.notEqual(-1, toolbarIDBefore);
- let parentGUIDBefore = toolbarBefore.parentid;
- let parentIDBefore = await PlacesUtils.promiseItemId(
- PlacesSyncUtils.bookmarks.recordIdToGuid(parentGUIDBefore));
- Assert.notEqual(-1, parentIDBefore);
+ let parentRecordIDBefore = toolbarBefore.parentid;
+ let parentGUIDBefore = PlacesSyncUtils.bookmarks.recordIdToGuid(parentRecordIDBefore);
+ let parentIDBefore = await PlacesUtils.promiseItemId(parentGUIDBefore);
Assert.equal("string", typeof(parentGUIDBefore));
_("Current parent: " + parentGUIDBefore + " (" + parentIDBefore + ").");
let to_apply = {
id: "zzzzzzzzzzzz",
type: "folder",
title: "Bookmarks Toolbar",
@@ -634,50 +661,49 @@ add_task(async function test_misreconcil
parentName: "",
parentid: "mobile", // Why not?
children: [],
};
let rec = new FakeRecord(BookmarkFolder, to_apply);
_("Applying record.");
- store.applyIncoming(rec);
+ store.applyIncomingBatch([rec]);
// Ensure that afterwards, toolbar is still there.
// As of 2012-12-05, this only passes because Places doesn't use "toolbar" as
// the real GUID, instead using a generated one. Sync does the translation.
let toolbarAfter = await store.createRecord("toolbar", "bookmarks");
- let parentGUIDAfter = toolbarAfter.parentid;
- let parentIDAfter = await PlacesUtils.promiseItemId(
- PlacesSyncUtils.bookmarks.recordIdToGuid(parentGUIDAfter));
+ let parentRecordIDAfter = toolbarAfter.parentid;
+ let parentGUIDAfter = PlacesSyncUtils.bookmarks.recordIdToGuid(
+ parentRecordIDAfter);
+ let parentIDAfter = await PlacesUtils.promiseItemId(parentGUIDAfter);
Assert.equal((await PlacesUtils.promiseItemGuid(toolbarIDBefore)),
PlacesUtils.bookmarks.toolbarGuid);
Assert.equal(parentGUIDBefore, parentGUIDAfter);
Assert.equal(parentIDBefore, parentIDAfter);
await store.wipe();
await engine.resetClient();
await PlacesSyncUtils.bookmarks.reset();
await promiseStopServer(server);
});
-add_task(async function test_sync_dateAdded() {
+add_bookmark_test(async function test_sync_dateAdded(engine) {
await Service.recordManager.clearCache();
await PlacesSyncUtils.bookmarks.reset();
- let engine = new BookmarksEngine(Service);
- await engine.initialize();
let store = engine._store;
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
let collection = server.user("foo").collection("bookmarks");
// TODO: Avoid random orange (bug 1374599), this is only necessary
// intermittently - reset the last sync date so that we'll get all bookmarks.
- engine.lastSync = 1;
+ await engine.setLastSync(1);
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 {
--- a/services/sync/tests/unit/test_bookmark_repair.js
+++ b/services/sync/tests/unit/test_bookmark_repair.js
@@ -1,54 +1,54 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
// Tests the bookmark repair requestor and responder end-to-end (ie, without
// many mocks)
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://gre/modules/osfile.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
Cu.import("resource://services-sync/bookmark_repair.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/doctor.js");
Cu.import("resource://services-sync/service.js");
Cu.import("resource://services-sync/engines/clients.js");
Cu.import("resource://services-sync/engines/bookmarks.js");
-const LAST_BOOKMARK_SYNC_PREFS = [
- "bookmarks.lastSync",
- "bookmarks.lastSyncLocal",
-];
-
const BOOKMARK_REPAIR_STATE_PREFS = [
"client.GUID",
"doctor.lastRepairAdvance",
- ...LAST_BOOKMARK_SYNC_PREFS,
...Object.values(BookmarkRepairRequestor.PREF).map(name =>
`repairs.bookmarks.${name}`
),
];
let clientsEngine;
let bookmarksEngine;
var recordedEvents = [];
add_task(async function setup() {
+ await Service.engineManager.unregister("bookmarks");
+ await Service.engineManager.register(BufferedBookmarksEngine);
+
clientsEngine = Service.clientsEngine;
clientsEngine.ignoreLastModifiedOnProcessCommands = true;
bookmarksEngine = Service.engineManager.get("bookmarks");
await generateNewKeys(Service.collectionKeys);
Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
recordedEvents.push({ object, method, value, extra });
};
});
function checkRecordedEvents(expected, message) {
- deepEqual(recordedEvents, expected, message);
+ // Ignore event telemetry from the merger.
+ let repairEvents = recordedEvents.filter(event => event.object != "mirror");
+ deepEqual(repairEvents, expected, message);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}
// Backs up and resets all preferences to their default values. Returns a
// function that restores the preferences when called.
function backupPrefs(names) {
let state = new Map();
@@ -125,29 +125,58 @@ add_task(async function test_bookmark_re
_(`Upload ${folderInfo.guid} and ${bookmarkInfo.guid} to server`);
let validationPromise = promiseValidationDone([]);
await Service.sync();
equal(clientsEngine.stats.numClients, 2, "Clients collection should have 2 records");
await validationPromise;
checkRecordedEvents([], "Should not start repair after first sync");
_("Back up last sync timestamps for remote client");
- let restoreRemoteLastBookmarkSync = backupPrefs(LAST_BOOKMARK_SYNC_PREFS);
+ let buf = await bookmarksEngine._store.ensureOpenMirror();
+ let metaRows = await buf.db.execute(`
+ SELECT key, value FROM meta`);
+ let metaInfos = [];
+ for (let row of metaRows) {
+ metaInfos.push({
+ key: row.getResultByName("key"),
+ value: row.getResultByName("value"),
+ });
+ }
_(`Delete ${bookmarkInfo.guid} locally and on server`);
// Now we will reach into the server and hard-delete the bookmark
user.collection("bookmarks").remove(bookmarkInfo.guid);
// And delete the bookmark, but cheat by telling places that Sync did
// it, so we don't end up with a tombstone.
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
- deepEqual((await bookmarksEngine.pullNewChanges()), {},
+ deepEqual((await PlacesSyncUtils.bookmarks.pullChanges()), {},
`Should not upload tombstone for ${bookmarkInfo.guid}`);
+ // Remove the bookmark from the buffer, too.
+ let itemRows = await buf.db.execute(`
+ SELECT guid, kind, title, urlId
+ FROM items
+ WHERE guid = :guid`,
+ { guid: bookmarkInfo.guid });
+ equal(itemRows.length, 1, `Bookmark ${
+ bookmarkInfo.guid} should exist in buffer`);
+ let bufInfos = [];
+ for (let row of itemRows) {
+ bufInfos.push({
+ guid: row.getResultByName("guid"),
+ kind: row.getResultByName("kind"),
+ title: row.getResultByName("title"),
+ urlId: row.getResultByName("urlId"),
+ });
+ }
+ await buf.db.execute(`DELETE FROM items WHERE guid = :guid`,
+ { guid: bookmarkInfo.guid });
+
// sync again - we should have a few problems...
_("Sync again to trigger repair");
validationPromise = promiseValidationDone([
{"name": "missingChildren", "count": 1},
{"name": "structuralDifferences", "count": 1},
]);
await Service.sync();
await validationPromise;
@@ -199,17 +228,24 @@ add_task(async function test_bookmark_re
await remoteClientsEngine.initialize();
remoteClientsEngine.localID = remoteID;
_("Restore missing bookmark");
// Pretend Sync wrote the bookmark, so that we upload it as part of the
// repair instead of the sync.
bookmarkInfo.source = PlacesUtils.bookmarks.SOURCE_SYNC;
await PlacesUtils.bookmarks.insert(bookmarkInfo);
- restoreRemoteLastBookmarkSync();
+ await buf.db.execute(`
+ INSERT INTO items(guid, urlId, kind, title)
+ VALUES(:guid, :urlId, :kind, :title)`,
+ bufInfos);
+ await buf.db.execute(`
+ REPLACE INTO meta(key, value)
+ VALUES(:key, :value)`,
+ metaInfos);
_("Sync as remote client");
await Service.sync();
checkRecordedEvents([{
object: "processcommand",
method: "repairRequest",
value: undefined,
extra: {
@@ -345,19 +381,24 @@ add_task(async function test_repair_clie
equal(clientsEngine.stats.numClients, 2);
await validationPromise;
// Delete the bookmark localy, but cheat by telling places that Sync did
// it, so Sync still thinks we have it.
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
- // sanity check we aren't going to sync this removal.
- do_check_empty((await bookmarksEngine.pullNewChanges()));
- // sanity check that the bookmark is not there anymore
+ // Delete the bookmark from the buffer, too.
+ let buf = await bookmarksEngine._store.ensureOpenMirror();
+ await buf.db.execute(`DELETE FROM items WHERE guid = :guid`,
+ { guid: bookmarkInfo.guid });
+
+ // Ensure we won't upload a tombstone for the removed bookmark.
+ Assert.deepEqual((await PlacesSyncUtils.bookmarks.pullChanges()), {});
+ // Ensure the bookmark no longer exists in Places.
Assert.equal(null, await PlacesUtils.bookmarks.fetch(bookmarkInfo.guid));
// sync again - we should have a few problems...
_("Syncing again.");
validationPromise = promiseValidationDone([
{"name": "clientMissing", "count": 1},
{"name": "structuralDifferences", "count": 1},
]);
@@ -476,16 +517,17 @@ add_task(async function test_repair_serv
await Service.sync();
// should have 2 clients
equal(clientsEngine.stats.numClients, 2);
await validationPromise;
// Now we will reach into the server and create a tombstone for that bookmark
// but with a last-modified in the past - this way our sync isn't going to
// pick up the record.
+ _(`Adding server tombstone for ${bookmarkInfo.guid}`);
server.insertWBO("foo", "bookmarks", new ServerWBO(bookmarkInfo.guid, encryptPayload({
id: bookmarkInfo.guid,
deleted: true,
}), (Date.now() - 60000) / 1000));
// sync again - we should have a few problems...
_("Syncing again.");
validationPromise = promiseValidationDone([
--- a/services/sync/tests/unit/test_bookmark_repair_responder.js
+++ b/services/sync/tests/unit/test_bookmark_repair_responder.js
@@ -10,17 +10,19 @@ Cu.import("resource://services-sync/book
// Disable validation so that we don't try to automatically repair the server
// when we sync.
Svc.Prefs.set("engine.bookmarks.validation.enabled", false);
// stub telemetry so we can easily check the right things are recorded.
var recordedEvents = [];
function checkRecordedEvents(expected) {
- deepEqual(recordedEvents, expected);
+ // Ignore event telemetry from the merger.
+ let repairEvents = recordedEvents.filter(event => event.object != "mirror");
+ deepEqual(repairEvents, expected);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}
function getServerBookmarks(server) {
return server.user("foo").collection("bookmarks");
}
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -439,40 +439,40 @@ add_task(async function test_onItemAdded
try {
await startTracking();
_("Insert a folder using the sync API");
let syncFolderID = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder, "Sync Folder",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let syncFolderGUID = await engine._store.GUIDForId(syncFolderID);
+ let syncFolderGUID = await PlacesUtils.promiseItemGuid(syncFolderID);
await verifyTrackedItems(["menu", syncFolderGUID]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
await resetTracker();
await startTracking();
_("Insert a bookmark using the sync API");
let syncBmkID = PlacesUtils.bookmarks.insertBookmark(syncFolderID,
CommonUtils.makeURI("https://example.org/sync"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Sync Bookmark");
- let syncBmkGUID = await engine._store.GUIDForId(syncBmkID);
+ let syncBmkGUID = await PlacesUtils.promiseItemGuid(syncBmkID);
await verifyTrackedItems([syncFolderGUID, syncBmkGUID]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
await resetTracker();
await startTracking();
_("Insert a separator using the sync API");
let syncSepID = PlacesUtils.bookmarks.insertSeparator(
PlacesUtils.bookmarks.bookmarksMenuFolder,
PlacesUtils.bookmarks.getItemIndex(syncFolderID));
- let syncSepGUID = await engine._store.GUIDForId(syncSepID);
+ let syncSepGUID = await PlacesUtils.promiseItemGuid(syncSepID);
await verifyTrackedItems(["menu", syncSepGUID]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
} finally {
_("Clean up.");
await cleanup();
}
});
@@ -563,17 +563,17 @@ add_task(async function test_onItemChang
await stopTracking();
_("Insert a bookmark");
let fx_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
+ let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
_(`Firefox GUID: ${fx_guid}`);
await startTracking();
_("Reset the bookmark's added date");
// Convert to microseconds for PRTime.
let dateAdded = (Date.now() - DAY_IN_MS) * 1000;
PlacesUtils.bookmarks.setItemDateAdded(fx_id, dateAdded);
@@ -587,70 +587,36 @@ add_task(async function test_onItemChang
await verifyTrackedItems([fx_guid]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
} finally {
_("Clean up.");
await cleanup();
}
});
-add_task(async function test_onItemChanged_changeBookmarkURI() {
- _("Changes to bookmark URIs should be tracked");
-
- try {
- await stopTracking();
-
- _("Insert a bookmark");
- let bm = await PlacesUtils.bookmarks.insert({
- parentGuid: PlacesUtils.bookmarks.menuGuid,
- url: "http://getfirefox.com",
- title: "Get Firefox!"
- });
- _(`Firefox GUID: ${bm.guid}`);
-
- _("Set a tracked annotation to make sure we only notify once");
- let id = await PlacesUtils.promiseItemId(bm.guid);
- PlacesUtils.annotations.setItemAnnotation(
- id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
- PlacesUtils.annotations.EXPIRE_NEVER);
-
- await startTracking();
-
- _("Change the bookmark's URI");
- bm.url = "https://www.mozilla.org/firefox";
- bm = await PlacesUtils.bookmarks.update(bm);
-
- await verifyTrackedItems([bm.guid]);
- Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2);
- } finally {
- _("Clean up.");
- await cleanup();
- }
-});
-
add_task(async function test_onItemTagged() {
_("Items tagged using the synchronous API should be tracked");
try {
await stopTracking();
_("Create a folder");
let folder = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let folderGUID = await engine._store.GUIDForId(folder);
+ let folderGUID = await PlacesUtils.promiseItemGuid(folder);
_("Folder ID: " + folder);
_("Folder GUID: " + folderGUID);
_("Track changes to tags");
let uri = CommonUtils.makeURI("http://getfirefox.com");
let b = PlacesUtils.bookmarks.insertBookmark(
folder, uri,
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
- let bGUID = await engine._store.GUIDForId(b);
+ let bGUID = await PlacesUtils.promiseItemGuid(b);
_("New item is " + b);
_("GUID: " + bGUID);
await startTracking();
_("Tag the item");
PlacesUtils.tagging.tagURI(uri, ["foo"]);
@@ -669,22 +635,22 @@ add_task(async function test_onItemUntag
try {
await stopTracking();
_("Insert tagged bookmarks");
let uri = CommonUtils.makeURI("http://getfirefox.com");
let fx1ID = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder, uri,
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
- let fx1GUID = await engine._store.GUIDForId(fx1ID);
+ let fx1GUID = await PlacesUtils.promiseItemGuid(fx1ID);
// Different parent and title; same URL.
let fx2ID = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.toolbarFolder, uri,
PlacesUtils.bookmarks.DEFAULT_INDEX, "Download Firefox");
- let fx2GUID = await engine._store.GUIDForId(fx2ID);
+ let fx2GUID = await PlacesUtils.promiseItemGuid(fx2ID);
PlacesUtils.tagging.tagURI(uri, ["foo"]);
await startTracking();
_("Remove the tag");
PlacesUtils.tagging.untagURI(uri, ["foo"]);
await verifyTrackedItems([fx1GUID, fx2GUID]);
@@ -804,17 +770,17 @@ add_task(async function test_onItemKeywo
let folder = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
PlacesUtils.bookmarks.DEFAULT_INDEX);
_("Track changes to keywords");
let uri = CommonUtils.makeURI("http://getfirefox.com");
let b = PlacesUtils.bookmarks.insertBookmark(
folder, uri,
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
- let bGUID = await engine._store.GUIDForId(b);
+ let bGUID = await PlacesUtils.promiseItemGuid(b);
_("New item is " + b);
_("GUID: " + bGUID);
await startTracking();
_("Give the item a keyword");
PlacesUtils.bookmarks.setKeywordForBookmark(b, "the_keyword");
@@ -909,17 +875,17 @@ add_task(async function test_onItemPostD
await stopTracking();
_("Insert a bookmark");
let fx_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
+ let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
_(`Firefox GUID: ${fx_guid}`);
await startTracking();
// PlacesUtils.setPostDataForBookmark is deprecated, but still used by
// PlacesTransactions.NewBookmark.
_("Post data for the bookmark should be ignored");
await PlacesUtils.setPostDataForBookmark(fx_id, "postData");
@@ -938,17 +904,17 @@ add_task(async function test_onItemAnnoC
await stopTracking();
let folder = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
PlacesUtils.bookmarks.DEFAULT_INDEX);
_("Track changes to annos.");
let b = PlacesUtils.bookmarks.insertBookmark(
folder, CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
- let bGUID = await engine._store.GUIDForId(b);
+ let bGUID = await PlacesUtils.promiseItemGuid(b);
_("New item is " + b);
_("GUID: " + bGUID);
await startTracking();
PlacesUtils.annotations.setItemAnnotation(
b, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
PlacesUtils.annotations.EXPIRE_NEVER);
// bookmark should be tracked, folder should not.
@@ -972,34 +938,34 @@ add_task(async function test_onItemAdded
try {
await startTracking();
_("Create a new root");
let rootID = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.placesRoot,
"New root",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let rootGUID = await engine._store.GUIDForId(rootID);
+ let rootGUID = await PlacesUtils.promiseItemGuid(rootID);
_(`New root GUID: ${rootGUID}`);
_("Insert a bookmark underneath the new root");
let untrackedBmkID = PlacesUtils.bookmarks.insertBookmark(
rootID,
CommonUtils.makeURI("http://getthunderbird.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Thunderbird!");
- let untrackedBmkGUID = await engine._store.GUIDForId(untrackedBmkID);
+ let untrackedBmkGUID = await PlacesUtils.promiseItemGuid(untrackedBmkID);
_(`New untracked bookmark GUID: ${untrackedBmkGUID}`);
_("Insert a bookmark underneath the Places root");
let rootBmkID = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.placesRoot,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
- let rootBmkGUID = await engine._store.GUIDForId(rootBmkID);
+ let rootBmkGUID = await PlacesUtils.promiseItemGuid(rootBmkID);
_(`New Places root bookmark GUID: ${rootBmkGUID}`);
_("New root and bookmark should be ignored");
await verifyTrackedItems([]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 3);
} finally {
_("Clean up.");
await cleanup();
@@ -1012,17 +978,17 @@ add_task(async function test_onItemDelet
try {
await stopTracking();
_("Insert a bookmark underneath the Places root");
let rootBmkID = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.placesRoot,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
- let rootBmkGUID = await engine._store.GUIDForId(rootBmkID);
+ let rootBmkGUID = await PlacesUtils.promiseItemGuid(rootBmkID);
_(`New Places root bookmark GUID: ${rootBmkGUID}`);
await startTracking();
PlacesUtils.bookmarks.removeItem(rootBmkID);
await verifyTrackedItems([]);
// We'll still increment the counter for the removed item.
@@ -1165,24 +1131,24 @@ add_task(async function test_onItemMoved
_("Items moved via the synchronous API should be tracked");
try {
let fx_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
+ let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
_("Firefox GUID: " + fx_guid);
let tb_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder,
CommonUtils.makeURI("http://getthunderbird.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Thunderbird!");
- let tb_guid = await engine._store.GUIDForId(tb_id);
+ let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
_("Thunderbird GUID: " + tb_guid);
await startTracking();
// Moving within the folder will just track the folder.
PlacesUtils.bookmarks.moveItem(
tb_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0);
await verifyTrackedItems(["menu"]);
@@ -1301,31 +1267,31 @@ add_task(async function test_onItemDelet
try {
await stopTracking();
_("Create a folder with two children");
let folder_id = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder,
"Test folder",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let folder_guid = await engine._store.GUIDForId(folder_id);
+ let folder_guid = await PlacesUtils.promiseItemGuid(folder_id);
_(`Folder GUID: ${folder_guid}`);
let fx_id = PlacesUtils.bookmarks.insertBookmark(
folder_id,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
+ let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
_(`Firefox GUID: ${fx_guid}`);
let tb_id = PlacesUtils.bookmarks.insertBookmark(
folder_id,
CommonUtils.makeURI("http://getthunderbird.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Thunderbird!");
- let tb_guid = await engine._store.GUIDForId(tb_id);
+ let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
_(`Thunderbird GUID: ${tb_guid}`);
await startTracking();
let txn = PlacesUtils.bookmarks.getRemoveFolderTransaction(folder_id);
// We haven't executed the transaction yet.
await verifyTrackerEmpty();
@@ -1359,24 +1325,24 @@ add_task(async function test_treeMoved()
_("Moving an entire tree of bookmarks should track the parents");
try {
// Create a couple of parent folders.
let folder1_id = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder,
"First test folder",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let folder1_guid = await engine._store.GUIDForId(folder1_id);
+ let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id);
// A second folder in the first.
let folder2_id = PlacesUtils.bookmarks.createFolder(
folder1_id,
"Second test folder",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let folder2_guid = await engine._store.GUIDForId(folder2_id);
+ let folder2_guid = await PlacesUtils.promiseItemGuid(folder2_id);
// Create a couple of bookmarks in the second folder.
PlacesUtils.bookmarks.insertBookmark(
folder2_id,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
PlacesUtils.bookmarks.insertBookmark(
@@ -1408,17 +1374,17 @@ add_task(async function test_onItemDelet
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
let tb_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder,
CommonUtils.makeURI("http://getthunderbird.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Thunderbird!");
- let tb_guid = await engine._store.GUIDForId(tb_id);
+ let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
await startTracking();
// Delete the last item - the item and parent should be tracked.
PlacesUtils.bookmarks.removeItem(tb_id);
await verifyTrackedItems(["menu", tb_guid]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
@@ -1548,34 +1514,34 @@ add_task(async function test_onItemDelet
_("Removing a folder's children should track the folder and its children");
try {
let fx_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.mobileFolderId,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
+ let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
_(`Firefox GUID: ${fx_guid}`);
let tb_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.mobileFolderId,
CommonUtils.makeURI("http://getthunderbird.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Thunderbird!");
- let tb_guid = await engine._store.GUIDForId(tb_id);
+ let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
_(`Thunderbird GUID: ${tb_guid}`);
let moz_id = PlacesUtils.bookmarks.insertBookmark(
PlacesUtils.bookmarks.bookmarksMenuFolder,
CommonUtils.makeURI("https://mozilla.org"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Mozilla"
);
- let moz_guid = await engine._store.GUIDForId(moz_id);
+ let moz_guid = await PlacesUtils.promiseItemGuid(moz_id);
_(`Mozilla GUID: ${moz_guid}`);
await startTracking();
_(`Mobile root ID: ${PlacesUtils.mobileFolderId}`);
PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.mobileFolderId);
await verifyTrackedItems(["mobile", fx_guid, tb_guid]);
@@ -1590,210 +1556,43 @@ add_task(async function test_onItemDelet
_("Deleting a tree of bookmarks should track all items");
try {
// Create a couple of parent folders.
let folder1_id = PlacesUtils.bookmarks.createFolder(
PlacesUtils.bookmarks.bookmarksMenuFolder,
"First test folder",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let folder1_guid = await engine._store.GUIDForId(folder1_id);
+ let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id);
// A second folder in the first.
let folder2_id = PlacesUtils.bookmarks.createFolder(
folder1_id,
"Second test folder",
PlacesUtils.bookmarks.DEFAULT_INDEX);
- let folder2_guid = await engine._store.GUIDForId(folder2_id);
+ let folder2_guid = await PlacesUtils.promiseItemGuid(folder2_id);
// Create a couple of bookmarks in the second folder.
let fx_id = PlacesUtils.bookmarks.insertBookmark(
folder2_id,
CommonUtils.makeURI("http://getfirefox.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
+ let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
let tb_id = PlacesUtils.bookmarks.insertBookmark(
folder2_id,
CommonUtils.makeURI("http://getthunderbird.com"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Get Thunderbird!");
- let tb_guid = await engine._store.GUIDForId(tb_id);
+ let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
await startTracking();
// Delete folder2 - everything we created should be tracked.
PlacesUtils.bookmarks.removeItem(folder2_id);
await verifyTrackedItems([fx_guid, tb_guid, folder1_guid, folder2_guid]);
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 3);
} finally {
_("Clean up.");
await cleanup();
}
});
-
-add_task(async function test_skip_migration() {
- await insertBookmarksToMigrate();
-
- let originalTombstones = await PlacesTestUtils.fetchSyncTombstones();
- let originalFields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-
- let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
- "bookmarks.json");
-
- _("No tracker file");
- {
- await Utils.jsonRemove("changes/bookmarks", tracker);
- ok(!(await OS.File.exists(filePath)), "Tracker file should not exist");
-
- await tracker._migrateOldEntries();
-
- let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
- deepEqual(fields, originalFields,
- "Sync fields should not change if tracker file is missing");
- let tombstones = await PlacesTestUtils.fetchSyncTombstones();
- deepEqual(tombstones, originalTombstones,
- "Tombstones should not change if tracker file is missing");
- }
-
- _("Existing tracker file; engine disabled");
- {
- await Utils.jsonSave("changes/bookmarks", tracker, {});
- ok(await OS.File.exists(filePath),
- "Tracker file should exist before disabled engine migration");
-
- engine.disabled = true;
- await tracker._migrateOldEntries();
- engine.disabled = false;
-
- let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
- deepEqual(fields, originalFields,
- "Sync fields should not change on disabled engine migration");
- let tombstones = await PlacesTestUtils.fetchSyncTombstones();
- deepEqual(tombstones, originalTombstones,
- "Tombstones should not change if tracker file is missing");
-
- ok(!(await OS.File.exists(filePath)),
- "Tracker file should be deleted after disabled engine migration");
- }
-
- _("Existing tracker file; first sync");
- {
- await Utils.jsonSave("changes/bookmarks", tracker, {});
- ok(await OS.File.exists(filePath),
- "Tracker file should exist before first sync migration");
-
- engine.lastSync = 0;
- await tracker._migrateOldEntries();
-
- let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
- deepEqual(fields, originalFields,
- "Sync fields should not change on first sync migration");
- let tombstones = await PlacesTestUtils.fetchSyncTombstones();
- deepEqual(tombstones, originalTombstones,
- "Tombstones should not change if tracker file is missing");
-
- ok(!(await OS.File.exists(filePath)),
- "Tracker file should be deleted after first sync migration");
- }
-
- await cleanup();
-});
-
-add_task(async function test_migrate_empty_tracker() {
- _("Migration with empty tracker file");
- await insertBookmarksToMigrate();
-
- await Utils.jsonSave("changes/bookmarks", tracker, {});
-
- engine.lastSync = Date.now() / 1000;
- await tracker._migrateOldEntries();
-
- let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
- for (let field of fields) {
- equal(field.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL,
- `Sync status of migrated bookmark ${field.guid} should be NORMAL`);
- strictEqual(field.syncChangeCounter, 0,
- `Change counter of migrated bookmark ${field.guid} should be 0`);
- }
-
- let tombstones = await PlacesTestUtils.fetchSyncTombstones();
- deepEqual(tombstones, [], "Migration should delete old tombstones");
-
- let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
- "bookmarks.json");
- ok(!(await OS.File.exists(filePath)),
- "Tracker file should be deleted after empty tracker migration");
-
- await cleanup();
-});
-
-add_task(async function test_migrate_existing_tracker() {
- _("Migration with existing tracker entries");
- await insertBookmarksToMigrate();
-
- let mozBmk = await PlacesUtils.bookmarks.fetch("0gtWTOgYcoJD");
- let fxBmk = await PlacesUtils.bookmarks.fetch("0dbpnMdxKxfg");
- let mozChangeTime = Math.floor(mozBmk.lastModified / 1000) - 60;
- let fxChangeTime = Math.floor(fxBmk.lastModified / 1000) + 60;
- await Utils.jsonSave("changes/bookmarks", tracker, {
- "0gtWTOgYcoJD": mozChangeTime,
- "0dbpnMdxKxfg": {
- modified: fxChangeTime,
- deleted: false,
- },
- "3kdIPWHs9hHC": {
- modified: 1479494951,
- deleted: true,
- },
- "l7DlMy2lL1jL": 1479496460,
- });
-
- engine.lastSync = Date.now() / 1000;
- await tracker._migrateOldEntries();
-
- let changedFields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "0gtWTOgYcoJD", "0dbpnMdxKxfg");
- for (let field of changedFields) {
- if (field.guid == "0gtWTOgYcoJD") {
- ok(field.lastModified.getTime(), mozBmk.lastModified.getTime(),
- `Modified time for ${field.guid} should not be reset to older change time`);
- } else if (field.guid == "0dbpnMdxKxfg") {
- equal(field.lastModified.getTime(), fxChangeTime * 1000,
- `Modified time for ${field.guid} should be updated to newer change time`);
- }
- equal(field.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL,
- `Sync status of migrated bookmark ${field.guid} should be NORMAL`);
- ok(field.syncChangeCounter > 0,
- `Change counter of migrated bookmark ${field.guid} should be > 0`);
- }
-
- let unchangedFields = await PlacesTestUtils.fetchBookmarkSyncFields(
- "r5ouWdPB3l28", "YK5Bdq5MIqL6");
- for (let field of unchangedFields) {
- equal(field.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL,
- `Sync status of unchanged bookmark ${field.guid} should be NORMAL`);
- strictEqual(field.syncChangeCounter, 0,
- `Change counter of unchanged bookmark ${field.guid} should be 0`);
- }
-
- let tombstones = await PlacesTestUtils.fetchSyncTombstones();
- await deepEqual(tombstones, [{
- guid: "3kdIPWHs9hHC",
- dateRemoved: new Date(1479494951 * 1000),
- }, {
- guid: "l7DlMy2lL1jL",
- dateRemoved: new Date(1479496460 * 1000),
- }], "Should write tombstones for deleted tracked items");
-
- let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
- "bookmarks.json");
- ok(!(await OS.File.exists(filePath)),
- "Tracker file should be deleted after existing tracker migration");
-
- await cleanup();
-});
--- a/services/sync/tests/unit/test_telemetry.js
+++ b/services/sync/tests/unit/test_telemetry.js
@@ -177,16 +177,17 @@ add_task(async function test_uploading()
greater(ping.engines[0].outgoing[0].sent, 0);
ok(!ping.engines[0].incoming);
await PlacesUtils.bookmarks.update({
guid: bmk.guid,
title: "New Title",
});
+ await store.wipe();
await engine.resetClient();
ping = await sync_engine_and_validate_telem(engine, false);
equal(ping.engines.length, 1);
equal(ping.engines[0].name, "bookmarks");
equal(ping.engines[0].outgoing.length, 1);
ok(!!ping.engines[0].incoming);
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -14,17 +14,17 @@
"AsyncSpellCheckTestHelper.jsm": ["onSpellCheck"],
"AutoMigrate.jsm": ["AutoMigrate"],
"Battery.jsm": ["GetBattery", "Battery"],
"blocklist-clients.js": ["AddonBlocklistClient", "GfxBlocklistClient", "OneCRLBlocklistClient", "PluginBlocklistClient"],
"blocklist-updater.js": ["checkVersions", "addTestBlocklistClient"],
"bogus_element_type.jsm": [],
"bookmark_repair.js": ["BookmarkRepairRequestor", "BookmarkRepairResponder"],
"bookmark_validator.js": ["BookmarkValidator", "BookmarkProblemData"],
- "bookmarks.js": ["BookmarksEngine", "PlacesItem", "Bookmark", "BookmarkFolder", "BookmarkQuery", "Livemark", "BookmarkSeparator"],
+ "bookmarks.js": ["BookmarksEngine", "PlacesItem", "Bookmark", "BookmarkFolder", "BookmarkQuery", "Livemark", "BookmarkSeparator", "BufferedBookmarksEngine"],
"bookmarks.jsm": ["PlacesItem", "Bookmark", "Separator", "Livemark", "BookmarkFolder", "DumpBookmarks"],
"BootstrapMonitor.jsm": ["monitor"],
"browser-loader.js": ["BrowserLoader"],
"browserid_identity.js": ["BrowserIDManager", "AuthenticationError"],
"CertUtils.jsm": ["BadCertHandler", "checkCert", "readCertPrefs", "validateCert"],
"clients.js": ["ClientEngine", "ClientsRec"],
"collection_repair.js": ["getRepairRequestor", "getAllRepairRequestors", "CollectionRepairRequestor", "getRepairResponder", "CollectionRepairResponder"],
"collection_validator.js": ["CollectionValidator", "CollectionProblemData"],
