Bug 1337993 - Ensure we mark all current bookmarks for reupload when bookmark engine enabled state changes. r=kitcambridge
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Mon, 13 Feb 2017 14:28:06 -0500
changeset 344505 56cd74ccf2c3b9fcad92cecffaca65e30e3edc7b
parent 344504 028a63d5008ce088fd9728f82c9d1f8704577d90
child 344506 f66f556b789311697a943d0d2a717f73d8969e6c
push id31413
push usercbook@mozilla.com
push dateFri, 24 Feb 2017 10:18:46 +0000
treeherdermozilla-central@c7935d540027 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs1337993
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1337993 - Ensure we mark all current bookmarks for reupload when bookmark engine enabled state changes. r=kitcambridge MozReview-Commit-ID: 4gOmqrzUr77
services/sync/modules/engines/bookmarks.js
services/sync/tests/unit/test_bookmark_decline_undecline.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -562,18 +562,18 @@ BookmarksEngine.prototype = {
     let changes = this._modified.changes;
     Async.promiseSpinningly(PlacesSyncUtils.bookmarks.pushChanges(changes));
   },
 
   _deleteId(id) {
     this._noteDeletedId(id);
   },
 
-  resetClient() {
-    SyncEngine.prototype.resetClient.call(this);
+  _resetClient() {
+    SyncEngine.prototype._resetClient.call(this);
     Async.promiseSpinningly(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.
   _switchItemToDupe(localDupeGUID, incomingItem) {
     let newChanges = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.dedupe(
       localDupeGUID, incomingItem.id, incomingItem.parentid));
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_bookmark_decline_undecline.js
@@ -0,0 +1,104 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
+Cu.import("resource://gre/modules/BookmarkJSONUtils.jsm");
+Cu.import("resource://gre/modules/Log.jsm");
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/engines.js");
+Cu.import("resource://services-sync/engines/bookmarks.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/util.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
+
+initTestLogging("Trace");
+
+Service.engineManager.register(BookmarksEngine);
+
+function serverForFoo(engine) {
+  // The bookmarks engine *always* tracks changes, meaning we might try
+  // and sync due to the bookmarks we ourselves create! Worse, because we
+  // do an engine sync only, there's no locking - so we end up with multiple
+  // syncs running. Neuter that by making the threshold very large.
+  Service.scheduler.syncThreshold = 10000000;
+  let clientsEngine = Service.clientsEngine;
+  return serverForUsers({"foo": "password"}, {
+    meta: {
+      global: {
+        syncID: Service.syncID,
+        storageVersion: STORAGE_VERSION,
+        engines: {
+          clients: {
+            version: clientsEngine.version,
+            syncID: clientsEngine.syncID,
+          },
+          bookmarks: {
+            version: engine.version,
+            syncID: engine.syncID,
+          },
+        },
+      },
+    },
+    crypto: {
+      keys: encryptPayload({
+        id: "keys",
+        // Generate a fake default key bundle to avoid resetting the client
+        // before the first sync.
+        default: [
+          Svc.Crypto.generateRandomKey(),
+          Svc.Crypto.generateRandomKey(),
+        ],
+      }),
+    },
+    bookmarks: {}
+  });
+}
+
+// A stored reference to the collection won't be valid after disabling.
+function getBookmarkWBO(server, guid) {
+  let coll = server.user("foo").collection("bookmarks");
+  if (!coll) {
+    return null;
+  }
+  return coll.wbo(guid);
+}
+
+add_task(async function test_decline_undecline() {
+  let engine = Service.engineManager.get("bookmarks");
+  let server = serverForFoo(engine);
+  await SyncTestingInfrastructure(server);
+
+  try {
+    let bzGuid = "999999999999";
+    await PlacesSyncUtils.bookmarks.insert({
+      kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
+      syncId: bzGuid,
+      parentSyncId: "menu",
+      url: "https://bugzilla.mozilla.org",
+
+    });
+
+    ok(!getBookmarkWBO(server, bzGuid), "Shouldn't have been uploaded yet");
+    Service.sync();
+    ok(getBookmarkWBO(server, bzGuid), "Should be present on server");
+
+    engine.enabled = false;
+    Service.sync();
+    ok(!getBookmarkWBO(server, bzGuid), "Shouldn't be present on server anymore");
+
+    engine.enabled = true;
+    Service.sync();
+    ok(getBookmarkWBO(server, bzGuid), "Should be present on server again");
+
+  } finally {
+    await PlacesSyncUtils.bookmarks.reset();
+    await promiseStopServer(server);
+  }
+});
+
+function run_test() {
+  initTestLogging("Trace");
+  generateNewKeys(Service.collectionKeys);
+  run_next_test();
+}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -133,16 +133,17 @@ tags = addons
 [test_bookmark_invalid.js]
 [test_bookmark_legacy_microsummaries_support.js]
 [test_bookmark_livemarks.js]
 [test_bookmark_order.js]
 [test_bookmark_places_query_rewriting.js]
 [test_bookmark_record.js]
 [test_bookmark_smart_bookmarks.js]
 [test_bookmark_store.js]
+[test_bookmark_decline_undecline.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_bookmark_tracker.js]
 requesttimeoutfactor = 4
 [test_bookmark_validator.js]
 [test_clients_engine.js]
 [test_clients_escape.js]
 [test_extension_storage_engine.js]